From 9b311588be7e72ab7627c55a001fa63ae5c8307c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 1 Jul 2026 03:06:40 +0000 Subject: [PATCH] fix(studio): resolve recent timeline regressions --- packages/parsers/src/gsapParser.test.ts | 20 +++ packages/parsers/src/gsapParser.ts | 7 +- .../components/editor/SnapToolbar.test.tsx | 124 ++++++++++++++++++ .../src/components/editor/SnapToolbar.tsx | 1 + packages/studio/src/hooks/useAppHotkeys.ts | 5 +- .../src/hooks/useRenderClipContent.test.ts | 55 +++++++- .../studio/src/hooks/useRenderClipContent.ts | 12 +- .../src/player/components/Timeline.test.ts | 52 +++++++- .../studio/src/player/components/Timeline.tsx | 2 - .../components/TimelineClipDiamonds.test.tsx | 73 +++++++++++ .../components/TimelineClipDiamonds.tsx | 1 + 11 files changed, 339 insertions(+), 13 deletions(-) create mode 100644 packages/studio/src/components/editor/SnapToolbar.test.tsx create mode 100644 packages/studio/src/player/components/TimelineClipDiamonds.test.tsx diff --git a/packages/parsers/src/gsapParser.test.ts b/packages/parsers/src/gsapParser.test.ts index 4f287f9e43..413fc53995 100644 --- a/packages/parsers/src/gsapParser.test.ts +++ b/packages/parsers/src/gsapParser.test.ts @@ -1656,6 +1656,26 @@ describe("keyframe mutations", () => { expect(kfs[1].properties.x).toBe(999); }); + it("addKeyframeToScript — preserves exactly one ease when updating an eased keyframe", () => { + const script = ` + const tl = gsap.timeline({ paused: true }); + tl.to("#hero", { + keyframes: { "0%": { scale: 1 }, "60%": { scale: 1.18, ease: "power2.out" }, "100%": { scale: 1 } }, + duration: 2 + }, 0); + `; + const id = getAnimId(script); + const updated = addKeyframeToScript(script, id, 60, { scale: 1.18, x: 10, y: 20 }); + const reparsed = parseGsapScript(updated); + const keyframe = reparsed.animations[0].keyframes!.keyframes.find( + (kf) => kf.percentage === 60, + )!; + + expect((updated.match(/ease:\s*"power2\.out"/g) ?? []).length).toBe(1); + expect(keyframe.ease).toBe("power2.out"); + expect(keyframe.properties).toMatchObject({ scale: 1.18, x: 10, y: 20 }); + }); + // ── backfillDefaults: editing one keyframe must not move the others ────── // UX invariant (CapCut/AE): keyframes are independent. Introducing a property // to one keyframe (e.g. `y` on an x-only tween) must backfill the other diff --git a/packages/parsers/src/gsapParser.ts b/packages/parsers/src/gsapParser.ts index 8121c7a22c..1aa5b269f2 100644 --- a/packages/parsers/src/gsapParser.ts +++ b/packages/parsers/src/gsapParser.ts @@ -1994,8 +1994,11 @@ function buildKeyframeValueNode( properties: Record, ease?: string, ): AstNode { - const entries = Object.entries(properties).map(([k, v]) => `${safeKey(k)}: ${valueToCode(v)}`); - if (ease) entries.push(`ease: ${JSON.stringify(ease)}`); + const effectiveEase = ease ?? (typeof properties.ease === "string" ? properties.ease : undefined); + const entries = Object.entries(properties) + .filter(([k]) => k !== "ease") + .map(([k, v]) => `${safeKey(k)}: ${valueToCode(v)}`); + if (effectiveEase) entries.push(`ease: ${JSON.stringify(effectiveEase)}`); return parseExpr(`{ ${entries.join(", ")} }`); } diff --git a/packages/studio/src/components/editor/SnapToolbar.test.tsx b/packages/studio/src/components/editor/SnapToolbar.test.tsx new file mode 100644 index 0000000000..9cac6538f1 --- /dev/null +++ b/packages/studio/src/components/editor/SnapToolbar.test.tsx @@ -0,0 +1,124 @@ +// @vitest-environment happy-dom + +import React, { act, useRef } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useAppHotkeys } from "../../hooks/useAppHotkeys"; +import { usePlayerStore } from "../../player/store/playerStore"; +import type { LeftSidebarHandle } from "../sidebar/LeftSidebar"; +import type { DomEditSelection } from "./domEditing"; +import { SnapToolbar } from "./SnapToolbar"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; + window.localStorage.clear(); + usePlayerStore.getState().reset(); +}); + +function renderToolbar(onSnapChange = vi.fn()) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render(); + }); + return { root, onSnapChange }; +} + +function AppHotkeyHarness() { + const domEditSelectionRef = useRef(null); + const clearDomSelectionRef = useRef<() => void>(() => undefined); + const domEditSaveTimestampRef = useRef(0); + const leftSidebarRef = useRef(null); + + useAppHotkeys({ + toggleTimelineVisibility: vi.fn(), + handleTimelineElementDelete: vi.fn(), + handleTimelineElementSplit: vi.fn(), + handleDomEditElementDelete: vi.fn(), + domEditSelectionRef, + clearDomSelectionRef, + editHistory: { + undo: vi.fn(async () => ({ ok: false })), + redo: vi.fn(async () => ({ ok: false })), + state: { undo: [], redo: [] }, + }, + readOptionalProjectFile: vi.fn(async () => ""), + readProjectFile: vi.fn(async () => ""), + writeProjectFile: vi.fn(async () => undefined), + domEditSaveTimestampRef, + showToast: vi.fn(), + syncHistoryPreviewAfterApply: vi.fn(async () => undefined), + waitForPendingDomEditSaves: vi.fn(async () => undefined), + leftSidebarRef, + handleCopy: vi.fn(() => false), + handlePaste: vi.fn(async () => undefined), + handleCut: vi.fn(async () => false), + onResetKeyframes: vi.fn(() => false), + onDeleteSelectedKeyframes: vi.fn(), + }); + + return null; +} + +function renderToolbarWithAppHotkeys(onSnapChange = vi.fn()) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + <> + + + , + ); + }); + return { root, onSnapChange }; +} + +describe("SnapToolbar keyboard shortcuts", () => { + it("toggles snap on an unclaimed S keypress", () => { + const { root, onSnapChange } = renderToolbar(); + + act(() => { + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "s", bubbles: true, cancelable: true }), + ); + }); + + expect(onSnapChange).toHaveBeenCalledWith(expect.objectContaining({ snapEnabled: false })); + act(() => root.unmount()); + }); + + it("does not toggle snap when another handler already prevented S", () => { + const { root, onSnapChange } = renderToolbar(); + const event = new KeyboardEvent("keydown", { + key: "s", + bubbles: true, + cancelable: true, + }); + event.preventDefault(); + + act(() => { + document.dispatchEvent(event); + }); + + expect(onSnapChange).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + + it("does not toggle snap when the app split shortcut claims S without a selected clip", () => { + const { root, onSnapChange } = renderToolbarWithAppHotkeys(); + + act(() => { + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "s", bubbles: true, cancelable: true }), + ); + }); + + expect(onSnapChange).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/components/editor/SnapToolbar.tsx b/packages/studio/src/components/editor/SnapToolbar.tsx index 6b75110da9..cd406c2faf 100644 --- a/packages/studio/src/components/editor/SnapToolbar.tsx +++ b/packages/studio/src/components/editor/SnapToolbar.tsx @@ -65,6 +65,7 @@ export const SnapToolbar = memo(function SnapToolbar({ onSnapChange }: SnapToolb useEffect(() => { // fallow-ignore-next-line complexity const handleKeyDown = (e: KeyboardEvent) => { + if (e.defaultPrevented) return; const t = e.target; if (t instanceof HTMLInputElement || t instanceof HTMLTextAreaElement) return; if (t instanceof HTMLElement && t.isContentEditable) return; diff --git a/packages/studio/src/hooks/useAppHotkeys.ts b/packages/studio/src/hooks/useAppHotkeys.ts index 2748bbbfce..77f6f57988 100644 --- a/packages/studio/src/hooks/useAppHotkeys.ts +++ b/packages/studio/src/hooks/useAppHotkeys.ts @@ -228,6 +228,9 @@ function dispatchPlainKey(event: KeyboardEvent, key: string, cb: HotkeyCallbacks } if (event.key === "s" && !event.altKey) { + // Reserve bare `s` for Split even when the current selection cannot split, + // so secondary listeners do not reinterpret the same key as Snap toggle. + event.preventDefault(); const { selectedElementId, elements, currentTime } = usePlayerStore.getState(); if (selectedElementId) { const el = elements.find((e) => (e.key ?? e.id) === selectedElementId); @@ -237,7 +240,6 @@ function dispatchPlainKey(event: KeyboardEvent, key: string, cb: HotkeyCallbacks currentTime > el.start && currentTime < el.start + el.duration ) { - event.preventDefault(); void cb.handleTimelineElementSplit(el, currentTime); return; } @@ -245,7 +247,6 @@ function dispatchPlainKey(event: KeyboardEvent, key: string, cb: HotkeyCallbacks // that isn't in the raw `elements` list, so the s-key can't resolve them. // Nudge toward the razor tool instead of failing silently. if (!el && selectedElementId.includes("#")) { - event.preventDefault(); cb.showToast("Use the razor tool (B) to split clips inside a sub-composition", "info"); return; } diff --git a/packages/studio/src/hooks/useRenderClipContent.test.ts b/packages/studio/src/hooks/useRenderClipContent.test.ts index 1239ecbc3d..5b99a52e26 100644 --- a/packages/studio/src/hooks/useRenderClipContent.test.ts +++ b/packages/studio/src/hooks/useRenderClipContent.test.ts @@ -1,5 +1,18 @@ -import { describe, expect, it } from "vitest"; +// @vitest-environment happy-dom + +import React, { act, isValidElement, type ReactNode } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it } from "vitest"; +import { AudioWaveform } from "../player/components/AudioWaveform"; +import type { TimelineElement } from "../player/store/playerStore"; import { normalizeCompositionSrc } from "./useRenderClipContent"; +import { useRenderClipContent } from "./useRenderClipContent"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); describe("normalizeCompositionSrc", () => { const origin = "http://localhost:5190"; @@ -48,3 +61,43 @@ describe("normalizeCompositionSrc", () => { expect(result).toBe("compositions/scenes/hero.html"); }); }); + +describe("useRenderClipContent", () => { + function renderClipContent(el: TimelineElement): ReactNode { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + let content: ReactNode = null; + + function Harness() { + const render = useRenderClipContent({ + projectIdRef: { current: "my-project" }, + compIdToSrc: new Map(), + activePreviewUrl: "/api/projects/my-project/preview", + effectiveTimelineDuration: 12, + }); + content = render(el, { clip: "#222", label: "#fff" }); + return null; + } + + act(() => { + root.render(React.createElement(Harness)); + }); + act(() => root.unmount()); + return content; + } + + it("renders audio clips as waveforms even when a composition preview URL is active", () => { + const content = renderClipContent({ + id: "voiceover", + tag: "audio", + start: 1, + duration: 4, + track: 1, + src: "assets/voiceover.mp3", + }); + + expect(isValidElement(content)).toBe(true); + if (isValidElement(content)) expect(content.type).toBe(AudioWaveform); + }); +}); diff --git a/packages/studio/src/hooks/useRenderClipContent.ts b/packages/studio/src/hooks/useRenderClipContent.ts index 57cc0ecf13..d0d58c862c 100644 --- a/packages/studio/src/hooks/useRenderClipContent.ts +++ b/packages/studio/src/hooks/useRenderClipContent.ts @@ -110,6 +110,13 @@ export function useRenderClipContent({ }); } + // Audio clips — waveform visualization. Resolve these before the generic + // activePreviewUrl thumbnail branch; audio rows need waveform data, not a + // captured frame from the currently drilled composition preview. + if (el.tag === "audio") { + return renderAudioClip(el, pid, style.label); + } + // When drilled into a composition, render all inner elements via // CompositionThumbnail at their start time — most accurate visual. if (activePreviewUrl && el.duration > 0) { @@ -131,11 +138,6 @@ export function useRenderClipContent({ el.duration < effectiveTimelineDuration * 0.92 && !/(backdrop|background|overlay|scrim|mask)/i.test(el.id); - // Audio clips — waveform visualization - if (el.tag === "audio") { - return renderAudioClip(el, pid, style.label); - } - if ((el.tag === "video" || el.tag === "img") && el.src) { const mediaSrc = el.src.startsWith("http") ? el.src diff --git a/packages/studio/src/player/components/Timeline.test.ts b/packages/studio/src/player/components/Timeline.test.ts index 524966d5c4..a84b4c2b6e 100644 --- a/packages/studio/src/player/components/Timeline.test.ts +++ b/packages/studio/src/player/components/Timeline.test.ts @@ -3,7 +3,7 @@ import React, { act } from "react"; import { createRoot } from "react-dom/client"; import { afterEach } from "vitest"; -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { Timeline, formatTimelineTickLabel, @@ -54,6 +54,56 @@ describe("Timeline provider boundary", () => { act(() => root.unmount()); }); + + it("opens the keyframe context menu without seeking to that keyframe", () => { + const host = document.createElement("div"); + document.body.append(host); + Object.defineProperty(host, "clientWidth", { + configurable: true, + value: 720, + }); + + usePlayerStore.setState({ + duration: 4, + timelineReady: true, + currentTime: 0.25, + selectedElementId: "clip-1", + elements: [{ id: "clip-1", tag: "div", start: 0, duration: 4, track: 0 }], + keyframeCache: new Map([ + [ + "clip-1", + { + format: "percentage", + keyframes: [{ percentage: 50, properties: { x: 100 }, tweenPercentage: 50 }], + }, + ], + ]), + }); + + const onSeek = vi.fn(); + const root = createRoot(host); + act(() => { + root.render(React.createElement(Timeline, { onSeek })); + }); + + const diamond = host.querySelector('button[title="50%"]'); + expect(diamond).not.toBeNull(); + + act(() => { + diamond!.dispatchEvent( + new MouseEvent("contextmenu", { + bubbles: true, + cancelable: true, + button: 2, + clientX: 120, + clientY: 40, + }), + ); + }); + + expect(onSeek).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); }); describe("generateTicks", () => { diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index 9a4aedf584..a90fc99393 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -487,8 +487,6 @@ export const Timeline = memo(function Timeline({ if (el) { setSelectedElementId(elId); onSelectElement?.(el); - const absTime = el.start + (pct / 100) * el.duration; - onSeek?.(absTime); } const kfData = keyframeCache.get(elId); const kf = kfData?.keyframes.find((k) => Math.abs(k.percentage - pct) < 0.2); diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx new file mode 100644 index 0000000000..c3d05defa4 --- /dev/null +++ b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx @@ -0,0 +1,73 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TimelineClipDiamonds } from "./TimelineClipDiamonds"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function pointerEvent(type: string, init: PointerEventInit): Event { + if (typeof PointerEvent === "function") return new PointerEvent(type, init); + return new MouseEvent(type, init); +} + +function renderDiamonds(onClickKeyframe = vi.fn()) { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + return { host, root, onClickKeyframe }; +} + +describe("TimelineClipDiamonds", () => { + it("treats primary pointerup without drag as a keyframe click", () => { + const { host, root, onClickKeyframe } = renderDiamonds(); + const diamond = host.querySelector('button[title="50%"]'); + expect(diamond).not.toBeNull(); + + act(() => { + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0 })); + }); + + expect(onClickKeyframe).toHaveBeenCalledWith(50); + act(() => root.unmount()); + }); + + it("does not treat secondary pointerup as a keyframe click", () => { + const { host, root, onClickKeyframe } = renderDiamonds(); + const diamond = host.querySelector('button[title="50%"]'); + expect(diamond).not.toBeNull(); + + act(() => { + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 2 })); + }); + + expect(onClickKeyframe).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 625f32a62a..febc4fa538 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -178,6 +178,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ const d = dragRef.current; // No drag armed (canDrag false / non-primary press) → treat as a click. if (!d || d.kfKey !== kfKey) { + if (e.button !== 0) return; if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage); else onClickKeyframe?.(kf.percentage); return;