Skip to content

Commit 777ebc1

Browse files
ciampomirkatyxla
authored
FontSizePicker: tidy up internal logic (#63553)
* Add unit tests * Refactor code and extract logic * Sync JS docs * CHANGELOG * Further simplify logic * Inline `getPickerType` utility * Inline `getHeaderHint` and `shouldUseSelectOverToggle` utilities, refactor to switch statement. * toBeInTheDocument() => toBeVisible() * Move units back at the start of the render function for a smaller diff --- Co-authored-by: ciampo <[email protected]> Co-authored-by: mirka <[email protected]> Co-authored-by: tyxla <[email protected]>
1 parent 6b3cb26 commit 777ebc1

File tree

4 files changed

+156
-82
lines changed

4 files changed

+156
-82
lines changed

‎packages/components/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
- `ToggleGroupControl`: support disabled options ([#63450](https://github.com/WordPress/gutenberg/pull/63450)).
1818
- `CustomSelectControl`: Stabilize `__experimentalShowSelectedHint` and `options[]. __experimentalHint` props ([#63248](https://github.com/WordPress/gutenberg/pull/63248)).
1919
- `SelectControl`: Add `"minimal"` variant ([#63265](https://github.com/WordPress/gutenberg/pull/63265)).
20+
- `FontSizePicker`: tidy up internal logic ([#63553](https://github.com/WordPress/gutenberg/pull/63553)).
2021

2122
### Internal
2223

‎packages/components/src/font-size-picker/index.tsx

Lines changed: 68 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ import { T_SHIRT_NAMES } from './constants';
3838

3939
const DEFAULT_UNITS = [ 'px', 'em', 'rem', 'vw', 'vh' ];
4040

41+
const MAX_TOGGLE_GROUP_SIZES = 5;
42+
4143
const UnforwardedFontSizePicker = (
4244
props: FontSizePickerProps,
4345
ref: ForwardedRef< any >
@@ -59,43 +61,49 @@ const UnforwardedFontSizePicker = (
5961
availableUnits: unitsProp,
6062
} );
6163

62-
const shouldUseSelectControl = fontSizes.length > 5;
6364
const selectedFontSize = fontSizes.find(
6465
( fontSize ) => fontSize.size === value
6566
);
6667
const isCustomValue = !! value && ! selectedFontSize;
6768

68-
const [ showCustomValueControl, setShowCustomValueControl ] = useState(
69-
! disableCustomFontSizes && isCustomValue
70-
);
71-
72-
const headerHint = useMemo( () => {
73-
if ( showCustomValueControl ) {
74-
return __( 'Custom' );
75-
}
69+
// Initially request a custom picker if the value is not from the predef list.
70+
const [ userRequestedCustom, setUserRequestedCustom ] =
71+
useState( isCustomValue );
7672

77-
if ( ! shouldUseSelectControl ) {
78-
if ( selectedFontSize ) {
79-
return (
80-
selectedFontSize.name ||
81-
T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ]
82-
);
83-
}
84-
return '';
85-
}
73+
let currentPickerType;
74+
if ( ! disableCustomFontSizes && userRequestedCustom ) {
75+
// While showing the custom value picker, switch back to predef only if
76+
// `disableCustomFontSizes` is set to `true`.
77+
currentPickerType = 'custom' as const;
78+
} else {
79+
currentPickerType =
80+
fontSizes.length > MAX_TOGGLE_GROUP_SIZES
81+
? ( 'select' as const )
82+
: ( 'togglegroup' as const );
83+
}
8684

87-
const commonUnit = getCommonSizeUnit( fontSizes );
88-
if ( commonUnit ) {
89-
return `(${ commonUnit })`;
85+
const headerHint = useMemo( () => {
86+
switch ( currentPickerType ) {
87+
case 'custom':
88+
return __( 'Custom' );
89+
case 'togglegroup':
90+
if ( selectedFontSize ) {
91+
return (
92+
selectedFontSize.name ||
93+
T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ]
94+
);
95+
}
96+
break;
97+
case 'select':
98+
const commonUnit = getCommonSizeUnit( fontSizes );
99+
if ( commonUnit ) {
100+
return `(${ commonUnit })`;
101+
}
102+
break;
90103
}
91104

92105
return '';
93-
}, [
94-
showCustomValueControl,
95-
shouldUseSelectControl,
96-
selectedFontSize,
97-
fontSizes,
98-
] );
106+
}, [ currentPickerType, selectedFontSize, fontSizes ] );
99107

100108
if ( fontSizes.length === 0 && disableCustomFontSizes ) {
101109
return null;
@@ -133,53 +141,45 @@ const UnforwardedFontSizePicker = (
133141
{ ! disableCustomFontSizes && (
134142
<HeaderToggle
135143
label={
136-
showCustomValueControl
144+
currentPickerType === 'custom'
137145
? __( 'Use size preset' )
138146
: __( 'Set custom size' )
139147
}
140148
icon={ settings }
141-
onClick={ () => {
142-
setShowCustomValueControl(
143-
! showCustomValueControl
144-
);
145-
} }
146-
isPressed={ showCustomValueControl }
149+
onClick={ () =>
150+
setUserRequestedCustom( ! userRequestedCustom )
151+
}
152+
isPressed={ currentPickerType === 'custom' }
147153
size="small"
148154
/>
149155
) }
150156
</Header>
151157
</Spacer>
152158
<div>
153-
{ !! fontSizes.length &&
154-
shouldUseSelectControl &&
155-
! showCustomValueControl && (
156-
<FontSizePickerSelect
157-
__next40pxDefaultSize={ __next40pxDefaultSize }
158-
fontSizes={ fontSizes }
159-
value={ value }
160-
disableCustomFontSizes={ disableCustomFontSizes }
161-
size={ size }
162-
onChange={ ( newValue ) => {
163-
if ( newValue === undefined ) {
164-
onChange?.( undefined );
165-
} else {
166-
onChange?.(
167-
hasUnits
168-
? newValue
169-
: Number( newValue ),
170-
fontSizes.find(
171-
( fontSize ) =>
172-
fontSize.size === newValue
173-
)
174-
);
175-
}
176-
} }
177-
onSelectCustom={ () =>
178-
setShowCustomValueControl( true )
159+
{ currentPickerType === 'select' && (
160+
<FontSizePickerSelect
161+
__next40pxDefaultSize={ __next40pxDefaultSize }
162+
fontSizes={ fontSizes }
163+
value={ value }
164+
disableCustomFontSizes={ disableCustomFontSizes }
165+
size={ size }
166+
onChange={ ( newValue ) => {
167+
if ( newValue === undefined ) {
168+
onChange?.( undefined );
169+
} else {
170+
onChange?.(
171+
hasUnits ? newValue : Number( newValue ),
172+
fontSizes.find(
173+
( fontSize ) =>
174+
fontSize.size === newValue
175+
)
176+
);
179177
}
180-
/>
181-
) }
182-
{ ! shouldUseSelectControl && ! showCustomValueControl && (
178+
} }
179+
onSelectCustom={ () => setUserRequestedCustom( true ) }
180+
/>
181+
) }
182+
{ currentPickerType === 'togglegroup' && (
183183
<FontSizePickerToggleGroup
184184
fontSizes={ fontSizes }
185185
value={ value }
@@ -200,7 +200,7 @@ const UnforwardedFontSizePicker = (
200200
} }
201201
/>
202202
) }
203-
{ ! disableCustomFontSizes && showCustomValueControl && (
203+
{ currentPickerType === 'custom' && (
204204
<Flex className="components-font-size-picker__custom-size-control">
205205
<FlexItem isBlock>
206206
<UnitControl
@@ -210,6 +210,8 @@ const UnforwardedFontSizePicker = (
210210
hideLabelFromVision
211211
value={ value }
212212
onChange={ ( newValue ) => {
213+
setUserRequestedCustom( true );
214+
213215
if ( newValue === undefined ) {
214216
onChange?.( undefined );
215217
} else {
@@ -240,6 +242,8 @@ const UnforwardedFontSizePicker = (
240242
initialPosition={ fallbackFontSize }
241243
withInputField={ false }
242244
onChange={ ( newValue ) => {
245+
setUserRequestedCustom( true );
246+
243247
if ( newValue === undefined ) {
244248
onChange?.( undefined );
245249
} else if ( hasUnits ) {

‎packages/components/src/font-size-picker/test/index.tsx

Lines changed: 83 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,29 @@ import userEvent from '@testing-library/user-event';
99
*/
1010
import FontSizePicker from '../';
1111
import type { FontSize } from '../types';
12+
/**
13+
* WordPress dependencies
14+
*/
15+
import { useState } from '@wordpress/element';
16+
17+
const ControlledFontSizePicker = ( {
18+
onChange,
19+
...props
20+
}: React.ComponentProps< typeof FontSizePicker > ) => {
21+
const [ fontSize, setFontSize ] =
22+
useState< typeof props.value >( undefined );
23+
24+
return (
25+
<FontSizePicker
26+
{ ...props }
27+
value={ fontSize }
28+
onChange={ ( newFontSize ) => {
29+
setFontSize( newFontSize );
30+
onChange?.( newFontSize );
31+
} }
32+
/>
33+
);
34+
};
1235

1336
describe( 'FontSizePicker', () => {
1437
test.each( [
@@ -118,9 +141,7 @@ describe( 'FontSizePicker', () => {
118141
render(
119142
<FontSizePicker fontSizes={ fontSizes } value={ value } />
120143
);
121-
expect(
122-
screen.getByLabelText( expectedLabel )
123-
).toBeInTheDocument();
144+
expect( screen.getByLabelText( expectedLabel ) ).toBeVisible();
124145
}
125146
);
126147

@@ -243,9 +264,7 @@ describe( 'FontSizePicker', () => {
243264
render(
244265
<FontSizePicker fontSizes={ fontSizes } value={ value } />
245266
);
246-
expect(
247-
screen.getByLabelText( expectedLabel )
248-
).toBeInTheDocument();
267+
expect( screen.getByLabelText( expectedLabel ) ).toBeVisible();
249268
}
250269
);
251270

@@ -360,9 +379,7 @@ describe( 'FontSizePicker', () => {
360379
render(
361380
<FontSizePicker fontSizes={ fontSizes } value={ value } />
362381
);
363-
expect(
364-
screen.getByLabelText( expectedLabel )
365-
).toBeInTheDocument();
382+
expect( screen.getByLabelText( expectedLabel ) ).toBeVisible();
366383
}
367384
);
368385

@@ -434,9 +451,7 @@ describe( 'FontSizePicker', () => {
434451
render(
435452
<FontSizePicker fontSizes={ fontSizes } value={ value } />
436453
);
437-
expect(
438-
screen.getByLabelText( expectedLabel )
439-
).toBeInTheDocument();
454+
expect( screen.getByLabelText( expectedLabel ) ).toBeVisible();
440455
}
441456
);
442457

@@ -493,7 +508,7 @@ describe( 'FontSizePicker', () => {
493508
render(
494509
<FontSizePicker fontSizes={ fontSizes } value={ value } />
495510
);
496-
expect( screen.getByRole( 'radiogroup' ) ).toBeInTheDocument();
511+
expect( screen.getByRole( 'radiogroup' ) ).toBeVisible();
497512
expect(
498513
screen.queryByRole( 'radio', { checked: true } )
499514
).not.toBeInTheDocument();
@@ -514,15 +529,48 @@ describe( 'FontSizePicker', () => {
514529
await user.click(
515530
screen.getByRole( 'option', { name: 'Custom' } )
516531
);
517-
expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument();
532+
expect( screen.getByLabelText( 'Custom' ) ).toBeVisible();
518533
expect( onChange ).not.toHaveBeenCalled();
519534
} );
520535
}
521536

522537
function commonTests( fontSizes: FontSize[] ) {
523538
it( 'shows custom input when value is unknown', () => {
524539
render( <FontSizePicker fontSizes={ fontSizes } value="3px" /> );
525-
expect( screen.getByLabelText( 'Custom' ) ).toBeInTheDocument();
540+
expect( screen.getByLabelText( 'Custom' ) ).toBeVisible();
541+
} );
542+
543+
it( 'hides custom input when disableCustomFontSizes is set to `true` with a custom font size', () => {
544+
const { rerender } = render(
545+
<FontSizePicker fontSizes={ fontSizes } value="3px" />
546+
);
547+
expect( screen.getByLabelText( 'Custom' ) ).toBeVisible();
548+
549+
rerender(
550+
<FontSizePicker
551+
disableCustomFontSizes
552+
fontSizes={ fontSizes }
553+
value="3px"
554+
/>
555+
);
556+
expect(
557+
screen.queryByLabelText( 'Custom' )
558+
).not.toBeInTheDocument();
559+
} );
560+
561+
it( "doesn't hide custom input when the selected font size is a predef", () => {
562+
const { rerender } = render(
563+
<FontSizePicker fontSizes={ fontSizes } value="3px" />
564+
);
565+
expect( screen.getByLabelText( 'Custom' ) ).toBeVisible();
566+
567+
rerender(
568+
<FontSizePicker
569+
fontSizes={ fontSizes }
570+
value={ fontSizes[ 0 ].size }
571+
/>
572+
);
573+
expect( screen.getByLabelText( 'Custom' ) ).toBeVisible();
526574
} );
527575

528576
it( 'allows custom values by default', async () => {
@@ -539,6 +587,26 @@ describe( 'FontSizePicker', () => {
539587
expect( onChange ).toHaveBeenCalledWith( '80px' );
540588
} );
541589

590+
it( 'allows switching between custom and predef inputs when pressing the dedicated toggle', async () => {
591+
const user = userEvent.setup();
592+
593+
render( <ControlledFontSizePicker fontSizes={ fontSizes } /> );
594+
595+
await user.click(
596+
screen.getByRole( 'button', { name: 'Set custom size' } )
597+
);
598+
599+
await user.type( screen.getByLabelText( 'Custom' ), '80' );
600+
601+
await user.click(
602+
screen.getByRole( 'button', { name: 'Use size preset' } )
603+
);
604+
605+
expect(
606+
screen.queryByLabelText( 'Custom' )
607+
).not.toBeInTheDocument();
608+
} );
609+
542610
it( 'does not allow custom values when disableCustomFontSizes is set', () => {
543611
render(
544612
<FontSizePicker

‎packages/components/src/font-size-picker/types.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,17 @@ export type FontSizePickerProps = {
3737
*/
3838
value?: number | string;
3939
/**
40-
* If `true`, the UI will contain a slider, instead of a numeric text input
41-
* field. If `false`, no slider will be present.
40+
* If `true`, a slider will be displayed alongside the input field when a
41+
* custom font size is active. Has no effect when `disableCustomFontSizes`
42+
* is `true`.
4243
*
4344
* @default false
4445
*/
4546
withSlider?: boolean;
4647
/**
4748
* If `true`, a reset button will be displayed alongside the input field
4849
* when a custom font size is active. Has no effect when
49-
* `disableCustomFontSizes` or `withSlider` is `true`.
50+
* `disableCustomFontSizes` is `true`.
5051
*
5152
* @default true
5253
*/

0 commit comments

Comments
 (0)