Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Fluid typography: ensure fontsizes are strings or integers #44847

Merged
merged 4 commits into from
Oct 11, 2022
Merged

Fluid typography: ensure fontsizes are strings or integers #44847

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c09375
Updating PHP doc to describe incoming type of $raw_value (string|int)
ramonjd Oct 10, 2022
0ab24b5
Update changelog
ramonjd Oct 11, 2022
95103ae
Font sizes can be string|int|float
ramonjd Oct 11, 2022
f10d15b
Expand tests for gutenberg_get_typography_value_and_unit
ramonjd Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions lib/block-supports/typography.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'`.
Expand All @@ -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.', 'gutenberg' ),
'6.1.0'
);
return null;
}

if ( empty( $raw_value ) ) {
return null;
}
Expand Down Expand Up @@ -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'];
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll also have to backport this bit, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was hoping we can piggy back onto WordPress/wordpress-develop#3431 🤞

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be able to fairly smoothly 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I've pulled the changes here into WordPress/wordpress-develop#3431

// 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;
Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.([#44847](https://github.com/WordPress/gutenberg/pull/44847))

## 10.2.0 (2022-10-05)

## 10.1.0 (2022-09-21)
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 15 additions & 11 deletions packages/block-editor/src/components/font-sizes/fluid-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
* @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().
*/
Expand Down Expand Up @@ -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;
}

Expand Down
34 changes: 33 additions & 1 deletion packages/block-editor/src/components/font-sizes/test/fluid-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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( () => {
Expand Down Expand Up @@ -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,
},
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
].forEach( ( { value, expected } ) => {
expect( getTypographyValueAndUnit( value ) ).toBe( expected );
} );
} );
} );
} );
28 changes: 28 additions & 0 deletions packages/edit-site/src/components/global-styles/test/typography-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -80,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: {
Expand Down
14 changes: 11 additions & 3 deletions packages/edit-site/src/components/global-styles/typography-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down
55 changes: 55 additions & 0 deletions phpunit/block-supports/typography-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -366,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,
Expand Down Expand Up @@ -521,4 +545,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' ) ),
);
}
}