Editor: Prevent wrapping text when showing icon labels in header #66038
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aaronrobertshaw
added
[Type] Enhancement
Closed
2 tasks
|
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: +8 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
On #65491 (comment) I pinged the @WordPress/gutenberg-components to ask why the |
|
Opened #66049 to discuss it |
|
I'd be fine to ship this as a temporary fix while debating the best way forward for handling this more broadly in |
|
Makes sense. If we do add the style directly to |
2 tasks
|
Thanks for the discussion here.
I'm not sure I agree 100%. If we were starting from a clean slate, it makes perfect sense. My concern is adding On that note, it appears that we have consensus to land this fix for 6.7 and follow-up more holistically via #66049 Once this PR gets a formal approval, I'll get it merged. |
aaronrobertshaw
added
[Type] Bug
andrewserong
approved these changes
Oct 14, 2024
There was a problem hiding this comment.
I'm not sure I agree 100%. If we were starting from a clean slate, it makes perfect sense. My concern is adding nowrap to default buttons after the fact might be a breaking change for some consumers. That's partly why I erred towards the smaller scope of this quick fix.
I like the idea of handling this at the leaf level, as it were, especially since this is a fix for 6.7. This is testing well for me, I notice that Zoom Out hasn't been translated yet in a lot of languages, so testing in Spanish didn't yield results different to English for this button, but overall this LGTM!
ramonjd
approved these changes
Oct 14, 2024
|
Given this was more of a visual bug and there is a desire to see it land in 6.7 as a bug fix. I've corrected the labels on this PR and added |
aaronrobertshaw
deleted the
fix/show-icon-labels-buttons-in-editor-header
branch
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Oct 14, 2024
) Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 39b217e |
It's a valid concern, and I think we're in a bit of a gray area. This change could also be interpreted as a bug fix, where the default variant was (mistakenly) left behind when setting the text wrapping styles in the component. In any case, the change to the default variant is proposed in #66049 and therefore I encourage everyone to help identify pros/cons of the approach (and unwanted side-effects) in that PR 🙏 |
ciampo
added a commit
that referenced
this pull request
Oct 14, 2024
) Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org>
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
…dPress#66038) Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: afercia <afercia@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #65491
What?
Prevents wrapping text for editor header buttons when
showIconLabelsis set totrue.Why?
Preventing wrapping keeps these buttons in line with others within the header and arguably more readable.
See #65491 for more details.
How?
Adds
white-space: nowrapfor buttons within the editor's header while the show icon labels preference is enabled.Testing Instructions
Test snippet to add pinned plugin
Paste the following into dev tools:
Screenshots or screencast
Before:
After:
