From d9f7d87c0e10ec967233a2db010630a24f187ee6 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Sun, 14 Jul 2024 17:27:33 +0200 Subject: [PATCH 1/9] Add unit tests --- .../src/font-size-picker/test/index.tsx | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/packages/components/src/font-size-picker/test/index.tsx b/packages/components/src/font-size-picker/test/index.tsx index 620d782e3abceb..eab98b93ec7e02 100644 --- a/packages/components/src/font-size-picker/test/index.tsx +++ b/packages/components/src/font-size-picker/test/index.tsx @@ -9,6 +9,29 @@ import userEvent from '@testing-library/user-event'; */ import FontSizePicker from '../'; import type { FontSize } from '../types'; +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; + +const ControlledFontSizePicker = ( { + onChange, + ...props +}: React.ComponentProps< typeof FontSizePicker > ) => { + const [ fontSize, setFontSize ] = + useState< typeof props.value >( undefined ); + + return ( + { + setFontSize( newFontSize ); + onChange?.( newFontSize ); + } } + /> + ); +}; describe( 'FontSizePicker', () => { test.each( [ @@ -525,6 +548,39 @@ describe( 'FontSizePicker', () => { expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); } ); + it( 'hides custom input when disableCustomFontSizes is set to `true` with a custom font size', () => { + const { rerender } = render( + + ); + expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + + rerender( + + ); + expect( + screen.queryByLabelText( 'Custom' ) + ).not.toBeInTheDocument(); + } ); + + it( "doesn't hide custom input when the selected font size is a predef", () => { + const { rerender } = render( + + ); + expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + + rerender( + + ); + expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + } ); + it( 'allows custom values by default', async () => { const user = userEvent.setup(); const onChange = jest.fn(); @@ -539,6 +595,26 @@ describe( 'FontSizePicker', () => { expect( onChange ).toHaveBeenCalledWith( '80px' ); } ); + it( 'allows switching between custom and predef inputs when pressing the dedicated toggle', async () => { + const user = userEvent.setup(); + + render( ); + + await user.click( + screen.getByRole( 'button', { name: 'Set custom size' } ) + ); + + await user.type( screen.getByLabelText( 'Custom' ), '80' ); + + await user.click( + screen.getByRole( 'button', { name: 'Use size preset' } ) + ); + + expect( + screen.queryByLabelText( 'Custom' ) + ).not.toBeInTheDocument(); + } ); + it( 'does not allow custom values when disableCustomFontSizes is set', () => { render( Date: Sun, 14 Jul 2024 17:28:03 +0200 Subject: [PATCH 2/9] Refactor code and extract logic --- .../components/src/font-size-picker/index.tsx | 175 +++++++++++------- .../components/src/font-size-picker/types.ts | 2 + 2 files changed, 110 insertions(+), 67 deletions(-) diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx index e4de2f6f6f5cf9..a547a26de59415 100644 --- a/packages/components/src/font-size-picker/index.tsx +++ b/packages/components/src/font-size-picker/index.tsx @@ -8,7 +8,7 @@ import type { ForwardedRef } from 'react'; */ import { __ } from '@wordpress/i18n'; import { settings } from '@wordpress/icons'; -import { useState, useMemo, forwardRef } from '@wordpress/element'; +import { useState, useEffect, forwardRef } from '@wordpress/element'; /** * Internal dependencies @@ -23,7 +23,11 @@ import { } from '../unit-control'; import { VisuallyHidden } from '../visually-hidden'; import { getCommonSizeUnit } from './utils'; -import type { FontSizePickerProps } from './types'; +import type { + FontSize, + FontSizePickerProps, + FontSizePickerType, +} from './types'; import { Container, Header, @@ -38,6 +42,49 @@ import { T_SHIRT_NAMES } from './constants'; const DEFAULT_UNITS = [ 'px', 'em', 'rem', 'vw', 'vh' ]; +const shouldUseSelectOverToggle = ( howManyfontSizes: number ) => + howManyfontSizes > 5; + +const getPickerType = ( + showCustomValuePicker: boolean, + fontSizes: FontSize[] +): FontSizePickerType => { + if ( showCustomValuePicker ) { + return 'custom'; + } + + return shouldUseSelectOverToggle( fontSizes.length ) + ? 'select' + : 'togglegroup'; +}; + +const getHeaderHint = ( + currentPickerType: FontSizePickerType, + selectedFontSize: FontSize | undefined, + fontSizes: FontSize[] +) => { + if ( currentPickerType === 'custom' ) { + return __( 'Custom' ); + } + + if ( ! shouldUseSelectOverToggle( fontSizes.length ) ) { + if ( selectedFontSize ) { + return ( + selectedFontSize.name || + T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ] + ); + } + return ''; + } + + const commonUnit = getCommonSizeUnit( fontSizes ); + if ( commonUnit ) { + return `(${ commonUnit })`; + } + + return ''; +}; + const UnforwardedFontSizePicker = ( props: FontSizePickerProps, ref: ForwardedRef< any > @@ -55,52 +102,47 @@ const UnforwardedFontSizePicker = ( withReset = true, } = props; - const units = useCustomUnits( { - availableUnits: unitsProp, - } ); - - const shouldUseSelectControl = fontSizes.length > 5; const selectedFontSize = fontSizes.find( ( fontSize ) => fontSize.size === value ); const isCustomValue = !! value && ! selectedFontSize; - const [ showCustomValueControl, setShowCustomValueControl ] = useState( - ! disableCustomFontSizes && isCustomValue + const [ currentPickerType, setCurrentPickerType ] = useState( + getPickerType( ! disableCustomFontSizes && isCustomValue, fontSizes ) ); - const headerHint = useMemo( () => { - if ( showCustomValueControl ) { - return __( 'Custom' ); - } - - if ( ! shouldUseSelectControl ) { - if ( selectedFontSize ) { - return ( - selectedFontSize.name || - T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ] - ); - } - return ''; - } - - const commonUnit = getCommonSizeUnit( fontSizes ); - if ( commonUnit ) { - return `(${ commonUnit })`; - } - - return ''; + useEffect( () => { + setCurrentPickerType( + getPickerType( + // If showing the custom value picker, switch back to predef only + // if `disableCustomFontSizes` is set to `true`. + currentPickerType === 'custom' + ? ! disableCustomFontSizes + : ! disableCustomFontSizes && isCustomValue, + fontSizes + ) + ); }, [ - showCustomValueControl, - shouldUseSelectControl, - selectedFontSize, + currentPickerType, + disableCustomFontSizes, + isCustomValue, fontSizes, ] ); + const units = useCustomUnits( { + availableUnits: unitsProp, + } ); + if ( fontSizes.length === 0 && disableCustomFontSizes ) { return null; } + const headerHint = getHeaderHint( + currentPickerType, + selectedFontSize, + fontSizes + ); + // If neither the value or first font size is a string, then FontSizePicker // operates in a legacy "unitless" mode where UnitControl can only be used // to select px values and onChange() is always called with number values. @@ -133,53 +175,52 @@ const UnforwardedFontSizePicker = ( { ! disableCustomFontSizes && ( { - setShowCustomValueControl( - ! showCustomValueControl + setCurrentPickerType( + getPickerType( + currentPickerType !== 'custom', + fontSizes + ) ); } } - isPressed={ showCustomValueControl } + isPressed={ currentPickerType === 'custom' } size="small" /> ) }
- { !! fontSizes.length && - shouldUseSelectControl && - ! showCustomValueControl && ( - { - if ( newValue === undefined ) { - onChange?.( undefined ); - } else { - onChange?.( - hasUnits - ? newValue - : Number( newValue ), - fontSizes.find( - ( fontSize ) => - fontSize.size === newValue - ) - ); - } - } } - onSelectCustom={ () => - setShowCustomValueControl( true ) + { currentPickerType === 'select' && ( + { + if ( newValue === undefined ) { + onChange?.( undefined ); + } else { + onChange?.( + hasUnits ? newValue : Number( newValue ), + fontSizes.find( + ( fontSize ) => + fontSize.size === newValue + ) + ); } - /> - ) } - { ! shouldUseSelectControl && ! showCustomValueControl && ( + } } + onSelectCustom={ () => + setCurrentPickerType( 'custom' ) + } + /> + ) } + { currentPickerType === 'togglegroup' && ( ) } - { ! disableCustomFontSizes && showCustomValueControl && ( + { currentPickerType === 'custom' && ( Date: Sun, 14 Jul 2024 17:30:49 +0200 Subject: [PATCH 3/9] Sync JS docs --- packages/components/src/font-size-picker/types.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/components/src/font-size-picker/types.ts b/packages/components/src/font-size-picker/types.ts index 1de39c5878df70..193319d20a971c 100644 --- a/packages/components/src/font-size-picker/types.ts +++ b/packages/components/src/font-size-picker/types.ts @@ -39,8 +39,9 @@ export type FontSizePickerProps = { */ value?: number | string; /** - * If `true`, the UI will contain a slider, instead of a numeric text input - * field. If `false`, no slider will be present. + * If `true`, a slider will be displayed alongside the input field when a + * custom font size is active. Has no effect when `disableCustomFontSizes` + * is `true`. * * @default false */ @@ -48,7 +49,7 @@ export type FontSizePickerProps = { /** * If `true`, a reset button will be displayed alongside the input field * when a custom font size is active. Has no effect when - * `disableCustomFontSizes` or `withSlider` is `true`. + * `disableCustomFontSizes` is `true`. * * @default true */ From 0e8f1c49c4c748e65d95e871692db8897fd1e0ff Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Mon, 15 Jul 2024 10:29:45 +0200 Subject: [PATCH 4/9] CHANGELOG --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 392d11fdd6673e..326d2876e65c6f 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -17,6 +17,7 @@ - `ToggleGroupControl`: support disabled options ([#63450](https://github.com/WordPress/gutenberg/pull/63450)). - `CustomSelectControl`: Stabilize `__experimentalShowSelectedHint` and `options[]. __experimentalHint` props ([#63248](https://github.com/WordPress/gutenberg/pull/63248)). - `SelectControl`: Add `"minimal"` variant ([#63265](https://github.com/WordPress/gutenberg/pull/63265)). +- `FontSizePicker`: tidy up internal logic ([#63553](https://github.com/WordPress/gutenberg/pull/63553)). ### Internal From b0e91a00ab2a2432efc2af6570482ad9e26e974e Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 16 Jul 2024 12:27:00 +0200 Subject: [PATCH 5/9] Further simplify logic --- .../components/src/font-size-picker/index.tsx | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx index a547a26de59415..7085487adbaf46 100644 --- a/packages/components/src/font-size-picker/index.tsx +++ b/packages/components/src/font-size-picker/index.tsx @@ -8,7 +8,7 @@ import type { ForwardedRef } from 'react'; */ import { __ } from '@wordpress/i18n'; import { settings } from '@wordpress/icons'; -import { useState, useEffect, forwardRef } from '@wordpress/element'; +import { useState, forwardRef } from '@wordpress/element'; /** * Internal dependencies @@ -107,27 +107,9 @@ const UnforwardedFontSizePicker = ( ); const isCustomValue = !! value && ! selectedFontSize; - const [ currentPickerType, setCurrentPickerType ] = useState( - getPickerType( ! disableCustomFontSizes && isCustomValue, fontSizes ) - ); - - useEffect( () => { - setCurrentPickerType( - getPickerType( - // If showing the custom value picker, switch back to predef only - // if `disableCustomFontSizes` is set to `true`. - currentPickerType === 'custom' - ? ! disableCustomFontSizes - : ! disableCustomFontSizes && isCustomValue, - fontSizes - ) - ); - }, [ - currentPickerType, - disableCustomFontSizes, - isCustomValue, - fontSizes, - ] ); + // Initially request a custom picker if the value is not from the predef list. + const [ userRequestedCustom, setUserRequestedCustom ] = + useState( isCustomValue ); const units = useCustomUnits( { availableUnits: unitsProp, @@ -137,6 +119,13 @@ const UnforwardedFontSizePicker = ( return null; } + const currentPickerType = getPickerType( + // If showing the custom value picker, switch back to predef only + // if `disableCustomFontSizes` is set to `true`. + ! disableCustomFontSizes && userRequestedCustom, + fontSizes + ); + const headerHint = getHeaderHint( currentPickerType, selectedFontSize, @@ -180,14 +169,9 @@ const UnforwardedFontSizePicker = ( : __( 'Set custom size' ) } icon={ settings } - onClick={ () => { - setCurrentPickerType( - getPickerType( - currentPickerType !== 'custom', - fontSizes - ) - ); - } } + onClick={ () => + setUserRequestedCustom( ! userRequestedCustom ) + } isPressed={ currentPickerType === 'custom' } size="small" /> @@ -215,9 +199,7 @@ const UnforwardedFontSizePicker = ( ); } } } - onSelectCustom={ () => - setCurrentPickerType( 'custom' ) - } + onSelectCustom={ () => setUserRequestedCustom( true ) } /> ) } { currentPickerType === 'togglegroup' && ( @@ -251,6 +233,8 @@ const UnforwardedFontSizePicker = ( hideLabelFromVision value={ value } onChange={ ( newValue ) => { + setUserRequestedCustom( true ); + if ( newValue === undefined ) { onChange?.( undefined ); } else { @@ -281,6 +265,8 @@ const UnforwardedFontSizePicker = ( initialPosition={ fallbackFontSize } withInputField={ false } onChange={ ( newValue ) => { + setUserRequestedCustom( true ); + if ( newValue === undefined ) { onChange?.( undefined ); } else if ( hasUnits ) { From dd5f9c3f9c1e12b470318a0642f642f109c19168 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 16 Jul 2024 13:21:52 +0200 Subject: [PATCH 6/9] Inline `getPickerType` utility --- .../components/src/font-size-picker/index.tsx | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx index 7085487adbaf46..82445ce353aae0 100644 --- a/packages/components/src/font-size-picker/index.tsx +++ b/packages/components/src/font-size-picker/index.tsx @@ -45,19 +45,6 @@ const DEFAULT_UNITS = [ 'px', 'em', 'rem', 'vw', 'vh' ]; const shouldUseSelectOverToggle = ( howManyfontSizes: number ) => howManyfontSizes > 5; -const getPickerType = ( - showCustomValuePicker: boolean, - fontSizes: FontSize[] -): FontSizePickerType => { - if ( showCustomValuePicker ) { - return 'custom'; - } - - return shouldUseSelectOverToggle( fontSizes.length ) - ? 'select' - : 'togglegroup'; -}; - const getHeaderHint = ( currentPickerType: FontSizePickerType, selectedFontSize: FontSize | undefined, @@ -117,14 +104,17 @@ const UnforwardedFontSizePicker = ( if ( fontSizes.length === 0 && disableCustomFontSizes ) { return null; + let currentPickerType; + if ( ! disableCustomFontSizes && userRequestedCustom ) { + // While showing the custom value picker, switch back to predef only if + // `disableCustomFontSizes` is set to `true`. + currentPickerType = 'custom' as const; + } else { + currentPickerType = shouldUseSelectOverToggle( fontSizes.length ) + ? ( 'select' as const ) + : ( 'togglegroup' as const ); } - const currentPickerType = getPickerType( - // If showing the custom value picker, switch back to predef only - // if `disableCustomFontSizes` is set to `true`. - ! disableCustomFontSizes && userRequestedCustom, - fontSizes - ); const headerHint = getHeaderHint( currentPickerType, From c707d1d8bbef7f651dbef293a4d0b5b1157fd849 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 16 Jul 2024 13:24:49 +0200 Subject: [PATCH 7/9] Inline `getHeaderHint` and `shouldUseSelectOverToggle` utilities, refactor to switch statement. --- .../components/src/font-size-picker/index.tsx | 77 ++++++++----------- .../components/src/font-size-picker/types.ts | 2 - 2 files changed, 32 insertions(+), 47 deletions(-) diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx index 82445ce353aae0..346c2d07e9784d 100644 --- a/packages/components/src/font-size-picker/index.tsx +++ b/packages/components/src/font-size-picker/index.tsx @@ -8,7 +8,7 @@ import type { ForwardedRef } from 'react'; */ import { __ } from '@wordpress/i18n'; import { settings } from '@wordpress/icons'; -import { useState, forwardRef } from '@wordpress/element'; +import { useState, useMemo, forwardRef } from '@wordpress/element'; /** * Internal dependencies @@ -23,11 +23,7 @@ import { } from '../unit-control'; import { VisuallyHidden } from '../visually-hidden'; import { getCommonSizeUnit } from './utils'; -import type { - FontSize, - FontSizePickerProps, - FontSizePickerType, -} from './types'; +import type { FontSizePickerProps } from './types'; import { Container, Header, @@ -42,35 +38,7 @@ import { T_SHIRT_NAMES } from './constants'; const DEFAULT_UNITS = [ 'px', 'em', 'rem', 'vw', 'vh' ]; -const shouldUseSelectOverToggle = ( howManyfontSizes: number ) => - howManyfontSizes > 5; - -const getHeaderHint = ( - currentPickerType: FontSizePickerType, - selectedFontSize: FontSize | undefined, - fontSizes: FontSize[] -) => { - if ( currentPickerType === 'custom' ) { - return __( 'Custom' ); - } - - if ( ! shouldUseSelectOverToggle( fontSizes.length ) ) { - if ( selectedFontSize ) { - return ( - selectedFontSize.name || - T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ] - ); - } - return ''; - } - - const commonUnit = getCommonSizeUnit( fontSizes ); - if ( commonUnit ) { - return `(${ commonUnit })`; - } - - return ''; -}; +const MAX_TOGGLE_GROUP_SIZES = 5; const UnforwardedFontSizePicker = ( props: FontSizePickerProps, @@ -102,25 +70,44 @@ const UnforwardedFontSizePicker = ( availableUnits: unitsProp, } ); - if ( fontSizes.length === 0 && disableCustomFontSizes ) { - return null; let currentPickerType; if ( ! disableCustomFontSizes && userRequestedCustom ) { // While showing the custom value picker, switch back to predef only if // `disableCustomFontSizes` is set to `true`. currentPickerType = 'custom' as const; } else { - currentPickerType = shouldUseSelectOverToggle( fontSizes.length ) - ? ( 'select' as const ) - : ( 'togglegroup' as const ); + currentPickerType = + fontSizes.length > MAX_TOGGLE_GROUP_SIZES + ? ( 'select' as const ) + : ( 'togglegroup' as const ); } + const headerHint = useMemo( () => { + switch ( currentPickerType ) { + case 'custom': + return __( 'Custom' ); + case 'togglegroup': + if ( selectedFontSize ) { + return ( + selectedFontSize.name || + T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ] + ); + } + break; + case 'select': + const commonUnit = getCommonSizeUnit( fontSizes ); + if ( commonUnit ) { + return `(${ commonUnit })`; + } + break; + } - const headerHint = getHeaderHint( - currentPickerType, - selectedFontSize, - fontSizes - ); + return ''; + }, [ currentPickerType, selectedFontSize, fontSizes ] ); + + if ( fontSizes.length === 0 && disableCustomFontSizes ) { + return null; + } // If neither the value or first font size is a string, then FontSizePicker // operates in a legacy "unitless" mode where UnitControl can only be used diff --git a/packages/components/src/font-size-picker/types.ts b/packages/components/src/font-size-picker/types.ts index 193319d20a971c..2296e968567882 100644 --- a/packages/components/src/font-size-picker/types.ts +++ b/packages/components/src/font-size-picker/types.ts @@ -1,5 +1,3 @@ -export type FontSizePickerType = 'select' | 'togglegroup' | 'custom'; - export type FontSizePickerProps = { /** * If `true`, it will not be possible to choose a custom fontSize. The user From a9b19539235bbb0927af925a7dc1228abfd35054 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 16 Jul 2024 13:26:11 +0200 Subject: [PATCH 8/9] toBeInTheDocument() => toBeVisible() --- .../src/font-size-picker/test/index.tsx | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/packages/components/src/font-size-picker/test/index.tsx b/packages/components/src/font-size-picker/test/index.tsx index eab98b93ec7e02..f91c12e352bf80 100644 --- a/packages/components/src/font-size-picker/test/index.tsx +++ b/packages/components/src/font-size-picker/test/index.tsx @@ -141,9 +141,7 @@ describe( 'FontSizePicker', () => { render( ); - expect( - screen.getByLabelText( expectedLabel ) - ).toBeInTheDocument(); + expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); } ); @@ -266,9 +264,7 @@ describe( 'FontSizePicker', () => { render( ); - expect( - screen.getByLabelText( expectedLabel ) - ).toBeInTheDocument(); + expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); } ); @@ -383,9 +379,7 @@ describe( 'FontSizePicker', () => { render( ); - expect( - screen.getByLabelText( expectedLabel ) - ).toBeInTheDocument(); + expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); } ); @@ -457,9 +451,7 @@ describe( 'FontSizePicker', () => { render( ); - expect( - screen.getByLabelText( expectedLabel ) - ).toBeInTheDocument(); + expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); } ); @@ -516,7 +508,7 @@ describe( 'FontSizePicker', () => { render( ); - expect( screen.getByRole( 'radiogroup' ) ).toBeInTheDocument(); + expect( screen.getByRole( 'radiogroup' ) ).toBeVisible(); expect( screen.queryByRole( 'radio', { checked: true } ) ).not.toBeInTheDocument(); @@ -537,7 +529,7 @@ describe( 'FontSizePicker', () => { await user.click( screen.getByRole( 'option', { name: 'Custom' } ) ); - expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); expect( onChange ).not.toHaveBeenCalled(); } ); } @@ -545,14 +537,14 @@ describe( 'FontSizePicker', () => { function commonTests( fontSizes: FontSize[] ) { it( 'shows custom input when value is unknown', () => { render( ); - expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); } ); it( 'hides custom input when disableCustomFontSizes is set to `true` with a custom font size', () => { const { rerender } = render( ); - expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); rerender( { const { rerender } = render( ); - expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); rerender( { value={ fontSizes[ 0 ].size } /> ); - expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument(); + expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); } ); it( 'allows custom values by default', async () => { From 30e5c64639ef9c7bad68b4dcc7de29932002faf4 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 16 Jul 2024 13:35:44 +0200 Subject: [PATCH 9/9] Move units back at the start of the render function for a smaller diff --- packages/components/src/font-size-picker/index.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx index 346c2d07e9784d..f93a09440a0688 100644 --- a/packages/components/src/font-size-picker/index.tsx +++ b/packages/components/src/font-size-picker/index.tsx @@ -57,6 +57,10 @@ const UnforwardedFontSizePicker = ( withReset = true, } = props; + const units = useCustomUnits( { + availableUnits: unitsProp, + } ); + const selectedFontSize = fontSizes.find( ( fontSize ) => fontSize.size === value ); @@ -66,10 +70,6 @@ const UnforwardedFontSizePicker = ( const [ userRequestedCustom, setUserRequestedCustom ] = useState( isCustomValue ); - const units = useCustomUnits( { - availableUnits: unitsProp, - } ); - let currentPickerType; if ( ! disableCustomFontSizes && userRequestedCustom ) { // While showing the custom value picker, switch back to predef only if