DropdownMenu v2: align component to shared styles and theme variables #50971
Description
Update: the main task behind this issue is still relevant, although the details may be out of date.
Overall list of things that need to be considered:
- border radius (2px vs 4px DropdownMenu V2 tweaks #56041 (comment))
- line height (DropdownMenu V2 tweaks #56041 (comment))
- border color (DropdownMenu V2 tweaks #56041 (comment))
- text color (main text, help text, disabled / hover / focus / pressed statuses)
- box-shadows
Currently, there are some values applied in the styles to the new DropdownMenu component that don't correspond (or can't be found) in the shared "design" config of the components package.
We should either update the new DropdownMenu component to use existing shared values (mostly colors), or discuss whether some of the component's styles should be extracted and added to the shared config.
Additionally, we should ideally update every color variable from being a hardcoded value (even if shared), to being a theme variable.
| Style | Current value | Notes | Status |
|---|---|---|---|
| menu background color | COLORS.ui.background |
Should be changed to use the theme's background color | ✅ Done |
| menu item text color | COLORS.gray[ 900 ] |
Should be changed to use the theme's foreground color | ✅ Done |
| disabled menu item text color | opacity: 0.5 |
It doesn't match the config for disabled background color and disabled text. We should either update the config, or update the component's styles. Whichever option we decide, we should probably associate those styles to a theme variable ? | ✅ Done |
| submenu trigger's arrow color when idle | opacity: 0.6 |
Should we be using a solid color instead, or is opacity ok? Whichever option we decide, we should change the value to be theme-friendly (or to add a variable in the theme provider for "secondary text color" or something similar)? | ✅ Done |
| hovered / focused / open menu item | COLORS.ui.theme (although it may change to COLORS.gray[ '100' ] |
If it changes to a light gray, should we add that as the default color for hover styles? In any case, we should use theme variables | ⏳ Updated to themed gray, although we may discuss codifying it and adding it to shared variables |
| group label text color | COLORS.gray[ 700 ] |
We should use theme variables instead | ⏳ Updated to themed gray, although we may discuss codifying it and adding it to shared variables |
| separator color | COLORS.gray[ 400 ] |
That color is currently associated with disabled border. We should either update the component to use the standard border color, or consider updating the border colors in the config. In any way, we should be using theme variables instead of hardcoded colors | ⏳ Updated to themed gray, although we may discuss codifying it and adding it to shared variables |
| menu's box shadow | box-shadow: 0.1px 4px 16.4px -0.5px rgba( 0, 0, 0, 0.1 ), 0px 5.5px 7.8px -0.3px rgba( 0, 0, 0, 0.1 ), 0px 2.7px 3.8px -0.2px rgba( 0, 0, 0, 0.1 ), 0px 0.7px 1px rgba( 0, 0, 0, 0.1 ); |
We should look into potentially standardising the box shadow that is applied across components. We could consider using the Elevation component instead, or at least add a shared box shadow style that all components can use |
✅ Done |