Layout: merge CSS rule for block gap

[ramonjd]

Resolves #43046

Alternative to

• Style engine: disable stylesheet optimization temporarily #43051

What?

In the layout abstraction, merge $selector > *,$selector > * + * selectors, only set margin-block-start if a gap value is available.

.wp-block-group.wp-container-1 > * + * {
	margin-block-start: 137px;
}
.wp-block-group.wp-container-2 > * + * {
	margin-block-start: 71px;
}
.wp-block-column.wp-container-3 > * + * {
	margin-block-start: 120px;
}
.wp-block-group.wp-container-5 > * + * {
	margin-block-start: 118px;
}
/* some styles... */
.wp-block-group.wp-container-1 > *,
.wp-block-group.wp-container-1 > * + *,
.wp-block-group.wp-container-2 > *,
.wp-block-group.wp-container-2 > * + *,
.wp-block-column.wp-container-3 > *,
.wp-block-column.wp-container-3 > * + *,
.wp-block-group.wp-container-5 > *,
.wp-block-group.wp-container-5 > * + * {
	margin-block-end: 0;
}

Why?

So that the style engine optimization engine prints out the rules separately.

Note: this doesn't address to root cause, that is, the style engine doesn't care about the order of rules when combining selectors. Combined selectors are printed at the bottom of the stylesheet.

Testing Instructions

(Taken from #43046)

1. Using trunk or Gutenberg Nightly, add a Columns block with a single Column block.
2. Add two Paragraph blocks and then Group the Paragraph blocks.
3. Set the Block Spacing on the Column block to 40px.
4. On the Group block inside of the Column block, set the Block Spacing 20px.
5. Check that the correct block gap is applied to the inner elements of the Group block.

Example block code

<!-- wp:columns {"style":{"spacing":{"blockGap":"1px"}}} -->
<div class="wp-block-columns"><!-- wp:column {"width":"100%"} -->
<div class="wp-block-column" style="flex-basis:100%"><!-- wp:group {"style":{"color":{"background":"#9eebbc"},"spacing":{"blockGap":"137px"}},"layout":{"contentSize":"342px"}} -->
<div class="wp-block-group has-background" style="background-color:#9eebbc"><!-- wp:paragraph {"style":{"elements":{"link":{"color":{"text":"#cd31d6"}}}}} -->
<p class="has-link-color"><a href="https://word">Test</a></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Test</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

<!-- wp:columns {"style":{"spacing":{"blockGap":"1px"}}} -->
<div class="wp-block-columns"><!-- wp:column {"width":"100%"} -->
<div class="wp-block-column" style="flex-basis:100%"><!-- wp:group {"style":{"color":{"background":"#1a4a2d"},"spacing":{"blockGap":"118px"}},"layout":{"contentSize":"342px"}} -->
<div class="wp-block-group has-background" style="background-color:#1a4a2d"><!-- wp:paragraph {"style":{"elements":{"link":{"color":{"text":"#cd31d6"}}}}} -->
<p class="has-link-color"><a href="https://word">Test</a></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Test</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

Trunk

This PR

[ndiego]

I tested this one as well, and it works as expected. Thanks!

That said, as you mentioned in your comment, things definitely get a bit tricky here. I think I would be inclined to disable optimization for now until more testing is done. The batching is pretty slick, but I'm worried about other issues related to style order. I'll be doing more testing this week to see if I can find anything else.

[andrewserong]

That said, as you mentioned in #43046 (comment), things definitely get a bit tricky here. I think I would be inclined to disable optimization for now until more testing is done.

Same here. Thanks for getting the multiple PRs up, though, so we've got options!

[ramonjd]

I guess it wouldn't hurt to merge this one anyway, what do you think @andrewserong ? Or should we keep it to use as a test case for follow up tweaks to the optimization code?

[andrewserong]

Or should we keep it to use as a test case for follow up tweaks to the optimization code?

I haven't had a chance to test this yet, but I'd lean toward keeping it as a test case, personally. Ideally we're adding a single selector at a time? Happy to give it a test later on today if you think it's better to get it technically working first, though!

[aristath]

We could simply change this selector to be more specific... Since this needs to override the other one and they both have the same specificity, we should raise the specificity of this one

Suggested change

	'selector' => "$selector > * + *",
	'selector' => "$selector.$selector > * + *",

[ramonjd]

Works great! Committed (without the extra .)

I'm happy to go with either if we think we should merge this one.

[andrewserong]

Nice that this one fixes the current use case, and it feels a bit neater than combining the selectors in the first rule. Since this is only output when there is a real gap value, I don't at all object to upping the specificity here 👍

I'm wondering if it reveals a drawback or potential issue for the optimisation that we should look into separately, though. It's possibly going to be fairly common for themes to expect styles to override earlier styles with the same specificity due to the order in which they're added. In the case of this particular gap rule, I believe it might have been originally implemented based on the rules from the following article / generator: https://every-layout.dev/layouts/stack/#the-generator

Also, once we go to implement optimisation in global styles, we'll need to be careful about the specificity of the rules, as upping the specificity at one level of the style chain might make it harder for rules at other levels. (e.g. base layout gap, versus block-level in theme gap, versus block-level in block attributes gap).