diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index a7b533663cdd5..a96b28ecff281 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Bug Fix + +- Fix `InputControl` blocking undo/redo while focused. ([#40518](https://github.com/WordPress/gutenberg/pull/40518)) + ### Enhancements - `BorderControl` now only displays the reset button in its popover when selections have already been made. ([#40917](https://github.com/WordPress/gutenberg/pull/40917)) diff --git a/packages/components/src/color-picker/hex-input.tsx b/packages/components/src/color-picker/hex-input.tsx index 27c167c315716..9331a52bf30ca 100644 --- a/packages/components/src/color-picker/hex-input.tsx +++ b/packages/components/src/color-picker/hex-input.tsx @@ -16,6 +16,7 @@ import { Spacer } from '../spacer'; import { space } from '../ui/utils/space'; import { ColorHexInputControl } from './styles'; import { COLORS } from '../utils/colors-values'; +import { useDraft } from './use-draft'; interface HexInputProps { color: Colord; @@ -24,14 +25,15 @@ interface HexInputProps { } export const HexInput = ( { color, onChange, enableAlpha }: HexInputProps ) => { - const handleChange = ( nextValue: string | undefined ) => { - if ( ! nextValue ) return; - const hexValue = nextValue.startsWith( '#' ) - ? nextValue - : '#' + nextValue; + const formattedValue = color.toHex().slice( 1 ).toUpperCase(); - onChange( colord( hexValue ) ); - }; + const draftHookProps = useDraft( { + value: formattedValue, + onChange: ( nextValue ) => { + nextValue = nextValue.replace( /^#/, '' ); + onChange( colord( '#' + nextValue ) ); + }, + } ); return ( { # } - value={ color.toHex().slice( 1 ).toUpperCase() } - onChange={ handleChange } + { ...draftHookProps } maxLength={ enableAlpha ? 9 : 7 } label={ __( 'Hex color' ) } hideLabelFromVision diff --git a/packages/components/src/color-picker/input-with-slider.tsx b/packages/components/src/color-picker/input-with-slider.tsx index fb45238ec5016..a7fc4f13adb30 100644 --- a/packages/components/src/color-picker/input-with-slider.tsx +++ b/packages/components/src/color-picker/input-with-slider.tsx @@ -7,6 +7,7 @@ import { Spacer } from '../spacer'; import { space } from '../ui/utils/space'; import { RangeControl, NumberControlWrapper } from './styles'; import { COLORS } from '../utils/colors-values'; +import { useDraft } from './use-draft'; interface InputWithSliderProps { min: number; @@ -25,6 +26,11 @@ export const InputWithSlider = ( { onChange, value, }: InputWithSliderProps ) => { + const draftHookProps = useDraft( { + value: `${ value }`, + onChange: ( nextValue ) => onChange( parseFloat( nextValue ) ), + } ); + return ( void; +}; + +type DraftState = { + value?: string; + isStale?: boolean; +}; + +export const useDraft = ( props: DraftHookProps ) => { + const refPreviousValue = useRef( props.value ); + const [ draft, setDraft ] = useState< DraftState >( {} ); + const value = draft.value !== undefined ? draft.value : props.value; + + // Determines when to discard the draft value to restore controlled status. + // To do so, it tracks the previous value and marks the draft value as stale + // after each render. + useEffect( () => { + const { current: previousValue } = refPreviousValue; + refPreviousValue.current = props.value; + if ( draft.value !== undefined && ! draft.isStale ) + setDraft( { ...draft, isStale: true } ); + else if ( draft.isStale && props.value !== previousValue ) + setDraft( {} ); + }, [ props.value, draft ] ); + + const onChange = ( nextValue: string ) => { + // Mutates the draft value to avoid an extra render and effect run. + setDraft( ( current ) => + Object.assign( current, { value: nextValue, isStale: false } ) + ); + props.onChange( nextValue ); + }; + const onBlur: FocusEventHandler = ( event ) => { + setDraft( {} ); + props.onBlur?.( event ); + }; + + return { value, onBlur, onChange }; +}; diff --git a/packages/components/src/input-control/input-field.tsx b/packages/components/src/input-control/input-field.tsx index 999f1d3b2d6a9..744f7f93c79dc 100644 --- a/packages/components/src/input-control/input-field.tsx +++ b/packages/components/src/input-control/input-field.tsx @@ -24,7 +24,6 @@ import type { WordPressComponentProps } from '../ui/context'; import { useDragCursor } from './utils'; import { Input } from './styles/input-control-styles'; import { useInputControlStateReducer } from './reducer/reducer'; -import { useUpdateEffect } from '../utils'; import type { InputFieldProps } from './types'; function InputField( @@ -67,40 +66,21 @@ function InputField( pressEnter, pressUp, reset, - } = useInputControlStateReducer( stateReducer, { - isDragEnabled, - value: valueProp, - isPressEnterToChange, - } ); + } = useInputControlStateReducer( + stateReducer, + { + isDragEnabled, + value: valueProp, + isPressEnterToChange, + }, + onChange + ); - const { _event, value, isDragging, isDirty } = state; + const { value, isDragging, isDirty } = state; const wasDirtyOnBlur = useRef( false ); const dragCursor = useDragCursor( isDragging, dragDirection ); - /* - * Handles synchronization of external and internal value state. - * If not focused and did not hold a dirty value[1] on blur - * updates the value from the props. Otherwise if not holding - * a dirty value[1] propagates the value and event through onChange. - * [1] value is only made dirty if isPressEnterToChange is true - */ - useUpdateEffect( () => { - if ( valueProp === value ) { - return; - } - if ( ! isFocused && ! wasDirtyOnBlur.current ) { - commit( valueProp, _event as SyntheticEvent ); - } else if ( ! isDirty ) { - onChange( value, { - event: _event as - | ChangeEvent< HTMLInputElement > - | PointerEvent< HTMLInputElement >, - } ); - wasDirtyOnBlur.current = false; - } - }, [ value, isDirty, isFocused, valueProp ] ); - const handleOnBlur = ( event: FocusEvent< HTMLInputElement > ) => { onBlur( event ); setIsFocused?.( false ); diff --git a/packages/components/src/input-control/reducer/actions.ts b/packages/components/src/input-control/reducer/actions.ts index d66eb5778aad8..343e652b2190c 100644 --- a/packages/components/src/input-control/reducer/actions.ts +++ b/packages/components/src/input-control/reducer/actions.ts @@ -20,7 +20,7 @@ export const PRESS_UP = 'PRESS_UP'; export const RESET = 'RESET'; interface EventPayload { - event?: SyntheticEvent; + event: SyntheticEvent; } interface Action< Type, ExtraPayload = {} > { diff --git a/packages/components/src/input-control/reducer/reducer.ts b/packages/components/src/input-control/reducer/reducer.ts index de13dd9a8f950..7521a567981c1 100644 --- a/packages/components/src/input-control/reducer/reducer.ts +++ b/packages/components/src/input-control/reducer/reducer.ts @@ -1,12 +1,12 @@ /** * External dependencies */ -import type { SyntheticEvent } from 'react'; +import type { SyntheticEvent, ChangeEvent, PointerEvent } from 'react'; /** * WordPress dependencies */ -import { useReducer } from '@wordpress/element'; +import { useReducer, useLayoutEffect, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -18,6 +18,7 @@ import { initialStateReducer, } from './state'; import * as actions from './actions'; +import type { InputChangeCallback } from '../types'; /** * Prepares initialState for the reducer. @@ -51,6 +52,14 @@ function inputControlStateReducer( composedStateReducers: StateReducer ): StateReducer { return ( state, action ) => { + // Updates state and returns early when there's no action type. These + // are controlled updates and need no exposure to additional reducers. + if ( ! ( 'type' in action ) ) { + return { + ...state, + value: `${ action.value ?? '' }`, + }; + } const nextState = { ...state }; switch ( action.type ) { @@ -97,7 +106,7 @@ function inputControlStateReducer( case actions.RESET: nextState.error = null; nextState.isDirty = false; - nextState.value = action.payload.value || state.initialValue; + nextState.value = action.payload.value ?? state.initialValue; break; /** @@ -108,10 +117,6 @@ function inputControlStateReducer( break; } - if ( action.payload.event ) { - nextState._event = action.payload.event; - } - /** * Send the nextState + action to the composedReducers via * this "bridge" mechanism. This allows external stateReducers @@ -131,13 +136,15 @@ function inputControlStateReducer( * This technique uses the "stateReducer" design pattern: * https://kentcdodds.com/blog/the-state-reducer-pattern/ * - * @param stateReducer An external state reducer. - * @param initialState The initial state for the reducer. + * @param stateReducer An external state reducer. + * @param initialState The initial state for the reducer. + * @param onChangeHandler A handler for the onChange event. * @return State, dispatch, and a collection of actions. */ export function useInputControlStateReducer( stateReducer: StateReducer = initialStateReducer, - initialState: Partial< InputState > = initialInputControlState + initialState: Partial< InputState > = initialInputControlState, + onChangeHandler: InputChangeCallback ) { const [ state, dispatch ] = useReducer< StateReducer >( inputControlStateReducer( stateReducer ), @@ -148,15 +155,7 @@ export function useInputControlStateReducer( nextValue: actions.ChangeEventAction[ 'payload' ][ 'value' ], event: actions.ChangeEventAction[ 'payload' ][ 'event' ] ) => { - /** - * Persist allows for the (Synthetic) event to be used outside of - * this function call. - * https://reactjs.org/docs/events.html#event-pooling - */ - if ( event && event.persist ) { - event.persist(); - } - + refEvent.current = event; dispatch( { type, payload: { value: nextValue, event }, @@ -166,21 +165,14 @@ export function useInputControlStateReducer( const createKeyEvent = ( type: actions.KeyEventAction[ 'type' ] ) => ( event: actions.KeyEventAction[ 'payload' ][ 'event' ] ) => { - /** - * Persist allows for the (Synthetic) event to be used outside of - * this function call. - * https://reactjs.org/docs/events.html#event-pooling - */ - if ( event && event.persist ) { - event.persist(); - } - + refEvent.current = event; dispatch( { type, payload: { event } } ); }; const createDragEvent = ( type: actions.DragEventAction[ 'type' ] ) => ( payload: actions.DragEventAction[ 'payload' ] ) => { + refEvent.current = payload.event; dispatch( { type, payload } ); }; @@ -188,8 +180,10 @@ export function useInputControlStateReducer( * Actions for the reducer */ const change = createChangeEvent( actions.CHANGE ); - const invalidate = ( error: unknown, event: SyntheticEvent ) => + const invalidate = ( error: unknown, event: SyntheticEvent ) => { + refEvent.current = event; dispatch( { type: actions.INVALIDATE, payload: { error, event } } ); + }; const reset = createChangeEvent( actions.RESET ); const commit = createChangeEvent( actions.COMMIT ); @@ -201,6 +195,36 @@ export function useInputControlStateReducer( const pressDown = createKeyEvent( actions.PRESS_DOWN ); const pressEnter = createKeyEvent( actions.PRESS_ENTER ); + const currentState = useRef( state ); + const currentValueProp = useRef( initialState.value ); + const refEvent = useRef< SyntheticEvent | null >( null ); + useLayoutEffect( () => { + currentState.current = state; + currentValueProp.current = initialState.value; + } ); + useLayoutEffect( () => { + if ( + refEvent.current && + state.value !== currentValueProp.current && + ! currentState.current.isDirty + ) { + onChangeHandler( state.value ?? '', { + event: refEvent.current as + | ChangeEvent< HTMLInputElement > + | PointerEvent< HTMLInputElement >, + } ); + } + }, [ state.value ] ); + useLayoutEffect( () => { + if ( + initialState.value !== currentState.current.value && + ! currentState.current.isDirty + ) { + dispatch( { value: initialState.value } ); + } + if ( refEvent.current ) refEvent.current = null; + }, [ initialState.value ] ); + return { change, commit, diff --git a/packages/components/src/input-control/reducer/state.ts b/packages/components/src/input-control/reducer/state.ts index be7dd3547300b..c7201d12a9df7 100644 --- a/packages/components/src/input-control/reducer/state.ts +++ b/packages/components/src/input-control/reducer/state.ts @@ -9,22 +9,24 @@ import type { Reducer } from 'react'; import type { InputAction } from './actions'; export interface InputState { - _event: Event | {}; error: unknown; - initialValue?: string; + initialValue: string; isDirty: boolean; isDragEnabled: boolean; isDragging: boolean; isPressEnterToChange: boolean; - value?: string; + value: string; } -export type StateReducer = Reducer< InputState, InputAction >; +export type StateReducer = Reducer< + InputState, + InputAction | Partial< InputState > +>; +export type SecondaryReducer = Reducer< InputState, InputAction >; export const initialStateReducer: StateReducer = ( state: InputState ) => state; export const initialInputControlState: InputState = { - _event: {}, error: null, initialValue: '', isDirty: false, diff --git a/packages/components/src/input-control/types.ts b/packages/components/src/input-control/types.ts index bdf0c33dfadf7..63ad4bb0c2d8a 100644 --- a/packages/components/src/input-control/types.ts +++ b/packages/components/src/input-control/types.ts @@ -65,7 +65,7 @@ interface BaseProps { export type InputChangeCallback< E = ChangeEvent< HTMLInputElement > | PointerEvent< HTMLInputElement >, P = {} -> = ( nextValue: string | undefined, extra: { event: E } & P ) => void; +> = ( nextValue: string, extra: { event: E } & P ) => void; export interface InputFieldProps extends BaseProps { /** diff --git a/packages/components/src/range-control/index.js b/packages/components/src/range-control/index.js index fb64ec8ea9b6c..606d53895f37d 100644 --- a/packages/components/src/range-control/index.js +++ b/packages/components/src/range-control/index.js @@ -18,8 +18,8 @@ import { useInstanceId } from '@wordpress/compose'; import BaseControl from '../base-control'; import Button from '../button'; import Icon from '../icon'; -import { COLORS } from '../utils'; -import { floatClamp, useControlledRangeValue } from './utils'; +import { COLORS, useControlledState } from '../utils'; +import { useUnimpededRangedNumberEntry } from './utils'; import InputRange from './input-range'; import RangeRail from './rail'; import SimpleTooltip from './tooltip'; @@ -70,13 +70,10 @@ function RangeControl( }, ref ) { - const [ value, setValue ] = useControlledRangeValue( { - min, - max, - value: valueProp, - initial: initialPosition, - } ); const isResetPendent = useRef( false ); + const [ value, setValue ] = useControlledState( valueProp, { + fallback: null, + } ); if ( step === 'any' ) { // The tooltip and number input field are hidden when the step is "any" @@ -102,15 +99,15 @@ function RangeControl( const isThumbFocused = ! disabled && isFocused; const isValueReset = value === null; - const currentValue = value !== undefined ? value : currentInput; - - const inputSliderValue = isValueReset ? '' : currentValue; + const usedValue = isValueReset + ? resetFallbackValue ?? initialPosition + : value ?? currentInput; - const rangeFillValue = isValueReset ? ( max - min ) / 2 + min : value; - - const calculatedFillValue = ( ( value - min ) / ( max - min ) ) * 100; - const fillValue = isValueReset ? 50 : calculatedFillValue; - const fillValueOffset = `${ clamp( fillValue, 0, 100 ) }%`; + const fillPercent = `${ + usedValue === null || usedValue === undefined + ? 50 + : ( ( clamp( usedValue, min, max ) - min ) / ( max - min ) ) * 100 + }%`; const classes = classnames( 'components-range-control', className ); @@ -129,56 +126,42 @@ function RangeControl( onChange( nextValue ); }; - const handleOnChange = ( nextValue ) => { - nextValue = parseFloat( nextValue ); - setValue( nextValue ); - /* - * Calls onChange only when nextValue is numeric - * otherwise may queue a reset for the blur event. - */ - if ( ! isNaN( nextValue ) ) { - if ( nextValue < min || nextValue > max ) { - nextValue = floatClamp( nextValue, min, max ); + const someNumberInputProps = useUnimpededRangedNumberEntry( { + max, + min, + value: usedValue ?? '', + onChange: ( nextValue ) => { + if ( ! isNaN( nextValue ) ) { + setValue( nextValue ); + onChange( nextValue ); + isResetPendent.current = false; + } else if ( allowReset ) { + isResetPendent.current = true; } - onChange( nextValue ); - isResetPendent.current = false; - } else if ( allowReset ) { - isResetPendent.current = true; - } - }; - - const handleOnInputNumberBlur = () => { - if ( isResetPendent.current ) { - handleOnReset(); - isResetPendent.current = false; - } - }; + }, + onBlur: () => { + if ( isResetPendent.current ) { + handleOnReset(); + isResetPendent.current = false; + } + }, + } ); const handleOnReset = () => { - let resetValue = parseFloat( resetFallbackValue ); - let onChangeResetValue = resetValue; + const resetValue = parseFloat( resetFallbackValue ); if ( isNaN( resetValue ) ) { - resetValue = null; - onChangeResetValue = undefined; + setValue( null ); + /* + * If the value is reset without a resetFallbackValue, the onChange + * callback receives undefined as that was the behavior when the + * component was stablized. + */ + onChange( undefined ); + } else { + setValue( resetValue ); + onChange( resetValue ); } - - setValue( resetValue ); - - /** - * Previously, this callback would always receive undefined as - * an argument. This behavior is unexpected, specifically - * when resetFallbackValue is defined. - * - * The value of undefined is not ideal. Passing it through - * to internal elements would change it from a - * controlled component to an uncontrolled component. - * - * For now, to minimize unexpected regressions, we're going to - * preserve the undefined callback argument, except when a - * resetFallbackValue is defined. - */ - onChange( onChangeResetValue ); }; const handleShowTooltip = () => setShowTooltip( true ); @@ -197,7 +180,7 @@ function RangeControl( }; const offsetStyle = { - [ isRTL() ? 'right' : 'left' ]: fillValueOffset, + [ isRTL() ? 'right' : 'left' ]: fillPercent, }; return ( @@ -235,7 +218,7 @@ function RangeControl( onMouseLeave={ onMouseLeave } ref={ setRef } step={ step } - value={ inputSliderValue } + value={ usedValue ?? '' } /> @@ -285,13 +268,9 @@ function RangeControl( disabled={ disabled } inputMode="decimal" isShiftStepEnabled={ isShiftStepEnabled } - max={ max } - min={ min } - onBlur={ handleOnInputNumberBlur } - onChange={ handleOnChange } shiftStep={ shiftStep } step={ step } - value={ inputSliderValue } + { ...someNumberInputProps } /> ) } { allowReset && ( diff --git a/packages/components/src/range-control/rail.js b/packages/components/src/range-control/rail.js index 34caa88a73622..39496bfc989f4 100644 --- a/packages/components/src/range-control/rail.js +++ b/packages/components/src/range-control/rail.js @@ -16,7 +16,7 @@ export default function RangeRail( { min = 0, max = 100, step = 1, - value = 0, + value, ...restProps } ) { return ( @@ -29,7 +29,7 @@ export default function RangeRail( { min={ min } max={ max } step={ step } - value={ value } + value={ value ?? ( max - min ) / 2 + min } /> ) } diff --git a/packages/components/src/range-control/test/index.js b/packages/components/src/range-control/test/index.js index c60f12b3b7072..5c1c41e48521b 100644 --- a/packages/components/src/range-control/test/index.js +++ b/packages/components/src/range-control/test/index.js @@ -1,7 +1,12 @@ /** * External dependencies */ -import { fireEvent, render } from '@testing-library/react'; +import { fireEvent, render, waitFor } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { useState } from '@wordpress/element'; /** * Internal dependencies @@ -15,14 +20,26 @@ const getNumberInput = ( container ) => const getResetButton = ( container ) => container.querySelector( '.components-range-control__reset' ); -describe( 'RangeControl', () => { +function ControlledRangeControl( props ) { + const [ value, setValue ] = useState( props.value ); + const onChange = ( v ) => { + setValue( v ); + props.onChange?.( v ); + }; + return ; +} + +describe.each( [ + [ 'uncontrolled', RangeControl ], + [ 'controlled', ControlledRangeControl ], +] )( 'RangeControl %s', ( ...modeAndComponent ) => { + const [ mode, Component ] = modeAndComponent; + describe( '#render()', () => { it( 'should trigger change callback with numeric value', () => { const onChange = jest.fn(); - const { container } = render( - - ); + const { container } = render( ); const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); @@ -39,10 +56,7 @@ describe( 'RangeControl', () => { it( 'should render with icons', () => { const { container } = render( - + ); const beforeIcon = container.querySelector( @@ -59,7 +73,7 @@ describe( 'RangeControl', () => { describe( 'validation', () => { it( 'should not apply if new value is lower than minimum', () => { - const { container } = render( ); + const { container } = render( ); const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); @@ -71,7 +85,7 @@ describe( 'RangeControl', () => { } ); it( 'should not apply if new value is greater than maximum', () => { - const { container } = render( ); + const { container } = render( ); const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); @@ -85,7 +99,7 @@ describe( 'RangeControl', () => { it( 'should not call onChange if new value is invalid', () => { const onChange = jest.fn(); const { container } = render( - + ); const numberInput = getNumberInput( container ); @@ -96,10 +110,10 @@ describe( 'RangeControl', () => { expect( onChange ).not.toHaveBeenCalled(); } ); - it( 'should keep invalid values in number input until loss of focus', () => { + it( 'should keep invalid values in number input until loss of focus', async () => { const onChange = jest.fn(); const { container } = render( - + ); const rangeInput = getRangeInput( container ); @@ -108,7 +122,7 @@ describe( 'RangeControl', () => { numberInput.focus(); fireEvent.change( numberInput, { target: { value: '-1.1' } } ); - expect( numberInput.value ).toBe( '-1.1' ); + await waitFor( () => expect( numberInput.value ).toBe( '-1.1' ) ); expect( rangeInput.value ).toBe( '-1' ); fireEvent.blur( numberInput ); @@ -118,7 +132,7 @@ describe( 'RangeControl', () => { it( 'should validate when provided a max or min of zero', () => { const { container } = render( - + ); const rangeInput = getRangeInput( container ); @@ -133,7 +147,7 @@ describe( 'RangeControl', () => { it( 'should validate when min and max are negative', () => { const { container } = render( - + ); const rangeInput = getRangeInput( container ); @@ -154,11 +168,7 @@ describe( 'RangeControl', () => { it( 'should take into account the step starting from min', () => { const onChange = jest.fn(); const { container } = render( - + ); const rangeInput = getRangeInput( container ); @@ -179,9 +189,7 @@ describe( 'RangeControl', () => { describe( 'initialPosition / value', () => { it( 'should render initial rendered value of 50% of min/max, if no initialPosition or value is defined', () => { - const { container } = render( - - ); + const { container } = render( ); const rangeInput = getRangeInput( container ); @@ -190,7 +198,7 @@ describe( 'RangeControl', () => { it( 'should render initialPosition if no value is provided', () => { const { container } = render( - + ); const rangeInput = getRangeInput( container ); @@ -200,7 +208,7 @@ describe( 'RangeControl', () => { it( 'should render value instead of initialPosition is provided', () => { const { container } = render( - + ); const rangeInput = getRangeInput( container ); @@ -211,7 +219,7 @@ describe( 'RangeControl', () => { describe( 'input field', () => { it( 'should render an input field by default', () => { - const { container } = render( ); + const { container } = render( ); const numberInput = getNumberInput( container ); @@ -220,7 +228,7 @@ describe( 'RangeControl', () => { it( 'should not render an input field, if disabled', () => { const { container } = render( - + ); const numberInput = getNumberInput( container ); @@ -229,7 +237,7 @@ describe( 'RangeControl', () => { } ); it( 'should render a zero value into input range and field', () => { - const { container } = render( ); + const { container } = render( ); const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); @@ -239,7 +247,7 @@ describe( 'RangeControl', () => { } ); it( 'should update both field and range on change', () => { - const { container } = render( ); + const { container } = render( ); const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); @@ -258,7 +266,7 @@ describe( 'RangeControl', () => { } ); it( 'should reset input values if next value is removed', () => { - const { container } = render( ); + const { container } = render( ); const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); @@ -274,14 +282,31 @@ describe( 'RangeControl', () => { } ); describe( 'reset', () => { - it( 'should reset to a custom fallback value, defined by a parent component', () => { + it.concurrent.each( [ + [ + 'initialPosition if it is defined', + { initialPosition: 21 }, + [ '21', undefined ], + ], + [ + 'resetFallbackValue if it is defined', + { resetFallbackValue: 34 }, + [ '34', 34 ], + ], + [ + 'resetFallbackValue if both it and initialPosition are defined', + { initialPosition: 21, resetFallbackValue: 34 }, + [ '34', 34 ], + ], + ] )( 'should reset to %s', ( ...all ) => { + const [ , propsForReset, [ expectedValue, expectedChange ] ] = all; const spy = jest.fn(); const { container } = render( - ); @@ -291,19 +316,20 @@ describe( 'RangeControl', () => { fireEvent.click( resetButton ); - expect( rangeInput.value ).toBe( '33' ); - expect( numberInput.value ).toBe( '33' ); - expect( spy ).toHaveBeenCalledWith( 33 ); + expect( rangeInput.value ).toBe( expectedValue ); + expect( numberInput.value ).toBe( expectedValue ); + expect( spy ).toHaveBeenCalledWith( expectedChange ); } ); it( 'should reset to a 50% of min/max value, of no initialPosition or value is defined', () => { const { container } = render( - ); diff --git a/packages/components/src/range-control/utils.js b/packages/components/src/range-control/utils.js index e2a758a6b4f1c..aaecb83e71135 100644 --- a/packages/components/src/range-control/utils.js +++ b/packages/components/src/range-control/utils.js @@ -2,7 +2,7 @@ /** * External dependencies */ -import { clamp, noop } from 'lodash'; +import { noop } from 'lodash'; /** * WordPress dependencies @@ -10,61 +10,63 @@ import { clamp, noop } from 'lodash'; import { useCallback, useRef, useEffect, useState } from '@wordpress/element'; /** - * Internal dependencies - */ -import { useControlledState } from '../utils/hooks'; - -/** - * A float supported clamp function for a specific value. + * Enables entry of out-of-range and invalid values that diverge from state. * - * @param {number|null} value The value to clamp. - * @param {number} min The minimum value. - * @param {number} max The maximum value. + * @param {Object} props Props + * @param {number|null} props.value Incoming value. + * @param {number} props.max Maximum valid value. + * @param {number} props.min Minimum valid value. + * @param {(event: Event) => void} props.onBlur Callback for blur events. + * @param {(next: number) => void} props.onChange Callback for changes. * - * @return {number} A (float) number + * @return {Object} Assorted props for the input. */ -export function floatClamp( value, min, max ) { - if ( typeof value !== 'number' ) { - return null; - } - - return parseFloat( clamp( value, min, max ) ); -} - -/** - * Hook to store a clamped value, derived from props. - * - * @param {Object} settings Hook settings. - * @param {number} settings.min The minimum value. - * @param {number} settings.max The maximum value. - * @param {number} settings.value The current value. - * @param {any} settings.initial The initial value. - * - * @return {[*, Function]} The controlled value and the value setter. - */ -export function useControlledRangeValue( { - min, +export function useUnimpededRangedNumberEntry( { max, - value: valueProp, - initial, + min, + onBlur, + onChange, + value, } ) { - const [ state, setInternalState ] = useControlledState( - floatClamp( valueProp, min, max ), - { initial, fallback: null } - ); - - const setState = useCallback( - ( nextValue ) => { - if ( nextValue === null ) { - setInternalState( null ); - } else { - setInternalState( floatClamp( nextValue, min, max ) ); - } - }, - [ min, max ] - ); + const ref = useRef(); + const isDiverging = useRef( false ); + /** @type {import('../input-control/types').InputChangeCallback}*/ + const changeHandler = ( next ) => { + next = parseFloat( next ); + if ( next < min || next > max ) { + isDiverging.current = true; + next = Math.max( min, Math.min( max, next ) ); + } + onChange( next ); + }; + const blurHandler = ( event ) => { + isDiverging.current = false; + onBlur?.( event ); + }; + // When the value entered in the input is out of range then a clamped value + // is sent through onChange and that goes on to update the input. In such + // circumstances this effect overwrites the input value with the entered + // value to avoid interfering with typing. E.g. Without this effect, if + // `min` is 20 and the user intends to type 25, as soon as 2 is typed the + // input will update to 20 and likely lead to an entry of 205. + useEffect( () => { + if ( ref.current && isDiverging.current ) { + const input = ref.current; + const entry = input.value; + const { defaultView } = input.ownerDocument; + defaultView.requestAnimationFrame( () => ( input.value = entry ) ); + isDiverging.current = false; + } + }, [ value ] ); - return [ state, setState ]; + return { + max, + min, + ref, + value, + onBlur: blurHandler, + onChange: changeHandler, + }; } /** diff --git a/packages/components/src/unit-control/index.tsx b/packages/components/src/unit-control/index.tsx index 440dc11b94e8b..6754ce721dd2d 100644 --- a/packages/components/src/unit-control/index.tsx +++ b/packages/components/src/unit-control/index.tsx @@ -34,7 +34,7 @@ import { } from './utils'; import { useControlledState } from '../utils/hooks'; import type { UnitControlProps, UnitControlOnChangeCallback } from './types'; -import type { StateReducer } from '../input-control/reducer/state'; +import type { SecondaryReducer } from '../input-control/reducer/state'; function UnforwardedUnitControl( unitControlProps: WordPressComponentProps< @@ -179,7 +179,7 @@ function UnforwardedUnitControl( const changeProps = { event, data }; // The `onChange` callback already gets called, no need to call it explicitely. - onUnitChange?.( validParsedUnit, changeProps ); + onUnitChange?.( validParsedUnit ?? '', changeProps ); setUnit( validParsedUnit ); } @@ -206,7 +206,7 @@ function UnforwardedUnitControl( * @param action Action triggering state change * @return The updated state to apply to InputControl */ - const unitControlStateReducer: StateReducer = ( state, action ) => { + const unitControlStateReducer: SecondaryReducer = ( state, action ) => { const nextState = { ...state }; /* @@ -226,7 +226,7 @@ function UnforwardedUnitControl( return nextState; }; - let stateReducer: StateReducer = unitControlStateReducer; + let stateReducer: SecondaryReducer = unitControlStateReducer; if ( stateReducerProp ) { stateReducer = ( state, action ) => { const baseState = unitControlStateReducer( state, action );