From 0d3a6710a3a59eabd4b3fc90669849ea3de5ec34 Mon Sep 17 00:00:00 2001 From: David Lyon Date: Tue, 27 Feb 2024 10:56:54 -0800 Subject: [PATCH 1/3] Fix slider debouncing, fix slider steps --- src/features/collections/CollectionDetail.tsx | 76 ++++++++++++++----- src/features/common.ts | 2 +- 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/src/features/collections/CollectionDetail.tsx b/src/features/collections/CollectionDetail.tsx index 5dd20659..c9db15a1 100644 --- a/src/features/collections/CollectionDetail.tsx +++ b/src/features/collections/CollectionDetail.tsx @@ -6,7 +6,7 @@ import { } from '../../common/api/collectionsApi'; import { usePageTitle } from '../layout/layoutSlice'; import styles from './Collections.module.scss'; -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { DataProduct } from './DataProduct'; import { snakeCaseToHumanReadable } from '../../common/utils/stringUtils'; import { MATCHER_LABELS, MatchModal } from './MatchModal'; @@ -37,6 +37,7 @@ import { Slider, MenuItem, TextField, Stack, Divider } from '@mui/material'; import { useForm } from 'react-hook-form'; import { CollectionOverview } from './CollectionOverview'; import { FilterChip } from '../../common/components/FilterChip'; +import { noOp } from '../common'; export const detailPath = ':id'; export const detailDataProductPath = ':id/:data_product'; @@ -440,9 +441,12 @@ const DateRangeFilterControls = ({ const { error: minError } = getFieldState('min', formState); const { error: maxError } = getFieldState('max', formState); const dispatch = useAppDispatch(); - const sliderTimeout = useRef(); + const [sliderPosition, setSliderPosition] = useState<[string, string]>([ + values.min, + values.max, + ]); - const setFilterRange = () => { + const setFilterRange = useCallback(() => { const validState = getValues(); const shouldClear = parseDate(validState.min) === filter.min_value && @@ -465,12 +469,27 @@ const DateRangeFilterControls = ({ ]) ); } - }; + }, [collectionId, column, context, dispatch, filter, getValues]); const submit = handleSubmit(() => { setFilterRange(); }); + useEffect(() => { + //Set slider position from values + setSliderPosition([values.min, values.max]); + }, [values.min, values.max]); + const [sliderMin, sliderMax] = sliderPosition; + useEffect(() => { + // Debounce setting the filter state from the slider for better UX + const sliderTimeout = window.setTimeout(() => { + setFilterRange(); + noOp(sliderMin, sliderMax); + }, 100); + return () => { + clearTimeout(sliderTimeout); + }; + }, [sliderMin, sliderMax, setFilterRange]); return ( @@ -499,9 +518,10 @@ const DateRangeFilterControls = ({ size="small" disableSwap getAriaLabel={() => `filter range for column ${column}`} - value={[parseDate(values.min), parseDate(values.max)]} + value={sliderPosition.map(parseDate)} min={filter.min_value} max={filter.max_value} + step={(filter.max_value - filter.min_value) / 100} valueLabelFormat={formatDate} marks={[filter.min_value, filter.max_value].map((v) => ({ value: v, @@ -511,11 +531,11 @@ const DateRangeFilterControls = ({ const range = newValue as [number, number]; setValue('min', formatDate(range[0])); setValue('max', formatDate(range[1])); - // Debounce setting the filter state from the slider for better UX - if (sliderTimeout.current) clearTimeout(sliderTimeout.current); - sliderTimeout.current = window.setTimeout(() => { - submit(); - }, 100); + const sliderRange: [string, string] = [ + formatDate(range[0]), + formatDate(range[1]), + ]; + setSliderPosition(sliderRange); }} valueLabelDisplay="auto" /> @@ -551,9 +571,12 @@ const RangeFilterControls = ({ const { error: minError } = getFieldState('min', formState); const { error: maxError } = getFieldState('max', formState); const dispatch = useAppDispatch(); - const sliderTimeout = useRef(); + const [sliderPosition, setSliderPosition] = useState<[number, number]>([ + values.min, + values.max, + ]); - const setFilterRange = () => { + const setFilterRange = useCallback(() => { const validState = getValues(); const shouldClear = validState.min === filter.min_value && @@ -576,12 +599,30 @@ const RangeFilterControls = ({ ]) ); } - }; + }, [collectionId, column, context, dispatch, filter, getValues]); const submit = handleSubmit(() => { setFilterRange(); }); + useEffect(() => { + //Set slider position from values + setSliderPosition([values.min, values.max]); + }, [values.min, values.max]); + + const [sliderMin, sliderMax] = sliderPosition; + useEffect(() => { + // Debounce setting the filter state from the slider for better UX + const sliderTimeout = window.setTimeout(() => { + setFilterRange(); + }, 100); + // Extra deps to trigger setFilterRange (noOp) + noOp(sliderMin, sliderMax); + return () => { + clearTimeout(sliderTimeout); + }; + }, [sliderMin, sliderMax, setFilterRange]); + return ( @@ -614,9 +655,10 @@ const RangeFilterControls = ({ size="small" disableSwap getAriaLabel={() => `filter range for column ${column}`} - value={[values.min, values.max]} + value={sliderPosition} min={filter.min_value} max={filter.max_value} + step={(filter.max_value - filter.min_value) / 100} marks={[filter.min_value, filter.max_value].map((v) => ({ value: v, label: v, @@ -625,11 +667,7 @@ const RangeFilterControls = ({ const range = newValue as [number, number]; setValue('min', range[0]); setValue('max', range[1]); - // Debounce setting the filter state from the slider for better UX - if (sliderTimeout.current) clearTimeout(sliderTimeout.current); - sliderTimeout.current = window.setTimeout(() => { - submit(); - }, 100); + setSliderPosition([range[0], range[1]]); }} valueLabelDisplay="auto" /> diff --git a/src/features/common.ts b/src/features/common.ts index 62bf0bf1..cabe383f 100644 --- a/src/features/common.ts +++ b/src/features/common.ts @@ -2,6 +2,6 @@ export const realname = 'Rosalind Franklin'; export const usernameRequested = 'rosalind_franklin'; export const realnameOther = 'Dorothy Hodgkin'; export const usernameOtherRequested = 'dorothy_hodgkin'; -export const noOp = () => { +export const noOp = (...args: unknown[]) => { /**noOp */ }; From 45eed0bc4dd9e134e372720b409b3dba19a96d91 Mon Sep 17 00:00:00 2001 From: David Lyon Date: Tue, 27 Feb 2024 11:39:51 -0800 Subject: [PATCH 2/3] fix filter clear issues --- src/features/collections/CollectionDetail.tsx | 58 ++++++++++--------- src/features/common.ts | 2 +- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/features/collections/CollectionDetail.tsx b/src/features/collections/CollectionDetail.tsx index c9db15a1..f2ef804b 100644 --- a/src/features/collections/CollectionDetail.tsx +++ b/src/features/collections/CollectionDetail.tsx @@ -6,7 +6,7 @@ import { } from '../../common/api/collectionsApi'; import { usePageTitle } from '../layout/layoutSlice'; import styles from './Collections.module.scss'; -import { useCallback, useEffect, useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { DataProduct } from './DataProduct'; import { snakeCaseToHumanReadable } from '../../common/utils/stringUtils'; import { MATCHER_LABELS, MatchModal } from './MatchModal'; @@ -37,7 +37,6 @@ import { Slider, MenuItem, TextField, Stack, Divider } from '@mui/material'; import { useForm } from 'react-hook-form'; import { CollectionOverview } from './CollectionOverview'; import { FilterChip } from '../../common/components/FilterChip'; -import { noOp } from '../common'; export const detailPath = ':id'; export const detailDataProductPath = ':id/:data_product'; @@ -441,12 +440,13 @@ const DateRangeFilterControls = ({ const { error: minError } = getFieldState('min', formState); const { error: maxError } = getFieldState('max', formState); const dispatch = useAppDispatch(); + const sliderTimeout = useRef(); const [sliderPosition, setSliderPosition] = useState<[string, string]>([ values.min, values.max, ]); - const setFilterRange = useCallback(() => { + const setFilterRange = () => { const validState = getValues(); const shouldClear = parseDate(validState.min) === filter.min_value && @@ -469,27 +469,27 @@ const DateRangeFilterControls = ({ ]) ); } - }, [collectionId, column, context, dispatch, filter, getValues]); + }; const submit = handleSubmit(() => { setFilterRange(); }); + useEffect(() => { //Set slider position from values setSliderPosition([values.min, values.max]); }, [values.min, values.max]); - const [sliderMin, sliderMax] = sliderPosition; + const [filterMin, filterMax] = [filter.min_value, filter.max_value]; useEffect(() => { - // Debounce setting the filter state from the slider for better UX - const sliderTimeout = window.setTimeout(() => { - setFilterRange(); - noOp(sliderMin, sliderMax); - }, 100); - return () => { - clearTimeout(sliderTimeout); - }; - }, [sliderMin, sliderMax, setFilterRange]); + //Clear when the filter is cleared + if (!filter.value) { + setValue('min', formatDate(filterMin)); + setValue('max', formatDate(filterMax)); + setSliderPosition([formatDate(filterMin), formatDate(filterMax)]); + } + }, [filter.value, filterMax, filterMin, setValue]); + return ( @@ -536,6 +536,9 @@ const DateRangeFilterControls = ({ formatDate(range[1]), ]; setSliderPosition(sliderRange); + const thisTimeout = (sliderTimeout.current = window.setTimeout(() => { + if (sliderTimeout.current === thisTimeout) submit(); + }, 100)); }} valueLabelDisplay="auto" /> @@ -571,12 +574,13 @@ const RangeFilterControls = ({ const { error: minError } = getFieldState('min', formState); const { error: maxError } = getFieldState('max', formState); const dispatch = useAppDispatch(); + const sliderTimeout = useRef(); const [sliderPosition, setSliderPosition] = useState<[number, number]>([ values.min, values.max, ]); - const setFilterRange = useCallback(() => { + const setFilterRange = () => { const validState = getValues(); const shouldClear = validState.min === filter.min_value && @@ -599,7 +603,7 @@ const RangeFilterControls = ({ ]) ); } - }, [collectionId, column, context, dispatch, filter, getValues]); + }; const submit = handleSubmit(() => { setFilterRange(); @@ -610,18 +614,15 @@ const RangeFilterControls = ({ setSliderPosition([values.min, values.max]); }, [values.min, values.max]); - const [sliderMin, sliderMax] = sliderPosition; + const [filterMin, filterMax] = [filter.min_value, filter.max_value]; useEffect(() => { - // Debounce setting the filter state from the slider for better UX - const sliderTimeout = window.setTimeout(() => { - setFilterRange(); - }, 100); - // Extra deps to trigger setFilterRange (noOp) - noOp(sliderMin, sliderMax); - return () => { - clearTimeout(sliderTimeout); - }; - }, [sliderMin, sliderMax, setFilterRange]); + //Clear when the filter is cleared + if (!filter.value) { + setValue('min', filterMin); + setValue('max', filterMax); + setSliderPosition([filterMin, filterMax]); + } + }, [filter.value, filterMax, filterMin, setValue]); return ( @@ -668,6 +669,9 @@ const RangeFilterControls = ({ setValue('min', range[0]); setValue('max', range[1]); setSliderPosition([range[0], range[1]]); + const thisTimeout = (sliderTimeout.current = window.setTimeout(() => { + if (sliderTimeout.current === thisTimeout) submit(); + }, 100)); }} valueLabelDisplay="auto" /> diff --git a/src/features/common.ts b/src/features/common.ts index cabe383f..62bf0bf1 100644 --- a/src/features/common.ts +++ b/src/features/common.ts @@ -2,6 +2,6 @@ export const realname = 'Rosalind Franklin'; export const usernameRequested = 'rosalind_franklin'; export const realnameOther = 'Dorothy Hodgkin'; export const usernameOtherRequested = 'dorothy_hodgkin'; -export const noOp = (...args: unknown[]) => { +export const noOp = () => { /**noOp */ }; From dd5aa8b58b99acfa6c90394b5c0e2991853f6fb8 Mon Sep 17 00:00:00 2001 From: David Lyon Date: Tue, 27 Feb 2024 12:25:03 -0800 Subject: [PATCH 3/3] integer slider step should be 1 --- src/features/collections/CollectionDetail.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/features/collections/CollectionDetail.tsx b/src/features/collections/CollectionDetail.tsx index f2ef804b..1e1ca977 100644 --- a/src/features/collections/CollectionDetail.tsx +++ b/src/features/collections/CollectionDetail.tsx @@ -659,7 +659,11 @@ const RangeFilterControls = ({ value={sliderPosition} min={filter.min_value} max={filter.max_value} - step={(filter.max_value - filter.min_value) / 100} + step={ + filter.type === 'int' + ? 1 + : (filter.max_value - filter.min_value) / 100 + } marks={[filter.min_value, filter.max_value].map((v) => ({ value: v, label: v,