spectrum-css icon indicating copy to clipboard operation
spectrum-css copied to clipboard

`spectrum-Menu-item` label alignment subject to cascade of text-align CSS property

Open Rocss opened this issue 1 year ago • 5 comments

Description

Whenever text-align is used on a parent element, all sp-menu-item labels inherit that instead of explicitly defining start.

Steps to reproduce

  1. Open any UI with a sp-menu
  2. Manually add text-align:center to .spectrum-Menu
  3. Observe menu items are centered now

Expected behavior

Menu items should not be centered and should perserve the initial text alignment

Screenshots

Screenshot 2024-08-30 at 16 39 08

Environment

  • Version of the impacted component(s):
  • Browser(s) and OS(s):

Additional context

Additional JIRA information: CCEX-136334

Rocss avatar Aug 30 '24 13:08 Rocss

Hi @Rocss! Thanks so much for reaching out with your issue. Can I ask what was the original goal in adding the text-align: center to the .spectrum-Menu element in your UI? May I suggest adding the text align property close to the element you are wanting to center? Perhaps if it's a heading to be centered, the text align could be set on the container for that heading, rather than a higher-level DOM element like the menu wrapper?

In our library, we would expect inline or CSS that is added by the application to always take precedence over the CSS assigned by the library. Ours is a utility but not meant to force styles or make them too difficult to override. We also offer a set of modifier custom properties that can be used to customize select parts of the UI. You can find those for Menu here: https://github.com/adobe/spectrum-css/blob/main/components/menu/metadata/mods.md.

Please do reach out if we can be of more assistance!

castastrophe avatar Sep 13 '24 14:09 castastrophe

Hey @castastrophe, this is a regression only became an issue after the Overlay V2 changes since overlays being inserted adjacent to the trigger. This bug will continue to happen in menus in similar situations. sp-menu-item should be explicit about its text alignment to address this, and if we want it to be possible to override the text alignment, that should be done through a --mod-* token, not the cascade.

It's a simple change and the team would be happy to provide a PR if y'all would accept it.

lazd avatar Sep 13 '24 18:09 lazd

@castastrophe After Larry's comment, would you please let us know if this is something you consider adding to the library?

Rocss avatar Oct 11 '24 07:10 Rocss

@Rocss thanks for following up with us on this.

With regard to what @lazd mentioned:

...the Overlay V2 changes since overlays being inserted adjacent to the trigger. This bug will continue to happen in menus in similar situations.

We'd love to understand this more and maybe see the DOM structure you're describing. Would you be able to spin up a small reproduction of the problem/use case?

I'm sure we all know how complex these menu items can get, so we want to be sure that we fully understand the impact of the change.

Thanks!

pfulton avatar Oct 11 '24 17:10 pfulton

@pfulton yep, here's one: https://studio.webcomponents.dev/edit/uxXuEl5ZlqeQqxT0dWq3/src/index.ts?p=stories

Rocss avatar Oct 14 '24 09:10 Rocss