From fe22cdaa85e712d9cdaf9d8567ae94fa31cc8395 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 11 Oct 2022 15:35:14 +1100 Subject: [PATCH] Fluid typography: ensure fontsizes are strings or integers (#44847) * 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 * Update changelog Add extra test for floats Add i18n domain * Font sizes can be string|int|float Updated tests and JSDoc type * Expand tests for gutenberg_get_typography_value_and_unit Fix typo in CHANGELOG.md --- lib/block-supports/typography.php | 30 +++- packages/block-editor/CHANGELOG.md | 8 + packages/block-editor/README.md | 2 +- .../src/components/font-sizes/fluid-utils.js | 26 +-- .../components/font-sizes/test/fluid-utils.js | 93 +++++++++- .../global-styles/test/typography-utils.js | 44 ++++- .../global-styles/typography-utils.js | 36 ++-- phpunit/block-supports/typography-test.php | 167 +++++++++++++++++- 8 files changed, 365 insertions(+), 41 deletions(-) diff --git a/lib/block-supports/typography.php b/lib/block-supports/typography.php index 9cd0197e1cb21..2126aaf847fde 100644 --- a/lib/block-supports/typography.php +++ b/lib/block-supports/typography.php @@ -266,8 +266,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|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'`. @@ -277,6 +277,16 @@ 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 ) && ! is_float( $raw_value ) ) { + _doing_it_wrong( + __FUNCTION__, + __( 'Raw size value must be a string, integer or a float.', 'gutenberg' ), + '6.1.0' + ); + return null; + } + + // Converts numeric values to pixel values by default. if ( empty( $raw_value ) ) { return null; } @@ -406,19 +416,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|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`. * * @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 aabc43c6ea99c..e2230b0c3ff1e 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +### Bug Fix + +- `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) + +## 10.1.0 (2022-09-21) + ## 10.0.0 (2022-09-13) ### Breaking change diff --git a/packages/block-editor/README.md b/packages/block-editor/README.md index 8db908636df9c..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`: 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..7294a83da106f 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,12 +156,12 @@ 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 !== 'string' && typeof rawValue !== 'number' ) { 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 a023a2826f5e4..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 @@ -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,92 @@ 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' ], + 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 ) ).toEqual( + 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 afd26054bd072..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 @@ -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: { @@ -34,10 +51,11 @@ describe( 'typography utils', () => { }, expected: '28px', }, - // Should coerce number to `px` and return non-fluid value. + // Should coerce integer to `px` and return fluid value. { preset: { size: 33, + fluid: true, }, typographySettings: { fluid: true, @@ -45,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: { @@ -79,6 +110,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/packages/edit-site/src/components/global-styles/typography-utils.js b/packages/edit-site/src/components/global-styles/typography-utils.js index 1f54549a2c278..6e26a3155ed54 100644 --- a/packages/edit-site/src/components/global-styles/typography-utils.js +++ b/packages/edit-site/src/components/global-styles/typography-utils.js @@ -9,25 +9,41 @@ */ 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. + */ + +/** + * @typedef {Object} Preset + * @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 + */ + /** * Returns a font-size value based on a given font-size preset. * Takes into account fluid typography parameters and attempts to return a css formula depending on available, valid values. * - * @param {Object} preset - * @param {string} preset.size A default font size. - * @param {string} preset.name A font size name, displayed in the UI. - * @param {string} preset.slug A font size slug. - * @param {Object} preset.fluid - * @param {string|undefined} preset.fluid.max A maximum font size value. - * @param {string|undefined} preset.fluid.min A minimum font size value. - * @param {Object} typographySettings - * @param {boolean} typographySettings.fluid Whether fluid typography is enabled. + * @param {Preset} preset + * @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 23df95afa1b9b..606904de72bf0 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,23 @@ public function data_generate_font_size_preset_fixtures() { 'expected_output' => '28px', ), - 'default_return_value_when_size_is_undefined' => array( + '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, ), @@ -331,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, @@ -349,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, @@ -358,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', ), @@ -366,7 +382,15 @@ 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_number_coerced_to_px' => array( + '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_integer_coerced_to_px' => array( 'font_size_preset' => array( 'size' => 33, ), @@ -374,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', @@ -383,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, @@ -392,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( @@ -404,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( @@ -594,4 +626,121 @@ public function data_generate_replace_inline_font_styles_with_fluid_values_fixtu ), ); } + + /** + * 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 + * + * @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: array' => array( array( '10' ) ), + ); + } }