Fomantic-ui: Colored icons in nested list structures are not shown in their color

[eikek]

Bug Report

I'm migrating from semantic-ui and found a difference that works in an unexpected way to me. When using nested lists where an item is a link, that is a a tag, icons are not colored. When using a div they are colored. When applying to the first level (not nested), it works with both div and a.

I fixed it for me now by adding one more .icon selector to the icon to make it more specific (see workaround below).

There is a jsfiddle showing the issue.

Steps to reproduce

1. Create a nested list structure
2. use a for items and add colored icons to it

Expected result

The icons should be rendered in their defined color.

Actual result

The icons are rendered in grey.

Testcase

https://jsfiddle.net/medc42zu/1/

Screenshot (if possible)

Version

2.8.7 (using the LESS package)

Workaround

I added this to site/elements/icon.overrides (the same as default, added one more .icon):

each(@colors, {
  @color: replace(@key, '@', '');
  @c: @colors[@@color][color];
  @l: @colors[@@color][light];

  i.@{color}.icon.icon.icon.icon.icon {
    color: @c;
  }
  & when (@variationIconInverted) {
    i.inverted.@{color}.icon.icon.icon.icon.icon {
      color: @l;
    }
    i.inverted.bordered.@{color}.icon.icon.icon.icon.icon,
    i.inverted.circular.@{color}.icon.icon.icon.icon.icon {
      background-color: @c;
      color: @white;
    }
  }
})

[lubber-de]

Your fix is correct, would you like to create the PR on your own as well?

[eikek]

Yes sure – wasn't sure about this fix being correct, thanks

[lubber-de]

Yes sure – wasn't sure about this fix being correct, thanks

Just add the .icon specificity inside icon.less instead of icon.overrides

[lubber-de]

we had a similiar PR before #438

[eikek]

Ah nice, thanks for the pointers. I did it this way. Is there an easy way to quickly test it?

[lubber-de]

• build it locally and test the created icon.css
• add the additional .icon inside browsers developers tools... that's enough proof 😆

[eikek]

Thanks. Well, I just looked at your PR, this jsfiddle idea with overriding seems better :-)

[eikek]

Thanks for your guidance and this so quickly! should this be closed now that the pr is merged (it sounds like it from the pr template, but didn't happen through github)

[lubber-de]

We are closing related tickets once we release the next version