feat(menu)!: migrate to core tokens & many new features

[pfulton]

Description

How and where has this been tested?

• How this was tested:
• Browser(s) and OS(s) this was tested with:

Screenshots

To-do list

• [ ] If my change impacts other components, I have tested to make sure they don't break.
• [ ] If my change impacts documentation, I have updated the documentation accordingly.
• [ ] I have read the CONTRIBUTING document.
• [ ] I have tested these changes in Windows High Contrast mode.

• [ ] This pull request is ready to merge.

[github-actions[bot]]

🚀 Deployed on https://pr-1508--spectrum-css.netlify.app

[bernhard-adobe]

@pfulton maybe we could add more screenshots to our PRs. I am running VRT here: https://spectrum-visual-regression.ci.corp.adobe.com/view/Spectrum%20CSS/job/css-vrt-test/43/

[yosevu]

This might be an issue on main since it doesn't appear to be relevant to this work. Capturing the screenshot until I find the source of the issue:

Also, is Action Menu a different component out of scope of this work? (I might answer my own question as I dig in...)

[yosevu]

Is it worth considering other spacing options to justify content when there are only two items so that there isn't such a large gap?

[yosevu]

I noticed there isn't a border around the menu or submenu and I don't see tokens in the designs, but it seems like they would give a more realistic context to the variant examples as well as to better separate the examples from the example headings e.g. "Menu with descriptions". Is this out of scope?

[yosevu]

I think a lot of my questions so far are scope related, so I will list them here:

• Are switch and multiple selection options in scope for this work?
• Is this Unavailable variant in scope? I see the contextual help example in the design, but I'm not sure if can be used for something else.

[pfulton]

I think a lot of my questions so far are scope related, so I will list them here:

* Are switch and multiple selection options in scope for this work?

* Is this Unavailable variant in scope? I see the contextual help example in the design, but I'm not sure if can be used for something else.

@yosevu:

• Switch and multiple selection are not in scope for this work, and we'll be tackling them later. Good call-out though!
• Unavailable: I'm not sure, and I asked in Jira. I'm thinking no, but we'll see. Thanks for pointing it out.

[pfulton]

I've fixed up all of the call-outs here, and I've also done some fixing up elsewhere in the docs site where these changes were affecting things. I think we're ready for a beta release.

[pfulton]

Released: @spectrum-css/[email protected]

[hunterloftis]

Here are those examples I promised of CSS that's challenging to transmogrify into a web component. I'm pulling these from this integration branch that I'm currently working on.

Theming rules where tokens are specified in the component's CSS rather than the theme's

https://github.com/adobe/spectrum-css/blob/f9f5e0b1ef1ad7c90895eb936f83b5931382b43e/components/menu/index.css#L162-L175

These rules end up disappearing with our current import tools. I think we could work around this by looking for specific selectors (like .spectrum--light) in the component CSS, and then applying them instead to sp-theme.

State that depends on an ancestor

https://github.com/adobe/spectrum-css/blob/f9f5e0b1ef1ad7c90895eb936f83b5931382b43e/components/menu/index.css#L555-L574

This gets compiled to .spectrum-Menu.is-selectable .spectrum-Menu-item.is-selected .spectrum-Menu-checkmark. In "vanilla" HTML & CSS, that relationship is maintained, but in a web component that breaks over shadow boundaries. Since the component is encapsulated, it can't tell if the checkmark <sp-icon> is contained within an <sp-menu-item> that is contained within a selectable <sp-menu>.

Our workaround for that is to recreate the rule at the level where the state is available, as a CSS var, which gets consumed inside the component.

Selectors that depend on DOM relationships

https://github.com/adobe/spectrum-css/blob/f9f5e0b1ef1ad7c90895eb936f83b5931382b43e/components/menu/index.css#L656-L663

https://github.com/adobe/spectrum-web-components/compare/hloftis/update-cssmenu-beta#diff-5ed00f38f2e20e3cf95d2b9840c848ab2da84a83546bf94daccfdbdb126acaafR39-R41

We try to match the structure, but it sometimes has to be slightly different in a web component. These are the easiest to work around though; I can either update our DOM to match, or I can update rules to be looser like this.

---

I don't know that we need to do any of these differently - we can work around all of them to one degree or another. But if there are equally-valid approaches at the CSS level that would translate more smoothly to a web component context, that would be awesome!

[lazd]

Stoked to see this one land, we're doing awful things over in Project X land to customize menu and need the --mod-* bits here to do it right.

[pfulton]

@lazd Very 🔜 !!

[pfulton]

Released updated beta: @spectrum-css/[email protected]

[najikahalsema]

Hello @pfulton , is it possible to catch this PR up to main and make a new beta release so that we have the @spectrum-css/tokens that this package requires?

[pfulton]

New beta version is: @spectrum-css/[email protected]

[najikahalsema]

Adding some notes based on what we talked about in last week's meeting (there's no rush, I just wanted to put something in the PR, my apologies for it being very late!):

• :focus and :hover states aren't getting applied to the background for menu items because the background colour settings are in the custom-*-vars files instead of the menu css files
• the margin for the spectrum-Menu-checkmark--withAdjacentText margin via -spectrum-text-to-visual-100 is 8px when it should be 10px or -spectrum-text-to-visual-300 to keep the selected item margin from narrowing
• The menu header element should have the same dimensions for padding as the menu items.
• For the UI icons like checkmark and chevron, the CSS expects SVG elements, but our icons don't apply the fill rule because it expects to work in conjunction with a color rule. Please use a color rule instead of/in conjunction with a fill rule for those icon elements. This affects the styling on states like disabled and hover.
• I don't know if this is a question or for you or if this is something I need to change on the SWC end, but if you could double check the menu divider CSS to make sure it doesn't have the divider touching the edges of the popover, that'd be great!

[jawinn]

This work is continued in the newer PR #1942