chore(menu): update to SHINE styles - #2046
Conversation
🦋 Changeset detectedLatest commit: 27e1da8 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
giamir
left a comment
There was a problem hiding this comment.
Great work @dancormier. I have left few comments and suggestions.
The examples for this component show radio group versions as well as a selected state. Both of these are subject to change after this PR is merged and once work is done on the radio component (SPARK-77).
Do we have a follow up ticket for updating the Menu component once Radio and Checkbox have been implemented?
Crazy thought, should we leave the MenuItemControl Svelte subcomponent out of scope for this PR since it will reuse Radio and Checkbox (do the work as part of the follow up ticket)?
giamir
left a comment
There was a problem hiding this comment.
Great work @dancormier. ❤️
I only had a small clarifying comment. Apart from that I think we should be good to merge. Thank you.
| --_me-action-bg: unset; | ||
| --_me-action-fc: var(--black-500); | ||
| --_me-item-p: var(--su8); | ||
| // Less variables for check svg fill color |
There was a problem hiding this comment.
Have you tried the mask approach I used in the navigation PR?
https://github.com/StackExchange/Stacks/pull/2043/files#diff-845a558e834104583bf3479fcdc5df7efdf518047bc7e75275acb2f99692311c
SPARK-71
This PR updates the Menu component to match updated SHINE styles (Figma) and adds a Svelte version of the component.
For the Svelte component, I made the choice to go with child components much like we do with
PostSummary. I'm not 100% on this approach and would appreciate any thoughts/opinions here.How to test
TODO before merge