From a1fccfa748c2f0d9c9045936af302c48d1ff20bb Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Fri, 10 Jul 2026 17:52:36 -0700 Subject: [PATCH] fix(studio): throttle FlatSlider commits instead of debouncing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pure debounce resets its timer on every pointermove, so a real drag (events faster than 40ms apart) never commits until the pointer pauses or lifts — killing live preview updates mid-drag. Throttle with a leading-edge commit + trailing flush instead. --- .../propertyPanelFlatPrimitives.test.tsx | 21 +++++--- .../editor/propertyPanelFlatPrimitives.tsx | 51 ++++++++++++++----- 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx index 69f25d70b..b06a8135a 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx @@ -306,8 +306,8 @@ describe("FlatSlider", () => { new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), ); }); - // Instant, un-debounced knob feedback via aria-valuenow (draft state) — - // this must update on every pointermove regardless of the commit debounce. + // Instant, un-throttled knob feedback via aria-valuenow (draft state) — + // this must update on every pointermove regardless of the commit throttle. expect(track.getAttribute("aria-valuenow")).toBe("10"); act(() => { track.dispatchEvent( @@ -327,7 +327,7 @@ describe("FlatSlider", () => { act(() => root.unmount()); }); - it("coalesces rapid drag commits to only the final value on release, not every step", () => { + it("throttles rapid drag commits to leading edge + final value on release, not every step", () => { const onCommit = vi.fn(); const { host, root } = renderInto( { value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), }); act(() => { + // pointerdown fires the leading-edge commit immediately — a live + // preview needs to move the instant the drag starts, not wait 40ms. track.dispatchEvent( new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), ); @@ -356,9 +358,12 @@ describe("FlatSlider", () => { new PointerEvent("pointermove", { bubbles: true, clientX: 100, pointerId: 1 }), ); }); - // None of the rapid intermediate positions (10, 80) have committed yet — - // only the debounce timer or the pointerup flush should ever call onCommit. - expect(onCommit).not.toHaveBeenCalled(); + // The leading-edge commit (10) fired; the rapid intermediate position (80) + // from the first pointermove never committed — it's within the 40ms + // throttle window, so only the trailing flush or the pointerup release + // gets to send the next value. + expect(onCommit).toHaveBeenCalledTimes(1); + expect(onCommit).toHaveBeenCalledWith(10); act(() => { // Real pointerup events always carry the pointer's true release position // (matches the last pointermove) — the handler recomputes from this @@ -368,8 +373,8 @@ describe("FlatSlider", () => { ); }); // Release flushes immediately with the LAST position only. - expect(onCommit).toHaveBeenCalledTimes(1); - expect(onCommit).toHaveBeenCalledWith(50); + expect(onCommit).toHaveBeenCalledTimes(2); + expect(onCommit).toHaveBeenNthCalledWith(2, 50); act(() => root.unmount()); }); diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx index dfcf801c9..8c119198f 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx @@ -241,18 +241,27 @@ export function FlatSlider({ onReset?: () => void; onCommit: (nextValue: number) => void; }) { - // Draft/debounce mirrors the legacy SliderControl (propertyPanelPrimitives.tsx): - // a real drag fires pointermove far faster than any commit should hit the - // network — `draft` gives the knob instant, drag-local feedback while the - // actual onCommit call is coalesced to the last value every 40ms, with an - // immediate flush on release so the drag never waits out the debounce. + // `draft` gives the knob instant, drag-local visual feedback. `onCommit` is + // throttled (not debounced) to at most once per 40ms: a real drag fires + // pointermove faster than that, and a pure debounce (reset the timer on + // every move) never commits until the pointer pauses or lifts — which kills + // live preview updates during a continuous drag. Throttling still fires on + // the leading edge and on a trailing timer, so the preview keeps updating + // while dragging, with an immediate flush on release for the final value. const [draft, setDraft] = useState(value); const commitTimerRef = useRef | null>(null); - const valueRef = useRef(value); - valueRef.current = value; + const lastCommitAtRef = useRef(0); + const pendingRef = useRef(null); + // Tracks the last value actually sent to onCommit — separate from `value` + // (the committed prop) because in a single pointerdown+pointerup click the + // leading-edge commit fires before the parent has re-rendered with the new + // prop, so the release flush must dedupe against what we just sent, not + // against the stale prop, or the same value commits twice. + const lastCommittedRef = useRef(value); useEffect(() => { setDraft(value); + lastCommittedRef.current = value; }, [value]); useEffect( () => () => { @@ -270,14 +279,30 @@ export function FlatSlider({ return Math.max(min, Math.min(max, stepped)); }; const commitDraft = (nextDraft: number) => { - if (commitTimerRef.current) clearTimeout(commitTimerRef.current); - if (nextDraft !== valueRef.current) onCommit(nextDraft); + if (commitTimerRef.current) { + clearTimeout(commitTimerRef.current); + commitTimerRef.current = null; + } + pendingRef.current = null; + lastCommitAtRef.current = Date.now(); + if (nextDraft !== lastCommittedRef.current) { + lastCommittedRef.current = nextDraft; + onCommit(nextDraft); + } }; const scheduleCommit = (nextDraft: number) => { - if (commitTimerRef.current) clearTimeout(commitTimerRef.current); - commitTimerRef.current = setTimeout(() => { - if (nextDraft !== valueRef.current) onCommit(nextDraft); - }, 40); + const elapsed = Date.now() - lastCommitAtRef.current; + if (elapsed >= 40) { + commitDraft(nextDraft); + return; + } + pendingRef.current = nextDraft; + if (!commitTimerRef.current) { + commitTimerRef.current = setTimeout(() => { + commitTimerRef.current = null; + if (pendingRef.current !== null) commitDraft(pendingRef.current); + }, 40 - elapsed); + } }; return (