-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Turn the wrap to multiple lines option off by default on the row block #40171
Conversation
Size Change: +18 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
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.
Thanks for picking this one up @talldan! Looks good for newly created blocks, and existing blocks aren't affected.
Should we also add this rule to the convert-to-group settings in the multi-select toolbar? https://github.com/wordpress/gutenberg/blob/4518134369e30b48cede39ecb062f767c37d3ef2/packages/block-editor/src/components/convert-to-group-buttons/toolbar.js#L18
@andrewserong Thanks for spotting that, I'll add it there too. |
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.
Thanks @talldan! That's working nicely now, and makes it much clearer from a user perspective what's going on when switching to a Row block:
LGTM!
Should we be changing the default instead and add the specific "should wrap" flag for the blocks that want it on? It seems like the way we represent the attribute (when toggled on it's "allow wrap") that the default (not toggled) should not be printing any attribute state. |
@mtias That would be nice. It would be hard to preserve the appearance of existing content with that approach. Any blocks without an explicit serialized There's also another problem I just noticed, using the toggle doesn't ever omit the attribute from the markup. It always toggles between 'wrap' and 'nowrap' rather than toggling between 'wrap' and undefined. |
If we enforce nowrap as the implicit default only for Row it might be alright, since it seems that the places where we actually wanted to wrap were very specific (menus, buttons, social icons, etc). |
What?
Fixes #39651
Turns the 'Allow wrap to multiple lines' option off by default for newly added Row blocks.
Why?
As mentioned in the issue, this setting is unexpected for the row block.
How?
Adds the
flexWrap: 'nowrap'
to the Row variation attributes.Doing it this way means it won't affect other blocks that use the flex layout where wrapping by default makes sense. It also won't change existing content that uses the row block.
Testing Instructions
Screenshots or screencast
Before
After