From 0d6e4cedf107c316ca5939a50b2bc8c950e0b0fd Mon Sep 17 00:00:00 2001 From: Sh031224 Date: Tue, 15 Sep 2026 17:19:59 +0900 Subject: [PATCH 1/2] =?UTF-8?q?=20fix(wds):=20=EB=B9=A0=EB=A5=B8=20?= =?UTF-8?q?=EB=93=9C=EB=9E=98=EA=B7=B8=EC=97=90=EC=84=9C=20onValueChangeCo?= =?UTF-8?q?mplete=20=EB=88=84=EB=9D=BD=20=EC=88=98=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 값 계산이 setValues 업데이터 안에서만 일어나서 렌더가 flush 되기 전에는 결과를 알 수 없었다. 빠른 드래그는 마지막 move 와 release 를 한 배치에 담기 때문에 release 핸들러의 values 는 드래그 시작 시점 그대로였고 변경 없음으로 판정돼 콜백이 호출되지 않았다. 마지막으로 커밋한 값을 ref 로 들고 계산을 즉시 수행해 move 와 release 가 직전 결과를 보게 한다. 부수효과가 업데이터 밖으로 나오면서 StrictMode 이중 호출에서 onValueChangeComplete 가 두 번 불리던 것도 함께 해소된다. Co-Authored-By: Claude Opus 5 (1M context) --- .../wds/src/components/slider/index.test.tsx | 71 ++++++++++++- packages/wds/src/components/slider/index.tsx | 99 +++++++++++-------- 2 files changed, 126 insertions(+), 44 deletions(-) diff --git a/packages/wds/src/components/slider/index.test.tsx b/packages/wds/src/components/slider/index.test.tsx index 8ce38d01e..2584df1cb 100644 --- a/packages/wds/src/components/slider/index.test.tsx +++ b/packages/wds/src/components/slider/index.test.tsx @@ -1,4 +1,10 @@ -import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { + act, + cleanup, + fireEvent, + render, + screen, +} from '@testing-library/react'; import { axe } from 'vitest-axe'; import { @@ -17,8 +23,7 @@ const TRACK_WIDTH = 100; * coordinates `fireEvent.pointerMove` passes in — dispatch a `MouseEvent` so * `clientX` survives. */ -const firePointer = ( - element: Element, +const makePointerEvent = ( type: 'pointerdown' | 'pointermove' | 'pointerup' | 'pointercancel', clientX: number, { @@ -33,7 +38,16 @@ const firePointer = ( buttons, }); Object.defineProperty(event, 'pointerId', { value: pointerId }); - fireEvent(element, event); + return event; +}; + +const firePointer = ( + element: Element, + type: 'pointerdown' | 'pointermove' | 'pointerup' | 'pointercancel', + clientX: number, + options: { pointerId?: number; buttons?: number } = {}, +) => { + fireEvent(element, makePointerEvent(type, clientX, options)); }; const getTrack = (container: HTMLElement) => @@ -576,6 +590,55 @@ describe('when operating a slider with a pointer', () => { expect(getValues(container)).toEqual(['80']); }); + it('should report completion when the release outruns the re-render', () => { + const onValueChangeComplete = vi.fn(); + const { container } = render( + , + ); + stubTrackRect(container); + + const thumb = getThumbs(container)[0]!; + firePointer(thumb, 'pointerdown', 20); + fireEvent.focus(thumb); + + /** + * A fast drag lands its last move and its release in the same batch, so + * the pointerup handler still closes over the values from before the move. + */ + act(() => { + thumb.dispatchEvent(makePointerEvent('pointermove', 70)); + thumb.dispatchEvent(makePointerEvent('pointerup', 70)); + }); + + expect(getValues(container)).toEqual(['70']); + expect(onValueChangeComplete).toHaveBeenCalledTimes(1); + expect(onValueChangeComplete).toHaveBeenCalledWith([70]); + }); + + it('should land on the last position when a drag is batched into one frame', () => { + const { container } = render( + , + ); + stubTrackRect(container); + + const thumb = getThumbs(container)[0]!; + firePointer(thumb, 'pointerdown', 20); + fireEvent.focus(thumb); + + act(() => { + thumb.dispatchEvent(makePointerEvent('pointermove', 40)); + thumb.dispatchEvent(makePointerEvent('pointermove', 60)); + thumb.dispatchEvent(makePointerEvent('pointermove', 85)); + }); + + expect(getValues(container)).toEqual(['85']); + }); + it('should still run the consumer pointer handlers', () => { const onPointerDown = vi.fn(); const onPointerMove = vi.fn(); diff --git a/packages/wds/src/components/slider/index.tsx b/packages/wds/src/components/slider/index.tsx index 3ee07c7b2..ce1b54f51 100644 --- a/packages/wds/src/components/slider/index.tsx +++ b/packages/wds/src/components/slider/index.tsx @@ -87,6 +87,15 @@ const Slider = forwardRef< const slideStartValues = useRef(values); + /** + * `values` is a render closure, and a fast drag lands several moves — and + * its release — inside one batch, before React re-renders. Every one of + * those would compute from, and compare against, the values the drag + * started at. This ref carries what was last committed so each step sees + * the one before it. + */ + const committedValues = useRef(values); + const handleValueChange = useCallback( (nextValue: number, index: number, isCompleted = false) => { const decimalCount = nextValue.toString().split('.')[1]?.length ?? 0; @@ -98,44 +107,48 @@ const Slider = forwardRef< const calculatedNextValue = clamp(snapToStep, [min, max]); - setValues((prevValues = []) => { - const nextValues = [ - ...prevValues.slice(0, index), - calculatedNextValue, - ...prevValues.slice(index + 1), - ]; - - const stepsBetweenValue = Math.min( - ...nextValues.slice(0, -1).map((v, i) => nextValues[i + 1]! - v), - ); - - if ( - (disableSwapThumbs && stepsBetweenValue < minStepBetweenThumbs) || - (minStepBetweenThumbs > 0 && - stepsBetweenValue < minStepBetweenThumbs) - ) { - return prevValues; - } - - const sortedNextValues = [...nextValues].sort((a, b) => a - b); - - /** - * `indexOf` resolves to the leftmost slot when thumbs share a value, - * which would hand the focus over to a thumb the user never touched. - * Keep the dragged thumb where it is unless sorting actually moved it. - */ - currentFocusedIndex.current = - sortedNextValues[index] === calculatedNextValue - ? index - : sortedNextValues.indexOf(calculatedNextValue); - - const hasChanged = - sortedNextValues.toString() !== prevValues.toString(); - if (hasChanged && isCompleted) { - onValueChangeComplete?.(sortedNextValues); - } - return hasChanged ? sortedNextValues : prevValues; - }); + const prevValues = committedValues.current; + + const nextValues = [ + ...prevValues.slice(0, index), + calculatedNextValue, + ...prevValues.slice(index + 1), + ]; + + const stepsBetweenValue = Math.min( + ...nextValues.slice(0, -1).map((v, i) => nextValues[i + 1]! - v), + ); + + if ( + (disableSwapThumbs && stepsBetweenValue < minStepBetweenThumbs) || + (minStepBetweenThumbs > 0 && stepsBetweenValue < minStepBetweenThumbs) + ) { + return; + } + + const sortedNextValues = [...nextValues].sort((a, b) => a - b); + + /** + * `indexOf` resolves to the leftmost slot when thumbs share a value, + * which would hand the focus over to a thumb the user never touched. + * Keep the dragged thumb where it is unless sorting actually moved it. + */ + currentFocusedIndex.current = + sortedNextValues[index] === calculatedNextValue + ? index + : sortedNextValues.indexOf(calculatedNextValue); + + if (sortedNextValues.toString() === prevValues.toString()) { + return; + } + + committedValues.current = sortedNextValues; + + if (isCompleted) { + onValueChangeComplete?.(sortedNextValues); + } + + setValues(sortedNextValues); }, [ max, @@ -181,14 +194,19 @@ const Slider = forwardRef< * Comparing a single index misses changes whenever thumbs end up * stacked or swapped, so compare the whole set instead. */ + const finalValues = committedValues.current; const hasChanged = - slideStartValues.current.toString() !== values.toString(); + slideStartValues.current.toString() !== finalValues.toString(); if (hasChanged) { - onValueChangeComplete?.(values); + onValueChangeComplete?.(finalValues); } }; + useEffect(() => { + committedValues.current = values; + }, [values]); + const initialValuesRef = useRef(values); useEffect(() => { @@ -280,6 +298,7 @@ const Slider = forwardRef< activePointerId.current = event.pointerId; slideStartValues.current = values; + committedValues.current = values; const target = event.target as HTMLElement; target.setPointerCapture(event.pointerId); From 557be38d5885f6b82892815d11dbdf19d6f64fa5 Mon Sep 17 00:00:00 2001 From: Sh031224 Date: Tue, 15 Sep 2026 17:39:39 +0900 Subject: [PATCH 2/2] =?UTF-8?q?=20fix(wds):=20=EB=93=9C=EB=9E=98=EA=B7=B8?= =?UTF-8?q?=20=EC=A4=91=20controlled=20=EB=A6=AC=EB=A0=8C=EB=8D=94?= =?UTF-8?q?=EA=B0=80=20=EC=A7=84=ED=96=89=EA=B0=92=EC=9D=84=20=EB=8D=AE?= =?UTF-8?q?=EC=96=B4=EC=93=B0=EC=A7=80=20=EC=95=8A=EB=8F=84=EB=A1=9D=20?= =?UTF-8?q?=EC=88=98=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit committedValues 동기화 effect 가 values 가 바뀔 때마다 무조건 ref 를 덮어썼다. controlled Slider 의 부모가 아직 반영하지 않은 값으로 리렌더하면 드래그 중에 ref 가 시작값으로 되돌아가 release 시점에 변경 없음으로 판정돼 onValueChangeComplete 가 다시 누락됐다. 활성 포인터가 있는 동안에는 동기화를 건너뛰고 드래그가 끝난 뒤 다시 맞춘다. 코드리뷰 지적 반영. Co-Authored-By: Claude Opus 5 (1M context) --- .../wds/src/components/slider/index.test.tsx | 27 +++++++++++++++++++ packages/wds/src/components/slider/index.tsx | 10 ++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/wds/src/components/slider/index.test.tsx b/packages/wds/src/components/slider/index.test.tsx index 2584df1cb..40639b3b4 100644 --- a/packages/wds/src/components/slider/index.test.tsx +++ b/packages/wds/src/components/slider/index.test.tsx @@ -639,6 +639,33 @@ describe('when operating a slider with a pointer', () => { expect(getValues(container)).toEqual(['85']); }); + it('should survive a controlled re-render that lands mid-drag', () => { + const onValueChangeComplete = vi.fn(); + const props = { + min: 0, + max: 100, + onValueChangeComplete, + }; + const { container, rerender } = render(); + stubTrackRect(container); + + const thumb = getThumbs(container)[0]!; + firePointer(thumb, 'pointerdown', 20); + fireEvent.focus(thumb); + firePointer(thumb, 'pointermove', 70); + + /** + * A controlled parent has not applied the change yet, but re-renders for + * its own reasons — handing back a fresh array holding the old value. + */ + rerender(); + + firePointer(thumb, 'pointerup', 70); + + expect(onValueChangeComplete).toHaveBeenCalledTimes(1); + expect(onValueChangeComplete).toHaveBeenCalledWith([70]); + }); + it('should still run the consumer pointer handlers', () => { const onPointerDown = vi.fn(); const onPointerMove = vi.fn(); diff --git a/packages/wds/src/components/slider/index.tsx b/packages/wds/src/components/slider/index.tsx index ce1b54f51..2c34fb39d 100644 --- a/packages/wds/src/components/slider/index.tsx +++ b/packages/wds/src/components/slider/index.tsx @@ -203,9 +203,17 @@ const Slider = forwardRef< } }; + /** + * Outside a drag the ref only mirrors what is rendered, so a controlled + * update or a form reset reaches it. During one it must not: a controlled + * parent can re-render with the value it has not applied yet, which would + * hand the drag back its own starting point and swallow the completion. + */ useEffect(() => { + if (activePointerId.current !== null) return; + committedValues.current = values; - }, [values]); + }); const initialValuesRef = useRef(values);