Skip to content

Commit

Permalink
RangeControl: Fix Reset and initialPosition (#20247)
Browse files Browse the repository at this point in the history 
* RangeControl: Fix reset and initialPosition logic

* Add tests for desired initialPosition, value, and reset scenarios

* Update storybook snapshots

* Fix null value fill for marks

* Remove unnecessary ref dependency in useEffect hook

* Adjust Range x Tooltip interactions

So that hovering on a tooltip no longer keeps it open
  • Loading branch information
Jon Quach authored Mar 9, 2020
1 parent c01bb1e commit 6066186
Show file tree
Hide file tree
Showing 5 changed files with 211 additions and 162 deletions.
42 changes: 26 additions & 16 deletions packages/components/src/range-control/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import classnames from 'classnames';
import { clamp, noop } from 'lodash';
import { clamp, isFinite, noop } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -18,7 +18,11 @@ import BaseControl from '../base-control';
import Button from '../button';
import Icon from '../icon';
import { color } from '../utils/colors';
import { useControlledRangeValue, useDebouncedHoverInteraction } from './utils';
import {
floatClamp,
useControlledRangeValue,
useDebouncedHoverInteraction,
} from './utils';
import RangeRail from './rail';
import SimpleTooltip from './tooltip';
import {
Expand All @@ -42,6 +46,7 @@ const BaseRangeControl = forwardRef(
allowReset = false,
beforeIcon,
className,
currentInput,
color: colorProp = color( 'blue.wordpress.700' ),
disabled = false,
help,
Expand All @@ -54,7 +59,7 @@ const BaseRangeControl = forwardRef(
onBlur = noop,
onChange = noop,
onFocus = noop,
onMouseEnter = noop,
onMouseMove = noop,
onMouseLeave = noop,
renderTooltipContent = ( v ) => v,
showTooltip: showTooltipProp,
Expand All @@ -67,15 +72,14 @@ const BaseRangeControl = forwardRef(
) => {
const isRTL = useRtl();

const sliderValue = initialPosition || valueProp;
const sliderValue = valueProp || initialPosition;
const [ value, setValue ] = useControlledRangeValue( {
min,
max,
value: sliderValue,
} );
const [ showTooltip, setShowTooltip ] = useState( showTooltipProp );
const [ isFocused, setIsFocused ] = useState( false );
const originalValueRef = useRef( value );

const inputRef = useRef();

Expand All @@ -90,7 +94,16 @@ const BaseRangeControl = forwardRef(
const isCurrentlyFocused = inputRef.current?.matches( ':focus' );
const isThumbFocused = ! disabled && isFocused;

const fillValue = ( ( value - min ) / ( max - min ) ) * 100;
const isValueReset = value === null;
const inputSliderValue = isValueReset ? '' : value;
const currentInputValue = isValueReset ? '' : value || currentInput;

const rangeFillValue = isValueReset
? floatClamp( max / 2, min, max )
: value;

const calculatedFillValue = ( ( value - min ) / ( max - min ) ) * 100;
const fillValue = isValueReset ? 50 : calculatedFillValue;
const fillValueOffset = `${ clamp( fillValue, 0, 100 ) }%`;

const classes = classnames( 'components-range-control', className );
Expand All @@ -103,7 +116,7 @@ const BaseRangeControl = forwardRef(
const id = `inspector-range-control-${ instanceId }`;

const describedBy = !! help ? `${ id }__help` : undefined;
const enableTooltip = showTooltipProp !== false;
const enableTooltip = showTooltipProp !== false && isFinite( value );

const handleOnChange = ( event ) => {
if ( ! event.target.checkValidity() ) {
Expand All @@ -117,10 +130,8 @@ const BaseRangeControl = forwardRef(
};

const handleOnReset = () => {
const nextValue = originalValueRef.current;

setValue( nextValue );
onChange( nextValue );
setValue( null );
onChange( undefined );
};

const handleShowTooltip = () => setShowTooltip( true );
Expand All @@ -141,7 +152,7 @@ const BaseRangeControl = forwardRef(
const hoverInteractions = useDebouncedHoverInteraction( {
onShow: handleShowTooltip,
onHide: handleHideTooltip,
onMouseEnter,
onMouseMove,
onMouseLeave,
} );

Expand Down Expand Up @@ -188,15 +199,15 @@ const BaseRangeControl = forwardRef(
step={ step }
tabIndex={ 0 }
type="range"
value={ value }
value={ inputSliderValue }
/>
<RangeRail
aria-hidden={ true }
marks={ marks }
max={ max }
min={ min }
step={ step }
value={ value }
value={ rangeFillValue }
/>
<Track
aria-hidden={ true }
Expand All @@ -211,7 +222,6 @@ const BaseRangeControl = forwardRef(
</ThumbWrapper>
{ enableTooltip && (
<SimpleTooltip
{ ...hoverInteractions }
className="components-range-control__tooltip"
inputRef={ inputRef }
renderTooltipContent={ renderTooltipContent }
Expand All @@ -236,7 +246,7 @@ const BaseRangeControl = forwardRef(
onChange={ handleOnChange }
step={ step }
type="number"
value={ value }
value={ currentInputValue }
/>
) }
{ allowReset && (
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/range-control/styles/range-control-styles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ export const Tooltip = styled.span`
min-width: 32px;
opacity: 0;
padding: 8px;
pointer-events: none;
position: absolute;
text-align: center;
transition: opacity 120ms ease;
Expand Down
117 changes: 116 additions & 1 deletion packages/components/src/range-control/test/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import TestUtils from 'react-dom/test-utils';
import TestUtils, { act } from 'react-dom/test-utils';

/**
* Internal dependencies
Expand Down Expand Up @@ -293,4 +293,119 @@ describe( 'RangeControl', () => {
expect( onChange ).toHaveBeenCalledWith( 0.225 );
} );
} );

describe( 'initialPosition / value', () => {
const getInputElement = ( wrapper ) =>
TestUtils.findRenderedDOMComponentWithClass(
wrapper,
'components-range-control__slider'
);

it( 'renders initial rendered value of 50% of min/max, if no initialPosition or value is defined', () => {
const wrapper = getWrapper( { min: 0, max: 10 } );
const inputElement = getInputElement( wrapper );

expect( inputElement.value ).toBe( '5' );
} );

it( 'renders initialPosition if no value is provided', () => {
const wrapper = getWrapper( {
initialPosition: 50,
value: undefined,
} );
const inputElement = getInputElement( wrapper );

expect( inputElement.value ).toBe( '50' );
} );

it( 'renders value instead of initialPosition is provided', () => {
const wrapper = getWrapper( { initialPosition: 50, value: 10 } );
const inputElement = getInputElement( wrapper );

expect( inputElement.value ).toBe( '10' );
} );
} );

describe( 'reset', () => {
class StatefulTestWrapper extends Component {
constructor( props ) {
super( props );
this.state = {
value: undefined,
};
this.handleOnChange = this.handleOnChange.bind( this );
}

handleOnChange( nextValue = this.props.resetFallbackValue ) {
this.setState( { value: nextValue } );
}

render() {
return (
<RangeControl
{ ...this.props }
value={ this.state.value }
onChange={ this.handleOnChange }
/>
);
}
}

const getStatefulWrapper = ( props = {} ) =>
TestUtils.renderIntoDocument(
<StatefulTestWrapper { ...props } />
);

const getInputElement = ( wrapper ) =>
TestUtils.findRenderedDOMComponentWithClass(
wrapper,
'components-range-control__slider'
);

it( 'resets to a custom fallback value, defined by a parent component', () => {
const wrapper = getStatefulWrapper( {
initialPosition: 50,
value: 10,
allowReset: true,
resetFallbackValue: 33,
} );

const resetButton = TestUtils.findRenderedDOMComponentWithClass(
wrapper,
'components-range-control__reset'
);

act( () => {
TestUtils.Simulate.click( resetButton );
} );

const inputElement = getInputElement( wrapper );

expect( inputElement.value ).toBe( '33' );
} );

it( 'resets to a 50% of min/max value, of no initialPosition or value is defined', () => {
const wrapper = getStatefulWrapper( {
initialPosition: undefined,
value: 10,
min: 0,
max: 100,
allowReset: true,
resetFallbackValue: undefined,
} );

const resetButton = TestUtils.findRenderedDOMComponentWithClass(
wrapper,
'components-range-control__reset'
);

act( () => {
TestUtils.Simulate.click( resetButton );
} );

const inputElement = getInputElement( wrapper );

expect( inputElement.value ).toBe( '50' );
} );
} );
} );
33 changes: 20 additions & 13 deletions packages/components/src/range-control/utils.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { clamp, noop } from 'lodash';
import { clamp, isFinite, noop } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -11,35 +11,42 @@ import { useCallback, useRef, useEffect, useState } from '@wordpress/element';
/**
* A float supported clamp function for a specific value.
*
* @param {number} value The value to clamp
* @param {number|null} value The value to clamp
* @param {number} min The minimum value
* @param {number} max The maxinum value
*
* @return {number} A (float) number
*/
function floatClamp( value, min, max ) {
export function floatClamp( value, min, max ) {
if ( ! isFinite( value ) ) {
return null;
}

return parseFloat( clamp( value, min, max ) );
}

/**
* Hook to store a clamped value, derived from props.
*/
export function useControlledRangeValue( { min, max, value: valueProp = 0 } ) {
export function useControlledRangeValue( { min, max, value: valueProp } ) {
const [ value, setValue ] = useState( floatClamp( valueProp, min, max ) );
const valueRef = useRef( value );

const setClampValue = ( nextValue ) => {
setValue( floatClamp( nextValue, min, max ) );
};
const setClampValue = useCallback(
( nextValue ) => {
setValue( floatClamp( nextValue, min, max ) );
},
[ setValue, min, max ]
);

useEffect( () => {
if ( valueRef.current !== valueProp ) {
setClampValue( valueProp );
valueRef.current = valueProp;
}
}, [ valueProp, setClampValue ] );
}, [ valueProp, setValue ] );

return [ value, setClampValue ];
return [ value, setValue ];
}

/**
Expand All @@ -49,7 +56,7 @@ export function useControlledRangeValue( { min, max, value: valueProp = 0 } ) {
export function useDebouncedHoverInteraction( {
onShow = noop,
onHide = noop,
onMouseEnter = noop,
onMouseMove = noop,
onMouseLeave = noop,
timeout = 300,
} ) {
Expand All @@ -65,8 +72,8 @@ export function useDebouncedHoverInteraction( {
[ timeout ]
);

const handleOnMouseEnter = useCallback( ( event ) => {
onMouseEnter( event );
const handleOnMouseMove = useCallback( ( event ) => {
onMouseMove( event );

setDebouncedTimeout( () => {
if ( ! show ) {
Expand All @@ -92,7 +99,7 @@ export function useDebouncedHoverInteraction( {
} );

return {
onMouseEnter: handleOnMouseEnter,
onMouseMove: handleOnMouseMove,
onMouseLeave: handleOnMouseLeave,
};
}
Loading

0 comments on commit 6066186

Please sign in to comment.