Block Spacing setting causes conflict with blockGap (Columns, Navigation, etc.)

[ndiego]

Description

When blockGap is enabled on a theme, the "Block Spacing" setting is enabled for a number of blocks, notably Columns and Navigation. With blockGap, the following CSS is added, which adds "block gap" via a margin-top: var( --wp--style--block-gap ); to each block. (There are separate issues with this, which I will detail in another issue)

Now if you apply the "Block Spacing" setting to, for example, the Columns block, the setting will override the margin-top applied by the CSS above. This is confusing. As a user, I would expect the "Block Spacing" setting to control with space between each column, not the top margin on the Columns block itself, especially since the Columns block supports the margin dimension.

The conflict is due to the "Block Spacing" setting applying its own --wp--style--block-gap variable. You can see that here:

Since the main blockGlap CSS also uses this variable to control the vertical distance between blocks, it gets overridden by the "Block Spacing" setting.

Step-by-step reproduction instructions

1. Install emptytheme or Twenty Twenty-Two.
2. Make sure blockGap is enabled in theme.json
3. Add a Columns block or Navigation block.
4. Apply a custom "Block Spacing"
5. See that this setting controls both the space between columns/nav items as well as the margin-top.

Screenshots, screen recording, code snippet

This is what a "Block Spacing" of 4rem should look like on emptytheme.

This is what it actually looks like.

Environment info

WordPress 5.8.2, Gutenberg 11.9

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[richtabor]

I confirmed this is happening (with Twenty Twenty-Two), and it creates a scenario that also breaks 1:1 between the editor and the front-end.

The desired result of the Block spacing control should be that it affects only the spacing between those columns — not the spacing applied to the columns block itself.

Editor:

Front-end:

[ndiego]

It's a tricky one to solve since --wp--style--block-gap is used all over the place. I have not been able to conceptualize an elegant solution. Creating a one-off fix for the Columns block is one thing, but this currently impact multiple blocks, and likely more in the future as blockGap and expanded upon.

One half-baked thought would be for individual blocks to set a variable --wp--style--block-gap-self or some other name. Then in the CSS do something like margin-left: var( --wp--style--block-gap-self, --wp--style--block-gap, 2em) 🤔

[ndiego]

@jasmussen sorry for the ping, but the more I think about this, I feel it should be tackled prior to 5.9 Beta 1 and backported if a solid solution can be found. I am happy to work on this but am coming up blank, other than the idea mentioned in the comment above.

[richtabor]

@ndiego I was thinking the same thing — gave it a shot on #36680 — I'm pretty sure it works, but would appreciate another set of eyes on it.

[chthonic-ds]

#35454 may point at a more elegant possible solution? I also like how --wp--style--block-row-gap and --wp--style--block-column-gap align with their native CSS Grid counterparts.

[ramonjd]

#35454 may point at a more elegant possible solution? I also like how --wp--style--block-row-gap and --wp--style--block-column-gap align with their native CSS Grid counterparts.

Thanks for surfacing #35454

In that PR we're experimenting with adding --wp--style--block-row-gap and --wp--style--block-column-gap, and providing controls to manipulate the values of each.

With #35454, if I turn on axial block gap and play around with the row gap I can reduce the margin-top of the block container in the frontend.

So it might help in the case of this issue, but I'm not sure. I'd appreciate another opinion!

Furthermore, I'm seeing that the editor doesn't reflect the margin-top value change - I think we might be seeing the effects of margin collapse there.

The desired result of the Block spacing control should be that it affects only the spacing between those columns — not the spacing applied to the columns block itself.

I agree with the above nevertheless. I'm not sure, but I think the goal was to have blockGap have some vertical effect on block spacing.

There's some related discussion on the original PR that introduced the margin-top rule: #33812

[chthonic-ds]

I think the goal was to have blockGap have some vertical effect on block spacing.

Yeah, as noted in 33812 it's following in the footsteps of the great work at https://every-layout.dev/layouts/stack/. The OP issue is partly a side effect of blockGap being treated as a universal spacing value instead of only styling the vertical context — this usage seems to go beyond the original intention.

[youknowriad]

I might have a solution for this: It would work long term for all blocks but won't work for the blocks that are not yet refactored to use the official layout API.

Basically, instead of using the CSS variable to define the gap styles:

.wp-container > * { margin-top: var( --wp--style--block-gap, 0.5em ); }

Have logic in place in the layout styles here if a block gap is defined, generate the correct styles for the children directly:

.wp-container > * { margin-top: 10px; }

The CSS variable should only kept for the theme.json based styles.
This approach also makes the block gap specific to a given container and not something inherited (grand children).

cc @andrewserong

[youknowriad]

Thinking more, I think the issue can also be reproduced using just theme.json (Global styles) if we target specific blocks but it's less visible there.

So I guess my solution here would be to only define the CSS variable for the "root" level, in all other cases, just render styles directly (like we do for duotone styles for instance)

[ramonjd]

Have logic in place in the layout styles here if a block gap is defined, generate the correct styles for the children directly:

Would we somehow pull the margin-top value from the --wp--style--block-gap itself?

If we could pull this off it would be ideal.

My worry with #35454 was that it would introduce yet two more gap-related CSS vars on top of the one #36680 would introduce. Nothing life-threatening of course , I might have had to rethink the implementation, that all. 😄

[richtabor]

on top of the one #36680 would introduce

We wouldn't need this PR/custom var, as the value would be applied directly with what @youknowriad is suggesting.

[andrewserong]

I might have a solution for this: It would work long term for all blocks but won't work for the blocks that are not yet refactored to use the official layout API.

I like the idea @youknowriad! We do have some custom logic in the Button block (width calculation) and the Columns block (left margins) that might be hard to replace in the shorter-term. I suppose as a shorter-term step, even updating the Layout spacing support to use the computed value (.wp-container > * { margin-top: 10px; }), it sounds like that would remove the conflict and the existing inlined CSS variable for Columns, Buttons, etc, should still work in the shorter-term? Then, we'd look at blocks on a case-by-case basis to gradually switch over to the Layout block support where it's appropriate.

Is that what you had in mind?

[youknowriad]

@andrewserong yes, but I wonder if we should just do it for everything (both flex and flow layouts) for consistency.

[andrewserong]

@youknowriad, I agree, it makes sense to do it for both flex and flow layouts 👍, we'll then just have a couple of outliers that haven't yet opted in to the Layout block support, which we can follow up on separately when we can.

[ndiego]

@andrewserong and @youknowriad, speaking of the Layout Block support, I believe #36624 may be related as well based on feedback from @aaronrobertshaw in #36627. I was about to start a PR exploring a fix for #36624 but remembered this issue mentioning Layout Block support. Should I start working on that or would a larger PR specifically related Layout Block support that tackles multiple issues make more sense?

[andrewserong]

Thanks for the ping @ndiego!

Should I start working on that or would a larger PR specifically related Layout Block support that tackles multiple issues make more sense?

I think a shorter-term PR to fix converting a group block into a block that does not opt-in to the Layout support (e.g. the Columns or Cover blocks as you reported in #36624) sounds like a good way to go, and like you mention in that issue, it'd be a good thing to fix for 5.9. From my perspective, switching blocks like the Columns block to opt-in to the Layout block support sounds like more of an "enhancement" and so possibly out of scope for 5.9, but might be something for us to try to get in for WP 6.0. That's just my opinion, though!