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

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

Open
Rocss opened this issue Aug 30, 2024 · 5 comments
Assignees
Labels
bug Results from a bug in the CSS implementation

Comments

@Rocss
Copy link
Contributor

Rocss commented Aug 30, 2024

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 Rocss added the bug Results from a bug in the CSS implementation label Aug 30, 2024
@castastrophe
Copy link
Collaborator

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!

@lazd
Copy link
Member

lazd commented Sep 13, 2024

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.

@Rocss
Copy link
Contributor Author

Rocss commented Oct 11, 2024

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

@pfulton
Copy link
Collaborator

pfulton commented Oct 11, 2024

@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!

@Rocss
Copy link
Contributor Author

Rocss commented Oct 14, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Results from a bug in the CSS implementation
Projects
None yet
Development

No branches or pull requests

4 participants