From 8075dadc70f5e75ff1d3d1c4f2bb16303d601c54 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 16:02:14 +0200 Subject: [PATCH 01/15] Rename file extensions --- .../components/src/unit-control/test/{index.js => index.tsx} | 2 +- .../components/src/unit-control/test/{utils.js => utils.ts} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename packages/components/src/unit-control/test/{index.js => index.tsx} (99%) rename packages/components/src/unit-control/test/{utils.js => utils.ts} (100%) diff --git a/packages/components/src/unit-control/test/index.js b/packages/components/src/unit-control/test/index.tsx similarity index 99% rename from packages/components/src/unit-control/test/index.js rename to packages/components/src/unit-control/test/index.tsx index d595f6171076bd..97a224a69d3b18 100644 --- a/packages/components/src/unit-control/test/index.js +++ b/packages/components/src/unit-control/test/index.tsx @@ -12,7 +12,7 @@ import { useState } from '@wordpress/element'; /** * Internal dependencies */ -import UnitControl from '../'; +import UnitControl from '..'; import { parseQuantityAndUnitFromRawValue } from '../utils'; function render( jsx ) { diff --git a/packages/components/src/unit-control/test/utils.js b/packages/components/src/unit-control/test/utils.ts similarity index 100% rename from packages/components/src/unit-control/test/utils.js rename to packages/components/src/unit-control/test/utils.ts From 67fae8bbcbff30189ab176737120954517075b3a Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 18:56:50 +0200 Subject: [PATCH 02/15] Rewrite `ControlledSyncUnits` part --- .../src/unit-control/test/index.tsx | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index 97a224a69d3b18..417b2b87fbb929 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -35,20 +35,31 @@ const getUnitLabel = () => document.body.querySelector( '.components-unit-control__unit-label' ); const ControlledSyncUnits = () => { - const [ state, setState ] = useState( { valueA: '', valueB: '' } ); + const [ state, setState ] = useState( { + valueA: '', + valueB: '', + } ); // Keep the unit sync'd between the two `UnitControl` instances. - const onUnitControlChange = ( fieldName, newValue ) => { - // eslint-disable-next-line @wordpress/no-unused-vars-before-return - const [ quantity, newUnit ] = parseQuantityAndUnitFromRawValue( + const onUnitControlChange = ( + fieldName: 'valueA' | 'valueB', + newValue?: string | number + ) => { + const parsedQuantityAndUnit = parseQuantityAndUnitFromRawValue( newValue ); + const quantity = parsedQuantityAndUnit[ 0 ]; if ( ! Number.isFinite( quantity ) ) { return; } - const nextState = { ...state, [ fieldName ]: newValue }; + const newUnit = parsedQuantityAndUnit[ 1 ]; + + const nextState = { + ...state, + [ fieldName ]: newValue, + }; Object.entries( state ).forEach( ( [ stateProp, stateValue ] ) => { const [ @@ -57,7 +68,9 @@ const ControlledSyncUnits = () => { ] = parseQuantityAndUnitFromRawValue( stateValue ); if ( stateProp !== fieldName && stateUnit !== newUnit ) { - nextState[ stateProp ] = `${ stateQuantity }${ newUnit }`; + nextState[ + stateProp as 'valueA' | 'valueB' + ] = `${ stateQuantity }${ newUnit }`; } } ); From f6c9d62c401d725a2df4b2d5c0903c1a020d4235 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 18:58:07 +0200 Subject: [PATCH 03/15] Use diff snapshot to test classnames --- .../test/__snapshots__/index.tsx.snap | 33 +++++++++++++++++++ .../src/unit-control/test/index.tsx | 10 ++++-- 2 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 packages/components/src/unit-control/test/__snapshots__/index.tsx.snap diff --git a/packages/components/src/unit-control/test/__snapshots__/index.tsx.snap b/packages/components/src/unit-control/test/__snapshots__/index.tsx.snap new file mode 100644 index 00000000000000..a723bc9f257d35 --- /dev/null +++ b/packages/components/src/unit-control/test/__snapshots__/index.tsx.snap @@ -0,0 +1,33 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`UnitControl Basic rendering should render custom className 1`] = ` +Snapshot Diff: +- First value ++ Second value + +@@ -1,10 +1,10 @@ +
+
+
+ { } ); it( 'should render custom className', () => { - render( ); + const { container: withoutClassName } = render( ); - const el = getComponent(); + const { container: withClassName } = render( + + ); - expect( el.classList.contains( 'hello' ) ).toBe( true ); + expect( withoutClassName.firstChild ).toMatchDiffSnapshot( + withClassName.firstChild + ); } ); it( 'should not render select, if units are disabled', () => { From 6ff8fd1088770a1362f012442f693b80674b2afb Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 18:59:07 +0200 Subject: [PATCH 04/15] Use `toBeInTheDocument` --- packages/components/src/unit-control/test/index.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index f904529f5582a8..09fc790b314bdf 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -100,8 +100,8 @@ describe( 'UnitControl', () => { const input = getInput(); const select = getSelect(); - expect( input ).toBeTruthy(); - expect( select ).toBeTruthy(); + expect( input ).toBeInTheDocument(); + expect( select ).toBeInTheDocument(); } ); it( 'should render custom className', () => { @@ -121,8 +121,8 @@ describe( 'UnitControl', () => { const input = getInput(); const select = getSelect(); - expect( input ).toBeTruthy(); - expect( select ).toBeFalsy(); + expect( input ).toBeInTheDocument(); + expect( select ).not.toBeInTheDocument(); } ); it( 'should render label if single units', () => { @@ -131,8 +131,8 @@ describe( 'UnitControl', () => { const select = getSelect(); const label = getUnitLabel(); - expect( select ).toBeFalsy(); - expect( label ).toBeTruthy(); + expect( select ).not.toBeInTheDocument(); + expect( label ).toBeInTheDocument(); } ); } ); From 46bb12ead0f5653ebc713d916e46d565ab777b47 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 18:59:59 +0200 Subject: [PATCH 05/15] Type internal test state setters --- .../src/unit-control/test/index.tsx | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index 09fc790b314bdf..be596250cfcbfd 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -158,8 +158,9 @@ describe( 'UnitControl', () => { } ); it( 'should increment value on UP press', async () => { - let state = '50px'; - const setState = ( nextState ) => ( state = nextState ); + let state: string | undefined = '50px'; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const { user } = render( @@ -172,8 +173,9 @@ describe( 'UnitControl', () => { } ); it( 'should increment value on UP + SHIFT press, with step', async () => { - let state = '50px'; - const setState = ( nextState ) => ( state = nextState ); + let state: string | undefined = '50px'; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const { user } = render( @@ -186,8 +188,9 @@ describe( 'UnitControl', () => { } ); it( 'should decrement value on DOWN press', async () => { - let state = 50; - const setState = ( nextState ) => ( state = nextState ); + let state: string | number | undefined = 50; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const { user } = render( @@ -200,8 +203,9 @@ describe( 'UnitControl', () => { } ); it( 'should decrement value on DOWN + SHIFT press, with step', async () => { - let state = 50; - const setState = ( nextState ) => ( state = nextState ); + let state: string | number | undefined = 50; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const { user } = render( @@ -214,8 +218,9 @@ describe( 'UnitControl', () => { } ); it( 'should cancel change when ESCAPE key is pressed', async () => { - let state = 50; - const setState = ( nextState ) => ( state = nextState ); + let state: string | number | undefined = 50; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const { user } = render( { } ); it( 'should run onBlur callback when quantity input is blurred', async () => { - let state = '33%'; const onChangeSpy = jest.fn(); const onBlurSpy = jest.fn(); - const setState = ( nextState ) => { + + let state: string | undefined = '33%'; + const setState: UnitControlOnChangeCallback = ( nextState ) => { onChangeSpy( nextState ); state = nextState; }; @@ -277,12 +283,11 @@ describe( 'UnitControl', () => { } ); it( 'should invoke onChange and onUnitChange callbacks when isPressEnterToChange is true and the component is blurred with an uncommitted value', async () => { - let state = '15px'; - const onUnitChangeSpy = jest.fn(); const onChangeSpy = jest.fn(); - const setState = ( nextState ) => { + let state: string | undefined = '15px'; + const setState: UnitControlOnChangeCallback = ( nextState ) => { onChangeSpy( nextState ); state = nextState; }; @@ -326,8 +331,9 @@ describe( 'UnitControl', () => { describe( 'Unit', () => { it( 'should update unit value on change', async () => { - let state = '14rem'; - const setState = ( nextState ) => ( state = nextState ); + let state: string | undefined = '14rem'; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const spy = jest.fn(); @@ -366,8 +372,9 @@ describe( 'UnitControl', () => { } ); it( 'should reset value on unit change, if unit has default value', async () => { - let state = 50; - const setState = ( nextState ) => ( state = nextState ); + let state: string | number | undefined = 50; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const units = [ { value: 'pt', label: 'pt', default: 25 }, @@ -394,8 +401,9 @@ describe( 'UnitControl', () => { } ); it( 'should not reset value on unit change, if disabled', async () => { - let state = 50; - const setState = ( nextState ) => ( state = nextState ); + let state: string | number | undefined = 50; + const setState: UnitControlOnChangeCallback = ( nextState ) => + ( state = nextState ); const units = [ { value: 'pt', label: 'pt', default: 25 }, @@ -422,8 +430,9 @@ describe( 'UnitControl', () => { } ); it( 'should set correct unit if single units', async () => { - let state = '50%'; - const setState = ( value ) => ( state = value ); + let state: string | undefined = '50%'; + const setState: UnitControlOnChangeCallback = ( value ) => + ( state = value ); const { user } = render( Date: Thu, 28 Apr 2022 19:09:05 +0200 Subject: [PATCH 06/15] Use `testing-library` semantic queries instead of `document.querySelector` --- .../src/unit-control/test/index.tsx | 52 ++++++++++++------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index be596250cfcbfd..a82f4bb136ae7a 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -14,6 +14,7 @@ import { useState } from '@wordpress/element'; */ import UnitControl from '..'; import { parseQuantityAndUnitFromRawValue } from '../utils'; +import type { UnitControlOnChangeCallback } from '../types'; function render( jsx ) { return { @@ -25,14 +26,17 @@ function render( jsx ) { }; } -const getComponent = () => - document.body.querySelector( '.components-unit-control' ); -const getInput = () => - document.body.querySelector( '.components-unit-control input' ); -const getSelect = () => - document.body.querySelector( '.components-unit-control select' ); -const getUnitLabel = () => - document.body.querySelector( '.components-unit-control__unit-label' ); +const getInput = ( { + isInputTypeText = false, +}: { + isInputTypeText?: boolean; +} = {} ) => + screen.getByRole( + isInputTypeText ? 'textbox' : 'spinbutton' + ) as HTMLInputElement; +const getSelect = () => screen.getByRole( 'combobox' ) as HTMLSelectElement; +const getSelectOptions = () => + screen.getAllByRole( 'option' ) as HTMLOptionElement[]; const ControlledSyncUnits = () => { const [ state, setState ] = useState( { @@ -119,7 +123,9 @@ describe( 'UnitControl', () => { it( 'should not render select, if units are disabled', () => { render( ); const input = getInput(); - const select = getSelect(); + // Using `queryByRole` instead of `getSelect` because we need to test + // for this element NOT to be in the document. + const select = screen.queryByRole( 'combobox' ); expect( input ).toBeInTheDocument(); expect( select ).not.toBeInTheDocument(); @@ -128,8 +134,8 @@ describe( 'UnitControl', () => { it( 'should render label if single units', () => { render( ); - const select = getSelect(); - const label = getUnitLabel(); + const select = screen.queryByRole( 'combobox' ); + const label = screen.getByText( '%' ); expect( select ).not.toBeInTheDocument(); expect( label ).toBeInTheDocument(); @@ -230,7 +236,8 @@ describe( 'UnitControl', () => { /> ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '300px' ); @@ -304,7 +311,8 @@ describe( 'UnitControl', () => { ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '41vh' ); @@ -360,8 +368,7 @@ describe( 'UnitControl', () => { render( ); - const select = getSelect(); - const options = select.querySelectorAll( 'option' ); + const options = getSelectOptions(); expect( options.length ).toBe( 2 ); @@ -545,7 +552,8 @@ describe( 'UnitControl', () => { /> ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '55 em' ); user.keyboard( '{Enter}' ); @@ -562,7 +570,8 @@ describe( 'UnitControl', () => { /> ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '61 PX' ); user.keyboard( '{Enter}' ); @@ -579,7 +588,8 @@ describe( 'UnitControl', () => { /> ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '55 em' ); user.keyboard( '{Enter}' ); @@ -596,7 +606,8 @@ describe( 'UnitControl', () => { /> ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '-10 %' ); user.keyboard( '{Enter}' ); @@ -613,7 +624,8 @@ describe( 'UnitControl', () => { /> ); - const input = getInput(); + // Input type is `text` when the `isPressEnterToChange` prop is passed + const input = getInput( { isInputTypeText: true } ); await user.clear( input ); await user.type( input, '123 rEm ' ); user.keyboard( '{Enter}' ); From 7d1eaec19b93fe52e842d942bc0ec632b93ac9c4 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 19:22:21 +0200 Subject: [PATCH 07/15] Tweak optional parameter for some utilities --- packages/components/src/unit-control/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/unit-control/utils.ts b/packages/components/src/unit-control/utils.ts index 87c0142ec9774c..0f1d69c8ac4ecf 100644 --- a/packages/components/src/unit-control/utils.ts +++ b/packages/components/src/unit-control/utils.ts @@ -218,7 +218,7 @@ export function parseQuantityAndUnitFromRawValue( */ export function getValidParsedQuantityAndUnit( rawValue: string | number, - allowedUnits: WPUnitControlUnit[], + allowedUnits?: WPUnitControlUnit[], fallbackQuantity?: number, fallbackUnit?: string ): [ number | undefined, string | undefined ] { @@ -295,7 +295,7 @@ export const useCustomUnits = ( { }: { units?: WPUnitControlUnit[]; availableUnits?: string[]; - defaultValues: Record< string, number >; + defaultValues?: Record< string, number >; } ): WPUnitControlUnit[] => { const customUnitsToReturn = filterUnitsWithSettings( availableUnits, From bcbf18680eed1d57b6eb6950d9e90f8ded8d4e77 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 19:22:48 +0200 Subject: [PATCH 08/15] Add type guard to `hasUnits` --- packages/components/src/unit-control/utils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/components/src/unit-control/utils.ts b/packages/components/src/unit-control/utils.ts index 0f1d69c8ac4ecf..573a6f88665109 100644 --- a/packages/components/src/unit-control/utils.ts +++ b/packages/components/src/unit-control/utils.ts @@ -155,7 +155,9 @@ export function getParsedQuantityAndUnit( * @param units List of units. * @return Whether the list actually contains any units. */ -export function hasUnits( units?: WPUnitControlUnit[] ): boolean { +export function hasUnits( + units?: WPUnitControlUnit[] +): units is WPUnitControlUnit[] { // Although the `isArray` check shouldn't be necessary (given the signature of // this typed function), it's better to stay on the side of caution, since // this function may be called from un-typed environments. From 8317e71802598e24a492d9d45ec92d6a4b1a9474 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 19:23:10 +0200 Subject: [PATCH 09/15] Fix TS errors in utils tests --- .../components/src/unit-control/test/utils.ts | 57 ++++++++++++------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/packages/components/src/unit-control/test/utils.ts b/packages/components/src/unit-control/test/utils.ts index b5fe385716dedf..7a6645d1c02564 100644 --- a/packages/components/src/unit-control/test/utils.ts +++ b/packages/components/src/unit-control/test/utils.ts @@ -7,21 +7,28 @@ import { getValidParsedQuantityAndUnit, getUnitsWithCurrentUnit, } from '../utils'; +import type { WPUnitControlUnit } from '../types'; describe( 'UnitControl utils', () => { describe( 'useCustomUnits', () => { it( 'should return filtered css units', () => { - const cssUnits = [ { value: 'px' }, { value: '%' } ]; + const cssUnits = [ + { value: 'px', label: 'pixel' }, + { value: '%', label: 'percent' }, + ]; const units = useCustomUnits( { availableUnits: [ 'em', 'px' ], units: cssUnits, } ); - expect( units ).toEqual( [ { value: 'px' } ] ); + expect( units ).toEqual( [ { value: 'px', label: 'pixel' } ] ); } ); it( 'should add default values to available units', () => { - const cssUnits = [ { value: 'px' }, { value: '%' } ]; + const cssUnits = [ + { value: 'px', label: 'pixel' }, + { value: '%', label: 'percent' }, + ]; const units = useCustomUnits( { availableUnits: [ '%', 'px' ], defaultValues: { '%': 10, px: 10 }, @@ -29,8 +36,8 @@ describe( 'UnitControl utils', () => { } ); expect( units ).toEqual( [ - { value: 'px', default: 10 }, - { value: '%', default: 10 }, + { value: 'px', label: 'pixel', default: 10 }, + { value: '%', label: 'percent', default: 10 }, ] ); } ); @@ -38,24 +45,32 @@ describe( 'UnitControl utils', () => { // Although the public APIs of the component expect a `number` as the type of the // default values, it's still good to test for strings (as it can happen in un-typed // environments) - const cssUnits = [ { value: 'px' }, { value: '%' } ]; + const cssUnits = [ + { value: 'px', label: 'pixel' }, + { value: '%', label: 'percent' }, + ]; const units = useCustomUnits( { availableUnits: [ '%', 'px' ], defaultValues: { + // @ts-ignore (passing a string instead of a number is the point of the test) '%': '14', + // @ts-ignore (passing a string instead of a number is the point of the test) px: 'not a valid numeric quantity', }, units: cssUnits, } ); expect( units ).toEqual( [ - { value: 'px', default: undefined }, - { value: '%', default: 14 }, + { value: 'px', label: 'pixel', default: undefined }, + { value: '%', label: 'percent', default: 14 }, ] ); } ); it( 'should return an empty array where availableUnits match no preferred css units', () => { - const cssUnits = [ { value: 'em' }, { value: 'vh' } ]; + const cssUnits = [ + { value: 'em', label: 'em' }, + { value: 'vh', label: 'vh' }, + ]; const units = useCustomUnits( { availableUnits: [ '%', 'px' ], defaultValues: { '%': 10, px: 10 }, @@ -69,16 +84,19 @@ describe( 'UnitControl utils', () => { describe( 'filterUnitsWithSettings', () => { it( 'should return filtered units array', () => { const preferredUnits = [ '%', 'px' ]; - const availableUnits = [ { value: 'px' }, { value: 'em' } ]; + const availableUnits = [ + { value: 'px', label: 'pixel' }, + { value: 'em', label: 'em' }, + ]; expect( filterUnitsWithSettings( preferredUnits, availableUnits ) - ).toEqual( [ { value: 'px' } ] ); + ).toEqual( [ { value: 'px', label: 'pixel' } ] ); } ); it( 'should return empty array where preferred units match no available css unit', () => { const preferredUnits = [ '%', 'px' ]; - const availableUnits = [ { value: 'em' } ]; + const availableUnits = [ { value: 'em', label: 'em' } ]; expect( filterUnitsWithSettings( preferredUnits, availableUnits ) @@ -92,13 +110,14 @@ describe( 'UnitControl utils', () => { const availableUnits = false; expect( + // @ts-ignore (passing `false` instead of a valid array of units is the point of the test) filterUnitsWithSettings( preferredUnits, availableUnits ) ).toEqual( [] ); } ); it( 'should return empty array where available units is set to an empty array', () => { const preferredUnits = [ '%', 'px' ]; - const availableUnits = []; + const availableUnits: WPUnitControlUnit[] = []; expect( filterUnitsWithSettings( preferredUnits, availableUnits ) @@ -127,7 +146,7 @@ describe( 'UnitControl utils', () => { it( 'should return fallback value', () => { const nextValue = 'thirteen'; - const preferredUnits = [ { value: 'em' } ]; + const preferredUnits = [ { value: 'em', label: 'em' } ]; const fallbackValue = 13; expect( @@ -155,7 +174,7 @@ describe( 'UnitControl utils', () => { it( 'should return first unit in preferred units collection as second fallback unit', () => { const nextValue = 101; - const preferredUnits = [ { value: 'px' } ]; + const preferredUnits = [ { value: 'px', label: 'pixel' } ]; expect( getValidParsedQuantityAndUnit( nextValue, preferredUnits ) @@ -185,8 +204,8 @@ describe( 'UnitControl utils', () => { expect( result ).toHaveLength( 3 ); const currentUnit = result.shift(); - expect( currentUnit.value ).toBe( '%' ); - expect( currentUnit.label ).toBe( '%' ); + expect( currentUnit?.value ).toBe( '%' ); + expect( currentUnit?.label ).toBe( '%' ); expect( result ).toEqual( limitedUnits ); } ); @@ -196,8 +215,8 @@ describe( 'UnitControl utils', () => { expect( result ).toHaveLength( 3 ); const currentUnit = result.shift(); - expect( currentUnit.value ).toBe( '%' ); - expect( currentUnit.label ).toBe( '%' ); + expect( currentUnit?.value ).toBe( '%' ); + expect( currentUnit?.label ).toBe( '%' ); expect( result ).toEqual( limitedUnits ); } ); From 3c90c080713eb2476f730fcf77278670ae4f78de Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 19:24:16 +0200 Subject: [PATCH 10/15] Add types to user-event wrapper function --- packages/components/src/unit-control/test/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index a82f4bb136ae7a..4e992deb8142ea 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -16,7 +16,7 @@ import UnitControl from '..'; import { parseQuantityAndUnitFromRawValue } from '../utils'; import type { UnitControlOnChangeCallback } from '../types'; -function render( jsx ) { +function render( jsx: React.ReactElement ) { return { user: userEvent.setup( { // Avoids timeout errors (https://github.com/testing-library/user-event/issues/565#issuecomment-1064579531). From 048222a6cda9399feab3b0bf0b0751c57c897ebf Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 19:55:24 +0200 Subject: [PATCH 11/15] CHANGELOG --- packages/components/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 2d132c096efc8c..b6bc54443a6c13 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Internal + +- `UnitControl`: migrate unit tests to TypeScript ([#40697](https://github.com/WordPress/gutenberg/pull/40697)). + ### Enhancements - `InputControl`: Add `__next36pxDefaultSize` flag for larger default size ([#40622](https://github.com/WordPress/gutenberg/pull/40622)). From ecae33e0a05738b58e1e5c69039f03116c54d6c0 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 28 Apr 2022 19:56:28 +0200 Subject: [PATCH 12/15] Add comment about failing test --- packages/components/src/unit-control/test/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index 4e992deb8142ea..245946612c99ca 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -135,6 +135,7 @@ describe( 'UnitControl', () => { render( ); const select = screen.queryByRole( 'combobox' ); + // The unit is not being displayed! const label = screen.getByText( '%' ); expect( select ).not.toBeInTheDocument(); From 54379dc6895389ae5affd154d6e98cb8ee9e9518 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 29 Apr 2022 16:07:33 +0200 Subject: [PATCH 13/15] ts-expect-error instead of ts-ignore --- packages/components/src/unit-control/test/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/unit-control/test/utils.ts b/packages/components/src/unit-control/test/utils.ts index 7a6645d1c02564..83d57a5b519dc3 100644 --- a/packages/components/src/unit-control/test/utils.ts +++ b/packages/components/src/unit-control/test/utils.ts @@ -52,9 +52,9 @@ describe( 'UnitControl utils', () => { const units = useCustomUnits( { availableUnits: [ '%', 'px' ], defaultValues: { - // @ts-ignore (passing a string instead of a number is the point of the test) + // @ts-expect-error (passing a string instead of a number is the point of the test) '%': '14', - // @ts-ignore (passing a string instead of a number is the point of the test) + // @ts-expect-error (passing a string instead of a number is the point of the test) px: 'not a valid numeric quantity', }, units: cssUnits, @@ -110,7 +110,7 @@ describe( 'UnitControl utils', () => { const availableUnits = false; expect( - // @ts-ignore (passing `false` instead of a valid array of units is the point of the test) + // @ts-expect-error (passing `false` instead of a valid array of units is the point of the test) filterUnitsWithSettings( preferredUnits, availableUnits ) ).toEqual( [] ); } ); From 8929d0beb50ae10fe55a6fe79effa8f3d6d16420 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 29 Apr 2022 16:09:41 +0200 Subject: [PATCH 14/15] Fix broken test by passing the missing `value` --- packages/components/src/unit-control/test/index.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/components/src/unit-control/test/index.tsx b/packages/components/src/unit-control/test/index.tsx index 245946612c99ca..541a1d5862484c 100644 --- a/packages/components/src/unit-control/test/index.tsx +++ b/packages/components/src/unit-control/test/index.tsx @@ -132,10 +132,14 @@ describe( 'UnitControl', () => { } ); it( 'should render label if single units', () => { - render( ); + render( + + ); const select = screen.queryByRole( 'combobox' ); - // The unit is not being displayed! const label = screen.getByText( '%' ); expect( select ).not.toBeInTheDocument(); From a237da4562152e07d414d7fa914ba647cf475d2f Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 29 Apr 2022 16:36:27 +0200 Subject: [PATCH 15/15] Add temporary stories to check single unit behavior --- .../src/unit-control/stories/index.tsx | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/components/src/unit-control/stories/index.tsx b/packages/components/src/unit-control/stories/index.tsx index 87dfd3a84c602d..5fdace14370ea6 100644 --- a/packages/components/src/unit-control/stories/index.tsx +++ b/packages/components/src/unit-control/stories/index.tsx @@ -109,10 +109,40 @@ TweakingTheNumberInput.args = { /** * When only one unit is available, the unit selection dropdown becomes static text. */ -export const WithSingleUnit: ComponentStory< +export const WithSingleUnitControlled: ComponentStory< typeof UnitControl -> = DefaultTemplate.bind( {} ); -WithSingleUnit.args = { +> = ( { onChange, ...args } ) => { + // Starting value is `undefined` + const [ value, setValue ] = useState< string | undefined >( undefined ); + + return ( +
+ { + setValue( v ); + onChange?.( v, extra ); + } } + /> +
+ ); +}; +WithSingleUnitControlled.args = { + ...Default.args, + units: CSS_UNITS.slice( 0, 1 ), +}; + +export const WithSingleUnitUncontrolled: ComponentStory< + typeof UnitControl +> = ( args ) => { + return ( +
+ +
+ ); +}; +WithSingleUnitUncontrolled.args = { ...Default.args, units: CSS_UNITS.slice( 0, 1 ), };