From 388938b09991a653237c22fce32762d73b8c9e02 Mon Sep 17 00:00:00 2001 From: Xavier Donnellon Date: Tue, 1 Aug 2023 13:14:12 -0500 Subject: [PATCH 1/5] fix(rangeslider): fix mouse/value attachment logic with debounce interval --- .../RangeSlider/RangeSlider.stories.tsx | 4 +- src/components/RangeSlider/RangeSlider.tsx | 65 ++++++++++++------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/components/RangeSlider/RangeSlider.stories.tsx b/src/components/RangeSlider/RangeSlider.stories.tsx index 7d620af4d..4f7724a66 100644 --- a/src/components/RangeSlider/RangeSlider.stories.tsx +++ b/src/components/RangeSlider/RangeSlider.stories.tsx @@ -104,8 +104,8 @@ export const Default: Story = ({ min={min} max={max} debounceInterval={debounceInterval} - onDebounceChange={newVal => { - action('onDebounceChange')(newVal); + onChange={newVal => { + action('onChange')(newVal); setVal(Math.round(newVal)); }} dragHandleAttachment={dragHandleAttachment} diff --git a/src/components/RangeSlider/RangeSlider.tsx b/src/components/RangeSlider/RangeSlider.tsx index 5d526ef64..d8409d928 100644 --- a/src/components/RangeSlider/RangeSlider.tsx +++ b/src/components/RangeSlider/RangeSlider.tsx @@ -311,9 +311,13 @@ export const RangeSlider = ({ const [ref, sliderBounds] = useMeasure({ polyfill: ResizeObserver }); - const pixelPositions = processedValues.map(val => { - return (val.value / domain) * sliderBounds.width; - }); + const pixelPositions = useMemo( + () => + processedValues.map(val => { + return (val.value / domain) * sliderBounds.width; + }), + [processedValues, sliderBounds, domain], + ); // get the x offset and an animation setter function const [{ dragHandleX }, springRef] = useSpring(() => ({ @@ -378,25 +382,29 @@ export const RangeSlider = ({ }; const bind = useDrag( - ({ down, movement: [deltaX] }) => { + ({ down: isDragging, movement: [deltaX] }) => { if (readonly) return; const delta = (deltaX / sliderBounds.width) * domain; valueBuffer.current = clamp(delta, min, max); - setDraggedHandle(down ? 0 : -1); + setDraggedHandle(isDragging ? 0 : -1); handleDrag(valueBuffer.current); - let animate = true; - if (prefersReducedMotion) animate = false; - if (!snapToValue) animate = springOnRelease ? !down : false; - - springRef.start({ - // Should handle follow value or drag gesture? - dragHandleX: snapToValue || !down ? (pixelPositions ?? [0])[0] : deltaX, - - immediate: !animate, - config: { friction: 13, tension: 100 }, - }); + if (dragHandleAttachment === 'mouse') { + if (isDragging) { + // constantly follow mouse during drag + springRef.start({ + dragHandleX: deltaX, + immediate: true, + }); + } else { + // after drag release, spring to value + springRef.start({ + dragHandleX: pixelPositions[0], + immediate: !springOnRelease, + }); + } + } }, { initial: [(pixelPositions ?? [0])[0], 0], @@ -411,23 +419,32 @@ export const RangeSlider = ({ }, ); - // Snap to value on initial load and when pixelPositions changes (on click) + // Once sliderBounds are read, set initial position useEffect(() => { - if (draggedHandle >= 0) return; + if (isInitializing.current && sliderBounds.width) { + springRef.start({ + dragHandleX: pixelPositions[0], + immediate: true, + onResolve: () => { + isInitializing.current = false; + }, + }); + } + }, [springRef, sliderBounds, isInitializing, pixelPositions]); - if (sliderBounds.x) { + // For snap to value, listen to changes in value and always animate to value + useEffect(() => { + if (snapToValue) { springRef.start({ dragHandleX: pixelPositions[0], - immediate: prefersReducedMotion || isInitializing.current, + immediate: prefersReducedMotion, config: { friction: 13, tension: 100 }, - onResolve: () => { - if (isInitializing) isInitializing.current = false; - }, }); } - }, [springRef, pixelPositions, draggedHandle, prefersReducedMotion, sliderBounds]); + }, [snapToValue, springRef, pixelPositions, prefersReducedMotion, sliderBounds]); + // Dispose of debounce timers useEffect(() => { return () => { debouncedOnChange.cancel(); From 6e47fb60145d5437156ce246a844cc32b293c10a Mon Sep 17 00:00:00 2001 From: Xavier Donnellon Date: Tue, 1 Aug 2023 13:17:12 -0500 Subject: [PATCH 2/5] remove unneccessary spring settings --- src/components/RangeSlider/RangeSlider.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/RangeSlider/RangeSlider.tsx b/src/components/RangeSlider/RangeSlider.tsx index d8409d928..e10dc00d6 100644 --- a/src/components/RangeSlider/RangeSlider.tsx +++ b/src/components/RangeSlider/RangeSlider.tsx @@ -437,9 +437,7 @@ export const RangeSlider = ({ if (snapToValue) { springRef.start({ dragHandleX: pixelPositions[0], - immediate: prefersReducedMotion, - config: { friction: 13, tension: 100 }, }); } }, [snapToValue, springRef, pixelPositions, prefersReducedMotion, sliderBounds]); From b32eb8ad7c7f1d48fb288bd73034cc372149edc0 Mon Sep 17 00:00:00 2001 From: Xavier Donnellon Date: Tue, 1 Aug 2023 13:18:47 -0500 Subject: [PATCH 3/5] remove unncessary useMemo --- src/components/RangeSlider/RangeSlider.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/RangeSlider/RangeSlider.tsx b/src/components/RangeSlider/RangeSlider.tsx index e10dc00d6..8a2d1ee02 100644 --- a/src/components/RangeSlider/RangeSlider.tsx +++ b/src/components/RangeSlider/RangeSlider.tsx @@ -311,13 +311,9 @@ export const RangeSlider = ({ const [ref, sliderBounds] = useMeasure({ polyfill: ResizeObserver }); - const pixelPositions = useMemo( - () => - processedValues.map(val => { - return (val.value / domain) * sliderBounds.width; - }), - [processedValues, sliderBounds, domain], - ); + const pixelPositions = processedValues.map(val => { + return (val.value / domain) * sliderBounds.width; + }); // get the x offset and an animation setter function const [{ dragHandleX }, springRef] = useSpring(() => ({ From 3998df74ee3fa6dc6d58beab36d310dcb2eb04c3 Mon Sep 17 00:00:00 2001 From: Xavier Donnellon Date: Tue, 1 Aug 2023 13:21:00 -0500 Subject: [PATCH 4/5] remove snapToValue bool --- src/components/RangeSlider/RangeSlider.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/RangeSlider/RangeSlider.tsx b/src/components/RangeSlider/RangeSlider.tsx index 8a2d1ee02..9ba12ffb4 100644 --- a/src/components/RangeSlider/RangeSlider.tsx +++ b/src/components/RangeSlider/RangeSlider.tsx @@ -239,8 +239,6 @@ export const RangeSlider = ({ ); } - const snapToValue = dragHandleAttachment === 'value'; - const { prefersReducedMotion } = useAccessibilityPreferences(); const isInitializing = useRef(true); @@ -430,13 +428,13 @@ export const RangeSlider = ({ // For snap to value, listen to changes in value and always animate to value useEffect(() => { - if (snapToValue) { + if (dragHandleAttachment === 'value') { springRef.start({ dragHandleX: pixelPositions[0], immediate: prefersReducedMotion, }); } - }, [snapToValue, springRef, pixelPositions, prefersReducedMotion, sliderBounds]); + }, [dragHandleAttachment, springRef, pixelPositions, prefersReducedMotion, sliderBounds]); // Dispose of debounce timers useEffect(() => { From 3de98779353bbdd5ce48e79184ea5f6aad665658 Mon Sep 17 00:00:00 2001 From: Xavier Donnellon Date: Tue, 1 Aug 2023 14:02:34 -0500 Subject: [PATCH 5/5] fix clicks, add animate flag --- .../RangeSlider/RangeSlider.stories.tsx | 9 ++---- src/components/RangeSlider/RangeSlider.tsx | 30 ++++++++++++++----- src/components/RangeSlider/types.ts | 4 ++- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/components/RangeSlider/RangeSlider.stories.tsx b/src/components/RangeSlider/RangeSlider.stories.tsx index 4f7724a66..50b62d760 100644 --- a/src/components/RangeSlider/RangeSlider.stories.tsx +++ b/src/components/RangeSlider/RangeSlider.stories.tsx @@ -69,12 +69,12 @@ export const Default: Story = ({ disabled, showDomainLabels, showSelectedRange, - springOnRelease, min, max, dragHandleAttachment, readonly, debounceInterval, + animated, }: DefaultProps) => { const [val, setVal] = useState(value); @@ -100,7 +100,7 @@ export const Default: Story = ({ readonly={readonly} showDomainLabels={showDomainLabels} showSelectedRange={showSelectedRange} - springOnRelease={springOnRelease} + animated={animated} min={min} max={max} debounceInterval={debounceInterval} @@ -126,9 +126,9 @@ Default.args = { showDomainLabels: false, showHandleLabels: true, showSelectedRange: true, - springOnRelease: true, dragHandleAttachment: 'value', debounceInterval: 10, + animated: true, }; type RatingProps = Omit & { @@ -140,7 +140,6 @@ export const Rating: Story = ({ disabled, showDomainLabels, showSelectedRange, - springOnRelease, min, max, }: RatingProps) => { @@ -154,7 +153,6 @@ export const Rating: Story = ({ disabled={disabled} showDomainLabels={showDomainLabels} showSelectedRange={showSelectedRange} - springOnRelease={springOnRelease} min={min} max={max} onChange={newVal => { @@ -177,7 +175,6 @@ Rating.args = { disabled: false, showDomainLabels: false, showSelectedRange: false, - springOnRelease: true, min: 0, max: 5, }; diff --git a/src/components/RangeSlider/RangeSlider.tsx b/src/components/RangeSlider/RangeSlider.tsx index 9ba12ffb4..60c2d30db 100644 --- a/src/components/RangeSlider/RangeSlider.tsx +++ b/src/components/RangeSlider/RangeSlider.tsx @@ -215,7 +215,8 @@ export const RangeSlider = ({ showSelectedRange = true, showHandleLabels = true, - springOnRelease = true, + springOnRelease, + animated = true, debounceInterval = 8, onDrag, @@ -238,6 +239,13 @@ export const RangeSlider = ({ 'From FoundryUI RangerSlider: onDrag callback is deprecated. Instead, use onChange or onDebounceChange.', ); } + if (springOnRelease !== undefined) { + animated = springOnRelease; + // eslint-disable-next-line no-console + console.warn( + 'From FoundryUI RangeSlider: springOnRelease is deprecated. Instead, use animated.', + ); + } const { prefersReducedMotion } = useAccessibilityPreferences(); const isInitializing = useRef(true); @@ -318,7 +326,6 @@ export const RangeSlider = ({ to: { dragHandleX: 0 }, friction: 13, tension: 100, - immediate: prefersReducedMotion, })); const handleSlideRailClick = useCallback( @@ -370,6 +377,7 @@ export const RangeSlider = ({ processedValues, ], ); + const handleSlideRailClickWithAnalytics = (e: any) => { if (readonly) return; handleEventWithAnalytics('RangeSlider', handleSlideRailClick, 'onClick', e, containerProps); @@ -395,7 +403,7 @@ export const RangeSlider = ({ // after drag release, spring to value springRef.start({ dragHandleX: pixelPositions[0], - immediate: !springOnRelease, + immediate: prefersReducedMotion || !animated, }); } } @@ -426,15 +434,23 @@ export const RangeSlider = ({ } }, [springRef, sliderBounds, isInitializing, pixelPositions]); - // For snap to value, listen to changes in value and always animate to value + // For snap to value, listen to changes in value and always animate to value. Also listens to clicks useEffect(() => { - if (dragHandleAttachment === 'value') { + if (dragHandleAttachment === 'value' || draggedHandle === -1) { springRef.start({ dragHandleX: pixelPositions[0], - immediate: prefersReducedMotion, + immediate: prefersReducedMotion || !animated, }); } - }, [dragHandleAttachment, springRef, pixelPositions, prefersReducedMotion, sliderBounds]); + }, [ + dragHandleAttachment, + springRef, + pixelPositions, + prefersReducedMotion, + sliderBounds, + animated, + draggedHandle, + ]); // Dispose of debounce timers useEffect(() => { diff --git a/src/components/RangeSlider/types.ts b/src/components/RangeSlider/types.ts index 833620a61..3bebe0f44 100644 --- a/src/components/RangeSlider/types.ts +++ b/src/components/RangeSlider/types.ts @@ -64,8 +64,8 @@ export type RangeSliderProps = { showDomainLabels?: boolean; showSelectedRange?: boolean; showHandleLabels?: boolean; + animated?: boolean; - springOnRelease?: boolean; /** Debounce interval (in ms) before calling `onDebounceChange`. */ debounceInterval?: number; @@ -88,6 +88,8 @@ export type RangeSliderProps = { */ dragHandleAttachment?: 'value' | 'mouse'; + /** @deprecated use `animated` instead. */ + springOnRelease?: boolean; /** @deprecated use onChange or onChangeDebounce instead. */ onDrag?: (val: number) => void; /** @deprecated do not use. */