[v12] feat(ui-popover,ui-options,ui-menu): rework Menu and Options#2391
[v12] feat(ui-popover,ui-options,ui-menu): rework Menu and Options#2391
Conversation
|
| background: componentTheme.selectedBackground, | ||
| color: componentTheme.highlightedLabelColor | ||
| color: componentTheme.selectedLabelColor |
There was a problem hiding this comment.
I changed this @hajnaldo as selectedLabelColor was not used before
| overflowX="hidden" | ||
| overflowY="hidden" |
There was a problem hiding this comment.
This was chaged by @adamlobler but it clipped the focus ring of the menu so I reverted this
| import { withStyle } from '@instructure/emotion' | ||
|
|
||
| import generateStyles from './styles' | ||
| import generateComponentTheme from './theme' |
There was a problem hiding this comment.
I did not delete the theme.ts files of Options yet because Drilldown uses them. I will remove them in the Drilldown rework PR.
ToMESSKa
force-pushed
the
INSTUI-4796-menu-and-option-dropdown
branch
2 times, most recently
from
cdefaf3 to
0476c97
Compare
ToMESSKa
changed the title
ToMESSKa
force-pushed
the
INSTUI-4796-menu-and-option-dropdown
branch
from
0476c97 to
cef00c7
Compare
|
@hajnaldo I implemented the new selected, highlighted and selected+highlighted coloring of the Menu.Items, please check. |
matyasf
left a comment
matyasf
left a comment
There was a problem hiding this comment.
looks good, it looks like the design to me and all the tokens are used. FYI design might make some changes about how a selected item looks
hajnaldo
left a comment
hajnaldo
left a comment
There was a problem hiding this comment.
Menu looks and works as expected from design pov.
Some comments regarding the Options:
- I think the "highlighted disabled" combo is not valid, because highlighted here means hover and a disabled item cannot be hovered
- odd behavior observed on the showcase: when clicking on an Option, it stays with the highlighted color, and after leaving the option, the background changes to the selected color
|
@ToMESSKa , pls disregard my comment about the Option, it works as expected. |
3 participants
INSTUI-4796
ISSUE:
Menu and Options needs to be migrated to the new theming system
TEST PLAN: