Skip to content
Prev Previous commit
Next Next commit
mend logic for number input value resolution in RangeControl
- on blur use handleOnReset if allowRest is true
- call onChange with a clamped value if new value is out-of-range
- update test to use invalid value instead of out-of-range value
- aside: remove an extraneous prop from number input
  • Loading branch information
stokesman committed Sep 28, 2020
commit a4314d59640caf2ed8759ab1f85b1df080527ad5
28 changes: 13 additions & 15 deletions packages/components/src/range-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function RangeControl(
value: valueProp,
initial: initialPosition,
} );
const pendentValue = useRef();
const isResetPendent = useRef( false );
const [ showTooltip, setShowTooltip ] = useState( showTooltipProp );
const [ isFocused, setIsFocused ] = useState( false );

Expand Down Expand Up @@ -128,25 +128,24 @@ function RangeControl(
nextValue = parseFloat( nextValue );
setValue( nextValue );
/*
* Calls onChange only if nextValue is valid,
* otherwise tracks it for resolution on blur
* Calls onChange only when nextValue is numeric
* otherwise may queue a reset for the blur event.
*/
if ( ! isNaN( nextValue ) && nextValue >= min && nextValue <= max ) {
if ( ! isNaN( nextValue ) ) {
if ( nextValue < min || nextValue > max ) {
nextValue = floatClamp( nextValue, min, max );
}
onChange( nextValue );
} else {
pendentValue.current = nextValue;
isResetPendent.current = false;
} else if ( allowReset ) {
isResetPendent.current = true;
}
};

const handleOnInputNumberBlur = () => {
if ( pendentValue.current !== null ) {
// Resets NaN values or clamps out-of-range values
const nextValue = isNaN( pendentValue.current )
? resetFallbackValue ?? initialPosition ?? valueProp
: floatClamp( pendentValue.current, min, max );
setValue( nextValue );
onChange( nextValue );
pendentValue.current = null;
if ( isResetPendent.current ) {
handleOnReset();
isResetPendent.current = false;
}
};

Expand Down Expand Up @@ -286,7 +285,6 @@ function RangeControl(
onChange={ handleOnChange }
shiftStep={ shiftStep }
step={ step }
type={ 'number' }
value={ inputSliderValue }
/>
) }
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/range-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe( 'RangeControl', () => {
} );

describe( 'validation', () => {
it( 'should not apply new value is lower than minimum', () => {
it( 'should not apply if new value is lower than minimum', () => {
const { container } = render( <RangeControl min={ 11 } /> );

const rangeInput = getRangeInput( container );
Expand All @@ -70,7 +70,7 @@ describe( 'RangeControl', () => {
expect( rangeInput.value ).not.toBe( '10' );
} );

it( 'should not apply new value is greater than maximum', () => {
it( 'should not apply if new value is greater than maximum', () => {
const { container } = render( <RangeControl max={ 20 } /> );

const rangeInput = getRangeInput( container );
Expand All @@ -91,7 +91,7 @@ describe( 'RangeControl', () => {
const numberInput = getNumberInput( container );

numberInput.focus();
fireEvent.change( numberInput, { target: { value: '25' } } );
fireEvent.change( numberInput, { target: { value: '25e' } } );

expect( onChange ).not.toHaveBeenCalled();
} );
Expand Down