From 33d84ef991e2ce318c10a5d2185602540cbb7bf8 Mon Sep 17 00:00:00 2001 From: Matthias Date: Thu, 12 Dec 2024 12:43:13 +0100 Subject: [PATCH] fix(Slider): bug when options are changing --- .changeset/nice-dots-pull.md | 5 +++ .../Slider/__stories__/Options.stories.tsx | 2 +- .../Slider/components/DoubleSlider.tsx | 43 ++++++++++--------- .../Slider/components/SingleSlider.tsx | 27 ++++++------ 4 files changed, 44 insertions(+), 33 deletions(-) create mode 100644 .changeset/nice-dots-pull.md diff --git a/.changeset/nice-dots-pull.md b/.changeset/nice-dots-pull.md new file mode 100644 index 0000000000..7230f20633 --- /dev/null +++ b/.changeset/nice-dots-pull.md @@ -0,0 +1,5 @@ +--- +"@ultraviolet/ui": patch +--- + +Fix `` having issue when options are changing diff --git a/packages/ui/src/components/Slider/__stories__/Options.stories.tsx b/packages/ui/src/components/Slider/__stories__/Options.stories.tsx index a31ad75622..faaecd238f 100644 --- a/packages/ui/src/components/Slider/__stories__/Options.stories.tsx +++ b/packages/ui/src/components/Slider/__stories__/Options.stories.tsx @@ -51,7 +51,7 @@ export const Options: StoryFn = args => { { - const leftSliderValue = localValue[0] === null ? min : localValue[0] - const rightSliderValue = localValue[1] === null ? max : localValue[1] - const newValues = [leftSliderValue, rightSliderValue] + const internalOnChangeRef = useCallback( + (localValue: (number | null)[]) => { + const leftSliderValue = localValue[0] === null ? min : localValue[0] + const rightSliderValue = localValue[1] === null ? max : localValue[1] + const newValues = [leftSliderValue, rightSliderValue] - setSelectedIndexes(newValues) - onChange?.([Math.min(...newValues), Math.max(...newValues)]) - }) + setSelectedIndexes(newValues) + onChange?.([Math.min(...newValues), Math.max(...newValues)]) + }, + [max, min, onChange], + ) // Get slider size (for options) useEffect(() => { @@ -199,30 +202,30 @@ export const DoubleSlider = ({ }, []) const handleMinChange = (newValue: number) => { - internalOnChangeRef.current([newValue, selectedIndexes[1]]) + internalOnChangeRef([newValue, selectedIndexes[1]]) } const handleMaxChange = (newValue: number) => { - internalOnChangeRef.current([selectedIndexes[0], newValue]) + internalOnChangeRef([selectedIndexes[0], newValue]) } const handleChangeInput = (val: number, side?: 'left' | 'right') => { if (side === 'left') { const newValue = Math.max(val, min) if (selectedIndexes[1]) { - internalOnChangeRef.current([ + internalOnChangeRef([ Math.min(newValue, selectedIndexes[1]), Math.max(newValue, selectedIndexes[1]), ]) - } else internalOnChangeRef.current([newValue, selectedIndexes[1]]) + } else internalOnChangeRef([newValue, selectedIndexes[1]]) } else if (side === 'right') { const newValue = Math.min(val, max) if (selectedIndexes[0]) { - internalOnChangeRef.current([ + internalOnChangeRef([ Math.min(newValue, selectedIndexes[0]), Math.max(newValue, selectedIndexes[0]), ]) - } else internalOnChangeRef.current([selectedIndexes[0], newValue]) + } else internalOnChangeRef([selectedIndexes[0], newValue]) } } @@ -249,9 +252,9 @@ export const DoubleSlider = ({ if (newVal !== null) { handleChangeInput(newVal, side) } else if (side === 'left') { - internalOnChangeRef.current([null, selectedIndexes[1]]) + internalOnChangeRef([null, selectedIndexes[1]]) } else if (side === 'right') { - internalOnChangeRef.current([selectedIndexes[0], null]) + internalOnChangeRef([selectedIndexes[0], null]) } }} onBlur={event => { @@ -260,15 +263,15 @@ export const DoubleSlider = ({ if (side === 'left') { const index = activeValue('left') if (index === 0) { - internalOnChangeRef.current([min, selectedIndexes[1]]) - } else internalOnChangeRef.current([selectedIndexes[0], max]) + internalOnChangeRef([min, selectedIndexes[1]]) + } else internalOnChangeRef([selectedIndexes[0], max]) } if (side === 'right') { const index = activeValue('right') if (index === 0) { - internalOnChangeRef.current([min, selectedIndexes[1]]) - } else internalOnChangeRef.current([selectedIndexes[0], max]) + internalOnChangeRef([min, selectedIndexes[1]]) + } else internalOnChangeRef([selectedIndexes[0], max]) } } }} diff --git a/packages/ui/src/components/Slider/components/SingleSlider.tsx b/packages/ui/src/components/Slider/components/SingleSlider.tsx index adf83562ba..8542084075 100644 --- a/packages/ui/src/components/Slider/components/SingleSlider.tsx +++ b/packages/ui/src/components/Slider/components/SingleSlider.tsx @@ -1,6 +1,6 @@ import { useTheme } from '@emotion/react' import styled from '@emotion/styled' -import { useEffect, useId, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' import { NumberInputV2 } from '../../NumberInputV2' import { Stack } from '../../Stack' import { Text } from '../../Text' @@ -121,21 +121,24 @@ export const SingleSlider = ({ return [] }, [options]) - const internalOnChangeRef = useRef((newValue: number) => { - setSelectedIndex(newValue ?? min) - onChange?.(newValue ?? min) - }) + const internalOnChange = useCallback( + (newValue: number) => { + setSelectedIndex(newValue ?? min) + onChange?.(newValue ?? min) + }, + [min, onChange], + ) // Make sure that min <= value <= max useEffect(() => { if (value < min) { - internalOnChangeRef.current(min) + internalOnChange(min) } else if (value > max) { - internalOnChangeRef.current(max) + internalOnChange(max) } else { setSelectedIndex(() => value ?? min) } - }, [value, max, min]) + }, [value, max, min, internalOnChange]) // Get slider size useEffect(() => { @@ -179,11 +182,11 @@ export const SingleSlider = ({ unit={typeof suffix === 'string' ? suffix : unit} onChange={newVal => { if (newVal) { - internalOnChangeRef.current(newVal) - } else internalOnChangeRef.current(0) + internalOnChange(newVal) + } else internalOnChange(0) }} onBlur={event => { - if (!event.target.value) internalOnChangeRef.current(min) + if (!event.target.value) internalOnChange(min) }} /> ) : ( @@ -246,7 +249,7 @@ export const SingleSlider = ({ type="range" value={selectedIndex} onChange={event => { - internalOnChangeRef.current(Number.parseFloat(event.target.value)) + internalOnChange(Number.parseFloat(event.target.value)) }} min={min} max={max}