carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.7k stars 1.79k forks source link

[Bug]: `OverflowMenu` styles overridden by `Button` #16836

Closed cuppajoey closed 3 weeks ago

cuppajoey commented 2 months ago

Package

@carbon/react

Browser

Chrome

Package version

v1.60.0

React version

v18.2.0

Description

When importing component-level stylesheets, rather than the full carbon stylesheet, Button's styles will override OverflowMenu's if the button stylesheet is imported later.

image

This is because OverflowMenu does not set the rendered IconButton's kind prop which defaults to primary and the style selectors have an equal specificity (.cds--btn--primary vs .cds--overflow-menu).

💡 Possible solution

The documentation for OverflowMenu states that it should only render a ghost button style. If that's true, we should set the kind="ghost" which would prevent primary button styles from overriding it.

At the very least, we should set "ghost" as a default value and allow consumers to change it.

Reproduction/example

https://stackblitz.com/edit/github-utkywz?file=src%2FApp.jsx,src%2Fbutton.scss,src%2Foverflow.scss

Steps to reproduce

  1. Render a Button and an OverflowMenu with default values.
  2. Import @carbon/styles/scss/components/overflow-menu.
  3. Later import @carbon/styles/scss/components/button.
  4. Observe the OverflowMenu adopt Button's primary styles.

Suggested Severity

Severity 4 = Unrelated to a user task, has a workaround or does not need a workaround.

Application/PAL

No response

Code of Conduct

Gururajj77 commented 2 months ago

Hi @cuppajoey , can you please check this stackblitz? I just added the index.scss with use @carbon/react. https://stackblitz.com/edit/github-utkywz-ag6nfs?file=src%2FApp.jsx,src%2Findex.scss,src%2Fmain.jsx

cuppajoey commented 2 months ago

Hi @cuppajoey , can you please check this stackblitz? I just added the index.scss with use @carbon/react. https://stackblitz.com/edit/github-utkywz-ag6nfs?file=src%2FApp.jsx,src%2Findex.scss,src%2Fmain.jsx

@Gururajj77 I understand that it works if you import Carbon's entire stylesheet, but that's not the issue. Carbon supports importing individual component stylesheets for consumers who don't want the entire bundle. For those consumers the import order causes an issue, as demonstrated in the original StackBlitz.

tay1orjones commented 2 months ago

Yep, I think we should do both:

  1. Update OverflowMenu to set kind="ghost"
  2. Fix the specificity issue when not using the central entrypoint so this is less likely to happen in other framework variants. Using :not() might be helpful.