patternfly / patternfly-org

Home of patternfly.org
https://www.patternfly.org
MIT License
70 stars 137 forks source link

icon color guidance #2507

Open ajacobs21e opened 3 years ago

ajacobs21e commented 3 years ago

https://www.patternfly.org/v4/guidelines/icons#icon-colors links to https://www.patternfly.org/v4/guidelines/colors#text-and-icons which doesn’t really explain what colors to use for icons. Some guidance which would be helpful to document is when icons should be black or grey. From Michael: Actionable icons (can click/tap them to perform an action) are a dark grey which turn black on hover. Non-actionable (decorative, including icons with tooltips or icons within an actionable element such as button) are always black.

This may be a separate issue @doruskova that some icons in the sketch library are grey and some are black. Maybe if we made them all the same color it would help?

doruskova commented 3 years ago

@ajacobs21e Agree that the guideline doesn't explain what colors to use. I don't have any problem changing the color to black even if I understand why some of them are grey. This change makes sense to me because sometimes you have to use a different icon, not from the actionable (grey) section, and make it grey anyway. I'm just a little bit concerned about what's gonna happen after this change :D I'll try to make a change offline to see the result. It'll definitely take some time to review all symbols and check the template file as well. Also, I'm wondering if we can change the color of the dynamic icon directly to black instead of having an additional color layer applied to this icon.

ajacobs21e commented 3 years ago

@doruskova the dynamic icon doesn't make sense to use anymore I don't think. @cshinn made it because of some limitation in Sketch where we couldn't use color layers or something. Maybe Chris remembers the exact problem. But whatever it is, that problem is gone now. I think it's just more confusing to have two ways to add an icon.