Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DropdownMenu v2: align component to shared styles and theme variables #50971

Open
Tracked by #50459
ciampo opened this issue May 25, 2023 · 1 comment
Open
Tracked by #50459

DropdownMenu v2: align component to shared styles and theme variables #50971

ciampo opened this issue May 25, 2023 · 1 comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@ciampo
Copy link
Contributor

ciampo commented May 25, 2023

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:


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
@ciampo ciampo changed the title Align component's styles to shared variables, and use Theme variables where relevant DropdownMenu v2: align component's styles to shared styles and theme variables May 25, 2023
@ciampo ciampo changed the title DropdownMenu v2: align component's styles to shared styles and theme variables DropdownMenu v2: align component to shared styles and theme variables May 25, 2023
@ciampo ciampo added [Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components labels May 25, 2023
@ciampo
Copy link
Contributor Author

ciampo commented May 25, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

2 participants