Skip to content

Commit

Permalink
Fix reset callback to use resetFallbackValue, if defined. Add tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Q committed May 11, 2020
1 parent 55bc26e commit 9647605
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
18 changes: 17 additions & 1 deletion packages/components/src/range-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,29 @@ const BaseRangeControl = forwardRef(

const handleOnReset = () => {
let resetValue = parseFloat( resetFallbackValue );
let onChangeResetValue = resetValue;

if ( isNaN( resetValue ) ) {
resetValue = null;
onChangeResetValue = undefined;
}

setValue( resetValue );
onChange( undefined );

/**
* Previously, this callback would always receive undefined as
* an argument. This behavior is unexpected, specifically
* when resetFallbackValue is defined.
*
* The value of undefined is not ideal. Passing it through
* to internal <input /> elements would change it from a
* controlled component to an uncontrolled component.
*
* For now, to minimize unexpected regressions, we're going to
* preserve the undefined callback argument, except when a
* resetFallbackValue is defined.
*/
onChange( onChangeResetValue );
};

const handleShowTooltip = () => setShowTooltip( true );
Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/range-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,13 @@ describe( 'RangeControl', () => {

describe( 'reset', () => {
it( 'should reset to a custom fallback value, defined by a parent component', () => {
const spy = jest.fn();
const { container } = render(
<RangeControl
initialPosition={ 50 }
value={ 10 }
allowReset={ true }
onChange={ spy }
resetFallbackValue={ 33 }
/>
);
Expand All @@ -322,16 +324,19 @@ describe( 'RangeControl', () => {

expect( rangeInput.value ).toBe( '33' );
expect( numberInput.value ).toBe( '33' );
expect( spy ).toHaveBeenCalledWith( 33 );
} );

it( 'should reset to a 50% of min/max value, of no initialPosition or value is defined', () => {
const spy = jest.fn();
const { container } = render(
<RangeControl
initialPosition={ undefined }
value={ 10 }
min={ 0 }
max={ 100 }
allowReset={ true }
onChange={ spy }
resetFallbackValue={ undefined }
/>
);
Expand All @@ -344,6 +349,7 @@ describe( 'RangeControl', () => {

expect( rangeInput.value ).toBe( '50' );
expect( numberInput.value ).toBe( '' );
expect( spy ).toHaveBeenCalledWith( undefined );
} );
} );
} );

0 comments on commit 9647605

Please sign in to comment.