-
Notifications
You must be signed in to change notification settings - Fork 55
fix(MenuItem): icon only active state in Teams theme #1464
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1464 +/- ##
=======================================
Coverage 73.14% 73.14%
=======================================
Files 805 805
Lines 6073 6073
Branches 1775 1794 +19
=======================================
Hits 4442 4442
Misses 1625 1625
Partials 6 6
Continue to review full report at Codecov.
|
...(iconOnly && { color: 'inherit' }), | ||
...(active && { | ||
...(iconOnly && { | ||
color: v.iconOnlyColorActive, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to be sure - could you, please, suggest where original inherit
value had resulted in suboptimal styles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In team's theme the iconOnly
Menu
should always have the brand color for the active items. The inherit color is black in default theme.
@@ -2,23 +2,27 @@ import { Menu } from '@stardust-ui/react' | |||
|
|||
interface StepsOptions { | |||
vertical?: boolean | |||
startIdx?: number | |||
endIdx?: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startItem
& endItem
?
@codepretty, could you, please, suggest what is prescribed by Teams design spec for the case @mnajdova had mentioned in the description? Thank you!
|
…olor' into fix/menu-item-icon-only-active-color
Fix #1465
Menu with icon only, didn't show any active state.
Teams default theme
Before:
After:
Teams dark theme
Before:
After:
Teams high-contrast theme
Before:
After:
Question
Not sure whether the icon in hc should be outline/filled. On focus it is filled, so I added the same logic here. I may revert this changes.