Ionic-framework: Anchors with ion-button and color are getting text and button color classes

Created on 29 Sep 2016  路  10Comments  路  Source: ionic-team/ionic-framework

The following markup:

  <a ion-button color="secondary">Button</a>

results in a button that looks like this:

screen shot 2016-09-29 at 1 55 55 pm

caused by it getting both the .text-secondary (anchor) and .button-secondary classes.

Most helpful comment

Same thing is happening on;

<a ion-item color="secondary">item</a>

All 10 comments

Didn't realize this was already created and fixed by https://github.com/driftyco/ionic/issues/8249. Leaving open to add unit tests to avoid this happening in the future.

Same thing is happening on;

<a ion-item color="secondary">item</a>

@alan-agius4 @brandyscarney maybe we should not provide [color] in <a> directly.

@brandyscarney , @manucorporat

Personally, I am not a big fan of this directive, It sort of turn primitive HTML elements into angular components and the results can be achieved using CSS classes only or in the case of Ionic utility attributes.

ex for primitive elements rather than
<span color="primary"></span>

would be:
<span color-primary></span>

Reason being is that utility attributes would be more flexible since you can apply them on any element even those that are not listed within the directive. ex: time, article, summary etc... More over, it can be applied on your grid system which is missing from this directive. Example on an ion-col.

Also, to extanded that another utility attribute can be added for the background color. ex: background-primary or bg-primary.

Proposed code:

.ios {
  @each $color-name, $color-base, $color-contrast in get-colors($colors-ios) {

    [color-#{$color-name}] {
      color: $color-base;
    }

    [background-#{$color-name}] {
      color: $color-contrast;
      background-color: $color-base;
    }

  }
}

I can create a PR if you agree with my proposal.

Thanks for the feedback @alan-agius4! The issue is we have had several people wanting to add a color based on a data set or variable, so with an attribute you could add it based on some expression:

<a [attr.color-primary]="isMd ? '' : null">My Link</a>

However, if you don't know the color that you want is primary you can't add the attribute (easily). For example, if you were looping through items and each one had a myColor property that you wanted to use as the color which evaluated to primary or secondary. With our current syntax, you could write something like:

<a [color]="item.myColor">My Link</a>

I'm not sure of the best solution for this. Looping in @adamdbradley for discussion.

Conviencer me there :) probably the neatest approach is what you suggested earlier, to add a unique selector for anchor tags.

Ex: <a ion-link color="primary">link</a>

Unless you want to consider adding Utility CSS Classes in your framework, class name can be text-primary, color-primary or simply primary

<a [ngClass]="'primary'">link</a> // string
<a [ngClass]="item.myColor">link</a> // in loop
<a [ngClass]="{'primary': isMd}">link</a>` //conditional

I'm thinking that may be the best solution. With a custom directive for anchor tags we wouldn't be automatically styling anchors to have the Ionic look. Adam and I are going to discuss this some more and I'll update the issue after. :)

The same problem here.

Submitted a pull request for this here: https://github.com/driftyco/ionic/pull/9096

Basically it adds an attribute called ion-text that should be added to any HTML element in order to use the color input. All of the other selectors are still there with a deprecation warning if you are not using the ion-text attribute and ion-item, ion-fab and ion-button are excluded from the a tag.

cc @alan-agius4 let me know what you think!

@brandyscarney, Its good :) much clean than the current

Was this page helpful?
0 / 5 - 0 ratings

Related issues

fdnhkj picture fdnhkj  路  3Comments

danbucholtz picture danbucholtz  路  3Comments

SebastianGiro picture SebastianGiro  路  3Comments

brandyscarney picture brandyscarney  路  3Comments

manucorporat picture manucorporat  路  3Comments