RangeControl: Fix Reset and initialPosition

packages/components/src/range-control/index.js

@@ -2,7 +2,7 @@
  * External dependencies
  */
 import classnames from 'classnames';
-import { clamp, noop } from 'lodash';
+import { clamp, isFinite, noop } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -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 {
@@ -42,6 +46,7 @@ const BaseRangeControl = forwardRef(
 			allowReset = false,
 			beforeIcon,
 			className,
+			currentInput,
 			color: colorProp = color( 'blue.wordpress.700' ),
 			disabled = false,
 			help,
@@ -54,7 +59,7 @@ const BaseRangeControl = forwardRef(
 			onBlur = noop,
 			onChange = noop,
 			onFocus = noop,
-			onMouseEnter = noop,
+			onMouseMove = noop,
 			onMouseLeave = noop,
 			renderTooltipContent = ( v ) => v,
 			showTooltip: showTooltipProp,
@@ -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();
 
@@ -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 );
@@ -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() ) {
@@ -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 );
@@ -141,7 +152,7 @@ const BaseRangeControl = forwardRef(
 		const hoverInteractions = useDebouncedHoverInteraction( {
 			onShow: handleShowTooltip,
 			onHide: handleHideTooltip,
-			onMouseEnter,
+			onMouseMove,
 			onMouseLeave,
 		} );
 
@@ -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 }
@@ -211,7 +222,6 @@ const BaseRangeControl = forwardRef(
 						</ThumbWrapper>
 						{ enableTooltip && (
 							<SimpleTooltip
-								{ ...hoverInteractions }
 								className="components-range-control__tooltip"
 								inputRef={ inputRef }
 								renderTooltipContent={ renderTooltipContent }
@@ -236,7 +246,7 @@ const BaseRangeControl = forwardRef(
 							onChange={ handleOnChange }
 							step={ step }
 							type="number"
-							value={ value }
+							value={ currentInputValue }
 						/>
 					) }
 					{ allowReset && (

packages/components/src/range-control/styles/range-control-styles.js

@@ -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;

packages/components/src/range-control/test/index.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import TestUtils from 'react-dom/test-utils';
+import TestUtils, { act } from 'react-dom/test-utils';
 
 /**
  * Internal dependencies
@@ -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' );
+		} );
+	} );
 } );

packages/components/src/range-control/utils.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { clamp, noop } from 'lodash';
+import { clamp, isFinite, noop } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -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 ];
 }
 
 /**
@@ -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,
 } ) {
@@ -65,8 +72,8 @@ export function useDebouncedHoverInteraction( {
 		[ timeout ]
 	);
 
-	const handleOnMouseEnter = useCallback( ( event ) => {
-		onMouseEnter( event );
+	const handleOnMouseMove = useCallback( ( event ) => {
+		onMouseMove( event );
 
 		setDebouncedTimeout( () => {
 			if ( ! show ) {
@@ -92,7 +99,7 @@ export function useDebouncedHoverInteraction( {
 	} );
 
 	return {
-		onMouseEnter: handleOnMouseEnter,
+		onMouseMove: handleOnMouseMove,
 		onMouseLeave: handleOnMouseLeave,
 	};
 }