Skip to content

Backport: add theme json class changes to edit style variations #3941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tellthemachines wants to merge 12 commits into from
Closed

Backport: add theme json class changes to edit style variations #3941

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
53ce333
Add theme json class changes to edit style variations
tellthemachines Jan 30, 2023
c99dace
Add test for style variations
oandregal Jan 30, 2023
2c58b47
fixup
oandregal Jan 30, 2023
923feca
Merge pull request #1 from oandregal/backport/edit-style-variations
tellthemachines Jan 31, 2023
fe1353c
Fix test output styles.
tellthemachines Jan 31, 2023
80ad395
Fix lint errors.
tellthemachines Jan 31, 2023
1f232da
Fix spacing in test file.
tellthemachines Jan 31, 2023
159a3e3
Remove trailing comma to resolve parsing error in < PHP 7.3.
hellofromtonya Jan 31, 2023
fc4a2a2
Tests: split and use data provider
hellofromtonya Jan 31, 2023
86ac232
Guard for non-array variations
hellofromtonya Jan 31, 2023
994c202
Micro-optimizations in ::get_styles_for_block()
hellofromtonya Jan 31, 2023
0768aa8
Formatting for multiline comments
hellofromtonya Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 83 additions & 8 deletions src/wp-includes/class-wp-theme-json.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,9 +665,17 @@ protected static function sanitize( $input, $valid_block_names, $valid_element_n
$schema_styles_blocks = array();
$schema_settings_blocks = array();
foreach ( $valid_block_names as $block ) {
$schema_settings_blocks[ $block ] = static::VALID_SETTINGS;
$schema_styles_blocks[ $block ] = $styles_non_top_level;
$schema_styles_blocks[ $block ]['elements'] = $schema_styles_elements;
// Build the schema for each block style variation.
$style_variation_names = isset( $input['styles']['blocks'][ $block ]['variations'] ) ? array_keys( $input['styles']['blocks'][ $block ]['variations'] ) : array();
Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the array structure exists, but the 'variations' are not an array? What would happen?

https://3v4l.org/kHpHT

  • = PHP 8.0

Fatal error: Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, string given

  • PHP 8.0:

Warning: array_keys() expects parameter 1 to be array, string given

To avoid these, also check if it is an array.

I'd also suggest switching to ! empty() instead of isset(). Why? To avoid doing array_keys() on an empty array.

Suggested change
$style_variation_names = isset( $input['styles']['blocks'][ $block ]['variations'] ) ? array_keys( $input['styles']['blocks'][ $block ]['variations'] ) : array();
$style_variation_names = array();
if (
! empty( $input['styles']['blocks'][ $block ]['variations'] ) &&
is_array( $input['styles']['blocks'][ $block ]['variations'] )
) {
$style_variation_names = array_keys( $input['styles']['blocks'][ $block ]['variations'] );
}

Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved here 86ac232.

$schema_styles_variations = array();
if ( ! empty( $style_variation_names ) ) {
$schema_styles_variations = array_fill_keys( $style_variation_names, $styles_non_top_level );
}

$schema_settings_blocks[ $block ] = static::VALID_SETTINGS;
$schema_styles_blocks[ $block ] = $styles_non_top_level;
$schema_styles_blocks[ $block ]['elements'] = $schema_styles_elements;
$schema_styles_blocks[ $block ]['variations'] = $schema_styles_variations;
}

$schema['styles'] = static::VALID_STYLES;
Expand Down Expand Up @@ -814,6 +822,15 @@ protected static function get_blocks_metadata() {
}
static::$blocks_metadata[ $block_name ]['elements'][ $el_name ] = implode( ',', $element_selector );
}
// If the block has style variations, append their selectors to the block metadata.
if ( ! empty( $block_type->styles ) ) {
$style_selectors = array();
foreach ( $block_type->styles as $style ) {
// The style variation classname is duplicated in the selector to ensure that it overrides core block styles.
$style_selectors[ $style['name'] ] = static::append_to_selector( '.is-style-' . $style['name'] . '.is-style-' . $style['name'], static::$blocks_metadata[ $block_name ]['selector'] );
}
static::$blocks_metadata[ $block_name ]['styleVariations'] = $style_selectors;
}
}

return static::$blocks_metadata;
Expand Down Expand Up @@ -2039,12 +2056,23 @@ private static function get_block_nodes( $theme_json ) {
$feature_selectors = $selectors[ $name ]['features'];
}

$variation_selectors = array();
if ( isset( $node['variations'] ) ) {
foreach ( $node['variations'] as $variation => $node ) {
$variation_selectors[] = array(
'path' => array( 'styles', 'blocks', $name, 'variations', $variation ),
'selector' => $selectors[ $name ]['styleVariations'][ $variation ],
);
}
}

$nodes[] = array(
'name' => $name,
'path' => array( 'styles', 'blocks', $name ),
'selector' => $selector,
'duotone' => $duotone_selector,
'features' => $feature_selectors,
'name' => $name,
'path' => array( 'styles', 'blocks', $name ),
'selector' => $selector,
'duotone' => $duotone_selector,
'features' => $feature_selectors,
'variations' => $variation_selectors,
);

if ( isset( $theme_json['styles']['blocks'][ $name ]['elements'] ) ) {
Expand Down Expand Up @@ -2124,6 +2152,48 @@ public function get_styles_for_block( $block_metadata ) {
}
}

