Hide the orientation switcher from the group block #39650
Merged
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
youknowriad
added
the
[Type] Bug
When toggled it also switched the block variation off, so you are back with a plain Group, which is confusing. |
|
Size Change: -4 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
andrewserong
approved these changes
Mar 23, 2022
There was a problem hiding this comment.
Thanks for following up here @youknowriad, apologies for missing this one — I think I must have only checked for allowOrientation within block.json files so missed that the variation set the attribute.
This is testing nicely for me with existing blocks made before the PR, and with new blocks added after the PR. As you linked to on the previous decision, keeping the Orientation control hidden for both variations of the Group block makes sense, so that we can support floated alignments in the flow variation 👍
When toggled it also switched the block variation off, so you are back with a plain Group, which is confusing.
It looks like that's because the Inherit toggle currently replaces the whole layout state rather than just toggling the inherit attribute. I think we can probably look at that in a separate PR.
This was referenced Mar 23, 2022
@youknowriad @mtias I've opened up a separate PR to hide the "inherit default layout" toggle on non "flow" layouts in #39665 — is that what you had in mind? |
|
I agree that removing these two settings is the right call. |
|
Thanks for the fix! |
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.
closes #39647
What?
This PR hides the orientation switcher from the "row" block variation.
Why?
It is arguable whether we should have a "column" block variation as well at some point for the group block, but until we do, it was never the intent to have an "orientation" switcher for the row variation. See original reasoning here #35819 (comment)
How?
Initially when we implemented the "allowOrientation" opt-out flag in the "layout" block support, it was mistakenly added as a block attribute property, while it should have been added as a block support flag.
The #39532 PR fixed that by making it a block support config (like it should be) but making sure the flag is false for the group "row" variation was missed.
Testing Instructions
Note
I just noticed that the "inherit default layout" is also visible for the "flex" layouts right now (row block). That doesn't make a lot of sense and it should probably be removed from there if the layout is not "flow"