From cc8d54ad863ea79c1ae639b9976f756dbdf1cdda Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 15 Sep 2022 14:05:06 +1200 Subject: [PATCH 01/18] Add spacing presets backports for 6.1 --- .../class-wp-theme-json-resolver.php | 5 +- src/wp-includes/class-wp-theme-json.php | 134 +++++++++++++++++- src/wp-includes/theme-i18n.json | 14 ++ 3 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/wp-includes/class-wp-theme-json-resolver.php b/src/wp-includes/class-wp-theme-json-resolver.php index 5d0dd099c20e5..74a0cd1daa0d9 100644 --- a/src/wp-includes/class-wp-theme-json-resolver.php +++ b/src/wp-includes/class-wp-theme-json-resolver.php @@ -418,7 +418,7 @@ public static function get_user_data() { * @since 5.8.0 * @since 5.9.0 Added user data, removed the `$settings` parameter, * added the `$origin` parameter. - * @since 6.1.0 Added block data. + * @since 6.1.0 Added block data and generation of spacingSizes array. * * @param string $origin Optional. To what level should we merge data. * Valid values are 'theme' or 'custom'. Default 'custom'. @@ -438,6 +438,9 @@ public static function get_merged_data( $origin = 'custom' ) { $result->merge( static::get_user_data() ); } + // Generate the default spacingSizes array based on the merged spacingScale settings. + $result->set_spacing_sizes(); + return $result; } diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index 4e7c704cce16b..fbdabaa2817f5 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -164,6 +164,24 @@ class WP_Theme_JSON { 'classes' => array( '.has-$slug-font-family' => 'font-family' ), 'properties' => array( 'font-family' ), ), + array( + 'path' => array( 'spacing', 'spacingSizes' ), + 'prevent_override' => false, + 'use_default_names' => true, + 'value_key' => 'size', + 'css_vars' => '--wp--preset--spacing--$slug', + 'classes' => array(), + 'properties' => array( 'padding', 'margin' ), + ), + array( + 'path' => array( 'spacing', 'spacingScale' ), + 'prevent_override' => false, + 'use_default_names' => true, + 'value_key' => 'size', + 'css_vars' => '--wp--preset--spacing--$slug', + 'classes' => array(), + 'properties' => array( 'padding', 'margin' ), + ), ); /** @@ -307,10 +325,13 @@ class WP_Theme_JSON { 'wideSize' => null, ), 'spacing' => array( - 'blockGap' => null, - 'margin' => null, - 'padding' => null, - 'units' => null, + 'customSpacingSize' => null, + 'spacingSizes' => null, + 'spacingScale' => null, + 'blockGap' => null, + 'margin' => null, + 'padding' => null, + 'units' => null, ), 'typography' => array( 'customFontSize' => null, @@ -2886,4 +2907,109 @@ public function get_data() { return $output; } + /** + * Sets the spacingSizes array based on the spacingScale values from theme.json. + * + * @since 6.1.0 + * + * @return void + */ + public function set_spacing_sizes() { + $spacing_scale = _wp_array_get( $this->theme_json, array( 'settings', 'spacing', 'spacingScale' ), array() ); + + if ( ! is_numeric( $spacing_scale['steps'] ) + || ! isset( $spacing_scale['mediumStep'] ) + || ! isset( $spacing_scale['unit'] ) + || ! isset( $spacing_scale['operator'] ) + || ! isset( $spacing_scale['increment'] ) + || ! isset( $spacing_scale['steps'] ) + || ! is_numeric( $spacing_scale['increment'] ) + || ! is_numeric( $spacing_scale['mediumStep'] ) + || ( '+' !== $spacing_scale['operator'] && '*' !== $spacing_scale['operator'] ) ) { + if ( ! empty( $spacing_scale ) ) { + trigger_error( __( 'Some of the theme.json settings.spacing.spacingScale values are invalid', 'gutenberg' ), E_USER_NOTICE ); + } + return null; + } + + // If theme authors want to prevent the generation of the core spacing scale they can set their theme.json spacingScale.steps to 0. + if ( 0 === $spacing_scale['steps'] ) { + return null; + } + + $unit = '%' === $spacing_scale['unit'] ? '%' : sanitize_title( $spacing_scale['unit'] ); + $current_step = $spacing_scale['mediumStep']; + $steps_mid_point = round( ( ( $spacing_scale['steps'] ) / 2 ), 0 ); + $x_small_count = null; + $below_sizes = array(); + $slug = 40; + $remainder = 0; + + for ( $x = $steps_mid_point - 1; $spacing_scale['steps'] > 1 && $slug > 0 && $x > 0; $x-- ) { + $current_step = '+' === $spacing_scale['operator'] + ? $current_step - $spacing_scale['increment'] + : ( $spacing_scale['increment'] > 1 ? $current_step / $spacing_scale['increment'] : $current_step * $spacing_scale['increment'] ); + + if ( $current_step <= 0 ) { + $remainder = $x; + break; + } + + $below_sizes[] = array( + /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Small */ + 'name' => $x === $steps_mid_point - 1 ? __( 'Small', 'gutenberg' ) : sprintf( __( '%sX-Small', 'gutenberg' ), strval( $x_small_count ) ), + 'slug' => (string) $slug, + 'size' => round( $current_step, 2 ) . $unit, + ); + + if ( $x === $steps_mid_point - 2 ) { + $x_small_count = 2; + } + + if ( $x < $steps_mid_point - 2 ) { + $x_small_count++; + } + + $slug = $slug - 10; + } + + $below_sizes = array_reverse( $below_sizes ); + + $below_sizes[] = array( + 'name' => __( 'Medium', 'gutenberg' ), + 'slug' => '50', + 'size' => $spacing_scale['mediumStep'] . $unit, + ); + + $current_step = $spacing_scale['mediumStep']; + $x_large_count = null; + $above_sizes = array(); + $slug = 60; + $steps_above = ( $spacing_scale['steps'] - $steps_mid_point ) + $remainder; + + for ( $x = 0; $x < $steps_above; $x++ ) { + $current_step = '+' === $spacing_scale['operator'] + ? $current_step + $spacing_scale['increment'] + : ( $spacing_scale['increment'] >= 1 ? $current_step * $spacing_scale['increment'] : $current_step / $spacing_scale['increment'] ); + + $above_sizes[] = array( + /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Large */ + 'name' => 0 === $x ? __( 'Large', 'gutenberg' ) : sprintf( __( '%sX-Large', 'gutenberg' ), strval( $x_large_count ) ), + 'slug' => (string) $slug, + 'size' => round( $current_step, 2 ) . $unit, + ); + + if ( 1 === $x ) { + $x_large_count = 2; + } + + if ( $x > 1 ) { + $x_large_count++; + } + + $slug = $slug + 10; + } + + _wp_array_set( $this->theme_json, array( 'settings', 'spacing', 'spacingSizes', 'default' ), array_merge( $below_sizes, $above_sizes ) ); + } } diff --git a/src/wp-includes/theme-i18n.json b/src/wp-includes/theme-i18n.json index f5a65dbddc37d..82f89bd3d282f 100644 --- a/src/wp-includes/theme-i18n.json +++ b/src/wp-includes/theme-i18n.json @@ -30,6 +30,13 @@ } ] }, + "spacing": { + "spacingSizes": [ + { + "name": "Space size name" + } + ] + }, "blocks": { "*": { "typography": { @@ -55,6 +62,13 @@ "name": "Gradient name" } ] + }, + "spacing": { + "spacingSizes": [ + { + "name": "Space size name" + } + ] } } } From f63c5c3e87f26bfd79585100f6c20c81dfcc07f9 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 15 Sep 2022 14:22:33 +1200 Subject: [PATCH 02/18] Add tests for set_spacing_sizes method --- tests/phpunit/tests/theme/wpThemeJson.php | 327 ++++++++++++++++++++++ 1 file changed, 327 insertions(+) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index f1e3e1ae8b9c1..a2ea5e9cd6fe1 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3493,4 +3493,331 @@ function test_get_styles_for_block_with_content_width() { $style_rules = $theme_json->get_styles_for_block( $metadata ); $this->assertSame( $expected, $root_rules . $style_rules ); } + + /** + * @dataProvider data_generate_spacing_scale_fixtures + * + * Tests generating the spacing presets array based on the spacing scale provided. + */ + function test_set_spacing_sizes( $spacing_scale, $expected_output ) { + $theme_json = new WP_Theme_JSON_Gutenberg( + array( + 'version' => 2, + 'settings' => array( + 'spacing' => array( + 'spacingScale' => $spacing_scale, + ), + ), + ) + ); + + $theme_json->set_spacing_sizes(); + $this->assertSame( $expected_output, _wp_array_get( $theme_json->get_raw_data(), array( 'settings', 'spacing', 'spacingSizes', 'default' ) ) ); + } + + /** + * Data provider for spacing scale tests. + * + * @return array + */ + function data_generate_spacing_scale_fixtures() { + return array( + 'one_step_spacing_scale' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 1, + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '4rem', + ), + ), + ), + + 'two_step_spacing_scale_should_add_step_above_medium' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 2, + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '4rem', + ), + array( + 'name' => 'Large', + 'slug' => '60', + 'size' => '5.5rem', + ), + ), + ), + + 'three_step_spacing_scale_should_add_step_above_and_below_medium' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 3, + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'Small', + 'slug' => '40', + 'size' => '2.5rem', + ), + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '4rem', + ), + array( + 'name' => 'Large', + 'slug' => '60', + 'size' => '5.5rem', + ), + ), + ), + + 'even_step_spacing_scale_steps_should_add_extra_step_above_medium' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 4, + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'Small', + 'slug' => '40', + 'size' => '2.5rem', + ), + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '4rem', + ), + array( + 'name' => 'Large', + 'slug' => '60', + 'size' => '5.5rem', + ), + array( + 'name' => 'X-Large', + 'slug' => '70', + 'size' => '7rem', + ), + ), + ), + + 'if_bottom_end_will_go_below_zero_should_add_extra_steps_above_medium_instead' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 2.5, + 'steps' => 5, + 'mediumStep' => 5, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'Small', + 'slug' => '40', + 'size' => '2.5rem', + ), + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '5rem', + ), + array( + 'name' => 'Large', + 'slug' => '60', + 'size' => '7.5rem', + ), + array( + 'name' => 'X-Large', + 'slug' => '70', + 'size' => '10rem', + ), + array( + 'name' => '2X-Large', + 'slug' => '80', + 'size' => '12.5rem', + ), + ), + ), + + 'multiplier_should_correctly_calculate_above_and_below_medium' => array( + 'spacingScale' => array( + 'operator' => '*', + 'increment' => 1.5, + 'steps' => 5, + 'mediumStep' => 1.5, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'X-Small', + 'slug' => '30', + 'size' => '0.67rem', + ), + array( + 'name' => 'Small', + 'slug' => '40', + 'size' => '1rem', + ), + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '1.5rem', + ), + array( + 'name' => 'Large', + 'slug' => '60', + 'size' => '2.25rem', + ), + array( + 'name' => 'X-Large', + 'slug' => '70', + 'size' => '3.38rem', + ), + ), + ), + + 'increment_<_1_combined_with_*_operator_should_act_as_divisor_to_calculate_above_and_below_medium' => array( + 'spacingScale' => array( + 'operator' => '*', + 'increment' => 0.25, + 'steps' => 5, + 'mediumStep' => 1.5, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => 'X-Small', + 'slug' => '30', + 'size' => '0.09rem', + ), + array( + 'name' => 'Small', + 'slug' => '40', + 'size' => '0.38rem', + ), + array( + 'name' => 'Medium', + 'slug' => '50', + 'size' => '1.5rem', + ), + array( + 'name' => 'Large', + 'slug' => '60', + 'size' => '6rem', + ), + array( + 'name' => 'X-Large', + 'slug' => '70', + 'size' => '24rem', + ), + ), + ), + ); + } + + /** + * @dataProvider data_set_spacing_sizes_when_invalid + * + * Tests generating the spacing presets array based on the spacing scale provided. + */ + public function test_set_spacing_sizes_when_invalid( $spacing_scale, $expected_output ) { + $this->expectNotice(); + $this->expectNoticeMessage( 'Some of the theme.json settings.spacing.spacingScale values are invalid' ); + + $theme_json = new WP_Theme_JSON_Gutenberg( + array( + 'version' => 2, + 'settings' => array( + 'spacing' => array( + 'spacingScale' => $spacing_scale, + ), + ), + ) + ); + + $theme_json->set_spacing_sizes(); + $this->assertSame( $expected_output, _wp_array_get( $theme_json->get_raw_data(), array( 'settings', 'spacing', 'spacingSizes', 'default' ) ) ); + } + + /** + * Data provider for spacing scale tests. + * + * @return array + */ + function data_set_spacing_sizes_when_invalid() { + return array( + + 'invalid_spacing_scale_values_missing_operator' => array( + 'spacingScale' => array( + 'operator' => '', + 'increment' => 1.5, + 'steps' => 1, + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => null, + ), + + 'invalid_spacing_scale_values_non_numeric_increment' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 'add two to previous value', + 'steps' => 1, + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => null, + ), + + 'invalid_spacing_scale_values_non_numeric_steps' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 'spiral staircase preferred', + 'mediumStep' => 4, + 'unit' => 'rem', + ), + 'expected_output' => null, + ), + + 'invalid_spacing_scale_values_non_numeric_medium_step' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 5, + 'mediumStep' => 'That which is just right', + 'unit' => 'rem', + ), + 'expected_output' => null, + ), + + 'invalid_spacing_scale_values_missing_unit' => array( + 'spacingScale' => array( + 'operator' => '+', + 'increment' => 1.5, + 'steps' => 5, + 'mediumStep' => 4, + ), + 'expected_output' => null, + ), + ); + } } From d62217ab2a86217efaa66bec764b8f7bf59dffde Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 15 Sep 2022 15:38:07 +1200 Subject: [PATCH 03/18] Fix errors --- src/wp-includes/block-editor.php | 2 +- tests/phpunit/tests/theme/wpThemeJson.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/block-editor.php b/src/wp-includes/block-editor.php index 2977afbeb8cb9..3cde142c641cb 100644 --- a/src/wp-includes/block-editor.php +++ b/src/wp-includes/block-editor.php @@ -488,7 +488,7 @@ function get_block_editor_settings( array $custom_settings, $block_editor_contex unset( $editor_settings['__experimentalFeatures']['spacing']['padding'] ); } if ( isset( $editor_settings['__experimentalFeatures']['spacing']['customSpacingSize'] ) ) { - $editor_settings['disableCustomSpacingSizes'] = ! $editor_ettings['__experimentalFeatures']['spacing']['customSpacingSize']; + $editor_settings['disableCustomSpacingSizes'] = ! $editor_settings['__experimentalFeatures']['spacing']['customSpacingSize']; unset( $editor_settings['__experimentalFeatures']['spacing']['customSpacingSize'] ); } diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index a2ea5e9cd6fe1..83628adccfbcb 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3496,11 +3496,11 @@ function test_get_styles_for_block_with_content_width() { /** * @dataProvider data_generate_spacing_scale_fixtures - * + * * Tests generating the spacing presets array based on the spacing scale provided. */ function test_set_spacing_sizes( $spacing_scale, $expected_output ) { - $theme_json = new WP_Theme_JSON_Gutenberg( + $theme_json = new WP_Theme_JSON( array( 'version' => 2, 'settings' => array( @@ -3742,7 +3742,7 @@ public function test_set_spacing_sizes_when_invalid( $spacing_scale, $expected_o $this->expectNotice(); $this->expectNoticeMessage( 'Some of the theme.json settings.spacing.spacingScale values are invalid' ); - $theme_json = new WP_Theme_JSON_Gutenberg( + $theme_json = new WP_Theme_JSON( array( 'version' => 2, 'settings' => array( From 6de2a47327de95638033ce2adb26292a75e699ce Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 15 Sep 2022 16:30:04 +1200 Subject: [PATCH 04/18] Another lint error --- tests/phpunit/tests/theme/wpThemeJson.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 83628adccfbcb..08ac2c8315778 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3735,7 +3735,7 @@ function data_generate_spacing_scale_fixtures() { /** * @dataProvider data_set_spacing_sizes_when_invalid - * + * * Tests generating the spacing presets array based on the spacing scale provided. */ public function test_set_spacing_sizes_when_invalid( $spacing_scale, $expected_output ) { From 26ccce6e2df4e3e877574de72fbeda5259a5c9d8 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 15 Sep 2022 16:55:16 +1200 Subject: [PATCH 05/18] Add backport ticket number to tests --- tests/phpunit/tests/theme/wpThemeJson.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 08ac2c8315778..25dc3449b28a7 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3496,6 +3496,8 @@ function test_get_styles_for_block_with_content_width() { /** * @dataProvider data_generate_spacing_scale_fixtures + * + * @ticket 56467 * * Tests generating the spacing presets array based on the spacing scale provided. */ @@ -3735,6 +3737,8 @@ function data_generate_spacing_scale_fixtures() { /** * @dataProvider data_set_spacing_sizes_when_invalid + * + * @ticket 56467 * * Tests generating the spacing presets array based on the spacing scale provided. */ From 6351f4e9a6101b1bfdaa1e9c8ca93230acf0dbbb Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 15 Sep 2022 17:02:25 +1200 Subject: [PATCH 06/18] Appease the linter --- tests/phpunit/tests/theme/wpThemeJson.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 25dc3449b28a7..0df0f47fbf5d0 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3496,7 +3496,7 @@ function test_get_styles_for_block_with_content_width() { /** * @dataProvider data_generate_spacing_scale_fixtures - * + * * @ticket 56467 * * Tests generating the spacing presets array based on the spacing scale provided. @@ -3737,7 +3737,7 @@ function data_generate_spacing_scale_fixtures() { /** * @dataProvider data_set_spacing_sizes_when_invalid - * + * * @ticket 56467 * * Tests generating the spacing presets array based on the spacing scale provided. From 247307daa8b96fe2c9e87aa4b26021fc4a7ac349 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 16 Sep 2022 09:24:40 +1200 Subject: [PATCH 07/18] Remove references to gutenberg text domain --- src/wp-includes/class-wp-theme-json.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index fbdabaa2817f5..7a4234a12f1fd 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -2927,7 +2927,7 @@ public function set_spacing_sizes() { || ! is_numeric( $spacing_scale['mediumStep'] ) || ( '+' !== $spacing_scale['operator'] && '*' !== $spacing_scale['operator'] ) ) { if ( ! empty( $spacing_scale ) ) { - trigger_error( __( 'Some of the theme.json settings.spacing.spacingScale values are invalid', 'gutenberg' ), E_USER_NOTICE ); + trigger_error( __( 'Some of the theme.json settings.spacing.spacingScale values are invalid' ), E_USER_NOTICE ); } return null; } @@ -2957,7 +2957,7 @@ public function set_spacing_sizes() { $below_sizes[] = array( /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Small */ - 'name' => $x === $steps_mid_point - 1 ? __( 'Small', 'gutenberg' ) : sprintf( __( '%sX-Small', 'gutenberg' ), strval( $x_small_count ) ), + 'name' => $x === $steps_mid_point - 1 ? __( 'Small' ) : sprintf( __( '%sX-Small' ), strval( $x_small_count ) ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, ); @@ -2976,7 +2976,7 @@ public function set_spacing_sizes() { $below_sizes = array_reverse( $below_sizes ); $below_sizes[] = array( - 'name' => __( 'Medium', 'gutenberg' ), + 'name' => __( 'Medium' ), 'slug' => '50', 'size' => $spacing_scale['mediumStep'] . $unit, ); @@ -2994,7 +2994,7 @@ public function set_spacing_sizes() { $above_sizes[] = array( /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Large */ - 'name' => 0 === $x ? __( 'Large', 'gutenberg' ) : sprintf( __( '%sX-Large', 'gutenberg' ), strval( $x_large_count ) ), + 'name' => 0 === $x ? __( 'Large' ) : sprintf( __( '%sX-Large' ), strval( $x_large_count ) ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, ); From 370f2968136c0689461410eb8544c5672bd53eb7 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 16 Sep 2022 09:47:55 +1200 Subject: [PATCH 08/18] Code tidy up of set_spacing_sizes from review --- src/wp-includes/class-wp-theme-json.php | 38 ++++++++++++++----------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index 7a4234a12f1fd..ad1d56ddac9d2 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -2939,38 +2939,42 @@ public function set_spacing_sizes() { $unit = '%' === $spacing_scale['unit'] ? '%' : sanitize_title( $spacing_scale['unit'] ); $current_step = $spacing_scale['mediumStep']; - $steps_mid_point = round( ( ( $spacing_scale['steps'] ) / 2 ), 0 ); + $steps_mid_point = round( $spacing_scale['steps'] / 2, 0 ); $x_small_count = null; $below_sizes = array(); $slug = 40; $remainder = 0; - for ( $x = $steps_mid_point - 1; $spacing_scale['steps'] > 1 && $slug > 0 && $x > 0; $x-- ) { - $current_step = '+' === $spacing_scale['operator'] - ? $current_step - $spacing_scale['increment'] - : ( $spacing_scale['increment'] > 1 ? $current_step / $spacing_scale['increment'] : $current_step * $spacing_scale['increment'] ); + for ( $below_midpoint_count = $steps_mid_point - 1; $spacing_scale['steps'] > 1 && $slug > 0 && $below_midpoint_count > 0; $below_midpoint_count-- ) { + if ( '+' === $spacing_scale['operator'] ) { + $current_step -= $spacing_scale['increment']; + } elseif ( $spacing_scale['increment'] > 1 ) { + $current_step /= $spacing_scale['increment']; + } else { + $current_step *= $spacing_scale['increment']; + } if ( $current_step <= 0 ) { - $remainder = $x; + $remainder = $below_midpoint_count; break; } $below_sizes[] = array( - /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Small */ - 'name' => $x === $steps_mid_point - 1 ? __( 'Small' ) : sprintf( __( '%sX-Small' ), strval( $x_small_count ) ), + /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Small. */ + 'name' => $below_midpoint_count === $steps_mid_point - 1 ? __( 'Small' ) : sprintf( __( '%sX-Small' ), strval( $x_small_count ) ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, ); - if ( $x === $steps_mid_point - 2 ) { + if ( $below_midpoint_count === $steps_mid_point - 2 ) { $x_small_count = 2; } - if ( $x < $steps_mid_point - 2 ) { + if ( $below_midpoint_count < $steps_mid_point - 2 ) { $x_small_count++; } - $slug = $slug - 10; + $slug -= 10; } $below_sizes = array_reverse( $below_sizes ); @@ -2987,27 +2991,27 @@ public function set_spacing_sizes() { $slug = 60; $steps_above = ( $spacing_scale['steps'] - $steps_mid_point ) + $remainder; - for ( $x = 0; $x < $steps_above; $x++ ) { + for ( $above_midpoint_count = 0; $above_midpoint_count < $steps_above; $above_midpoint_count++ ) { $current_step = '+' === $spacing_scale['operator'] ? $current_step + $spacing_scale['increment'] : ( $spacing_scale['increment'] >= 1 ? $current_step * $spacing_scale['increment'] : $current_step / $spacing_scale['increment'] ); $above_sizes[] = array( - /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Large */ - 'name' => 0 === $x ? __( 'Large' ) : sprintf( __( '%sX-Large' ), strval( $x_large_count ) ), + /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Large. */ + 'name' => 0 === $above_midpoint_count ? __( 'Large' ) : sprintf( __( '%sX-Large' ), strval( $x_large_count ) ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, ); - if ( 1 === $x ) { + if ( 1 === $above_midpoint_count ) { $x_large_count = 2; } - if ( $x > 1 ) { + if ( $above_midpoint_count > 1 ) { $x_large_count++; } - $slug = $slug + 10; + $slug += 10; } _wp_array_set( $this->theme_json, array( 'settings', 'spacing', 'spacingSizes', 'default' ), array_merge( $below_sizes, $above_sizes ) ); From e0c53fd0661611c7677b48dc5718878027a9cdfe Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 16 Sep 2022 10:17:17 +1200 Subject: [PATCH 09/18] Improve translators comment --- src/wp-includes/class-wp-theme-json.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index ad1d56ddac9d2..c55f3b7c22a0e 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -2960,7 +2960,7 @@ public function set_spacing_sizes() { } $below_sizes[] = array( - /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Small. */ + /* translators: %s: Digit to indicate multiple of sizing, eg. 2X-Small. */ 'name' => $below_midpoint_count === $steps_mid_point - 1 ? __( 'Small' ) : sprintf( __( '%sX-Small' ), strval( $x_small_count ) ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, @@ -2997,7 +2997,7 @@ public function set_spacing_sizes() { : ( $spacing_scale['increment'] >= 1 ? $current_step * $spacing_scale['increment'] : $current_step / $spacing_scale['increment'] ); $above_sizes[] = array( - /* translators: %s: Multiple of t-shirt sizing, eg. 2X-Large. */ + /* translators: %s: Digit to indicate multiple of sizing, eg. 2X-Large. */ 'name' => 0 === $above_midpoint_count ? __( 'Large' ) : sprintf( __( '%sX-Large' ), strval( $x_large_count ) ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, From fe83c61230f50350e031becddce33696606ce2fe Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 16 Sep 2022 10:50:27 +1200 Subject: [PATCH 10/18] Improvements to tests from review --- tests/phpunit/tests/theme/wpThemeJson.php | 57 +++++++++++------------ 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 0df0f47fbf5d0..50482502096e0 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3495,13 +3495,16 @@ function test_get_styles_for_block_with_content_width() { } /** - * @dataProvider data_generate_spacing_scale_fixtures + * Tests generating the spacing presets array based on the spacing scale provided. * * @ticket 56467 * - * Tests generating the spacing presets array based on the spacing scale provided. + * @dataProvider data_generate_spacing_scale_fixtures + * + * @param array $spacing_scale Example spacing scale definitions from the data provider. + * @param array $expected_output Expected output from data provider. */ - function test_set_spacing_sizes( $spacing_scale, $expected_output ) { + function test_should_set_spacing_sizes( $spacing_scale, $expected_output ) { $theme_json = new WP_Theme_JSON( array( 'version' => 2, @@ -3520,11 +3523,13 @@ function test_set_spacing_sizes( $spacing_scale, $expected_output ) { /** * Data provider for spacing scale tests. * + * @ticket 56467 + * * @return array */ function data_generate_spacing_scale_fixtures() { return array( - 'one_step_spacing_scale' => array( + 'only one value when single step in spacing scale' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3540,8 +3545,7 @@ function data_generate_spacing_scale_fixtures() { ), ), ), - - 'two_step_spacing_scale_should_add_step_above_medium' => array( + 'one step above medium when two steps in spacing scale' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3562,8 +3566,7 @@ function data_generate_spacing_scale_fixtures() { ), ), ), - - 'three_step_spacing_scale_should_add_step_above_and_below_medium' => array( + 'one step above medium and one below when three steps in spacing scale' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3589,8 +3592,7 @@ function data_generate_spacing_scale_fixtures() { ), ), ), - - 'even_step_spacing_scale_steps_should_add_extra_step_above_medium' => array( + 'extra step added above medium when an even number of steps > 2 specified' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3621,8 +3623,7 @@ function data_generate_spacing_scale_fixtures() { ), ), ), - - 'if_bottom_end_will_go_below_zero_should_add_extra_steps_above_medium_instead' => array( + 'extra steps above medium if bottom end will go below zero' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 2.5, @@ -3658,8 +3659,7 @@ function data_generate_spacing_scale_fixtures() { ), ), ), - - 'multiplier_should_correctly_calculate_above_and_below_medium' => array( + 'multiplier correctly calculated above and below medium' => array( 'spacingScale' => array( 'operator' => '*', 'increment' => 1.5, @@ -3695,8 +3695,7 @@ function data_generate_spacing_scale_fixtures() { ), ), ), - - 'increment_<_1_combined_with_*_operator_should_act_as_divisor_to_calculate_above_and_below_medium' => array( + 'increment < 1 combined showing * operator acting as divisor above and below medium' => array( 'spacingScale' => array( 'operator' => '*', 'increment' => 0.25, @@ -3736,13 +3735,16 @@ function data_generate_spacing_scale_fixtures() { } /** - * @dataProvider data_set_spacing_sizes_when_invalid + * Tests generating the spacing presets array based on the spacing scale provided. * * @ticket 56467 * - * Tests generating the spacing presets array based on the spacing scale provided. + * @dataProvider data_set_spacing_sizes_when_invalid + * + * @param array $spacing_scale Example spacing scale definitions from the data provider. + * @param array $expected_output Expected output from data provider. */ - public function test_set_spacing_sizes_when_invalid( $spacing_scale, $expected_output ) { + public function test_set_spacing_sizes_validation( $spacing_scale, $expected_output ) { $this->expectNotice(); $this->expectNoticeMessage( 'Some of the theme.json settings.spacing.spacingScale values are invalid' ); @@ -3764,12 +3766,13 @@ public function test_set_spacing_sizes_when_invalid( $spacing_scale, $expected_o /** * Data provider for spacing scale tests. * + * @ticket 56467 + * * @return array */ function data_set_spacing_sizes_when_invalid() { return array( - - 'invalid_spacing_scale_values_missing_operator' => array( + 'spacing scale is missing operator value' => array( 'spacingScale' => array( 'operator' => '', 'increment' => 1.5, @@ -3779,8 +3782,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - - 'invalid_spacing_scale_values_non_numeric_increment' => array( + 'spacing scale has non numeric increment' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 'add two to previous value', @@ -3790,8 +3792,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - - 'invalid_spacing_scale_values_non_numeric_steps' => array( + 'spacing scale has non numeric steps' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3801,8 +3802,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - - 'invalid_spacing_scale_values_non_numeric_medium_step' => array( + 'spacing scale has non numeric medium step' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3812,8 +3812,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - - 'invalid_spacing_scale_values_missing_unit' => array( + 'spacing scale is missing unit value' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, From eae1c237593d0196078e10d8a752a236e3d7021a Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 16 Sep 2022 10:57:05 +1200 Subject: [PATCH 11/18] Appease the linter --- tests/phpunit/tests/theme/wpThemeJson.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 50482502096e0..2aeff417348e2 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3772,7 +3772,7 @@ public function test_set_spacing_sizes_validation( $spacing_scale, $expected_out */ function data_set_spacing_sizes_when_invalid() { return array( - 'spacing scale is missing operator value' => array( + 'spacing scale is missing operator value' => array( 'spacingScale' => array( 'operator' => '', 'increment' => 1.5, @@ -3782,7 +3782,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale has non numeric increment' => array( + 'spacing scale has non numeric increment' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 'add two to previous value', @@ -3792,7 +3792,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale has non numeric steps' => array( + 'spacing scale has non numeric steps' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, @@ -3812,7 +3812,7 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale is missing unit value' => array( + 'spacing scale is missing unit value' => array( 'spacingScale' => array( 'operator' => '+', 'increment' => 1.5, From b2a79116456b8f93fb8e6b3aa3c38377b5e2c0b6 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Tue, 20 Sep 2022 10:11:37 +1200 Subject: [PATCH 12/18] Naming fixes from code review --- tests/phpunit/tests/theme/wpThemeJson.php | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 2aeff417348e2..e02eafca30b5b 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3530,7 +3530,7 @@ function test_should_set_spacing_sizes( $spacing_scale, $expected_output ) { function data_generate_spacing_scale_fixtures() { return array( 'only one value when single step in spacing scale' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 1, @@ -3546,7 +3546,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'one step above medium when two steps in spacing scale' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 2, @@ -3567,7 +3567,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'one step above medium and one below when three steps in spacing scale' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 3, @@ -3593,7 +3593,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'extra step added above medium when an even number of steps > 2 specified' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 4, @@ -3624,7 +3624,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'extra steps above medium if bottom end will go below zero' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 2.5, 'steps' => 5, @@ -3660,7 +3660,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'multiplier correctly calculated above and below medium' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '*', 'increment' => 1.5, 'steps' => 5, @@ -3696,7 +3696,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'increment < 1 combined showing * operator acting as divisor above and below medium' => array( - 'spacingScale' => array( + 'spacing_scale' => array( 'operator' => '*', 'increment' => 0.25, 'steps' => 5, @@ -3744,7 +3744,7 @@ function data_generate_spacing_scale_fixtures() { * @param array $spacing_scale Example spacing scale definitions from the data provider. * @param array $expected_output Expected output from data provider. */ - public function test_set_spacing_sizes_validation( $spacing_scale, $expected_output ) { + public function test_set_spacing_sizes_should_detect_invalid_spacing_scale( $spacing_scale, $expected_output ) { $this->expectNotice(); $this->expectNoticeMessage( 'Some of the theme.json settings.spacing.spacingScale values are invalid' ); @@ -3772,8 +3772,8 @@ public function test_set_spacing_sizes_validation( $spacing_scale, $expected_out */ function data_set_spacing_sizes_when_invalid() { return array( - 'spacing scale is missing operator value' => array( - 'spacingScale' => array( + 'missing operator value' => array( + 'spacing_scale' => array( 'operator' => '', 'increment' => 1.5, 'steps' => 1, @@ -3782,8 +3782,8 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale has non numeric increment' => array( - 'spacingScale' => array( + 'non numeric increment' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 'add two to previous value', 'steps' => 1, @@ -3792,8 +3792,8 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale has non numeric steps' => array( - 'spacingScale' => array( + 'non numeric steps' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 'spiral staircase preferred', @@ -3802,8 +3802,8 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale has non numeric medium step' => array( - 'spacingScale' => array( + 'non numeric medium step' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 5, @@ -3812,8 +3812,8 @@ function data_set_spacing_sizes_when_invalid() { ), 'expected_output' => null, ), - 'spacing scale is missing unit value' => array( - 'spacingScale' => array( + 'missing unit value' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 5, From ea0df5240d0aa6f5fea96af9bed2a4bce63d8d16 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Tue, 20 Sep 2022 10:16:52 +1200 Subject: [PATCH 13/18] fix linting issues --- tests/phpunit/tests/theme/wpThemeJson.php | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index e02eafca30b5b..d1c08ecb43fe6 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3530,7 +3530,7 @@ function test_should_set_spacing_sizes( $spacing_scale, $expected_output ) { function data_generate_spacing_scale_fixtures() { return array( 'only one value when single step in spacing scale' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 1, @@ -3546,7 +3546,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'one step above medium when two steps in spacing scale' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 2, @@ -3567,7 +3567,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'one step above medium and one below when three steps in spacing scale' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 3, @@ -3593,7 +3593,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'extra step added above medium when an even number of steps > 2 specified' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 4, @@ -3624,7 +3624,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'extra steps above medium if bottom end will go below zero' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 2.5, 'steps' => 5, @@ -3660,7 +3660,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'multiplier correctly calculated above and below medium' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '*', 'increment' => 1.5, 'steps' => 5, @@ -3696,7 +3696,7 @@ function data_generate_spacing_scale_fixtures() { ), ), 'increment < 1 combined showing * operator acting as divisor above and below medium' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '*', 'increment' => 0.25, 'steps' => 5, @@ -3773,7 +3773,7 @@ public function test_set_spacing_sizes_should_detect_invalid_spacing_scale( $spa function data_set_spacing_sizes_when_invalid() { return array( 'missing operator value' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '', 'increment' => 1.5, 'steps' => 1, @@ -3783,7 +3783,7 @@ function data_set_spacing_sizes_when_invalid() { 'expected_output' => null, ), 'non numeric increment' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 'add two to previous value', 'steps' => 1, @@ -3793,7 +3793,7 @@ function data_set_spacing_sizes_when_invalid() { 'expected_output' => null, ), 'non numeric steps' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 'spiral staircase preferred', @@ -3803,7 +3803,7 @@ function data_set_spacing_sizes_when_invalid() { 'expected_output' => null, ), 'non numeric medium step' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 5, @@ -3813,7 +3813,7 @@ function data_set_spacing_sizes_when_invalid() { 'expected_output' => null, ), 'missing unit value' => array( - 'spacing_scale' => array( + 'spacing_scale' => array( 'operator' => '+', 'increment' => 1.5, 'steps' => 5, From bfe216b6854e0edb815a1653318401dd32d9ac01 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Tue, 20 Sep 2022 13:27:29 +1200 Subject: [PATCH 14/18] Update to show numbers for scale labels when 7 or less steps --- src/wp-includes/class-wp-theme-json.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index c55f3b7c22a0e..0cbbaf10fe9f0 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -3014,6 +3014,15 @@ public function set_spacing_sizes() { $slug += 10; } - _wp_array_set( $this->theme_json, array( 'settings', 'spacing', 'spacingSizes', 'default' ), array_merge( $below_sizes, $above_sizes ) ); + $spacing_sizes = array_merge( $below_sizes, $above_sizes ); + + // If there are 7 or less steps in the scale revert to numbers for labels instead of t-shirt sizes. + if ( $spacing_scale['steps'] <= 7 ) { + for ( $spacing_sizes_count = 0; $spacing_sizes_count < count( $spacing_sizes ); $spacing_sizes_count++ ) { + $spacing_sizes[ $spacing_sizes_count ]['name'] = strval( $spacing_sizes_count + 1 ); + } + } + + _wp_array_set( $this->theme_json, array( 'settings', 'spacing', 'spacingSizes', 'default' ), $spacing_sizes ); } } From c5a22df2098db516313ee39f84a83b3f082ff651 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Tue, 20 Sep 2022 14:08:17 +1200 Subject: [PATCH 15/18] Remove uses of strval and fix tests --- src/wp-includes/class-wp-theme-json.php | 6 +- tests/phpunit/tests/theme/wpThemeJson.php | 94 +++++++++++++++++------ 2 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index 0cbbaf10fe9f0..e2d89e26905f5 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -2961,7 +2961,7 @@ public function set_spacing_sizes() { $below_sizes[] = array( /* translators: %s: Digit to indicate multiple of sizing, eg. 2X-Small. */ - 'name' => $below_midpoint_count === $steps_mid_point - 1 ? __( 'Small' ) : sprintf( __( '%sX-Small' ), strval( $x_small_count ) ), + 'name' => $below_midpoint_count === $steps_mid_point - 1 ? __( 'Small' ) : sprintf( __( '%sX-Small' ), (string) $x_small_count ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, ); @@ -2998,7 +2998,7 @@ public function set_spacing_sizes() { $above_sizes[] = array( /* translators: %s: Digit to indicate multiple of sizing, eg. 2X-Large. */ - 'name' => 0 === $above_midpoint_count ? __( 'Large' ) : sprintf( __( '%sX-Large' ), strval( $x_large_count ) ), + 'name' => 0 === $above_midpoint_count ? __( 'Large' ) : sprintf( __( '%sX-Large' ), (string) $x_large_count ), 'slug' => (string) $slug, 'size' => round( $current_step, 2 ) . $unit, ); @@ -3019,7 +3019,7 @@ public function set_spacing_sizes() { // If there are 7 or less steps in the scale revert to numbers for labels instead of t-shirt sizes. if ( $spacing_scale['steps'] <= 7 ) { for ( $spacing_sizes_count = 0; $spacing_sizes_count < count( $spacing_sizes ); $spacing_sizes_count++ ) { - $spacing_sizes[ $spacing_sizes_count ]['name'] = strval( $spacing_sizes_count + 1 ); + $spacing_sizes[ $spacing_sizes_count ]['name'] = (string) $spacing_sizes_count + 1; } } diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index d1c08ecb43fe6..6128ba92f2dee 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3539,7 +3539,7 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'Medium', + 'name' => '1', 'slug' => '50', 'size' => '4rem', ), @@ -3555,12 +3555,12 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'Medium', + 'name' => '1', 'slug' => '50', 'size' => '4rem', ), array( - 'name' => 'Large', + 'name' => '2', 'slug' => '60', 'size' => '5.5rem', ), @@ -3576,17 +3576,17 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'Small', + 'name' => '1', 'slug' => '40', 'size' => '2.5rem', ), array( - 'name' => 'Medium', + 'name' => '2', 'slug' => '50', 'size' => '4rem', ), array( - 'name' => 'Large', + 'name' => '3', 'slug' => '60', 'size' => '5.5rem', ), @@ -3602,22 +3602,22 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'Small', + 'name' => '1', 'slug' => '40', 'size' => '2.5rem', ), array( - 'name' => 'Medium', + 'name' => '2', 'slug' => '50', 'size' => '4rem', ), array( - 'name' => 'Large', + 'name' => '3', 'slug' => '60', 'size' => '5.5rem', ), array( - 'name' => 'X-Large', + 'name' => '4', 'slug' => '70', 'size' => '7rem', ), @@ -3633,27 +3633,27 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'Small', + 'name' => '1', 'slug' => '40', 'size' => '2.5rem', ), array( - 'name' => 'Medium', + 'name' => '2', 'slug' => '50', 'size' => '5rem', ), array( - 'name' => 'Large', + 'name' => '3', 'slug' => '60', 'size' => '7.5rem', ), array( - 'name' => 'X-Large', + 'name' => '4', 'slug' => '70', 'size' => '10rem', ), array( - 'name' => '2X-Large', + 'name' => '5', 'slug' => '80', 'size' => '12.5rem', ), @@ -3669,27 +3669,27 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'X-Small', + 'name' => '1', 'slug' => '30', 'size' => '0.67rem', ), array( - 'name' => 'Small', + 'name' => '2', 'slug' => '40', 'size' => '1rem', ), array( - 'name' => 'Medium', + 'name' => '3', 'slug' => '50', 'size' => '1.5rem', ), array( - 'name' => 'Large', + 'name' => '4', 'slug' => '60', 'size' => '2.25rem', ), array( - 'name' => 'X-Large', + 'name' => '5', 'slug' => '70', 'size' => '3.38rem', ), @@ -3705,15 +3705,56 @@ function data_generate_spacing_scale_fixtures() { ), 'expected_output' => array( array( - 'name' => 'X-Small', + 'name' => '1', 'slug' => '30', 'size' => '0.09rem', ), array( - 'name' => 'Small', + 'name' => '2', 'slug' => '40', 'size' => '0.38rem', ), + array( + 'name' => 'Medium', + 'slug' => '3', + 'size' => '1.5rem', + ), + array( + 'name' => 'Large', + 'slug' => '4', + 'size' => '6rem', + ), + array( + 'name' => 'X-Large', + 'slug' => '5', + 'size' => '24rem', + ), + ), + ), + 't-shirt sizing used if more than 7 steps in scale' => array( + 'spacing_scale' => array( + 'operator' => '*', + 'increment' => 1.5, + 'steps' => 8, + 'mediumStep' => 1.5, + 'unit' => 'rem', + ), + 'expected_output' => array( + array( + 'name' => '2X-Small', + 'slug' => '20', + 'size' => '0.44rem', + ), + array( + 'name' => 'X-Small', + 'slug' => '30', + 'size' => '0.67rem', + ), + array( + 'name' => 'Small', + 'slug' => '40', + 'size' => '1rem', + ), array( 'name' => 'Medium', 'slug' => '50', @@ -3722,12 +3763,17 @@ function data_generate_spacing_scale_fixtures() { array( 'name' => 'Large', 'slug' => '60', - 'size' => '6rem', + 'size' => '2.25rem', ), array( 'name' => 'X-Large', 'slug' => '70', - 'size' => '24rem', + 'size' => '3.38rem', + ), + array( + 'name' => '2X-Large', + 'slug' => '80', + 'size' => '5.06rem', ), ), ), From d1edcd830ba31bbbf14a89d2652e6185b79186ca Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Tue, 20 Sep 2022 16:10:51 +1200 Subject: [PATCH 16/18] Make addition to spacing size before casting to string --- src/wp-includes/class-wp-theme-json.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index e2d89e26905f5..38fd88b7e0863 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -3019,7 +3019,7 @@ public function set_spacing_sizes() { // If there are 7 or less steps in the scale revert to numbers for labels instead of t-shirt sizes. if ( $spacing_scale['steps'] <= 7 ) { for ( $spacing_sizes_count = 0; $spacing_sizes_count < count( $spacing_sizes ); $spacing_sizes_count++ ) { - $spacing_sizes[ $spacing_sizes_count ]['name'] = (string) $spacing_sizes_count + 1; + $spacing_sizes[ $spacing_sizes_count ]['name'] = (string) ( $spacing_sizes_count + 1 ); } } From 23eb2a9557080695c9f598346ec272ebbc02d950 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Tue, 20 Sep 2022 16:28:51 +1200 Subject: [PATCH 17/18] Fix test failures --- tests/phpunit/tests/theme/wpThemeJson.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 6128ba92f2dee..d67b5872b91c2 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -3715,18 +3715,18 @@ function data_generate_spacing_scale_fixtures() { 'size' => '0.38rem', ), array( - 'name' => 'Medium', - 'slug' => '3', + 'name' => '3', + 'slug' => '50', 'size' => '1.5rem', ), array( - 'name' => 'Large', - 'slug' => '4', + 'name' => '4', + 'slug' => '60', 'size' => '6rem', ), array( - 'name' => 'X-Large', - 'slug' => '5', + 'name' => '5', + 'slug' => '70', 'size' => '24rem', ), ), @@ -3775,6 +3775,11 @@ function data_generate_spacing_scale_fixtures() { 'slug' => '80', 'size' => '5.06rem', ), + array( + 'name' => '3X-Large', + 'slug' => '90', + 'size' => '7.59rem', + ), ), ), ); From b0b1d6d6fe22d593ab1e4eb0dd2f96e7b31776d4 Mon Sep 17 00:00:00 2001 From: Jb Audras Date: Wed, 21 Sep 2022 12:30:44 +0200 Subject: [PATCH 18/18] small `return` mention improvement Co-authored-by: Mukesh Panchal --- src/wp-includes/class-wp-theme-json.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index 38fd88b7e0863..752cbee00d96a 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -2912,7 +2912,7 @@ public function get_data() { * * @since 6.1.0 * - * @return void + * @return null|void */ public function set_spacing_sizes() { $spacing_scale = _wp_array_get( $this->theme_json, array( 'settings', 'spacing', 'spacingScale' ), array() );