// If there are style variations, generate the declarations for them, including any feature selectors the block may have.
$style_variation_declarations = array();
if ( ! empty( $block_metadata['variations'] ) ) {
foreach ( $block_metadata['variations'] as $style_variation ) {
$style_variation_node = _wp_array_get( $this->theme_json, $style_variation['path'], array() );
$style_variation_selector = $style_variation['selector'];

// If the block has feature selectors, generate the declarations for them within the current style variation.
if ( ! empty( $block_metadata['features'] ) ) {
foreach ( $block_metadata['features'] as $feature_name => $feature_selector ) {
if ( ! empty( $style_variation_node[ $feature_name ] ) ) {
Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unwrap the inner code by inverting this check. Why? Shifts the code to the left to make it more readable, as more levels of indentation can reduce readability.

Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved here 994c202.

// Prepend the variation selector to the feature selector.
$split_feature_selectors = explode( ',', $feature_selector );
$feature_selectors = array_map(
function( $split_feature_selector ) use ( $style_variation_selector ) {
return trim( $style_variation_selector ) . trim( $split_feature_selector );
Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things here for micro-optimizations:

  • Use static function. That way the callback is created once rather than on each iteration.

  • Move the trim( $style_variation_selector ) outside of the foreach. It does not change without the loop. By moving it outside of the loop, trimming happens once instead of on each iteration.

Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved here 994c202.

},
$split_feature_selectors
);
$combined_feature_selectors = implode( ',', $feature_selectors );

// Compute declarations for the feature.
$new_feature_declarations = static::compute_style_properties( array( $feature_name => $style_variation_node[ $feature_name ] ), $settings, null, $this->theme_json );

// Merge new declarations with any that already exist for
// the feature selector. This may occur when multiple block
// support features use the same custom selector.
Copy link
Contributor

@costdev costdev Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiline comments should be formatted as such:

/*
 * Line 1
 * Line 2
 * ...
 */

Ref
🔢 Applies elsewhere in the PR.

However, I note that this file contains many multiline comments that don't adhere to the standards.

In lieu of a change in this PR that creates an inconsistency in this file, I strongly suggest a follow up PR to bring this file up to the documentation coding standards, and that all future code being backported meets the standard.

Copy link
Contributor

@hellofromtonya hellofromtonya Jan 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved here 0768aa8.

if ( isset( $style_variation_declarations[ $combined_feature_selectors ] ) ) {
$style_variation_declarations[ $combined_feature_selectors ] = array_merge( $style_variation_declarations[ $combined_feature_selectors ], $new_feature_declarations );
} else {
$style_variation_declarations[ $combined_feature_selectors ] = $new_feature_declarations;
}
// Remove the feature from the variation's node now the
// styles will be included under the feature level selector.
unset( $style_variation_node[ $feature_name ] );
}
}
}
// Compute declarations for remaining styles not covered by feature level selectors.
$style_variation_declarations[ $style_variation_selector ] = static::compute_style_properties( $style_variation_node, $settings, null, $this->theme_json );
}
}
/*
* Get a reference to element name from path.
* $block_metadata['path'] = array( 'styles','elements','link' );
Expand Down Expand Up @@ -2214,6 +2284,11 @@ function( $pseudo_selector ) use ( $selector ) {
$block_rules .= static::to_ruleset( $feature_selector, $individual_feature_declarations );
}

// 6. Generate and append the style variation rulesets.
foreach ( $style_variation_declarations as $style_variation_selector => $individual_style_variation_declarations ) {
$block_rules .= static::to_ruleset( $style_variation_selector, $individual_style_variation_declarations );
}

return $block_rules;
}

Expand Down
60 changes: 60 additions & 0 deletions tests/phpunit/tests/theme/wpThemeJson.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3595,6 +3595,66 @@ public function test_get_styles_for_block_with_content_width() {
$this->assertSame( $expected, $root_rules . $style_rules );
}

/**
* @ticket 57583
*/
public function test_get_styles_for_block_with_style_variations() {
$theme_json = new WP_Theme_JSON(
array(
'version' => 2,
'styles' => array(
'blocks' => array(
'core/quote' => array(
'variations' => array(
'plain' => array(
'color' => array(
'background' => 'hotpink',
),
'unregisteredProperty' => 'value',
),
),
),
),
),
),
);

// Validate structure is sanitized.
$expected_theme_json = array(
'version' => 2,
'styles' => array(
'blocks' => array(
'core/quote' => array(
'variations' => array(
'plain' => array(
'color' => array(
'background' => 'hotpink',
),
),
),
),
),
),
);
$sanitized_theme_json = $theme_json->get_raw_data();
$this->assertEqualSetsWithIndex( $expected_theme_json, $sanitized_theme_json );

// Validate styles are generated properly.
$metadata = array(
'path' => array( 'styles', 'blocks', 'core/quote' ),
'selector' => '.wp-block-quote',
'variations' => array(
array(
'path' => array( 'styles', 'blocks', 'core/quote', 'variations', 'plain' ),
'selector' => '.is-style-plain.is-style-plain.wp-block-quote',
),
),
);
$expected_styles = '.is-style-plain.is-style-plain.wp-block-quote{background-color: hotpink;}';
$actual_styles = $theme_json->get_styles_for_block( $metadata );
$this->assertSame( $expected_styles, $actual_styles );
}

/**
* @ticket 56611
*/
Expand Down