Fix: RangeControl does not validates the min and max properties (#12952)

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
 

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.

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 (
 		<BaseControl
@@ -55,14 +85,19 @@ function RangeControl( {
 				value={ initialSliderValue }
 				onChange={ onChangeValue }
 				aria-describedby={ !! help ? id + '__help' : undefined }
+				min={ min }
+				max={ max }
 				{ ...props } />
 			{ afterIcon && <Dashicon icon={ afterIcon } /> }
 			<input
 				className="components-range-control__number"
 				type="number"
 				onChange={ onChangeValue }
 				aria-label={ label }
-				value={ value }
+				value={ currentInputValue }
+				min={ min }
+				max={ max }
+				onBlur={ resetCurrentInput }
 				{ ...props }
 			/>
 			{ allowReset &&
@@ -74,4 +109,9 @@ function RangeControl( {
 	);
 }
 
-export default withInstanceId( RangeControl );
+export default compose( [
+	withInstanceId,
+	withState( {
+		currentInput: null,
+	} ),
+] )( RangeControl );

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();
+		} );
+	} );
 } );