diff --git a/packages/components/src/angle-picker-control/angle-circle.js b/packages/components/src/angle-picker-control/angle-circle.js index f2a761284d96e2..6122cda215daf1 100644 --- a/packages/components/src/angle-picker-control/angle-circle.js +++ b/packages/components/src/angle-picker-control/angle-circle.js @@ -31,8 +31,7 @@ function AngleCircle( { value, onChange, ...props } ) { // Prevent (drag) mouse events from selecting and accidentally // triggering actions from other elements. event.preventDefault(); - // Ensure the input isn't focused as preventDefault would leave it - document.activeElement.blur(); + onChange( getAngle( centerX, centerY, event.clientX, event.clientY ) ); }; diff --git a/packages/components/src/box-control/test/index.js b/packages/components/src/box-control/test/index.js index 81b1ab7bd2bf68..613aac6092fe65 100644 --- a/packages/components/src/box-control/test/index.js +++ b/packages/components/src/box-control/test/index.js @@ -28,7 +28,6 @@ describe( 'BoxControl', () => { const input = container.querySelector( 'input' ); const unitSelect = container.querySelector( 'select' ); - input.focus(); fireEvent.change( input, { target: { value: '100%' } } ); fireEvent.keyDown( input, { keyCode: ENTER } ); @@ -42,17 +41,14 @@ describe( 'BoxControl', () => { const { container, getByText } = render( ); const input = container.querySelector( 'input' ); const unitSelect = container.querySelector( 'select' ); - const reset = getByText( /Reset/ ); - input.focus(); fireEvent.change( input, { target: { value: '100px' } } ); fireEvent.keyDown( input, { keyCode: ENTER } ); expect( input.value ).toBe( '100' ); expect( unitSelect.value ).toBe( 'px' ); - reset.focus(); - fireEvent.click( reset ); + fireEvent.click( getByText( /Reset/ ) ); expect( input.value ).toBe( '' ); expect( unitSelect.value ).toBe( 'px' ); @@ -72,17 +68,14 @@ describe( 'BoxControl', () => { const { container, getByText } = render( ); const input = container.querySelector( 'input' ); const unitSelect = container.querySelector( 'select' ); - const reset = getByText( /Reset/ ); - input.focus(); fireEvent.change( input, { target: { value: '100px' } } ); fireEvent.keyDown( input, { keyCode: ENTER } ); expect( input.value ).toBe( '100' ); expect( unitSelect.value ).toBe( 'px' ); - reset.focus(); - fireEvent.click( reset ); + fireEvent.click( getByText( /Reset/ ) ); expect( input.value ).toBe( '' ); expect( unitSelect.value ).toBe( 'px' ); @@ -109,17 +102,14 @@ describe( 'BoxControl', () => { const { container, getByText } = render( ); const input = container.querySelector( 'input' ); const unitSelect = container.querySelector( 'select' ); - const reset = getByText( /Reset/ ); - input.focus(); fireEvent.change( input, { target: { value: '100px' } } ); fireEvent.keyDown( input, { keyCode: ENTER } ); expect( input.value ).toBe( '100' ); expect( unitSelect.value ).toBe( 'px' ); - reset.focus(); - fireEvent.click( reset ); + fireEvent.click( getByText( /Reset/ ) ); expect( input.value ).toBe( '' ); expect( unitSelect.value ).toBe( 'px' ); diff --git a/packages/components/src/input-control/index.js b/packages/components/src/input-control/index.js index 56eb23d950f704..86d58484d9abfd 100644 --- a/packages/components/src/input-control/index.js +++ b/packages/components/src/input-control/index.js @@ -34,7 +34,9 @@ export function InputControl( isPressEnterToChange = false, label, labelPosition = 'top', + onBlur = noop, onChange = noop, + onFocus = noop, onValidate = noop, onKeyDown = noop, prefix, @@ -50,6 +52,16 @@ export function InputControl( const id = useUniqueId( idProp ); const classes = classNames( 'components-input-control', className ); + const handleOnBlur = ( event ) => { + onBlur( event ); + setIsFocused( false ); + }; + + const handleOnFocus = ( event ) => { + onFocus( event ); + setIsFocused( true ); + }; + return ( state, value: valueProp, - type, ...props }, ref @@ -67,27 +63,35 @@ function InputField( const { _event, value, isDragging, isDirty } = state; + const valueRef = useRef( value ); const dragCursor = useDragCursor( isDragging, dragDirection ); - /* - * Syncs value state using the focus state to determine the direction. - * Without focus it updates the value from the props. With focus it - * propagates the value and event through onChange. - */ - useUpdateEffect( () => { - if ( valueProp === value ) { + useEffect( () => { + /** + * Handles syncing incoming value changes with internal state. + * This effectively enables a "controlled" state. + * https://reactjs.org/docs/forms.html#controlled-components + */ + if ( valueProp !== valueRef.current ) { + update( valueProp ); + valueRef.current = valueProp; + + // Quick return to avoid firing the onChange callback return; } - if ( ! isFocused ) { - update( valueProp ); - } else if ( ! isDirty ) { + + /** + * Fires the onChange callback when internal state value changes. + */ + if ( value !== valueRef.current && ! isDirty ) { onChange( value, { event: _event } ); + + valueRef.current = value; } - }, [ value, isDirty, isFocused, valueProp ] ); + }, [ value, isDirty, valueProp ] ); const handleOnBlur = ( event ) => { onBlur( event ); - setIsFocused( false ); /** * If isPressEnterToChange is set, this commits the value to @@ -102,22 +106,8 @@ function InputField( } }; - /* - * Works around the odd UA (e.g. Firefox) that does not focus inputs of - * type=number when their spinner arrows are pressed. - */ - let handleOnMouseDown; - if ( type === 'number' ) { - handleOnMouseDown = ( event ) => { - if ( event.target !== event.target.ownerDocument.activeElement ) { - event.target.focus(); - } - }; - } - const handleOnFocus = ( event ) => { onFocus( event ); - setIsFocused( true ); }; const handleOnChange = ( event ) => { @@ -205,11 +195,9 @@ function InputField( onChange={ handleOnChange } onFocus={ handleOnFocus } onKeyDown={ handleOnKeyDown } - onMouseDown={ handleOnMouseDown } ref={ ref } size={ size } value={ value } - type={ type } /> ); } diff --git a/packages/components/src/input-control/test/index.js b/packages/components/src/input-control/test/index.js index 4198a2308fd473..86615318497fc6 100644 --- a/packages/components/src/input-control/test/index.js +++ b/packages/components/src/input-control/test/index.js @@ -49,7 +49,7 @@ describe( 'InputControl', () => { render( ); const input = getInput(); - input.focus(); + fireEvent.change( input, { target: { value: 'There' } } ); expect( input.value ).toBe( 'There' ); @@ -59,23 +59,21 @@ describe( 'InputControl', () => { it( 'should work as a controlled component', () => { const spy = jest.fn(); const { rerender } = render( - + ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: 'two' } } ); + fireEvent.change( input, { target: { value: 'State' } } ); - // Ensuring is controlled - fireEvent.blur( input ); + // Assuming is controlled... // Updating the value - rerender( ); + rerender( ); - expect( input.value ).toBe( 'three' ); + expect( input.value ).toBe( 'New' ); - /* + /** * onChange called only once. onChange is not called when a * parent component explicitly passed a (new value) change down to * the . @@ -91,7 +89,7 @@ describe( 'InputControl', () => { const input = getInput(); - // Assuming is controlled (not focused) + // Assuming is controlled... // Updating the value rerender( ); diff --git a/packages/components/src/number-control/test/index.js b/packages/components/src/number-control/test/index.js index 883b7f6d9591fd..8c006647bd2ff3 100644 --- a/packages/components/src/number-control/test/index.js +++ b/packages/components/src/number-control/test/index.js @@ -1,7 +1,8 @@ /** * External dependencies */ -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, unmountComponentAtNode } from 'react-dom'; +import { act, Simulate } from 'react-dom/test-utils'; /** * WordPress dependencies @@ -12,16 +13,24 @@ import { UP, DOWN, ENTER } from '@wordpress/keycodes'; /** * Internal dependencies */ -import BaseNumberControl from '../'; +import NumberControl from '../'; -const getInput = () => screen.getByTestId( 'input' ); +let container = null; -const fireKeyDown = ( data ) => - fireEvent.keyDown( document.activeElement || document.body, data ); +function getInput() { + return container.querySelector( 'input' ); +} + +beforeEach( () => { + container = document.createElement( 'div' ); + document.body.appendChild( container ); +} ); -const NumberControl = ( props ) => ( - -); +afterEach( () => { + unmountComponentAtNode( container ); + container.remove(); + container = null; +} ); function StatefulNumberControl( props ) { const [ value, setValue ] = useState( props.value ); @@ -39,56 +48,86 @@ function StatefulNumberControl( props ) { describe( 'NumberControl', () => { describe( 'Basic rendering', () => { it( 'should render', () => { - render( ); - expect( getInput() ).not.toBeNull(); + act( () => { + render( , container ); + } ); + + const input = getInput(); + + expect( input ).not.toBeNull(); } ); it( 'should render custom className', () => { - render( ); - expect( getInput() ).toBeTruthy(); + act( () => { + render( , container ); + } ); + + const input = container.querySelector( '.hello' ); + + expect( input ).toBeTruthy(); } ); } ); describe( 'onChange handling', () => { it( 'should provide onChange callback with number value', () => { const spy = jest.fn(); - - render( - spy( v ) } /> - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: 10 } } ); - expect( spy ).toHaveBeenCalledWith( '10' ); + input.value = 10; + + act( () => { + Simulate.change( input ); + } ); + + const changeValue = spy.mock.calls[ 0 ][ 0 ]; + + expect( changeValue ).toBe( '10' ); } ); } ); describe( 'Validation', () => { it( 'should clamp value within range on ENTER keypress', () => { - render( ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: -100 } } ); - fireKeyDown( { keyCode: ENTER } ); + input.value = -100; + + act( () => { + Simulate.change( input ); + Simulate.keyDown( input, { keyCode: ENTER } ); + } ); /** * This is zero because the value has been adjusted to * respect the min/max range of the input. */ - expect( input.value ).toBe( '0' ); } ); it( 'should parse to number value on ENTER keypress', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: '10 abc' } } ); - fireKeyDown( { keyCode: ENTER } ); + input.value = '10 abc'; + + act( () => { + Simulate.change( input ); + Simulate.keyDown( input, { keyCode: ENTER } ); + } ); expect( input.value ).toBe( '0' ); } ); @@ -97,83 +136,122 @@ describe( 'NumberControl', () => { describe( 'Key UP interactions', () => { it( 'should fire onKeyDown callback', () => { const spy = jest.fn(); + act( () => { + render( + , + container + ); + } ); - render( ); + const input = getInput(); - getInput().focus(); - fireKeyDown( { keyCode: UP } ); + act( () => { + Simulate.keyDown( input, { keyCode: UP } ); + } ); expect( spy ).toHaveBeenCalled(); } ); it( 'should increment by step on key UP press', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: UP } ); + + act( () => { + Simulate.keyDown( input, { keyCode: UP } ); + } ); expect( input.value ).toBe( '6' ); } ); it( 'should increment from a negative value', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: UP } ); + + act( () => { + Simulate.keyDown( input, { keyCode: UP } ); + } ); expect( input.value ).toBe( '-4' ); } ); it( 'should increment by shiftStep on key UP + shift press', () => { - render( ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: UP, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: UP, shiftKey: true } ); + } ); expect( input.value ).toBe( '20' ); } ); it( 'should increment by custom shiftStep on key UP + shift press', () => { - render( ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: UP, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: UP, shiftKey: true } ); + } ); expect( input.value ).toBe( '100' ); } ); it( 'should increment but be limited by max on shiftStep', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: UP, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: UP, shiftKey: true } ); + } ); expect( input.value ).toBe( '99' ); } ); it( 'should not increment by shiftStep if disabled', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: UP, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: UP, shiftKey: true } ); + } ); expect( input.value ).toBe( '6' ); } ); @@ -182,82 +260,119 @@ describe( 'NumberControl', () => { describe( 'Key DOWN interactions', () => { it( 'should fire onKeyDown callback', () => { const spy = jest.fn(); - render( ); + act( () => { + render( + , + container + ); + } ); - getInput().focus(); - fireKeyDown( { keyCode: DOWN } ); + const input = getInput(); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN } ); + } ); expect( spy ).toHaveBeenCalled(); } ); it( 'should decrement by step on key DOWN press', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: DOWN } ); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN } ); + } ); expect( input.value ).toBe( '4' ); } ); it( 'should decrement from a negative value', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: DOWN } ); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN } ); + } ); expect( input.value ).toBe( '-6' ); } ); it( 'should decrement by shiftStep on key DOWN + shift press', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: DOWN, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN, shiftKey: true } ); + } ); expect( input.value ).toBe( '0' ); } ); it( 'should decrement by custom shiftStep on key DOWN + shift press', () => { - render( ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: DOWN, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN, shiftKey: true } ); + } ); expect( input.value ).toBe( '-100' ); } ); it( 'should decrement but be limited by min on shiftStep', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: DOWN, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN, shiftKey: true } ); + } ); expect( input.value ).toBe( '4' ); } ); it( 'should not decrement by shiftStep if disabled', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireKeyDown( { keyCode: DOWN, shiftKey: true } ); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN, shiftKey: true } ); + } ); expect( input.value ).toBe( '4' ); } ); diff --git a/packages/components/src/range-control/README.md b/packages/components/src/range-control/README.md index f5e38538369823..3f37df519471ba 100644 --- a/packages/components/src/range-control/README.md +++ b/packages/components/src/range-control/README.md @@ -213,7 +213,8 @@ const MyRangeControl() { #### onChange -A function that receives the new value. The value will be less than `max` and more than `min` unless a reset (enabled by `allowReset`) has occured. In which case the value will be either that of `resetFallbackValue` if it has been specified or otherwise `undefined`. +A function that receives the new value. +If allowReset is true, when onChange is called without any parameter passed it should reset the value. - Type: `function` - Required: Yes @@ -221,20 +222,18 @@ A function that receives the new value. The value will be less than `max` and mo #### min -The minimum `value` allowed. +The minimum value accepted. If smaller values are inserted onChange will not be called and the value gets reverted when blur event fires. - Type: `Number` - Required: No -- Default: 0 - Platform: Web | Mobile #### max -The maximum `value` allowed. +The maximum value accepted. If higher values are inserted onChange will not be called and the value gets reverted when blur event fires. - Type: `Number` - Required: No -- Default: 100 - Platform: Web | Mobile #### railColor diff --git a/packages/components/src/range-control/index.js b/packages/components/src/range-control/index.js index 90f55c3c0cd555..7b85c0f4e66184 100644 --- a/packages/components/src/range-control/index.js +++ b/packages/components/src/range-control/index.js @@ -26,13 +26,13 @@ import { ActionRightWrapper, AfterIconWrapper, BeforeIconWrapper, - InputNumber, Root, Track, ThumbWrapper, Thumb, Wrapper, } from './styles/range-control-styles'; +import InputField from './input-field'; import { useRTL } from '../utils/rtl'; function RangeControl( @@ -77,7 +77,6 @@ function RangeControl( value: valueProp, initial: initialPosition, } ); - const isResetPendent = useRef( false ); const [ showTooltip, setShowTooltip ] = useState( showTooltipProp ); const [ isFocused, setIsFocused ] = useState( false ); @@ -120,33 +119,17 @@ function RangeControl( const handleOnRangeChange = ( event ) => { const nextValue = parseFloat( event.target.value ); - setValue( nextValue ); - onChange( nextValue ); + handleOnChange( 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 ); - } - onChange( nextValue ); - isResetPendent.current = false; - } else if ( allowReset ) { - isResetPendent.current = true; - } - }; - - const handleOnInputNumberBlur = () => { - if ( isResetPendent.current ) { + if ( isNaN( nextValue ) ) { handleOnReset(); - isResetPendent.current = false; + return; } + + setValue( nextValue ); + onChange( clamp( nextValue, min, max ) ); }; const handleOnReset = () => { @@ -273,16 +256,14 @@ function RangeControl( ) } { withInputField && ( - to be updated independently before the + * value is applied and propagated. This independent updating action is + * necessary to accommodate individual keystroke values that may not + * be considered "valid" (e.g. within the min - max range). + */ + const [ value, setValue ] = useState( valueProp ); + + /** + * Syncs incoming value (prop) within internal state. + */ + useEffect( () => { + if ( ! isNaN( valueProp ) ) { + setValue( ( prev ) => { + return valueProp !== prev ? valueProp : prev; + } ); + } + }, [ valueProp ] ); + + const handleOnReset = ( event ) => { + onReset( event ); + setValue( '' ); + }; + + const handleOnCommit = ( event ) => { + const nextValue = parseFloat( event.target.value ); + + if ( isNaN( nextValue ) ) { + handleOnReset(); + return; + } + + setValue( nextValue ); + onChange( nextValue ); + }; + + const handleOnBlur = ( event ) => { + onBlur( event ); + handleOnCommit( event ); + }; + + const handleOnChange = ( next, { event } ) => { + /** + * Update internal state. This allows for `InputNumber` to store + * values before a valid "commit". Example for these cases my include + * users typing in larger numbers, such as "123" (which would be parsed + * one character at a time). + */ + setValue( next ); + + const nextValue = parseFloat( next ); + + /** + * Only commit values if the value is a number. + * This prevents committing values such as "" (empty string), which + * occurs when the input is cleared. + */ + if ( isNaN( nextValue ) ) return; + + /** + * Prevent submitting if changes are invalid. + * This only applies to values being entered via KEY_DOWN. + * + * Pressing the up/down arrows of the HTML input also triggers a + * change event. However, those values will be (pre)validated by the + * HTML input. + */ + if ( event.target.checkValidity && ! event.target.checkValidity() ) { + return; + } + + /** + * We're passing in the next value in the shape of an event, as + * that is what the handleOnCommit callback expects. + * + * We have to use the "next" value (provided by `InputNumber`) as + * it my vary from the value from the input in the DOM. This occurs + * if the value is adjusted by enhanced mechanics such as "step" jumping + * or dragging to update. + */ + handleOnCommit( { target: { value: nextValue } } ); + }; + + const handleOnKeyDown = ( event ) => { + const { keyCode } = event; + onKeyDown( event ); + + if ( keyCode === ENTER ) { + event.preventDefault(); + handleOnCommit( event ); + } + }; + + return ( + + ); +} diff --git a/packages/components/src/range-control/test/index.js b/packages/components/src/range-control/test/index.js index 2ccb8d90293fa9..8dad132f2e216c 100644 --- a/packages/components/src/range-control/test/index.js +++ b/packages/components/src/range-control/test/index.js @@ -27,11 +27,10 @@ describe( 'RangeControl', () => { const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); - rangeInput.focus(); fireEvent.change( rangeInput, { target: { value: '5' } } ); - numberInput.focus(); fireEvent.change( numberInput, { target: { value: '10' } } ); + fireEvent.blur( numberInput ); expect( onChange ).toHaveBeenCalledWith( 5 ); expect( onChange ).toHaveBeenCalledWith( 10 ); @@ -67,10 +66,10 @@ describe( 'RangeControl', () => { fireEvent.change( numberInput, { target: { value: '10' } } ); fireEvent.blur( numberInput ); - expect( rangeInput.value ).not.toBe( '10' ); + expect( rangeInput.value ).toBe( '11' ); } ); - it( 'should not apply if new value is greater than maximum', () => { + it( 'should not apply new value is greater than maximum', () => { const { container } = render( ); const rangeInput = getRangeInput( container ); @@ -82,38 +81,20 @@ describe( 'RangeControl', () => { expect( rangeInput.value ).not.toBe( '21' ); } ); - it( 'should not call onChange if new value is invalid', () => { + it( 'should call onChange if new value is valid', () => { const onChange = jest.fn(); const { container } = render( ); - const numberInput = getNumberInput( container ); - - numberInput.focus(); - fireEvent.change( numberInput, { target: { value: '25e' } } ); - - expect( onChange ).not.toHaveBeenCalled(); - } ); - - it( 'should keep invalid values in number input until loss of focus', () => { - const onChange = jest.fn(); - const { container } = render( - - ); - const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); - numberInput.focus(); - fireEvent.change( numberInput, { target: { value: '-1.1' } } ); - - expect( numberInput.value ).toBe( '-1.1' ); - expect( rangeInput.value ).toBe( '-1' ); - + fireEvent.change( numberInput, { target: { value: '15' } } ); fireEvent.blur( numberInput ); - expect( onChange ).toHaveBeenCalledWith( -1 ); - expect( numberInput.value ).toBe( '-1' ); + + expect( onChange ).toHaveBeenCalledWith( 15 ); + expect( rangeInput.value ).toBe( '15' ); } ); it( 'should validate when provided a max or min of zero', () => { @@ -124,7 +105,6 @@ describe( 'RangeControl', () => { const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); - numberInput.focus(); fireEvent.change( numberInput, { target: { value: '1' } } ); fireEvent.blur( numberInput ); @@ -139,15 +119,19 @@ describe( 'RangeControl', () => { const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); - numberInput.focus(); - fireEvent.change( numberInput, { target: { value: '-101' } } ); + fireEvent.blur( numberInput ); + expect( rangeInput.value ).toBe( '-100' ); fireEvent.change( numberInput, { target: { value: '-49' } } ); + fireEvent.blur( numberInput ); + expect( rangeInput.value ).toBe( '-50' ); fireEvent.change( numberInput, { target: { value: '-50' } } ); + fireEvent.blur( numberInput ); + expect( rangeInput.value ).toBe( '-50' ); } ); @@ -164,13 +148,14 @@ describe( 'RangeControl', () => { const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); - numberInput.focus(); fireEvent.change( numberInput, { target: { value: '0.125' } } ); + fireEvent.blur( numberInput ); expect( onChange ).toHaveBeenCalledWith( 0.125 ); expect( rangeInput.value ).toBe( '0.125' ); fireEvent.change( numberInput, { target: { value: '0.225' } } ); + fireEvent.blur( numberInput ); expect( onChange ).toHaveBeenCalledWith( 0.225 ); expect( rangeInput.value ).toBe( '0.225' ); @@ -244,14 +229,13 @@ describe( 'RangeControl', () => { const rangeInput = getRangeInput( container ); const numberInput = getNumberInput( container ); - rangeInput.focus(); fireEvent.change( rangeInput, { target: { value: 13 } } ); expect( rangeInput.value ).toBe( '13' ); expect( numberInput.value ).toBe( '13' ); - numberInput.focus(); fireEvent.change( numberInput, { target: { value: 7 } } ); + fireEvent.blur( numberInput ); expect( rangeInput.value ).toBe( '7' ); expect( numberInput.value ).toBe( '7' ); diff --git a/packages/components/src/unit-control/test/index.js b/packages/components/src/unit-control/test/index.js index 21d102a6f97e34..8eac58f0dfdbaf 100644 --- a/packages/components/src/unit-control/test/index.js +++ b/packages/components/src/unit-control/test/index.js @@ -1,32 +1,46 @@ /** * External dependencies */ -import { render, fireEvent } from '@testing-library/react'; +import { render, unmountComponentAtNode } from 'react-dom'; +import { act, Simulate } from 'react-dom/test-utils'; +import { render as testRender } from '@testing-library/react'; /** * WordPress dependencies */ -import { UP, DOWN, ENTER } from '@wordpress/keycodes'; +import { UP, DOWN } from '@wordpress/keycodes'; /** * Internal dependencies */ import UnitControl from '../'; +let container = null; + +beforeEach( () => { + container = document.createElement( 'div' ); + document.body.appendChild( container ); +} ); + +afterEach( () => { + unmountComponentAtNode( container ); + container.remove(); + container = null; +} ); + const getComponent = () => - document.body.querySelector( '.components-unit-control' ); + container.querySelector( '.components-unit-control' ); const getInput = () => - document.body.querySelector( '.components-unit-control input' ); + container.querySelector( '.components-unit-control input' ); const getSelect = () => - document.body.querySelector( '.components-unit-control select' ); - -const fireKeyDown = ( data ) => - fireEvent.keyDown( document.activeElement || document.body, data ); + container.querySelector( '.components-unit-control select' ); describe( 'UnitControl', () => { describe( 'Basic rendering', () => { it( 'should render', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); const select = getSelect(); @@ -35,7 +49,9 @@ describe( 'UnitControl', () => { } ); it( 'should render custom className', () => { - render( ); + act( () => { + render( , container ); + } ); const el = getComponent(); @@ -43,7 +59,9 @@ describe( 'UnitControl', () => { } ); it( 'should not render select, if units are disabled', () => { - render( ); + act( () => { + render( , container ); + } ); const input = getInput(); const select = getSelect(); @@ -54,39 +72,61 @@ describe( 'UnitControl', () => { describe( 'Value', () => { it( 'should update value on change', () => { - let state = '50px'; - const setState = jest.fn( ( value ) => ( state = value ) ); + let state = 50; + const setState = ( nextState ) => ( state = nextState ); - render( ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: 62 } } ); - expect( setState ).toHaveBeenCalledTimes( 1 ); + act( () => { + Simulate.change( input, { target: { value: 62 } } ); + } ); + expect( state ).toBe( '62px' ); } ); it( 'should increment value on UP press', () => { - let state = '50px'; + let state = 50; const setState = ( nextState ) => ( state = nextState ); - render( ); + act( () => { + render( + , + container + ); + } ); + + const input = getInput(); - getInput().focus(); - fireKeyDown( { keyCode: UP } ); + act( () => { + Simulate.keyDown( input, { keyCode: UP } ); + } ); expect( state ).toBe( '51px' ); } ); it( 'should increment value on UP + SHIFT press, with step', () => { - let state = '50px'; + let state = 50; const setState = ( nextState ) => ( state = nextState ); - render( ); + act( () => { + render( + , + container + ); + } ); - getInput().focus(); - fireKeyDown( { keyCode: UP, shiftKey: true } ); + const input = getInput(); + + act( () => { + Simulate.keyDown( input, { keyCode: UP, shiftKey: true } ); + } ); expect( state ).toBe( '60px' ); } ); @@ -95,10 +135,18 @@ describe( 'UnitControl', () => { let state = 50; const setState = ( nextState ) => ( state = nextState ); - render( ); + act( () => { + render( + , + container + ); + } ); + + const input = getInput(); - getInput().focus(); - fireKeyDown( { keyCode: DOWN } ); + act( () => { + Simulate.keyDown( input, { keyCode: DOWN } ); + } ); expect( state ).toBe( '49px' ); } ); @@ -107,10 +155,18 @@ describe( 'UnitControl', () => { let state = 50; const setState = ( nextState ) => ( state = nextState ); - render( ); + act( () => { + render( + , + container + ); + } ); - getInput().focus(); - fireKeyDown( { keyCode: DOWN, shiftKey: true } ); + const input = getInput(); + + act( () => { + Simulate.keyDown( input, { keyCode: DOWN, shiftKey: true } ); + } ); expect( state ).toBe( '40px' ); } ); @@ -121,11 +177,18 @@ describe( 'UnitControl', () => { let state = 'px'; const setState = ( nextState ) => ( state = nextState ); - render( ); + act( () => { + render( + , + container + ); + } ); const select = getSelect(); - select.focus(); - fireEvent.change( select, { target: { value: 'em' } } ); + + act( () => { + Simulate.change( select, { target: { value: 'em' } } ); + } ); expect( state ).toBe( 'em' ); } ); @@ -135,8 +198,9 @@ describe( 'UnitControl', () => { { value: 'pt', label: 'pt', default: 0 }, { value: 'vmax', label: 'vmax', default: 10 }, ]; - - render( ); + act( () => { + render( , container ); + } ); const select = getSelect(); const options = select.querySelectorAll( 'option' ); @@ -157,24 +221,29 @@ describe( 'UnitControl', () => { { value: 'pt', label: 'pt', default: 25 }, { value: 'vmax', label: 'vmax', default: 75 }, ]; - - render( - - ); + act( () => { + render( + , + container + ); + } ); const select = getSelect(); - select.focus(); - fireEvent.change( select, { target: { value: 'vmax' } } ); + act( () => { + Simulate.change( select, { target: { value: 'vmax' } } ); + } ); expect( state ).toBe( '75vmax' ); - fireEvent.change( select, { target: { value: 'pt' } } ); + act( () => { + Simulate.change( select, { target: { value: 'pt' } } ); + } ); expect( state ).toBe( '25pt' ); } ); @@ -187,136 +256,146 @@ describe( 'UnitControl', () => { { value: 'pt', label: 'pt', default: 25 }, { value: 'vmax', label: 'vmax', default: 75 }, ]; - - render( - - ); + act( () => { + render( + , + container + ); + } ); const select = getSelect(); - select.focus(); - fireEvent.change( select, { target: { value: 'vmax' } } ); + act( () => { + Simulate.change( select, { target: { value: 'vmax' } } ); + } ); expect( state ).toBe( '50vmax' ); - fireEvent.change( select, { target: { value: 'pt' } } ); + act( () => { + Simulate.change( select, { target: { value: 'pt' } } ); + } ); expect( state ).toBe( '50pt' ); } ); } ); - describe( 'Unit Parser', () => { let state = '10px'; - const setState = jest.fn( ( nextState ) => ( state = nextState ) ); + const setState = ( nextState ) => ( state = nextState ); it( 'should parse unit from input', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: '55 em' } } ); - fireKeyDown( { keyCode: ENTER } ); + + act( () => { + Simulate.change( input, { target: { value: '55 em' } } ); + } ); expect( state ).toBe( '55em' ); } ); it( 'should parse PX unit from input', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: '61 PX' } } ); - fireKeyDown( { keyCode: ENTER } ); + + act( () => { + Simulate.change( input, { target: { value: '61 PX' } } ); + } ); expect( state ).toBe( '61px' ); } ); it( 'should parse EM unit from input', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: '55 em' } } ); - fireKeyDown( { keyCode: ENTER } ); + + act( () => { + Simulate.change( input, { target: { value: '55 em' } } ); + } ); expect( state ).toBe( '55em' ); } ); it( 'should parse % unit from input', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { target: { value: '-10 %' } } ); - fireKeyDown( { keyCode: ENTER } ); + + act( () => { + Simulate.change( input, { target: { value: '-10 %' } } ); + } ); expect( state ).toBe( '-10%' ); } ); it( 'should parse REM unit from input', () => { - render( - - ); + act( () => { + render( + , + container + ); + } ); const input = getInput(); - input.focus(); - fireEvent.change( input, { - target: { value: '123 rEm ' }, + + act( () => { + Simulate.change( input, { + target: { value: '123 rEm ' }, + } ); } ); - fireKeyDown( { keyCode: ENTER } ); expect( state ).toBe( '123rem' ); } ); it( 'should update unit after initial render and with new unit prop', () => { - const { rerender } = render( ); + const { container: testContainer, rerender } = testRender( + + ); - const select = getSelect(); + const select = testContainer.querySelector( 'select' ); expect( select.value ).toBe( '%' ); - rerender( ); + rerender( ); expect( select.value ).toBe( 'em' ); } ); it( 'should fallback to default unit if parsed unit is invalid', () => { - render( ); + const { container: testContainer } = testRender( + + ); + + const select = testContainer.querySelector( 'select' ); - expect( getSelect().value ).toBe( 'px' ); + expect( select.value ).toBe( 'px' ); } ); } ); } );