refactor: menu color tokens #13926
refactor: menu color tokens #13926
Conversation
uses a "default, low, medium, high" structure for menu backgrounds and text colors, representing different layers to stacking menu sections. Blends this with existing DS to introduce "low" "medium" and "high" background colors.
wackerow
added
design system
wackerow
requested review from
corwintines,
pettinarip and
minimalsm
as code owners
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
@wackerow great job on this. Its much cleaner and easier to reuse these colors now.
Not blocker: I agree with the naming, my only concern is the misunderstanding of usage between the old and new backgrounds (ie background-highlight vs background-high). I was thinking of something more specific like background-shade-low to make them feel like lower level background colors and avoid potential overuse. Not strong opinion either so feel free to ignore xD
src/styles/semantic-tokens.css
Outdated
| @@ -132,7 +123,11 @@ | |||
|
|
|||
| --background: var(--black); | |||
| --background-highlight: var(--gray-900); | |||
| --background-medium: var(--gray-950); /* TODO: VERIFY */ | |||
|
|
|||
| /* TODO: Add "low" "medium" and "high" to design system */ | |||
There was a problem hiding this comment.
nitpick: would avoid adding todos that are outside of the scope of the codebase as they are hard to maintain and easy to forget.
There was a problem hiding this comment.
Yes yes, sounds like we have consensus and approval on this proposal, so I'll go ahead and remove that and add notes in the DS
There was a problem hiding this comment.
LGTM! Will leave it upto you whether to address @pettinarip's comments here or separately
|
Pulled in; Removed TODO's and updated design system in Figma... Will chat about and address the naming adjustments separately. |
Description
Proposal: Migrate our "menu" tokens to tokens that are more generalizable.
Instead of declaring a whole array of specific "menu" tokens, with different level numbers, this PR suggests adding the additional shades of gray that are needed as additional
backgroundtoken shades.We currently only have
backgroundandbackground-highlightfor background tokens. This would addlow,mediumandhighbackground variants to be used as layers to any stack (ie. nested menus, nested accordions, or any other nested elements).These tokens were then used directly at the component level, updating and removing the existing usage of "menu-x-(active-)background" with the new corresponding background token.
Similar approach with text, but unsure of potential naming convention here. Went with a nested approach under "body" as "body-menu-DEFAULT/low/medium/high"
Shade update:
This also fixes the shade that was
--menu-1-active-background: var(--gray-150);to now use--background-low: var(--gray-100);in light mode:Related Issue
Design system; theming updates