Skip to content

Commit eced441

Browse files
ramonjdramonjd
committed
Themes: restore layout styles for block style variations with a block gap
This commit passes a style variation's "block" to `get_layout_styles()` so the block support check passes. It fixes a bug where a block style variation that declared `spacing.blockGap` silently produced no CSS. The cause was due to running the layout block support check against a variation node, which is not a registered block and cannot be used to check for layout support. Developed in: #13398 Reviewed by adamsilverstein. Merges [63523] to the 7.1 branch. Props kimjiwoon, ramonopoly. Fixes #66044. git-svn-id: https://develop.svn.wordpress.org/branches/7.1@63547 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 9fced37 commit eced441

2 files changed

Lines changed: 204 additions & 3 deletions

File tree

‎src/wp-includes/class-wp-theme-json.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3880,8 +3880,18 @@ public function get_styles_for_block( $block_metadata ) {
38803880
// Only store if the variation has blockGap defined.
38813881
if ( isset( $style_variation_node['spacing']['blockGap'] ) ) {
38823882
// Append block selector to the variation selector for proper targeting.
3883-
$variation_metadata_with_selector = $style_variation;
3884-
$variation_metadata_with_selector['selector'] = $style_variation['selector'] . $block_metadata['css'];
3883+
$variation_metadata_with_selector = $style_variation;
3884+
$variation_metadata_with_selector['selector'] = $style_variation['selector'] . $block_metadata['css'];
3885+
3886+
/*
3887+
* `get_layout_styles()` reads `name` as a block name, to check that the block
3888+
* supports layout at all. A variation node's `name` is the variation slug,
3889+
* which is never a registered block, so the check fails and every variation
3890+
* gap rule is discarded. Pass the block the variation belongs to, so the
3891+
* support check answers the question it is actually asking.
3892+
*/
3893+
$variation_metadata_with_selector['name'] = $block_name;
3894+
38853895
$style_variation_layout_metadata[ $style_variation['selector'] ] = array(
38863896
'metadata' => $variation_metadata_with_selector,
38873897
'node' => $style_variation_node,
@@ -3937,7 +3947,11 @@ public function get_styles_for_block( $block_metadata ) {
39373947
if ( isset( $breakpoint_node['spacing']['blockGap'] ) ) {
39383948
$variation_layout_metadata = $style_variation;
39393949
$variation_layout_metadata['selector'] = $style_variation['selector'] . $block_metadata['css'];
3940-
$variation_responsive_css .= $this->get_layout_styles(
3950+
3951+
// The variation slug is not a block name here either. See above.
3952+
$variation_layout_metadata['name'] = $block_name;
3953+
3954+
$variation_responsive_css .= $this->get_layout_styles(
39413955
$variation_layout_metadata,
39423956
array(
39433957
'node' => $breakpoint_node,

‎tests/phpunit/tests/theme/wpThemeJson.php‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7829,6 +7829,193 @@ public function test_opt_in_to_block_style_variations() {
78297829
$this->assertSame( $expected, $button_variations );
78307830
}
78317831

7832+
/**
7833+
* Tests that a block style variation declaring `spacing.blockGap` emits a layout
7834+
* gap rule scoped to the variation.
7835+
*
7836+
* The variation node carries the variation slug in its `name`, which
7837+
* `get_layout_styles()` reads as a block name. The metadata passed to that
7838+
* method therefore has to name the block the variation belongs to.
7839+
*
7840+
* @ticket 66044
7841+
*
7842+
* @covers WP_Theme_JSON::get_styles_for_block
7843+
*/
7844+
public function test_block_style_variation_with_block_gap_emits_layout_styles() {
7845+
$registry = WP_Block_Styles_Registry::get_instance();
7846+
$registry->register( 'core/group', array( 'name' => 'custom-group' ) );
7847+
7848+
$theme_json = new WP_Theme_JSON(
7849+
array(
7850+
'version' => WP_Theme_JSON::LATEST_SCHEMA,
7851+
'settings' => array(
7852+
'spacing' => array(
7853+
'blockGap' => true,
7854+
),
7855+
),
7856+
'styles' => array(
7857+
'blocks' => array(
7858+
'core/group' => array(
7859+
'variations' => array(
7860+
'custom-group' => array(
7861+
'spacing' => array(
7862+
'blockGap' => '3em',
7863+
),
7864+
),
7865+
),
7866+
),
7867+
),
7868+
),
7869+
),
7870+
'blocks'
7871+
);
7872+
7873+
$stylesheet = $theme_json->get_stylesheet(
7874+
array( 'styles' ),
7875+
array( 'custom' ),
7876+
array(
7877+
'include_block_style_variations' => true,
7878+
'skip_root_layout_styles' => true,
7879+
)
7880+
);
7881+
7882+
$registry->unregister( 'core/group', 'custom-group' );
7883+
7884+
$this->assertStringContainsString(
7885+
':root :where(.wp-block-group.is-style-custom-group.wp-block-group-is-layout-flex){gap: 3em;}',
7886+
$stylesheet,
7887+
'The variation should emit a gap rule scoped to itself.'
7888+
);
7889+
}
7890+
7891+
/**
7892+
* Tests that a block style variation declaring `spacing.blockGap` inside a
7893+
* viewport breakpoint emits a layout gap rule within the media query.
7894+
*
7895+
* The responsive branch takes its own copy of the variation metadata, so it
7896+
* needs the owning block name for the same reason the base branch does.
7897+
*
7898+
* @ticket 66044
7899+
*
7900+
* @covers WP_Theme_JSON::get_styles_for_block
7901+
*/
7902+
public function test_block_style_variation_with_responsive_block_gap_emits_layout_styles() {
7903+
$registry = WP_Block_Styles_Registry::get_instance();
7904+
$registry->register( 'core/group', array( 'name' => 'custom-group' ) );
7905+
7906+
$theme_json = new WP_Theme_JSON(
7907+
array(
7908+
'version' => WP_Theme_JSON::LATEST_SCHEMA,
7909+
'settings' => array(
7910+
'spacing' => array(
7911+
'blockGap' => true,
7912+
),
7913+
'viewport' => array(
7914+
'mobile' => '599px',
7915+
),
7916+
),
7917+
'styles' => array(
7918+
'blocks' => array(
7919+
'core/group' => array(
7920+
'variations' => array(
7921+
'custom-group' => array(
7922+
'spacing' => array(
7923+
'blockGap' => '3em',
7924+
),
7925+
'@mobile' => array(
7926+
'spacing' => array(
7927+
'blockGap' => '1em',
7928+
),
7929+
),
7930+
),
7931+
),
7932+
),
7933+
),
7934+
),
7935+
),
7936+
'blocks'
7937+
);
7938+
7939+
$stylesheet = $theme_json->get_stylesheet(
7940+
array( 'styles' ),
7941+
array( 'custom' ),
7942+
array(
7943+
'include_block_style_variations' => true,
7944+
'skip_root_layout_styles' => true,
7945+
)
7946+
);
7947+
7948+
$registry->unregister( 'core/group', 'custom-group' );
7949+
7950+
$this->assertStringContainsString(
7951+
'@media (width <= 599px)',
7952+
$stylesheet,
7953+
'The breakpoint media query should be emitted.'
7954+
);
7955+
$this->assertStringContainsString(
7956+
':root :where(.wp-block-group.is-style-custom-group.wp-block-group-is-layout-flex){gap: 1em;}',
7957+
$stylesheet,
7958+
'The variation should emit a gap rule inside the breakpoint.'
7959+
);
7960+
}
7961+
7962+
/**
7963+
* Tests that the layout support check still applies to a variation's blockGap.
7964+
*
7965+
* The variation metadata carries the block it belongs to, so a block without
7966+
* layout support emits no gap rule -- the check is answered, not skipped.
7967+
*
7968+
* @ticket 66044
7969+
*
7970+
* @covers WP_Theme_JSON::get_styles_for_block
7971+
*/
7972+
public function test_block_style_variation_block_gap_respects_layout_support() {
7973+
$registry = WP_Block_Styles_Registry::get_instance();
7974+
$registry->register( 'core/paragraph', array( 'name' => 'custom-paragraph' ) );
7975+
7976+
$theme_json = new WP_Theme_JSON(
7977+
array(
7978+
'version' => WP_Theme_JSON::LATEST_SCHEMA,
7979+
'settings' => array(
7980+
'spacing' => array(
7981+
'blockGap' => true,
7982+
),
7983+
),
7984+
'styles' => array(
7985+
'blocks' => array(
7986+
'core/paragraph' => array(
7987+
'variations' => array(
7988+
'custom-paragraph' => array(
7989+
'spacing' => array(
7990+
'blockGap' => '3em',
7991+
),
7992+
),
7993+
),
7994+
),
7995+
),
7996+
),
7997+
),
7998+
'blocks'
7999+
);
8000+
8001+
$stylesheet = $theme_json->get_stylesheet(
8002+
array( 'styles' ),
8003+
array( 'custom' ),
8004+
array(
8005+
'include_block_style_variations' => true,
8006+
'skip_root_layout_styles' => true,
8007+
)
8008+
);
8009+
8010+
$registry->unregister( 'core/paragraph', 'custom-paragraph' );
8011+
8012+
$this->assertStringNotContainsString(
8013+
'is-style-custom-paragraph',
8014+
$stylesheet,
8015+
'core/paragraph has no layout support, so its variation should emit no gap rule.'
8016+
);
8017+
}
8018+
78328019
/**
78338020
* Tests that block-level settings inherit global default settings when not explicitly set.
78348021
*

0 commit comments

Comments
 (0)