Blockbase: remove margin-top from first descendants of columns and groups

[jffng]

Changes proposed in this Pull Request:

This PR removes the top margin of first children of columns and groups. It started as an exploration to add a default padding to all groups with borders.

Related issue(s):

Helps with #4412

[jffng]

These styles might better belong if blockbase, but I'm not sure about this idea in general.

My reasoning for adding it is — I don't think that the first piece of text should have a top margin when contained in a column or group, because it ends up stacking if there is a padding declared. But maybe there is a better way of solving this.

[MaggieCabrera]

I'm not sure we would do this for every theme, it feels kind of opinionated, it makes sense for Skatepark because of the border but that's it. When I first saw the pattern using the border I was thinking about it having a block style even but this approach makes sense too, but just in the scope of this specific theme.

[kjellr]

... it makes sense for Skatepark because of the border but that's it.

Since Blockbase opts into border controls, there's a UI for users to create this bordered effect on any Group block though — using Blockbase or any of its children.

I don't love these rules (and I hope Gutenberg adds greater per-block margin controls so folks can manually fix this sort of thing in the editor), but I think if we add these, they should be in Blockbase.

[kjellr]

Also, putting them in Blockbase would mean more universal expectations around this behavior for our themes, and also would make it easier to strip this out if it's not needed in the future. (Or if we come up with a better solution)

[MaggieCabrera]

yeah, I see what you mean, I don't like it but I agree, and having it on Blockbase is easier to control and go back to it when it eventually can be done better. I just hope we don't have to do this too much, Blockbase is growing instead of shrinking.

[jffng]

I tinkered with this some more and am coming to a different conclusion — I don't think we should add these rules at all. My reasoning:

• This PR should solve the editor and front-end mismatch reported in the original issue.
• The .has-border-color class is only applied when a border color is specified. The default border is applied via inline css directly to the element, so we do not have a reliable way to add padding to all groups with borders because not all of them have the has-border-color class.

That leaves us with two options:

1. Apply a default padding to all group blocks via theme.json. This approach seems like it could be okay, because the user can overwrite this default padding it in the block inspector.
2. Apply the padding in our patterns via the group's block style attributes, as we're doing here:

   themes/skatepark/inc/patterns/mixed-media-in-container.php

   Line 15 in 5626fd4

   <!-- wp:group {"align":"wide","style":{"border":{"style":"solid"},"spacing":{"padding":{"top":"1%","right":"2.5%","bottom":"2.5%","left":"2.5%"}}},"className":"skatepark-mixed-media-in-container"} -->

   The downside here is that if a user creates a group block and adds a border on their own, it will have no padding and they will have to add it themselves.

[scruffian]

I agree that these rules should live in blockbase. I have removed the border rules from skatepark altogether, I agree that they should live in the pattern.

[jffng]

Hi @melchoyce — would you mind retesting with the markup you provided in #4412?

To simplify my long winded comment above, we do not have a reliable way to add padding to groups with borders. Despite this, I think this PR is still an improvement.

[melchoyce]

Just checking, the end goal is it should look like this by default?

[jffng]

That's right.

[melchoyce]

Cool. Let's merge this, and talk about this option more when @kjellr is back:

Apply a default padding to all group blocks via theme.json. This approach seems like it could be okay, because the user can overwrite this default padding it in the block inspector.

We should also consider bumping this issue upstream, if that's possible (I might be understanding the problem)

[scruffian]

We should also consider bumping this issue upstream, if that's possible (I might be understanding the problem)

Added an issue for this WordPress/gutenberg#34315