Skip to content
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

Columns: Add transform to unwrap the contents #45666

Merged
merged 1 commit into from
Nov 9, 2022
Merged

Columns: Add transform to unwrap the contents #45666

merged 1 commit into from
Nov 9, 2022

Conversation

dsas
Copy link
Contributor

@dsas dsas commented Nov 9, 2022

What?

Add a transform to move the column contents out into standalone blocks.

Closes #43878

Why?

Improves consistency with group and quote blocks. See #43878

How?

The innerBlocks of a columns block are column block. This transform gets all of these column blocks and then returns a list of all of their innerBlocks.

Testing Instructions

  1. Open a Post or Page
  2. Add a Columns block
  3. Add a whole bunch of content to the different columns
  4. Select the Columns block
  5. Choose 'Unwrap' from the Columns menu
  6. See the Columns block get replaced by the inner contents.

Screenshots or screencast

Screen.Capture.on.2022-11-09.at.14-42-30.mp4

Add a transform to move the column contents out into standalone blocks.
@dsas dsas requested a review from ajitbohra as a code owner November 9, 2022 21:43
@codesandbox
Copy link

codesandbox bot commented Nov 9, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@getdave getdave self-requested a review November 9, 2022 23:21
Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works well. Thank you!

Screen.Capture.on.2022-11-09.at.16-23-08.mp4

Comment on lines +114 to +115
.map( ( innerBlock ) => innerBlock.innerBlocks )
.flat(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would flatMap help here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally forgot that was a thing, thank-you! I've opened a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't give you a chance 😆

@getdave getdave added [Type] Enhancement A suggestion for improvement. [Block] Columns Affects the Columns Block [Package] Block library /packages/block-library and removed [Type] Enhancement A suggestion for improvement. labels Nov 9, 2022
@getdave getdave merged commit 07c948d into WordPress:trunk Nov 9, 2022
@github-actions github-actions bot added this to the Gutenberg 14.6 milestone Nov 9, 2022
@paaljoachim
Copy link
Contributor

paaljoachim commented Nov 16, 2022

Thank you for working on this @dsas Dean! It is much appreciated!

Should it not be called Paragraph? As in one transforms the columns to Paragraph blocks.
Screenshot 2022-11-16 at 22 06 21

Columns transforms to Paragraphs

I find it strange to see the word Unwrap where only blocks are listed in the Transform drop down.

@jasmussen

@getdave
Copy link
Contributor

getdave commented Nov 16, 2022

Thank you for working on this @dsas Dean! It is much appreciated!

Should it not be called Paragraph? As in one transforms the columns to Paragraph blocks. Screenshot 2022-11-16 at 22 06 21

Columns transforms to Paragraphs

I find it strange to see the word Unwrap where only blocks are listed in the Transform drop down.

@jasmussen

@paaljoachim I'm pretty sure it's all types of blocks that can be "unwrapped". @dsas can you confirm?

@paaljoachim
Copy link
Contributor

paaljoachim commented Nov 16, 2022

At first glanze I might think that Unwrap is a new block type. Transforming to Paragraph(s) then I know what I am getting.
Perhaps it should say unwrap to Paragraph. Which does not seem right to me though. Again it would be nice to keep to the consistent block language in the Transform to drop down. As a user clicks the transform to and expects to see familier blocks the specific selected block can be transformed to.

Hmm I just tested Group. Clicking to transform a Group block it then lists Unwrap.
Screenshot 2022-11-16 at 22 57 54

That was new to me. Unwrap to me means take a part a Group or Columns into their single pieces. Seems pretty natural if there are multiple types of blocks inside a container type of block. I unwrapped a Paragraph block and an Image block from the Group block.

Thank you @dsas Dean! For tackling this for the Columns block!
I will back away now.....

@dsas
Copy link
Contributor Author

dsas commented Nov 16, 2022

I'm pretty sure it's all types of blocks that can be "unwrapped". @dsas can you confirm?

Yep exactly this. It doesn't care what kind of blocks are inside the columns.

I know where @paaljoachim is coming from - it is slightly different to the other transforms, it might be worth raising separately or looking back on discussion on when this was introduced to Group.

@dsas dsas deleted the update/unwrap-columns-block branch November 16, 2022 23:45
@jasmussen
Copy link
Contributor

jasmussen commented Nov 17, 2022

Thanks for the ping, thanks for the PR. In principle, since Unwrap exists, it makes sense for containers to be able to unwrap.

However I do regret that we introduced this unwrap feature in the block transforms menu in the first place, for the reasons Paal mentions is one: you transform into other blocks (even though I'm aware you are not transforming the image into a paragraph), but mostly because the term "Unwrap" is not very clear. Conceptually it's also very much related to "Ungroup" which sits in the ellipsis menu. In that light, I would much rather we had added "Ungroup" to every container ellipsis menu, in the same position as Ungroup (even if we needed to call it Unwrap or otherwise for non-group blocks) — and and let it get a shortcut such as ⌘⇧G. I think that's still possible, and something we should do, but nothing urgent. CC: @priethor

@getdave
Copy link
Contributor

getdave commented Nov 17, 2022

Yeh maybe an "Ungroup" style option would be better. @dsas do you fancy making a follow up Issue to track this?

@paaljoachim
Copy link
Contributor

The ellipsis drop down menu is one that I know I often forget to check, but I do usually check the Transform drop down. Basically it would be nice to have the Ungroup in the Transform drop down (in addition to the ellipsis menu) as it gives the option to yes transform the container into their own individual parts. The Transform menu is about changing the current type of block into another block. That could also mean ungrouping container blocks back to their individual blocks.

@jasmussen
Copy link
Contributor

I don't know that I agree, at least not as a starting point. At the moment, the transforms is very leaned towards shifting into other block types, rather than adding or removing groups (or other containers). I do think that the transforms menu is ripe for a redesign that could improve the intuitiveness of the panel, and potentially at that point subsume group/ungroup controls. But for now, I would think the ellipsis menu the best place to add these, and it would also afford us a visual spot for shortcut keys.

@richtabor
Copy link
Member

Yea, it's odd having both ungroup and unwrap. I lean towards this being an over-optimization.

Just thinking through scenarios:

  1. If you go from a paragraph to a columns block and want to undo it in the moment, there's the undo functionality.

  2. If you no longer want columns at all, you need to move the blocks out of the columns (this is what we"unwrap" is optimizing for).

  3. We already have "Ungroup" for group blocks, which works as expected/makes sense.

Do we need "Unwrap" on any other block, other than columns?

Perhaps instead of "Unwrap" in the block transforms, we move it to where the Group/Ungroup shows up for just the columns block. Also, "Unwrap" isn't very clear; seems more like a layout control than manipulating the block/content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block Needs User Documentation Needs new user documentation [Package] Block library /packages/block-library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding a way to unwrap a Columns block in transforms menu
6 participants