diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index a21b3d305a7e2b..f8ad35ad5c4269 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -8,6 +8,7 @@ - `withFilters` has been optimized to avoid binding hook handlers for each mounted instance of the component, instead using a single centralized hook delegator. - `withFilters` has been optimized to reuse a single shared component definition for all filtered instances of the component. +- Make `RangeControl` validate min and max properties. ### Bug Fixes diff --git a/packages/components/src/range-control/README.md b/packages/components/src/range-control/README.md index 28fcae0c82c95e..1f2024c8b60866 100644 --- a/packages/components/src/range-control/README.md +++ b/packages/components/src/range-control/README.md @@ -169,6 +169,21 @@ If allowReset is true, when onChange is called without any parameter passed it s - Type: `function` - Required: Yes +#### min + +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 + + +#### max + +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 + ## Related components - To collect a numerical input in a text field, use the `TextControl` component. diff --git a/packages/components/src/range-control/index.js b/packages/components/src/range-control/index.js index ea341b5fc8b27a..1503813def5cbf 100644 --- a/packages/components/src/range-control/index.js +++ b/packages/components/src/range-control/index.js @@ -8,7 +8,7 @@ import classnames from 'classnames'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { withInstanceId } from '@wordpress/compose'; +import { compose, withInstanceId, withState } from '@wordpress/compose'; /** * Internal dependencies @@ -17,6 +17,7 @@ import { BaseControl, Button, Dashicon } from '../'; function RangeControl( { className, + currentInput, label, value, instanceId, @@ -26,19 +27,48 @@ function RangeControl( { help, allowReset, initialPosition, + min, + max, + setState, ...props } ) { const id = `inspector-range-control-${ instanceId }`; - const resetValue = () => onChange(); + const currentInputValue = currentInput === null ? value : currentInput; + const resetValue = () => { + resetCurrentInput(); + onChange(); + }; + const resetCurrentInput = () => { + if ( currentInput !== null ) { + setState( { + currentInput: null, + } ); + } + }; + const onChangeValue = ( event ) => { const newValue = event.target.value; - if ( newValue === '' ) { - resetValue(); + const newNumericValue = parseInt( newValue, 10 ); + // If the input value is invalid temporarily save it to the state, + // without calling on change. + if ( + isNaN( newNumericValue ) || + ( min !== undefined && newNumericValue < min ) || + ( max !== undefined && newNumericValue > max ) + ) { + setState( { + currentInput: newValue, + } ); return; } - onChange( Number( newValue ) ); + // The input is valid, reset the local state property used to temporaly save the value, + // and call onChange with the new value as a number. + resetCurrentInput(); + onChange( newNumericValue ); }; - const initialSliderValue = isFinite( value ) ? value : initialPosition || ''; + const initialSliderValue = isFinite( value ) ? + currentInputValue : + initialPosition || ''; return ( { afterIcon && } { allowReset && @@ -74,4 +109,9 @@ function RangeControl( { ); } -export default withInstanceId( RangeControl ); +export default compose( [ + withInstanceId, + withState( { + currentInput: null, + } ), +] )( RangeControl ); diff --git a/packages/components/src/range-control/test/index.js b/packages/components/src/range-control/test/index.js index 4b2912665dd999..5f8e0bccb8aa84 100644 --- a/packages/components/src/range-control/test/index.js +++ b/packages/components/src/range-control/test/index.js @@ -81,4 +81,138 @@ describe( 'RangeControl', () => { expect( icons[ 1 ].props.icon ).toBe( 'format-video' ); } ); } ); + + describe( 'validation', () => { + it( 'does not calls onChange if the new value is lower than minimum', () => { + // Mount: With shallow, cannot find input child of BaseControl + const onChange = jest.fn(); + const wrapper = getWrapper( { onChange, min: 11, value: 12 } ); + + const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass( + wrapper, + 'components-range-control__number' + ); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '10' }, + } + ); + + expect( onChange ).not.toHaveBeenCalled(); + } ); + + it( 'does not calls onChange if the new value is greater than maximum', () => { + // Mount: With shallow, cannot find input child of BaseControl + const onChange = jest.fn(); + const wrapper = getWrapper( { onChange, max: 20, value: 12 } ); + + const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass( + wrapper, + 'components-range-control__number' + ); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '21' }, + } + ); + + expect( onChange ).not.toHaveBeenCalled(); + } ); + + it( 'calls onChange after invalid inputs if the new input is valid', () => { + // Mount: With shallow, cannot find input child of BaseControl + const onChange = jest.fn(); + const wrapper = getWrapper( { onChange, min: 11, max: 20, value: 12 } ); + + const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass( + wrapper, + 'components-range-control__number' + ); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '10' }, + } + ); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '21' }, + } + ); + + expect( onChange ).not.toHaveBeenCalled(); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '14' }, + } + ); + + expect( onChange ).toHaveBeenCalledWith( 14 ); + } ); + + it( 'validates when provided a max or min of zero', () => { + const onChange = jest.fn(); + const wrapper = getWrapper( { onChange, min: -100, max: 0, value: 0 } ); + + const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass( + wrapper, + 'components-range-control__number' + ); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '1' }, + } + ); + + expect( onChange ).not.toHaveBeenCalled(); + } ); + + it( 'validates when min and max are negative', () => { + const onChange = jest.fn(); + const wrapper = getWrapper( { onChange, min: -100, max: -50, value: -60 } ); + + const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass( + wrapper, + 'components-range-control__number' + ); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '-101' }, + } + ); + + expect( onChange ).not.toHaveBeenCalled(); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '-49' }, + } + ); + + expect( onChange ).not.toHaveBeenCalled(); + + TestUtils.Simulate.change( + numberInputElement(), + { + target: { value: '-50' }, + } + ); + + expect( onChange ).toHaveBeenCalled(); + } ); + } ); } );