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

fix(Modal): button side-effects when used within a button group #3138

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from

Conversation

romhml
Copy link
Collaborator

@romhml romhml commented Jan 18, 2025

πŸ”— Linked issue

Resolves #2978

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Fixes side-effects on button components in a modal which are rendered as if they are in a ButtonGroup if the modal is within a ButtonGroup.

The solution here is to override Button's group injection key in the Modal component. This is not ideal but solves the issue. A better solution could be to update the button groups style to only affect direct childs (https://github.com/nuxt/ui/blob/v3/src/theme/button-group.ts#L1).

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@romhml romhml requested a review from benjamincanac January 18, 2025 14:46
Copy link

cloudflare-workers-and-pages bot commented Jan 18, 2025

Deploying ui3 with Β Cloudflare Pages Β Cloudflare Pages

Latest commit: cd9f97c
Status:Β βœ…Β  Deploy successful!
Preview URL: https://15302a26.ui-6q2.pages.dev
Branch Preview URL: https://fix-modal-within-button-grou.ui-6q2.pages.dev

View logs

@romhml romhml force-pushed the fix/modal-within-button-group branch from d0e3124 to cd9f97c Compare January 18, 2025 14:53
Copy link

pkg-pr-new bot commented Jan 18, 2025

npm i https://pkg.pr.new/@nuxt/ui@3138

commit: cd9f97c

@benjamincanac
Copy link
Member

@romhml We cannot affect only direct children as this whole system has been made to handle components that have their button wrapped with a div for example 😬

@romhml
Copy link
Collaborator Author

romhml commented Jan 20, 2025

@benjamincanac isn't this case handled by the buttonGroupVariantWithRoot variant? I don't see a lot of use cases where a button that should not be affected by the ButtonGroup will be nested inside one aside for the modal component, if that's the case I think we can go with the solution here even though it's not perfect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 #1289
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The button styles in UModal body should not be affected by UButtonGroup.
2 participants