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

Visual bug with DropdownMenu.Trigger and Button when upgrading from 0.50.0 to 0.51.2 #1628

Closed
lonnelars opened this issue Mar 4, 2024 · 6 comments
Labels
🐛 bug Something isn't working

Comments

@lonnelars
Copy link

Description of the bug

When upgrading from 0.50.0 to 0.51.2, there is a visual bug with the Button component, when used as a trigger for the Dropdown.Menu. We're using the button variant "tertiary", and in 0.51.2 the background color changes to dark blue.

Steps To Reproduce

  1. Clone https://github.com/lonnelars/fd-bug
  2. Install with npm install and run app with npm run dev.
  3. Open localhost:3000 and see that the button is dark blue.
  4. Stop app.
  5. Downgrade @digdir/design-system-react to 0.50.0.
  6. Run npm install and npm run dev again.
  7. The button now has white background and light blue text.

Additional Information

No response

@lonnelars lonnelars added the 🐛 bug Something isn't working label Mar 4, 2024
@Barsnes
Copy link
Member

Barsnes commented Mar 4, 2024

Hi, this bug is due to css-specificity, which we have an open issue for here.
Since the trigger is already a Button with variant='primary' set by default, the class is always present, and takes precedence.

DropdownMenu.Trigger is already a button, so you don't need to use asChild here.
If you change to this code, it should work just fine 😄

<DropdownMenu>
  <DropdownMenu.Trigger variant="tertiary">
    Open menu
  </DropdownMenu.Trigger>
  <DropdownMenu.Content>
    <DropdownMenu.Item href="/about">About</DropdownMenu.Item>
    <DropdownMenu.Item href="/contact">Contact</DropdownMenu.Item>
  </DropdownMenu.Content>
</DropdownMenu>

@lonnelars
Copy link
Author

Thanks, I'll follow #1456 for updates.

In our actual code we need an onClick on the button, that is why we use asChild. This is just a minimal example to demonstrate the bug.

@lonnelars lonnelars closed this as not planned Won't fix, can't repro, duplicate, stale Mar 4, 2024
@Barsnes
Copy link
Member

Barsnes commented Mar 4, 2024

Thanks, I'll follow #1456 for updates.

In our actual code we need an onClick on the button, that is why we use asChild. This is just a minimal example to demonstrate the bug.

Adding onClick directly on the trigger should work, feel free to open another issue if this is not the case 😄

@lonnelars
Copy link
Author

lonnelars commented Mar 4, 2024

Sounds good, I'll do that. I have followed the documentation, which says that I need to use asChild if I want to add onClick (bottom of the page).

@Barsnes
Copy link
Member

Barsnes commented Mar 4, 2024

Sounds good, I'll do that. I have followed the documentation, which says that I need to use asChild if I want to add onClick (bottom of the page).

Looking at the code for the trigger, if you send an onClick event, our internal one should be overridden, so the documentation is correct here.
You could try to do this markup:

<DropdownMenu>
  <DropdownMenu.Trigger variant="tertiary" asChild>
    <button onClick={...}>
      Open menu
    </button>
  </DropdownMenu.Trigger>
  <DropdownMenu.Content>
    <DropdownMenu.Item href="/about">About</DropdownMenu.Item>
    <DropdownMenu.Item href="/contact">Contact</DropdownMenu.Item>
  </DropdownMenu.Content>
</DropdownMenu>

Here we don't use Designsystemets Button, so there will be no conflicting styles.
Might need to investigate how we want this to behave @mimarz

@lonnelars
Copy link
Author

The documentation says

Dersom du skal legge på funksjoner som onClick, legg det på ditt element, og legg asChild på DropdownMenu.Trigger.

In my mind, that means that I have to use asChild if I want onClick on the trigger.

Adding onClick directly on DropdownMenu.Trigger works great, so my problems are solved. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants