Button: prevent text wrapping on default variant #66068
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -15 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
Seems fine! 👍 👍 |
There was a problem hiding this comment.
I'm in favor of this change, otherwise the focus outline would cover the text:
One thing I'm wondering is whether to force nowrap on the link variation as well.
Only this variation has auto height, so the focus outline issue doesn't occur:
| trunk | This PR |
|---|---|
 |
 |
|
I'd like to suggest to test this change with the One of the first things I noticed is the buttons in the Styles panel header show overlapping text:
This issue is already tracked on #61761 but making the text Note: |
@afercia thank you for flagging. It is true that it's an existing situation, but as you say — the changes in this PR would amplify it. We should ideally fix that before merging this change. More in general, this is exactly the kind of situation that we often warn against when making changes to a component like If we wanted to continue with this change, we'd have to check all instances in the editor and make adjustments as needed — and we'd appreciate as much help as possible (at lease in flagging those potential regressions), since we have a limited resources.
Yup, good point. |
|
I'm a bit concerned that this might break in quite a few places that expect wrapping currently. Example:
|
|
Given the amount of work that it would require to search and fix all those instances, and considering that the fixes may not be trivial, it looks like the approach suggested in this PR is not viable. A few alternatives:
A simpler alternative, inspired by the last point above, is to keep The last option seems the best compromise in terms of amount of work required (very little), potential regressions (none), and still providing a way to apply the desired text wrapping behaviour. If we ever rework the component and provide a new version (which I think we may at some point), that will be our chance to align text wrapping for all variants. What do folks think? |
|
I personally think the So in that sense, I'm not that enthusiastic about adding |
|
@mirka that's a valid point (and I agree with it!) What do you suggest as the best course of action? Changing text wrapping on those variables? Leaving everything as-is? |
While I'd agree that buttons should not have a fixed height and should allow text wrapping as native HTML buttons do, I think it's key to make the Gutenberg buttons behave consistently with the Core buttons. As mentioned on the issue, the buttons in core use white-space: nowrap since ages. |
I was going to say the same thing. Not allowing text to wrap can lead to situations where portions of a button label are entirely hidden from vision. |
|
To add more context: However, that's about the fixed height which is less than ideal and should be avoided. As far as I can tell, the Gutenberg buttons use a fixed height instead (except the is-link variant). When Text wrapping is a different matter: Core buttons still use However, many parts of the Gutenberg UI use fixed heights. For example, the editor top bar
On the other side, allowing a fluid height will break the layout of containers with a fixed height e.g. the top bar. Basically, given the design constraints, either everything should be 'fixed' height or fluid (including cotnainers like the top bar). |
|
A lot of great discussion here, thanks everyone 👍 If I've understood the positions expressed so far, it seems like the general consensus is that we should not add The introduction of the
Keeping the status quo for the default button variant would get my vote if we can provide viable workarounds. The issues of consistency with Core's UI buttons and fixed height/nowrap vs fluid containers etc. are definitely worth discussing. Given the complexity and constraints in charting a path forward there though, perhaps that discussion would be best placed on a dedicated issue for it? |
I share this concern but it's worth mentioning many of these cases are actually ad-hoc implementations that override styles of the default buttons and shouldn't have been implemented this way in the first place. Specifically, the design pattern with "icon on top and text below it' is being discussed on #62373. The fundamental problem is that the current button variants aren't flexible enough to achieve that design pattern. Either the base button should support many more props and variants (which is hard to justify) or there should be a 'naked' variant of the button that comes with all the functionality but it's completely unstyled. It's basically a technical debt issue which is costly to solve interms of development time. |
|
Thank you all for the conversation (which is still valid!). I'm just going to close this PR as it looks like the solution proposed is not ideal anyway. |
What?
Fixes #66049
Prevent text wrapping on the default variant of the
Buttoncomponent, in order to align it with all other variantsWhy?
The discrepancy in how text wraps across different variants of the
Buttoncomponent leads to inconsistent UI — and it looks like it was an oversight in the initial implementation.How?
Tweaked styles
Testing Instructions
Buttondoesn't wrap to a new line in all variants (including the default variant)Screenshots or screencast