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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 24 additions & 6 deletions lib/block-supports/typography.php
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|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'`.
Expand All @@ -273,6 +273,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;
}
Expand Down Expand Up @@ -402,19 +412,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'];
}

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
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, 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)
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/README.md
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: 13 additions & 13 deletions packages/block-editor/src/components/font-sizes/fluid-utils.js
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.
* @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,20 +148,20 @@ 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 !== '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`;
}

Expand Down
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,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,
},
ramonjd marked this conversation as resolved.
Show resolved Hide resolved
{
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
);
} );
} );
} );
} );
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 All @@ -34,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,
Expand All @@ -46,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: {
Expand Down Expand Up @@ -80,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: {
Expand Down
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
Loading