From e3bc714a5b7a4d41faa9896e38bbf9e3281598bb Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 3 Mar 2022 12:38:36 +0100 Subject: [PATCH 01/14] `InputControl`: always commit value to internal state on `blur` --- packages/components/src/input-control/input-field.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/components/src/input-control/input-field.tsx b/packages/components/src/input-control/input-field.tsx index 402f9196dccd1..17177874bc99b 100644 --- a/packages/components/src/input-control/input-field.tsx +++ b/packages/components/src/input-control/input-field.tsx @@ -104,14 +104,13 @@ function InputField( onBlur( event ); setIsFocused?.( false ); - /** - * If isPressEnterToChange is set, this commits the value to - * the onChange callback. - */ + // If isPressEnterToChange is set, cause the `onChange` callback to be fired. if ( isPressEnterToChange && isDirty ) { wasDirtyOnBlur.current = true; - handleOnCommit( event ); } + + // Commit the value (which allows consumers to apply their validation) + handleOnCommit( event ); }; const handleOnFocus = ( event: FocusEvent< HTMLInputElement > ) => { From b2f2138ca76c31810e33ec1a7fa7734f21ef7268 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 3 Mar 2022 12:57:23 +0100 Subject: [PATCH 02/14] `NumberControl`: constain value also on the `UPDATE` action --- packages/components/src/number-control/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/number-control/index.js b/packages/components/src/number-control/index.js index 4d840682ecf41..917e5cface3ce 100644 --- a/packages/components/src/number-control/index.js +++ b/packages/components/src/number-control/index.js @@ -152,7 +152,8 @@ export function NumberControl( */ if ( type === inputControlActionTypes.PRESS_ENTER || - type === inputControlActionTypes.COMMIT + type === inputControlActionTypes.COMMIT || + type === inputControlActionTypes.UPDATE ) { const applyEmptyValue = required === false && currentValue === ''; From bb15bd225f9e014e9866d27703945ec248567830 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 3 Mar 2022 14:03:18 +0100 Subject: [PATCH 03/14] CHANGELOG --- packages/components/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 52d33a582e8c6..ecba87f220e04 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -15,6 +15,10 @@ - Delete the `composeStateReducers` utility function ([#39262](https://github.com/WordPress/gutenberg/pull/39262)). - `BoxControl`: stop using `UnitControl`'s deprecated `unit` prop ([#39511](https://github.com/WordPress/gutenberg/pull/39511)). +### Bug Fix + +- `NumberControl`: commit (and constrain) value on `blur` event ([#39186](https://github.com/WordPress/gutenberg/pull/39186)). + ## 19.6.0 (2022-03-11) ### Enhancements From 823cf7bc1fca146f776e6bb8d22bf0569eec3c79 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 3 Mar 2022 14:15:51 +0100 Subject: [PATCH 04/14] Add unit test --- .../components/src/number-control/test/index.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/components/src/number-control/test/index.js b/packages/components/src/number-control/test/index.js index db51a66681c67..90123669c03ac 100644 --- a/packages/components/src/number-control/test/index.js +++ b/packages/components/src/number-control/test/index.js @@ -82,6 +82,22 @@ describe( 'NumberControl', () => { expect( input.value ).toBe( '0' ); } ); + it( 'should clamp value within range on blur', () => { + render( ); + + const input = getInput(); + input.focus(); + fireEvent.change( input, { target: { value: 41 } } ); + + // Before blurring, the value is still un-clamped + expect( input.value ).toBe( '41' ); + + input.blur(); + + // After blur, value is clamped + expect( input.value ).toBe( '10' ); + } ); + it( 'should parse to number value on ENTER keypress when required', () => { render( ); From a1c6fc78672431b0b1fd35ac99d59eb1dba69fc6 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 8 Mar 2022 12:28:25 +0100 Subject: [PATCH 05/14] Remove `UPDATE` action from `InputControl` s state reducer --- packages/components/src/input-control/input-field.tsx | 3 +-- packages/components/src/input-control/reducer/actions.ts | 8 +------- packages/components/src/input-control/reducer/reducer.ts | 7 ------- packages/components/src/number-control/index.js | 3 +-- 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/components/src/input-control/input-field.tsx b/packages/components/src/input-control/input-field.tsx index 17177874bc99b..085ee38d367ac 100644 --- a/packages/components/src/input-control/input-field.tsx +++ b/packages/components/src/input-control/input-field.tsx @@ -67,7 +67,6 @@ function InputField( pressEnter, pressUp, reset, - update, } = useInputControlStateReducer( stateReducer, { isDragEnabled, value: valueProp, @@ -91,7 +90,7 @@ function InputField( return; } if ( ! isFocused && ! wasDirtyOnBlur.current ) { - update( valueProp, _event as SyntheticEvent ); + commit( valueProp, _event as SyntheticEvent ); } else if ( ! isDirty ) { onChange( value, { event: _event as ChangeEvent< HTMLInputElement >, diff --git a/packages/components/src/input-control/reducer/actions.ts b/packages/components/src/input-control/reducer/actions.ts index 687a4905d0bb8..d66eb5778aad8 100644 --- a/packages/components/src/input-control/reducer/actions.ts +++ b/packages/components/src/input-control/reducer/actions.ts @@ -18,7 +18,6 @@ export const PRESS_DOWN = 'PRESS_DOWN'; export const PRESS_ENTER = 'PRESS_ENTER'; export const PRESS_UP = 'PRESS_UP'; export const RESET = 'RESET'; -export const UPDATE = 'UPDATE'; interface EventPayload { event?: SyntheticEvent; @@ -42,14 +41,9 @@ export type DragStartAction = Action< typeof DRAG_START, DragProps >; export type DragEndAction = Action< typeof DRAG_END, DragProps >; export type DragAction = Action< typeof DRAG, DragProps >; export type ResetAction = Action< typeof RESET, Partial< ValuePayload > >; -export type UpdateAction = Action< typeof UPDATE, ValuePayload >; export type InvalidateAction = Action< typeof INVALIDATE, { error: unknown } >; -export type ChangeEventAction = - | ChangeAction - | ResetAction - | CommitAction - | UpdateAction; +export type ChangeEventAction = ChangeAction | ResetAction | CommitAction; export type DragEventAction = DragStartAction | DragEndAction | DragAction; diff --git a/packages/components/src/input-control/reducer/reducer.ts b/packages/components/src/input-control/reducer/reducer.ts index d94da7e104242..de13dd9a8f950 100644 --- a/packages/components/src/input-control/reducer/reducer.ts +++ b/packages/components/src/input-control/reducer/reducer.ts @@ -100,11 +100,6 @@ function inputControlStateReducer( nextState.value = action.payload.value || state.initialValue; break; - case actions.UPDATE: - nextState.value = action.payload.value; - nextState.isDirty = false; - break; - /** * Validation */ @@ -197,7 +192,6 @@ export function useInputControlStateReducer( dispatch( { type: actions.INVALIDATE, payload: { error, event } } ); const reset = createChangeEvent( actions.RESET ); const commit = createChangeEvent( actions.COMMIT ); - const update = createChangeEvent( actions.UPDATE ); const dragStart = createDragEvent( actions.DRAG_START ); const drag = createDragEvent( actions.DRAG ); @@ -220,6 +214,5 @@ export function useInputControlStateReducer( pressUp, reset, state, - update, } as const; } diff --git a/packages/components/src/number-control/index.js b/packages/components/src/number-control/index.js index 917e5cface3ce..4d840682ecf41 100644 --- a/packages/components/src/number-control/index.js +++ b/packages/components/src/number-control/index.js @@ -152,8 +152,7 @@ export function NumberControl( */ if ( type === inputControlActionTypes.PRESS_ENTER || - type === inputControlActionTypes.COMMIT || - type === inputControlActionTypes.UPDATE + type === inputControlActionTypes.COMMIT ) { const applyEmptyValue = required === false && currentValue === ''; From 1e5123c022f3ef343cb62b4e8769018eea173639 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Mar 2022 13:17:25 +0100 Subject: [PATCH 06/14] Add unit test to check that `onChange` gets called when the value is clamped on `blur` --- .../src/number-control/test/index.js | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/components/src/number-control/test/index.js b/packages/components/src/number-control/test/index.js index 90123669c03ac..96c6afbd5aa96 100644 --- a/packages/components/src/number-control/test/index.js +++ b/packages/components/src/number-control/test/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; /** * WordPress dependencies @@ -63,6 +63,36 @@ describe( 'NumberControl', () => { expect( spy ).toHaveBeenCalledWith( '10' ); } ); + + it( 'should call onChange callback when value is clamped on blur', async () => { + const spy = jest.fn(); + render( + spy( v ) } + /> + ); + + const input = getInput(); + input.focus(); + fireEvent.change( input, { target: { value: 1 } } ); + + // Before blurring, the value is still un-clamped + expect( input.value ).toBe( '1' ); + + input.blur(); + + // After blur, value is clamped + expect( input.value ).toBe( '4' ); + + await waitFor( () => { + expect( spy ).toHaveBeenCalledTimes( 2 ); + expect( spy ).toHaveBeenNthCalledWith( 1, '1' ); + expect( spy ).toHaveBeenNthCalledWith( 2, 4 ); + } ); + } ); } ); describe( 'Validation', () => { From 6f5696e9a44f95a2f92d4e361239002a98af7d85 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Mar 2022 13:22:48 +0100 Subject: [PATCH 07/14] Fix input control blur logic, so that `onChange` is called after clamping the value --- packages/components/src/input-control/input-field.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/components/src/input-control/input-field.tsx b/packages/components/src/input-control/input-field.tsx index 085ee38d367ac..cc827d719bd79 100644 --- a/packages/components/src/input-control/input-field.tsx +++ b/packages/components/src/input-control/input-field.tsx @@ -103,13 +103,14 @@ function InputField( onBlur( event ); setIsFocused?.( false ); - // If isPressEnterToChange is set, cause the `onChange` callback to be fired. - if ( isPressEnterToChange && isDirty ) { + /** + * If isPressEnterToChange is set, this commits the value to + * the onChange callback. + */ + if ( isDirty || ! event.target.validity.valid ) { wasDirtyOnBlur.current = true; + handleOnCommit( event ); } - - // Commit the value (which allows consumers to apply their validation) - handleOnCommit( event ); }; const handleOnFocus = ( event: FocusEvent< HTMLInputElement > ) => { From 5db0df6e7d650613a50ead9f2d0a25840c8830ec Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Mar 2022 13:23:29 +0100 Subject: [PATCH 08/14] Comments --- packages/components/src/number-control/index.js | 2 +- packages/components/src/number-control/test/index.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/number-control/index.js b/packages/components/src/number-control/index.js index 4d840682ecf41..5e21b8ab970f5 100644 --- a/packages/components/src/number-control/index.js +++ b/packages/components/src/number-control/index.js @@ -148,7 +148,7 @@ export function NumberControl( } /** - * Handles commit (ENTER key press or on blur if isPressEnterToChange) + * Handles commit (ENTER key press or blur) */ if ( type === inputControlActionTypes.PRESS_ENTER || diff --git a/packages/components/src/number-control/test/index.js b/packages/components/src/number-control/test/index.js index 96c6afbd5aa96..669ff9a9af29c 100644 --- a/packages/components/src/number-control/test/index.js +++ b/packages/components/src/number-control/test/index.js @@ -87,6 +87,7 @@ describe( 'NumberControl', () => { // After blur, value is clamped expect( input.value ).toBe( '4' ); + // After the blur, the `onChange` callback fires asynchronously. await waitFor( () => { expect( spy ).toHaveBeenCalledTimes( 2 ); expect( spy ).toHaveBeenNthCalledWith( 1, '1' ); From f6d2654550006ca2e8ab48d437b2de8545ce4696 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Mar 2022 15:06:23 +0100 Subject: [PATCH 09/14] Use `event.target.validity.valid` in docs, storybook and unit tests --- .../components/src/number-control/README.md | 14 +++++++++ .../src/number-control/stories/index.js | 21 ++++++++----- .../src/number-control/test/index.js | 31 +++++++++++++++++++ 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/packages/components/src/number-control/README.md b/packages/components/src/number-control/README.md index 2e57f69fbc36c..b3c62ebbcc687 100644 --- a/packages/components/src/number-control/README.md +++ b/packages/components/src/number-control/README.md @@ -97,6 +97,20 @@ The minimum `value` allowed. - Required: No - Default: `-Infinity` +### onChange + +Callback fired whenever the value of the input changes. + +The callback receives two arguments: + +1. `newValue`: the new value of the input +2. `extra`: an object containing, under the `event` key, the original browser event. + +Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props). In order to check the value's validity, check the `event.target.validity.valid` property from the callback's second argument. + +- Type: `(newValue, extra) => void` +- Required: No + ### required If `true` enforces a valid number within the control's min/max range. If `false` allows an empty string as a valid value. diff --git a/packages/components/src/number-control/stories/index.js b/packages/components/src/number-control/stories/index.js index 25106b0c364b8..9c8496cc908c2 100644 --- a/packages/components/src/number-control/stories/index.js +++ b/packages/components/src/number-control/stories/index.js @@ -23,6 +23,7 @@ export default { function Example() { const [ value, setValue ] = useState( '0' ); + const [ isValidValue, setIsValidValue ] = useState( true ); const props = { disabled: boolean( 'disabled', false ), @@ -32,18 +33,24 @@ function Example() { label: text( 'label', 'Number' ), min: number( 'min', 0 ), max: number( 'max', 100 ), - placeholder: text( 'placeholder', 0 ), + placeholder: text( 'placeholder', '0' ), required: boolean( 'required', false ), shiftStep: number( 'shiftStep', 10 ), - step: text( 'step', 1 ), + step: text( 'step', '1' ), }; return ( - setValue( v ) } - /> + <> + { + setValue( v ); + setIsValidValue( extra.event.target.validity.valid ); + } } + /> +

Is valid? { isValidValue ? 'Yes' : 'No' }

+ ); } diff --git a/packages/components/src/number-control/test/index.js b/packages/components/src/number-control/test/index.js index 669ff9a9af29c..7be791db232ec 100644 --- a/packages/components/src/number-control/test/index.js +++ b/packages/components/src/number-control/test/index.js @@ -94,6 +94,37 @@ describe( 'NumberControl', () => { expect( spy ).toHaveBeenNthCalledWith( 2, 4 ); } ); } ); + + it( 'should call onChange callback when value is not valid', () => { + const spy = jest.fn(); + render( + + spy( v, extra.event.target.validity.valid ) + } + /> + ); + + const input = getInput(); + input.focus(); + fireEvent.change( input, { target: { value: 14 } } ); + + expect( input.value ).toBe( '14' ); + + fireKeyDown( { keyCode: ENTER } ); + + expect( input.value ).toBe( '10' ); + + expect( spy ).toHaveBeenCalledTimes( 2 ); + + // First call: invalid, unclamped value + expect( spy ).toHaveBeenNthCalledWith( 1, '14', false ); + // Second call: valid, clamped value + expect( spy ).toHaveBeenNthCalledWith( 2, 10, true ); + } ); } ); describe( 'Validation', () => { From 115456668e2bbae347bdd679f2f0457c9e434d39 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 10 Mar 2022 16:01:22 +0100 Subject: [PATCH 10/14] README --- packages/components/src/number-control/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/number-control/README.md b/packages/components/src/number-control/README.md index b3c62ebbcc687..fdce7df660c01 100644 --- a/packages/components/src/number-control/README.md +++ b/packages/components/src/number-control/README.md @@ -106,7 +106,7 @@ The callback receives two arguments: 1. `newValue`: the new value of the input 2. `extra`: an object containing, under the `event` key, the original browser event. -Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props). In order to check the value's validity, check the `event.target.validity.valid` property from the callback's second argument. +Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props, or it could not match the `step`). In order to check the value's validity, check the `event.target.validity.valid` property from the callback's second argument. - Type: `(newValue, extra) => void` - Required: No From 046ca573417adf9629b8b50bdab66c178ce70749 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 15 Mar 2022 15:28:35 +0100 Subject: [PATCH 11/14] Add docs and update example about input validity --- packages/components/src/number-control/README.md | 2 +- packages/components/src/number-control/stories/index.js | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/components/src/number-control/README.md b/packages/components/src/number-control/README.md index fdce7df660c01..31a97c64031ac 100644 --- a/packages/components/src/number-control/README.md +++ b/packages/components/src/number-control/README.md @@ -106,7 +106,7 @@ The callback receives two arguments: 1. `newValue`: the new value of the input 2. `extra`: an object containing, under the `event` key, the original browser event. -Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props, or it could not match the `step`). In order to check the value's validity, check the `event.target.validity.valid` property from the callback's second argument. +Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props, or it could not match the `step`). In order to check the value's validity, check the `event.target?.validity.valid` property from the callback's second argument (_note: given how the component works internally, the `validity` property is not guaranteed to always be defined on `event.target`. When the `validity` property is not defined, you can assume the value received as valid_). - Type: `(newValue, extra) => void` - Required: No diff --git a/packages/components/src/number-control/stories/index.js b/packages/components/src/number-control/stories/index.js index 9c8496cc908c2..d604be981709a 100644 --- a/packages/components/src/number-control/stories/index.js +++ b/packages/components/src/number-control/stories/index.js @@ -46,7 +46,13 @@ function Example() { value={ value } onChange={ ( v, extra ) => { setValue( v ); - setIsValidValue( extra.event.target.validity.valid ); + // Given how the component works internally, the `validity` property + // is not guaranteed to always be defined on `event.target`. When the + // `validity` property is not defined, you can assume the value + // received as valid. + setIsValidValue( + extra.event.target.validity?.valid ?? true + ); } } />

Is valid? { isValidValue ? 'Yes' : 'No' }

From 5a6154f2bf6058f47b5fe286ec1d9bebc7344c49 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 15 Mar 2022 17:16:22 +0100 Subject: [PATCH 12/14] Update `onChange` event type --- .../components/src/input-control/input-field.tsx | 2 +- packages/components/src/input-control/types.ts | 3 ++- packages/components/src/unit-control/index.tsx | 13 +++++++------ packages/components/src/unit-control/types.ts | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/components/src/input-control/input-field.tsx b/packages/components/src/input-control/input-field.tsx index cc827d719bd79..d4c673cc9729a 100644 --- a/packages/components/src/input-control/input-field.tsx +++ b/packages/components/src/input-control/input-field.tsx @@ -93,7 +93,7 @@ function InputField( commit( valueProp, _event as SyntheticEvent ); } else if ( ! isDirty ) { onChange( value, { - event: _event as ChangeEvent< HTMLInputElement >, + event: _event as ChangeEvent< HTMLInputElement > | PointerEvent, } ); wasDirtyOnBlur.current = false; } diff --git a/packages/components/src/input-control/types.ts b/packages/components/src/input-control/types.ts index c6867deba8717..25a10ca70ce8c 100644 --- a/packages/components/src/input-control/types.ts +++ b/packages/components/src/input-control/types.ts @@ -6,6 +6,7 @@ import type { ReactNode, ChangeEvent, SyntheticEvent, + PointerEvent, } from 'react'; import type { useDrag } from '@use-gesture/react'; @@ -33,7 +34,7 @@ interface BaseProps { } export type InputChangeCallback< - E = ChangeEvent< HTMLInputElement >, + E = ChangeEvent< HTMLInputElement > | PointerEvent, P = {} > = ( nextValue: string | undefined, extra: { event: E } & P ) => void; diff --git a/packages/components/src/unit-control/index.tsx b/packages/components/src/unit-control/index.tsx index a477eacbcc8fc..1b823c566f0e3 100644 --- a/packages/components/src/unit-control/index.tsx +++ b/packages/components/src/unit-control/index.tsx @@ -7,6 +7,7 @@ import type { ForwardedRef, SyntheticEvent, ChangeEvent, + PointerEvent, } from 'react'; import { omit } from 'lodash'; import classnames from 'classnames'; @@ -45,7 +46,7 @@ function UnforwardedUnitControl( isResetValueOnUnitChange = false, isUnitSelectTabbable = true, label, - onChange, + onChange: onChangeProp, onUnitChange, size = 'default', style, @@ -89,14 +90,14 @@ function UnforwardedUnitControl( const handleOnQuantityChange = ( nextQuantityValue: number | string | undefined, - changeProps: { event: ChangeEvent< HTMLInputElement > } + changeProps: { event: ChangeEvent< HTMLInputElement > | PointerEvent } ) => { if ( nextQuantityValue === '' || typeof nextQuantityValue === 'undefined' || nextQuantityValue === null ) { - onChange?.( '', changeProps ); + onChangeProp?.( '', changeProps ); return; } @@ -111,7 +112,7 @@ function UnforwardedUnitControl( unit ).join( '' ); - onChange?.( onChangeValue, changeProps ); + onChangeProp?.( onChangeValue, changeProps ); }; const handleOnUnitChange: UnitControlOnChangeCallback = ( @@ -126,7 +127,7 @@ function UnforwardedUnitControl( nextValue = `${ data.default }${ nextUnitValue }`; } - onChange?.( nextValue, changeProps ); + onChangeProp?.( nextValue, changeProps ); onUnitChange?.( nextUnitValue, changeProps ); setUnit( nextUnitValue ); @@ -155,7 +156,7 @@ function UnforwardedUnitControl( : undefined; const changeProps = { event, data }; - onChange?.( + onChangeProp?.( `${ validParsedQuantity ?? '' }${ validParsedUnit }`, changeProps ); diff --git a/packages/components/src/unit-control/types.ts b/packages/components/src/unit-control/types.ts index 46d8707b9639f..795aaae9e3029 100644 --- a/packages/components/src/unit-control/types.ts +++ b/packages/components/src/unit-control/types.ts @@ -39,7 +39,7 @@ export type WPUnitControlUnit = { }; export type UnitControlOnChangeCallback = InputChangeCallback< - SyntheticEvent< HTMLSelectElement | HTMLInputElement >, + SyntheticEvent< HTMLSelectElement | HTMLInputElement | Element >, { data?: WPUnitControlUnit } >; From 43ff576b7a722f9bab938f86e507ccef52f9cc3f Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 15 Mar 2022 20:59:36 +0100 Subject: [PATCH 13/14] Override `event.target`, update types --- .../src/input-control/input-field.tsx | 17 +++++++++++++++-- packages/components/src/input-control/types.ts | 2 +- packages/components/src/unit-control/index.tsx | 6 +++++- packages/components/src/unit-control/types.ts | 2 +- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/components/src/input-control/input-field.tsx b/packages/components/src/input-control/input-field.tsx index d4c673cc9729a..999f1d3b2d6a9 100644 --- a/packages/components/src/input-control/input-field.tsx +++ b/packages/components/src/input-control/input-field.tsx @@ -93,7 +93,9 @@ function InputField( commit( valueProp, _event as SyntheticEvent ); } else if ( ! isDirty ) { onChange( value, { - event: _event as ChangeEvent< HTMLInputElement > | PointerEvent, + event: _event as + | ChangeEvent< HTMLInputElement > + | PointerEvent< HTMLInputElement >, } ); wasDirtyOnBlur.current = false; } @@ -167,7 +169,18 @@ function InputField( const dragGestureProps = useDrag< PointerEvent< HTMLInputElement > >( ( dragProps ) => { - const { distance, dragging, event } = dragProps; + const { distance, dragging, event, target } = dragProps; + + // The `target` prop always references the `input` element while, by + // default, the `dragProps.event.target` property would reference the real + // event target (i.e. any DOM element that the pointer is hovering while + // dragging). Ensuring that the `target` is always the `input` element + // allows consumers of `InputControl` (or any higher-level control) to + // check the input's validity by accessing `event.target.validity.valid`. + dragProps.event = { + ...dragProps.event, + target, + }; if ( ! distance ) return; event.stopPropagation(); diff --git a/packages/components/src/input-control/types.ts b/packages/components/src/input-control/types.ts index 25a10ca70ce8c..0bc27aa56cc35 100644 --- a/packages/components/src/input-control/types.ts +++ b/packages/components/src/input-control/types.ts @@ -34,7 +34,7 @@ interface BaseProps { } export type InputChangeCallback< - E = ChangeEvent< HTMLInputElement > | PointerEvent, + E = ChangeEvent< HTMLInputElement > | PointerEvent< HTMLInputElement >, P = {} > = ( nextValue: string | undefined, extra: { event: E } & P ) => void; diff --git a/packages/components/src/unit-control/index.tsx b/packages/components/src/unit-control/index.tsx index 1b823c566f0e3..956cbdbd087e1 100644 --- a/packages/components/src/unit-control/index.tsx +++ b/packages/components/src/unit-control/index.tsx @@ -90,7 +90,11 @@ function UnforwardedUnitControl( const handleOnQuantityChange = ( nextQuantityValue: number | string | undefined, - changeProps: { event: ChangeEvent< HTMLInputElement > | PointerEvent } + changeProps: { + event: + | ChangeEvent< HTMLInputElement > + | PointerEvent< HTMLInputElement >; + } ) => { if ( nextQuantityValue === '' || diff --git a/packages/components/src/unit-control/types.ts b/packages/components/src/unit-control/types.ts index 795aaae9e3029..46d8707b9639f 100644 --- a/packages/components/src/unit-control/types.ts +++ b/packages/components/src/unit-control/types.ts @@ -39,7 +39,7 @@ export type WPUnitControlUnit = { }; export type UnitControlOnChangeCallback = InputChangeCallback< - SyntheticEvent< HTMLSelectElement | HTMLInputElement | Element >, + SyntheticEvent< HTMLSelectElement | HTMLInputElement >, { data?: WPUnitControlUnit } >; From dad09a55c25cdd9a614619298fea70f32d65d5ab Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Tue, 15 Mar 2022 21:00:41 +0100 Subject: [PATCH 14/14] Revert docs and Storybook changes about `target.visibility` potentially not being defined --- packages/components/src/number-control/README.md | 2 +- packages/components/src/number-control/stories/index.js | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/components/src/number-control/README.md b/packages/components/src/number-control/README.md index 31a97c64031ac..21fae9ff888a2 100644 --- a/packages/components/src/number-control/README.md +++ b/packages/components/src/number-control/README.md @@ -106,7 +106,7 @@ The callback receives two arguments: 1. `newValue`: the new value of the input 2. `extra`: an object containing, under the `event` key, the original browser event. -Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props, or it could not match the `step`). In order to check the value's validity, check the `event.target?.validity.valid` property from the callback's second argument (_note: given how the component works internally, the `validity` property is not guaranteed to always be defined on `event.target`. When the `validity` property is not defined, you can assume the value received as valid_). +Note that the value received as the first argument of the callback is _not_ guaranteed to be a valid value (e.g. it could be outside of the range defined by the [`min`, `max`] props, or it could not match the `step`). In order to check the value's validity, check the `event.target?.validity.valid` property from the callback's second argument. - Type: `(newValue, extra) => void` - Required: No diff --git a/packages/components/src/number-control/stories/index.js b/packages/components/src/number-control/stories/index.js index d604be981709a..9c8496cc908c2 100644 --- a/packages/components/src/number-control/stories/index.js +++ b/packages/components/src/number-control/stories/index.js @@ -46,13 +46,7 @@ function Example() { value={ value } onChange={ ( v, extra ) => { setValue( v ); - // Given how the component works internally, the `validity` property - // is not guaranteed to always be defined on `event.target`. When the - // `validity` property is not defined, you can assume the value - // received as valid. - setIsValidValue( - extra.event.target.validity?.valid ?? true - ); + setIsValidValue( extra.event.target.validity.valid ); } } />

Is valid? { isValidValue ? 'Yes' : 'No' }