Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ionic 2 Alert Bug: setCssClass overrides existing css classes #6835

Closed
ch-weiss opened this issue Jun 10, 2016 · 8 comments
Closed

Ionic 2 Alert Bug: setCssClass overrides existing css classes #6835

ch-weiss opened this issue Jun 10, 2016 · 8 comments
Assignees
Milestone

Comments

@ch-weiss
Copy link

ch-weiss commented Jun 10, 2016

Short description of the problem:

The Alert method setCssClass does not add a cssClass as documented but sets it and overrides existing css classes that have beeen set while creating the Alert (or that have been passed on by Selections)

What behavior are you expecting?

The method works as documented and adds cssClasses without overriding existing ones. Maybe the name of the method should be changed to addCssClass to make things more clear.

Steps to reproduce:

  1. Easiest way: Look at the implementation of the Alert method setCssClass
  2. Alternatively: Create an Alert with an cssClass given
  3. Look ath the Dom. The css class does not appear in the Dom because it gets overriden with the default css classes of an Alert (via the method setCssClass)

Which Ionic Version? 2.x

@ch-weiss ch-weiss changed the title Ionic 2 Alert Bug: setCssClass overrides existing cssClasses Ionic 2 Alert Bug: setCssClass overrides existing css classes Jun 10, 2016
@brandyscarney brandyscarney added this to the 2.0.0-beta.11 milestone Jun 24, 2016
@brandyscarney brandyscarney self-assigned this Jun 24, 2016
@jgw96
Copy link
Contributor

jgw96 commented Jun 30, 2016

Hello, thanks for opening an issue with us! Would you be able to provide a plunker that demonstrates this issue? Thanks for using Ionic!

@jgw96 jgw96 added the needs: reply the issue needs a response from the user label Jun 30, 2016
@ch-weiss
Copy link
Author

ch-weiss commented Jul 1, 2016

Thank you for your interest. I think in this context it makes more sense to have a look at the current implementation of the method setCssClass in Alert which is supposed to add a cssClass but it really sets it instead of adding:

257 /**
258 * @param {string} cssClass CSS class name to add to the alert's outer wrapper
259 */
260 setCssClass(cssClass: string) {
261 this.data.cssClass = cssClass;
262 }

@Ionitron Ionitron removed the needs: reply the issue needs a response from the user label Jul 1, 2016
@guillenotfound
Copy link
Contributor

This seems to be fixed by: 215c6d8.

@brandyscarney
Copy link
Member

So what I'm seeing with the latest is:

let alert = this.alertCtrl.create({
  title: 'Alert',
  buttons: ['Cancel', 'OK'],
  cssClass: 'my-alert'
});

alert.present(alert);

This creates an alert with the classes: my-alert alert-cmp

and if I call the function to set the class like this:

let alert = this.alertCtrl.create({
  title: 'Alert',
  buttons: ['Cancel', 'OK'],
  cssClass: 'my-alert'
});

alert.setCssClass('custom-alert');
alert.present(alert);

then the following classes are added: custom-alert alert-cmp

I think this function is doing what it should, but maybe you are asking for a function that just adds classes @ch-weiss?

@ch-weiss
Copy link
Author

ch-weiss commented Jul 8, 2016

@brandyscarney: Thank you for the explanation. The problem I identified is in the context of selections. Selections offer the possibility to specify a css class parameter which is passed on to the generated alert. I did this but in the dom I did not find the specified css class. So I debugged what happens and found that during alert generation the css class is passed on to the alert. Then the standard alert classes of Ionic are set using the method setCssClass of alert. This setting overrides the css class that came from the selection. When I change the implementation of the method setCssClass to add the css class parameter given to it instead of setting it all works fine and the csss class parameter that comes from the selection is in the dom as well as Ionics standard css class for alerts
So maybe the setCssClass method works as intended and there is an issue in the alert generation from selections. But I did not check yet whether this issue is still there in beta 10.

@brandyscarney
Copy link
Member

@ch-weiss Thanks for the description! I'm able to see the problem using selects.

@brandyscarney
Copy link
Member

This will be fixed in the beta 11 release. Thanks for the issue!

@ch-weiss
Copy link
Author

ch-weiss commented Jul 8, 2016

@brandyscarney Thanks for your attention and to the whole team for the awesome work!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants