From 567b0aa017cee9705a7487e113884a7591d957cb Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Fri, 10 Jul 2026 17:23:17 -0700 Subject: [PATCH] fix(studio): debounce flat slider drag commits to prevent rapid-fire writes --- ...pertyPanelFlatColorGradingSection.test.tsx | 1 + .../propertyPanelFlatMediaSection.test.tsx | 7 ++- .../propertyPanelFlatPrimitives.test.tsx | 62 +++++++++++++++++-- .../editor/propertyPanelFlatPrimitives.tsx | 55 +++++++++++++--- .../propertyPanelFlatStyleSections.test.tsx | 4 ++ 5 files changed, 114 insertions(+), 15 deletions(-) diff --git a/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx index 73ef2a06f..bf128ca0a 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx @@ -52,6 +52,7 @@ function dragSliderTrack(row: Element, clientX: number, trackWidth: number) { }); act(() => { track.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX })); + track.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX })); }); } diff --git a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx index cce0af420..125f5a183 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.test.tsx @@ -155,7 +155,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => { it("commits a new volume value on slider track pointerdown", () => { const onSetAttribute = vi.fn(); - const element = makeVideoElement({ dataAttributes: { volume: "0.5" } }); + const element = makeVideoElement({ dataAttributes: { volume: "0.2" } }); const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); @@ -177,8 +177,9 @@ describe("FlatMediaSection — volume/rate/media-start", () => { }); act(() => { volumeTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 })); + volumeTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 })); }); - // min=0, max=100, ratio=0.5 -> raw=50 -> commit(50) -> 50/100=0.5 -> "0.5" + // starting volume 0.2 (draft=20); min=0, max=100, ratio=0.5 -> raw=50 -> commit(50) -> 50/100=0.5 -> "0.5" expect(onSetAttribute).toHaveBeenCalledWith("volume", "0.5"); act(() => root.unmount()); }); @@ -207,6 +208,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => { }); act(() => { rateTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 100 })); + rateTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 100 })); }); // min=25, max=300, ratio=1.0 -> raw=300 -> commit(300) -> 300/100=3 -> "3" expect(onSetAttribute).toHaveBeenCalledWith("playback-rate", "3"); @@ -237,6 +239,7 @@ describe("FlatMediaSection — volume/rate/media-start", () => { }); act(() => { mediaStartTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 100 })); + mediaStartTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 100 })); }); // no source-duration set -> mediaStartMax=Math.max(30, Math.ceil(0+10))=30 -> max=3000 // ratio=1.0 -> raw=3000 -> commit(3000) -> (3000/100).toFixed(2) = "30.00" diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx index 0e3d5eb92..69f25d70b 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.test.tsx @@ -234,11 +234,11 @@ describe("FlatSlider", () => { const { host, root } = renderInto( , ); @@ -249,6 +249,7 @@ describe("FlatSlider", () => { }); act(() => { track.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 100 })); + track.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 100 })); }); expect(onCommit).toHaveBeenCalledWith(50); act(() => root.unmount()); @@ -276,12 +277,13 @@ describe("FlatSlider", () => { track.dispatchEvent( new MouseEvent("pointerdown", { bubbles: true, clientX: 20, clientY: 18 }), ); + track.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 20 })); }); expect(onCommit).toHaveBeenCalledWith(10); act(() => root.unmount()); }); - it("commits continuously while dragging, not just on the initial pointerdown", () => { + it("tracks the knob instantly on every pointermove during a drag (draft state)", () => { const onCommit = vi.fn(); const { host, root } = renderInto( { new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), ); }); - expect(onCommit).toHaveBeenLastCalledWith(10); + // Instant, un-debounced knob feedback via aria-valuenow (draft state) — + // this must update on every pointermove regardless of the commit debounce. + expect(track.getAttribute("aria-valuenow")).toBe("10"); act(() => { track.dispatchEvent( new PointerEvent("pointermove", { bubbles: true, clientX: 160, pointerId: 1 }), ); }); - expect(onCommit).toHaveBeenLastCalledWith(80); + expect(track.getAttribute("aria-valuenow")).toBe("80"); act(() => { track.dispatchEvent( new PointerEvent("pointermove", { bubbles: true, clientX: 100, pointerId: 1 }), ); }); - expect(onCommit).toHaveBeenLastCalledWith(50); + expect(track.getAttribute("aria-valuenow")).toBe("50"); act(() => { track.dispatchEvent(new PointerEvent("pointerup", { bubbles: true, pointerId: 1 })); }); act(() => root.unmount()); }); + it("coalesces rapid drag commits to only the final value on release, not every step", () => { + const onCommit = vi.fn(); + const { host, root } = renderInto( + , + ); + const track = host.querySelector('[data-flat-slider-track="true"]'); + if (!track) throw new Error("expected a track element"); + Object.defineProperty(track, "getBoundingClientRect", { + value: () => ({ left: 0, width: 200, top: 0, height: 20, right: 200, bottom: 20 }), + }); + act(() => { + track.dispatchEvent( + new PointerEvent("pointerdown", { bubbles: true, clientX: 20, pointerId: 1 }), + ); + track.dispatchEvent( + new PointerEvent("pointermove", { bubbles: true, clientX: 160, pointerId: 1 }), + ); + track.dispatchEvent( + 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(); + act(() => { + // Real pointerup events always carry the pointer's true release position + // (matches the last pointermove) — the handler recomputes from this + // rather than trusting a possibly-stale `draft` closure. + track.dispatchEvent( + new PointerEvent("pointerup", { bubbles: true, clientX: 100, pointerId: 1 }), + ); + }); + // Release flushes immediately with the LAST position only. + expect(onCommit).toHaveBeenCalledTimes(1); + expect(onCommit).toHaveBeenCalledWith(50); + act(() => root.unmount()); + }); + it("ignores pointermove once a drag has ended (pointer capture released)", () => { const onCommit = vi.fn(); const { host, root } = renderInto( diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx index 52eba79a3..dfcf801c9 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx @@ -1,4 +1,4 @@ -import { type ReactNode } from "react"; +import { useEffect, useRef, useState, type ReactNode } from "react"; import { RotateCcw } from "../../icons/SystemIcons"; import { CommitField } from "./propertyPanelPrimitives"; import { @@ -241,13 +241,43 @@ export function FlatSlider({ onReset?: () => void; onCommit: (nextValue: number) => void; }) { - const clampedPct = Math.max(0, Math.min(100, ((value - min) / Math.max(max - min, 1e-6)) * 100)); + // 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. + const [draft, setDraft] = useState(value); + const commitTimerRef = useRef | null>(null); + const valueRef = useRef(value); + valueRef.current = value; - const commitFromClientX = (clientX: number, rect: DOMRect) => { + useEffect(() => { + setDraft(value); + }, [value]); + useEffect( + () => () => { + if (commitTimerRef.current) clearTimeout(commitTimerRef.current); + }, + [], + ); + + const clampedPct = Math.max(0, Math.min(100, ((draft - min) / Math.max(max - min, 1e-6)) * 100)); + + const stepFromClientX = (clientX: number, rect: DOMRect) => { const ratio = Math.max(0, Math.min(1, (clientX - rect.left) / Math.max(rect.width, 1))); const raw = min + ratio * (max - min); const stepped = Math.round(raw / step) * step; - onCommit(Math.max(min, Math.min(max, stepped))); + return Math.max(min, Math.min(max, stepped)); + }; + const commitDraft = (nextDraft: number) => { + if (commitTimerRef.current) clearTimeout(commitTimerRef.current); + if (nextDraft !== valueRef.current) onCommit(nextDraft); + }; + const scheduleCommit = (nextDraft: number) => { + if (commitTimerRef.current) clearTimeout(commitTimerRef.current); + commitTimerRef.current = setTimeout(() => { + if (nextDraft !== valueRef.current) onCommit(nextDraft); + }, 40); }; return ( @@ -257,22 +287,33 @@ export function FlatSlider({ data-flat-slider-track="true" role="slider" aria-label={label} - aria-valuenow={value} + aria-valuenow={draft} aria-disabled={disabled} className={`relative h-5 flex-1 ${disabled ? "cursor-not-allowed" : "cursor-pointer"}`} onPointerDown={(e) => { if (disabled) return; e.currentTarget.setPointerCapture(e.pointerId); - commitFromClientX(e.clientX, e.currentTarget.getBoundingClientRect()); + const stepped = stepFromClientX(e.clientX, e.currentTarget.getBoundingClientRect()); + setDraft(stepped); + scheduleCommit(stepped); }} onPointerMove={(e) => { if (disabled || !e.currentTarget.hasPointerCapture(e.pointerId)) return; - commitFromClientX(e.clientX, e.currentTarget.getBoundingClientRect()); + const stepped = stepFromClientX(e.clientX, e.currentTarget.getBoundingClientRect()); + setDraft(stepped); + scheduleCommit(stepped); }} onPointerUp={(e) => { if (e.currentTarget.hasPointerCapture(e.pointerId)) { e.currentTarget.releasePointerCapture(e.pointerId); } + // Recompute from the event itself rather than reading the `draft` + // closure — if pointerdown+pointerup land in the same React batch + // (e.g. a very fast click), the onPointerUp handler can still be + // bound to the pre-drag render, making `draft` stale. + const stepped = stepFromClientX(e.clientX, e.currentTarget.getBoundingClientRect()); + setDraft(stepped); + commitDraft(stepped); }} >
diff --git a/packages/studio/src/components/editor/propertyPanelFlatStyleSections.test.tsx b/packages/studio/src/components/editor/propertyPanelFlatStyleSections.test.tsx index e5c27540f..018f17ca5 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatStyleSections.test.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatStyleSections.test.tsx @@ -372,6 +372,7 @@ describe("FlatStyleSection — blur sliders", () => { }); act(() => { track.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 })); + track.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 })); }); // filterBlurValue=4 -> max=Math.max(40, 4)=40; clientX=50 of width 100 -> ratio 0.5 -> 20px. expect(onSetStyle).toHaveBeenCalledWith("filter", "blur(20px)"); @@ -390,6 +391,7 @@ describe("FlatStyleSection — blur sliders", () => { }); act(() => { backdropTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 })); + backdropTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 })); }); // backdropBlurValue=6 -> max=Math.max(60, 6)=60; clientX=50 of width 100 -> ratio 0.5 -> 30px. expect(onSetStyle).toHaveBeenCalledWith("backdrop-filter", "blur(30px)"); @@ -551,6 +553,7 @@ describe("FlatStyleSection — Overflow and Mask", () => { }); act(() => { maskInsetTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 })); + maskInsetTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 })); }); // clipInsetValue=8 -> max=Math.max(120, 8)=120; clientX=50 of width 100 -> ratio 0.5 -> 60px. // border-radius is unset here, so the clip-path's own `round 4px` is not reused — radiusValue @@ -587,6 +590,7 @@ describe("FlatStyleSection — Opacity", () => { }); act(() => { opacityTrack.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true, clientX: 50 })); + opacityTrack.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, clientX: 50 })); }); expect(onSetStyle).toHaveBeenCalledWith("opacity", "0.5"); act(() => root.unmount());