Opened 6 weeks ago
Closed 6 weeks ago
#66044 closed defect (bug) (fixed)
spacing.blockGap in a block style variation stops emitting any gap
| Reported by: | ramonopoly | Owned by: | ramonopoly |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1.1 |
| Component: | Themes | Version: | 7.1 |
| Severity: | normal | Keywords: | has-patch has-unit-tests dev-reviewed commit |
| Cc: | Focuses: |
Description
This ticket tracks the backporting of the following Gutenberg PR: https://github.com/WordPress/gutenberg/pull/82335
A block style variation that declares spacing.blockGap produced gap CSS in WordPress 7.0 and produces none in 7.1. Nothing else in the variation is affected — css, colours and the rest still emit. There is no error or warning; the gap silently falls back to whatever the theme or Core sets globally.
https://github.com/WordPress/gutenberg/pull/82335 fixes it by passing to get_layout_styles() the block the variation belongs to, so the support check answers the question it is actually asking.
Props to @kimjiwoon
Change History (15)
This ticket was mentioned in PR #13398 on WordPress/wordpress-develop by @kimjiwoon.
6 weeks ago
#1
- Keywords has-patch has-unit-tests added
This ticket was mentioned in Slack in #core-editor by kimjiwoon. View the logs.
6 weeks ago
This ticket was mentioned in Slack in #core by adrianduffell. View the logs.
6 weeks ago
This ticket was mentioned in PR #13419 on WordPress/wordpress-develop by @andraganescu.
6 weeks ago
#4
Package update ticket: https://core.trac.wordpress.org/ticket/66037
## What
Backport of #13398 to the 7.1 branch, for WordPress 7.1.1.
This is the core patch accompanying the Gutenberg package update for 7.1.1 (WordPress/gutenberg#82383), which cherry-picks WordPress/gutenberg#82335 into wp/7.1. The Gutenberg PR registered this backport in backport-changelog/7.1/13398.md.
The other PHP change shipping in that package update (WordPress/gutenberg#81908, block-supports/states.php) is already on the 7.1 branch via changeset 63474, so this is the only remaining core patch.
## Why
WP_Theme_JSON::get_styles_for_block() passes a block style variation node to get_layout_styles(), which reads the metadata name as a block name to check layout support. A variation node's name is the variation slug, which is never a registered block, so the check fails and every variation blockGap rule is discarded. The fix passes the block the variation belongs to.
## Testing
- The diff is applied verbatim from #13398 (currently open against
trunk); it applied cleanly on7.1and both files passphp -l. - Adds unit tests:
test_block_style_variation_with_block_gap_emits_layout_styles,test_block_style_variation_with_responsive_block_gap_emits_layout_styles,test_block_style_variation_block_gap_respects_layout_supportintests/phpunit/tests/theme/wpThemeJson.php.
Props to @Jiwoon-Kim, the author of the original fix (kept as the git author of the commit).
🤖 Generated with Claude Code
@andraganescu commented on PR #13419:
6 weeks ago
#5
Oh, we don't need this because https://github.com/WordPress/gutenberg/issues/13398 will be backported to 7.1.1 by committer according to handbook.
@ramonopoly commented on PR #13419:
6 weeks ago
#6
Thanks for backporting to 7.1!
I'll go ahead and commit https://github.com/WordPress/wordpress-develop/pull/13398 to trunk then.
#8
@
6 weeks ago
- Keywords dev-feedback added
- Resolution fixed
- Status closed → reopened
Reopening to consider backporting https://core.trac.wordpress.org/changeset/63523 for the 7.1 branch.
@andraganescu commented on PR #13419:
6 weeks ago
#9
In https://core.trac.wordpress.org/ticket/66044#comment:8 the core patch will be backported, closing again.
#10
@
6 weeks ago
- Keywords dev-reviewed commit added; dev-feedback removed
@ramonopoly - I reviewed the PR and the changes look good for backporting to me. I will add dev-reviewed as my explicit 2nd sign off.
One small suggestion that isn't a blocker: I wasn't able to test this manually, I only reviewed the code and trust this fixes a genuine 7.1 problem. It would be nice if the PR or trac ticket included clear steps for manual reproduction - or even a Playground blueprint. I typically would try to reproduce an issue manually and observe the code change actually fixes the issue, maybe go back and check the issue didn't exist in 7.0, etc. I'm not sure how to do that exactly with this ticket.
#11
@
6 weeks ago
@adamsilverstein Thanks, that is a good suggestion. Here is a minimal manual reproduction that makes the 7.0 / 7.1 / fixed comparison explicit.
Setup
- Start a clean installation with any block theme active.
- In the active theme, create styles/blocks/group-gap-test.json:
{ "version": 3, "title": "Gap test", "slug": "gap-test", "blockTypes": [ "core/group" ], "styles": { "spacing": { "blockGap": "3em" } } }
- Create a post containing a Group block. Change its layout to Row, add two Paragraph blocks, then select the Group and choose the Gap test block style. Publish and view the post on the front end.
- In DevTools, inspect the Group. It should have both is-style-gap-test and wp-block-group-is-layout-flex. Search the page's generated global-styles CSS for is-style-gap-test.
Expected comparison
- 7.0: the generated stylesheet contains a rule equivalent to :root :where(.wp-block-group.is-style-gap-test.wp-block-group-is-layout-flex){gap: 3em;} , and the Row has a visibly 3em gap.
- 7.1.0: the Group still receives is-style-gap-test, but no matching layout rule is generated; the Row falls back to its normal gap.
- trunk / r63523: the same rule and 3em gap return.
For the patched build, the PR's standard Playground artifact is available here: Test PR #13398 with WordPress Playground. Repeat steps 2-4 there. This isolates the regression to generated layout CSS: the variation class is present in all three cases; only 7.1.0 drops the layout rule.
I agree this is the kind of concise manual path and Playground entry point that should accompany similar Core tickets going forward.
#13
@
6 weeks ago
Thanks for helping move this forward folks! I didn't realize we needed double sign off for trunk (I committed), or was it just for the point release?
Should we close this ticket or wait until it's in the 7.1 branch?
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Backport of https://github.com/WordPress/gutenberg/pull/82335
## Problem
A block style variation that declares
spacing.blockGapemits no layout CSS at all in 7.1 — not only the gap, but the flow, constrained, flex and grid rules together. 7.0 emitted them. There is no error; the spacing silently falls back to whatever the theme or Core sets globally.get_block_nodes()stores the variation slug as the node'sname. That array reachesget_layout_styles()as$block_metadata, wherenameis read as a block name and looked up in the block type registry to check for layout support:A variation slug is never a registered block, so the check fails and the method returns before generating anything.
## Fix
The metadata passed to
get_layout_styles()now names the block the variation belongs to, at both call sites: the basespacing.blockGap, and the one inside a viewport breakpoint, which takes its own copy of the metadata.The variation node keeps its own slug, so feature-selector processing is unaffected, and the layout support check is answered against a real block rather than skipped.
get_layout_styles()itself is untouched — its guard is correct for real block nodes, and giving it the owning block name is what lets it do its job: acore/paragraphvariation declaringblockGapnow emits nothing, where dropping the key entirely would produce rules for layout classes that block never carries.## Testing Instructions
Three tests: the base gap, the responsive gap inside its media query, and the layout support check. The first two fail without the change.
Manually, in a block theme add
styles/blocks/buttons-connected.json:{ "version": 3, "title": "Connected", "slug": "connected", "blockTypes": [ "core/buttons" ], "styles": { "spacing": { "blockGap": "var:preset|spacing|25" } } }Add a Buttons block with two buttons, apply the Connected style, and inspect the container on the front end. Expected: a gap rule for the variation. Before the change: none.