Fix price slider constraints (#1125)

* Fix price slider constraints

* Update inline comments

* use previous tests

Author: mikejolley

assets/js/base/components/price-slider/index.js

@@ -12,7 +12,7 @@ import {
 } from '@wordpress/element';
 import PropTypes from 'prop-types';
 import classnames from 'classnames';
-import { useDebounce } from '@woocommerce/base-hooks';
+import { useDebounce, usePrevious } from '@woocommerce/base-hooks';
 
 /**
  * Internal dependencies
@@ -47,15 +47,25 @@ const PriceSlider = ( {
 		formatCurrencyForInput( maxPrice, priceFormat, currencySymbol )
 	);
 	const debouncedChangeValue = useDebounce( [ minPrice, maxPrice ], 500 );
+	const prevMinConstraint = usePrevious( minConstraint );
+	const prevMaxConstraint = usePrevious( maxConstraint );
 
 	useEffect( () => {
-		if ( minPrice === undefined || minConstraint > minPrice ) {
+		if (
+			minPrice === undefined ||
+			minConstraint > minPrice ||
+			minPrice === prevMinConstraint
+		) {
 			setMinPrice( minConstraint );
 		}
 	}, [ minConstraint ] );
 
 	useEffect( () => {
-		if ( maxPrice === undefined || maxConstraint < maxPrice ) {
+		if (
+			maxPrice === undefined ||
+			maxConstraint < maxPrice ||
+			maxPrice === prevMaxConstraint
+		) {
 			setMaxPrice( maxConstraint );
 		}
 	}, [ maxConstraint ] );
@@ -82,6 +92,18 @@ const PriceSlider = ( {
 	 * Handles styles for the shaded area of the range slider.
 	 */
 	const getProgressStyle = useMemo( () => {
+		if (
+			! isFinite( minPrice ) ||
+			! isFinite( maxPrice ) ||
+			! isFinite( minConstraint ) ||
+			! isFinite( maxConstraint )
+		) {
+			return {
+				'--low': '0%',
+				'--high': '100%',
+			};
+		}
+
 		const low =
 			Math.round(
 				100 *
@@ -99,7 +121,7 @@ const PriceSlider = ( {
 			'--low': low + '%',
 			'--high': high + '%',
 		};
-	}, [ minPrice, minConstraint, maxPrice, maxConstraint ] );
+	}, [ minPrice, maxPrice, minConstraint, maxConstraint ] );
 
 	/**
 	 * Trigger the onChange prop callback with new values.

assets/js/base/hooks/index.js

@@ -4,3 +4,4 @@ export * from './use-store-products';
 export * from './use-collection';
 export * from './use-collection-header';
 export * from './use-debounce';
+export * from './use-previous';

assets/js/base/hooks/test/use-previous.js

@@ -0,0 +1,57 @@
+/**
+ * External dependencies
+ */
+import TestRenderer, { act } from 'react-test-renderer';
+
+/**
+ * Internal dependencies
+ */
+import { usePrevious } from '../use-previous';
+
+describe( 'usePrevious', () => {
+	const TestComponent = ( { testValue } ) => {
+		const previousValue = usePrevious( testValue );
+		return <div testValue={ testValue } previousValue={ previousValue } />;
+	};
+
+	let renderer;
+	beforeEach( () => ( renderer = null ) );
+
+	it( 'should be undefined at first pass', () => {
+		act( () => {
+			renderer = TestRenderer.create( <TestComponent testValue={ 1 } /> );
+		} );
+		const testValue = renderer.root.findByType( 'div' ).props.testValue;
+		const testPreviousValue = renderer.root.findByType( 'div' ).props
+			.previousValue;
+
+		expect( testValue ).toBe( 1 );
+		expect( testPreviousValue ).toBe( undefined );
+	} );
+
+	it( 'test new and previous value', () => {
+		let testValue;
+		let testPreviousValue;
+		act( () => {
+			renderer = TestRenderer.create( <TestComponent testValue={ 1 } /> );
+		} );
+
+		act( () => {
+			renderer.update( <TestComponent testValue={ 2 } /> );
+		} );
+		testValue = renderer.root.findByType( 'div' ).props.testValue;
+		testPreviousValue = renderer.root.findByType( 'div' ).props
+			.previousValue;
+		expect( testValue ).toBe( 2 );
+		expect( testPreviousValue ).toBe( 1 );
+
+		act( () => {
+			renderer.update( <TestComponent testValue={ 3 } /> );
+		} );
+		testValue = renderer.root.findByType( 'div' ).props.testValue;
+		testPreviousValue = renderer.root.findByType( 'div' ).props
+			.previousValue;
+		expect( testValue ).toBe( 3 );
+		expect( testPreviousValue ).toBe( 2 );
+	} );
+} );

assets/js/base/hooks/use-previous.js

@@ -0,0 +1,15 @@
+import { useRef, useEffect } from 'react';
+
+/**
+ * Use Previous from https://usehooks.com/usePrevious/.
+ * @param {mixed} value
+ */
+export const usePrevious = ( value ) => {
+	const ref = useRef();
+
+	useEffect( () => {
+		ref.current = value;
+	}, [ value ] );
+
+	return ref.current;
+};

assets/js/blocks/price-filter/block.js

@@ -45,10 +45,12 @@ const PriceFilterBlock = ( { attributes } ) => {
 	const { showInputFields, showFilterButton } = attributes;
 	const minConstraint = isLoading
 		? undefined
-		: parseInt( results.min_price, 10 );
+		: // Round up to nearest 10 to match the step attribute.
+		  Math.floor( parseInt( results.min_price, 10 ) / 10 ) * 10;
 	const maxConstraint = isLoading
 		? undefined
-		: parseInt( results.max_price, 10 );
+		: // Round down to nearest 10 to match the step attribute.
+		  Math.ceil( parseInt( results.max_price, 10 ) / 10 ) * 10;
 
 	const onChange = useCallback(
 		( prices ) => {