From 69020699dfb595194c8bf85117525045439bcdb2 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 04:47:25 +0200 Subject: [PATCH 1/8] fix(studio): seek ruler clicks to the pressed position and round retime percentages A ruler press with no pointer movement settled the playhead at t=0 instead of the clicked time. handlePointerUp replays pendingClientXRef, which only the pointermove path wrote, so a plain click fell back to the ref's initial 0 and overwrote the correct pointerdown seek. Seed the ref on pointerdown. The keyframe retime move branch also returned the raw quotient while the resize branch rounded to 3dp, so values like 74.81203007518799% landed in the user's source and churned the diff on every drag. Round at the point of computation so the no-op test and the written value agree. --- .../components/editor/keyframeRetime.test.ts | 42 ++++++ .../src/components/editor/keyframeRetime.ts | 5 +- .../components/useTimelineRangeSelection.ts | 5 + .../useTimelineRangeSelectionScrub.test.tsx | 127 ++++++++++++++++++ 4 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 packages/studio/src/player/components/useTimelineRangeSelectionScrub.test.tsx diff --git a/packages/studio/src/components/editor/keyframeRetime.test.ts b/packages/studio/src/components/editor/keyframeRetime.test.ts index 4047d5c771..81264f0ab4 100644 --- a/packages/studio/src/components/editor/keyframeRetime.test.ts +++ b/packages/studio/src/components/editor/keyframeRetime.test.ts @@ -180,3 +180,45 @@ describe("resolveKeyframeRetime — guards", () => { expect(r.pctRemap).toEqual([]); }); }); + +describe("resolveKeyframeRetime — move percentages are rounded like the resize path", () => { + // The move branch used to return the raw quotient, so `74.81203007518799%` + // landed in the user's source and churned the diff on every drag. + const decimals = (n: number): number => String(n).split(".")[1]?.length ?? 0; + + it("rounds a repeating quotient to 3dp", () => { + const r = resolveKeyframeRetime({ + tweenStart: 2, + tweenDuration: 3, + keyframes: KEYFRAMES, + draggedTweenPct: 0, + dropAbsTime: 3, // (3-2)/3 = 33.333333333333336% + }); + expect(r.kind).toBe("move"); + expect(r.toTweenPct).toBe(33.333); + }); + + it("never emits more than 3 decimal places", () => { + for (const tweenDuration of [3, 7, 9, 11, 133]) { + const r = resolveKeyframeRetime({ + tweenStart: 2, + tweenDuration, + keyframes: KEYFRAMES, + draggedTweenPct: 0, + dropAbsTime: 3, + }); + expect(r.kind).toBe("move"); + expect(decimals(r.toTweenPct ?? 0)).toBeLessThanOrEqual(3); + } + }); + + it("leaves an already-short percentage untouched", () => { + const r = resolveKeyframeRetime({ + ...WINDOW, + keyframes: KEYFRAMES, + draggedTweenPct: 0, + dropAbsTime: 3, // (3-2)/4 = exactly 25% + }); + expect(r.toTweenPct).toBe(25); + }); +}); diff --git a/packages/studio/src/components/editor/keyframeRetime.ts b/packages/studio/src/components/editor/keyframeRetime.ts index f7fa1d1344..c722e6abb0 100644 --- a/packages/studio/src/components/editor/keyframeRetime.ts +++ b/packages/studio/src/components/editor/keyframeRetime.ts @@ -121,7 +121,10 @@ export function resolveKeyframeRetime(opts: { // Within the tween window → plain move (re-key the tween-%). if (dropAbsTime >= tweenStart - EPSILON_TIME && dropAbsTime <= tweenEnd + EPSILON_TIME) { - const toTweenPct = clamp(((dropAbsTime - tweenStart) / tweenDuration) * 100, 0, 100); + // Round here, not at the return: the no-op test below and the value written + // to source must be the same number. The resize branch already rounds, so + // this keeps both write paths at the authored 3dp precision. + const toTweenPct = round3(clamp(((dropAbsTime - tweenStart) / tweenDuration) * 100, 0, 100)); if (Math.abs(toTweenPct - draggedTweenPct) < NOOP_EPSILON_PCT) return { kind: "noop" }; return { kind: "move", toTweenPct }; } diff --git a/packages/studio/src/player/components/useTimelineRangeSelection.ts b/packages/studio/src/player/components/useTimelineRangeSelection.ts index 1a98e251c6..c4cd930460 100644 --- a/packages/studio/src/player/components/useTimelineRangeSelection.ts +++ b/packages/studio/src/player/components/useTimelineRangeSelection.ts @@ -268,6 +268,11 @@ export function useTimelineRangeSelection({ if (!point || !scrollRect || isTimelineRulerPress(e.clientY, scrollRect.top)) { isDragging.current = true; setIsScrubbing(true); + // Seed the pending coordinate so a press with no pointermove still + // replays THIS x on pointerup. `updateScrubDrag` is the only other + // writer, so without this a plain click settles on the ref's initial + // 0 and clamps the playhead back to t=0. + pendingClientXRef.current = e.clientX; seekFromX(e.clientX); return; } diff --git a/packages/studio/src/player/components/useTimelineRangeSelectionScrub.test.tsx b/packages/studio/src/player/components/useTimelineRangeSelectionScrub.test.tsx new file mode 100644 index 0000000000..61c93c6a40 --- /dev/null +++ b/packages/studio/src/player/components/useTimelineRangeSelectionScrub.test.tsx @@ -0,0 +1,127 @@ +// @vitest-environment happy-dom + +// Regression guard for the ruler-click seek. `handlePointerUp` replays +// `pendingClientXRef`, which only `updateScrubDrag` (pointermove) used to write, +// so a press with no move settled on the ref's initial 0 and clamped the +// playhead to t=0 — silently discarding the correct pointerdown seek. +import React, { act } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { mountReactHarness } from "../../hooks/domSelectionTestHarness"; +import { useTimelineRangeSelection } from "./useTimelineRangeSelection"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** Scroll container top; a press within RULER_H of this counts as a ruler press. */ +const SCROLL_TOP = 100; +/** Comfortably inside the ruler band (RULER_H is 28 at time of writing). */ +const RULER_Y = SCROLL_TOP + 4; + +type Handlers = ReturnType; + +function makeScrollEl(): HTMLDivElement { + const el = document.createElement("div"); + el.getBoundingClientRect = () => + ({ top: SCROLL_TOP, left: 0, width: 1000, height: 400 }) as DOMRect; + return el; +} + +function makePointerEvent(clientX: number, clientY: number): React.PointerEvent { + const currentTarget = document.createElement("div"); + currentTarget.setPointerCapture = vi.fn(); + return { + button: 0, + shiftKey: false, + metaKey: false, + ctrlKey: false, + pointerId: 1, + clientX, + clientY, + target: document.createElement("div"), + currentTarget, + } as unknown as React.PointerEvent; +} + +const roots: Array<{ unmount: () => void }> = []; + +afterEach(() => { + for (const root of roots.splice(0)) act(() => root.unmount()); +}); + +function setup(): { handlers: () => Handlers; seekFromX: ReturnType } { + const seekFromX = vi.fn(); + const scrollEl = makeScrollEl(); + let latest: Handlers | null = null; + + // Hoisted so they survive re-renders. The hook mutates `isDragging.current` + // across the press, and a fresh object per render would reset it to false. + const scrollRef = { current: scrollEl }; + const ppsRef = { current: 25 }; + const dragScrollRaf = { current: 0 }; + const isDragging = { current: false }; + const elementsRef = { current: [] }; + const trackOrderRef = { current: [] }; + const rowHeightsRef = { current: [] }; + + function Probe(): null { + latest = useTimelineRangeSelection({ + scrollRef, + ppsRef, + effectiveDuration: 30, + pps: 25, + seekFromX, + autoScrollDuringDrag: vi.fn(), + dragScrollRaf, + isDragging, + setShowPopover: vi.fn(), + elementsRef, + trackOrderRef, + rowHeightsRef, + contentOrigin: 0, + }); + return null; + } + + roots.push(mountReactHarness()); + return { + handlers: () => { + if (!latest) throw new Error("hook did not render"); + return latest; + }, + seekFromX, + }; +} + +describe("useTimelineRangeSelection — ruler press seek", () => { + it("replays the pressed x on pointerup when the pointer never moved", () => { + const { handlers, seekFromX } = setup(); + + act(() => handlers().handlePointerDown(makePointerEvent(1000, RULER_Y))); + act(() => handlers().handlePointerUp()); + + // Both the press and the settle must land on the SAME x. The bug settled on + // 0, so the LAST call is the one that decides where the playhead ends up. + expect(seekFromX).toHaveBeenCalledWith(1000); + expect(seekFromX).not.toHaveBeenCalledWith(0); + expect(seekFromX.mock.calls.at(-1)).toEqual([1000]); + }); + + it("does not fall back to x=0 for a press nearer the left edge either", () => { + const { handlers, seekFromX } = setup(); + + act(() => handlers().handlePointerDown(makePointerEvent(400, RULER_Y))); + act(() => handlers().handlePointerUp()); + + expect(seekFromX.mock.calls.at(-1)).toEqual([400]); + }); + + it("settles on the final pointermove x when the pointer did move", () => { + const { handlers, seekFromX } = setup(); + + act(() => handlers().handlePointerDown(makePointerEvent(700, RULER_Y))); + act(() => handlers().handlePointerMove(makePointerEvent(760, RULER_Y))); + act(() => handlers().handlePointerUp()); + + // The move must win over the seeded press coordinate. + expect(seekFromX.mock.calls.at(-1)).toEqual([760]); + }); +}); From 4e74eefddd4c1e38f7c97662cc52fae6d2a01698 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 04:51:19 +0200 Subject: [PATCH 2/8] fix(studio): meet the 24x24 pointer target minimum on toolbar and lane controls --- .../src/components/StudioLeftSidebar.tsx | 15 +++-- .../src/components/StudioRightPanel.tsx | 10 ++- .../studio/src/components/TimelineToolbar.tsx | 4 +- .../components/editor/KeyframeNavigation.tsx | 5 ++ .../editor/propertyPanelPrimitives.tsx | 4 +- .../components/nle/TimelineResizeDivider.tsx | 13 ++-- .../src/components/pointerTargetSize.test.tsx | 65 +++++++++++++++++++ .../components/sidebar/CompositionsTab.tsx | 5 +- .../components/TimelineClipDiamonds.tsx | 12 +++- .../components/TimelineDiamondConnectors.tsx | 5 +- 10 files changed, 121 insertions(+), 17 deletions(-) create mode 100644 packages/studio/src/components/pointerTargetSize.test.tsx diff --git a/packages/studio/src/components/StudioLeftSidebar.tsx b/packages/studio/src/components/StudioLeftSidebar.tsx index 40842ecc58..4ca96ee692 100644 --- a/packages/studio/src/components/StudioLeftSidebar.tsx +++ b/packages/studio/src/components/StudioLeftSidebar.tsx @@ -154,10 +154,10 @@ export function StudioLeftSidebar({ onAddAssetToTimeline={onAddAssetToTimeline} onAddCompositionToTimeline={onAddCompositionToTimeline} /> - {/* Vertical resize divider: 3px visible seam, 8px pointer-capture zone via + {/* Vertical resize divider: 3px visible seam, 13px pointer-capture zone via the absolutely-positioned inner hit area. The outer element is w-[3px] so - it contributes only 3px of gap in the flex row; the inner -left-[2.5px] - element widens the hit area to 8px without affecting layout. */} + it contributes only 3px of gap in the flex row; the inner -left-[2px] + element widens the hit area without affecting layout. */}
- {/* Expanded hit zone: 8px wide, centered on the 3px seam */} -
+ {/* Expanded hit zone, deliberately asymmetric: 2px into the sidebar card, + the 3px seam, then 8px into the preview pane's p-2 stage gutter — the + only dead space adjacent to this seam. It stops at 13px rather than the + 24px WCAG 2.2 (2.5.8) target because the next pixel on either side is + live: the sidebar's scrolling tab content on the left, the preview + stage on the right. Silently stealing their clicks is the worse bug. */} +
{/* Visible hairline */}
diff --git a/packages/studio/src/components/StudioRightPanel.tsx b/packages/studio/src/components/StudioRightPanel.tsx index fc96e3605c..cf9c69f09b 100644 --- a/packages/studio/src/components/StudioRightPanel.tsx +++ b/packages/studio/src/components/StudioRightPanel.tsx @@ -446,7 +446,7 @@ export function StudioRightPanel({ return ( <> - {/* Vertical resize divider: 3px visible seam, 8px pointer-capture zone via + {/* Vertical resize divider: 3px visible seam, 13px pointer-capture zone via the absolutely-positioned inner hit area. */}
- {/* Expanded hit zone: 8px wide, centered on the 3px seam */} -
+ {/* Expanded hit zone, deliberately asymmetric: 8px into the preview pane's + p-2 stage gutter (the only dead space here), the 3px seam, then 2px + into the inspector card. It stops at 13px rather than the 24px WCAG 2.2 + (2.5.8) target because the next pixel on either side is live: the + preview stage on the left, the inspector's own controls on the right. */} +
{/* Visible hairline */}
diff --git a/packages/studio/src/components/TimelineToolbar.tsx b/packages/studio/src/components/TimelineToolbar.tsx index c745f087d1..eda3588918 100644 --- a/packages/studio/src/components/TimelineToolbar.tsx +++ b/packages/studio/src/components/TimelineToolbar.tsx @@ -449,7 +449,9 @@ export function TimelineToolbar({ domEditSession, onSplitElement }: TimelineTool setZoomMode("manual"); setManualZoomPercent(timelineSliderToZoomPercent(Number(e.target.value))); }} - className="mx-1 w-[96px] cursor-pointer appearance-none bg-transparent [&::-webkit-slider-runnable-track]:h-[2px] [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:bg-neutral-700 [&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:w-[10px] [&::-webkit-slider-thumb]:h-[10px] [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-white [&::-webkit-slider-thumb]:-mt-1 [&::-webkit-slider-thumb]:shadow-[0_0_0_2px_#0a0a0a,0_1px_3px_rgba(0,0,0,0.5)] [&::-webkit-slider-thumb]:cursor-grab [&::-webkit-slider-thumb:active]:cursor-grabbing" + // h-6 on the input is the 24x24 WCAG 2.2 (2.5.8) target: the visible + // track stays 2px and the thumb 10px, only the pointer box grows. + className="mx-1 h-6 w-[96px] cursor-pointer appearance-none bg-transparent [&::-webkit-slider-runnable-track]:h-[2px] [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:bg-neutral-700 [&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:w-[10px] [&::-webkit-slider-thumb]:h-[10px] [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-white [&::-webkit-slider-thumb]:-mt-1 [&::-webkit-slider-thumb]:shadow-[0_0_0_2px_#0a0a0a,0_1px_3px_rgba(0,0,0,0.5)] [&::-webkit-slider-thumb]:cursor-grab [&::-webkit-slider-thumb:active]:cursor-grabbing" />