Skip to content

Commit

Permalink
Fix: RangeControl does not validates the min and max properties
Browse files Browse the repository at this point in the history
  • Loading branch information
jorgefilipecosta committed Jan 25, 2019
1 parent 310cb4b commit cfb0d82
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 8 deletions.
15 changes: 15 additions & 0 deletions packages/components/src/range-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,18 @@ 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
56 changes: 48 additions & 8 deletions packages/components/src/range-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -17,6 +17,7 @@ import { BaseControl, Button, Dashicon } from '../';

function RangeControl( {
className,
currentInput,
label,
value,
instanceId,
Expand All @@ -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 temporarly save it to the state,
// without calling on change.
if (
isNaN( newNumericValue ) ||
( min && newNumericValue < min ) ||
( max && 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
Expand All @@ -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 &&
Expand All @@ -74,4 +109,9 @@ function RangeControl( {
);
}

export default withInstanceId( RangeControl );
export default compose( [
withInstanceId,
withState( {
currentInput: null,
} ),
] )( RangeControl );
78 changes: 78 additions & 0 deletions packages/components/src/range-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,82 @@ 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 );
} );
} );
} );

0 comments on commit cfb0d82

Please sign in to comment.