Outline Buttons Get Wrong Color and Border

[calendee]

Short description of the problem:

When a button with the outline property has the outline property conditionally removed, it becomes a solid color button with the wrong color applied. Instead of being the designated color, it uses the primary color. Later, if the button has the outline property conditionally applied again, the button correctly switches to the designated color but with the wrong border color. The applied border color is primary.

Additionally, as noted in #8845 , once a button has the outline property conditionally re-applied, it will never reappear as an outline button. It will always be a solid color button.

NOTE: This problem began with RC1/2. In Beta 11 the problem did not exist.

What behavior are you expecting?

When a button has the outline property conditionally removed, it should take on it's designated color class instead of primary. When the outline class is reapplied, the border should be the designated color instead of primary.

Steps to reproduce:
Open this Plunker: http://plnkr.co/edit/c0ybwYtvFTaT9dGvUje4?p=preview

1. Notice that the buttons are the correct color
2. Tap on "SECONDARY".
3. Notice that the SECONDARY button becomes the primary (blue) color
4. Notice that the DANGER button has a blue border instead of red. It should also be an "outline" button.
5. Tap on "DANGER"
6. Notice that the SECONDARY button got the correct color. but it should be an outline button and should NOT have a primary color border

Other information: (e.g. stacktraces, related issues, suggestions how to fix, stackoverflow links, forum links, etc)

Which Ionic Version? 1.x or 2.x
Ionic 2 RC1 and RC2

Plunker that shows an example of your issue

See steps to reproduce for the plunker.

Run ionic info from terminal/cmd prompt: (paste output below):

Project 1 Info:

Cordova CLI: 6.2.0
Gulp version: CLI version 3.9.1
Gulp local: Local version 3.9.1
Ionic Framework Version: 2.0.0-rc.1
Ionic CLI Version: 2.1.4
Ionic App Lib Version: 2.1.2
Ionic App Scripts Version: 0.0.38
ios-deploy version: 1.9.0
ios-sim version: 5.0.9
OS: Mac OS X El Capitan
Node Version: v6.7.0
Xcode version: Xcode 7.3.1 Build version 7D1014

Project 2 Info:
Cordova CLI: 6.2.0
Gulp version: CLI version 3.9.1
Gulp local:
Ionic Framework Version: 2.0.0-rc.2
Ionic CLI Version: 2.1.4
Ionic App Lib Version: 2.1.2
Ionic App Scripts Version: 0.0.39
ios-deploy version: 1.9.0
ios-sim version: 5.0.9
OS: Mac OS X El Capitan
Node Version: v6.7.0
Xcode version: Xcode 7.3.1 Build version 7D1014

[ncapito]

Is someone looking into/working on this issue? Don't want to duplicate efforts.

[brandyscarney]

Hey @ncapito, I haven't looked into this issue yet. Hoping to get to this one and #8845 soon!

[ncapito]

Let me know when you start. I'm fumbling around trying to figure out how you all have your dev env's setup. I think I got e2e running and now I'm diving into the source for button.

[brandyscarney]

@ncapito We have some steps here that might help: https://github.com/driftyco/ionic/blob/master/scripts/README.md#development

Specifically these steps should work: https://github.com/driftyco/ionic/blob/master/scripts/README.md#building--running-e2e-tests

Let me know if you run into problems and I can help! :)

[ncapito]

My simple test was to have a primary class, and just toggle the outline input.. So I have narrowed it down to this function.

https://github.com/driftyco/ionic/blob/master/src/components/button/button.ts#L232

  /** @private */
  _attr(type: string, attrName: string, attrValue: boolean) {
    if (type === '_style') {
      this._updateColor(this._color, isTrueProperty(attrValue));
    }

When the "outline" input is removed it updates the classes it incorrectly adds the primary class. To fix my case it's pretty simple I just invert: this._updateColor(this._color, !isTrueProperty(attrValue)); . But this breaks a lot of the e2e examples. In my opinion the best way to fix it is to narrow the responsibility of the _attr class. e.g. if I'm assigning / modifying outline i probably shouldn't be "updating" colors as well.

Just some initial thoughts. I'll probably have a proposed solution soon but I'm not familiar enough with the code to make sure i dont break anything ;)

[brandyscarney]

@ncapito Well the color needs to be updated when outline changes because the button gets a class with outline and the color. So if you have:

<button ion-button [outline]="isOutline" color="dark">Button</button>

For example let's say we're in ios mode; when isOutline evaluates to true the class added will be .button-outline-ios-dark:

<button ion-button outline="true" color="dark" class="button button-outline-ios button-outline-ios-dark">Button</button>

and when it evaluates to false it will be .button-ios-dark:

<button ion-button outline="false" color="dark" class="button button-ios button-ios-dark">Button</button>

So it needs to call the color function to make sure the old color class (with outline) gets removed and the new color class gets added. Otherwise these two classes would conflict. Hope that makes sense. :)

[ncapito]

You are correct @brandyscarney . I'm hopeful the solution below works.

  /** @private */

  _attr(type: string, attrName: string, attrValue: boolean) {

    if (type === '_style') {
      let toAdd = isTrueProperty(attrValue);
      //if we are adding the outline class remove the old color
      if(attrName === 'outline' && toAdd){
        toAdd = !toAdd;
      }
      this._updateColor(this._color, toAdd);
    }

[brandyscarney]

Awesome @ncapito. Thanks! I'll try to take a look at it today.

[brandyscarney]

Thanks @ncapito for the PR. I've released a nightly version ([email protected]) with this fix, could you and @calendee try it out and let me know if you find any issues?

npm install --save ionic-angular@nightly

Thanks!

[calendee]

@brandyscarney Can confirm the issue is fixed with the nightlies in a local repo and on Plunker.

Thanks to you and @ncapito for the rapid turnaround!

Here's an updated Plunker that shows the problem fixed in the nightlies : http://plnkr.co/edit/ClgMHjlDRtSXXejRdgaM?p=preview