Remove CSS that causes conflict with theme.json

[ndiego]

Description

Now that theme.json supports margin for column blocks, the removed CSS takes precedent over any configured theme.json margin styles. This impedes theme.json adoption. Since this CSS served no real purpose anyway, I propose that it is removed.

How has this been tested?

This has been tested with TT1 blocks, TT2 and custom FSE themes and Gutenberg 11.9.

Types of changes

Bug fix

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).

[jasmussen]

Thank you for the PR! Thanks for the ping!

So, here's what's up. For non-block themes, we have a blanket rule that outputs top and bottom margins for everything that's defined to be a block — in the editor only. This rule:

html :where(.wp-block) {
	...

	// Provide every block with a default base margin. This margin provides a consistent spacing
	// between blocks in the editor.
	margin-top: $default-block-margin;
	margin-bottom: $default-block-margin;

The problem here is, what is defined as a "block" is pretty generic, and includes both the columns block, and the column block. And so if we remove the rule as this PR does, the following happens in TT1:

It's fine in a block theme, because there aren't those pre-existing margins applied in the editor:

And so if we remove this rule, classic themes that use columns will look different in the editor. So I think we need a different fix: keep the rule in the editor.scss file, so that it applies only to the editor, but lower specificity. For example, this rule:

html :where(.wp-block-column) {
    margin-top: 0;
    margin-bottom: 0;
}

Without testing theme.json, this should have low enough specificity that anything you add there should override it. I consider this :where trick to be a way to apply soft defaults.

For me it works *:

• It works if we ignore the blanket [data-block] margins that TT1 outputs, but those should probably be revisited in the first place. So the fix above should work for most classit themes that don't provide blanket editor block margin styles like that, which feels like the right balance to find.

[ndiego]

Ahh I see. Thanks for the additional information. I have added your recommended CSS and it works well with theme.json configured margins. I also tested in TT1 and removed the blanket [data-block] margins that it outputs and all still looks good. Should I put together a PR to remove that in TT1, or to you think there may be other reasons to keep it?

Anyway, this should be good to go unless you have any further concerns. Thanks for reviewing!

[jasmussen]

Thanks for the fix, kudos on the extra comment. Codewise this looks good and should work well. I don't have the bandwidth to test this one right now, but things to verify is to smoke test a few classic themes. A way to test would be a 2 columns block with paragraphs inside, then paragraphs before and after. This PR should look the same as trunk — with the notable exception of TT1 which has that rule I mentioned.

[ndiego]

I have gone ahead and tested with Twenty Twenty-One, Twenty Twenty, and Twenty Nineteen as well as TT1 Blocks, and TT2 with custom margins in theme.json. All works as expected.

[jasmussen]

I've restarted the tests as the failure seems unrelated. If someone else is able to give this one a quick sanity chec, the code should be good to go.

[Mamaduka]

Hi, @ndiego

Can you rebase this branch on top of the trunk? Failing E2E is skipped for now, so this should fix the checks.

[ndiego]

@Mamaduka just rebased. Let me know if this needs anything else and hopefully the checks all pass 🤞

[ndiego]

@Mamaduka and @jasmussen I have retested this and it's still good to go. Anyway we can get this fix merged?

[jasmussen]

Sorry for missing this. It's in bug territory so it can still land. I'll give this one a spin again tomorrow and see if I can't green light it, if no-one else beats me to it. Thanks for your work.

[jasmussen]

Thanks for the PR. I gave it a thorough test.

To recap, the problem to solve is that in classic themes, a blanket top/bottom margin is output on every block, including column (singular). Those rules are not output in pure block themes, but are in classic themes. In that light, this PR solves a problem for classic themes that use theme.json.

Block theme test first — and in this case there should be no difference to trunk:

All works, and the code takes even if it doesn't unset anything. The use of :where means low specificity.

Classic theme:

The rule does take effect:

Here's what it looks like without the rule:

Note that I had to rebase to test this PR. This is good to land. My only question is, should we add a small additional comment to note that this rule is meant for classic themes that use theme.json, and isn't needed (but is harmless) for block themes?

Pre-approving, but would love that small comment added, then you can land this. Thanks again!

[noisysocks]

Merging this as it has the backport label and I see that it's approved.

[ndiego]

Thanks @noisysocks, and thanks @jasmussen for the additional review. I tested again just now and all looks good.