Remove unwrap from transforms and add ungroup to more blocks
#50385
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
ntsekouras
requested review from
mcsf,
Mamaduka,
youknowriad,
oandregal,
gziolo,
jasmussen and
richtabor
ntsekouras
added
[Feature] Blocks
|
Size Change: -22.4 kB (-2%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 5c6d77b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5023231722
|
|
Thanks for this one!
Designwise I would suggest this one is good to go, for a few reasons:
We should follow up with ⌘G and ⌘⇧G for grouping and ungrouping later on. |
|
This is an interesting idea to extend |
gziolo
added
Needs Dev Note
I agree. Though it begs to question if transforming to "Group" or "Columns" should be a transform. They're not really transforming. And we already have "Group" as an action within the Block Settings Menu (same as Ungroup). |
ntsekouras
force-pushed
the
remove/unwrap-transform
branch
from
bde0831
to
162e847
Compare
ntsekouras
requested review from
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
Fuzzy separate conversation, yes. I think it can be valid as a transform: transforming a columns block should essentially change the container into a group instead. Whereas if you choose the "Group" item from the ellipsis menu, it should wrap the columns block in a group. That's my instinct: transforms should always transform. Multi-selection falls under that umbrella, select 3 paragraphs and transform to group or columns, and that should work as it does today. The main thing I personally find confusing, is when the option listed as a transform actually wraps in a new container instead of actually transforming. |
|
Hey @ntsekouras 👋, as we talked internally regarding the mobile native version, it would be great to postpone merging this PR until we implement the mobile native version of I think the implementation shouldn't be complex if we can use the I'm afraid I don't bandwidth this week to focus on this, but hopefully, I can spare time next week and unblock the mobile part, as well as address the failure in unit tests. Thanks 🙇 ! |
mcsf
reviewed
May 10, 2023
...es/block-editor/src/components/convert-to-group-buttons/use-convert-to-group-button-props.js
Show resolved
Hide resolved
Co-authored-by: Miguel Fonseca <[email protected]>
ntsekouras
force-pushed
the
remove/unwrap-transform
branch
from
208b125
to
7bbf8cc
Compare
In #50693, we'll incorporate into this PR the needed changes for the mobile native version (including unit test fixes). |
* Add `useConvertToGroupButtons` hook Most of the logic of this hook has been extracted from `ConvertToGroupButton` component. The main difference is that we return the configuration for the block actions instead of a component. * Add Group and Ungroup options to block actions menu * Remove `canUnwrap` option from `getBlockTransformOptions` test helper * Update tests for Group, Quote and Columns blocks
The mobile native changes have been incorporating into this PR via 5c6d77b. |
westonruter
added a commit
that referenced
this pull request
May 19, 2023
…dd/static-closures * 'trunk' of https://github.com/WordPress/gutenberg: (26 commits) Add transparent outline to input control BackdropUI focus style. (#50772) Added wrapper element for RichText in File block (#50607) Remove the experimental flag of the command center (#50781) Update the document title in the site editor to open the command center (#50369) Remove `unwrap` from transforms and add `ungroup` to more blocks (#50385) Add new experimental version of DropdownMenu (#49473) Force display of in custom css input boxes to LTR (#50768) Polish experimental navigation block (#50670) Support negation operator in selectors in the Interactivity API (#50732) Minor updates to theme.json schema pages (#50742) $revisions_controller is not used. Let's delete it. (#50763) Remove OffCanvasEditor (#50705) Mobile - E2E test - Update code to use the new navigateUp helper (#50736) Try: Smaller external link icon (#50728) Block Editor: Remove unused 'useIsDimensionsSupportValid' method (#50735) Fix flaky media inserter drag-and-dropping e2e test (#50740) docs: Fix change log typo (#50737) Edit Site: Fix `useEditedEntityRecord()` loading state (#50730) Fix labelling, description, and focus style of the block transform to pattern previews (#50577) Fix Global Styles sidebar block selection on zoom out mode (#50708) ...
6 tasks
24 tasks
This was referenced Jul 12, 2023
Merged
6 tasks
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.
What?
Part of: #45841
Regarding the above issue, this PR will not add any shortcuts to reduce its scope. Additionally this PR could (and maybe will) be split in two separate ones. This is because it does two things:
unwrapfrom the block switcher forGroup, Columns, Quoteblocks and essentially partially or fully reverting 1, 2, 3.ungroupHow and Why?
This PR suggests a new
transformsAPI for theungroupfunctionality in block settings menu controls(ellipsis menu in block toolbar). If a block, besides the set grouping block(see here), want to render theUngroupbutton, should add anungroupfunction in theirtransformsof the block type.This new function accepts the block's attributes and innerBlocks(although we might consider passing the whole block) and is responsible for returning the new blocks to replace the selected block. This is because blocks can have a different unwrapping handling - ex. Columns need to also remove the
core/columnblocks etc..Why in
transformsAPI?No strong opinions here, but I felt it's less impactful than adding a new
BlockAPI and it can make sense semantically. We could do it there too though.Why we even need a new API?
My first try was just adding the
Ungroupbutton inside the blocks' edit function withBlockSettingsMenuControls. That would be the best option, if we could handle the wanted position of the menu item. Right now thefillsare rendered above theGroupmenu item and creates a disconnect with the rest of the menu items.Testing Instructions
Group, Quote, Columnsblocksunwrapoption in block switcher is goneUngroupmenu item in the ellipsis menu in block toolbar works as theunwrapbutton was working. I'm saying this because there is already a flow there in the logic about when to show this button. Checking innerBlocks existence is not enough because for example in an seemingly empty Columns block we have emptycore/columnblocks.unwrapoption.Tasks
unwrap