-
Notifications
You must be signed in to change notification settings - Fork 29
fix: do not wrap button contents #2214
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
base: develop
Are you sure you want to change the base?
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
…quelabs/cauldron into fix/dont-wrap-button-contents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR prevents button contents from wrapping by default, addressing issue #2190. The change ensures buttons maintain their natural width to accommodate content without wrapping unless explicitly overridden.
Changes:
- Added
min-width: max-contentCSS property to button styles to prevent text wrapping - Added e2e test to verify default no-wrap behavior and override capability
Reviewed changes
Copilot reviewed 2 out of 19 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/styles/button.css | Adds min-width CSS property to prevent button text wrapping |
| packages/react/src/components/Button/screenshots.e2e.tsx | Adds test coverage for button wrapping behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Bracciata I am seeing that there used to be a max-width of 6.25rem on the button. The change you have implemented is what we would like from design but I am not sure if this will have a cascading effect across our products. Anyway to confirm if this is a breaking change or not? Here is the cauldron ticket referring to the change #1982 |
There is a good chance it is a breaking change within some repo. Thoughts on doing min-width: min(max-content, 6.25rem)? Correction. Not a breaking change as there was previously a min width and this is a desired change. The minimum still fits all content. |
shawnsharpDQ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 birds with 1 stone! Love it
Closes #2190 and https://github.com/dequelabs/walnut/issues/13150 ##1982
Updates buttons so they do not wrap unless overridden to wrap.
QA Steps:
Navigate to a page with a button
Shrink the width of the page until it is less than the width of the button
Verify the button doesn't wrap and instead overflows the page.