Updating PHP doc to describe incoming type of $raw_value (string|int)

This PR also harmonizes the JS checks And review comments from #44807 (review) These changes have already been backported to Core in WordPress/wordpress-develop#3428
WordPress · Oct 11, 2022 · 0c09375 · 0c09375
1 parent d20cc8b
commit 0c09375
Show file tree

Hide file tree

Showing 8 changed files with 151 additions and 22 deletions.
diff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php
@@ -262,8 +262,8 @@ function gutenberg_render_typography_support( $block_content, $block ) {
  *
  * @access private
  *
- * @param string|number $raw_value Raw size value from theme.json.
- * @param array         $options   {
+ * @param string|int $raw_value Raw size value from theme.json.
+ * @param array      $options   {
  *     Optional. An associative array of options. Default is empty array.
  *
  *     @type string        $coerce_to        Coerce the value to rem or px. Default `'rem'`.
@@ -273,6 +273,15 @@ function gutenberg_render_typography_support( $block_content, $block ) {
  * @return array An array consisting of `'value'` and `'unit'` properties.
  */
 function gutenberg_get_typography_value_and_unit( $raw_value, $options = array() ) {
+	if ( ! is_string( $raw_value ) && ! is_int( $raw_value ) ) {
+		_doing_it_wrong(
+			__FUNCTION__,
+			__( 'Raw size value must be a string or integer.' ),
+			'6.1.0'
+		);
+		return null;
+	}
+
 	if ( empty( $raw_value ) ) {
 		return null;
 	}
@@ -402,19 +411,27 @@ function gutenberg_get_computed_fluid_typography_value( $args = array() ) {
  * @param array $preset                     {
  *     Required. fontSizes preset value as seen in theme.json.
  *
- *     @type string $name Name of the font size preset.
- *     @type string $slug Kebab-case unique identifier for the font size preset.
- *     @type string $size CSS font-size value, including units where applicable.
+ *     @type string     $name Name of the font size preset.
+ *     @type string     $slug Kebab-case unique identifier for the font size preset.
+ *     @type string|int $size CSS font-size value, including units where applicable.
  * }
  * @param bool  $should_use_fluid_typography An override to switch fluid typography "on". Can be used for unit testing. Default is `false`.
  *
  * @return string|null Font-size value or `null` if a size is not passed in $preset.
  */
 function gutenberg_get_typography_font_size_value( $preset, $should_use_fluid_typography = false ) {
-	if ( ! isset( $preset['size'] ) || empty( $preset['size'] ) ) {
+	if ( ! isset( $preset['size'] ) ) {
 		return null;
 	}
 
+	/*
+	 * Catches empty values and 0/'0'.
+	 * Fluid calculations cannot be performed on 0.
+	 */
+	if ( empty( $preset['size'] ) ) {
+		return $preset['size'];
+	}
+
 	// Check if fluid font sizes are activated.
 	$typography_settings         = gutenberg_get_global_settings( array( 'typography' ) );
 	$should_use_fluid_typography = isset( $typography_settings['fluid'] ) && true === $typography_settings['fluid'] ? true : $should_use_fluid_typography;

diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Bug Fix
+
+-   `FontSizePicker`: Update fluid utils so that only string and integers are treated as valid font sizes for the purposes of fluid typography.
+
 ## 10.2.0 (2022-10-05)
 
 ## 10.1.0 (2022-09-21)

diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md
@@ -423,7 +423,7 @@ _Parameters_
 -   _args_ `Object`:
 -   _args.minimumViewPortWidth_ `?string`: Minimum viewport size from which type will have fluidity. Optional if fontSize is specified.
 -   _args.maximumViewPortWidth_ `?string`: Maximum size up to which type will have fluidity. Optional if fontSize is specified.
--   _args.fontSize_ `?string`: Size to derive maximumFontSize and minimumFontSize from, if necessary. Optional if minimumFontSize and maximumFontSize are specified.
+-   _args.fontSize_ `?string|?number`: Size to derive maximumFontSize and minimumFontSize from, if necessary. Optional if minimumFontSize and maximumFontSize are specified.
 -   _args.maximumFontSize_ `?string`: Maximum font size for any clamp() calculation. Optional.
 -   _args.minimumFontSize_ `?string`: Minimum font size for any clamp() calculation. Optional.
 -   _args.scaleFactor_ `?number`: A scale factor to determine how fast a font scales within boundaries. Optional.

diff --git a/packages/block-editor/src/components/font-sizes/fluid-utils.js b/packages/block-editor/src/components/font-sizes/fluid-utils.js
@@ -32,15 +32,15 @@ const DEFAULT_MAXIMUM_FONT_SIZE_FACTOR = 1.5;
  * } );
  * ```
  *
- * @param {Object}  args
- * @param {?string} args.minimumViewPortWidth  Minimum viewport size from which type will have fluidity. Optional if fontSize is specified.
- * @param {?string} args.maximumViewPortWidth  Maximum size up to which type will have fluidity. Optional if fontSize is specified.
- * @param {?string} args.fontSize              Size to derive maximumFontSize and minimumFontSize from, if necessary. Optional if minimumFontSize and maximumFontSize are specified.
- * @param {?string} args.maximumFontSize       Maximum font size for any clamp() calculation. Optional.
- * @param {?string} args.minimumFontSize       Minimum font size for any clamp() calculation. Optional.
- * @param {?number} args.scaleFactor           A scale factor to determine how fast a font scales within boundaries. Optional.
- * @param {?number} args.minimumFontSizeFactor How much to scale defaultFontSize by to derive minimumFontSize. Optional.
- * @param {?number} args.maximumFontSizeFactor How much to scale defaultFontSize by to derive maximumFontSize. Optional.
+ * @param {Object}          args
+ * @param {?string}         args.minimumViewPortWidth  Minimum viewport size from which type will have fluidity. Optional if fontSize is specified.
+ * @param {?string}         args.maximumViewPortWidth  Maximum size up to which type will have fluidity. Optional if fontSize is specified.
+ * @param {?string|?number} args.fontSize              Size to derive maximumFontSize and minimumFontSize from, if necessary. Optional if minimumFontSize and maximumFontSize are specified.
+ * @param {?string}         args.maximumFontSize       Maximum font size for any clamp() calculation. Optional.
+ * @param {?string}         args.minimumFontSize       Minimum font size for any clamp() calculation. Optional.
+ * @param {?number}         args.scaleFactor           A scale factor to determine how fast a font scales within boundaries. Optional.
+ * @param {?number}         args.minimumFontSizeFactor How much to scale defaultFontSize by to derive minimumFontSize. Optional.
+ * @param {?number}         args.maximumFontSizeFactor How much to scale defaultFontSize by to derive maximumFontSize. Optional.
  *
  * @return {string|null} A font-size value using clamp().
  */
@@ -148,15 +148,19 @@ export function getComputedFluidTypographyValue( {
 
 /**
  * Internal method that checks a string for a unit and value and returns an array consisting of `'value'` and `'unit'`, e.g., [ '42', 'rem' ].
- * A raw font size of `value + unit` is expected. If the value is a number, it will convert to `value + 'px'`.
+ * A raw font size of `value + unit` is expected. If the value is an integer, it will convert to `value + 'px'`.
  *
  * @param {string|number}    rawValue Raw size value from theme.json.
  * @param {Object|undefined} options  Calculation options.
  *
  * @return {{ unit: string, value: number }|null} An object consisting of `'value'` and `'unit'` properties.
  */
 export function getTypographyValueAndUnit( rawValue, options = {} ) {
-	if ( ! rawValue ) {
+	if ( typeof rawValue === 'number' && ! Number.isInteger( rawValue ) ) {
+		return null;
+	}
+
+	if ( typeof rawValue !== 'string' && typeof rawValue !== 'number' ) {
 		return null;
 	}
 

diff --git a/packages/block-editor/src/components/font-sizes/test/fluid-utils.js b/packages/block-editor/src/components/font-sizes/test/fluid-utils.js
@@ -6,7 +6,10 @@ import { logged } from '@wordpress/deprecated';
 /**
  * Internal dependencies
  */
-import { getComputedFluidTypographyValue } from '../fluid-utils';
+import {
+	getComputedFluidTypographyValue,
+	getTypographyValueAndUnit,
+} from '../fluid-utils';
 
 describe( 'getComputedFluidTypographyValue()', () => {
 	afterEach( () => {
@@ -74,4 +77,33 @@ describe( 'getComputedFluidTypographyValue()', () => {
 			'clamp(15px, 0.9375rem + ((1vw - 7.68px) * 5.409), 60px)'
 		);
 	} );
+
+	describe( 'getTypographyValueAndUnit', () => {
+		it( 'should return the expected return values', () => {
+			[
+				{
+					value: null,
+					expected: null,
+				},
+				{
+					value: false,
+					expected: null,
+				},
+				{
+					value: true,
+					expected: null,
+				},
+				{
+					value: 10.1234,
+					expected: null,
+				},
+				{
+					value: [ '10' ],
+					expected: null,
+				},
+			].forEach( ( { value, expected } ) => {
+				expect( getTypographyValueAndUnit( value ) ).toBe( expected );
+			} );
+		} );
+	} );
 } );
diff --git a/packages/edit-site/src/components/global-styles/test/typography-utils.js b/packages/edit-site/src/components/global-styles/test/typography-utils.js
@@ -15,6 +15,23 @@ describe( 'typography utils', () => {
 					typographySettings: undefined,
 					expected: '28px',
 				},
+				// Default return value where font size is 0.
+				{
+					preset: {
+						size: 0,
+					},
+					typographySettings: undefined,
+					expected: 0,
+				},
+				// Default return value where font size is '0'.
+				{
+					preset: {
+						size: '0',
+					},
+					typographySettings: undefined,
+					expected: '0',
+				},
+
 				// Default return non-fluid value where `size` is undefined.
 				{
 					preset: {

diff --git a/packages/edit-site/src/components/global-styles/typography-utils.js b/packages/edit-site/src/components/global-styles/typography-utils.js
@@ -12,12 +12,12 @@ import { getComputedFluidTypographyValue } from '@wordpress/block-editor';
 /**
  * @typedef {Object} FluidPreset
  * @property {string|undefined} max A maximum font size value.
- * @property {string|undefined} min A minimum font size value.
+ * @property {?string|undefined} min A minimum font size value.
  */
 
 /**
  * @typedef {Object} Preset
- * @property {string}                        size  A default font size.
+ * @property {?string|?number}               size  A default font size.
  * @property {string}                        name  A font size name, displayed in the UI.
  * @property {string}                        slug  A font size slug
  * @property {boolean|FluidPreset|undefined} fluid A font size slug
@@ -31,11 +31,19 @@ import { getComputedFluidTypographyValue } from '@wordpress/block-editor';
  * @param {Object}  typographySettings
  * @param {boolean} typographySettings.fluid Whether fluid typography is enabled.
  *
- * @return {string} An font-size value
+ * @return {string|*} A font-size value or the value of preset.size.
  */
 export function getTypographyFontSizeValue( preset, typographySettings ) {
 	const { size: defaultSize } = preset;
 
+	/*
+	 * Catches falsy values and 0/'0'.
+	 * Fluid calculations cannot be performed on 0.
+	 */
+	if ( ! defaultSize || '0' === defaultSize ) {
+		return defaultSize;
+	}
+
 	if ( true !== typographySettings?.fluid ) {
 		return defaultSize;
 	}

diff --git a/phpunit/block-supports/typography-test.php b/phpunit/block-supports/typography-test.php
@@ -323,6 +323,22 @@ public function data_generate_font_size_preset_fixtures() {
 				'expected_output'             => '28px',
 			),
 
+			'size: int 0'                                  => array(
+				'font_size_preset'            => array(
+					'size' => 0,
+				),
+				'should_use_fluid_typography' => true,
+				'expected_output'             => 0,
+			),
+
+			'size: string 0'                               => array(
+				'font_size_preset'            => array(
+					'size' => '0',
+				),
+				'should_use_fluid_typography' => true,
+				'expected_output'             => '0',
+			),
+
 			'default_return_value_when_size_is_undefined'  => array(
 				'font_size_preset'            => array(
 					'size' => null,
@@ -521,4 +537,35 @@ public function data_generate_replace_inline_font_styles_with_fluid_values_fixtu
 			),
 		);
 	}
+
+	/**
+	 * Tests generating font size values, including fluid formulae, from fontSizes preset.
+	 *
+	 * @ticket 56467
+	 *
+	 * @covers ::gutenberg_get_typography_value_and_unit
+	 *
+	 * @dataProvider data_invalid_size_wp_get_typography_value_and_unit
+	 * @expectedIncorrectUsage gutenberg_get_typography_value_and_unit
+	 *
+	 * @param mixed $raw_value Raw size value to test.
+	 */
+	public function test_invalid_size_wp_get_typography_value_and_unit( $raw_value ) {
+		$this->assertNull( gutenberg_get_typography_value_and_unit( $raw_value ) );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array
+	 */
+	public function data_invalid_size_wp_get_typography_value_and_unit() {
+		return array(
+			'size: null'  => array( null ),
+			'size: false' => array( false ),
+			'size: true'  => array( true ),
+			'size: float' => array( 10.1234 ),
+			'size: array' => array( array( '10' ) ),
+		);
+	}
 }