[EuiIcon] Fix two-tone icons to inherit parent color when nested in specific components#4760
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
cchaos
left a comment
There was a problem hiding this comment.
Code looks great. I think the inherit prop is extremely clear what it will do (even if it's the same a default for normal icons).
My only comments are around documentation. We need it to be very clear which components force their color inheritance. I think that's just in the form of adding an example to each of the other components affected. Like changing one of these context menu items to use the app icon.
| // The app icon only gets the .euiIcon--app class if no color is passed or if color="default" is passed | ||
| 'euiIcon--app': isAppIcon && !appIconHasColor, |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
cchaos
left a comment
There was a problem hiding this comment.
🎉 Thanks for updating all those other examples. It'll be super clear now how those icons are handled, and even helps us continue to test those other usages. Just had a couple final things.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
|
Thanks @cchaos. 🙏🏽 I went through the final suggestions. Can you take another look? |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4760/ |
…pecific components (#4760) * Making app icons to inherit parent color when nested in specific components * Improving example * CL * Improving comment msg * Adding color inherit * Checking if icon has color applied * Color inherit example * Addressing PR review * WIP * CL and list group icon props Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
Summary
Closes #4739 and closes #4755.
This PR fixes a bug in EuiIcon where two-tone icons were not inheriting their parent color when nested in specific components. The two-tone colors would always prevail and consumers would expect these icons to behave more similarly to normal glyphs.
So to make the icon behave similarly to normal glyphs inside specific components like EuiBadge, EuiCallOut, or EuiContextMenu we pass
color="inherit"to enforce two-tone icons to inherit the parent colors and this way they don't get the.euiIcon--app. So they just behave as consumers expect.As we can see in the following image, these icons now behave as expected. They inherit their parent color. So if there's a disabled state, a link color, the icon just inherits it.
Checklist
Checked in mobileChecked Code Sandbox works for the any docs examplesChecked for breaking changes and labeled appropriatelyChecked for accessibility including keyboard-only and screenreader modes