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

Allow Group blocks to transform into Cover blocks #30445

Closed
kjellr opened this issue Apr 1, 2021 · 7 comments · Fixed by #30890
Closed

Allow Group blocks to transform into Cover blocks #30445

kjellr opened this issue Apr 1, 2021 · 7 comments · Fixed by #30890
Assignees
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Block] Group Affects the Group Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@kjellr
Copy link
Contributor

kjellr commented Apr 1, 2021

It's currently possible to transform a Cover block into a group block, but not the other way around. It would be great to add that option in.

Transform options for Cover Block Transform options for Group Block
Screen Shot 2021-04-01 at 8 11 40 AM Screen Shot 2021-04-01 at 8 12 01 AM
@kjellr kjellr added [Type] Enhancement A suggestion for improvement. [Block] Cover Affects the Cover Block - used to display content laid over a background image [Block] Group Affects the Group Block labels Apr 1, 2021
@youknowriad youknowriad added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Apr 1, 2021
@Soean
Copy link
Member

Soean commented Apr 12, 2021

You can't transform a Cover block into a Group block, only wrap a group block around the Cover block (This is possible for every block). You can use ungroup to remove the group around the block.

Bildschirmfoto 2021-04-12 um 21 36 07

If we add the ungroup option to the transfer dropdown, we should build for all blocks, not just for the cover block.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 16, 2021
@ntsekouras
Copy link
Contributor

I personally can't find much value in this transform, as Cover block needs user input to initialize and we'll have to make arbitrary choices about them. This can confuse users I think. Maybe by having extra checks for having an Image block in group's InnerBlocks and use that as the Cover's background 🤔 ?

Even that would need more changes since the isMatch from transforms API would need to be extended to provide the innerBlocks as well and not only the attributes.

Reference: https://developer.wordpress.org/block-editor/reference-guides/block-api/block-transforms/#block

@getsource
Copy link
Member

getsource commented Apr 16, 2021

I'm not sure if it's better for me to reply here or in the PR.

It's worth noting that this does not require user input if there is an existing background of some sort, but I hear you.
I'm mostly thinking it might be easier for a user to take this path, if they started out with a group, and decided to move to a cover block. Could also only allow the transform when that's the case, but that blocks users from migrating a group when they intend to add an image to the background.

I thought about adding something to migrate an image from innerBlocks, but I also wonder if that would be confusing, since it'd be difficult to know which image to pick, and could generate user surprise. I suppose with the previews, it would show the user what would happen, though. I can try to put together a path of some sort to do that, if folks think it'd be helpful.

@kjellr
Copy link
Contributor Author

kjellr commented Apr 16, 2021

Perhaps we only make this available for Group Blocks that have a solid color background set? Then there's a clear 1:1 cover block equivalent to transition to. (Once #14744 is resolved, there should be more possibilities here too).

@ntsekouras
Copy link
Contributor

ntsekouras commented Apr 16, 2021

It's worth noting that this does not require user input if there is an existing background of some sort

Perhaps we only make this available for Group Blocks that have a solid color background set?

Handling Group blocks that have background/gradient should work well with the current API isMatch. Agreed.

Also I'm not aware if we can extract the theme's main background color programmatically to set as background to Cover if not one is set..

@kjellr
Copy link
Contributor Author

kjellr commented Apr 16, 2021

Also I'm not aware if we can extract the theme's main background color programmatically to set as background to Cover if not one is set..

Yeah, I'm not sure that we can. Though it might get easier to do that if the theme's defined a background via theme.json.

@getsource
Copy link
Member

Updated the PR to only offer transformation to cover when a background color or gradient is available in the Group block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Block] Group Affects the Group Block Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants