Skip to content

Commit

Permalink
Fluid Typography: Fix bug in global styles where fluid clamp rules we…
Browse files Browse the repository at this point in the history
…re not calculated for custom values (#44761)

* Fluid Typography: Fix bug where fluid clamp rules were not calculated for custom global styles values

* Add inline comments

* Add tests for JS changes

* Fluid Typography: ensure global styles preset fluid sizes are calculated correctly (#44791)

* Forked #44761

* Ensuring the font size picker select box shows the right labels

* update comment. Typescript has beaten me. It's much more convenient to use getFontSizeOptions(), but I guess we'll have to refactor.

* Adding comment about Typescript bug (YAY, it wasn't me being dumb with TS for once)

* Added tests yo

* Changeo loggo

* Create a new FontSizeSelectOption return type for getSelectedOption to:
1. Clean up type changes in #44791
2. Make TS linter be quiet

* Revert accidental changes to emptytheme

* Revert type changes and other calamities

* Remove additional size value from getToggleGroupOptions test as I believe it is no longer expected

Co-authored-by: Ramon <[email protected]>
Co-authored-by: ramonjd <[email protected]>
  • Loading branch information
3 people committed Oct 11, 2022
1 parent 0a95882 commit 7f891cc
Show file tree
Hide file tree
Showing 9 changed files with 167 additions and 20 deletions.
12 changes: 12 additions & 0 deletions lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,18 @@ protected static function compute_style_properties( $styles, $settings = array()
continue;
}

// Calculates fluid typography rules where available.
if ( 'font-size' === $css_property ) {
/*
* gutenberg_get_typography_font_size_value() will check
* if fluid typography has been activated and also
* whether the incoming value can be converted to a fluid value.
* Values that already have a "clamp()" function will not pass the test,
* and therefore the original $value will be returned.
*/
$value = gutenberg_get_typography_font_size_value( array( 'size' => $value ) );
}

$declarations[] = array(
'name' => $css_property,
'value' => $value,
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Bug Fix

- `FontSizePicker`: Ensure that fluid font size presets appear correctly in the UI controls ([#44791](https://github.com/WordPress/gutenberg/pull/44791))
- The `LinkedButton` to unlink sides in `BoxControl`, `BorderBoxControl` and `BorderRadiusControl` have changed from a rectangular primary button to an icon-only button, with a sentence case tooltip, and default-size icon for better legibility. The `Button` component has been fixed so when `isSmall` and `icon` props are set, and no text is present, the button shape is square rather than rectangular.
- `Popover`: fix limitShift logic by adding iframe offset correctly [#42950](https://github.com/WordPress/gutenberg/pull/42950)).

Expand Down
10 changes: 5 additions & 5 deletions packages/components/src/font-size-picker/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,33 +116,33 @@ describe( 'getToggleGroupOptions', () => {
).toEqual( [
{
key: '1',
value: '1',
label: 'S',
name: '1',
value: '1',
},
{
key: '2',
value: '2',
label: 'M',
name: '2',
value: '2',
},
{
key: '3',
value: '3',
label: 'L',
name: '3',
value: '3',
},
{
key: '4',
value: '4',
label: 'XL',
name: '4',
value: '4',
},
{
key: '5',
value: '5',
label: 'XXL',
name: 'XXL',
value: '5',
},
] );
} );
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/font-size-picker/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function getSelectOptions( optionsArray, disableCustomFontSizes ) {
];
return options.map( ( { slug, name, size } ) => ( {
key: slug,
name,
name: name || slug,
size,
__experimentalHint:
size && isSimpleCssValue( size ) && parseFloat( size ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ describe( 'typography utils', () => {
typographySettings: undefined,
expected: '28px',
},
// Default return non-fluid value where `size` is undefined.
{
preset: {
size: undefined,
},
typographySettings: undefined,
expected: undefined,
},
// Should return non-fluid value when fluid is `false`.
{
preset: {
Expand All @@ -26,6 +34,41 @@ describe( 'typography utils', () => {
},
expected: '28px',
},
// Should coerce number to `px` and return fluid value.
{
preset: {
size: 33,
fluid: true,
},
typographySettings: {
fluid: true,
},
expected:
'clamp(24.75px, 1.546875rem + ((1vw - 7.68px) * 2.975), 49.5px)',
},
// Should return incoming value when already clamped.
{
preset: {
size: 'clamp(21px, 1.3125rem + ((1vw - 7.68px) * 2.524), 42px)',
fluid: false,
},
typographySettings: {
fluid: true,
},
expected:
'clamp(21px, 1.3125rem + ((1vw - 7.68px) * 2.524), 42px)',
},
// Should return incoming value with unsupported unit.
{
preset: {
size: '1000%',
fluid: false,
},
typographySettings: {
fluid: true,
},
expected: '1000%',
},
// Should return fluid value.
{
preset: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,11 @@ describe( 'global styles renderer', () => {
},
typography: {
fontFamily: 'sans-serif',
fontSize: '14px',
},
};

it( 'Should output padding variables and other properties if useRootPaddingAwareAlignments is enabled', () => {
it( 'should output padding variables and other properties if useRootPaddingAwareAlignments is enabled', () => {
expect(
getStylesDeclarations( blockStyles, 'body', true )
).toEqual( [
Expand All @@ -724,10 +725,11 @@ describe( 'global styles renderer', () => {
'--wp--style--root--padding-left: 33px',
'background-color: var(--wp--preset--color--light-green-cyan)',
'font-family: sans-serif',
'font-size: 14px',
] );
} );

it( 'Should output padding and other properties if useRootPaddingAwareAlignments is disabled', () => {
it( 'should output padding and other properties if useRootPaddingAwareAlignments is disabled', () => {
expect(
getStylesDeclarations( blockStyles, 'body', false )
).toEqual( [
Expand All @@ -737,10 +739,11 @@ describe( 'global styles renderer', () => {
'padding-bottom: 33px',
'padding-left: 33px',
'font-family: sans-serif',
'font-size: 14px',
] );
} );

it( 'Should not output padding variables if selector is not root', () => {
it( 'should not output padding variables if selector is not root', () => {
expect(
getStylesDeclarations(
blockStyles,
Expand All @@ -754,6 +757,57 @@ describe( 'global styles renderer', () => {
'padding-bottom: 33px',
'padding-left: 33px',
'font-family: sans-serif',
'font-size: 14px',
] );
} );

it( 'should output clamp values for font-size when fluid typography is enabled', () => {
expect(
getStylesDeclarations(
blockStyles,
'.wp-block-site-title',
true,
{
settings: {
typography: {
fluid: true,
},
},
}
)
).toEqual( [
'background-color: var(--wp--preset--color--light-green-cyan)',
'padding-top: 33px',
'padding-right: 33px',
'padding-bottom: 33px',
'padding-left: 33px',
'font-family: sans-serif',
'font-size: clamp(10.5px, 0.65625rem + ((1vw - 7.68px) * 1.262), 21px)',
] );
} );

it( 'should output direct values for font-size when fluid typography is disabled', () => {
expect(
getStylesDeclarations(
blockStyles,
'.wp-block-site-title',
true,
{
settings: {
typography: {
fluid: false,
},
},
}
)
).toEqual( [
'background-color: var(--wp--preset--color--light-green-cyan)',
'padding-top: 33px',
'padding-right: 33px',
'padding-bottom: 33px',
'padding-left: 33px',
'font-family: sans-serif',
'font-size: 14px',
] );
} );
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { useState } from '@wordpress/element';
* Internal dependencies
*/
import { getSupportedGlobalStylesPanels, useSetting, useStyle } from './hooks';
import { getTypographyFontSizeValue } from './typography-utils';

export function useHasTypographyPanel( name ) {
const hasLineHeight = useHasLineHeightControl( name );
Expand Down Expand Up @@ -87,7 +88,19 @@ export default function TypographyPanel( { name, element } ) {
} else if ( element && element !== 'text' ) {
prefix = `elements.${ element }.`;
}
const [ fluidTypography ] = useSetting( 'typography.fluid', name );
const [ fontSizes ] = useSetting( 'typography.fontSizes', name );

// Convert static font size values to fluid font sizes if fluidTypography is activated.
const fontSizesWithFluidValues = fontSizes.map( ( font ) => {
if ( !! fluidTypography ) {
font.size = getTypographyFontSizeValue( font, {
fluid: fluidTypography,
} );
}
return font;
} );

const disableCustomFontSizes = ! useSetting(
'typography.customFontSize',
name
Expand Down Expand Up @@ -180,7 +193,7 @@ export default function TypographyPanel( { name, element } ) {
>
<ToggleGroupControlOption
value="heading"
/* translators: 'All' refers to selecting all heading levels
/* translators: 'All' refers to selecting all heading levels
and applying the same style to h1-h6. */
label={ __( 'All' ) }
/>
Expand Down Expand Up @@ -227,7 +240,7 @@ export default function TypographyPanel( { name, element } ) {
<FontSizePicker
value={ fontSize }
onChange={ setFontSize }
fontSizes={ fontSizes }
fontSizes={ fontSizesWithFluidValues }
disableCustomFontSizes={ disableCustomFontSizes }
size="__unstable-large"
__nextHasNoMarginBottom
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,27 @@
* ---------------------------------------------------------------
*/

/**
* @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} 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
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
* Internal dependencies
*/
import { PRESET_METADATA, ROOT_BLOCK_SELECTOR, scopeSelector } from './utils';
import { getTypographyFontSizeValue } from './typography-utils';
import { GlobalStylesContext } from './context';
import { useSetting } from './hooks';

Expand Down Expand Up @@ -276,6 +277,21 @@ export function getStylesDeclarations(
}
}

// Calculate fluid typography rules where available.
if ( cssProperty === 'font-size' ) {
/*
* getTypographyFontSizeValue() will check
* if fluid typography has been activated and also
* whether the incoming value can be converted to a fluid value.
* Values that already have a "clamp()" function will not pass the test,
* and therefore the original $value will be returned.
*/
ruleValue = getTypographyFontSizeValue(
{ size: ruleValue },
tree?.settings?.typography
);
}

output.push( `${ cssProperty }: ${ ruleValue }` );
} );

Expand Down

0 comments on commit 7f891cc

Please sign in to comment.