From 0c093752995ccb70038aaf76f18a1ae51e0820b9 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 11 Oct 2022 09:45:37 +1100 Subject: [PATCH 1/4] Updating PHP doc to describe incoming type of $raw_value (string|int) This PR also harmonizes the JS checks And review comments from https://github.com/WordPress/gutenberg/pull/44807#pullrequestreview-1136360189 These changes have already been backported to Core in https://github.com/WordPress/wordpress-develop/pull/3428 --- lib/block-supports/typography.php | 29 +++++++++--- packages/block-editor/CHANGELOG.md | 4 ++ packages/block-editor/README.md | 2 +- .../src/components/font-sizes/fluid-utils.js | 26 +++++----- .../components/font-sizes/test/fluid-utils.js | 34 +++++++++++++- .../global-styles/test/typography-utils.js | 17 +++++++ .../global-styles/typography-utils.js | 14 ++++-- phpunit/block-supports/typography-test.php | 47 +++++++++++++++++++ 8 files changed, 151 insertions(+), 22 deletions(-) diff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php index 22dfdba97fbb8..372b150a0ea49 100644 --- 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 index e3dee2d0dc251..110f5eeb90210 100644 --- 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 index 8db908636df9c..703cd13256a5d 100644 --- 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 index 761cd52c97bc6..602ed2b29aca2 100644 --- 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,7 +148,7 @@ 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. @@ -156,7 +156,11 @@ export function getComputedFluidTypographyValue( { * @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 index a023a2826f5e4..745e761503f51 100644 --- 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 index 2ab86fb7c8292..99804a01414ac 100644 --- 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 index 1435fcec16bf6..6e26a3155ed54 100644 --- 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 index d21619a3a5286..d8d02c66c220e 100644 --- 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' ) ), + ); + } } From 0ab24b54eb21bb025a10f2d56452109f7968993a Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 11 Oct 2022 11:32:11 +1100 Subject: [PATCH 2/4] Update changelog Add extra test for floats Add i18n domain --- lib/block-supports/typography.php | 2 +- packages/block-editor/CHANGELOG.md | 2 +- .../components/global-styles/test/typography-utils.js | 11 +++++++++++ phpunit/block-supports/typography-test.php | 8 ++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php index 372b150a0ea49..3fb10c02463ff 100644 --- a/lib/block-supports/typography.php +++ b/lib/block-supports/typography.php @@ -276,7 +276,7 @@ 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.' ), + __( 'Raw size value must be a string or integer.', 'gutenberg' ), '6.1.0' ); return null; diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md index 110f5eeb90210..a8f3f1978ad74 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -4,7 +4,7 @@ ### Bug Fix -- `FontSizePicker`: Update fluid utils so that only string and integers are treated as valid font sizes for the purposes of fluid typography. +- `FontSizePicker`: Update fluid utils so that only string and integers are treated as valid font sizes for the purposes of fluid typography.([#44847](https://github.com/WordPress/gutenberg/pull/44847)) ## 10.2.0 (2022-10-05) 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 index 99804a01414ac..d87ae277cc8c0 100644 --- a/packages/edit-site/src/components/global-styles/test/typography-utils.js +++ b/packages/edit-site/src/components/global-styles/test/typography-utils.js @@ -97,6 +97,17 @@ describe( 'typography utils', () => { expected: 'clamp(1.3125rem, 1.3125rem + ((1vw - 0.48rem) * 2.524), 2.625rem)', }, + // Should return fluid value for floats with units. + { + preset: { + size: '100.175px', + }, + typographySettings: { + fluid: true, + }, + expected: + 'clamp(75.13125px, 4.695703125rem + ((1vw - 7.68px) * 9.03), 150.2625px)', + }, // Should return default fluid values with empty fluid array. { preset: { diff --git a/phpunit/block-supports/typography-test.php b/phpunit/block-supports/typography-test.php index d8d02c66c220e..8972418942d73 100644 --- a/phpunit/block-supports/typography-test.php +++ b/phpunit/block-supports/typography-test.php @@ -382,6 +382,14 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(1.3125rem, 1.3125rem + ((1vw - 0.48rem) * 2.524), 2.625rem)', ), + 'return_fluid_value_with_floats_with_units' => array( + 'font_size_preset' => array( + 'size' => '100.175px', + ), + 'should_use_fluid_typography' => true, + 'expected_output' => 'clamp(75.13125px, 4.695703125rem + ((1vw - 7.68px) * 9.03), 150.2625px)', + ), + 'return_fluid_value_with_number_coerced_to_px' => array( 'font_size_preset' => array( 'size' => 33, From 95103ae86f967223475e0ab95644a749053be54b Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 11 Oct 2022 12:41:17 +1100 Subject: [PATCH 3/4] Font sizes can be string|int|float Updated tests and JSDoc type --- lib/block-supports/typography.php | 14 ++++---- packages/block-editor/CHANGELOG.md | 2 +- packages/block-editor/README.md | 2 +- .../src/components/font-sizes/fluid-utils.js | 22 +++++------- .../components/font-sizes/test/fluid-utils.js | 4 --- .../global-styles/test/typography-utils.js | 15 +++++++- phpunit/block-supports/typography-test.php | 35 +++++++++++-------- 7 files changed, 53 insertions(+), 41 deletions(-) diff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php index 3fb10c02463ff..931f1ce1087f0 100644 --- 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|int $raw_value Raw size value from theme.json. - * @param array $options { + * @param string|int|float $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,10 +273,10 @@ 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 ) ) { + if ( ! is_string( $raw_value ) && ! is_int( $raw_value ) && ! is_float( $raw_value ) ) { _doing_it_wrong( __FUNCTION__, - __( 'Raw size value must be a string or integer.', 'gutenberg' ), + __( 'Raw size value must be a string, integer or a float.', 'gutenberg' ), '6.1.0' ); return null; @@ -411,9 +411,9 @@ 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|int $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|float $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`. * diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md index a8f3f1978ad74..5c3780fc52666 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -4,7 +4,7 @@ ### Bug Fix -- `FontSizePicker`: Update fluid utils so that only string and integers are treated as valid font sizes for the purposes of fluid typography.([#44847](https://github.com/WordPress/gutenberg/pull/44847)) +- `FontSizePicker`: Update fluid utils so that only string, floats integers are treated as valid font sizes for the purposes of fluid typography.([#44847](https://github.com/WordPress/gutenberg/pull/44847)) ## 10.2.0 (2022-10-05) diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md index 703cd13256a5d..d30f527997028 100644 --- 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|?number`: 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 index 602ed2b29aca2..0c48892600300 100644 --- 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|?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. + * @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(). */ @@ -156,10 +156,6 @@ export function getComputedFluidTypographyValue( { * @return {{ unit: string, value: number }|null} An object consisting of `'value'` and `'unit'` properties. */ export function getTypographyValueAndUnit( rawValue, options = {} ) { - 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 index 745e761503f51..bc769ac2ab9b2 100644 --- a/packages/block-editor/src/components/font-sizes/test/fluid-utils.js +++ b/packages/block-editor/src/components/font-sizes/test/fluid-utils.js @@ -93,10 +93,6 @@ describe( 'getComputedFluidTypographyValue()', () => { value: true, expected: null, }, - { - value: 10.1234, - expected: null, - }, { value: [ '10' ], expected: null, 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 index d87ae277cc8c0..97ebeb583e497 100644 --- a/packages/edit-site/src/components/global-styles/test/typography-utils.js +++ b/packages/edit-site/src/components/global-styles/test/typography-utils.js @@ -51,7 +51,7 @@ describe( 'typography utils', () => { }, expected: '28px', }, - // Should coerce number to `px` and return fluid value. + // Should coerce integer to `px` and return fluid value. { preset: { size: 33, @@ -63,6 +63,19 @@ describe( 'typography utils', () => { expected: 'clamp(24.75px, 1.546875rem + ((1vw - 7.68px) * 2.975), 49.5px)', }, + + // Should coerce float to `px` and return fluid value. + { + preset: { + size: 100.23, + fluid: true, + }, + typographySettings: { + fluid: true, + }, + expected: + 'clamp(75.1725px, 4.69828125rem + ((1vw - 7.68px) * 9.035), 150.345px)', + }, // Should return incoming value when already clamped. { preset: { diff --git a/phpunit/block-supports/typography-test.php b/phpunit/block-supports/typography-test.php index 8972418942d73..4e5fb5126cc21 100644 --- a/phpunit/block-supports/typography-test.php +++ b/phpunit/block-supports/typography-test.php @@ -315,7 +315,7 @@ public function test_gutenberg_get_typography_font_size_value( $font_size_preset */ public function data_generate_font_size_preset_fixtures() { return array( - 'default_return_value' => array( + 'default_return_value' => array( 'font_size_preset' => array( 'size' => '28px', ), @@ -323,7 +323,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => '28px', ), - 'size: int 0' => array( + 'size: int 0' => array( 'font_size_preset' => array( 'size' => 0, ), @@ -331,7 +331,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 0, ), - 'size: string 0' => array( + 'size: string 0' => array( 'font_size_preset' => array( 'size' => '0', ), @@ -339,7 +339,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => '0', ), - 'default_return_value_when_size_is_undefined' => array( + 'default_return_value_when_size_is_undefined' => array( 'font_size_preset' => array( 'size' => null, ), @@ -347,7 +347,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => null, ), - 'default_return_value_when_fluid_is_false' => array( + 'default_return_value_when_fluid_is_false' => array( 'font_size_preset' => array( 'size' => '28px', 'fluid' => false, @@ -365,7 +365,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(21px, 1.3125rem + ((1vw - 7.68px) * 2.524), 42px)', ), - 'default_return_value_with_unsupported_unit' => array( + 'default_return_value_with_unsupported_unit' => array( 'font_size_preset' => array( 'size' => '1000%', 'fluid' => false, @@ -374,7 +374,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => '1000%', ), - 'return_fluid_value' => array( + 'return_fluid_value' => array( 'font_size_preset' => array( 'size' => '1.75rem', ), @@ -382,7 +382,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(1.3125rem, 1.3125rem + ((1vw - 0.48rem) * 2.524), 2.625rem)', ), - 'return_fluid_value_with_floats_with_units' => array( + 'return_fluid_value_with_floats_with_units' => array( 'font_size_preset' => array( 'size' => '100.175px', ), @@ -390,7 +390,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(75.13125px, 4.695703125rem + ((1vw - 7.68px) * 9.03), 150.2625px)', ), - 'return_fluid_value_with_number_coerced_to_px' => array( + 'return_fluid_value_with_integer_coerced_to_px' => array( 'font_size_preset' => array( 'size' => 33, ), @@ -398,6 +398,14 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(24.75px, 1.546875rem + ((1vw - 7.68px) * 2.975), 49.5px)', ), + 'return_fluid_value_with_float_coerced_to_px' => array( + 'font_size_preset' => array( + 'size' => 100.23, + ), + 'should_use_fluid_typography' => true, + 'expected_output' => 'clamp(75.1725px, 4.69828125rem + ((1vw - 7.68px) * 9.035), 150.345px)', + ), + 'return_default_fluid_values_with_empty_fluid_array' => array( 'font_size_preset' => array( 'size' => '28px', @@ -407,7 +415,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(21px, 1.3125rem + ((1vw - 7.68px) * 2.524), 42px)', ), - 'return_default_fluid_values_with_null_value' => array( + 'return_default_fluid_values_with_null_value' => array( 'font_size_preset' => array( 'size' => '28px', 'fluid' => null, @@ -416,7 +424,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => 'clamp(21px, 1.3125rem + ((1vw - 7.68px) * 2.524), 42px)', ), - 'return_size_with_invalid_fluid_units' => array( + 'return_size_with_invalid_fluid_units' => array( 'font_size_preset' => array( 'size' => '10em', 'fluid' => array( @@ -428,7 +436,7 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => '10em', ), - 'return_fluid_clamp_value' => array( + 'return_fluid_clamp_value' => array( 'font_size_preset' => array( 'size' => '28px', 'fluid' => array( @@ -547,7 +555,7 @@ public function data_generate_replace_inline_font_styles_with_fluid_values_fixtu } /** - * Tests generating font size values, including fluid formulae, from fontSizes preset. + * Tests that invalid font size values are not parsed. * * @ticket 56467 * @@ -572,7 +580,6 @@ public function data_invalid_size_wp_get_typography_value_and_unit() { 'size: null' => array( null ), 'size: false' => array( false ), 'size: true' => array( true ), - 'size: float' => array( 10.1234 ), 'size: array' => array( array( '10' ) ), ); } From f10d15b1310f705c420e40c950e750e44e00157a Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 11 Oct 2022 14:25:38 +1100 Subject: [PATCH 4/4] Expand tests for gutenberg_get_typography_value_and_unit Fix typo in CHANGELOG.md --- lib/block-supports/typography.php | 1 + packages/block-editor/CHANGELOG.md | 2 +- .../src/components/font-sizes/fluid-utils.js | 4 +- .../components/font-sizes/test/fluid-utils.js | 65 +++++++++++++- phpunit/block-supports/typography-test.php | 89 ++++++++++++++++++- 5 files changed, 156 insertions(+), 5 deletions(-) diff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php index 931f1ce1087f0..587fb26463590 100644 --- a/lib/block-supports/typography.php +++ b/lib/block-supports/typography.php @@ -282,6 +282,7 @@ function gutenberg_get_typography_value_and_unit( $raw_value, $options = array() return null; } + // Converts numeric values to pixel values by default. if ( empty( $raw_value ) ) { return null; } diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md index 5c3780fc52666..e2230b0c3ff1e 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -4,7 +4,7 @@ ### Bug Fix -- `FontSizePicker`: Update fluid utils so that only string, floats integers are treated as valid font sizes for the purposes of fluid typography.([#44847](https://github.com/WordPress/gutenberg/pull/44847)) +- `FontSizePicker`: Update fluid utils so that only string, floats and integers are treated as valid font sizes for the purposes of fluid typography.([#44847](https://github.com/WordPress/gutenberg/pull/44847)) ## 10.2.0 (2022-10-05) diff --git a/packages/block-editor/src/components/font-sizes/fluid-utils.js b/packages/block-editor/src/components/font-sizes/fluid-utils.js index 0c48892600300..7294a83da106f 100644 --- a/packages/block-editor/src/components/font-sizes/fluid-utils.js +++ b/packages/block-editor/src/components/font-sizes/fluid-utils.js @@ -160,8 +160,8 @@ export function getTypographyValueAndUnit( rawValue, options = {} ) { return null; } - // Converts numbers to pixel values by default. - if ( typeof rawValue === 'number' ) { + // Converts numeric values to pixel values by default. + if ( isFinite( rawValue ) ) { rawValue = `${ rawValue }px`; } 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 index bc769ac2ab9b2..cd45c3593e1a4 100644 --- a/packages/block-editor/src/components/font-sizes/test/fluid-utils.js +++ b/packages/block-editor/src/components/font-sizes/test/fluid-utils.js @@ -97,8 +97,71 @@ describe( 'getComputedFluidTypographyValue()', () => { value: [ '10' ], expected: null, }, + { + value: '10vh', + expected: null, + }, + { + value: 'calc(2 * 10px)', + expected: null, + }, + { + value: 'clamp(15px, 0.9375rem + ((1vw - 7.68px) * 5.409), 60px)', + expected: null, + }, + { + value: '10', + expected: { + value: 10, + unit: 'px', + }, + }, + { + value: 11, + expected: { + value: 11, + unit: 'px', + }, + }, + { + value: 11.234, + expected: { + value: 11.234, + unit: 'px', + }, + }, + { + value: '12rem', + expected: { + value: 12, + unit: 'rem', + }, + }, + { + value: '12px', + expected: { + value: 12, + unit: 'px', + }, + }, + { + value: '12em', + expected: { + value: 12, + unit: 'em', + }, + }, + { + value: '12.74em', + expected: { + value: 12.74, + unit: 'em', + }, + }, ].forEach( ( { value, expected } ) => { - expect( getTypographyValueAndUnit( value ) ).toBe( expected ); + expect( getTypographyValueAndUnit( value ) ).toEqual( + expected + ); } ); } ); } ); diff --git a/phpunit/block-supports/typography-test.php b/phpunit/block-supports/typography-test.php index 4e5fb5126cc21..6bd1200e17d74 100644 --- a/phpunit/block-supports/typography-test.php +++ b/phpunit/block-supports/typography-test.php @@ -555,7 +555,94 @@ public function data_generate_replace_inline_font_styles_with_fluid_values_fixtu } /** - * Tests that invalid font size values are not parsed. + * Tests that valid font size values are parsed. + * + * @ticket 56467 + * + * @covers ::gutenberg_get_typography_value_and_unit + * + * @dataProvider data_valid_size_wp_get_typography_value_and_unit + * + * @param mixed $raw_value Raw size value to test. + * @param mixed $expected An expected return value. + */ + public function test_valid_size_wp_get_typography_value_and_unit( $raw_value, $expected ) { + $this->assertEquals( $expected, gutenberg_get_typography_value_and_unit( $raw_value ) ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_valid_size_wp_get_typography_value_and_unit() { + return array( + 'size: 10vh with default units do not match' => array( + 'raw_value' => '10vh', + 'expected' => null, + ), + 'size: calc() values do not match' => array( + 'raw_value' => 'calc(2 * 10px)', + 'expected' => null, + ), + 'size: clamp() values do not match' => array( + 'raw_value' => 'clamp(15px, 0.9375rem + ((1vw - 7.68px) * 5.409), 60px)', + 'expected' => null, + ), + 'size: `"10"`' => array( + 'raw_value' => '10', + 'expected' => array( + 'value' => 10, + 'unit' => 'px', + ), + ), + 'size: `11`' => array( + 'raw_value' => 11, + 'expected' => array( + 'value' => 11, + 'unit' => 'px', + ), + ), + 'size: `11.234`' => array( + 'raw_value' => '11.234', + 'expected' => array( + 'value' => 11.234, + 'unit' => 'px', + ), + ), + 'size: `"12rem"`' => array( + 'raw_value' => '12rem', + 'expected' => array( + 'value' => 12, + 'unit' => 'rem', + ), + ), + 'size: `"12px"`' => array( + 'raw_value' => '12px', + 'expected' => array( + 'value' => 12, + 'unit' => 'px', + ), + ), + 'size: `"12em"`' => array( + 'raw_value' => '12em', + 'expected' => array( + 'value' => 12, + 'unit' => 'em', + ), + ), + 'size: `"12.74em"`' => array( + 'raw_value' => '12.74em', + 'expected' => array( + 'value' => 12.74, + 'unit' => 'em', + ), + ), + ); + } + + /** + * Tests that invalid font size values are not parsed and trigger incorrect usage. * * @ticket 56467 *