From c824b481e203de1d33c6d95f949cef32469c721f Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Wed, 6 Nov 2019 18:25:37 +0000 Subject: [PATCH] Fix price slider constraints (https://github.com/woocommerce/woocommerce-blocks/pull/1125) * Fix price slider constraints * Update inline comments * use previous tests --- .../js/base/components/price-slider/index.js | 30 ++++++++-- .../assets/js/base/hooks/index.js | 1 + .../assets/js/base/hooks/test/use-previous.js | 57 +++++++++++++++++++ .../assets/js/base/hooks/use-previous.js | 15 +++++ .../assets/js/blocks/price-filter/block.js | 6 +- 5 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 plugins/woocommerce-blocks/assets/js/base/hooks/test/use-previous.js create mode 100644 plugins/woocommerce-blocks/assets/js/base/hooks/use-previous.js diff --git a/plugins/woocommerce-blocks/assets/js/base/components/price-slider/index.js b/plugins/woocommerce-blocks/assets/js/base/components/price-slider/index.js index d0521f90b80..8310106a090 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/price-slider/index.js +++ b/plugins/woocommerce-blocks/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. diff --git a/plugins/woocommerce-blocks/assets/js/base/hooks/index.js b/plugins/woocommerce-blocks/assets/js/base/hooks/index.js index 8f22f0027e0..475e2f58a61 100644 --- a/plugins/woocommerce-blocks/assets/js/base/hooks/index.js +++ b/plugins/woocommerce-blocks/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'; diff --git a/plugins/woocommerce-blocks/assets/js/base/hooks/test/use-previous.js b/plugins/woocommerce-blocks/assets/js/base/hooks/test/use-previous.js new file mode 100644 index 00000000000..df2baaf046d --- /dev/null +++ b/plugins/woocommerce-blocks/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
; + }; + + let renderer; + beforeEach( () => ( renderer = null ) ); + + it( 'should be undefined at first pass', () => { + act( () => { + renderer = TestRenderer.create( ); + } ); + 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( ); + } ); + + act( () => { + renderer.update( ); + } ); + 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( ); + } ); + testValue = renderer.root.findByType( 'div' ).props.testValue; + testPreviousValue = renderer.root.findByType( 'div' ).props + .previousValue; + expect( testValue ).toBe( 3 ); + expect( testPreviousValue ).toBe( 2 ); + } ); +} ); diff --git a/plugins/woocommerce-blocks/assets/js/base/hooks/use-previous.js b/plugins/woocommerce-blocks/assets/js/base/hooks/use-previous.js new file mode 100644 index 00000000000..d0580e96fcc --- /dev/null +++ b/plugins/woocommerce-blocks/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; +}; diff --git a/plugins/woocommerce-blocks/assets/js/blocks/price-filter/block.js b/plugins/woocommerce-blocks/assets/js/blocks/price-filter/block.js index d56509b3b5e..d8f8f26e763 100644 --- a/plugins/woocommerce-blocks/assets/js/blocks/price-filter/block.js +++ b/plugins/woocommerce-blocks/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 ) => {