Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RangeControl: disable reset button consistently #64579

Merged
merged 8 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@

### Bug Fixes

- `RangeControl`: disable reset button when the current value is equal to the reset value ([#64579](https://github.com/WordPress/gutenberg/pull/64579)).
- `RangeControl`: tweak mark and label absolute positioning ([#64487](https://github.com/WordPress/gutenberg/pull/64487)).

### Internal
Expand Down
47 changes: 38 additions & 9 deletions packages/components/src/range-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,25 @@ import { space } from '../utils/space';

const noop = () => {};

/**
* Computes the value that `RangeControl` should reset to when pressing
* the reset button.
*/
function computeResetValue( {
resetFallbackValue,
initialPosition,
}: Pick< RangeControlProps, 'resetFallbackValue' | 'initialPosition' > ) {
if ( resetFallbackValue !== undefined ) {
return ! Number.isNaN( resetFallbackValue ) ? resetFallbackValue : null;
}

if ( initialPosition !== undefined ) {
return ! Number.isNaN( initialPosition ) ? initialPosition : null;
}

return null;
}

function UnforwardedRangeControl(
props: WordPressComponentProps< RangeControlProps, 'input', false >,
forwardedRef: ForwardedRef< HTMLInputElement >
Expand Down Expand Up @@ -166,13 +185,12 @@ function UnforwardedRangeControl(
};

const handleOnReset = () => {
let resetValue: number | null = parseFloat( `${ resetFallbackValue }` );
let onChangeResetValue: number | undefined = resetValue;

if ( isNaN( resetValue ) ) {
resetValue = null;
onChangeResetValue = undefined;
}
// Reset to `resetFallbackValue` if defined, otherwise set internal value
// to `null` — which, if propagated to the `value` prop, will cause
// the value to be reset to the `initialPosition` prop if defined.
const resetValue = Number.isNaN( resetFallbackValue )
? null
: resetFallbackValue ?? null;

setValue( resetValue );

Expand All @@ -189,7 +207,7 @@ function UnforwardedRangeControl(
* preserve the undefined callback argument, except when a
* resetFallbackValue is defined.
*/
onChange( onChangeResetValue );
onChange( resetValue ?? undefined );
};

const handleShowTooltip = () => setShowTooltip( true );
Expand All @@ -210,6 +228,7 @@ function UnforwardedRangeControl(
const offsetStyle = {
[ isRTL() ? 'right' : 'left' ]: fillValueOffset,
};

return (
<BaseControl
__nextHasNoMarginBottom={ __nextHasNoMarginBottom }
Expand Down Expand Up @@ -329,7 +348,17 @@ function UnforwardedRangeControl(
className="components-range-control__reset"
// If the RangeControl itself is disabled, the reset button shouldn't be in the tab sequence.
accessibleWhenDisabled={ ! disabled }
disabled={ disabled || value === undefined }
// The reset button should be disabled if RangeControl itself is disabled,
// or if the current `value` is equal to the value that would be currently
// assigned when clicking the button.
disabled={
disabled ||
value ===
computeResetValue( {
resetFallbackValue,
initialPosition,
} )
}
variant="secondary"
size="small"
onClick={ handleOnReset }
Expand Down
60 changes: 47 additions & 13 deletions packages/components/src/range-control/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -298,36 +298,67 @@ describe( 'RangeControl', () => {
} );

describe( 'reset', () => {
it( 'should reset to a custom fallback value, defined by a parent component', () => {
it( 'should clear the input value when clicking the reset button', () => {
const spy = jest.fn();
render( <RangeControl allowReset onChange={ spy } /> );

const resetButton = getResetButton();
const rangeInput = getRangeInput();
const numberInput = getNumberInput();

fireChangeEvent( numberInput, '14' );

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

fireEvent.click( resetButton );

// range input resets to min + (max-min)/2
expect( rangeInput.value ).toBe( '50' );
expect( numberInput.value ).toBe( '' );
expect( spy ).toHaveBeenCalledWith( undefined );

expect( resetButton ).toHaveAttribute( 'aria-disabled', 'true' );
} );

it( 'should reset to the `initialPosition` value when clicking the reset button', () => {
const spy = jest.fn();
render(
<RangeControl
initialPosition={ 10 }
allowReset
initialPosition={ 23 }
onChange={ spy }
resetFallbackValue={ 33 }
/>
);

const resetButton = getResetButton();
const rangeInput = getRangeInput();
const numberInput = getNumberInput();

fireChangeEvent( numberInput, '14' );

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

fireEvent.click( resetButton );

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

expect( resetButton ).toHaveAttribute( 'aria-disabled', 'true' );
} );

it( 'should reset to a 50% of min/max value, of no initialPosition or value is defined', () => {
it( 'should reset to the `resetFallbackValue` value when clicking the reset button', () => {
const spy = jest.fn();
render(
<RangeControl
initialPosition={ undefined }
min={ 0 }
max={ 100 }
initialPosition={ 10 }
allowReset
resetFallbackValue={ undefined }
onChange={ spy }
resetFallbackValue={ 33 }
/>
);

Expand All @@ -337,8 +368,11 @@ describe( 'RangeControl', () => {

fireEvent.click( resetButton );

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

expect( resetButton ).toHaveAttribute( 'aria-disabled', 'true' );
} );
} );
} );
Loading