Group Block: Add support for margin control to the group block

[apeatling]

Description

Add support for controlling margins on the group block using the margin block support.

How has this been tested?

Tested locally.

Screenshots

Types of changes

New feature

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[ramonjd]

I think this is a good change. 👍

At the very least, it opens up margin controls for many blocks that don't yet have margin block support (via nesting them in a group block).

It allows me to add margins, for example to images:

I mentioned over in #33835 that nesting a cover block, for example, inside a group block with full spacing support allows great flexibility already.

And the behaviour is what I'd expect to see:

group-margin-cover.mp4

group-margin-image-full-width.mp4

[aaronrobertshaw]

This generally tests well for me 👍

The only issue I see is the conflict between layouts support (#33359, #29335) and the new left/right margins in some themes and situations.

For example, take the TT1 Blocks theme. When the group block is a top-level block it will have a layout set that constrains the margin-left/right to auto !important. When it is a nested block it won't have the layout margins and so the margin support added in this PR will take effect.

I don't know how much of an issue this is although it is poor UX for users trying to adjust left/right margins and seeing nothing happening.

Being able to conditionally opt into block support is something that has come up before but previously it's been worked around. In this case, it would be good if we could adjust the group block's sides config for margin based on whether it was nested or not. If not nested, you could only adjust top and bottom margins.

Once the new ToolsPanel is merged (🤞 that is imminent), I think it might pay for the margin support control to be a non-default control to lower its prominence. This might mitigate some of the current conflicts between a group block's left/right margin when it's within a specific layout.

[ramonjd]

Being able to conditionally opt into block support is something that has come up before but previously it's been worked around.

That would be amazing, especially if we could, in this case, read the layout attribute.

[apeatling]

Once the new ToolsPanel is merged (🤞 that is imminent), I think it might pay for the margin support control to be a non-default control to lower its prominence. This might mitigate some of the current conflicts between a group block's left/right margin when it's within a specific layout.

Agreed with this, I think it should be hidden by default, and always be secondary to any gap support in a parent block.

[apeatling]

The only issue I see is the conflict between layouts support (#33359, #29335) and the new left/right margins in some themes and situations.

For example, take the TT1 Blocks theme. When the group block is a top-level block it will have a layout set that constrains the margin-left/right to auto !important. When it is a nested block it won't have the layout margins and so the margin support added in this PR will take effect.

I think this is a good reason for margin options to be hidden in the UI unless they are specifically accessed and used within the new ToolsPanel. I'd assume that the flex layout with gap support would be the first port of call and precede this. I was thinking that we'd work on this once this PR is in, since it can change across all blocks that opt in to this support. What do you think?

[aaronrobertshaw]

I think this is a good reason for margin options to be hidden in the UI unless they are specifically accessed and used within the new ToolsPanel. I'd assume that the flex layout with gap support would be the first port of call and precede this. I was thinking that we'd work on this once this PR is in, since it can change across all blocks that opt in to this support. What do you think?

I personally don't have a strong opinion on merging this now. That's under the stated provision that the intention is not to display it by default once the ToolsPanel and dimension panel PR lands.

There has been a general desire however to manage the introduction of new controls to the sidebar. This might mean needing to hold off on merging this until the ToolsPanel actually lands.

@jasmussen Do you have any guidance on this one?

[jasmussen]

I think this is a good reason for margin options to be hidden in the UI unless they are specifically accessed and used within the new ToolsPanel. I'd assume that the flex layout with gap support would be the first port of call and precede this. I was thinking that we'd work on this once this PR is in, since it can change across all blocks that opt in to this support. What do you think?

I'd agree with this. Lately I'm becoming a fan of the container handling many appearance properties for child blocks, and child blocks only overriding those when need be.

There has been a general desire however to manage the introduction of new controls to the sidebar. This might mean needing to hold off on merging this until the ToolsPanel actually lands.

Holding off margin until explicitly added by the tools panel feels implied in Andy's comment. Since the tools panel is so close to landing, though, it seems more a question of how soon after this panel would be swapped out with the fancy new one.

[apeatling]

Holding off margin until explicitly added by the tools panel feels implied in Andy's comment. Since the tools panel is so close to landing, though, it seems more a question of how soon after this panel would be swapped out with the fancy new one.

Yes. Let's hold off on merging this until ToolsPanel is in and we can support that here.

[ramonjd]

Holding off margin until explicitly added by the tools panel feels implied in Andy's comment. Since the tools panel is so close to landing, though, it seems more a question of how soon after this panel would be swapped out with the fancy new one.

Yes. Let's hold off on merging this until ToolsPanel is in and we can support that here.

Making a note to self that I'll follow the same advice for the margin on the cover block.
#33835

[apeatling]

I've now rebased this PR to include the new Dimensions panel using the ToolsPanel component. I've also added padding as the default expanded view, and left margin hidden and available via selection.

I think this should be good for another review. 👍

[stacimc]

This is looking good to me! +1 for padding as a default control and margin hidden. I tested in the block and site editors, and tested setting values from theme.json.

I noticed in tt1-blocks that there's some theme styling that applies margin-top: 0; margin-bottom: 0 to Group blocks with a background color applied. This gets overridden successfully by per-block margin selections via the Dimensions panel, but will override margins set in global styles or in theme.json. I think this is slightly different from the issue with layouts already mentioned because it also applies to nested Group blocks, and because it causes a disparity between what's seen in the editor vs the frontend.

Eg:

• In global styles (or theme.json, either work), add a 100px top and right margin, and a 20px bottom and left margin
• Add a group block and select a background color
• Insert a second group block inside the first and set a contrasting background color

In the editor you'll see that only the top and bottom margin were applied to the top-level block (this is the issue pointed out earlier), but all four margins were applied to the nested block:

But on the frontend, none of the margins are applied to the top level block, and only the right and left margins are applied to the nested block:

I'm not sure to what extent this should be considered a theme issue/an extension of the issue brought up earlier. I'm a bit worried about the disparity between what is seen in the editor vs the frontend, which is particularly confusing because sometimes it's top/bottom that "disappears" and sometimes it's left/right.

It is worth pointing out that margins added at the block level via the Dimensions panel do always work, so if a user notices this disparity they're still able to manually tweak it for each individual block. (Edit -- except for top-level Group blocks as already pointed out)

[aaronrobertshaw]

I'm not sure to what extent this should be considered a theme issue/an extension of the issue brought up earlier.

@stacimc I don't think this is a theme specific issue as the style actually comes from the theme.scss file provided by the group block itself.

Apparently, these styles are part of the set that needs adjusting when the editor resizes. I'm not familiar with this so it could do with some further investigation.

If we need to ensure that the theme.json/global styles values are honoured in the editor perhaps the problem style could be updated to make use of the :where() CSS selector so it has a lower specificity than the generated theme.json/global styles.

[apeatling]

Closing this one out because we are taking the block gap direction for now, but this may be revisited in the future.