From f63a4464a1d17da3548a99e2c1a537254479ad46 Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Mon, 13 Jul 2026 17:53:43 -0700 Subject: [PATCH 01/19] feat(studio): mirror canvas z-order actions into timeline lanes, badge z overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track order = default paint order; authored z = advanced override. - timelineZMirror.ts: pure resolver mapping a successful z-menu action to a timeline lane move — closest track in the action's direction that is free over the clip's whole span, else a new lane adjacent to the crossed neighbor; temporal-overlap scope (default pending product sign-off, see module doc); visual zone only; same-file reference scoping; persistTrack via the shared authored-space rules. null for non-clips (menu stays z-only) and at-extreme/no-overlap cases. - useCanvasZOrderTimelineMirror.ts: after the z commit resolves, the mirror persists the lane move through the same machinery as a timeline lane drag (optimistic store update, authoredTrack refresh, rollback); inserts reuse commitTrackInsert's renumber via a shared buildTrackInsertEdits core. Both writes share one coalesce key (zReorderCoalesceKey) and fold into ONE undo entry (test proves it over the real history reducer). The mirror never triggers the lane->z stacking sync, so it cannot fight the z values the action just set. - timelineZOverride.ts + TimelineClip badge: clips whose paint order contradicts lane order among temporally-overlapping same-context visual neighbors (laneIsAbove XOR paintsAbove, the stacking-sync predicates) show a 'z' badge — authored z overrides are surfaced instead of silently disagreeing with the timeline. - Timeline.tsx track derivations extracted to useTimelineTrackDerivations (600-line cap). --- .../components/editor/CanvasContextMenu.tsx | 19 +- .../src/components/editor/DomEditOverlay.tsx | 7 +- .../src/components/nle/PreviewOverlays.tsx | 28 +- .../useCanvasZOrderTimelineMirror.test.tsx | 305 +++++++++++++++ .../nle/useCanvasZOrderTimelineMirror.ts | 107 ++++++ .../src/hooks/useElementLifecycleOps.ts | 24 +- .../studio/src/player/components/Timeline.tsx | 25 +- .../src/player/components/TimelineClip.tsx | 36 ++ .../src/player/components/TimelineLanes.tsx | 4 + .../components/timelineClipDragCommit.test.ts | 118 ++++++ .../components/timelineClipDragCommit.ts | 92 ++++- .../player/components/timelineStackingSync.ts | 14 +- .../player/components/timelineZMirror.test.ts | 351 ++++++++++++++++++ .../src/player/components/timelineZMirror.ts | 183 +++++++++ .../components/timelineZOverride.test.ts | 90 +++++ .../player/components/timelineZOverride.ts | 97 +++++ .../components/useTimelineTrackDerivations.ts | 45 +++ 17 files changed, 1498 insertions(+), 47 deletions(-) create mode 100644 packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx create mode 100644 packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts create mode 100644 packages/studio/src/player/components/timelineZMirror.test.ts create mode 100644 packages/studio/src/player/components/timelineZMirror.ts create mode 100644 packages/studio/src/player/components/timelineZOverride.test.ts create mode 100644 packages/studio/src/player/components/timelineZOverride.ts create mode 100644 packages/studio/src/player/components/useTimelineTrackDerivations.ts diff --git a/packages/studio/src/components/editor/CanvasContextMenu.tsx b/packages/studio/src/components/editor/CanvasContextMenu.tsx index cf06d95487..ca9e44897e 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.tsx @@ -50,8 +50,17 @@ interface CanvasContextMenuProps { * affected siblings). The menu does NOT touch the live DOM — wire to * handleDomZIndexReorderCommit, which applies the live styles itself * (see module-level wiring comment). + * + * `crossed` is the sibling a forward/backward step moved past, resolved from + * the SAME pre-mutation render order as the patches (null for front/back or + * when there is no neighbor). The host uses it to mirror the z action into a + * timeline lane move (resolveZMirrorLaneMove's crossedKey). */ - onApplyZIndex?: (patches: ZOrderPatch[], action: ZOrderAction) => void; + onApplyZIndex?: ( + patches: ZOrderPatch[], + action: ZOrderAction, + crossed: HTMLElement | null, + ) => void; /** * Called after a successful bring-forward / send-backward with the sibling * the target stepped over (resolved from the SAME pre-mutation state as the @@ -113,14 +122,16 @@ export const CanvasContextMenu = memo(function CanvasContextMenu({ const patches = resolveZOrderChange(el, action); if (patches === null) return; // Resolve the crossed neighbor BEFORE the commit path mutates live styles — - // both resolvers must read the same pre-change render order. - const crossed = onZOrderCrossed ? resolveCrossedNeighbor(el, action) : null; + // both resolvers must read the same pre-change render order. Always resolved + // (not only for the flash): onApplyZIndex forwards it so the host can mirror + // the z step into a timeline lane move. + const crossed = resolveCrossedNeighbor(el, action); // Do NOT pre-apply styles here: handleDomZIndexReorderCommit writes the // live z-index (and injects position:relative for static elements) in the // same synchronous flow, so feedback is still instant — and it must read // the PRE-change styles itself, both to capture true rollback values and // to detect a static position that needs persisting. - onApplyZIndex(patches, action); + onApplyZIndex(patches, action, crossed); if (crossed && onZOrderCrossed) onZOrderCrossed(crossed, action); onClose(); } diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 18bfe32c17..2c6d6add3c 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -103,6 +103,9 @@ interface DomEditOverlayProps { selection: DomEditSelection, patches: ZOrderPatch[], action: ZOrderAction, + /** Sibling a forward/backward step moved past (pre-mutation render order); + * null for front/back. Feeds the timeline z-mirror's crossedKey. */ + crossed: HTMLElement | null, ) => void; } @@ -529,8 +532,8 @@ export const DomEditOverlay = memo(function DomEditOverlay({ } onApplyZIndex={ onApplyZIndex - ? (patches, action) => { - onApplyZIndex(contextMenu.sel, patches, action); + ? (patches, action, crossed) => { + onApplyZIndex(contextMenu.sel, patches, action, crossed); } : undefined } diff --git a/packages/studio/src/components/nle/PreviewOverlays.tsx b/packages/studio/src/components/nle/PreviewOverlays.tsx index 19c9a234f7..678a42432c 100644 --- a/packages/studio/src/components/nle/PreviewOverlays.tsx +++ b/packages/studio/src/components/nle/PreviewOverlays.tsx @@ -19,6 +19,8 @@ import { readStudioUiPreferences } from "../../utils/studioUiPreferences"; import { readHfId, type DomEditSelection } from "../editor/domEditing"; import { buildStableSelector } from "../editor/domEditingDom"; import { deriveTimelineStoreKey } from "../../player/lib/timelineElementHelpers"; +import { zReorderCoalesceKey } from "../../hooks/useElementLifecycleOps"; +import { useCanvasZOrderTimelineMirror } from "./useCanvasZOrderTimelineMirror"; import type { BlockPreviewInfo } from "../sidebar/BlocksTab"; import type { GestureRecordingState } from "../editor/GestureRecordControl"; import type { ReactNode } from "react"; @@ -158,6 +160,7 @@ export function PreviewOverlays({ handleDomEditElementDelete, handleDomZIndexReorderCommit, } = useDomEditActionsContext(); + const mirrorZOrderToTimeline = useCanvasZOrderTimelineMirror(); // fallow-ignore-next-line complexity const [snapPrefs, setSnapPrefs] = useState(() => { @@ -224,7 +227,7 @@ export function PreviewOverlays({ onRotationCommit={handleDomRotationCommit} onStyleCommit={handleDomStyleCommit} onDeleteSelection={handleDomEditElementDelete} - onApplyZIndex={(sel, patches, action) => { + onApplyZIndex={(sel, patches, action, crossed) => { const { entries, dropped } = resolveZIndexEntries(sel, patches); if (dropped.length > 0) { // These siblings can't be written to source. Apply their live z @@ -237,7 +240,28 @@ export function PreviewOverlays({ dropped.map((patch) => describeZIndexElement(patch.element)).join(", "), ); } - if (entries.length > 0) handleDomZIndexReorderCommit(entries, undefined, action); + if (entries.length === 0) return; + // Shared undo coalesce key: passed to BOTH the z persist and the + // timeline lane mirror below so editHistory folds the two records + // into one undo entry (same value handleDomZIndexReorderCommit would + // default to — passed explicitly so the mirror shares it by + // construction, not by formula duplication). + const coalesceKey = zReorderCoalesceKey(entries, action); + // Mirror the z action into a timeline lane move AFTER the z commit + // resolves: the two persists patch the same source file, so they are + // serialized (same reasoning as the lane-drag's move→z ordering). + // A failed z commit already toasted + rolled back — skip the mirror. + handleDomZIndexReorderCommit(entries, coalesceKey, action) + .then(() => + mirrorZOrderToTimeline({ + selectionKey: entries.find((e) => e.element === sel.element)?.key, + action, + crossed, + sourceFile: sel.sourceFile, + coalesceKey, + }), + ) + .catch(() => undefined); }} gridVisible={snapPrefs.gridVisible} gridSpacing={snapPrefs.gridSpacing} diff --git a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx new file mode 100644 index 0000000000..4a9fd7e338 --- /dev/null +++ b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx @@ -0,0 +1,305 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { usePlayerStore, type TimelineElement } from "../../player"; +import { TimelineEditProvider } from "../../contexts/TimelineEditContext"; +import type { TimelineEditCallbacks } from "../../player/components/timelineCallbacks"; +import { useElementLifecycleOps, zReorderCoalesceKey } from "../../hooks/useElementLifecycleOps"; +import { + useCanvasZOrderTimelineMirror, + type MirrorZOrderInput, +} from "./useCanvasZOrderTimelineMirror"; +import { mountReactHarness } from "../../hooks/domSelectionTestHarness"; +import { + buildEditHistoryEntry, + createEmptyEditHistory, + pushEditHistoryEntry, + type EditHistoryState, +} from "../../utils/editHistory"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; + // Wrapped in act: mounted harnesses subscribe to the store via + // useExpandedTimelineElements, so the reset re-renders them. + act(() => usePlayerStore.getState().setElements([])); +}); + +/** Set the store elements inside act (mounted harnesses re-render on it). */ +function setStoreElements(elements: TimelineElement[]): void { + act(() => usePlayerStore.getState().setElements(elements)); +} + +// Store keys follow buildTimelineElementKey's `#` branch — +// the same shape deriveTimelineStoreKey produces for reorder entries. +function storeEl(domId: string, track: number, start: number, duration: number): TimelineElement { + return { + id: domId, + key: `index.html#${domId}`, + tag: "video", + start, + duration, + track, + domId, + }; +} + +type ReorderEntries = Array<{ + element: HTMLElement; + zIndex: number; + id?: string; + selector?: string; + sourceFile: string; + key?: string; +}>; + +interface HarnessApi { + commitZ: (entries: ReorderEntries, coalesceKey: string, action: string) => Promise; + mirror: (input: MirrorZOrderInput) => Promise; +} + +/** + * Mount the REAL wiring pair PreviewOverlays composes — handleDomZIndexReorderCommit + * (z persist) + useCanvasZOrderTimelineMirror (lane mirror) — over a shared, + * real editHistory reducer so the undo-fold assertion exercises the actual + * pushEditHistoryEntry coalescing: + * + * - the z sink mimics commitDomEditPatchBatches' recordEdit call verbatim + * (kind "manual", options.coalesceKey — see useDomEditCommits.ts), and + * - the move sink mimics persistTimelineBatchEdit → saveProjectFilesWithHistory + * (kind "timeline", the coalesceKey forwarded through onMoveElements — see + * timelineEditingHelpers.ts / studioFileHistory.ts), + * + * with a deterministic clock inside the reducer's 300ms coalesce window. + */ +function mountMirrorHarness(history: { + state: EditHistoryState; + now: () => number; + fileContent: { current: string }; + moveCoalesceKeys: string[]; +}) { + const record = ( + label: string, + kind: "manual" | "timeline", + coalesceKey: string, + after: string, + ) => { + const entry = buildEditHistoryEntry({ + id: `e-${history.now()}`, + projectId: "p", + label, + kind, + coalesceKey, + now: history.now(), + files: { "index.html": { before: history.fileContent.current, after } }, + }); + history.fileContent.current = after; + history.state = pushEditHistoryEntry(history.state, entry); + }; + + const onMoveElements: TimelineEditCallbacks["onMoveElements"] = (_edits, coalesceKey) => { + history.moveCoalesceKeys.push(coalesceKey ?? ""); + record("Move timeline clips", "timeline", coalesceKey ?? "", "C-move"); + }; + + const api: Partial = {}; + function Harness() { + const { handleDomZIndexReorderCommit } = useElementLifecycleOps({ + activeCompPath: "index.html", + showToast: vi.fn(), + writeProjectFile: vi.fn(async () => {}), + domEditSaveTimestampRef: { current: 0 }, + editHistory: { recordEdit: vi.fn(async () => {}) }, + projectIdRef: { current: null }, + reloadPreview: vi.fn(), + clearDomSelection: vi.fn(), + commitDomEditPatchBatches: async (_batches, options) => { + record(options.label, "manual", options.coalesceKey, "B-z"); + }, + }); + api.commitZ = handleDomZIndexReorderCommit; + api.mirror = useCanvasZOrderTimelineMirror(); + return null; + } + mountReactHarness( + + + , + ); + return api as HarnessApi; +} + +function makeHistory() { + let tick = 1000; + return { + state: createEmptyEditHistory(), + // Deterministic clock: consecutive records land 50ms apart — inside the + // reducer's default 300ms coalesce window, as in the live flow where the + // mirror is dispatched right after the z persist resolves. + now: () => (tick += 50), + fileContent: { current: "A-original" }, + moveCoalesceKeys: [] as string[], + }; +} + +function domTarget(id: string): HTMLElement { + const el = document.createElement("div"); + el.id = id; + document.body.appendChild(el); + return el; +} + +describe("useCanvasZOrderTimelineMirror", () => { + it("folds the z write and the mirrored lane write into ONE undo entry (shared coalesce key)", async () => { + // Timeline: t on lane 2, b on lane 1 (the crossed neighbor), a on lane 0 + // free over t's span → bring-forward mirrors to a kind:"move" onto lane 0. + setStoreElements([storeEl("a", 0, 20, 5), storeEl("b", 1, 0, 10), storeEl("t", 2, 0, 10)]); + const history = makeHistory(); + const api = mountMirrorHarness(history); + + const target = domTarget("t"); + const entries: ReorderEntries = [ + { element: target, zIndex: 7, id: "t", sourceFile: "index.html", key: "index.html#t" }, + ]; + const coalesceKey = zReorderCoalesceKey(entries, "bring-forward"); + expect(coalesceKey).toBe("z-reorder:bring-forward:t"); + + await act(async () => { + // The PreviewOverlays wiring: z commit first, mirror after it resolves, + // BOTH with the same key. + await api.commitZ(entries, coalesceKey, "bring-forward"); + const mirrored = await api.mirror({ + selectionKey: "index.html#t", + action: "bring-forward", + crossed: domTarget("b"), + sourceFile: "index.html", + coalesceKey, + }); + expect(mirrored).toBe(true); + }); + + // The move persist received the EXACT z coalesce key… + expect(history.moveCoalesceKeys).toEqual([coalesceKey]); + // …and the real reducer folded the two records into one undo entry spanning + // pre-z "before" → post-move "after". One Cmd+Z reverts both writes. + expect(history.state.undo).toHaveLength(1); + expect(history.state.undo[0].files["index.html"]).toMatchObject({ + before: "A-original", + after: "C-move", + }); + // Timeline UI reflects the lane change without a reload: optimistic store update. + const t = usePlayerStore.getState().elements.find((e) => e.key === "index.html#t"); + expect(t?.track).toBe(0); + }); + + it("z-only actions leave the timeline untouched (resolver null → single z undo entry)", async () => { + // t has NO overlapping neighbor above → bring-forward has no lane mirror. + setStoreElements([storeEl("a", 0, 20, 5), storeEl("t", 1, 0, 10)]); + const history = makeHistory(); + const api = mountMirrorHarness(history); + + const entries: ReorderEntries = [ + { + element: domTarget("t"), + zIndex: 3, + id: "t", + sourceFile: "index.html", + key: "index.html#t", + }, + ]; + const coalesceKey = zReorderCoalesceKey(entries, "bring-forward"); + await act(async () => { + await api.commitZ(entries, coalesceKey, "bring-forward"); + const mirrored = await api.mirror({ + selectionKey: "index.html#t", + action: "bring-forward", + crossed: null, + sourceFile: "index.html", + coalesceKey, + }); + expect(mirrored).toBe(false); + }); + + expect(history.moveCoalesceKeys).toEqual([]); // no lane persist dispatched + expect(history.state.undo).toHaveLength(1); // just the z entry + const t = usePlayerStore.getState().elements.find((e) => e.key === "index.html#t"); + expect(t?.track).toBe(1); // lane unchanged + }); + + it("elements that are not timeline clips resolve false without touching the persist path", async () => { + setStoreElements([storeEl("a", 0, 0, 10)]); + const history = makeHistory(); + const api = mountMirrorHarness(history); + const mirrored = await act(async () => + api.mirror({ + selectionKey: undefined, // canvas-only decoration: no timeline key + action: "send-to-back", + crossed: null, + sourceFile: "index.html", + coalesceKey: "z-reorder:send-to-back:x", + }), + ); + expect(mirrored).toBe(false); + expect(history.moveCoalesceKeys).toEqual([]); + }); + + it("maps the crossed neighbor to its timeline key and rebases expanded sub-comp children", async () => { + // t is an expanded sub-comp child (expandedParentStart 5, absolute start 5): + // the mirror must forward its persist in LOCAL time (start 0), the same + // rebase a timeline lane drag applies (forwardRebasedTimelineMoveElements). + setStoreElements([ + { + ...storeEl("a", 0, 25, 5), + sourceFile: "sub.html", + key: "sub.html#a", + expandedParentStart: 5, + }, + { + ...storeEl("b", 1, 5, 10), + sourceFile: "sub.html", + key: "sub.html#b", + expandedParentStart: 5, + }, + { + ...storeEl("t", 2, 5, 10), + sourceFile: "sub.html", + key: "sub.html#t", + expandedParentStart: 5, + }, + ]); + const edits: Array<{ element: TimelineElement; updates: { start: number; track: number } }> = + []; + const onMoveElements: TimelineEditCallbacks["onMoveElements"] = (batch) => { + edits.push(...batch); + }; + const api: Partial = {}; + function Harness() { + api.mirror = useCanvasZOrderTimelineMirror(); + return null; + } + mountReactHarness( + + + , + ); + + const mirrored = await act(async () => + api.mirror!({ + selectionKey: "sub.html#t", + action: "bring-forward", + // The crossed sibling maps to sub.html#b via its DOM id + sourceFile — + // the same derivation reorder entries use (deriveTimelineStoreKey). + crossed: domTarget("b"), + sourceFile: "sub.html", + coalesceKey: "z-reorder:bring-forward:t", + }), + ); + expect(mirrored).toBe(true); + expect(edits).toHaveLength(1); + // Rebased to sub-comp local coords: absolute 5 − parent start 5 = 0. + expect(edits[0].element.start).toBe(0); + expect(edits[0].updates).toMatchObject({ start: 0, track: 0 }); + }); +}); diff --git a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts new file mode 100644 index 0000000000..665eb10eee --- /dev/null +++ b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts @@ -0,0 +1,107 @@ +import { useCallback, useRef } from "react"; +import { usePlayerStore } from "../../player"; +import { useExpandedTimelineElements } from "../../player/hooks/useExpandedTimelineElements"; +import { useTimelineEditContextOptional } from "../../contexts/TimelineEditContext"; +import { + displayTrackOrder, + resolveZMirrorLaneMove, + type ZMirrorAction, +} from "../../player/components/timelineZMirror"; +import { commitZMirrorLaneMove } from "../../player/components/timelineClipDragCommit"; +import { deriveTimelineStoreKey } from "../../player/lib/timelineElementHelpers"; +import { buildStableSelector } from "../editor/domEditingDom"; +import { forwardRebasedTimelineMoveElements } from "./TimelinePane"; + +export interface MirrorZOrderInput { + /** Timeline store key of the element the menu acted on (entry.key), if any. */ + selectionKey: string | undefined; + action: ZMirrorAction; + /** Sibling a forward/backward step moved past (from resolveCrossedNeighbor). */ + crossed: HTMLElement | null; + /** Source file of the selection — siblings share it (same document). */ + sourceFile: string; + /** The z persist's coalesce key (zReorderCoalesceKey) — REQUIRED so the lane + * write folds into the same undo entry as the z write. */ + coalesceKey: string; +} + +/** + * Mirror a successful canvas z-order menu action into a timeline LANE move. + * + * The caller (PreviewOverlays) invokes the returned callback AFTER the z commit + * resolved — serializing the two same-file writes, exactly like the lane-drag's + * move→z ordering (see persistMoveEdits' doc) — and with the SAME coalesce key + * the z persist recorded, so editHistory folds both records into one undo entry. + * + * Element source: `useExpandedTimelineElements()` — the same expanded display + * set the Timeline renders and the resolver expects (post-normalizeToZones + * lanes, expanded sub-comp children on their synthetic rows). No new expansion + * is built here. + * + * The mirror persists through the SAME machinery as a timeline lane drag + * (commitZMirrorLaneMove → persistMoveEdits → onMoveElements, with expanded + * children rebased to local coords via forwardRebasedTimelineMoveElements) — + * optimistic store update + rollback included, so the timeline reflects the + * lane change without a reload. The deps below deliberately OMIT + * `readZIndex`/`onStackingPatches`: the lane→z stacking sync + * (syncStackingForEdit) must not fire and recompute the z values the user just + * set — commitZMirrorLaneMove never calls it, and without these deps it would + * no-op even if called. + * + * Resolves `true` when a lane move persisted, `false` for z-only actions (no + * timeline mirror applies) or a rolled-back persist. + */ +export function useCanvasZOrderTimelineMirror(): (input: MirrorZOrderInput) => Promise { + const elements = useExpandedTimelineElements(); + const elementsRef = useRef(elements); + elementsRef.current = elements; + const { onMoveElements } = useTimelineEditContextOptional(); + + return useCallback( + (input: MirrorZOrderInput) => { + const els = elementsRef.current; + const element = input.selectionKey + ? els.find((e) => (e.key ?? e.id) === input.selectionKey) + : undefined; + // Not a timeline clip (canvas-only decoration) → z-only action, unchanged. + if (!element) return Promise.resolve(false); + + // Map the crossed neighbor to its timeline key the same way z-reorder + // entries get theirs (siblingZIndexEntry): DOM id, else stable selector, + // scoped to the selection's source file. + const crossedKey = input.crossed + ? deriveTimelineStoreKey({ + domId: input.crossed.id || undefined, + selector: buildStableSelector(input.crossed), + sourceFile: input.sourceFile, + }) + : null; + + const move = resolveZMirrorLaneMove({ + action: input.action, + element, + elements: els, + crossedKey, + }); + if (!move) return Promise.resolve(false); + + return commitZMirrorLaneMove( + element, + move, + { + elements: els, + trackOrder: displayTrackOrder(els), + updateElement: (key, updates) => usePlayerStore.getState().updateElement(key, updates), + onMoveElements: onMoveElements + ? (edits, coalesceKey, operation) => + forwardRebasedTimelineMoveElements(edits, coalesceKey, operation, onMoveElements) + : undefined, + // NO readZIndex / onStackingPatches: see the hook doc — the lane→z + // stacking sync must not re-trigger and fight the just-set z values. + }, + input.coalesceKey, + ); + }, + [onMoveElements], + ); +} diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index f8ac938d73..3e6d05f838 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -25,6 +25,26 @@ interface UseElementLifecycleOpsParams extends DomEditCommitBaseParams { onElementDeleted?: (selection: DomEditSelection) => void; } +/** + * Undo coalesce key for a z-reorder commit. The key carries the action kind so + * two DIFFERENT actions on the same element set (e.g. "bring-forward" then + * "send-backward" within the coalesce window) never merge into one undo step. + * + * Exported as THE single implementation of the key: the canvas z-order mirror + * (useCanvasZOrderTimelineMirror) passes this exact key into its timeline lane + * persist so editHistory folds the z write and the track write into one undo + * entry — a drifting duplicate formula would silently split the undo. + */ +export function zReorderCoalesceKey( + entries: ReadonlyArray<{ element: HTMLElement; id?: string; selector?: string }>, + actionKind?: string, +): string { + const ids = entries + .map((e) => e.id ?? e.selector ?? e.element.getAttribute("data-hf-id") ?? "el") + .join(":"); + return `z-reorder:${actionKind ?? "reorder"}:${ids}`; +} + export function useElementLifecycleOps({ activeCompPath, showToast, @@ -158,9 +178,7 @@ export function useElementLifecycleOps({ // same element set (e.g. "bring-forward" then "send-backward" within the // coalesce window) never merge into one undo step. Callers that share a // gesture (lane moves) pass an explicit gestureCoalesceKey instead. - const coalesceKey = - gestureCoalesceKey ?? - `z-reorder:${actionKind ?? "reorder"}:${entries.map((e) => e.id ?? e.selector ?? e.element.getAttribute("data-hf-id") ?? "el").join(":")}`; + const coalesceKey = gestureCoalesceKey ?? zReorderCoalesceKey(entries, actionKind); const patchesBySourceFile = new Map(); const rollbacks: Array<() => void> = []; for (const entry of entries) { diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index 8bd838e3ce..0900cec5da 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -9,7 +9,7 @@ import { defaultTimelineTheme } from "./timelineTheme"; import { useTimelineRangeSelection } from "./useTimelineRangeSelection"; import { useTimelinePlayhead } from "./useTimelinePlayhead"; import { useTimelineActiveClips } from "./useTimelineActiveClips"; -import { type TrackVisualStyle, getTrackStyle } from "./timelineIcons"; +import { getTrackStyle } from "./timelineIcons"; import { useTimelineZoom } from "./useTimelineZoom"; import { useTimelineAssetDrop } from "./timelineDragDrop"; import { TimelineEmptyState } from "./TimelineEmptyState"; @@ -20,6 +20,7 @@ import { TimelineOverlays } from "./TimelineOverlays"; import { useTimelineEditPinning } from "./useTimelineEditPinning"; import { useTimelineStackingSync } from "./useTimelineStackingSync"; import { useTimelineGeometry } from "./useTimelineGeometry"; +import { useTimelineTrackDerivations } from "./useTimelineTrackDerivations"; import { GUTTER, generateTicks, @@ -195,25 +196,8 @@ export const Timeline = memo(function Timeline({ return Number.isFinite(result) ? result : safeDur; }, [rawElements, duration]); - const tracks = useMemo(() => { - const map = new Map(); - for (const el of expandedElements) { - const list = map.get(el.track) ?? []; - list.push(el); - map.set(el.track, list); - } - return Array.from(map.entries()).sort(([a], [b]) => a - b); - }, [expandedElements]); - - const trackStyles = useMemo(() => { - const map = new Map(); - for (const [trackNum, els] of tracks) { - map.set(trackNum, getTrackStyle(els[0]?.tag ?? "")); - } - return map; - }, [tracks]); - - const trackOrder = useMemo(() => tracks.map(([trackNum]) => trackNum), [tracks]); + const { tracks, trackStyles, trackOrder, zOverrideKeys } = + useTimelineTrackDerivations(expandedElements); const trackOrderRef = useRef(trackOrder); trackOrderRef.current = trackOrder; const expandedElementsRef = useRef(expandedElements); @@ -492,6 +476,7 @@ export const Timeline = memo(function Timeline({ theme={theme} displayTrackOrder={displayTrackOrder} trackOrder={trackOrder} + zOverrideKeys={zOverrideKeys} tracks={tracks} trackStyles={trackStyles} selectedElementId={selectedElementId} diff --git a/packages/studio/src/player/components/TimelineClip.tsx b/packages/studio/src/player/components/TimelineClip.tsx index 79abf1c39b..1ea00a4d94 100644 --- a/packages/studio/src/player/components/TimelineClip.tsx +++ b/packages/studio/src/player/components/TimelineClip.tsx @@ -15,6 +15,12 @@ interface TimelineClipProps { capabilities: TimelineEditCapabilities; theme?: TimelineTheme; isComposition: boolean; + /** + * Paint order contradicts lane order for this clip (an authored z override — + * see timelineZOverride.computeZOverrideKeys). Renders a small "z" badge so + * the lane order isn't silently misleading about what the canvas paints. + */ + hasZOverride?: boolean; onHoverStart: () => void; onHoverEnd: () => void; onPointerDown?: (e: React.PointerEvent) => void; @@ -37,6 +43,7 @@ export const TimelineClip = memo(function TimelineClip({ capabilities, theme = defaultTimelineTheme, isComposition, + hasZOverride = false, onHoverStart, onHoverEnd, onPointerDown, @@ -160,6 +167,35 @@ export const TimelineClip = memo(function TimelineClip({ /> )} + {/* "z" badge: this clip's authored z contradicts lane order — the lanes no + longer tell you what paints on top. Kept above the trim handles (z 5) + and gated on width so micro clips don't clutter. */} + {hasZOverride && widthPx >= 24 && ( + + z + + )} {showLabel && {displayLabel}} {showDefaultText && ( diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 9a2078a48b..288b53cbf8 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -38,6 +38,8 @@ export interface TimelineLaneBaseProps { theme: TimelineTheme; displayTrackOrder: number[]; trackOrder: number[]; + /** Clips whose paint order contradicts lane order — get the "z" badge (see timelineZOverride.ts). */ + zOverrideKeys: ReadonlySet; tracks: [number, TimelineElement[]][]; trackStyles: Map; selectedElementId: string | null; @@ -100,6 +102,7 @@ export function TimelineLanes({ theme, displayTrackOrder, trackOrder, + zOverrideKeys, tracks, trackStyles, selectedElementId, @@ -300,6 +303,7 @@ export function TimelineLanes({ capabilities={capabilities} theme={theme} isComposition={isComposition} + hasZOverride={zOverrideKeys.has(elementKey)} onHoverStart={() => setHoveredClip(clipKey)} onHoverEnd={() => setHoveredClip(null)} onResizeStart={ diff --git a/packages/studio/src/player/components/timelineClipDragCommit.test.ts b/packages/studio/src/player/components/timelineClipDragCommit.test.ts index a09c4e25d1..fd8fa3922d 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.test.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.test.ts @@ -3,6 +3,7 @@ import type { TimelineElement } from "../store/playerStore"; import type { DraggedClipState } from "./useTimelineClipDrag"; import { commitDraggedClipMove, + commitZMirrorLaneMove, type DragCommitDeps, type TimelineMoveEdit, } from "./timelineClipDragCommit"; @@ -1024,3 +1025,120 @@ describe("commitDraggedClipMove", () => { }); }); }); + +describe("commitZMirrorLaneMove", () => { + const mirrorDeps = (elements: TimelineElement[], trackOrder: number[]) => { + const updateElement = vi.fn(); + const onMoveElements = vi.fn(); + return { + updateElement, + onMoveElements, + deps: { elements, trackOrder, updateElement, onMoveElements } as DragCommitDeps, + }; + }; + + it("kind:move persists start + display lane with the persistTrack override (same shape as a lane drag)", async () => { + // t sits at lane 2 in a sparse file (authored 7); the mirror lands it on + // lane 0 whose authored track is 3. + const t = { ...el("t", 2, 0, 10), authoredTrack: 7 }; + const elements = [{ ...el("a", 0, 20, 5), authoredTrack: 3 }, el("b", 1, 0, 10), t]; + const { updateElement, onMoveElements, deps } = mirrorDeps(elements, [0, 1, 2]); + const moved = await commitZMirrorLaneMove( + t, + { kind: "move", displayTrack: 0, persistTrack: 3 }, + deps, + "z-reorder:bring-forward:t", + ); + expect(moved).toBe(true); + // Optimistic store update: DISPLAY lane + the written authoredTrack mirror. + expect(updateElement).toHaveBeenCalledWith("t", { + start: 0, + track: 0, + authoredTrack: 3, + }); + // Persist: authored-space track, the z persist's coalesce key, lane-reorder op. + expect(onMoveElements).toHaveBeenCalledTimes(1); + const [persistEdits, coalesceKey, operation] = onMoveElements.mock.calls[0]; + expect(editMap(persistEdits)).toEqual({ t: { start: 0, track: 3 } }); + expect(coalesceKey).toBe("z-reorder:bring-forward:t"); + expect(operation).toBe("lane-reorder"); + }); + + it("kind:insert reuses the track-insert renumber core (+1 shift below the new lane)", async () => { + // a,b,c mutually overlapping on lanes 0/1/2. Mirror-insert c at row 1: c + // lands on the new lane, b (at/below) shifts down — identical to the drag + // insert test above, proving the shared core (no duplicated renumber logic). + const elements = [el("a", 0, 0, 5), el("b", 1, 0, 5), el("c", 2, 0, 5)]; + const { onMoveElements, deps } = mirrorDeps(elements, [0, 1, 2]); + const moved = await commitZMirrorLaneMove( + elements[2], + { kind: "insert", insertRow: 1 }, + deps, + "z-reorder:bring-forward:c", + ); + expect(moved).toBe(true); + expect(onMoveElements).toHaveBeenCalledTimes(1); + expect(onMoveElements.mock.calls[0][1]).toBe("z-reorder:bring-forward:c"); + expect(onMoveElements.mock.calls[0][2]).toBe("track-insert"); + const map = editMap(onMoveElements.mock.calls[0][0]); + expect(map.a.track).toBe(0); + expect(map.c.track).toBe(1); + expect(map.b.track).toBe(2); + }); + + it("never triggers the lane→z stacking sync (it would fight the just-set z values)", async () => { + // Even with BOTH z-sync deps supplied (the drag paths would engage them for + // a vertical move like this), the mirror commit must not emit stacking + // patches — the z values were just written by the user's menu action. + const t = el("t", 2, 0, 10); + const elements = [el("a", 0, 20, 5), el("b", 1, 0, 10), t]; + const { deps } = mirrorDeps(elements, [0, 1, 2]); + const onStackingPatches = vi.fn(); + const moved = await commitZMirrorLaneMove( + t, + { kind: "move", displayTrack: 0, persistTrack: 0 }, + { ...deps, readZIndex: () => 0, onStackingPatches }, + "z-reorder:bring-forward:t", + ); + await flushMicrotasks(); + expect(moved).toBe(true); + expect(onStackingPatches).not.toHaveBeenCalled(); + }); + + it("resolves false and rolls the store back when the persist rejects", async () => { + const t = el("t", 1, 0, 10); + const elements = [el("a", 0, 20, 5), t]; + const updateElement = vi.fn(); + const onMoveElements = vi.fn().mockRejectedValue(new Error("boom")); + const moved = await commitZMirrorLaneMove( + t, + { kind: "move", displayTrack: 0, persistTrack: 0 }, + { elements, trackOrder: [0, 1], updateElement, onMoveElements }, + "z-reorder:send-backward:t", + ); + expect(moved).toBe(false); + // Optimistic write then rollback to the original lane. + expect(updateElement).toHaveBeenLastCalledWith("t", { + start: 0, + track: 1, + authoredTrack: undefined, + }); + }); + + it("resolves false for a refused insert (locked clip would need renumbering)", async () => { + // b is locked and sits at/below the insert row, so the whole-set renumber + // is refused — no persist call. + const a = el("a", 0, 0, 5); + const b: TimelineElement = { ...el("b", 1, 0, 5), timelineLocked: true }; + const c = el("c", 2, 0, 5); + const { onMoveElements, deps } = mirrorDeps([a, b, c], [0, 1, 2]); + const moved = await commitZMirrorLaneMove( + c, + { kind: "insert", insertRow: 1 }, + deps, + "z-reorder:bring-forward:c", + ); + expect(moved).toBe(false); + expect(onMoveElements).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index d84982cb3b..7dc3125f02 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -1,5 +1,8 @@ import type { TimelineElement } from "../store/playerStore"; import type { DraggedClipState } from "./useTimelineClipDrag"; +// Type-only: erased at runtime, so the timelineZMirror → timelineClipDragCommit +// value-import edge stays acyclic. +import type { ZMirrorLaneMove } from "./timelineZMirror"; import { classifyZone, normalizeToZones } from "./timelineZones"; import { computeStackingPatches, type StackingPatch } from "./timelineStackingSync"; import { getTimelineEditCapabilities } from "./timelineEditing"; @@ -173,7 +176,7 @@ function persistMoveEdits( /** Same-source-file predicate: authored track numbers only compare within ONE * file's coordinate space (an expanded sub-comp child's authoredTrack is in ITS * file, not the host timeline's). `undefined` means the active composition. */ -const sameSourceFile = (a: TimelineElement, b: TimelineElement): boolean => +export const sameSourceFile = (a: TimelineElement, b: TimelineElement): boolean => (a.sourceFile ?? null) === (b.sourceFile ?? null); /** @@ -203,7 +206,7 @@ const sameSourceFile = (a: TimelineElement, b: TimelineElement): boolean => * Edge-created lanes (min-1 / max+1 inserts) route through the insert path, * never here. */ -function authoredTrackForLane( +export function authoredTrackForLane( lane: number, elements: TimelineElement[], dragged: TimelineElement, @@ -375,21 +378,23 @@ export function commitDraggedClipMove(drag: DraggedClipState, deps: DragCommitDe * vertical move syncs the dragged clip's stacking afterwards. */ // fallow-ignore-next-line complexity -function commitTrackInsert( - drag: DraggedClipState, - deps: DragCommitDeps, +function buildTrackInsertEdits( + element: TimelineElement, + previewStart: number, + insertRow: number, multi: { keys: ReadonlySet; movedStart: (e: TimelineElement) => number; } | null, -): void { + deps: DragCommitDeps, +): { candidate: TimelineElement[]; edits: TimelineMoveEdit[] } | null { const { elements, trackOrder } = deps; - const dragKey = keyOf(drag.element); - const targetTrack = insertTrackValue(trackOrder, drag.insertRow!); - // Drop-intent set: dragged clip at the fractional insert lane (so it sorts + const editKey = keyOf(element); + const targetTrack = insertTrackValue(trackOrder, insertRow); + // Drop-intent set: edited clip at the fractional insert lane (so it sorts // between its neighbours), selection members time-shifted, others as-is. const candidate = elements.map((e) => { - if (keyOf(e) === dragKey) return { ...e, start: drag.previewStart, track: targetTrack }; + if (keyOf(e) === editKey) return { ...e, start: previewStart, track: targetTrack }; if (multi?.keys.has(keyOf(e))) return { ...e, start: multi.movedStart(e) }; return e; }); @@ -407,7 +412,7 @@ function commitTrackInsert( console.warn( `[Timeline] Track insert refused: locked clip ${keyOf(src)} would need renumbering`, ); - return; + return null; } } const edits: TimelineMoveEdit[] = []; @@ -418,11 +423,32 @@ function commitTrackInsert( // a locked/implicit clip. if (!canMoveElement(src)) continue; const start = - keyOf(norm) === dragKey || multi?.keys.has(keyOf(norm)) - ? (multi?.movedStart(src) ?? drag.previewStart) + keyOf(norm) === editKey || multi?.keys.has(keyOf(norm)) + ? (multi?.movedStart(src) ?? previewStart) : src.start; edits.push({ element: src, updates: { start, track: norm.track } }); } + return { candidate, edits }; +} + +function commitTrackInsert( + drag: DraggedClipState, + deps: DragCommitDeps, + multi: { + keys: ReadonlySet; + movedStart: (e: TimelineElement) => number; + } | null, +): void { + const dragKey = keyOf(drag.element); + const built = buildTrackInsertEdits( + drag.element, + drag.previewStart, + drag.insertRow!, + multi, + deps, + ); + if (!built) return; + const { candidate, edits } = built; const coalesceKey = `clip-lane-move:${laneChangeGestureSeq++}`; void persistMoveEdits(edits, deps, coalesceKey, "track-insert").then((moved) => { @@ -449,6 +475,46 @@ function commitTrackInsert( }); } +/** + * Commit the timeline lane move that MIRRORS a canvas z-order menu action + * (resolveZMirrorLaneMove's non-null result). Same machinery as a lane drag: + * + * - kind "move": persistMoveEdits with `{start: element.start, track: displayTrack}` + * + `persistTrack` — identical shape to commitDraggedClipMove's lane-change + * branch (optimistic store update, authoredTrack mirror, rollback on failure). + * - kind "insert": buildTrackInsertEdits — the SAME renumber core commitTrackInsert + * uses — then the same atomic persist. + * + * Deliberately NO syncStackingForEdit here: the z values were just set by the + * user's menu action, and the lane→z sync would recompute (and fight) them. The + * mirror caller also omits `readZIndex`/`onStackingPatches` from `deps`, so even + * a future call into the sync would no-op (double protection; see + * useCanvasZOrderTimelineMirror). + * + * `coalesceKey` MUST be the z persist's key (`z-reorder::`) so + * editHistory folds the z write and this track write into ONE undo entry. + * + * Resolves `true` once the move persisted, `false` on rollback / refused insert. + */ +export function commitZMirrorLaneMove( + element: TimelineElement, + move: NonNullable, + deps: DragCommitDeps, + coalesceKey: string, +): Promise { + if (move.kind === "move") { + const edit: TimelineMoveEdit = { + element, + updates: { start: element.start, track: move.displayTrack }, + persistTrack: move.persistTrack, + }; + return persistMoveEdits([edit], deps, coalesceKey, "lane-reorder"); + } + const built = buildTrackInsertEdits(element, element.start, move.insertRow, null, deps); + if (!built || built.edits.length === 0) return Promise.resolve(false); + return persistMoveEdits(built.edits, deps, coalesceKey, "track-insert"); +} + /** * Compute + apply z-index patches for the edited clip(s) after a DELIBERATE * vertical lane change. Projects the drop-intent element set (`candidate`: the diff --git a/packages/studio/src/player/components/timelineStackingSync.ts b/packages/studio/src/player/components/timelineStackingSync.ts index e86bfe49af..6f771b7c5c 100644 --- a/packages/studio/src/player/components/timelineStackingSync.ts +++ b/packages/studio/src/player/components/timelineStackingSync.ts @@ -86,7 +86,10 @@ const contextKey = (el: { stackingContextId?: string | null }): string | null => * epsilon guards against float fuzz (e.g. 5.0000001 vs 5) spuriously overlapping two * abutting clips and shuffling lanes. The two are intended to differ, not align. */ -function overlapsInTime(a: StackingElement, b: StackingElement): boolean { +export function overlapsInTime( + a: Pick, + b: Pick, +): boolean { return a.start < b.start + b.duration - EPS && b.start < a.start + a.duration - EPS; } @@ -114,9 +117,14 @@ interface MutZ extends StackingElement { * Does `a` currently paint ON TOP of `b`? Higher z wins; equal z breaks by DOM * order (later in DOM paints on top). When either domIndex is absent, equal z is * treated as "not strictly above" (ambiguous) — callers should supply domIndex to - * disambiguate (see StackingElement.domIndex). Operates on resolved (`MutZ`) clips. + * disambiguate (see StackingElement.domIndex). Exported (like laneIsAbove) as the + * ONE paint-order predicate — the z-override badge (timelineZOverride.ts) must + * agree with the sync on what "paints above" means. */ -function paintsAbove(a: MutZ, b: MutZ): boolean { +export function paintsAbove( + a: Pick, + b: Pick, +): boolean { if (a.zIndex !== b.zIndex) return a.zIndex > b.zIndex; if (a.domIndex != null && b.domIndex != null) return a.domIndex > b.domIndex; return false; diff --git a/packages/studio/src/player/components/timelineZMirror.test.ts b/packages/studio/src/player/components/timelineZMirror.test.ts new file mode 100644 index 0000000000..ca30fbda32 --- /dev/null +++ b/packages/studio/src/player/components/timelineZMirror.test.ts @@ -0,0 +1,351 @@ +import { describe, expect, it } from "vitest"; +import type { TimelineElement } from "../store/playerStore"; +import { resolveZMirrorLaneMove, type ZMirrorInput } from "./timelineZMirror"; + +function el( + id: string, + track: number, + start: number, + duration: number, + extra: Partial = {}, +): TimelineElement { + return { id, key: id, tag: "video", start, duration, track, domId: id, ...extra }; +} + +function audio(id: string, track: number, start: number, duration: number): TimelineElement { + return el(id, track, start, duration, { tag: "audio" }); +} + +function resolve( + action: ZMirrorInput["action"], + element: TimelineElement, + elements: TimelineElement[], + crossedKey?: string | null, +) { + return resolveZMirrorLaneMove({ action, element, elements, crossedKey }); +} + +describe("resolveZMirrorLaneMove — bring-forward / send-backward", () => { + // Stack: a on lane 0, b on lane 1, target on lane 2 — all overlapping in time. + const stack = () => { + const a = el("a", 0, 0, 10); + const b = el("b", 1, 0, 10); + const t = el("t", 2, 0, 10); + return { a, b, t, elements: [a, b, t] }; + }; + + it("bring-forward with crossedKey lands on the closest free lane above the neighbor", () => { + // Free lane 0 exists above the crossed neighbor's lane... make lane 0 free by + // shifting a out of the span. + const a = el("a", 0, 20, 5); // lane 0 free over t's span + const b = el("b", 1, 0, 10); + const t = el("t", 2, 0, 10); + expect(resolve("bring-forward", t, [a, b, t], "b")).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 0, + }); + }); + + it("bring-forward with crossedKey inserts above the neighbor when no lane is free", () => { + const { t, elements } = stack(); + // Lanes 0 and 1 both occupied over t's span → new lane at the boundary + // ABOVE the crossed neighbor (row of lane 1 in the ascending order). + expect(resolve("bring-forward", t, elements, "b")).toEqual({ kind: "insert", insertRow: 1 }); + }); + + it("bring-forward without crossedKey uses the closest overlapping neighbor above", () => { + const { t, elements } = stack(); + // Closest overlapping neighbor above lane 2 is b (lane 1); lanes 0/1 are + // occupied → insert above b, same as the crossedKey case. + expect(resolve("bring-forward", t, elements)).toEqual({ kind: "insert", insertRow: 1 }); + }); + + it("bring-forward with an unknown crossedKey falls back to the temporal neighbor", () => { + const { t, elements } = stack(); + expect(resolve("bring-forward", t, elements, "nope")).toEqual({ + kind: "insert", + insertRow: 1, + }); + }); + + it("bring-forward returns null when nothing overlaps above and no crossedKey", () => { + const a = el("a", 0, 20, 5); // above but NOT overlapping in time + const t = el("t", 1, 0, 10); + expect(resolve("bring-forward", t, [a, t])).toBeNull(); + }); + + it("send-backward lands on the closest free lane below the neighbor", () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); + const c = el("c", 2, 20, 5); // lane 2 free over t's span + expect(resolve("send-backward", t, [t, b, c], "b")).toEqual({ + kind: "move", + displayTrack: 2, + persistTrack: 2, + }); + }); + + it("send-backward inserts below the neighbor when no lane below is free", () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); + const c = el("c", 2, 0, 10); + // Boundary below b's lane (row 1 + 1 = 2). + expect(resolve("send-backward", t, [t, b, c], "b")).toEqual({ kind: "insert", insertRow: 2 }); + }); + + it("send-backward returns null when nothing overlaps below and no crossedKey", () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 20, 5); + expect(resolve("send-backward", t, [t, b])).toBeNull(); + }); + + it("skips an occupied lane and takes the next free one in direction", () => { + // Above neighbor b (lane 2): lane 1 occupied over the span, lane 0 free. + const a = el("a", 0, 30, 5); + const x = el("x", 1, 5, 10); + const b = el("b", 2, 0, 10); + const t = el("t", 3, 0, 10); + expect(resolve("bring-forward", t, [a, x, b, t], "b")).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 0, + }); + }); + + it("returns null when the closest free lane is the clip's own lane (z/track divergence)", () => { + // Crossed neighbor sits BELOW the clip in lane space (diverged z): searching + // up from lane 2 finds lane 1 free — the clip's own lane → already in place. + const t = el("t", 1, 0, 10); + const b = el("b", 2, 0, 10); + expect(resolve("bring-forward", t, [t, b], "b")).toBeNull(); + }); +}); + +describe("resolveZMirrorLaneMove — bring-to-front / send-to-back", () => { + it("bring-to-front moves above the topmost temporally-overlapping clip", () => { + const a = el("a", 0, 20, 5); // lane 0 free over t's span + const b = el("b", 1, 0, 10); // topmost overlap + const c = el("c", 2, 0, 10); + const t = el("t", 3, 0, 10); + expect(resolve("bring-to-front", t, [a, b, c, t])).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 0, + }); + }); + + it("bring-to-front inserts above the topmost overlap when no lane is free", () => { + const b = el("b", 0, 0, 10); + const c = el("c", 1, 0, 10); + const t = el("t", 2, 0, 10); + expect(resolve("bring-to-front", t, [b, c, t])).toEqual({ kind: "insert", insertRow: 0 }); + }); + + it("bring-to-front is null when already topmost among overlaps (temporal scope)", () => { + // A clip exists on a higher lane but does NOT overlap in time — with the + // default temporal-overlap scope the target is already at the front. + const a = el("a", 0, 20, 5); + const t = el("t", 1, 0, 10); + const c = el("c", 2, 0, 10); + expect(resolve("bring-to-front", t, [a, t, c])).toBeNull(); + }); + + it("send-to-back moves below the bottommost temporally-overlapping clip", () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); // bottommost overlap + const c = el("c", 2, 20, 5); // lane 2 free over t's span + expect(resolve("send-to-back", t, [t, b, c])).toEqual({ + kind: "move", + displayTrack: 2, + persistTrack: 2, + }); + }); + + it("send-to-back inserts below the bottommost overlap when no lane is free", () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); + const c = el("c", 2, 0, 10); + expect(resolve("send-to-back", t, [t, b, c])).toEqual({ kind: "insert", insertRow: 3 }); + }); + + it("send-to-back is null when already bottommost among overlaps", () => { + const t = el("t", 1, 0, 10); + const a = el("a", 0, 0, 10); + expect(resolve("send-to-back", t, [a, t])).toBeNull(); + }); + + it("returns null when nothing overlaps at all", () => { + const t = el("t", 0, 0, 10); + const a = el("a", 1, 20, 5); + for (const action of ["bring-to-front", "send-to-back"] as const) { + expect(resolve(action, t, [t, a])).toBeNull(); + } + }); +}); + +describe("resolveZMirrorLaneMove — span freeness", () => { + it("a lane free at the clip's start but occupied later in the span is NOT free", () => { + const t = el("t", 2, 0, 10); + const b = el("b", 1, 0, 10); // crossed neighbor + // Lane 0: nothing at t=0, but occupied over [6, 9) — inside t's span. + const late = el("late", 0, 6, 3); + expect(resolve("bring-forward", t, [late, b, t], "b")).toEqual({ + kind: "insert", + insertRow: 1, + }); + }); + + it("half-open spans: a clip starting exactly at the moved clip's end does not occupy", () => { + const t = el("t", 2, 0, 10); + const b = el("b", 1, 0, 10); + const adjacent = el("adj", 0, 10, 5); // [10, 15) touches [0, 10) but no overlap + expect(resolve("bring-forward", t, [adjacent, b, t], "b")).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 0, + }); + }); + + it("freeness is file-agnostic: an other-file clip occupies the lane", () => { + const t = el("t", 2, 0, 10); + const b = el("b", 1, 0, 10); + const foreign = el("f", 0, 0, 10, { sourceFile: "sub.html" }); + expect(resolve("bring-forward", t, [foreign, b, t], "b")).toEqual({ + kind: "insert", + insertRow: 1, + }); + }); +}); + +describe("resolveZMirrorLaneMove — zone boundary (audio untouched)", () => { + // Visual lanes 0-1, audio lanes 2-3. + const zoned = () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); + const m = audio("music", 2, 0, 30); + const vo = audio("vo", 3, 0, 30); + return { t, b, m, vo, elements: [t, b, m, vo] }; + }; + + it("send-backward never lands on an audio lane — inserts at the zone boundary", () => { + const { t, elements } = zoned(); + // Lane 2 (audio) is out of bounds even though "below"; boundary row 2 sits + // between the bottom visual lane and the first audio lane — a visual insert. + expect(resolve("send-backward", t, elements, "b")).toEqual({ kind: "insert", insertRow: 2 }); + }); + + it("send-to-back stops at the visual zone edge", () => { + const { t, elements } = zoned(); + expect(resolve("send-to-back", t, elements)).toEqual({ kind: "insert", insertRow: 2 }); + }); + + it("audio clips never mirror (returns null)", () => { + const { m, elements } = zoned(); + for (const action of [ + "bring-to-front", + "bring-forward", + "send-backward", + "send-to-back", + ] as const) { + expect(resolve(action, m, elements)).toBeNull(); + } + }); + + it("audio clips do not count as overlap references for visual clips", () => { + // Only audio below the target → send-backward has no visual neighbor → null. + const t = el("t", 0, 0, 10); + const m = audio("music", 1, 0, 30); + expect(resolve("send-backward", t, [t, m])).toBeNull(); + expect(resolve("send-to-back", t, [t, m])).toBeNull(); + }); +}); + +describe("resolveZMirrorLaneMove — authored (persist) space", () => { + it("persistTrack takes the target lane occupant's authoredTrack, not the display lane", () => { + // Sparse file: authored tracks 3/5/7 displayed as lanes 0/1/2. Occupant of + // the free-over-span target lane 0 (authored 3) anchors the persist value. + const a = el("a", 0, 20, 5, { authoredTrack: 3 }); + const b = el("b", 1, 0, 10, { authoredTrack: 5 }); + const t = el("t", 2, 0, 10, { authoredTrack: 7 }); + expect(resolve("bring-forward", t, [a, b, t], "b")).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 3, + }); + }); + + it("falls back to nearest-same-file lane offset when the target lane has no same-file occupant", () => { + // The moved clip is an expanded sub-comp child; the target lane's only + // occupant belongs to the host file, so the persist value offsets from the + // nearest same-file lane instead (authored 4 at lane 1 → lane 0 = 3). + const host = el("h", 0, 20, 5); // host-file clip on the target lane (not overlapping) + const sib = el("s", 1, 0, 10, { sourceFile: "sub.html", authoredTrack: 4 }); + const t = el("t", 2, 0, 10, { sourceFile: "sub.html", authoredTrack: 5 }); + expect(resolve("bring-forward", t, [host, sib, t], "s")).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 3, + }); + }); +}); + +describe("resolveZMirrorLaneMove — stacking-context (source file) scoping", () => { + it("other-file clips are not overlap references (extremes computed per file)", () => { + // A host clip overlaps above the sub-comp child, but the child's own file + // has nothing above it → bring-to-front is null (already at ITS front). + const host = el("h", 0, 0, 10); + const t = el("t", 1, 0, 10, { sourceFile: "sub.html" }); + expect(resolve("bring-to-front", t, [host, t])).toBeNull(); + }); + + it("same-file overlaps in an expanded sub-comp resolve within the child's lanes", () => { + const host = el("h", 0, 0, 10); + const sib = el("s", 1, 0, 10, { sourceFile: "sub.html", authoredTrack: 0 }); + const t = el("t", 2, 0, 10, { sourceFile: "sub.html", authoredTrack: 1 }); + // Topmost same-file overlap is sib (lane 1); lane 0 is occupied by the host + // over the span (freeness is file-agnostic) → insert above sib's lane. + expect(resolve("bring-to-front", t, [host, sib, t])).toEqual({ + kind: "insert", + insertRow: 1, + }); + }); +}); + +describe("resolveZMirrorLaneMove — degenerate inputs and determinism", () => { + it("zero-duration element returns null", () => { + const t = el("t", 1, 0, 0); + const a = el("a", 0, 0, 10); + expect(resolve("bring-to-front", t, [a, t])).toBeNull(); + }); + + it("single-clip timeline returns null for every action", () => { + const t = el("t", 0, 0, 10); + for (const action of [ + "bring-to-front", + "bring-forward", + "send-backward", + "send-to-back", + ] as const) { + expect(resolve(action, t, [t])).toBeNull(); + } + }); + + it("identical inputs produce identical outputs (deterministic, input untouched)", () => { + const make = () => { + const a = el("a", 0, 20, 5, { authoredTrack: 3 }); + const b = el("b", 1, 0, 10, { authoredTrack: 5 }); + const t = el("t", 2, 0, 10, { authoredTrack: 7 }); + return { t, elements: [a, b, t] }; + }; + const first = make(); + const snapshot = structuredClone(first.elements); + const r1 = resolve("bring-forward", first.t, first.elements, "b"); + const r2 = resolve("bring-forward", first.t, first.elements, "b"); + const fresh = make(); + const r3 = resolve("bring-forward", fresh.t, fresh.elements, "b"); + expect(r1).toEqual(r2); + expect(r1).toEqual(r3); + expect(first.elements).toEqual(snapshot); // pure — never mutates its input + }); +}); diff --git a/packages/studio/src/player/components/timelineZMirror.ts b/packages/studio/src/player/components/timelineZMirror.ts new file mode 100644 index 0000000000..33e6d849c6 --- /dev/null +++ b/packages/studio/src/player/components/timelineZMirror.ts @@ -0,0 +1,183 @@ +import type { TimelineElement } from "../store/playerStore"; +import { classifyZone } from "./timelineZones"; +import { isLaneFree, timeRangesOverlap } from "./timelineCollision"; +import { authoredTrackForLane, sameSourceFile } from "./timelineClipDragCommit"; + +/** + * Mirror a canvas z-order action (Bring to Front / Bring Forward / Send Backward / + * Send to Back) into a timeline LANE move — the pure resolver, no UI wiring. + * + * ── The model ──────────────────────────────────────────────────────────────── + * Track order is the DEFAULT paint order; authored z is the ADVANCED override. + * Render truth stays z — the renderer never reads track index — and the studio + * maintains z ↔ track consistency at EDIT time: a deliberate vertical lane move + * syncs z (timelineStackingSync), and a z-order menu action calls THIS resolver + * to compute the accompanying lane move. When the user authors z that diverges + * from track order, the divergence is surfaced by a badge (computeZOverrideKeys + * in timelineZOverride.ts, rendered by TimelineClip); + * the mirror never fights an authored override, it only keeps the default in + * step. + * + * ── Locked rules (agreed design — do not re-litigate here) ─────────────────── + * - The mirror computes a lane move to ACCOMPANY a z action on a timeline clip; + * it never replaces the z patch. + * - Move the clip to the closest track in the action's direction that is FREE + * over the clip's whole [start, start + duration) span; if no free track + * exists in that direction, CREATE one adjacent to the reference neighbor + * (commitTrackInsert semantics). + * - Direction: bring-forward/front = toward LOWER display lanes (up = above); + * send-backward/back = toward HIGHER lanes, but only within the visual zone — + * the audio zone is untouched and never crossed (a bottom-of-zone insert lands + * AT the visual/audio boundary, i.e. still a visual lane). + * - Reference scope: same stacking context = same source file = same timeline + * lane space (matches the menu's sibling scoping). The comparison set for + * "which track is above/below me" is same-file clips; lane FREENESS is + * file-agnostic (any clip in the zone occupies its lane for everyone). + * - Non-clip decorations (no timeline presence) are out of scope — callers keep + * z-only behavior for them. Audio elements never mirror (z on audio is + * meaningless); the resolver returns null. + * + * ── OPEN product question ──────────────────────────────────────────────────── + * send-to-back / bring-to-front scope: below/above EVERYTHING visual, or only + * the clips that temporally overlap the moved clip? The default implemented + * here is TEMPORAL-OVERLAP scope (the extreme is computed over same-file clips + * that overlap the moved clip in time), pending M/Bin sign-off. A clip with no + * temporal overlaps in the direction is "already at the extreme" → null. + * + * Deterministic: a pure function of its inputs — no Date, no randomness, no DOM. + */ + +export type ZMirrorAction = "bring-to-front" | "bring-forward" | "send-backward" | "send-to-back"; + +export interface ZMirrorInput { + action: ZMirrorAction; + /** The clip acted on — store/display space (post-normalizeToZones lanes), + * carrying `authoredTrack` when the display lane diverges from the file. */ + element: TimelineElement; + /** The expanded display element set (same set the drag commit reasons on). */ + elements: TimelineElement[]; + /** Timeline key of the neighbor the z action stepped over (forward/backward), + * when known — see resolveCrossedNeighbor in canvasContextMenuZOrder. */ + crossedKey?: string | null; +} + +export type ZMirrorLaneMove = + | { + /** Land on an existing display lane. */ + kind: "move"; + /** Display lane to move to (store space). */ + displayTrack: number; + /** Authored-space value to write (authoredTrackForLane translation). */ + persistTrack: number; + } + | { + /** Create a new lane: boundary row compatible with commitTrackInsert's + * insertRow (index into the ascending display trackOrder; 0 = above the + * top lane, length = below the bottom). */ + kind: "insert"; + insertRow: number; + } + | null; + +const keyOf = (el: TimelineElement): string => el.key ?? el.id; + +/** Ascending unique display lanes of `elements` — identical to how Timeline.tsx + * builds `trackOrder`, so `insertRow` indexes the same boundary space. Exported + * so the mirror wiring can hand commitZMirrorLaneMove the matching trackOrder. */ +export function displayTrackOrder(elements: TimelineElement[]): number[] { + return [...new Set(elements.map((el) => el.track))].sort((a, b) => a - b); +} + +/** + * Resolve the timeline lane move that mirrors a z-order action on `element`. + * Returns null when no timeline mirror applies: audio / zero-length clips, no + * reference neighbor in the action's direction (the menu action was likely + * disabled or a no-op), or the clip is already laned where the action puts it. + */ +export function resolveZMirrorLaneMove(input: ZMirrorInput): ZMirrorLaneMove { + const { action, element, elements } = input; + if (classifyZone(element) === "audio") return null; + if (!(element.duration > 0)) return null; + + const selfKey = keyOf(element); + const start = element.start; + const end = element.start + element.duration; + const up = action === "bring-forward" || action === "bring-to-front"; + + // Same stacking context (= same source file), visual, temporally overlapping. + const overlapSet = elements.filter( + (el) => + keyOf(el) !== selfKey && + classifyZone(el) === "visual" && + sameSourceFile(el, element) && + timeRangesOverlap(start, end, el.start, el.start + el.duration), + ); + + const referenceLane = resolveReferenceLane(input, overlapSet, up); + if (referenceLane == null) return null; + + const order = displayTrackOrder(elements); + const visualLanes = displayTrackOrder(elements.filter((el) => classifyZone(el) === "visual")); + + // Closest free lane strictly beyond the reference, lane-by-lane in direction, + // whole-span freeness, same zone (visual lanes only — never into audio). + const refIdx = visualLanes.indexOf(referenceLane); + if (refIdx === -1) return null; // reference is not a visual lane — no mirror + const step = up ? -1 : 1; + for (let i = refIdx + step; i >= 0 && i < visualLanes.length; i += step) { + const lane = visualLanes[i]; + if (isLaneFree(elements, lane, start, end, selfKey)) { + // The closest free lane is the clip's OWN lane (possible only when z and + // track had diverged): the clip already sits where the action puts it. + if (lane === element.track) return null; + return { + kind: "move", + displayTrack: lane, + persistTrack: authoredTrackForLane(lane, elements, element), + }; + } + } + + // No free lane before the zone edge → create one adjacent to the reference: + // the boundary between the reference lane and the next lane in direction. + return { kind: "insert", insertRow: order.indexOf(referenceLane) + (up ? 0 : 1) }; +} + +/** + * The lane the search starts from (the "reference neighbor"): + * - forward/backward: the crossed neighbor when provided and valid (a visual + * clip in the set); otherwise the closest temporally-overlapping same-file + * clip in the direction. None → null (the menu was probably disabled). + * - front/back: the extreme of the temporal-overlap set — topmost (lowest lane) + * for front, bottommost (highest lane) for back — restricted to overlaps + * strictly beyond the clip's own lane. None → already at the extreme → null. + */ +function resolveReferenceLane( + input: ZMirrorInput, + overlapSet: TimelineElement[], + up: boolean, +): number | null { + const stepAction = input.action === "bring-forward" || input.action === "send-backward"; + if (stepAction) { + const crossedLane = crossedNeighborLane(input); + // Unknown / absent / non-visual crossed key → the temporal neighbor below. + if (crossedLane != null) return crossedLane; + } + + // Overlapping same-file lanes strictly beyond the moved clip's lane, in direction. + const lanes = overlapSet + .map((el) => el.track) + .filter((lane) => (up ? lane < input.element.track : lane > input.element.track)); + if (lanes.length === 0) return null; + + // Step actions want the CLOSEST lane in direction (max when up, min when + // down); front/back want the EXTREME of the set (min when up, max when down). + return (stepAction === up ? Math.max : Math.min)(...lanes); +} + +/** The crossed neighbor's display lane, when the key names a visual clip in the set. */ +function crossedNeighborLane({ elements, crossedKey }: ZMirrorInput): number | null { + if (crossedKey == null) return null; + const crossed = elements.find((el) => keyOf(el) === crossedKey); + return crossed && classifyZone(crossed) === "visual" ? crossed.track : null; +} diff --git a/packages/studio/src/player/components/timelineZOverride.test.ts b/packages/studio/src/player/components/timelineZOverride.test.ts new file mode 100644 index 0000000000..3b42eadee3 --- /dev/null +++ b/packages/studio/src/player/components/timelineZOverride.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "vitest"; +import type { TimelineElement } from "../store/playerStore"; +import { computeZOverrideKeys } from "./timelineZOverride"; + +// Elements are listed in discovery order — the array index is the DOM document +// position (the equal-z tie-break input), matching the production caller. +function el( + id: string, + track: number, + zIndex: number | undefined, + extra: Partial = {}, +): TimelineElement { + return { id, key: id, tag: "video", start: 0, duration: 10, track, domId: id, zIndex, ...extra }; +} + +describe("computeZOverrideKeys", () => { + it("no badge when paint order matches lane order", () => { + // top lane (0) has the higher z — consistent. + const top = el("top", 0, 5); + const bottom = el("bottom", 1, 2); + expect(computeZOverrideKeys([top, bottom]).size).toBe(0); + }); + + it("marks BOTH ends of a strict contradiction (either listing direction)", () => { + // top lane paints BELOW the bottom lane — both clips carry the badge. + const top = el("top", 0, 1); + const bottom = el("bottom", 1, 9); + expect(computeZOverrideKeys([top, bottom])).toEqual(new Set(["top", "bottom"])); + // Same pair discovered in the other DOM order — same verdict (z is strict, + // so domIndex never enters). + expect(computeZOverrideKeys([bottom, top])).toEqual(new Set(["top", "bottom"])); + }); + + it("equal z resolves by DOM order: later-in-DOM paints on top", () => { + // Consistent: the top-lane clip is later in the DOM, so at equal z it paints + // on top — exactly what its lane says. + const bottom = el("bottom", 1, 3); + const top = el("top", 0, 3); + expect(computeZOverrideKeys([bottom, top]).size).toBe(0); + // Contradiction: the top-lane clip is EARLIER in the DOM, so at equal z it + // paints below — both ends marked. + expect(computeZOverrideKeys([top, bottom])).toEqual(new Set(["top", "bottom"])); + }); + + it("non-overlapping clips never contradict", () => { + const top = el("top", 0, 1); + const bottom = el("bottom", 1, 9, { start: 20, duration: 5 }); + expect(computeZOverrideKeys([top, bottom]).size).toBe(0); + }); + + it("abutting clips (epsilon half-open spans) do not overlap", () => { + const a = el("a", 0, 1); + const b = el("b", 1, 9, { start: 10, duration: 5 }); // starts exactly at a's end + expect(computeZOverrideKeys([a, b]).size).toBe(0); + }); + + it("clips in different stacking contexts are never compared", () => { + const a = el("a", 0, 1, { stackingContextId: "ctx-1" }); + const b = el("b", 1, 9, { stackingContextId: "ctx-2" }); + expect(computeZOverrideKeys([a, b]).size).toBe(0); + // null and undefined both mean the root context — those DO compare. + const c = el("c", 0, 1, { stackingContextId: null }); + const d = el("d", 1, 9, { stackingContextId: undefined }); + expect(computeZOverrideKeys([c, d])).toEqual(new Set(["c", "d"])); + }); + + it("audio clips and unresolved-z clips are excluded", () => { + const visual = el("v", 1, 9); + const music = el("m", 0, 1, { tag: "audio" }); // would contradict if compared + const unknownZ = el("u", 0, undefined); // unresolved z never fabricates a contradiction + const nanZ = el("n", 0, Number.NaN); + expect(computeZOverrideKeys([music, unknownZ, nanZ, visual]).size).toBe(0); + }); + + it("zero-duration clips are excluded", () => { + const ghost = el("g", 0, 1, { duration: 0 }); + const solid = el("s", 1, 9); + expect(computeZOverrideKeys([ghost, solid]).size).toBe(0); + }); + + it("marks only the contradicting pair in a mixed stack", () => { + // Lanes 0/1/2 with z 9/1/5: (top,mid) contradicts (1 < ... wait — top z9 + // beats both, consistent; mid z1 vs low z5 contradicts (mid is the upper + // lane but paints below) → mid + low marked, top clean. + const top = el("top", 0, 9); + const mid = el("mid", 1, 1); + const low = el("low", 2, 5); + expect(computeZOverrideKeys([top, mid, low])).toEqual(new Set(["mid", "low"])); + }); +}); diff --git a/packages/studio/src/player/components/timelineZOverride.ts b/packages/studio/src/player/components/timelineZOverride.ts new file mode 100644 index 0000000000..e10ea611b4 --- /dev/null +++ b/packages/studio/src/player/components/timelineZOverride.ts @@ -0,0 +1,97 @@ +/** + * timelineZOverride — detect clips whose PAINT order diverges from LANE order. + * + * Track order is the DEFAULT paint order; authored z is the ADVANCED override + * (see timelineZMirror's model comment). When a user authors z that contradicts + * lane order, the timeline surfaces it with a "z" badge on the affected clips + * (TimelineClip) instead of silently showing a lane order the canvas ignores. + * + * Rule (deterministic, symmetric): mark clip X when there exists a + * temporally-overlapping, same-stacking-context, visual neighbor Y such that + * `laneIsAbove(X, Y) XOR paintsAbove(X, Y)` — the lane relation and the paint + * relation disagree. Both ends of a strict contradiction get marked (X above in + * lane but painting below ⇒ Y below in lane but painting above). The predicates + * are the EXACT ones timelineStackingSync uses (laneIsAbove, paintsAbove with + * the z/domIndex tie-break, epsilon overlapsInTime) so badge and sync can never + * disagree about what a contradiction is. + * + * zIndex source: the store's `TimelineElement.zIndex`. It is read from the live + * DOM at element-build time, and — on this branch — handleDomZIndexReorderCommit + * syncs it synchronously on every z commit via the entry's timeline key, so the + * store z is fresh immediately after a canvas z-order menu action (no reload + * needed for the badge to update). Clips with an UNRESOLVED z (undefined / + * non-finite) are excluded outright, mirroring the sync's NaN exclusion — an + * unknown z must not fabricate a phantom contradiction. + * + * Cost: O(n · overlaps) pairwise scan within each stacking context — fine at + * timeline sizes (dozens to low hundreds of clips, see TimelineLanes' + * no-virtualization note). + */ + +import type { TimelineElement } from "../store/playerStore"; +import { classifyZone } from "./timelineZones"; +import { laneIsAbove, overlapsInTime, paintsAbove } from "./timelineStackingSync"; + +interface OverrideCandidate { + key: string; + start: number; + duration: number; + track: number; + zIndex: number; + domIndex: number; + contextKey: string | null; +} + +/** Visual clips with a resolved z, projected for the pairwise scan. Audio has + * no visual stacking; an unresolved z (undefined / NaN) is excluded outright. + * `domIndex` is the discovery-order array index (= DOM document position). */ +function toOverrideCandidates(elements: TimelineElement[]): OverrideCandidate[] { + const candidates: OverrideCandidate[] = []; + for (let domIndex = 0; domIndex < elements.length; domIndex += 1) { + const el = elements[domIndex]; + const comparable = + classifyZone(el) !== "audio" && el.duration > 0 && Number.isFinite(el.zIndex ?? Number.NaN); + if (!comparable) continue; + candidates.push({ + key: el.key ?? el.id, + start: el.start, + duration: el.duration, + track: el.track, + zIndex: el.zIndex!, + domIndex, + // Same normalization as timelineStackingSync's contextKey: null and + // undefined both mean the root stacking context. + contextKey: el.stackingContextId ?? null, + }); + } + return candidates; +} + +/** The pair is comparable at all: leaf z is meaningless across stacking + * contexts, and only temporal overlap creates a paint relation. */ +function pairComparable(x: OverrideCandidate, y: OverrideCandidate): boolean { + return x.contextKey === y.contextKey && overlapsInTime(x, y); +} + +/** + * Keys of clips whose paint order contradicts their lane order. + * + * @param elements The expanded DISPLAY element set in discovery order — its + * array index is the DOM document position (the same + * assumption syncStackingForEdit documents), which feeds the + * equal-z DOM-order tie-break. + */ +export function computeZOverrideKeys(elements: TimelineElement[]): Set { + const candidates = toOverrideCandidates(elements); + const marked = new Set(); + for (let i = 0; i < candidates.length; i += 1) { + for (let j = i + 1; j < candidates.length; j += 1) { + const x = candidates[i]; + const y = candidates[j]; + if (!pairComparable(x, y)) continue; + if (laneIsAbove(x, y) !== paintsAbove(x, y)) marked.add(x.key); + if (laneIsAbove(y, x) !== paintsAbove(y, x)) marked.add(y.key); + } + } + return marked; +} diff --git a/packages/studio/src/player/components/useTimelineTrackDerivations.ts b/packages/studio/src/player/components/useTimelineTrackDerivations.ts new file mode 100644 index 0000000000..d8fb2c3219 --- /dev/null +++ b/packages/studio/src/player/components/useTimelineTrackDerivations.ts @@ -0,0 +1,45 @@ +import { useMemo } from "react"; +import type { TimelineElement } from "../store/playerStore"; +import { getTrackStyle, type TrackVisualStyle } from "./timelineIcons"; +import { computeZOverrideKeys } from "./timelineZOverride"; + +/** + * Per-render track derivations Timeline.tsx feeds the canvas/lanes: the lane → + * clip grouping (`tracks`, ascending), per-lane visual styles, the ascending + * `trackOrder`, and the z-override badge set. Extracted from Timeline.tsx as a + * cohesive unit (600-line studio cap); each memo keys on the expanded display + * element set exactly as before. + */ +export function useTimelineTrackDerivations(expandedElements: TimelineElement[]): { + tracks: [number, TimelineElement[]][]; + trackStyles: Map; + trackOrder: number[]; + zOverrideKeys: ReadonlySet; +} { + const tracks = useMemo(() => { + const map = new Map(); + for (const el of expandedElements) { + const list = map.get(el.track) ?? []; + list.push(el); + map.set(el.track, list); + } + return Array.from(map.entries()).sort(([a], [b]) => a - b); + }, [expandedElements]); + + const trackStyles = useMemo(() => { + const map = new Map(); + for (const [trackNum, els] of tracks) { + map.set(trackNum, getTrackStyle(els[0]?.tag ?? "")); + } + return map; + }, [tracks]); + + const trackOrder = useMemo(() => tracks.map(([trackNum]) => trackNum), [tracks]); + + // Clips whose authored z contradicts lane order get a "z" badge (see + // timelineZOverride.ts). Memoized on the expanded set: store zIndex is synced + // synchronously on z commits, so this re-derives right after a menu action. + const zOverrideKeys = useMemo(() => computeZOverrideKeys(expandedElements), [expandedElements]); + + return { tracks, trackStyles, trackOrder, zOverrideKeys }; +} From f609bd742fd4de5db23e025c39b515072f9a5674 Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Mon, 13 Jul 2026 18:24:33 -0700 Subject: [PATCH 02/19] fix(studio): fold mirrored z-order gestures into one undo entry across slow persists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live verification caught the z write and the mirrored lane write splitting into two undo entries: the mirror runs after the z persist's server round trip, which exceeds editHistory's default 300ms coalesce window under real latency (the unit test's deterministic clock sat inside it). zReorderCoalesceKey now mints a per-gesture-unique key (monotonic seq, the laneChangeGestureSeq precedent) and both records carry coalesceMs Infinity — distinct gestures can never merge, and one gesture always folds regardless of write latency. coalesceMs threaded through the persist chain alongside coalesceKey. Also hardens the existing lane-drag move->z fold, which had the same latent split. Fold test now simulates a 400ms gap (failed before the fix, passes after); a two-separate-gestures test asserts two entries. --- packages/studio/src/App.tsx | 6 +- .../src/components/nle/TimelinePane.test.ts | 3 + .../src/components/nle/TimelinePane.tsx | 12 ++- .../useCanvasZOrderTimelineMirror.test.tsx | 79 +++++++++++++++++-- .../nle/useCanvasZOrderTimelineMirror.ts | 21 ++++- .../nle/useTimelineEditCallbacks.ts | 1 + .../studio/src/hooks/domEditCommitTypes.ts | 2 + .../src/hooks/timelineEditingHelpers.ts | 3 + .../studio/src/hooks/timelineMoveAdapter.ts | 3 +- .../src/hooks/useDomEditCommits.test.tsx | 4 + .../studio/src/hooks/useDomEditCommits.ts | 1 + .../src/hooks/useElementLifecycleOps.test.tsx | 12 ++- .../src/hooks/useElementLifecycleOps.ts | 33 ++++++-- .../src/hooks/useTimelineGroupEditing.ts | 13 ++- .../player/components/timelineCallbacks.ts | 5 +- .../components/timelineClipDragCommit.ts | 22 ++++-- 16 files changed, 187 insertions(+), 33 deletions(-) diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 4cb09db319..52048298fb 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -189,10 +189,10 @@ export function StudioApp() { }>, coalesceKey?: string, operation: TimelineMoveOperation = "timing", + coalesceMs?: number, ) => { - await persistTimelineMoveEditsAtomically(edits, coalesceKey, operation, { - handleTimelineGroupMove: timelineEditing.handleTimelineGroupMove, - }); + const deps = { handleTimelineGroupMove: timelineEditing.handleTimelineGroupMove }; + await persistTimelineMoveEditsAtomically(edits, coalesceKey, operation, deps, coalesceMs); }, [timelineEditing.handleTimelineGroupMove], ); diff --git a/packages/studio/src/components/nle/TimelinePane.test.ts b/packages/studio/src/components/nle/TimelinePane.test.ts index e01813bc43..1e4c0c082f 100644 --- a/packages/studio/src/components/nle/TimelinePane.test.ts +++ b/packages/studio/src/components/nle/TimelinePane.test.ts @@ -22,6 +22,7 @@ describe("TimelinePane move wrapper", () => { "clip-lane-move:7", "track-insert", onMoveElements, + Number.POSITIVE_INFINITY, ); expect(onMoveElements).toHaveBeenCalledWith( [ @@ -32,6 +33,8 @@ describe("TimelinePane move wrapper", () => { ], "clip-lane-move:7", "track-insert", + // The per-gesture coalesce window rides along with the shared key. + Number.POSITIVE_INFINITY, ); }); diff --git a/packages/studio/src/components/nle/TimelinePane.tsx b/packages/studio/src/components/nle/TimelinePane.tsx index 1b0b402eab..3c9587f756 100644 --- a/packages/studio/src/components/nle/TimelinePane.tsx +++ b/packages/studio/src/components/nle/TimelinePane.tsx @@ -21,7 +21,9 @@ export function forwardRebasedTimelineMoveElements( edits: TimelineMoveEdit[], coalesceKey?: string, operation?: TimelineMoveOperation, + coalesceMs?: number, ) => Promise | void, + coalesceMs?: number, ) { return onMoveElements( edits.map(({ element, updates }) => { @@ -34,6 +36,7 @@ export function forwardRebasedTimelineMoveElements( }), coalesceKey, operation, + coalesceMs, ); } @@ -160,6 +163,7 @@ export function TimelinePane({ edits: Array<{ element: TimelineElement; updates: Pick }>, coalesceKey?: string, operation?: TimelineMoveOperation, + coalesceMs?: number, ) => { // Match the sibling handlers: report the telemetry when the batch touches at // least one expanded sub-comp child (the clips being rebased to local coords). @@ -167,7 +171,13 @@ export function TimelinePane({ trackStudioExpandedClipEdit({ action: "move" }); } if (!onMoveElements) return; - return forwardRebasedTimelineMoveElements(edits, coalesceKey, operation, onMoveElements); + return forwardRebasedTimelineMoveElements( + edits, + coalesceKey, + operation, + onMoveElements, + coalesceMs, + ); }, [onMoveElements], ); diff --git a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx index 4a9fd7e338..f9d320b242 100644 --- a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx +++ b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx @@ -84,6 +84,7 @@ function mountMirrorHarness(history: { label: string, kind: "manual" | "timeline", coalesceKey: string, + coalesceMs: number | undefined, after: string, ) => { const entry = buildEditHistoryEntry({ @@ -92,6 +93,7 @@ function mountMirrorHarness(history: { label, kind, coalesceKey, + coalesceMs, now: history.now(), files: { "index.html": { before: history.fileContent.current, after } }, }); @@ -99,9 +101,14 @@ function mountMirrorHarness(history: { history.state = pushEditHistoryEntry(history.state, entry); }; - const onMoveElements: TimelineEditCallbacks["onMoveElements"] = (_edits, coalesceKey) => { + const onMoveElements: TimelineEditCallbacks["onMoveElements"] = ( + _edits, + coalesceKey, + _operation, + coalesceMs, + ) => { history.moveCoalesceKeys.push(coalesceKey ?? ""); - record("Move timeline clips", "timeline", coalesceKey ?? "", "C-move"); + record("Move timeline clips", "timeline", coalesceKey ?? "", coalesceMs, "C-move"); }; const api: Partial = {}; @@ -116,7 +123,7 @@ function mountMirrorHarness(history: { reloadPreview: vi.fn(), clearDomSelection: vi.fn(), commitDomEditPatchBatches: async (_batches, options) => { - record(options.label, "manual", options.coalesceKey, "B-z"); + record(options.label, "manual", options.coalesceKey, options.coalesceMs, "B-z"); }, }); api.commitZ = handleDomZIndexReorderCommit; @@ -135,10 +142,12 @@ function makeHistory() { let tick = 1000; return { state: createEmptyEditHistory(), - // Deterministic clock: consecutive records land 50ms apart — inside the + // Deterministic clock: consecutive records land 400ms apart — PAST the // reducer's default 300ms coalesce window, as in the live flow where the - // mirror is dispatched right after the z persist resolves. - now: () => (tick += 50), + // mirror is dispatched only after the z persist's server round-trip + // resolves (real network latency exceeds 300ms). The fold must therefore + // ride the gesture's explicit coalesceMs window, not the default. + now: () => (tick += 400), fileContent: { current: "A-original" }, moveCoalesceKeys: [] as string[], }; @@ -164,7 +173,10 @@ describe("useCanvasZOrderTimelineMirror", () => { { element: target, zIndex: 7, id: "t", sourceFile: "index.html", key: "index.html#t" }, ]; const coalesceKey = zReorderCoalesceKey(entries, "bring-forward"); - expect(coalesceKey).toBe("z-reorder:bring-forward:t"); + // Gesture-unique key: action + ids + a per-call gesture sequence, so two + // SEPARATE actions on the same selection never share a key (see the + // two-gestures test below) while this gesture's two records do. + expect(coalesceKey).toMatch(/^z-reorder:bring-forward:t:g\d+$/); await act(async () => { // The PreviewOverlays wiring: z commit first, mirror after it resolves, @@ -194,6 +206,59 @@ describe("useCanvasZOrderTimelineMirror", () => { expect(t?.track).toBe(0); }); + it("two SEPARATE gestures on the same selection produce TWO undo entries (distinct keys)", async () => { + // Two consecutive gestures on the same element: each mints its own coalesce + // key (gesture sequence), so even an unbounded per-gesture window must never + // merge distinct user actions into one undo step. + setStoreElements([ + storeEl("t", 0, 0, 10), + storeEl("b", 1, 0, 10), + storeEl("c", 2, 0, 10), + storeEl("a", 3, 20, 5), // free lane 3 over t's span + ]); + const history = makeHistory(); + const api = mountMirrorHarness(history); + + const target = domTarget("t"); + const entries: ReorderEntries = [ + { element: target, zIndex: 7, id: "t", sourceFile: "index.html", key: "index.html#t" }, + ]; + + // Identical action + identical selection still mints a fresh key per call. + expect(zReorderCoalesceKey(entries, "send-backward")).not.toBe( + zReorderCoalesceKey(entries, "send-backward"), + ); + + const keys: string[] = []; + // Gesture 1: t (lane 0) sent backward past b → lands on the free lane 3. + // Gesture 2: t brought forward past c → back onto the now-free lane 0. + // Both gestures mirror (lane move persists), so each records a z+move pair. + const gestures = [ + { action: "send-backward" as const, crossedId: "b" }, + { action: "bring-forward" as const, crossedId: "c" }, + ]; + for (const { action, crossedId } of gestures) { + const coalesceKey = zReorderCoalesceKey(entries, action); + keys.push(coalesceKey); + await act(async () => { + await api.commitZ(entries, coalesceKey, action); + const mirrored = await api.mirror({ + selectionKey: "index.html#t", + action, + crossed: domTarget(crossedId), + sourceFile: "index.html", + coalesceKey, + }); + expect(mirrored).toBe(true); + }); + } + + expect(keys[0]).not.toBe(keys[1]); // fresh key per gesture + // Each gesture folded its own z+move pair, but the two gestures stayed apart. + expect(history.state.undo).toHaveLength(2); + expect(history.moveCoalesceKeys).toEqual(keys); + }); + it("z-only actions leave the timeline untouched (resolver null → single z undo entry)", async () => { // t has NO overlapping neighbor above → bring-forward has no lane mirror. setStoreElements([storeEl("a", 0, 20, 5), storeEl("t", 1, 0, 10)]); diff --git a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts index 665eb10eee..7e2833057b 100644 --- a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts +++ b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.ts @@ -32,6 +32,10 @@ export interface MirrorZOrderInput { * resolved — serializing the two same-file writes, exactly like the lane-drag's * move→z ordering (see persistMoveEdits' doc) — and with the SAME coalesce key * the z persist recorded, so editHistory folds both records into one undo entry. + * Because the z round-trip puts a real-network gap between the two records, the + * lane persist carries an unbounded per-gesture coalesce window (see + * commitZMirrorLaneMove) — the shared key is unique per gesture + * (zReorderCoalesceKey's gesture seq), so the fold stays gesture-scoped. * * Element source: `useExpandedTimelineElements()` — the same expanded display * set the Timeline renders and the resolver expects (post-normalizeToZones @@ -93,13 +97,26 @@ export function useCanvasZOrderTimelineMirror(): (input: MirrorZOrderInput) => P trackOrder: displayTrackOrder(els), updateElement: (key, updates) => usePlayerStore.getState().updateElement(key, updates), onMoveElements: onMoveElements - ? (edits, coalesceKey, operation) => - forwardRebasedTimelineMoveElements(edits, coalesceKey, operation, onMoveElements) + ? (edits, coalesceKey, operation, coalesceMs) => + forwardRebasedTimelineMoveElements( + edits, + coalesceKey, + operation, + onMoveElements, + coalesceMs, + ) : undefined, // NO readZIndex / onStackingPatches: see the hook doc — the lane→z // stacking sync must not re-trigger and fight the just-set z values. }, input.coalesceKey, + // Unbounded fold window: this record lands only AFTER the z persist's + // server round-trip resolved, so the gap between the gesture's two + // records exceeds editHistory's 300ms default under real latency and + // the fold would silently split into two undo entries. The shared key + // is unique per gesture (zReorderCoalesceKey's gesture seq), so the + // unbounded window can never merge two distinct user actions. + Number.POSITIVE_INFINITY, ); }, [onMoveElements], diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts index ec6f9ff725..7413aa18a4 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts @@ -22,6 +22,7 @@ export interface TimelineEditCallbackDeps { edits: Array<{ element: TimelineElement; updates: Pick }>, coalesceKey?: string, operation?: TimelineMoveOperation, + coalesceMs?: number, ) => Promise | void; handleTimelineElementResize: ( element: TimelineElement, diff --git a/packages/studio/src/hooks/domEditCommitTypes.ts b/packages/studio/src/hooks/domEditCommitTypes.ts index d8e762f1a1..ce41752ba8 100644 --- a/packages/studio/src/hooks/domEditCommitTypes.ts +++ b/packages/studio/src/hooks/domEditCommitTypes.ts @@ -11,6 +11,8 @@ export type CommitDomEditPatchBatches = ( options: { label: string; coalesceKey: string; + /** Per-entry undo coalesce window override (ms) — see EditHistoryEntry.coalesceMs. */ + coalesceMs?: number; /** * Request skipping the preview iframe reload after a successful persist. * Only honored when the persist is provably in sync with the live DOM: diff --git a/packages/studio/src/hooks/timelineEditingHelpers.ts b/packages/studio/src/hooks/timelineEditingHelpers.ts index 6304e259da..f2b391cddd 100644 --- a/packages/studio/src/hooks/timelineEditingHelpers.ts +++ b/packages/studio/src/hooks/timelineEditingHelpers.ts @@ -299,6 +299,8 @@ export interface PersistTimelineBatchEditInput { domEditSaveTimestampRef: React.MutableRefObject; pendingTimelineEditPathRef: React.MutableRefObject>; coalesceKey?: string; + /** Per-entry undo coalesce window override (ms) — see EditHistoryEntry.coalesceMs. */ + coalesceMs?: number; } export async function persistTimelineBatchEdit( @@ -347,6 +349,7 @@ export async function persistTimelineBatchEdit( label: input.label, kind: "timeline", coalesceKey: input.coalesceKey, + coalesceMs: input.coalesceMs, files, readFile: async (path) => originals.get(path) ?? readFileContent(input.projectId, path), writeFile: input.writeProjectFile, diff --git a/packages/studio/src/hooks/timelineMoveAdapter.ts b/packages/studio/src/hooks/timelineMoveAdapter.ts index 7672888e61..f5b9b2fb65 100644 --- a/packages/studio/src/hooks/timelineMoveAdapter.ts +++ b/packages/studio/src/hooks/timelineMoveAdapter.ts @@ -23,6 +23,7 @@ export function persistTimelineMoveEditsAtomically( coalesceKey: string | undefined, operation: TimelineMoveOperation, deps: AtomicMoveDeps, + coalesceMs?: number, ): Promise { return deps.handleTimelineGroupMove( edits.map(({ element, updates }) => ({ @@ -34,6 +35,6 @@ export function persistTimelineMoveEditsAtomically( // ("timing") omit it so they stay eligible for the SDK fast path. track: operation === "timing" ? undefined : updates.track, })), - { coalesceKey }, + { coalesceKey, coalesceMs }, ); } diff --git a/packages/studio/src/hooks/useDomEditCommits.test.tsx b/packages/studio/src/hooks/useDomEditCommits.test.tsx index 88bd5f0c14..1c0667400d 100644 --- a/packages/studio/src/hooks/useDomEditCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditCommits.test.tsx @@ -336,6 +336,10 @@ describe("useDomEditCommits z-index reorder persistence", () => { label: "Reorder layers", kind: "manual", coalesceKey: "z-reorder:test", + // Unbounded per-gesture fold window (keys are unique per gesture): + // the z entry and its mirror/lane counterpart fold across the server + // round-trip that separates them. + coalesceMs: Number.POSITIVE_INFINITY, files: { "index.html": { before: original, after } }, }); // FIX: a z-only reorder must NOT remount the preview iframe ("the blink"). diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index 3b88442071..87d0af9ad0 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -408,6 +408,7 @@ export function useDomEditCommits({ label: options.label, kind: "manual", coalesceKey: options.coalesceKey, + coalesceMs: options.coalesceMs, files, }); forceReloadSdkSession?.(); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.test.tsx b/packages/studio/src/hooks/useElementLifecycleOps.test.tsx index 979cdea6c9..d34b8623db 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.test.tsx +++ b/packages/studio/src/hooks/useElementLifecycleOps.test.tsx @@ -18,6 +18,7 @@ afterEach(() => { interface BatchOptions { label: string; coalesceKey: string; + coalesceMs?: number; skipReload?: boolean; } @@ -219,10 +220,15 @@ describe("useElementLifecycleOps — z-index reorder payload", () => { }); // Same element set, different actions — the keys must differ so the two - // edits never coalesce into one undo step within the coalesce window. + // edits never coalesce into one undo step. Each key also carries a fresh + // gesture sequence, which is what makes the commit's unbounded per-gesture + // coalesce window safe (see zReorderCoalesceKey). expect(captured).toHaveLength(2); - expect(captured[0]?.options.coalesceKey).toBe("z-reorder:bring-forward:clip-a"); - expect(captured[1]?.options.coalesceKey).toBe("z-reorder:send-backward:clip-a"); + expect(captured[0]?.options.coalesceKey).toMatch(/^z-reorder:bring-forward:clip-a:g\d+$/); + expect(captured[1]?.options.coalesceKey).toMatch(/^z-reorder:send-backward:clip-a:g\d+$/); + expect(captured[0]?.options.coalesceKey).not.toBe(captured[1]?.options.coalesceKey); + // The two-phase gesture fold rides an unbounded window on both records. + expect(captured[0]?.options.coalesceMs).toBe(Number.POSITIVE_INFINITY); act(() => root.unmount()); }); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index 3e6d05f838..3ddbe7fa53 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -25,15 +25,25 @@ interface UseElementLifecycleOpsParams extends DomEditCommitBaseParams { onElementDeleted?: (selection: DomEditSelection) => void; } +// One coalesce key per z-reorder gesture. A monotonic counter — NOT Date.now() +// / Math.random(), which the determinism rules forbid — matches the +// laneChangeGestureSeq precedent in timelineClipDragCommit.ts: the key only has +// to be unique per gesture and identical across the gesture's records. +let zReorderGestureSeq = 0; + /** - * Undo coalesce key for a z-reorder commit. The key carries the action kind so - * two DIFFERENT actions on the same element set (e.g. "bring-forward" then - * "send-backward" within the coalesce window) never merge into one undo step. + * Undo coalesce key for ONE z-reorder gesture — unique per call. The key + * carries the action kind + element ids for debuggability, plus a gesture + * sequence so two SEPARATE user actions (even the same action on the same + * selection) never share a key. That uniqueness is what makes the unbounded + * per-gesture coalesce window (see handleDomZIndexReorderCommit) safe: the + * fold can only ever merge records of the SAME gesture. * - * Exported as THE single implementation of the key: the canvas z-order mirror - * (useCanvasZOrderTimelineMirror) passes this exact key into its timeline lane - * persist so editHistory folds the z write and the track write into one undo - * entry — a drifting duplicate formula would silently split the undo. + * Exported as THE single implementation of the key: the canvas z-order wiring + * (PreviewOverlays) mints it once per gesture and passes the same instance to + * both the z persist and the timeline lane mirror (useCanvasZOrderTimelineMirror) + * so editHistory folds the z write and the track write into one undo entry — + * recomputing the key per record would silently split the undo. */ export function zReorderCoalesceKey( entries: ReadonlyArray<{ element: HTMLElement; id?: string; selector?: string }>, @@ -42,7 +52,7 @@ export function zReorderCoalesceKey( const ids = entries .map((e) => e.id ?? e.selector ?? e.element.getAttribute("data-hf-id") ?? "el") .join(":"); - return `z-reorder:${actionKind ?? "reorder"}:${ids}`; + return `z-reorder:${actionKind ?? "reorder"}:${ids}:g${zReorderGestureSeq++}`; } export function useElementLifecycleOps({ @@ -244,6 +254,13 @@ export function useElementLifecycleOps({ return commitDomEditPatchBatches(batches, { label: "Reorder layers", coalesceKey, + // Unbounded window: every key this commit records under is unique per + // gesture (zReorderCoalesceKey's gesture seq, or the lane drag's + // clip-lane-move:), so the fold can only merge records of the SAME + // gesture — and those records are separated by a server round-trip + // (move→z on a lane drag, z→lane-mirror on a canvas action), which + // under real network latency exceeds the 300ms default window. + coalesceMs: Number.POSITIVE_INFINITY, skipReload: true, }).catch((error) => { for (const rollback of rollbacks) rollback(); diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index 3dace0f6f6..c6022df4a6 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -37,6 +37,8 @@ export interface TimelineGroupResizeChange { export interface TimelineGroupCommitOptions { beforeTiming?: Promise; coalesceKey?: string; + /** Per-entry undo coalesce window override (ms) — see EditHistoryEntry.coalesceMs. */ + coalesceMs?: number; } interface UseTimelineGroupEditingOptions { @@ -137,6 +139,7 @@ export function useTimelineGroupEditing({ label: string, batchChanges: PersistTimelineBatchChange[], coalesceKey: string, + coalesceMs?: number, ) => { await persistTimelineBatchEdit({ projectId, @@ -148,6 +151,7 @@ export function useTimelineGroupEditing({ domEditSaveTimestampRef, pendingTimelineEditPathRef, coalesceKey, + coalesceMs, }); forceReloadSdkSession?.(); }, @@ -176,6 +180,7 @@ export function useTimelineGroupEditing({ needsExtension: boolean; label: string; coalesceKey: string; + coalesceMs?: number; }): Promise => { const sharedPath = allChangesSharePath(input.changes, activeCompPath); const canUseSdk = @@ -196,7 +201,7 @@ export function useTimelineGroupEditing({ compositionPath: activeCompPath, readProjectFile: (path) => readFileContent(projectIdRef.current ?? "", path), }, - { label: input.label, coalesceKey: input.coalesceKey }, + { label: input.label, coalesceKey: input.coalesceKey, coalesceMs: input.coalesceMs }, ); }, [ @@ -234,6 +239,7 @@ export function useTimelineGroupEditing({ // the just-patched live DOM. See syncPreviewContentDuration. syncPreviewContentDuration(previewIframeRef.current); const coalesceKey = options?.coalesceKey ?? moveCoalesceKey(changes); + const coalesceMs = options?.coalesceMs; return enqueueGroupOperation("Move timeline clips", async (projectId) => { await options?.beforeTiming; const handledBySdk = await trySdkBatchPersist({ @@ -243,6 +249,7 @@ export function useTimelineGroupEditing({ needsExtension, label: "Move timeline clips", coalesceKey, + coalesceMs, }); if (handledBySdk) return; @@ -261,6 +268,7 @@ export function useTimelineGroupEditing({ ), })), coalesceKey, + coalesceMs, ); await finishGroupTimingGsapFallback({ projectId, @@ -327,6 +335,7 @@ export function useTimelineGroupEditing({ // the just-patched live DOM. See syncPreviewContentDuration. syncPreviewContentDuration(previewIframeRef.current); const coalesceKey = options?.coalesceKey ?? resizeCoalesceKey(changes); + const coalesceMs = options?.coalesceMs; return enqueueGroupOperation("Resize timeline clips", async (projectId) => { await options?.beforeTiming; const handledBySdk = await trySdkBatchPersist({ @@ -339,6 +348,7 @@ export function useTimelineGroupEditing({ needsExtension, label: "Resize timeline clips", coalesceKey, + coalesceMs, }); if (handledBySdk) return; @@ -355,6 +365,7 @@ export function useTimelineGroupEditing({ }), })), coalesceKey, + coalesceMs, ); await finishGroupTimingGsapFallback({ projectId, diff --git a/packages/studio/src/player/components/timelineCallbacks.ts b/packages/studio/src/player/components/timelineCallbacks.ts index 033b3ecb56..26d1e4bd77 100644 --- a/packages/studio/src/player/components/timelineCallbacks.ts +++ b/packages/studio/src/player/components/timelineCallbacks.ts @@ -31,11 +31,14 @@ export interface TimelineEditCallbacks { ) => Promise | void; /** Atomic multi-clip move (single undo) for main-track ripple + track-insert. * `coalesceKey` (drag-commit gesture id) merges the move history entry with a - * lane change's follow-up z-reorder entry into one undo step. */ + * lane change's follow-up z-reorder entry into one undo step; `coalesceMs` + * widens that entry's fold window when a server round-trip separates the + * gesture's records (per-gesture-unique keys keep the fold gesture-scoped). */ onMoveElements?: ( edits: Array<{ element: TimelineElement; updates: Pick }>, coalesceKey?: string, operation?: TimelineMoveOperation, + coalesceMs?: number, ) => Promise | void; onResizeElement?: ( element: TimelineElement, diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index 7dc3125f02..a8364551cb 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -34,11 +34,14 @@ export interface DragCommitDeps { /** Atomic multi-clip persist (single undo) for lane changes + track inserts. * `coalesceKey`, when supplied, tags the resulting "Move timeline clips" * history entry so it merges with the lane change's z-reorder entry (see the - * lane-change branch below). */ + * lane-change branch below). `coalesceMs` widens that entry's fold window + * (per-gesture-unique keys make an unbounded window safe) — required when a + * server round-trip sits between the gesture's two records. */ onMoveElements?: ( edits: TimelineMoveEdit[], coalesceKey?: string, operation?: TimelineMoveOperation, + coalesceMs?: number, ) => Promise | void; /** * The current multi-selection (store.selectedElementIds). When the dragged @@ -110,6 +113,7 @@ function persistMoveEdits( deps: DragCommitDeps, coalesceKey?: string, operation: TimelineMoveOperation = "timing", + coalesceMs?: number, ): Promise { if (edits.length === 0) return Promise.resolve(true); const { updateElement, onMoveElement, onMoveElements } = deps; @@ -151,7 +155,7 @@ function persistMoveEdits( : { element: e.element, updates: { ...e.updates, track: e.persistTrack } }, ); const persisted = onMoveElements - ? onMoveElements(persistEdits, coalesceKey, operation) + ? onMoveElements(persistEdits, coalesceKey, operation, coalesceMs) : Promise.all(persistEdits.map((e) => Promise.resolve(onMoveElement?.(e.element, e.updates)))); return Promise.resolve(persisted).then( () => true, @@ -491,8 +495,13 @@ function commitTrackInsert( * a future call into the sync would no-op (double protection; see * useCanvasZOrderTimelineMirror). * - * `coalesceKey` MUST be the z persist's key (`z-reorder::`) so - * editHistory folds the z write and this track write into ONE undo entry. + * `coalesceKey` MUST be the z persist's key (`z-reorder:::g`) + * so editHistory folds the z write and this track write into ONE undo entry, and + * `coalesceMs` MUST widen this record's fold window: the mirror only runs after + * the z persist's server round-trip resolved, so under real network latency the + * gap between the two records exceeds the reducer's 300ms default and the fold + * would never happen live. The key is unique per gesture, so an unbounded + * window can never merge distinct gestures. * * Resolves `true` once the move persisted, `false` on rollback / refused insert. */ @@ -501,6 +510,7 @@ export function commitZMirrorLaneMove( move: NonNullable, deps: DragCommitDeps, coalesceKey: string, + coalesceMs?: number, ): Promise { if (move.kind === "move") { const edit: TimelineMoveEdit = { @@ -508,11 +518,11 @@ export function commitZMirrorLaneMove( updates: { start: element.start, track: move.displayTrack }, persistTrack: move.persistTrack, }; - return persistMoveEdits([edit], deps, coalesceKey, "lane-reorder"); + return persistMoveEdits([edit], deps, coalesceKey, "lane-reorder", coalesceMs); } const built = buildTrackInsertEdits(element, element.start, move.insertRow, null, deps); if (!built || built.edits.length === 0) return Promise.resolve(false); - return persistMoveEdits(built.edits, deps, coalesceKey, "track-insert"); + return persistMoveEdits(built.edits, deps, coalesceKey, "track-insert", coalesceMs); } /** From b634dbd1d7678d7365854a1430b8eca02f160d35 Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Mon, 13 Jul 2026 19:20:34 -0700 Subject: [PATCH 03/19] feat(studio): flashless lane mirror, z-order menu icons, close-gap track menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Track-only batch moves (the z-mirror's lane hop and the insert renumber) skip the GSAP fallback round-trip and the preview reload entirely — the renderer never reads data-track-index, and the live DOM patch + optimistic store update cover the UI. Mixed batches keep current behavior. Kills the canvas blink on mirrored Bring/Send actions (live-verified: an iframe-scoped marker survives the whole gesture). - The four z-order menu items get 16px stroke icons (single layer diamond + directional arrow for Forward/Backward; pierced two-layer stack for Front/Back); labels unchanged — they are the industry-standard names. - New track context menu on empty lane space: 'Close gap' (shifts the next clip and every clip after it on that lane left by the clicked gap's width; leading gaps count, so a single clip with empty space before it compacts to 0) and 'Close all gaps' (whole lane contiguous from 0). Pure gap math in timelineGaps.ts; persists through the drag path's atomic batch move (one undo per action); refuses when a clip that must shift is locked; items disable when there is nothing to close. --- .../editor/CanvasContextMenu.test.tsx | 40 ++- .../components/editor/CanvasContextMenu.tsx | 56 ++- .../useCanvasZOrderTimelineMirror.test.tsx | 23 +- .../src/hooks/elementLifecycleOpsTestUtils.ts | 25 ++ .../studio/src/hooks/fetchStubTestUtils.ts | 16 + .../src/hooks/timelineTimingSync.test.ts | 14 +- .../src/hooks/useElementLifecycleOps.test.tsx | 88 ++--- .../src/hooks/useTimelineEditing.test.tsx | 333 ++++++++++-------- .../src/hooks/useTimelineGroupEditing.ts | 19 + .../studio/src/player/components/Timeline.tsx | 46 ++- .../src/player/components/TimelineLanes.tsx | 18 + .../player/components/TimelineOverlays.tsx | 33 ++ .../player/components/TrackGapContextMenu.tsx | 100 ++++++ .../components/timelineClipDragCommit.ts | 6 +- .../components/timelineGapCommit.test.ts | 163 +++++++++ .../player/components/timelineGapCommit.ts | 98 ++++++ .../player/components/timelineGaps.test.ts | 156 ++++++++ .../src/player/components/timelineGaps.ts | 119 +++++++ .../player/components/timelineZMirror.test.ts | 47 +-- .../src/player/components/useTrackGapMenu.ts | 106 ++++++ 20 files changed, 1228 insertions(+), 278 deletions(-) create mode 100644 packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts create mode 100644 packages/studio/src/hooks/fetchStubTestUtils.ts create mode 100644 packages/studio/src/player/components/TrackGapContextMenu.tsx create mode 100644 packages/studio/src/player/components/timelineGapCommit.test.ts create mode 100644 packages/studio/src/player/components/timelineGapCommit.ts create mode 100644 packages/studio/src/player/components/timelineGaps.test.ts create mode 100644 packages/studio/src/player/components/timelineGaps.ts create mode 100644 packages/studio/src/player/components/useTrackGapMenu.ts diff --git a/packages/studio/src/components/editor/CanvasContextMenu.test.tsx b/packages/studio/src/components/editor/CanvasContextMenu.test.tsx index 57a093e81b..cd9ef44c56 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.test.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.test.tsx @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { installReactActEnvironment, makeSelection } from "../../hooks/domSelectionTestHarness"; import { resolveZIndexEntries } from "../nle/PreviewOverlays"; import { useElementLifecycleOps } from "../../hooks/useElementLifecycleOps"; +import { makeLifecycleOpsParams } from "../../hooks/elementLifecycleOpsTestUtils"; import type { DomEditPatchBatch } from "../../hooks/domEditCommitTypes"; import { CanvasContextMenu } from "./CanvasContextMenu"; import type { ZOrderAction, ZOrderPatch } from "./canvasContextMenuZOrder"; @@ -102,9 +103,26 @@ describe("CanvasContextMenu — handler gating", () => { renderMenu({ selection: makeSelection("Target", el), onApplyZIndex: vi.fn() }); - expect(zOrderButtons()).toHaveLength(4); + const buttons = zOrderButtons(); + expect(buttons).toHaveLength(4); expect(hasDeleteItem()).toBe(false); expect(document.body.querySelector(".border-t")).toBeNull(); + + // Labels stay the exact industry-standard names (the icons add no text)... + expect(buttons.map((b) => b.textContent)).toEqual([ + "Bring forward", + "Send backward", + "Bring to front", + "Send to back", + ]); + // ...and each item leads with a stroke icon that inherits the item's text + // color (currentColor), so the disabled muted tone applies to it too. + for (const button of buttons) { + const svg = button.firstElementChild; + expect(svg?.tagName.toLowerCase()).toBe("svg"); + expect(svg?.getAttribute("stroke")).toBe("currentColor"); + expect(svg?.getAttribute("aria-hidden")).toBe("true"); + } }); it("shows only Delete (no z-order items, no divider) when onApplyZIndex is absent", () => { @@ -158,19 +176,13 @@ function renderCommitHook(captured: CapturedBatchCall[]) { type Commit = ReturnType["handleDomZIndexReorderCommit"]; let commit: Commit | undefined; function Harness() { - ({ handleDomZIndexReorderCommit: commit } = useElementLifecycleOps({ - activeCompPath: "index.html", - showToast: vi.fn(), - writeProjectFile: vi.fn(async () => {}), - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit: vi.fn(async () => {}) }, - projectIdRef: { current: null }, - reloadPreview: vi.fn(), - clearDomSelection: vi.fn(), - commitDomEditPatchBatches: async (batches, options) => { - captured.push({ batches, options }); - }, - })); + ({ handleDomZIndexReorderCommit: commit } = useElementLifecycleOps( + makeLifecycleOpsParams({ + commitDomEditPatchBatches: async (batches, options) => { + captured.push({ batches, options }); + }, + }), + )); return null; } const hookHost = document.createElement("div"); diff --git a/packages/studio/src/components/editor/CanvasContextMenu.tsx b/packages/studio/src/components/editor/CanvasContextMenu.tsx index ca9e44897e..2f991e1061 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.tsx @@ -79,6 +79,57 @@ interface CanvasContextMenuProps { type ZAction = "bring-forward" | "send-backward" | "bring-to-front" | "send-to-back"; +// Stacked-layer + arrow glyphs, one per z action (16px, stroke, currentColor — +// matches the studio's inline-SVG conventions: fill="none", 1.2 stroke, round +// caps/joins). Single actions show ONE layer diamond with the arrow stepping +// one way; front/back show a TWO-diamond stack with the arrow piercing through +// and beyond it. `paths` are the d attributes, drawn in order. +const Z_ACTION_ICONS: Record = { + "bring-forward": [ + "M3 11 L8 8.5 L13 11 L8 13.5 Z", // layer diamond (bottom) + "M8 8.5 L8 2", // arrow shaft up + "M5.5 4.5 L8 2 L10.5 4.5", // arrow head + ], + "send-backward": [ + "M3 5 L8 2.5 L13 5 L8 7.5 Z", // layer diamond (top) + "M8 7.5 L8 14", // arrow shaft down + "M5.5 11.5 L8 14 L10.5 11.5", // arrow head + ], + "bring-to-front": [ + "M3 9.5 L8 7 L13 9.5 L8 12 Z", // upper layer of the stack + "M3 12.5 L8 10 L13 12.5 L8 15 Z", // lower layer of the stack + "M8 12.5 L8 2", // arrow piercing up through/above the stack + "M5.5 4.5 L8 2 L10.5 4.5", // arrow head + ], + "send-to-back": [ + "M3 4 L8 1.5 L13 4 L8 6.5 Z", // upper layer of the stack + "M3 7 L8 4.5 L13 7 L8 9.5 Z", // lower layer of the stack + "M8 3.5 L8 14", // arrow piercing down through/below the stack + "M5.5 11.5 L8 14 L10.5 11.5", // arrow head + ], +}; + +function ZActionIcon({ action }: { action: ZAction }) { + return ( + + ); +} + const Z_ACTIONS: Array<{ action: ZAction; label: string }> = [ { action: "bring-forward", label: "Bring forward" }, { action: "send-backward", label: "Send backward" }, @@ -196,7 +247,10 @@ export const CanvasContextMenu = memo(function CanvasContextMenu({ if (enabled) handleZAction(action); }} > - {label} + {/* Icon inherits the item's text color via currentColor, so the + disabled muted tone applies to both icon and label. */} + + {label} ); })} diff --git a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx index f9d320b242..94abb6424a 100644 --- a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx +++ b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import React, { act } from "react"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; import { usePlayerStore, type TimelineElement } from "../../player"; import { TimelineEditProvider } from "../../contexts/TimelineEditContext"; import type { TimelineEditCallbacks } from "../../player/components/timelineCallbacks"; @@ -10,6 +10,7 @@ import { useCanvasZOrderTimelineMirror, type MirrorZOrderInput, } from "./useCanvasZOrderTimelineMirror"; +import { makeLifecycleOpsParams } from "../../hooks/elementLifecycleOpsTestUtils"; import { mountReactHarness } from "../../hooks/domSelectionTestHarness"; import { buildEditHistoryEntry, @@ -113,19 +114,13 @@ function mountMirrorHarness(history: { const api: Partial = {}; function Harness() { - const { handleDomZIndexReorderCommit } = useElementLifecycleOps({ - activeCompPath: "index.html", - showToast: vi.fn(), - writeProjectFile: vi.fn(async () => {}), - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit: vi.fn(async () => {}) }, - projectIdRef: { current: null }, - reloadPreview: vi.fn(), - clearDomSelection: vi.fn(), - commitDomEditPatchBatches: async (_batches, options) => { - record(options.label, "manual", options.coalesceKey, options.coalesceMs, "B-z"); - }, - }); + const { handleDomZIndexReorderCommit } = useElementLifecycleOps( + makeLifecycleOpsParams({ + commitDomEditPatchBatches: async (_batches, options) => { + record(options.label, "manual", options.coalesceKey, options.coalesceMs, "B-z"); + }, + }), + ); api.commitZ = handleDomZIndexReorderCommit; api.mirror = useCanvasZOrderTimelineMirror(); return null; diff --git a/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts b/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts new file mode 100644 index 0000000000..a0b86cc8f9 --- /dev/null +++ b/packages/studio/src/hooks/elementLifecycleOpsTestUtils.ts @@ -0,0 +1,25 @@ +import { vi } from "vitest"; +import type { useElementLifecycleOps } from "./useElementLifecycleOps"; + +type LifecycleOpsParams = Parameters[0]; + +/** + * Baseline `useElementLifecycleOps` params for tests: inert stubs for every + * dependency, overridden per test (typically just `commitDomEditPatchBatches`). + * Shared by the z-reorder commit tests and the timeline-mirror harness. + */ +export function makeLifecycleOpsParams( + overrides: Partial & Pick, +): LifecycleOpsParams { + return { + activeCompPath: "index.html", + showToast: vi.fn(), + writeProjectFile: vi.fn(async () => {}), + domEditSaveTimestampRef: { current: 0 }, + editHistory: { recordEdit: vi.fn(async () => {}) }, + projectIdRef: { current: null }, + reloadPreview: vi.fn(), + clearDomSelection: vi.fn(), + ...overrides, + }; +} diff --git a/packages/studio/src/hooks/fetchStubTestUtils.ts b/packages/studio/src/hooks/fetchStubTestUtils.ts new file mode 100644 index 0000000000..1cd61d13e3 --- /dev/null +++ b/packages/studio/src/hooks/fetchStubTestUtils.ts @@ -0,0 +1,16 @@ +// Shared helpers for test-side `fetch` stubs (timelineTimingSync.test.ts, +// useTimelineEditing.test.tsx): a JSON Response factory and a Request → URL +// normalizer. Test-only module — imported exclusively from *.test.* files. + +export function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +export function requestUrl(input: Parameters[0]): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.toString(); + return input.url; +} diff --git a/packages/studio/src/hooks/timelineTimingSync.test.ts b/packages/studio/src/hooks/timelineTimingSync.test.ts index f0e45e3ab3..d1a6e8d21a 100644 --- a/packages/studio/src/hooks/timelineTimingSync.test.ts +++ b/packages/studio/src/hooks/timelineTimingSync.test.ts @@ -1,6 +1,7 @@ // @vitest-environment happy-dom import { afterEach, describe, expect, it, vi } from "vitest"; import { usePlayerStore } from "../player/store/playerStore"; +import { jsonResponse, requestUrl } from "./fetchStubTestUtils"; import { captureDurationRollback, finishClipTimingFallback, @@ -14,19 +15,6 @@ afterEach(() => { vi.unstubAllGlobals(); }); -function jsonResponse(body: unknown): Response { - return new Response(JSON.stringify(body), { - status: 200, - headers: { "content-type": "application/json" }, - }); -} - -function requestUrl(input: Parameters[0]): string { - if (typeof input === "string") return input; - if (input instanceof URL) return input.toString(); - return input.url; -} - /** * Stub fetch: `/files/` reads return contents from the queue (repeating the * last entry), the GSAP-mutation endpoint answers with `gsapBody` (a thrown diff --git a/packages/studio/src/hooks/useElementLifecycleOps.test.tsx b/packages/studio/src/hooks/useElementLifecycleOps.test.tsx index d34b8623db..240452317a 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.test.tsx +++ b/packages/studio/src/hooks/useElementLifecycleOps.test.tsx @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { usePlayerStore } from "../player"; import type { DomEditPatchBatch } from "./domEditCommitTypes"; import { useElementLifecycleOps } from "./useElementLifecycleOps"; +import { makeLifecycleOpsParams } from "./elementLifecycleOpsTestUtils"; import { mountReactHarness } from "./domSelectionTestHarness"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -46,19 +47,13 @@ function renderReorderHook( onReady: (commit: ReorderCommit) => void, ) { function Harness() { - const { handleDomZIndexReorderCommit } = useElementLifecycleOps({ - activeCompPath: "index.html", - showToast: vi.fn(), - writeProjectFile: vi.fn(async () => {}), - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit: vi.fn(async () => {}) }, - projectIdRef: { current: null }, - reloadPreview: vi.fn(), - clearDomSelection: vi.fn(), - commitDomEditPatchBatches: async (batches, options) => { - capturedCalls.push({ batches, options }); - }, - }); + const { handleDomZIndexReorderCommit } = useElementLifecycleOps( + makeLifecycleOpsParams({ + commitDomEditPatchBatches: async (batches, options) => { + capturedCalls.push({ batches, options }); + }, + }), + ); onReady(handleDomZIndexReorderCommit); return null; } @@ -252,19 +247,13 @@ describe("useElementLifecycleOps — z-index reorder payload", () => { let commit: ReorderCommit | undefined; let resolveBatch: (() => void) | undefined; function Harness() { - const { handleDomZIndexReorderCommit } = useElementLifecycleOps({ - activeCompPath: "index.html", - showToast: vi.fn(), - writeProjectFile: vi.fn(async () => {}), - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit: vi.fn(async () => {}) }, - projectIdRef: { current: null }, - reloadPreview: vi.fn(), - clearDomSelection: vi.fn(), - // Persist stays pending so the assertion below can only be satisfied - // by the SYNCHRONOUS store update (the lane-sync path's requirement). - commitDomEditPatchBatches: () => new Promise((resolve) => (resolveBatch = resolve)), - }); + const { handleDomZIndexReorderCommit } = useElementLifecycleOps( + makeLifecycleOpsParams({ + // Persist stays pending so the assertion below can only be satisfied + // by the SYNCHRONOUS store update (the lane-sync path's requirement). + commitDomEditPatchBatches: () => new Promise((resolve) => (resolveBatch = resolve)), + }), + ); commit = handleDomZIndexReorderCommit; return null; } @@ -307,22 +296,16 @@ describe("useElementLifecycleOps — z-index reorder payload", () => { let commit: ReorderCommit | undefined; function Harness() { - const { handleDomZIndexReorderCommit } = useElementLifecycleOps({ - activeCompPath: "index.html", - showToast: vi.fn(), - writeProjectFile: vi.fn(async () => {}), - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit: vi.fn(async () => {}) }, - projectIdRef: { current: null }, - reloadPreview: vi.fn(), - clearDomSelection: vi.fn(), - commitDomEditPatchBatches: vi.fn(async () => { - // The live styles were applied by the hook before persist ran. - expect(el.style.zIndex).toBe("2"); - expect(el.style.position).toBe("relative"); - throw failure; + const { handleDomZIndexReorderCommit } = useElementLifecycleOps( + makeLifecycleOpsParams({ + commitDomEditPatchBatches: vi.fn(async () => { + // The live styles were applied by the hook before persist ran. + expect(el.style.zIndex).toBe("2"); + expect(el.style.position).toBe("relative"); + throw failure; + }), }), - }); + ); commit = handleDomZIndexReorderCommit; return null; } @@ -373,20 +356,17 @@ describe("useElementLifecycleOps — z-index reorder payload", () => { let commit: ReorderCommit | undefined; function Harness() { - const { handleDomZIndexReorderCommit } = useElementLifecycleOps({ - activeCompPath: "index.html", - showToast: vi.fn(), - writeProjectFile, - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit }, - projectIdRef: { current: "demo" }, - reloadPreview: vi.fn(), - clearDomSelection: vi.fn(), - forceReloadSdkSession, - commitDomEditPatchBatches: vi.fn(async () => { - throw originalError; + const { handleDomZIndexReorderCommit } = useElementLifecycleOps( + makeLifecycleOpsParams({ + writeProjectFile, + editHistory: { recordEdit }, + projectIdRef: { current: "demo" }, + forceReloadSdkSession, + commitDomEditPatchBatches: vi.fn(async () => { + throw originalError; + }), }), - }); + ); commit = handleDomZIndexReorderCommit; return null; } diff --git a/packages/studio/src/hooks/useTimelineEditing.test.tsx b/packages/studio/src/hooks/useTimelineEditing.test.tsx index 449d143bc3..0a36731f6c 100644 --- a/packages/studio/src/hooks/useTimelineEditing.test.tsx +++ b/packages/studio/src/hooks/useTimelineEditing.test.tsx @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { openComposition } from "@hyperframes/sdk"; import { afterEach, describe, expect, it, vi } from "vitest"; import { usePlayerStore, type TimelineElement } from "../player"; +import { jsonResponse, requestUrl } from "./fetchStubTestUtils"; import { useElementLifecycleOps } from "./useElementLifecycleOps"; import { useTimelineEditing } from "./useTimelineEditing"; @@ -209,19 +210,6 @@ function renderTimelineEditingHookWithLifecycle(input: { return { move, unmount }; } -function jsonResponse(body: unknown): Response { - return new Response(JSON.stringify(body), { - status: 200, - headers: { "content-type": "application/json" }, - }); -} - -function requestUrl(input: Parameters[0]): string { - if (typeof input === "string") return input; - if (input instanceof URL) return input.toString(); - return input.url; -} - async function flushAsyncWork(): Promise { for (let i = 0; i < 8; i += 1) { await Promise.resolve(); @@ -251,6 +239,72 @@ function stubProjectFetch( return fetchMock; } +/** + * One clip on track 0 + the editing hook wired to project "p1" — shared setup + * for the single-clip move-path tests (horizontal, vertical-only, track-only, + * diagonal) so their arrange blocks aren't clones of each other. + */ +function setupSingleClipHarness(options?: { + source?: string; + clipStyle?: string; + onZIndexCommit?: (entries: ZIndexEntry[]) => Promise; +}) { + const iframe = createPreviewIframe([{ id: "clip", track: 0, style: options?.clipStyle }]); + const clip = timelineElement({ id: "clip", track: 0, zIndex: 0 }); + const commit = + options?.onZIndexCommit ?? + vi.fn<(entries: ZIndexEntry[]) => Promise>().mockResolvedValue(undefined); + const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); + const reloadPreview = vi.fn(); + const fetchMock = stubProjectFetch( + options?.source ?? '
', + ); + const hook = renderTimelineEditingHook({ + timelineElements: [clip], + iframe, + onZIndexCommit: commit, + projectId: "p1", + writeProjectFile, + recordEdit: vi.fn(async () => {}), + reloadPreview, + }); + return { iframe, clip, commit, writeProjectFile, reloadPreview, fetchMock, ...hook }; +} + +/** Assert a lane write landed in both the live iframe DOM and the persisted file. */ +function expectLanePersisted( + iframe: HTMLIFrameElement, + writeProjectFile: { mock: { calls: unknown[][] } }, + track: string, +): void { + const doc = iframe.contentDocument; + if (!doc) throw new Error("Expected iframe document"); + expect(doc.getElementById("clip")?.getAttribute("data-track-index")).toBe(track); + expect(writeProjectFile.mock.calls[0]![1]).toContain(`data-track-index="${track}"`); +} + +/** + * Two 1s clips — a@0s on track 0, b@1s on track 1 — plus the matching iframe. + * Shared by the group-move tests; `bSourceFile` puts b in its own file for the + * cross-file partition test. + */ +function makeTwoClipPair(bSourceFile?: string) { + const iframe = createPreviewIframe([ + { id: "a", track: 0 }, + { id: "b", track: 1 }, + ]); + const a = timelineElement({ id: "a", track: 0, zIndex: 0, start: 0, duration: 1 }); + const b = timelineElement({ + id: "b", + track: 1, + zIndex: 0, + start: 1, + duration: 1, + sourceFile: bSourceFile, + }); + return { iframe, a, b }; +} + const ROOT_DURATION_FALLBACK_SOURCE = [ `
`, `
`, @@ -608,98 +662,59 @@ describe("useTimelineEditing timeline z-index reorder", () => { }); it("keeps horizontal-only drag on the timing and GSAP shift path without z-index writes", async () => { - const iframe = createPreviewIframe([{ id: "clip", track: 0 }]); - const clip = timelineElement({ id: "clip", track: 0, zIndex: 0 }); - const commit = vi.fn<(entries: ZIndexEntry[]) => Promise>().mockResolvedValue(undefined); - const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); - const recordEdit = vi.fn(async (_entry) => {}); - const reloadPreview = vi.fn(); - const fetchMock = stubProjectFetch('
'); - const { move, unmount } = renderTimelineEditingHook({ - timelineElements: [clip], - iframe, - onZIndexCommit: commit, - projectId: "p1", - writeProjectFile, - recordEdit, - reloadPreview, - }); + const h = setupSingleClipHarness(); await act(async () => { - await move(clip, { start: 1.25, track: clip.track }); + await h.move(h.clip, { start: 1.25, track: h.clip.track }); }); - const doc = iframe.contentDocument; + const doc = h.iframe.contentDocument; if (!doc) throw new Error("Expected iframe document"); expect(doc.getElementById("clip")?.getAttribute("data-start")).toBe("1.25"); expect(doc.getElementById("clip")?.getAttribute("data-track-index")).toBe("0"); - expect(commit).not.toHaveBeenCalled(); - expect(writeProjectFile.mock.calls[0]![1]).toContain('data-start="1.25"'); - expect(writeProjectFile.mock.calls[0]![1]).toContain('data-track-index="0"'); - expect(writeProjectFile.mock.calls[0]![1]).not.toContain("z-index"); + expect(h.commit).not.toHaveBeenCalled(); + expect(h.writeProjectFile.mock.calls[0]![1]).toContain('data-start="1.25"'); + expect(h.writeProjectFile.mock.calls[0]![1]).toContain('data-track-index="0"'); + expect(h.writeProjectFile.mock.calls[0]![1]).not.toContain("z-index"); expect( - fetchMock.mock.calls.some((call) => requestUrl(call[0]).includes("gsap-mutations")), + h.fetchMock.mock.calls.some((call) => requestUrl(call[0]).includes("gsap-mutations")), ).toBe(true); - unmount(); + h.unmount(); }); it("persists a vertical-only lane move (start unchanged) through the single-element fallback", async () => { // Regression: `if (!startChanged) return` ran BEFORE the file persist, so a // pure lane change routed through onMoveElement (no onMoveElements wired) // wrote NOTHING — the lane snapped back on the next reload. - const iframe = createPreviewIframe([{ id: "clip", track: 0 }]); - const clip = timelineElement({ id: "clip", track: 0, zIndex: 0 }); - const commit = vi.fn<(entries: ZIndexEntry[]) => Promise>().mockResolvedValue(undefined); - const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); - stubProjectFetch('
'); - const { move, unmount } = renderTimelineEditingHook({ - timelineElements: [clip], - iframe, - onZIndexCommit: commit, - projectId: "p1", - writeProjectFile, - recordEdit: vi.fn(async () => {}), - }); + const h = setupSingleClipHarness(); await act(async () => { // Vertical-only: same start, new track (already authored-space on this path). - await move(clip, { start: clip.start, track: 2 }); + await h.move(h.clip, { start: h.clip.start, track: 2 }); }); - const doc = iframe.contentDocument; - if (!doc) throw new Error("Expected iframe document"); - // Live DOM patched so a pre-reload re-discovery doesn't snap the lane back... - expect(doc.getElementById("clip")?.getAttribute("data-track-index")).toBe("2"); - // ...and the file write carries the new data-track-index with start intact. - expect(writeProjectFile).toHaveBeenCalled(); - expect(writeProjectFile.mock.calls[0]![1]).toContain('data-track-index="2"'); - expect(writeProjectFile.mock.calls[0]![1]).toContain('data-start="0"'); + // Live DOM patched (no pre-reload lane snap-back) and the file write + // carries the new data-track-index with start intact. + expectLanePersisted(h.iframe, h.writeProjectFile, "2"); + expect(h.writeProjectFile.mock.calls[0]![1]).toContain('data-start="0"'); - unmount(); + h.unmount(); }); it("orders the timing write after the z-index commit so a diagonal drag can't clobber the restack", async () => { - const iframe = createPreviewIframe([ - { id: "clip", track: 0, style: "position: relative; z-index: 0" }, - ]); - const clip = timelineElement({ id: "clip", track: 0, zIndex: 0 }); // Gate the z-index commit so we can observe whether the timing write waits. let releaseCommit!: () => void; const commitGate = new Promise((resolve) => { releaseCommit = resolve; }); - const commit = vi.fn<(entries: ZIndexEntry[]) => Promise>().mockReturnValue(commitGate); - const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); - stubProjectFetch('
'); - const { move, unmount } = renderTimelineEditingHook({ - timelineElements: [clip], - iframe, - onZIndexCommit: commit, - projectId: "p1", - writeProjectFile, - recordEdit: vi.fn(async () => {}), + const h = setupSingleClipHarness({ + clipStyle: "position: relative; z-index: 0", + onZIndexCommit: vi + .fn<(entries: ZIndexEntry[]) => Promise>() + .mockReturnValue(commitGate), }); + const { clip, commit, writeProjectFile, move, unmount } = h; // Diagonal drag: both a time move (start change) and a restack (z-index change). let movePromise!: Promise; @@ -784,19 +799,7 @@ describe("useTimelineEditing timeline z-index reorder", () => { "index.html": '
', "scene.html": '
', }; - const iframe = createPreviewIframe([ - { id: "a", track: 0 }, - { id: "b", track: 1 }, - ]); - const a = timelineElement({ id: "a", track: 0, zIndex: 0, start: 0, duration: 1 }); - const b = timelineElement({ - id: "b", - track: 1, - zIndex: 0, - start: 1, - duration: 1, - sourceFile: "scene.html", - }); + const { iframe, a, b } = makeTwoClipPair("scene.html"); const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); const recordEdit = vi.fn(async (_entry) => {}); stubProjectFetch(files); @@ -867,6 +870,73 @@ describe("useTimelineEditing timeline z-index reorder", () => { unmount(); }); + it("skips the GSAP fallback and reload for a TRACK-ONLY group move (z-mirror lane move)", async () => { + // The mirrored z-order lane move persists {start: unchanged, track: new}. + // Nothing timing-related changed — data-track-index is never read by the + // renderer — so the persist must NOT be followed by the GSAP round-trip or + // the full preview reload (the reload is what blinked the canvas). + const h = setupSingleClipHarness({ + source: '
', + }); + + await act(async () => { + await h.groupMove([{ element: h.clip, start: h.clip.start, track: 2 }]); + await flushAsyncWork(); + }); + + // The lane write still persisted (live DOM + file)... + expectLanePersisted(h.iframe, h.writeProjectFile, "2"); + expect(h.writeProjectFile).toHaveBeenCalledTimes(1); + // ...but no GSAP mutation ran and the preview was NOT reloaded. + expect( + h.fetchMock.mock.calls.some((call) => requestUrl(call[0]).includes("gsap-mutations")), + ).toBe(false); + expect(h.reloadPreview).not.toHaveBeenCalled(); + + h.unmount(); + }); + + it("keeps the GSAP fallback + reload for a MIXED batch (any start change)", async () => { + // One clip changes lane only, the other shifts in time — the batch is not + // track-only, so the existing behavior (GSAP shift + full reload when no + // rewritten scriptText comes back) must be preserved. + const source = [ + '
', + '
', + ].join("\n"); + const { iframe, a, b } = makeTwoClipPair(); + const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); + const reloadPreview = vi.fn(); + const fetchMock = stubProjectFetch(source, { ok: true, mutated: false }); + const { groupMove, unmount } = renderTimelineEditingHook({ + timelineElements: [a, b], + iframe, + onZIndexCommit: vi.fn().mockResolvedValue(undefined), + projectId: "p1", + writeProjectFile, + recordEdit: vi.fn(async () => {}), + reloadPreview, + }); + + await act(async () => { + await groupMove([ + { element: a, start: a.start, track: 2 }, + { element: b, start: 1.5, track: b.track }, + ]); + await flushAsyncWork(); + }); + + expect(writeProjectFile).toHaveBeenCalledTimes(1); + // The time-shifted clip still goes through the GSAP shift endpoint... + expect( + fetchMock.mock.calls.some((call) => requestUrl(call[0]).includes("gsap-mutations")), + ).toBe(true); + // ...and with no rewritten scriptText the sync escalates to one full reload. + expect(reloadPreview).toHaveBeenCalledTimes(1); + + unmount(); + }); + it("matches the single-clip move output when a group move contains one clip", async () => { const source = '
'; const clip = timelineElement({ id: "clip", track: 0, zIndex: 0, start: 0, duration: 1 }); @@ -949,79 +1019,56 @@ describe("useTimelineEditing duration rollback on failed persist", () => { return { iframe, clip, hook, writeError }; } - it("rolls back the store duration and live root when a move persist fails", async () => { - const { iframe, clip, hook, writeError } = setupFailedPersist(); - + /** + * Run a persist that must reject with the harness's writeError, then assert + * the store duration AND the live root rolled back to the pre-edit 4s — + * the shared act/assert core of the four failed-persist tests below. + */ + async function expectPersistRollback( + ctx: ReturnType, + run: () => Promise, + ): Promise { let rejection: unknown; await act(async () => { - // Move past the end: the optimistic sync grows the readout to 5s. - await hook.move(clip, { start: 3, track: clip.track }).catch((error) => { + await run().catch((error) => { rejection = error; }); await flushAsyncWork(); }); - - expect(rejection).toBe(writeError); + expect(rejection).toBe(ctx.writeError); expect(usePlayerStore.getState().duration).toBe(4); - expect(rootDurationAttr(iframe)).toBe("4"); + expect(rootDurationAttr(ctx.iframe)).toBe("4"); + } - hook.unmount(); + it("rolls back the store duration and live root when a move persist fails", async () => { + const ctx = setupFailedPersist(); + // Move past the end: the optimistic sync grows the readout to 5s. + await expectPersistRollback(ctx, () => + ctx.hook.move(ctx.clip, { start: 3, track: ctx.clip.track }), + ); + ctx.hook.unmount(); }); it("rolls back the store duration and live root when a resize persist fails", async () => { - const { iframe, clip, hook, writeError } = setupFailedPersist(); - - let rejection: unknown; - await act(async () => { - await hook - .resize(clip, { start: 0, duration: 6, playbackStart: undefined }) - .catch((error) => { - rejection = error; - }); - await flushAsyncWork(); - }); - - expect(rejection).toBe(writeError); - expect(usePlayerStore.getState().duration).toBe(4); - expect(rootDurationAttr(iframe)).toBe("4"); - - hook.unmount(); + const ctx = setupFailedPersist(); + await expectPersistRollback(ctx, () => + ctx.hook.resize(ctx.clip, { start: 0, duration: 6, playbackStart: undefined }), + ); + ctx.hook.unmount(); }); it("rolls back the store duration and live root when a group move persist fails", async () => { - const { iframe, clip, hook, writeError } = setupFailedPersist(); - - let rejection: unknown; - await act(async () => { - await hook.groupMove([{ element: clip, start: 3.5 }]).catch((error) => { - rejection = error; - }); - await flushAsyncWork(); - }); - - expect(rejection).toBe(writeError); - expect(usePlayerStore.getState().duration).toBe(4); - expect(rootDurationAttr(iframe)).toBe("4"); - - hook.unmount(); + const ctx = setupFailedPersist(); + await expectPersistRollback(ctx, () => ctx.hook.groupMove([{ element: ctx.clip, start: 3.5 }])); + ctx.hook.unmount(); }); it("rolls back the store duration and live root when a group resize persist fails", async () => { - const { iframe, clip, hook, writeError } = setupFailedPersist(); - - let rejection: unknown; - await act(async () => { - await hook.groupResize([{ element: clip, start: 0, duration: 7 }]).catch((error) => { - rejection = error; - }); - await flushAsyncWork(); - }); - - expect(rejection).toBe(writeError); - expect(usePlayerStore.getState().duration).toBe(4); - expect(rootDurationAttr(iframe)).toBe("4"); - - hook.unmount(); + const ctx = setupFailedPersist(); + await expectPersistRollback(ctx, () => + ctx.hook.groupResize([{ element: ctx.clip, start: 0, duration: 7 }]), + ); + ctx.hook.unmount(); }); it("rolls back the store duration and live root when a delete persist fails", async () => { diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index c6022df4a6..2d9ff139b8 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -228,12 +228,27 @@ export function useTimelineGroupEditing({ patchIframeDomTiming(previewIframeRef.current, change.element, attrs); } + // TRACK-ONLY batch: every change keeps its start (moves never carry a + // duration change), so nothing timing-related changed — the batch only + // rewrites data-track-index, which the renderer never reads (documented + // in core runtime/timeline.ts; track is a studio lane concept). The live + // DOM patch above + the gesture owner's optimistic store update fully + // cover the UI, so after the persist there is nothing to GSAP-shift and + // nothing for the preview to recompute: skip the fallback below entirely. + // Running it anyway is what made the mirrored z-order lane move blink — + // a zero-delta batch yields no scriptText, and finishGroupTimingGsapFallback + // full-reloads the iframe when there is no script to soft-swap. + const trackOnly = changes.every((change) => change.start === change.element.start); + const maxEnd = Math.max(...changes.map((change) => change.start + change.element.duration)); // Snapshot the duration BEFORE the optimistic updates below so a failed // persist can roll the readout + live root back (see captureDurationRollback). const rollbackDuration = captureDurationRollback(previewIframeRef.current); // needsExtension gates the SDK path (setTiming can't grow the root duration), // so read the store BEFORE the readout sync below optimistically updates it. + // Track-only batches leave every clip end unchanged, so both this and the + // readout sync below are provable no-ops there — kept unconditional so the + // duration machinery stays on one code path. const needsExtension = extendRootDurationIfNeeded(maxEnd); // Optimistic duration readout: content-driven (grow AND shrink), read from // the just-patched live DOM. See syncPreviewContentDuration. @@ -270,6 +285,10 @@ export function useTimelineGroupEditing({ coalesceKey, coalesceMs, ); + // Track-only: no timing delta → no GSAP positions to shift and no + // reload (see the trackOnly doc above). Mixed batches (any start + // change) keep the full fallback below. + if (trackOnly) return; await finishGroupTimingGsapFallback({ projectId, iframe: previewIframeRef.current, diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index 0900cec5da..1d30609a36 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -30,6 +30,7 @@ import { import { STUDIO_PREVIEW_FPS } from "../lib/time"; import { useResolvedTimelineEditCallbacks } from "./useResolvedTimelineEditCallbacks"; import type { TimelineProps } from "./TimelineTypes"; +import { useTrackGapMenu } from "./useTrackGapMenu"; // Re-export pure utilities so existing imports from "./Timeline" still resolve. export { @@ -115,15 +116,14 @@ export const Timeline = memo(function Timeline({ const [razorGuideX, setRazorGuideX] = useState(null); useMountEffect(() => { - const down = (e: KeyboardEvent) => e.key === "Shift" && setShiftHeld(true); - const up = (e: KeyboardEvent) => e.key === "Shift" && setShiftHeld(false); + const key = (e: KeyboardEvent) => e.key === "Shift" && setShiftHeld(e.type === "keydown"); const blur = () => setShiftHeld(false); - window.addEventListener("keydown", down); - window.addEventListener("keyup", up); + window.addEventListener("keydown", key); + window.addEventListener("keyup", key); window.addEventListener("blur", blur); return () => { - window.removeEventListener("keydown", down); - window.removeEventListener("keyup", up); + window.removeEventListener("keydown", key); + window.removeEventListener("keyup", key); window.removeEventListener("blur", blur); }; }); @@ -159,8 +159,7 @@ export const Timeline = memo(function Timeline({ containerRef.current = el; }, []); - // Last horizontal scroll offset, RESTORED across the post-edit iframe reload (which clamps into - // a scroll jump); with the pinned zoom this keeps the user parked at the same spot after edits. + // Last horizontal scroll offset, restored across the post-edit iframe reload (pinned zoom). const lastScrollLeftRef = useRef(0); const setScrollRef = useCallback( (el: HTMLDivElement | null) => { @@ -191,8 +190,7 @@ export const Timeline = memo(function Timeline({ const effectiveDuration = useMemo(() => { const safeDur = Number.isFinite(duration) ? duration : 0; if (rawElements.length === 0) return safeDur; - const maxEnd = Math.max(...rawElements.map((el) => el.start + el.duration)); - const result = Math.max(safeDur, maxEnd); + const result = Math.max(safeDur, ...rawElements.map((el) => el.start + el.duration)); return Number.isFinite(result) ? result : safeDur; }, [rawElements, duration]); @@ -206,8 +204,7 @@ export const Timeline = memo(function Timeline({ const ppsRef = useRef(100); const durationRef = useRef(effectiveDuration); durationRef.current = effectiveDuration; - // Declared here (used before the fitPps derivation below) so the edit-pin - // wrappers can close over it; `fitPpsRef.current` is refreshed each render. + // Declared before the fitPps derivation so the edit-pin wrappers can close over it. const fitPpsRef = useRef(100); const { @@ -236,6 +233,15 @@ export const Timeline = memo(function Timeline({ expandedElementsRef, }); + const { gapMenuModel, openGapMenu, dismissGapMenu, closeTrackGap, closeAllTrackGaps } = + useTrackGapMenu({ + tracks, + expandedElementsRef, + trackOrderRef, + onMoveElement: pinnedOnMoveElement, + onMoveElements: pinnedOnMoveElements, + }); + const { draggedClip, setDraggedClip, @@ -363,8 +369,7 @@ export const Timeline = memo(function Timeline({ trackOrderRef, onSelectElement, }); - // Wire setRangeSelection into the stable ref consumed by useTimelineClipDrag - setRangeSelectionRef.current = setRangeSelection; + setRangeSelectionRef.current = setRangeSelection; // stable ref consumed by useTimelineClipDrag const prevSelectedRef = useRef(selectedElementRef.current); // eslint-disable-next-line no-restricted-syntax, react-hooks/exhaustive-deps @@ -443,8 +448,7 @@ export const Timeline = memo(function Timeline({ tabIndex={-1} className={`${zoomMode === "fit" ? "overflow-x-hidden" : "overflow-x-auto"} overflow-y-auto h-full outline-none`} onScroll={(e) => { - // Remember the live offset so it can be restored across a post-edit reload. - lastScrollLeftRef.current = e.currentTarget.scrollLeft; + lastScrollLeftRef.current = e.currentTarget.scrollLeft; // restored across post-edit reload }} onDragOver={handleAssetDragOver} onDragLeave={() => clearDropPreview()} @@ -545,8 +549,14 @@ export const Timeline = memo(function Timeline({ e.preventDefault(); setSelectedElementId(el.key ?? el.id); onSelectElement?.(el); + dismissGapMenu(); setClipContextMenu({ x: e.clientX, y: e.clientY, element: el }); }} + onContextMenuLane={(e, track, time) => { + if (draggedClip?.started || resizingClip) return; + setClipContextMenu(null); + openGapMenu({ x: e.clientX, y: e.clientY, track, time }); + }} /> {activeTool === "razor" && razorGuideX !== null && (
); diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 288b53cbf8..5a8bf48e1a 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -82,6 +82,12 @@ export interface TimelineLaneBaseProps { toClipPercentage: number, ) => void; onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void; + /** + * Right-click on EMPTY lane space (not on a clip — those preventDefault + * before this fires — not the gutter/ruler, not below the lanes). `time` is + * the timeline time (seconds) under the pointer on that lane. + */ + onContextMenuLane?: (e: React.MouseEvent, track: number, time: number) => void; beatAnalysis?: MusicBeatAnalysis | null; } @@ -136,6 +142,7 @@ export function TimelineLanes({ onContextMenuKeyframe, onMoveKeyframe, onContextMenuClip, + onContextMenuLane, beatAnalysis, onToggleTrackHidden, onResizeElement, @@ -221,6 +228,17 @@ export function TimelineLanes({ transition: "opacity 120ms ease", }} className="relative" + onContextMenu={(e: React.MouseEvent) => { + // Clip / keyframe-diamond context menus preventDefault at the + // target before this bubble handler runs — respect them so a + // right-click on a clip never also opens the gap menu. + if (e.defaultPrevented || !onContextMenuLane) return; + const rect = e.currentTarget.getBoundingClientRect(); + const time = (e.clientX - rect.left) / pps; + if (time < 0) return; + e.preventDefault(); + onContextMenuLane(e, trackNum, time); + }} > {/* Faint beat lines in every track's background (behind the clips); the active move-snap target is highlighted. */} diff --git a/packages/studio/src/player/components/TimelineOverlays.tsx b/packages/studio/src/player/components/TimelineOverlays.tsx index 0f0efcb73d..f3e5f1fc4e 100644 --- a/packages/studio/src/player/components/TimelineOverlays.tsx +++ b/packages/studio/src/player/components/TimelineOverlays.tsx @@ -8,6 +8,7 @@ import { type KeyframeDiamondContextMenuState, } from "./KeyframeDiamondContextMenu"; import { ClipContextMenu } from "./ClipContextMenu"; +import { TrackGapContextMenu } from "./TrackGapContextMenu"; import { TimelineShortcutHint } from "./TimelineShortcutHint"; interface ClipContextMenuState { @@ -16,6 +17,16 @@ interface ClipContextMenuState { element: TimelineElement; } +/** Resolved model for the empty-lane-space (track gap) context menu. */ +interface TrackGapContextMenuState { + x: number; + y: number; + gapWidth: number | null; + canCloseGap: boolean; + canCloseAllGaps: boolean; + hasAnyGaps: boolean; +} + interface TimelineOverlaysProps { theme: TimelineTheme; showShortcutHint: boolean; @@ -36,6 +47,10 @@ interface TimelineOverlaysProps { onSplitElement: TimelineEditCallbacks["onSplitElement"]; pinZoomBeforeEdit: () => void; onDeleteElement?: (element: TimelineElement) => Promise | void; + gapContextMenu: TrackGapContextMenuState | null; + onDismissGapContextMenu: () => void; + onCloseTrackGap: () => void; + onCloseAllTrackGaps: () => void; } // The timeline's floating overlays, rendered as siblings above the scroll area: @@ -61,6 +76,10 @@ export function TimelineOverlays({ onSplitElement, pinZoomBeforeEdit, onDeleteElement, + gapContextMenu, + onDismissGapContextMenu, + onCloseTrackGap, + onCloseAllTrackGaps, }: TimelineOverlaysProps) { return ( <> @@ -117,6 +136,20 @@ export function TimelineOverlays({ }} /> )} + + {gapContextMenu && ( + + )} ); } diff --git a/packages/studio/src/player/components/TrackGapContextMenu.tsx b/packages/studio/src/player/components/TrackGapContextMenu.tsx new file mode 100644 index 0000000000..1d7fb0be45 --- /dev/null +++ b/packages/studio/src/player/components/TrackGapContextMenu.tsx @@ -0,0 +1,100 @@ +import { memo } from "react"; +import { createPortal } from "react-dom"; +import { useContextMenuDismiss } from "../../hooks/useContextMenuDismiss"; + +interface TrackGapContextMenuProps { + x: number; + y: number; + /** Width (seconds) of the gap under the pointer, or null when no clip exists to the right. */ + gapWidth: number | null; + /** "Close gap" actionable: a gap exists AND every clip that must shift is movable. */ + canCloseGap: boolean; + /** "Close all gaps" actionable: the lane has gaps AND every shifting clip is movable. */ + canCloseAllGaps: boolean; + /** The lane has at least one gap (distinguishes the two disabled reasons). */ + hasAnyGaps: boolean; + onClose: () => void; + onCloseGap: () => void; + onCloseAllGaps: () => void; +} + +/** + * Context menu for right-clicking EMPTY space on a timeline lane + * (CapCut/Premiere-style). Offers "Close gap" (collapse the clicked gap by + * shifting the following clips on that lane left) and "Close all gaps" + * (compact the whole lane contiguous from 0). Styling mirrors ClipContextMenu. + */ +export const TrackGapContextMenu = memo(function TrackGapContextMenu({ + x, + y, + gapWidth, + canCloseGap, + canCloseAllGaps, + hasAnyGaps, + onClose, + onCloseGap, + onCloseAllGaps, +}: TrackGapContextMenuProps) { + const menuRef = useContextMenuDismiss(onClose); + + const menuWidth = 200; + const menuHeight = 68; + const overflowY = y + menuHeight - window.innerHeight; + const adjustedX = x + menuWidth > window.innerWidth ? x - menuWidth : x; + const adjustedY = overflowY > 0 ? y - overflowY - 8 : y; + + const itemClass = (enabled: boolean) => + `w-full flex items-center justify-between px-3 py-1.5 text-xs text-left ${ + enabled + ? "text-neutral-300 hover:bg-neutral-800 cursor-pointer" + : "text-neutral-600 cursor-not-allowed" + }`; + + return createPortal( +
+ {/* "Close gap" is only offered when a gap exists under the pointer; it + stays visible-but-disabled when a shifting clip is locked, so the + refusal is discoverable rather than a silently missing item. */} + {gapWidth != null && ( + + )} + +
, + document.body, + ); +}); diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index a8364551cb..805d853787 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -107,8 +107,12 @@ function canMoveElement(element: TimelineElement): boolean { * `false` so the caller also skips the z-sync (no orphaned z patch). * * The DOM is updated synchronously up front; the returned promise never rejects. + * + * Exported for reuse by non-drag batch time-moves (track gap closing — see + * timelineGapCommit.ts) so they share the same optimistic-apply, rollback, and + * atomic single-undo persist semantics as a drag commit. */ -function persistMoveEdits( +export function persistMoveEdits( edits: TimelineMoveEdit[], deps: DragCommitDeps, coalesceKey?: string, diff --git a/packages/studio/src/player/components/timelineGapCommit.test.ts b/packages/studio/src/player/components/timelineGapCommit.test.ts new file mode 100644 index 0000000000..5fdb280398 --- /dev/null +++ b/packages/studio/src/player/components/timelineGapCommit.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it, vi } from "vitest"; +import type { TimelineElement } from "../store/playerStore"; +import type { DragCommitDeps } from "./timelineClipDragCommit"; +import { + canShiftTrackGapClips, + commitCloseAllTrackGaps, + commitCloseTrackGap, +} from "./timelineGapCommit"; +import { resolveAllTrackGaps } from "./timelineGaps"; + +function el(id: string, start: number, duration: number, track = 0): TimelineElement { + // domId + video tag → getTimelineEditCapabilities(...).canMove === true + return { id, tag: "video", start, duration, track, domId: id }; +} + +function lockedEl(id: string, start: number, duration: number): TimelineElement { + return { ...el(id, start, duration), timelineLocked: true }; +} + +function makeDeps(laneElements: TimelineElement[]) { + const onMoveElements = vi.fn(() => Promise.resolve()); + const updateElement = vi.fn(); + const deps: DragCommitDeps = { + elements: laneElements, + trackOrder: [0], + updateElement, + onMoveElements, + }; + return { deps, onMoveElements, updateElement }; +} + +/** Assert exactly ONE atomic persist batch; return its flattened edits + coalesce key. */ +function singleBatch(onMoveElements: ReturnType) { + expect(onMoveElements).toHaveBeenCalledTimes(1); + const [edits, coalesceKey] = onMoveElements.mock.calls[0] as unknown as [ + Array<{ element: TimelineElement; updates: { start: number; track: number } }>, + string, + ]; + return { coalesceKey, edits: edits.map((e) => ({ id: e.element.id, ...e.updates })) }; +} + +describe("commitCloseTrackGap", () => { + it("persists ONE atomic batch shifting the next clip and every clip after it", () => { + const lane = [el("a", 0, 2), el("b", 5, 3), el("c", 10, 1)]; + const { deps, onMoveElements } = makeDeps(lane); + + expect(commitCloseTrackGap(lane, 3, deps)).toBe(true); + + const { edits, coalesceKey } = singleBatch(onMoveElements); + // Gap is [2, 5) → width 3; b and c shift left by 3, tracks unchanged. + expect(edits).toEqual([ + { id: "b", start: 2, track: 0 }, + { id: "c", start: 7, track: 0 }, + ]); + expect(typeof coalesceKey).toBe("string"); + expect(coalesceKey).toMatch(/^track-gap-close:\d+$/); + }); + + it("optimistically applies the same starts to the store", () => { + const lane = [el("a", 0, 2), el("b", 5, 3)]; + const { deps, updateElement } = makeDeps(lane); + commitCloseTrackGap(lane, 3, deps); + expect(updateElement).toHaveBeenCalledWith("b", { start: 2, track: 0 }); + }); + + it("closes the leading gap (first clip lands at 0)", () => { + const lane = [el("a", 2, 3), el("b", 6, 1)]; + const { deps, onMoveElements } = makeDeps(lane); + + expect(commitCloseTrackGap(lane, 1, deps)).toBe(true); + const [edits] = onMoveElements.mock.calls[0] as unknown as [ + Array<{ element: TimelineElement; updates: { start: number; track: number } }>, + ]; + expect(edits.map((e) => ({ id: e.element.id, start: e.updates.start }))).toEqual([ + { id: "a", start: 0 }, + { id: "b", start: 4 }, + ]); + }); + + it("uses a fresh coalesce key per gesture", () => { + const lane = [el("a", 0, 2), el("b", 5, 3)]; + const first = makeDeps(lane); + const second = makeDeps(lane); + commitCloseTrackGap(lane, 3, first.deps); + commitCloseTrackGap(lane, 3, second.deps); + const keyA = first.onMoveElements.mock.calls[0][1 as never]; + const keyB = second.onMoveElements.mock.calls[0][1 as never]; + expect(keyA).not.toEqual(keyB); + }); + + it("refuses (no write) when there is no clip right of the point", () => { + const lane = [el("a", 0, 2)]; + const { deps, onMoveElements, updateElement } = makeDeps(lane); + expect(commitCloseTrackGap(lane, 5, deps)).toBe(false); + expect(onMoveElements).not.toHaveBeenCalled(); + expect(updateElement).not.toHaveBeenCalled(); + }); + + it("refuses (no partial compaction) when ANY shifting clip is unmovable", () => { + const lane = [el("a", 0, 2), el("b", 5, 3), lockedEl("c", 10, 1)]; + const { deps, onMoveElements, updateElement } = makeDeps(lane); + expect(commitCloseTrackGap(lane, 3, deps)).toBe(false); + expect(onMoveElements).not.toHaveBeenCalled(); + expect(updateElement).not.toHaveBeenCalled(); + }); +}); + +describe("commitCloseAllTrackGaps", () => { + it("compacts the whole lane in ONE atomic batch (leading gap included)", () => { + const lane = [el("a", 1, 2), el("b", 5, 3), el("c", 10, 1)]; + const { deps, onMoveElements } = makeDeps(lane); + + expect(commitCloseAllTrackGaps(lane, deps)).toBe(true); + + const { edits, coalesceKey } = singleBatch(onMoveElements); + expect(edits).toEqual([ + { id: "a", start: 0, track: 0 }, + { id: "b", start: 2, track: 0 }, + { id: "c", start: 5, track: 0 }, + ]); + expect(coalesceKey).toMatch(/^track-gap-close:\d+$/); + }); + + it("refuses when the track is already contiguous (no gaps)", () => { + const lane = [el("a", 0, 2), el("b", 2, 3)]; + const { deps, onMoveElements } = makeDeps(lane); + expect(commitCloseAllTrackGaps(lane, deps)).toBe(false); + expect(onMoveElements).not.toHaveBeenCalled(); + }); + + it("refuses when any shifting clip is unmovable, even if others could move", () => { + const lane = [lockedEl("a", 1, 2), el("b", 5, 3)]; + const { deps, onMoveElements, updateElement } = makeDeps(lane); + expect(commitCloseAllTrackGaps(lane, deps)).toBe(false); + expect(onMoveElements).not.toHaveBeenCalled(); + expect(updateElement).not.toHaveBeenCalled(); + }); + + it("proceeds when an unmovable clip does NOT need to shift", () => { + // Locked clip already sits flush at 0 — only movable clips shift. + const lane = [lockedEl("a", 0, 2), el("b", 4, 1)]; + const { deps, onMoveElements } = makeDeps(lane); + expect(commitCloseAllTrackGaps(lane, deps)).toBe(true); + const [edits] = onMoveElements.mock.calls[0] as unknown as [ + Array<{ element: TimelineElement; updates: { start: number } }>, + ]; + expect(edits.map((e) => ({ id: e.element.id, start: e.updates.start }))).toEqual([ + { id: "b", start: 2 }, + ]); + }); +}); + +describe("canShiftTrackGapClips", () => { + it("is true only when every named clip is movable", () => { + const lane = [el("a", 1, 2), lockedEl("b", 5, 3)]; + expect(canShiftTrackGapClips(lane, [{ key: "a", newStart: 0 }])).toBe(true); + expect(canShiftTrackGapClips(lane, resolveAllTrackGaps(lane))).toBe(false); + }); + + it("is false for unknown keys", () => { + expect(canShiftTrackGapClips([el("a", 0, 1)], [{ key: "ghost", newStart: 0 }])).toBe(false); + }); +}); diff --git a/packages/studio/src/player/components/timelineGapCommit.ts b/packages/studio/src/player/components/timelineGapCommit.ts new file mode 100644 index 0000000000..9f350d9732 --- /dev/null +++ b/packages/studio/src/player/components/timelineGapCommit.ts @@ -0,0 +1,98 @@ +import type { TimelineElement } from "../store/playerStore"; +import { getTimelineEditCapabilities } from "./timelineEditing"; +import { + persistMoveEdits, + type DragCommitDeps, + type TimelineMoveEdit, +} from "./timelineClipDragCommit"; +import { + resolveAllTrackGaps, + resolveCloseGapShifts, + resolveTrackGapAt, + type TrackGapShift, +} from "./timelineGaps"; + +/** + * Commit layer for the track-gap context menu ("Close gap" / "Close all gaps"). + * + * Each action is ONE atomic {@link persistMoveEdits} batch — pure time moves + * (`updates.track === element.track`, no authored-track rewrite) tagged with a + * per-gesture-unique coalesce key, so an action is exactly one undo entry and + * flows through the existing move pipeline (optimistic store apply + rollback, + * SDK fast path, patchIframeDomTiming preview). + * + * Refusal rule: if ANY clip that must shift is unmovable + * ({@link getTimelineEditCapabilities}.canMove === false), the whole action is + * refused — never a partial compaction. The menu disables the item via + * {@link canShiftTrackGapClips}; the commit re-checks as defense in depth. + */ + +const keyOf = (e: TimelineElement) => e.key ?? e.id; + +// Per-gesture-unique coalesce key. A monotonic counter — NOT Date.now() / +// Math.random() (determinism rules) — mirrors laneChangeGestureSeq in +// timelineClipDragCommit.ts. +let gapCloseGestureSeq = 0; + +/** True when every clip named in `shifts` may be time-moved. */ +export function canShiftTrackGapClips( + laneElements: readonly TimelineElement[], + shifts: readonly TrackGapShift[], +): boolean { + const byKey = new Map(laneElements.map((e) => [keyOf(e), e])); + return shifts.every((s) => { + const element = byKey.get(s.key); + return element != null && getTimelineEditCapabilities(element).canMove; + }); +} + +function buildShiftEdits( + laneElements: readonly TimelineElement[], + shifts: readonly TrackGapShift[], +): TimelineMoveEdit[] | null { + if (shifts.length === 0 || !canShiftTrackGapClips(laneElements, shifts)) return null; + const byKey = new Map(laneElements.map((e) => [keyOf(e), e])); + return shifts.map((s) => { + const element = byKey.get(s.key)!; + return { element, updates: { start: s.newStart, track: element.track } }; + }); +} + +function commitShifts( + laneElements: readonly TimelineElement[], + shifts: readonly TrackGapShift[], + deps: DragCommitDeps, +): boolean { + const edits = buildShiftEdits(laneElements, shifts); + if (!edits) return false; + void persistMoveEdits(edits, deps, `track-gap-close:${gapCloseGestureSeq++}`); + return true; +} + +/** + * Close the ONE gap under `time` on the lane: the next clip and every clip + * after it on that lane shift left by the gap's width. Returns false (and + * writes nothing) when there is no gap at the point or a shifting clip is + * unmovable. + */ +export function commitCloseTrackGap( + laneElements: readonly TimelineElement[], + time: number, + deps: DragCommitDeps, +): boolean { + const gap = resolveTrackGapAt(laneElements, time); + if (!gap) return false; + return commitShifts(laneElements, resolveCloseGapShifts(laneElements, gap), deps); +} + +/** + * Compact the whole lane (leading gap included): clips become contiguous from + * 0, order and durations preserved. Returns false (and writes nothing) when + * the lane has no gaps or a shifting clip is unmovable. + */ +export function commitCloseAllTrackGaps( + laneElements: readonly TimelineElement[], + deps: DragCommitDeps, +): boolean { + return commitShifts(laneElements, resolveAllTrackGaps(laneElements), deps); +} diff --git a/packages/studio/src/player/components/timelineGaps.test.ts b/packages/studio/src/player/components/timelineGaps.test.ts new file mode 100644 index 0000000000..bb2fae2f08 --- /dev/null +++ b/packages/studio/src/player/components/timelineGaps.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from "vitest"; +import type { TimelineElement } from "../store/playerStore"; +import { + resolveAllTrackGaps, + resolveCloseGapShifts, + resolveTrackGapAt, + trackHasGaps, +} from "./timelineGaps"; + +function el(id: string, start: number, duration: number): TimelineElement { + return { id, tag: "video", start, duration, track: 0, domId: id }; +} + +describe("resolveTrackGapAt", () => { + it("resolves a middle gap between two clips", () => { + const els = [el("a", 0, 2), el("b", 5, 3)]; + const gap = resolveTrackGapAt(els, 3); + expect(gap).toEqual({ gapStart: 2, gapEnd: 5, followingKeys: ["b"] }); + }); + + it("resolves the leading gap before the first clip (gapStart = 0)", () => { + const els = [el("a", 2, 3), el("b", 6, 1)]; + const gap = resolveTrackGapAt(els, 1); + expect(gap).toEqual({ gapStart: 0, gapEnd: 2, followingKeys: ["a", "b"] }); + }); + + it("includes EVERY clip at/after the gap in followingKeys", () => { + const els = [el("a", 0, 1), el("b", 3, 1), el("c", 5, 1), el("d", 8, 1)]; + const gap = resolveTrackGapAt(els, 2); + expect(gap).toEqual({ gapStart: 1, gapEnd: 3, followingKeys: ["b", "c", "d"] }); + }); + + it("returns null when there is no clip to the right of the point", () => { + const els = [el("a", 0, 2)]; + expect(resolveTrackGapAt(els, 5)).toBeNull(); + expect(resolveTrackGapAt([], 1)).toBeNull(); // empty lane + }); + + it("returns null when the point is inside a clip (half-open interval)", () => { + const els = [el("a", 1, 2), el("b", 5, 1)]; + expect(resolveTrackGapAt(els, 2)).toBeNull(); // strictly inside + expect(resolveTrackGapAt(els, 1)).toBeNull(); // at clip start (occupied) + // At clip END (half-open) the point is free — the gap to "b" resolves. + expect(resolveTrackGapAt(els, 3)).toEqual({ gapStart: 3, gapEnd: 5, followingKeys: ["b"] }); + }); + + it("resolves the leading gap for a single clip", () => { + const gap = resolveTrackGapAt([el("a", 4, 2)], 1); + expect(gap).toEqual({ gapStart: 0, gapEnd: 4, followingKeys: ["a"] }); + }); + + it("treats epsilon-adjacent clips as gapless (float drift)", () => { + // 8.4 + 2.7 = 11.100000000000001 — the classic drift: no point near the + // seam resolves a gap. + const drifted = [el("a", 8.4, 2.7), el("b", 11.1, 2)]; + expect(resolveTrackGapAt(drifted, 11.0999)).toBeNull(); + expect(resolveTrackGapAt(drifted, 11.1005)).toBeNull(); + // A sub-epsilon sliver between clips is not a closable gap either. + const sliver = [el("a", 0, 2.0004), el("b", 2.001, 1)]; + expect(resolveTrackGapAt(sliver, 1.9995)).toBeNull(); + }); + + it("handles overlapping clips sanely (uses the max end left of the point)", () => { + const els = [el("a", 0, 4), el("b", 1, 2), el("c", 6, 1)]; + const gap = resolveTrackGapAt(els, 5); + expect(gap).toEqual({ gapStart: 4, gapEnd: 6, followingKeys: ["c"] }); + }); + + it("prefers the key over the id when present", () => { + const withKey = { ...el("a", 3, 1), key: "a-key" }; + const gap = resolveTrackGapAt([withKey], 1); + expect(gap?.followingKeys).toEqual(["a-key"]); + }); +}); + +describe("resolveCloseGapShifts", () => { + it("shifts the following clips left by exactly the gap width", () => { + const els = [el("a", 0, 2), el("b", 5, 3), el("c", 10, 1)]; + const gap = resolveTrackGapAt(els, 3)!; + expect(resolveCloseGapShifts(els, gap)).toEqual([ + { key: "b", newStart: 2 }, + { key: "c", newStart: 7 }, + ]); + }); + + it("closing the leading gap lands the first clip at 0", () => { + const els = [el("a", 2, 3), el("b", 6, 1)]; + const gap = resolveTrackGapAt(els, 1)!; + expect(resolveCloseGapShifts(els, gap)).toEqual([ + { key: "a", newStart: 0 }, + { key: "b", newStart: 4 }, + ]); + }); + + it("rounds shifted starts to millisecond precision", () => { + const els = [el("a", 0, 1.1), el("b", 3.3000000000000003, 1)]; + const gap = resolveTrackGapAt(els, 2)!; + const shifts = resolveCloseGapShifts(els, gap); + expect(shifts).toEqual([{ key: "b", newStart: 1.1 }]); + }); +}); + +describe("resolveAllTrackGaps", () => { + it("compacts every gap: contiguous from 0, order and durations preserved", () => { + const els = [el("a", 1, 2), el("b", 5, 3), el("c", 10, 1)]; + expect(resolveAllTrackGaps(els)).toEqual([ + { key: "a", newStart: 0 }, + { key: "b", newStart: 2 }, + { key: "c", newStart: 5 }, + ]); + }); + + it("includes the leading gap for a single clip", () => { + expect(resolveAllTrackGaps([el("a", 4, 2)])).toEqual([{ key: "a", newStart: 0 }]); + }); + + it("returns only the clips whose start actually changes", () => { + const els = [el("a", 0, 2), el("b", 2, 1), el("c", 5, 1)]; + expect(resolveAllTrackGaps(els)).toEqual([{ key: "c", newStart: 3 }]); + }); + + it("returns [] for an already-contiguous track and for an empty lane", () => { + expect(resolveAllTrackGaps([el("a", 0, 2), el("b", 2, 3)])).toEqual([]); + expect(resolveAllTrackGaps([])).toEqual([]); + }); + + it("ignores epsilon-level drift instead of emitting no-op shifts", () => { + const els = [el("a", 0, 8.4), el("b", 8.4, 2.7), el("c", 11.100000000000001, 2)]; + expect(resolveAllTrackGaps(els)).toEqual([]); + }); + + it("serializes overlapping clips in start order (sum-of-durations rule)", () => { + const els = [el("a", 0, 4), el("b", 2, 2)]; + expect(resolveAllTrackGaps(els)).toEqual([{ key: "b", newStart: 4 }]); + }); + + it("is deterministic for identical starts (key tie-break)", () => { + const els = [el("b", 3, 1), el("a", 3, 2)]; + expect(resolveAllTrackGaps(els)).toEqual([ + { key: "a", newStart: 0 }, + { key: "b", newStart: 2 }, + ]); + }); +}); + +describe("trackHasGaps", () => { + it("detects gaps, including the leading gap", () => { + expect(trackHasGaps([el("a", 1, 2)])).toBe(true); + expect(trackHasGaps([el("a", 0, 2), el("b", 4, 1)])).toBe(true); + }); + + it("is false for contiguous or empty tracks", () => { + expect(trackHasGaps([el("a", 0, 2), el("b", 2, 1)])).toBe(false); + expect(trackHasGaps([])).toBe(false); + }); +}); diff --git a/packages/studio/src/player/components/timelineGaps.ts b/packages/studio/src/player/components/timelineGaps.ts new file mode 100644 index 0000000000..f45e2def01 --- /dev/null +++ b/packages/studio/src/player/components/timelineGaps.ts @@ -0,0 +1,119 @@ +import type { TimelineElement } from "../store/playerStore"; + +/** + * Pure gap math for a single timeline display lane (CapCut/Premiere-style + * "Close gap" / "Close all gaps"). Operates on the DISPLAY element set the + * timeline renders for one lane — the caller passes the clips of the + * right-clicked lane only; cross-lane behavior is out of scope by design. + * + * Conventions: + * - A clip occupies the half-open interval [start, start + duration). + * - Comparisons are epsilon-tolerant ({@link TRACK_GAP_EPSILON_S}) so float + * drift (e.g. 8.4 + 2.7 = 11.100000000000001) never fabricates a sliver gap. + * - Computed starts are rounded to millisecond precision, matching the drag + * commit's `round3`. + */ +const TRACK_GAP_EPSILON_S = 1e-3; + +const keyOf = (e: TimelineElement) => e.key ?? e.id; +const round3 = (v: number) => Math.round(v * 1000) / 1000; +const endOf = (e: TimelineElement) => e.start + e.duration; + +/** Lane clips sorted by start (key as a deterministic tie-break). */ +function sortedLaneClips(elements: readonly TimelineElement[]): TimelineElement[] { + return [...elements].sort((a, b) => a.start - b.start || keyOf(a).localeCompare(keyOf(b))); +} + +export interface TrackGapAt { + /** Gap left edge: the max end of the clips left of the point (0 for the leading gap). */ + gapStart: number; + /** Gap right edge: the start of the next clip on the lane. */ + gapEnd: number; + /** Keys of the next clip and every clip after it on the lane, in start order. */ + followingKeys: string[]; +} + +/** + * Resolve the gap under a right-clicked point on one lane. + * + * Returns null when the point sits inside a clip, when there is no clip to the + * right of the point (nothing to close), or when the neighbouring clips are + * epsilon-adjacent (no real gap). + */ +export function resolveTrackGapAt( + elements: readonly TimelineElement[], + time: number, + epsilon: number = TRACK_GAP_EPSILON_S, +): TrackGapAt | null { + const clips = sortedLaneClips(elements); + // Point inside a clip's half-open [start, end) → not empty space. + const occupied = clips.some((c) => time >= c.start - epsilon && time < endOf(c) - epsilon); + if (occupied) return null; + + const following = clips.filter((c) => c.start > time - epsilon); + if (following.length === 0) return null; // nothing to the right — nothing to close + + const gapEnd = following[0].start; + // Max end among clips left of the point (they all end at/before it since the + // point is unoccupied); 0 for the leading gap before the first clip. + const gapStart = Math.max( + 0, + ...clips.filter((c) => c.start <= time - epsilon).map((c) => endOf(c)), + ); + if (gapEnd - gapStart <= epsilon) return null; // epsilon-adjacent — no gap + + return { gapStart, gapEnd, followingKeys: following.map(keyOf) }; +} + +export interface TrackGapShift { + key: string; + newStart: number; +} + +/** + * Compact the whole lane: every clip lands at the sum of the durations of the + * clips before it (contiguous from 0, order and durations preserved). + * Overlapping clips (spill lanes) are serialized in start order — sane, if + * lossy for deliberate overlaps; the display lane set should not contain them. + * + * Returns ONLY the clips whose start actually changes (beyond epsilon). + */ +export function resolveAllTrackGaps( + elements: readonly TimelineElement[], + epsilon: number = TRACK_GAP_EPSILON_S, +): TrackGapShift[] { + const shifts: TrackGapShift[] = []; + let cursor = 0; + for (const clip of sortedLaneClips(elements)) { + const newStart = round3(cursor); + if (Math.abs(newStart - clip.start) > epsilon) { + shifts.push({ key: keyOf(clip), newStart }); + } + cursor += clip.duration; + } + return shifts; +} + +/** Whether the lane has any gap "Close all gaps" would collapse. */ +export function trackHasGaps( + elements: readonly TimelineElement[], + epsilon: number = TRACK_GAP_EPSILON_S, +): boolean { + return resolveAllTrackGaps(elements, epsilon).length > 0; +} + +/** + * Per-clip shifts for closing ONE gap: the next clip and every clip after it + * on the lane move left by the gap's width. Starts are clamped at 0 (float + * safety; real shifts never cross the gap's own left edge). + */ +export function resolveCloseGapShifts( + elements: readonly TimelineElement[], + gap: TrackGapAt, +): TrackGapShift[] { + const width = gap.gapEnd - gap.gapStart; + const followSet = new Set(gap.followingKeys); + return sortedLaneClips(elements) + .filter((c) => followSet.has(keyOf(c))) + .map((c) => ({ key: keyOf(c), newStart: Math.max(0, round3(c.start - width)) })); +} diff --git a/packages/studio/src/player/components/timelineZMirror.test.ts b/packages/studio/src/player/components/timelineZMirror.test.ts index ca30fbda32..3540890321 100644 --- a/packages/studio/src/player/components/timelineZMirror.test.ts +++ b/packages/studio/src/player/components/timelineZMirror.test.ts @@ -25,6 +25,22 @@ function resolve( return resolveZMirrorLaneMove({ action, element, elements, crossedKey }); } +// Target on TOP lane 0; b/c fully occupy the two lanes below over t's span. +const stackBelow = () => { + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); + const c = el("c", 2, 0, 10); + return { t, elements: [t, b, c] }; +}; + +// Sparse file: authored tracks 3/5/7 displayed as lanes 0/1/2 (a free over t's span). +const sparseAuthored = () => { + const a = el("a", 0, 20, 5, { authoredTrack: 3 }); + const b = el("b", 1, 0, 10, { authoredTrack: 5 }); + const t = el("t", 2, 0, 10, { authoredTrack: 7 }); + return { t, elements: [a, b, t] }; +}; + describe("resolveZMirrorLaneMove — bring-forward / send-backward", () => { // Stack: a on lane 0, b on lane 1, target on lane 2 — all overlapping in time. const stack = () => { @@ -87,11 +103,9 @@ describe("resolveZMirrorLaneMove — bring-forward / send-backward", () => { }); it("send-backward inserts below the neighbor when no lane below is free", () => { - const t = el("t", 0, 0, 10); - const b = el("b", 1, 0, 10); - const c = el("c", 2, 0, 10); + const { t, elements } = stackBelow(); // Boundary below b's lane (row 1 + 1 = 2). - expect(resolve("send-backward", t, [t, b, c], "b")).toEqual({ kind: "insert", insertRow: 2 }); + expect(resolve("send-backward", t, elements, "b")).toEqual({ kind: "insert", insertRow: 2 }); }); it("send-backward returns null when nothing overlaps below and no crossedKey", () => { @@ -163,10 +177,8 @@ describe("resolveZMirrorLaneMove — bring-to-front / send-to-back", () => { }); it("send-to-back inserts below the bottommost overlap when no lane is free", () => { - const t = el("t", 0, 0, 10); - const b = el("b", 1, 0, 10); - const c = el("c", 2, 0, 10); - expect(resolve("send-to-back", t, [t, b, c])).toEqual({ kind: "insert", insertRow: 3 }); + const { t, elements } = stackBelow(); + expect(resolve("send-to-back", t, elements)).toEqual({ kind: "insert", insertRow: 3 }); }); it("send-to-back is null when already bottommost among overlaps", () => { @@ -263,12 +275,9 @@ describe("resolveZMirrorLaneMove — zone boundary (audio untouched)", () => { describe("resolveZMirrorLaneMove — authored (persist) space", () => { it("persistTrack takes the target lane occupant's authoredTrack, not the display lane", () => { - // Sparse file: authored tracks 3/5/7 displayed as lanes 0/1/2. Occupant of - // the free-over-span target lane 0 (authored 3) anchors the persist value. - const a = el("a", 0, 20, 5, { authoredTrack: 3 }); - const b = el("b", 1, 0, 10, { authoredTrack: 5 }); - const t = el("t", 2, 0, 10, { authoredTrack: 7 }); - expect(resolve("bring-forward", t, [a, b, t], "b")).toEqual({ + // Occupant of the free-over-span target lane 0 (authored 3) anchors the persist value. + const { t, elements } = sparseAuthored(); + expect(resolve("bring-forward", t, elements, "b")).toEqual({ kind: "move", displayTrack: 0, persistTrack: 3, @@ -332,17 +341,11 @@ describe("resolveZMirrorLaneMove — degenerate inputs and determinism", () => { }); it("identical inputs produce identical outputs (deterministic, input untouched)", () => { - const make = () => { - const a = el("a", 0, 20, 5, { authoredTrack: 3 }); - const b = el("b", 1, 0, 10, { authoredTrack: 5 }); - const t = el("t", 2, 0, 10, { authoredTrack: 7 }); - return { t, elements: [a, b, t] }; - }; - const first = make(); + const first = sparseAuthored(); const snapshot = structuredClone(first.elements); const r1 = resolve("bring-forward", first.t, first.elements, "b"); const r2 = resolve("bring-forward", first.t, first.elements, "b"); - const fresh = make(); + const fresh = sparseAuthored(); const r3 = resolve("bring-forward", fresh.t, fresh.elements, "b"); expect(r1).toEqual(r2); expect(r1).toEqual(r3); diff --git a/packages/studio/src/player/components/useTrackGapMenu.ts b/packages/studio/src/player/components/useTrackGapMenu.ts new file mode 100644 index 0000000000..e354f21b8f --- /dev/null +++ b/packages/studio/src/player/components/useTrackGapMenu.ts @@ -0,0 +1,106 @@ +import { useCallback, useMemo, useState, type MutableRefObject } from "react"; +import { usePlayerStore, type TimelineElement } from "../store/playerStore"; +import type { DragCommitDeps } from "./timelineClipDragCommit"; +import { resolveAllTrackGaps, resolveCloseGapShifts, resolveTrackGapAt } from "./timelineGaps"; +import { + canShiftTrackGapClips, + commitCloseAllTrackGaps, + commitCloseTrackGap, +} from "./timelineGapCommit"; + +/** Right-click anchor on EMPTY lane space: pointer position + clicked lane/time. */ +interface TrackGapMenuAnchor { + x: number; + y: number; + track: number; + time: number; +} + +/** + * Track-gap context menu (right-click on empty lane space) — state, the + * derived menu model, and the two commit actions. Extracted from Timeline.tsx + * as a cohesive unit (600-line studio cap); behavior identical. + * + * Only the ANCHOR is state; the menu model (gap under the pointer, compaction, + * movability) derives from live `tracks` so an open menu reflects concurrent + * edits. Commits are ONE atomic batch each via the existing move-persist + * pipeline (see timelineGapCommit.ts). + */ +export function useTrackGapMenu({ + tracks, + expandedElementsRef, + trackOrderRef, + onMoveElement, + onMoveElements, +}: { + tracks: [number, TimelineElement[]][]; + expandedElementsRef: MutableRefObject; + trackOrderRef: MutableRefObject; + onMoveElement: DragCommitDeps["onMoveElement"]; + onMoveElements: DragCommitDeps["onMoveElements"]; +}) { + const updateElement = usePlayerStore((s) => s.updateElement); + const [gapContextMenu, setGapContextMenu] = useState(null); + + const gapMenuLaneElements = useMemo( + () => (gapContextMenu ? (tracks.find(([t]) => t === gapContextMenu.track)?.[1] ?? []) : null), + [gapContextMenu, tracks], + ); + const gapMenuModel = useMemo(() => { + if (!gapContextMenu || !gapMenuLaneElements) return null; + const gap = resolveTrackGapAt(gapMenuLaneElements, gapContextMenu.time); + const allShifts = resolveAllTrackGaps(gapMenuLaneElements); + return { + x: gapContextMenu.x, + y: gapContextMenu.y, + gapWidth: gap ? gap.gapEnd - gap.gapStart : null, + canCloseGap: + gap != null && + canShiftTrackGapClips(gapMenuLaneElements, resolveCloseGapShifts(gapMenuLaneElements, gap)), + hasAnyGaps: allShifts.length > 0, + canCloseAllGaps: + allShifts.length > 0 && canShiftTrackGapClips(gapMenuLaneElements, allShifts), + }; + }, [gapContextMenu, gapMenuLaneElements]); + + const closeTrackGap = useCallback(() => { + if (!gapContextMenu || !gapMenuLaneElements) return; + commitCloseTrackGap(gapMenuLaneElements, gapContextMenu.time, { + elements: expandedElementsRef.current, + trackOrder: trackOrderRef.current, + updateElement, + onMoveElement, + onMoveElements, + }); + }, [ + gapContextMenu, + gapMenuLaneElements, + expandedElementsRef, + trackOrderRef, + updateElement, + onMoveElement, + onMoveElements, + ]); + const closeAllTrackGaps = useCallback(() => { + if (!gapMenuLaneElements) return; + commitCloseAllTrackGaps(gapMenuLaneElements, { + elements: expandedElementsRef.current, + trackOrder: trackOrderRef.current, + updateElement, + onMoveElement, + onMoveElements, + }); + }, [ + gapMenuLaneElements, + expandedElementsRef, + trackOrderRef, + updateElement, + onMoveElement, + onMoveElements, + ]); + + const openGapMenu = useCallback((anchor: TrackGapMenuAnchor) => setGapContextMenu(anchor), []); + const dismissGapMenu = useCallback(() => setGapContextMenu(null), []); + + return { gapMenuModel, openGapMenu, dismissGapMenu, closeTrackGap, closeAllTrackGaps }; +} From c6ad6c0b06b0d6c34c068ff0d9c76a6dc9e2a38a Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Mon, 13 Jul 2026 22:00:56 -0700 Subject: [PATCH 04/19] fix(studio): rebind-only preview sync for unmutated timing edits, classical z-menu order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Timing edits that rewrote NO GSAP positions (gap closes and moves of selector-addressed caption clips, zero-delta batches, comps without a rewritable script) full-reloaded the preview — and the rerun-current-scripts attempt was wrong for real compositions: re-executing init-style scripts (three.js scenes, caption engines) is exactly the unsafe case, verified live by doubled init warnings and a fallback reload anyway. The correct observation: when mutated === false the existing __timelines are still valid — only the runtime's clip visibility windows are stale, and the live DOM timing attributes were already patched. So the no-mutation path now runs applySoftReloadFinalization only (seek + __hfForceTimelineRebind + manual-edits reapply), extracted from the soft-reload machinery — zero script execution. This also un-blinks comps with no GSAP script at all, which previously always remounted. Rewritten-script soft reloads, cannot-soft-reload, otherFileChanged, and mutation failures keep their existing behavior. gsapSoftReload's undo/redo restore section moved verbatim to gsapUndoRestore.ts for the 600-line cap. Also: z-order menu items reordered to the classical arrangement (Bring to Front, Bring Forward, Send Backward, Send to Back). Live-verified on a three.js-heavy composition: Close-all-gaps shifted 4 caption clips with correct cumulative amounts, the preview iframe was never remounted (marker survived), and one undo reverted everything. --- .../editor/CanvasContextMenu.test.tsx | 2 +- .../components/editor/CanvasContextMenu.tsx | 2 +- .../src/hooks/timelineTimingSync.test.ts | 241 ++++++++++++++++++ .../studio/src/hooks/timelineTimingSync.ts | 73 +++++- .../studio/src/hooks/usePreviewPersistence.ts | 2 +- .../src/hooks/useTimelineEditing.test.tsx | 72 +++++- .../src/hooks/useTimelineGroupEditing.ts | 4 +- .../studio/src/utils/gsapSoftReload.test.ts | 169 ++++-------- packages/studio/src/utils/gsapSoftReload.ts | 194 +++----------- .../studio/src/utils/gsapUndoRestore.test.ts | 117 +++++++++ packages/studio/src/utils/gsapUndoRestore.ts | 165 ++++++++++++ 11 files changed, 762 insertions(+), 279 deletions(-) create mode 100644 packages/studio/src/utils/gsapUndoRestore.test.ts create mode 100644 packages/studio/src/utils/gsapUndoRestore.ts diff --git a/packages/studio/src/components/editor/CanvasContextMenu.test.tsx b/packages/studio/src/components/editor/CanvasContextMenu.test.tsx index cd9ef44c56..0ba25bd011 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.test.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.test.tsx @@ -110,9 +110,9 @@ describe("CanvasContextMenu — handler gating", () => { // Labels stay the exact industry-standard names (the icons add no text)... expect(buttons.map((b) => b.textContent)).toEqual([ + "Bring to front", "Bring forward", "Send backward", - "Bring to front", "Send to back", ]); // ...and each item leads with a stroke icon that inherits the item's text diff --git a/packages/studio/src/components/editor/CanvasContextMenu.tsx b/packages/studio/src/components/editor/CanvasContextMenu.tsx index 2f991e1061..0fae550544 100644 --- a/packages/studio/src/components/editor/CanvasContextMenu.tsx +++ b/packages/studio/src/components/editor/CanvasContextMenu.tsx @@ -131,9 +131,9 @@ function ZActionIcon({ action }: { action: ZAction }) { } const Z_ACTIONS: Array<{ action: ZAction; label: string }> = [ + { action: "bring-to-front", label: "Bring to front" }, { action: "bring-forward", label: "Bring forward" }, { action: "send-backward", label: "Send backward" }, - { action: "bring-to-front", label: "Bring to front" }, { action: "send-to-back", label: "Send to back" }, ]; diff --git a/packages/studio/src/hooks/timelineTimingSync.test.ts b/packages/studio/src/hooks/timelineTimingSync.test.ts index d1a6e8d21a..c6e91cdd53 100644 --- a/packages/studio/src/hooks/timelineTimingSync.test.ts +++ b/packages/studio/src/hooks/timelineTimingSync.test.ts @@ -2,9 +2,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { usePlayerStore } from "../player/store/playerStore"; import { jsonResponse, requestUrl } from "./fetchStubTestUtils"; +import type { TimelineElement } from "../player/store/playerStore"; import { captureDurationRollback, finishClipTimingFallback, + finishGroupTimingGsapFallback, readFileContent, shiftGsapPositions, } from "./timelineTimingSync"; @@ -136,3 +138,242 @@ describe("fetch URL encoding (user-influenced segments)", () => { ); }); }); + +/** + * Live-preview iframe stub: inline GSAP script elements plus the runtime hooks + * the timing rebind needs. `appendedScripts` records any script a sync path + * executes — the rebind-only path must record NONE — and `scriptEls` lets tests + * assert the original script elements were left untouched in the document. + */ +const LIVE_SCRIPT = + 'window.__timelines = window.__timelines || {}; window.__timelines["root"] = tl;'; +const LIVE_CAPTION_SCRIPT = + 'window.__timelines = window.__timelines || {}; window.__timelines["captions"] = capTl;'; + +function buildLivePreviewIframe(liveScripts: string[] = [LIVE_SCRIPT]) { + const scriptEls = liveScripts.map((text) => { + const el = document.createElement("script"); + el.textContent = text; + return el; + }); + const container = document.createElement("div"); + for (const el of scriptEls) container.appendChild(el); + + const contentWindow = { + gsap: { timeline: vi.fn(), set: vi.fn() }, + __hfForceTimelineRebind: vi.fn() as unknown, + __timelines: { root: { kill: vi.fn() } } as Record, + __player: { getTime: () => 0, seek: vi.fn() }, + __hfStudioManualEditsApply: vi.fn(), + }; + + const appendedScripts: string[] = []; + const realAppendChild = container.appendChild.bind(container); + container.appendChild = (node: T): T => { + const result = realAppendChild(node); + if (node instanceof HTMLScriptElement) appendedScripts.push(node.textContent ?? ""); + return result; + }; + + const contentDocument = { + querySelectorAll: (sel: string) => (sel === "script:not([src])" ? scriptEls : []), + createElement: (tag: string) => document.createElement(tag), + body: container, + head: document.createElement("div"), + }; + + return { + iframe: { contentWindow, contentDocument } as unknown as HTMLIFrameElement, + contentWindow, + container, + scriptEls, + appendedScripts, + }; +} + +describe("nothing-to-rewrite timing edits rebind in place (no script re-execution)", () => { + it("single clip without a domId: rebinds + seeks, executes NO script, no full reload", async () => { + // A selector-addressed clip (e.g. a .sub caption) has no domId, so there is + // no GSAP mutation to run — the timing attributes are already live-patched + // and __timelines is still valid, so the runtime only needs to re-derive + // the clip windows (rebind) and re-seek. Re-running init-style scripts + // (three.js setups etc.) is exactly the unsafe case this must avoid. + const { iframe, contentWindow, container, scriptEls, appendedScripts } = + buildLivePreviewIframe(); + const reloadPreview = vi.fn(); + + await finishClipTimingFallback({ + ...clipFallbackInput({ reloadPreview, recordEdit: vi.fn(async () => {}) }), + iframe, + domId: undefined, + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + expect(contentWindow.__player.seek).toHaveBeenCalledTimes(1); + expect(contentWindow.__hfStudioManualEditsApply).toHaveBeenCalledTimes(1); + // NO script executed, the original script element untouched in place. + expect(appendedScripts).toHaveLength(0); + expect(container.contains(scriptEls[0]!)).toBe(true); + expect(scriptEls[0]!.textContent).toBe(LIVE_SCRIPT); + }); + + it("multi-script document (e.g. three.js + captions): rebind-only, both scripts untouched", async () => { + // Real compositions commonly hold heavy inline scripts (main timeline, + // three.js setup, captions). None of them may run twice — the rebind path + // must not create, remove, or re-execute any of them. + const { iframe, contentWindow, container, scriptEls, appendedScripts } = buildLivePreviewIframe( + [LIVE_SCRIPT, LIVE_CAPTION_SCRIPT], + ); + const rootKill = vi.fn(); + const captionsKill = vi.fn(); + contentWindow.__timelines = { root: { kill: rootKill }, captions: { kill: captionsKill } }; + const reloadPreview = vi.fn(); + + await finishClipTimingFallback({ + ...clipFallbackInput({ reloadPreview, recordEdit: vi.fn(async () => {}) }), + iframe, + domId: undefined, + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(appendedScripts).toHaveLength(0); + expect(container.contains(scriptEls[0]!)).toBe(true); + expect(container.contains(scriptEls[1]!)).toBe(true); + // The still-valid timelines are NOT killed — nothing re-registers them. + expect(rootKill).not.toHaveBeenCalled(); + expect(captionsKill).not.toHaveBeenCalled(); + expect(contentWindow.__timelines.root).toBeDefined(); + expect(contentWindow.__timelines.captions).toBeDefined(); + // One finalization: one seek, one rebind. + expect(contentWindow.__player.seek).toHaveBeenCalledTimes(1); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + }); + + it("comp with ZERO GSAP scripts also rebinds in place (previously full-reloaded)", async () => { + // The rebind hook is installed by the runtime unconditionally — it does not + // depend on GSAP — so a script-less comp gets the flashless path too. + const { iframe, contentWindow, appendedScripts } = buildLivePreviewIframe([]); + const reloadPreview = vi.fn(); + + await finishClipTimingFallback({ + ...clipFallbackInput({ reloadPreview, recordEdit: vi.fn(async () => {}) }), + iframe, + domId: undefined, + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + expect(contentWindow.__player.seek).toHaveBeenCalledTimes(1); + expect(appendedScripts).toHaveLength(0); + }); + + it("full-reloads when the runtime rebind hook is unavailable", async () => { + const { iframe, contentWindow, appendedScripts } = buildLivePreviewIframe(); + contentWindow.__hfForceTimelineRebind = undefined; + const reloadPreview = vi.fn(); + + await finishClipTimingFallback({ + ...clipFallbackInput({ reloadPreview, recordEdit: vi.fn(async () => {}) }), + iframe, + domId: undefined, + }); + + expect(reloadPreview).toHaveBeenCalledTimes(1); + expect(appendedScripts).toHaveLength(0); + }); + + it("still full-reloads when the server MUTATED the file but returned no script", async () => { + // mutated:true with scriptText:null (older server) means the live script is + // now STALE relative to disk — a rebind against it would show wrong + // positions. + stubFetch(["", ""], { mutated: true, scriptText: null }); + const { iframe, contentWindow, appendedScripts } = buildLivePreviewIframe(); + const reloadPreview = vi.fn(); + + await finishClipTimingFallback({ + ...clipFallbackInput({ reloadPreview, recordEdit: vi.fn(async () => {}) }), + iframe, + }); + + expect(reloadPreview).toHaveBeenCalledTimes(1); + expect(contentWindow.__hfForceTimelineRebind).not.toHaveBeenCalled(); + expect(appendedScripts).toHaveLength(0); + }); + + it("mutated WITH a rewritten script keeps the script-swap soft path (not rebind-only)", async () => { + // A genuine rewrite must re-run the REWRITTEN script — the rebind-only + // shortcut is reserved for the no-op case where every script is unchanged. + stubFetch(["", ""], { + mutated: true, + scriptText: 'window.__timelines["root"] = tl2;', + }); + const { iframe, contentWindow, appendedScripts } = buildLivePreviewIframe(); + const reloadPreview = vi.fn(); + + await finishClipTimingFallback({ + ...clipFallbackInput({ reloadPreview, recordEdit: vi.fn(async () => {}) }), + iframe, + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(appendedScripts).toHaveLength(1); + expect(appendedScripts[0]).toContain('__timelines["root"] = tl2;'); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + }); + + it("group batch where every change had nothing to rewrite (gap close over no-domId clips) rebinds in place", async () => { + stubFetch([""], { mutated: false, scriptText: null }); + const { iframe, contentWindow, container, scriptEls, appendedScripts } = + buildLivePreviewIframe(); + const reloadPreview = vi.fn(); + const element = { sourceFile: "index.html" } as TimelineElement; + + await finishGroupTimingGsapFallback({ + projectId: "p1", + iframe, + reloadPreview, + label: "Move timeline clips", + errorLabel: "Failed to shift GSAP positions", + recordEdit: vi.fn(async () => {}) as never, + activeCompPath: "index.html", + changes: [{ element }, { element }], + resolveChangePath: () => "index.html", + // No domId → nothing to rewrite for ANY change (the gap-close blink path). + mutateChange: () => null, + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + expect(contentWindow.__player.seek).toHaveBeenCalledTimes(1); + // No script executed, the live script element untouched. + expect(appendedScripts).toHaveLength(0); + expect(container.contains(scriptEls[0]!)).toBe(true); + }); + + it("group batch touching ANOTHER file still full-reloads even when nothing was rewritten", async () => { + stubFetch([""], { mutated: false, scriptText: null }); + const { iframe, contentWindow, appendedScripts } = buildLivePreviewIframe(); + const reloadPreview = vi.fn(); + + await finishGroupTimingGsapFallback({ + projectId: "p1", + iframe, + reloadPreview, + label: "Move timeline clips", + errorLabel: "Failed to shift GSAP positions", + recordEdit: vi.fn(async () => {}) as never, + activeCompPath: "index.html", + changes: [ + { element: { sourceFile: "index.html" } as TimelineElement }, + { element: { sourceFile: "scenes/intro.html" } as TimelineElement }, + ], + resolveChangePath: (el) => el.sourceFile ?? "index.html", + mutateChange: () => null, + }); + + expect(reloadPreview).toHaveBeenCalledTimes(1); + expect(contentWindow.__hfForceTimelineRebind).not.toHaveBeenCalled(); + expect(appendedScripts).toHaveLength(0); + }); +}); diff --git a/packages/studio/src/hooks/timelineTimingSync.ts b/packages/studio/src/hooks/timelineTimingSync.ts index 4db660b338..e0aeafc532 100644 --- a/packages/studio/src/hooks/timelineTimingSync.ts +++ b/packages/studio/src/hooks/timelineTimingSync.ts @@ -3,7 +3,7 @@ // edit's undo history, and swapping the rewritten script into the live preview // without a full iframe reload when possible. import { type TimelineElement, usePlayerStore } from "../player/store/playerStore"; -import { applySoftReload } from "../utils/gsapSoftReload"; +import { applySoftReload, applySoftReloadFinalization } from "../utils/gsapSoftReload"; import { furthestClipEndFromDocument } from "../player/lib/timelineElementHelpers"; import type { RecordEditInput } from "../utils/studioFileHistory"; import { patchDocumentRootDuration } from "./timelineEditingGsap"; @@ -77,7 +77,9 @@ export function captureDurationRollback(iframe: HTMLIFrameElement | null): () => * `scriptText` is the rewritten root GSAP script — feeding it to `applySoftReload` * swaps the runtime timeline in place (no iframe reload = no all-clips flash). Null * when the endpoint didn't return one (older server, or a multi-script comp the - * soft path can't scope), in which case the caller full-reloads as before. + * soft path can't scope) — the caller then full-reloads when `mutated`, or + * rebinds the runtime timing in place when nothing was rewritten (see + * syncTimingEditPreview). */ export type GsapMutationStatus = { mutated: boolean; scriptText: string | null }; @@ -98,6 +100,26 @@ function readMutationError(value: unknown, fallback: string): string { return fallback; } +/** + * Flashless preview sync for a timing edit that rewrote NO script: the live + * DOM timing attributes were already patched (patchIframeDomTiming), the GSAP + * timelines in `window.__timelines` are still valid, so the preview only needs + * the runtime to re-derive clip visibility windows from the live + * `data-start`/`data-duration` attributes and re-seek to the current time. + * That is exactly applySoftReload's finalization (seek → + * `__hfForceTimelineRebind` → manual-edits reapply) — run WITHOUT touching or + * re-executing any script. Re-running init-style scripts is the unsafe case + * this replaces: a comp loading three.js / heavy inline scripts must not have + * them executed twice. + * + * Works for zero-GSAP compositions too — the rebind hook is installed by the + * runtime unconditionally. Returns false when the iframe/runtime hook is + * unavailable (caller full-reloads). + */ +function rebindPreviewTiming(iframe: HTMLIFrameElement | null, currentTime: number): boolean { + return applySoftReloadFinalization(iframe, currentTime); +} + /** * Sync the live preview after a TIMING-ONLY edit (move / resize), preferring a * soft reload over the full iframe reload that flashes every clip. @@ -115,15 +137,34 @@ function readMutationError(value: unknown, fallback: string): string { * Escalates to the full `reloadPreview()` only on the PERMANENT `cannot-soft-reload` * result (no gsap runtime / rebind hook / scopable key / script element, or the * re-run threw). The TRANSIENT `verify-failed` is NOT escalated — the live re-run - * already applied the shift; a remount would re-flash for nothing. When the server - * returned no `scriptText` (older server, multi-script comp), we also full-reload. + * already applied the shift; a remount would re-flash for nothing. + * + * When there is no rewritten `scriptText`, `mutated` disambiguates two very + * different situations: + * - `mutated: false` — nothing was rewritten because there was NOTHING TO + * REWRITE (the clip has no domId so no id-addressed tweens, the delta was + * zero, or the server confirmed a no-op). Every script is unchanged and the + * timing attributes are already live-patched, so (when `rebindWhenUnmutated` + * allows it) `rebindPreviewTiming` re-seeks + rebinds and the runtime + * re-derives the clip windows from the live DOM — no script re-execution, + * no full-reload blink. This covers comps with zero GSAP scripts too; only + * a missing iframe/runtime hook falls back to the full reload. + * - `mutated: true` with no script — the file on disk WAS rewritten but the + * server returned no script (older server, multi-script comp): the live + * script is now stale, so a rebind against it would show wrong positions → + * full-reload. */ function syncTimingEditPreview( iframe: HTMLIFrameElement | null, - outcome: Pick, + outcome: GsapMutationStatus, currentTime: number, reloadPreview: () => void, + rebindWhenUnmutated: boolean, ): void { + if (!outcome.scriptText && !outcome.mutated && rebindWhenUnmutated) { + if (!rebindPreviewTiming(iframe, currentTime)) reloadPreview(); + return; + } if (!iframe || !outcome.scriptText) { reloadPreview(); return; @@ -140,6 +181,14 @@ async function finishTimelineTimingFallback(input: { reloadPreview: () => void; gsapMutation?: () => Promise; onGsapError: (error: unknown) => void; + /** + * When the mutation produced no rewrite (mutated:false, no scriptText), + * rebind the runtime timing in place (no script re-execution) instead of + * full-reloading (see syncTimingEditPreview). Callers pass false when a full + * reload is the only sync that reflects everything (e.g. a multi-file group + * edit). + */ + rebindWhenUnmutated: boolean; }): Promise { let outcome: GsapMutationStatus = { mutated: false, scriptText: null }; if (input.gsapMutation) { @@ -155,6 +204,7 @@ async function finishTimelineTimingFallback(input: { outcome, usePlayerStore.getState().currentTime, input.reloadPreview, + input.rebindWhenUnmutated, ); } @@ -297,8 +347,11 @@ export type SingleClipGsapEdit = * Post-persist GSAP sync for a SINGLE-clip timing edit (move / resize): runs the * server shift/scale mutation, folds the rewrite into the timing edit's history * entry (see foldGsapMutationIntoHistory), then soft-reloads the preview with - * the rewritten script — full reload when the mutation is skipped, failed, or - * returned no script. + * the rewritten script. When there was nothing to rewrite (no domId — e.g. a + * selector-addressed caption clip — zero delta, or a server no-op) it rebinds + * the runtime timing in place instead of full-reloading; full reload remains + * for genuine rewrites without a returned script and for comps the soft path + * can't handle (see syncTimingEditPreview). */ export function finishClipTimingFallback(input: { iframe: HTMLIFrameElement | null; @@ -347,6 +400,7 @@ export function finishClipTimingFallback(input: { }) : undefined, onGsapError, + rebindWhenUnmutated: true, }); } @@ -408,5 +462,10 @@ export async function finishGroupTimingGsapFallback { h.unmount(); }); + it("rebinds the preview in place (no blink) for a time-move of a no-domId clip", async () => { + // The gap-close blink path: "Close gap" issues pure TIME moves through + // handleTimelineGroupMove. A selector-addressed clip (no domId — e.g. a + // .sub caption) has no id-addressed GSAP tweens, so the mutation step has + // nothing to rewrite and used to fall through to a full reloadPreview(). + // The sync must instead rebind the runtime timing in place — seek + rebind + // re-derive the clip windows from the already-patched DOM — WITHOUT + // re-executing any script (a comp's init-style scripts, e.g. a three.js + // setup, must never run twice) and without the full-reload blink. + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("Expected iframe document"); + doc.body.innerHTML = + '
'; + const liveScript = doc.createElement("script"); + liveScript.textContent = + 'window.__timelines = window.__timelines || {}; window.__timelines["root"] = { kill: function () {} };'; + doc.body.appendChild(liveScript); + const win = iframe.contentWindow as unknown as Record; + win.gsap = { timeline: vi.fn(), set: vi.fn() }; + win.__timelines = { root: { kill: vi.fn() } }; + win.__hfForceTimelineRebind = vi.fn(); + win.__player = { getTime: () => 0, seek: vi.fn() }; + + const cap: TimelineElement = { + ...timelineElement({ id: "cap", track: 0, zIndex: 0, start: 2, duration: 1 }), + domId: undefined, + selector: ".cap", + selectorIndex: 0, + }; + const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); + const reloadPreview = vi.fn(); + const fetchMock = stubProjectFetch( + '
', + ); + const { groupMove, unmount } = renderTimelineEditingHook({ + timelineElements: [cap], + iframe, + onZIndexCommit: vi.fn().mockResolvedValue(undefined), + projectId: "p1", + writeProjectFile, + recordEdit: vi.fn(async () => {}), + reloadPreview, + }); + + await act(async () => { + // Mirror timelineGapCommit: a pure time move keeping the current lane. + await groupMove([{ element: cap, start: 1, track: cap.track }]); + await flushAsyncWork(); + }); + + // The move persisted (live DOM + file)... + expect(doc.querySelector(".cap")?.getAttribute("data-start")).toBe("1"); + expect(writeProjectFile.mock.calls[0]![1]).toContain('data-start="1"'); + // ...no GSAP mutation ran (nothing id-addressed to rewrite)... + expect( + fetchMock.mock.calls.some((call) => requestUrl(call[0]).includes("gsap-mutations")), + ).toBe(false); + // ...and the preview was rebound in place, NOT full-reloaded (blink), + // with the live script element left untouched (no re-execution). + expect(reloadPreview).not.toHaveBeenCalled(); + expect(win.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + expect(doc.body.contains(liveScript)).toBe(true); + expect(doc.querySelectorAll("script")).toHaveLength(1); + + unmount(); + }); + it("keeps the GSAP fallback + reload for a MIXED batch (any start change)", async () => { // One clip changes lane only, the other shifts in time — the batch is not // track-only, so the existing behavior (GSAP shift + full reload when no - // rewritten scriptText comes back) must be preserved. + // rewritten scriptText comes back — this stub iframe has no runtime rebind + // hook, so the in-place rebind can't apply) must be preserved. const source = [ '
', '
', diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index 2d9ff139b8..29bcb29f37 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -237,7 +237,9 @@ export function useTimelineGroupEditing({ // nothing for the preview to recompute: skip the fallback below entirely. // Running it anyway is what made the mirrored z-order lane move blink — // a zero-delta batch yields no scriptText, and finishGroupTimingGsapFallback - // full-reloads the iframe when there is no script to soft-swap. + // used to full-reload the iframe when there was no script to soft-swap + // (it now rebinds the runtime timing in place, but a track-only batch + // needs NO preview sync at all, so the skip stays). const trackOnly = changes.every((change) => change.start === change.element.start); const maxEnd = Math.max(...changes.map((change) => change.start + change.element.duration)); diff --git a/packages/studio/src/utils/gsapSoftReload.test.ts b/packages/studio/src/utils/gsapSoftReload.test.ts index 6dae557369..c2178e5f85 100644 --- a/packages/studio/src/utils/gsapSoftReload.test.ts +++ b/packages/studio/src/utils/gsapSoftReload.test.ts @@ -3,9 +3,8 @@ import { describe, it, expect, vi } from "vitest"; import { applySoftReload, + applySoftReloadFinalization, ensureMotionPathPluginLoaded, - diffSoftReloadableRestore, - applyUndoRestoreToPreview, } from "./gsapSoftReload"; const SCRIPT_TEXT = ` @@ -66,6 +65,7 @@ function buildMockIframe(overrides: Record = {}) { iframe: { contentWindow, contentDocument } as unknown as HTMLIFrameElement, contentWindow, mockTimeline, + container, }; } @@ -288,6 +288,58 @@ describe("applySoftReload", () => { }); }); +// ── Finalization-only path: seek → rebind → manual edits, with NO script +// execution — the flashless sync for timing edits that changed no script. +describe("applySoftReloadFinalization", () => { + it("seeks, rebinds, and reapplies manual edits without touching any script", () => { + const { iframe, contentWindow, container, mockTimeline } = buildMockIframe(); + const scriptsBefore = container.querySelectorAll("script").length; + + expect(applySoftReloadFinalization(iframe, 2.0)).toBe(true); + + expect(contentWindow.__player.seek).toHaveBeenCalledWith(2.0); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + expect(contentWindow.__hfStudioManualEditsApply).toHaveBeenCalledTimes(1); + // No script executed or removed; the live timeline was never killed. + expect(container.querySelectorAll("script").length).toBe(scriptsBefore); + expect(mockTimeline.kill).not.toHaveBeenCalled(); + expect(contentWindow.__timelines.root).toBe(mockTimeline); + }); + + it("runs inside __hfSuppressSceneMutations when the runtime provides it", () => { + const suppress = vi.fn((fn: () => T): T => fn()); + const { iframe, contentWindow } = buildMockIframe({ + __hfSuppressSceneMutations: suppress, + }); + + expect(applySoftReloadFinalization(iframe, 1.5)).toBe(true); + + expect(suppress).toHaveBeenCalledTimes(1); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + }); + + it("does NOT require gsap — a script-less runtime with the rebind hook works", () => { + const { iframe, contentWindow } = buildMockIframe({ gsap: undefined }); + expect(applySoftReloadFinalization(iframe, 0)).toBe(true); + expect(contentWindow.__hfForceTimelineRebind).toHaveBeenCalledTimes(1); + }); + + it("returns false when the iframe or the rebind hook is unavailable", () => { + expect(applySoftReloadFinalization(null, 0)).toBe(false); + const { iframe } = buildMockIframe({ __hfForceTimelineRebind: undefined }); + expect(applySoftReloadFinalization(iframe, 0)).toBe(false); + }); + + it("returns false when the rebind throws (caller full-reloads)", () => { + const { iframe } = buildMockIframe({ + __hfForceTimelineRebind: vi.fn(() => { + throw new Error("runtime mid-teardown"); + }), + }); + expect(applySoftReloadFinalization(iframe, 0)).toBe(false); + }); +}); + function buildBootstrapIframe(overrides: Record = {}) { const head = document.createElement("div"); const appendedScripts: HTMLScriptElement[] = []; @@ -452,116 +504,3 @@ describe("applySoftReload authored-opacity restore", () => { expect(restoreOpacity(el)).toBe(""); }); }); - -// ── Bug 2: undo/redo restore soft-apply ────────────────────────────────────── - -const wrap = (body: string) => `${body}`; - -describe("diffSoftReloadableRestore", () => { - it("reports the changed id for an attribute/inline-style-only diff", () => { - const prev = wrap(`
t
`); - const next = wrap(`
t
`); - expect(diffSoftReloadableRestore(prev, next)).toEqual({ changedElementIds: ["a"] }); - }); - - it("treats a structural change (added element) as NOT soft-reloadable", () => { - const prev = wrap(`
t
`); - const next = wrap(`
t
t
`); - expect(diffSoftReloadableRestore(prev, next)).toBeNull(); - }); - - it("treats an element text/child change as NOT soft-reloadable", () => { - const prev = wrap(`
one
`); - const next = wrap(`
two
`); - expect(diffSoftReloadableRestore(prev, next)).toBeNull(); - }); - - it("allows a GSAP-script-only change (no id'd-attribute diff)", () => { - const prev = wrap( - `
t
`, - ); - const next = wrap( - `
t
`, - ); - expect(diffSoftReloadableRestore(prev, next)).toEqual({ changedElementIds: [] }); - }); -}); - -function buildLiveIframe(bodyHtml: string) { - const doc = document.implementation.createHTMLDocument(""); - doc.body.innerHTML = bodyHtml; - const contentWindow = { - gsap: { timeline: () => {} }, - __hfForceTimelineRebind: () => {}, - __timelines: {} as Record, - __player: { getTime: () => 3, seek: vi.fn() }, - __hfStudioManualEditsApply: vi.fn(), - }; - return { - iframe: { contentWindow, contentDocument: doc } as unknown as HTMLIFrameElement, - contentWindow, - doc, - }; -} - -describe("applyUndoRestoreToPreview", () => { - const ROOT = "index.html"; - - it("soft-applies an attribute/style-only restore: syncs the live element, no full reload", () => { - const { iframe, contentWindow, doc } = buildLiveIframe( - `
t
`, - ); - const reloadPreview = vi.fn(); - const files = { - [ROOT]: { - previous: wrap( - `
t
`, - ), - restored: wrap(`
t
`), - }, - }; - const outcome = applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview); - expect(outcome).toBe("soft"); - expect(reloadPreview).not.toHaveBeenCalled(); - // Live element reverted to the restored inline style. - expect(doc.getElementById("a")!.getAttribute("style")).toBe("translate: 0px 0px"); - // No GSAP script in the restore → the manual-edit reapply runs, playhead held. - expect(contentWindow.__player.seek).toHaveBeenCalledWith(3); - expect(contentWindow.__hfStudioManualEditsApply).toHaveBeenCalled(); - }); - - it("full-reloads a multi-file restore", () => { - const { iframe } = buildLiveIframe(`
t
`); - const reloadPreview = vi.fn(); - const files = { - [ROOT]: { - previous: wrap(`
t
`), - restored: wrap(`
t
`), - }, - "scenes/intro.html": { previous: "a", restored: "b" }, - }; - expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full"); - expect(reloadPreview).toHaveBeenCalledTimes(1); - }); - - it("full-reloads a structural restore (split/delete undo)", () => { - const { iframe } = buildLiveIframe(`
t
t
`); - const reloadPreview = vi.fn(); - const files = { - [ROOT]: { - previous: wrap(`
t
t
`), - restored: wrap(`
t
`), - }, - }; - expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full"); - expect(reloadPreview).toHaveBeenCalledTimes(1); - }); - - it("full-reloads when the restore touches a sub-comp, not the active comp", () => { - const { iframe } = buildLiveIframe(`
t
`); - const reloadPreview = vi.fn(); - const files = { "scenes/intro.html": { previous: "a", restored: "b" } }; - expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full"); - expect(reloadPreview).toHaveBeenCalledTimes(1); - }); -}); diff --git a/packages/studio/src/utils/gsapSoftReload.ts b/packages/studio/src/utils/gsapSoftReload.ts index cf265b891a..71b624276d 100644 --- a/packages/studio/src/utils/gsapSoftReload.ts +++ b/packages/studio/src/utils/gsapSoftReload.ts @@ -90,7 +90,7 @@ function isGsapScript(text: string): boolean { ); } -function findGsapScriptElements(doc: Document): HTMLScriptElement[] { +export function findGsapScriptElements(doc: Document): HTMLScriptElement[] { const results: HTMLScriptElement[] = []; const scripts = doc.querySelectorAll("script:not([src])"); for (const script of scripts) { @@ -102,8 +102,9 @@ function findGsapScriptElements(doc: Document): HTMLScriptElement[] { /** * Extract the GSAP timeline script text from a serialized HTML document, for * feeding into applySoftReload. Returns null when zero or multiple GSAP scripts - * are present (ambiguous — caller should fall back to a full reload), matching - * applySoftReload's own single-script requirement. + * are present (ambiguous — a serialized snapshot can't say WHICH script a + * single rewritten text corresponds to; caller should fall back to a full + * reload), matching applySoftReload's own single-script requirement. */ export function extractGsapScriptText(html: string): string | null { const doc = new DOMParser().parseFromString(html, "text/html"); @@ -183,157 +184,53 @@ export interface SoftReloadOptions { authoredHtml?: string; } -/** One file's restore from the edit-history store: before (live) / after (target) bytes. */ -export interface UndoRestoreFile { - previous: string; - restored: string; -} -function idElementMap(doc: Document): Map { - const map = new Map(); - for (const el of doc.querySelectorAll("[id]")) { - const id = el.getAttribute("id"); - if (id) map.set(id, el); - } - return map; -} - -// Strip id'd elements to bare `id` and blank GSAP scripts, in place: docs that -// differ only in id'd attributes/inline-style/script text normalize equal; any -// residual difference is beyond soft-reload's reach → caller full-reloads. -function normalizeSoftResidual(doc: Document): void { - for (const el of doc.querySelectorAll("[id]")) { - const id = el.getAttribute("id"); - for (const name of [...el.getAttributeNames()]) { - if (name !== "id") el.removeAttribute(name); - } - if (id) el.setAttribute("id", id); - } - for (const script of findGsapScriptElements(doc)) script.textContent = ""; -} - -// Soft-reloadable iff the docs differ SOLELY in id'd-element attributes/inline -// style and/or the GSAP script; returns the changed ids to sync onto the live -// DOM. Structural/text diffs → null → the caller full-reloads. Pure. -export function diffSoftReloadableRestore( - previous: string, - restored: string, -): { changedElementIds: string[] } | null { - let prevDoc: Document; - let nextDoc: Document; - try { - prevDoc = new DOMParser().parseFromString(previous, "text/html"); - nextDoc = new DOMParser().parseFromString(restored, "text/html"); - } catch { - return null; - } - const prevById = idElementMap(prevDoc); - const nextById = idElementMap(nextDoc); - // A different id set means an element was added or removed (e.g. a split, a - // delete) — structural, so soft-reload can't express it. - if (prevById.size !== nextById.size) return null; - const changedElementIds: string[] = []; - for (const [id, nextEl] of nextById) { - const prevEl = prevById.get(id); - if (!prevEl || prevEl.tagName !== nextEl.tagName) return null; - // A change inside the element (text / children) is out of soft scope; only - // its own attributes may differ. (GSAP scripts are handled via re-run.) - if (prevEl.innerHTML !== nextEl.innerHTML) return null; - if (prevEl.outerHTML !== nextEl.outerHTML) changedElementIds.push(id); - } - // Confirm nothing OUTSIDE id'd-element attributes and GSAP scripts changed. - normalizeSoftResidual(prevDoc); - normalizeSoftResidual(nextDoc); - if (prevDoc.documentElement.outerHTML !== nextDoc.documentElement.outerHTML) return null; - return { changedElementIds }; -} - -/** Copy every attribute from `source` onto the live `target`, dropping extras. */ -function syncElementAttributes(target: Element, source: Element): void { - for (const name of [...target.getAttributeNames()]) { - if (!source.hasAttribute(name)) target.removeAttribute(name); - } - for (const name of source.getAttributeNames()) { - target.setAttribute(name, source.getAttribute(name) ?? ""); - } +/** + * The soft reload's finalization step, shared with the rebind-only preview sync + * below: seek → force timeline rebind → reapply studio manual edits. + * + * Seek BEFORE rebind: __hfForceTimelineRebind's own internal force-render + * (see init.ts) renders the freshly-created timeline at whatever the + * runtime's internal scrub position already is, not at whatever we pass + * here afterward — a redundant seek() call after rebind can be a GSAP + * no-op if the timeline already reports being at that time internally. + */ +function finalizeSoftReload(win: IframeWindow, currentTime: number): void { + win.__player?.seek?.(currentTime); + win.__hfForceTimelineRebind?.(); + win.__hfStudioManualEditsApply?.(); } /** - * Soft-apply an undo/redo restore to the live preview WITHOUT a full iframe - * remount (which blanks the frame black and re-flashes the WebGL context). Only - * the active composition — the document living in the root iframe — is eligible; - * a sub-comp or multi-file restore falls back to `reloadPreview`. + * Run ONLY applySoftReload's finalization (seek → __hfForceTimelineRebind → + * manual-edits reapply) against the live iframe — executing NO scripts and + * touching NO script elements. `__hfForceTimelineRebind` makes the runtime + * re-derive every clip's visibility window from the live DOM's `data-start` / + * `data-duration` attributes (init.ts: bindRootTimelineIfAvailable + + * syncTimedElementVisibility), so this is the flashless sync for a timing edit + * whose attributes were already live-patched and whose GSAP scripts are + * unchanged (`window.__timelines` still valid). Works for compositions with + * zero GSAP scripts too — the rebind hook is installed unconditionally by the + * runtime, independent of any animation library. * - * The restore is soft-applied when its only differences are id'd-element - * attributes / inline-style and/or the GSAP script (see diffSoftReloadableRestore): - * 1. Each changed element's attribute surface (inline style, data-start / - * -duration, the studio manual-offset props + flags) is synced onto the live - * element — so a canvas-position revert lands on the live DOM the runtime's - * seek-reapply reads from, not just on disk. - * 2. The restored GSAP script is re-run in place via applySoftReload, which - * re-seeks to `currentTime` (playhead-invariant) and re-folds manual edits. - * With no single script, the manual-edit reapply is invoked directly. - * - * Returns "soft" when applied in place, "full" when it escalated to reloadPreview - * (ineligible restore, missing target, or a permanent soft-reload failure). + * Returns false when the iframe/runtime hook is unavailable or the run threw — + * the caller should escalate to a full reload. */ -export function applyUndoRestoreToPreview( +export function applySoftReloadFinalization( iframe: HTMLIFrameElement | null, - activeCompPath: string | null, - files: Record | undefined, currentTime: number, - reloadPreview: () => void, -): "soft" | "full" { - const paths = files ? Object.keys(files) : []; - // Soft path only covers the single active-comp document in the root iframe. - if (!iframe || !activeCompPath || !files || paths.length !== 1 || paths[0] !== activeCompPath) { - reloadPreview(); - return "full"; - } - const doc = iframe.contentDocument; - const win = iframe.contentWindow as IframeWindow | null; - if (!doc || !win) { - reloadPreview(); - return "full"; - } - const { previous, restored } = files[activeCompPath]!; - const diff = diffSoftReloadableRestore(previous, restored); - if (!diff) { - reloadPreview(); - return "full"; - } - - // Sync each changed element's attributes onto the live DOM from the restored - // markup, so the runtime's seek-reapply (which reads inline offset props off - // the live element) folds the REVERTED values, not the stale current ones. - const restoredById = idElementMap(new DOMParser().parseFromString(restored, "text/html")); - for (const id of diff.changedElementIds) { - const liveEl = doc.getElementById(id); - const restoredEl = restoredById.get(id); - if (liveEl && restoredEl) syncElementAttributes(liveEl, restoredEl); - } - - const script = extractGsapScriptText(restored); - if (script) { - const result = applySoftReload(iframe, script, { - onAsyncFailure: reloadPreview, - currentTimeOverride: currentTime, - }); - if (result === "cannot-soft-reload") { - reloadPreview(); - return "full"; - } - return "soft"; - } - // No single GSAP script to re-run — the change was pure attribute/style. Re-fold - // manual edits and hold the playhead so the synced attributes take visible effect. +): boolean { + const win = iframe?.contentWindow as IframeWindow | null; + if (!win?.__hfForceTimelineRebind) return false; try { - win.__player?.seek?.(currentTime); - win.__hfStudioManualEditsApply?.(); + if (win.__hfSuppressSceneMutations) { + win.__hfSuppressSceneMutations(() => finalizeSoftReload(win, currentTime)); + } else { + finalizeSoftReload(win, currentTime); + } + return true; } catch { - reloadPreview(); - return "full"; + return false; } - return "soft"; } export function applySoftReload( @@ -523,14 +420,7 @@ export function applySoftReload( const s = doc.createElement("script"); s.textContent = `(function(){${scriptText}\n})();`; doc.body.appendChild(s); - // Seek BEFORE rebind: __hfForceTimelineRebind's own internal force-render - // (see init.ts) renders the freshly-created timeline at whatever the - // runtime's internal scrub position already is, not at whatever we pass - // here afterward — a redundant seek() call after rebind can be a GSAP - // no-op if the timeline already reports being at that time internally. - win.__player?.seek?.(currentTime); - win.__hfForceTimelineRebind?.(); - win.__hfStudioManualEditsApply?.(); + finalizeSoftReload(win, currentTime); }; const needsMotionPath = /motionPath\s*[:{]/.test(scriptText); diff --git a/packages/studio/src/utils/gsapUndoRestore.test.ts b/packages/studio/src/utils/gsapUndoRestore.test.ts new file mode 100644 index 0000000000..b4b48878e8 --- /dev/null +++ b/packages/studio/src/utils/gsapUndoRestore.test.ts @@ -0,0 +1,117 @@ +// @vitest-environment happy-dom + +import { describe, it, expect, vi } from "vitest"; +import { applyUndoRestoreToPreview, diffSoftReloadableRestore } from "./gsapUndoRestore"; + +// ── Bug 2: undo/redo restore soft-apply ────────────────────────────────────── + +const wrap = (body: string) => `${body}`; + +describe("diffSoftReloadableRestore", () => { + it("reports the changed id for an attribute/inline-style-only diff", () => { + const prev = wrap(`
t
`); + const next = wrap(`
t
`); + expect(diffSoftReloadableRestore(prev, next)).toEqual({ changedElementIds: ["a"] }); + }); + + it("treats a structural change (added element) as NOT soft-reloadable", () => { + const prev = wrap(`
t
`); + const next = wrap(`
t
t
`); + expect(diffSoftReloadableRestore(prev, next)).toBeNull(); + }); + + it("treats an element text/child change as NOT soft-reloadable", () => { + const prev = wrap(`
one
`); + const next = wrap(`
two
`); + expect(diffSoftReloadableRestore(prev, next)).toBeNull(); + }); + + it("allows a GSAP-script-only change (no id'd-attribute diff)", () => { + const prev = wrap( + `
t
`, + ); + const next = wrap( + `
t
`, + ); + expect(diffSoftReloadableRestore(prev, next)).toEqual({ changedElementIds: [] }); + }); +}); + +function buildLiveIframe(bodyHtml: string) { + const doc = document.implementation.createHTMLDocument(""); + doc.body.innerHTML = bodyHtml; + const contentWindow = { + gsap: { timeline: () => {} }, + __hfForceTimelineRebind: () => {}, + __timelines: {} as Record, + __player: { getTime: () => 3, seek: vi.fn() }, + __hfStudioManualEditsApply: vi.fn(), + }; + return { + iframe: { contentWindow, contentDocument: doc } as unknown as HTMLIFrameElement, + contentWindow, + doc, + }; +} + +describe("applyUndoRestoreToPreview", () => { + const ROOT = "index.html"; + + it("soft-applies an attribute/style-only restore: syncs the live element, no full reload", () => { + const { iframe, contentWindow, doc } = buildLiveIframe( + `
t
`, + ); + const reloadPreview = vi.fn(); + const files = { + [ROOT]: { + previous: wrap( + `
t
`, + ), + restored: wrap(`
t
`), + }, + }; + const outcome = applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview); + expect(outcome).toBe("soft"); + expect(reloadPreview).not.toHaveBeenCalled(); + // Live element reverted to the restored inline style. + expect(doc.getElementById("a")!.getAttribute("style")).toBe("translate: 0px 0px"); + // No GSAP script in the restore → the manual-edit reapply runs, playhead held. + expect(contentWindow.__player.seek).toHaveBeenCalledWith(3); + expect(contentWindow.__hfStudioManualEditsApply).toHaveBeenCalled(); + }); + + it("full-reloads a multi-file restore", () => { + const { iframe } = buildLiveIframe(`
t
`); + const reloadPreview = vi.fn(); + const files = { + [ROOT]: { + previous: wrap(`
t
`), + restored: wrap(`
t
`), + }, + "scenes/intro.html": { previous: "a", restored: "b" }, + }; + expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full"); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); + + it("full-reloads a structural restore (split/delete undo)", () => { + const { iframe } = buildLiveIframe(`
t
t
`); + const reloadPreview = vi.fn(); + const files = { + [ROOT]: { + previous: wrap(`
t
t
`), + restored: wrap(`
t
`), + }, + }; + expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full"); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); + + it("full-reloads when the restore touches a sub-comp, not the active comp", () => { + const { iframe } = buildLiveIframe(`
t
`); + const reloadPreview = vi.fn(); + const files = { "scenes/intro.html": { previous: "a", restored: "b" } }; + expect(applyUndoRestoreToPreview(iframe, ROOT, files, 3, reloadPreview)).toBe("full"); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/studio/src/utils/gsapUndoRestore.ts b/packages/studio/src/utils/gsapUndoRestore.ts new file mode 100644 index 0000000000..02226fdead --- /dev/null +++ b/packages/studio/src/utils/gsapUndoRestore.ts @@ -0,0 +1,165 @@ +// Soft-apply of undo/redo restores to the live preview: diff a restored file +// against the live one, sync attribute-only changes onto the live DOM, and +// re-run the restored GSAP script via applySoftReload — avoiding the full +// iframe remount (black flash + WebGL context loss) whenever the restore is +// expressible in place. +import { applySoftReload, extractGsapScriptText, findGsapScriptElements } from "./gsapSoftReload"; + +type PreviewWindow = Window & { + __player?: { seek?: (t: number) => void }; + __hfStudioManualEditsApply?: () => void; +}; + +/** One file's restore from the edit-history store: before (live) / after (target) bytes. */ +export interface UndoRestoreFile { + previous: string; + restored: string; +} + +function idElementMap(doc: Document): Map { + const map = new Map(); + for (const el of doc.querySelectorAll("[id]")) { + const id = el.getAttribute("id"); + if (id) map.set(id, el); + } + return map; +} + +// Strip id'd elements to bare `id` and blank GSAP scripts, in place: docs that +// differ only in id'd attributes/inline-style/script text normalize equal; any +// residual difference is beyond soft-reload's reach → caller full-reloads. +function normalizeSoftResidual(doc: Document): void { + for (const el of doc.querySelectorAll("[id]")) { + const id = el.getAttribute("id"); + for (const name of [...el.getAttributeNames()]) { + if (name !== "id") el.removeAttribute(name); + } + if (id) el.setAttribute("id", id); + } + for (const script of findGsapScriptElements(doc)) script.textContent = ""; +} + +// Soft-reloadable iff the docs differ SOLELY in id'd-element attributes/inline +// style and/or the GSAP script; returns the changed ids to sync onto the live +// DOM. Structural/text diffs → null → the caller full-reloads. Pure. +export function diffSoftReloadableRestore( + previous: string, + restored: string, +): { changedElementIds: string[] } | null { + let prevDoc: Document; + let nextDoc: Document; + try { + prevDoc = new DOMParser().parseFromString(previous, "text/html"); + nextDoc = new DOMParser().parseFromString(restored, "text/html"); + } catch { + return null; + } + const prevById = idElementMap(prevDoc); + const nextById = idElementMap(nextDoc); + // A different id set means an element was added or removed (e.g. a split, a + // delete) — structural, so soft-reload can't express it. + if (prevById.size !== nextById.size) return null; + const changedElementIds: string[] = []; + for (const [id, nextEl] of nextById) { + const prevEl = prevById.get(id); + if (!prevEl || prevEl.tagName !== nextEl.tagName) return null; + // A change inside the element (text / children) is out of soft scope; only + // its own attributes may differ. (GSAP scripts are handled via re-run.) + if (prevEl.innerHTML !== nextEl.innerHTML) return null; + if (prevEl.outerHTML !== nextEl.outerHTML) changedElementIds.push(id); + } + // Confirm nothing OUTSIDE id'd-element attributes and GSAP scripts changed. + normalizeSoftResidual(prevDoc); + normalizeSoftResidual(nextDoc); + if (prevDoc.documentElement.outerHTML !== nextDoc.documentElement.outerHTML) return null; + return { changedElementIds }; +} + +/** Copy every attribute from `source` onto the live `target`, dropping extras. */ +function syncElementAttributes(target: Element, source: Element): void { + for (const name of [...target.getAttributeNames()]) { + if (!source.hasAttribute(name)) target.removeAttribute(name); + } + for (const name of source.getAttributeNames()) { + target.setAttribute(name, source.getAttribute(name) ?? ""); + } +} + +/** + * Soft-apply an undo/redo restore to the live preview WITHOUT a full iframe + * remount (which blanks the frame black and re-flashes the WebGL context). Only + * the active composition — the document living in the root iframe — is eligible; + * a sub-comp or multi-file restore falls back to `reloadPreview`. + * + * The restore is soft-applied when its only differences are id'd-element + * attributes / inline-style and/or the GSAP script (see diffSoftReloadableRestore): + * 1. Each changed element's attribute surface (inline style, data-start / + * -duration, the studio manual-offset props + flags) is synced onto the live + * element — so a canvas-position revert lands on the live DOM the runtime's + * seek-reapply reads from, not just on disk. + * 2. The restored GSAP script is re-run in place via applySoftReload, which + * re-seeks to `currentTime` (playhead-invariant) and re-folds manual edits. + * With no single script, the manual-edit reapply is invoked directly. + * + * Returns "soft" when applied in place, "full" when it escalated to reloadPreview + * (ineligible restore, missing target, or a permanent soft-reload failure). + */ +export function applyUndoRestoreToPreview( + iframe: HTMLIFrameElement | null, + activeCompPath: string | null, + files: Record | undefined, + currentTime: number, + reloadPreview: () => void, +): "soft" | "full" { + const paths = files ? Object.keys(files) : []; + // Soft path only covers the single active-comp document in the root iframe. + if (!iframe || !activeCompPath || !files || paths.length !== 1 || paths[0] !== activeCompPath) { + reloadPreview(); + return "full"; + } + const doc = iframe.contentDocument; + const win = iframe.contentWindow as PreviewWindow | null; + if (!doc || !win) { + reloadPreview(); + return "full"; + } + const { previous, restored } = files[activeCompPath]!; + const diff = diffSoftReloadableRestore(previous, restored); + if (!diff) { + reloadPreview(); + return "full"; + } + + // Sync each changed element's attributes onto the live DOM from the restored + // markup, so the runtime's seek-reapply (which reads inline offset props off + // the live element) folds the REVERTED values, not the stale current ones. + const restoredById = idElementMap(new DOMParser().parseFromString(restored, "text/html")); + for (const id of diff.changedElementIds) { + const liveEl = doc.getElementById(id); + const restoredEl = restoredById.get(id); + if (liveEl && restoredEl) syncElementAttributes(liveEl, restoredEl); + } + + const script = extractGsapScriptText(restored); + if (script) { + const result = applySoftReload(iframe, script, { + onAsyncFailure: reloadPreview, + currentTimeOverride: currentTime, + }); + if (result === "cannot-soft-reload") { + reloadPreview(); + return "full"; + } + return "soft"; + } + // No single GSAP script to re-run — the change was pure attribute/style. Re-fold + // manual edits and hold the playhead so the synced attributes take visible effect. + try { + win.__player?.seek?.(currentTime); + win.__hfStudioManualEditsApply?.(); + } catch { + reloadPreview(); + return "full"; + } + return "soft"; +} From 8443867b9f9d058c3bd05f29a6d4ca9fa23e4f68 Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Mon, 13 Jul 2026 22:25:16 -0700 Subject: [PATCH 05/19] fix(studio): bound forward/backward mirror to a one-element step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User-specified semantic: Bring Forward / Send Backward move the clip past EXACTLY ONE element. The mirror's lane target is now bounded by the next temporally-overlapping element beyond the crossed neighbor: a free lane strictly between the two is taken (closest to the neighbor), and when they are back-to-back a new track is inserted immediately beyond the crossed element — never past the second one. Previously the resolver took the closest free lane anywhere beyond the neighbor, which could carry the track past a second element while the z action only stepped past one — a track/paint contradiction our own zOverride badge would flag. Front/back keep whole-set semantics (past everything; back stays above the audio zone). End-to-end test pins the 3-stacked case through commitZMirrorLaneMove to the persisted renumbered tracks. --- .../useCanvasZOrderTimelineMirror.test.tsx | 17 ++- .../components/timelineClipDragCommit.test.ts | 34 ++++++ .../player/components/timelineZMirror.test.ts | 112 +++++++++++++++++- .../src/player/components/timelineZMirror.ts | 106 +++++++++++++---- 4 files changed, 234 insertions(+), 35 deletions(-) diff --git a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx index 94abb6424a..b12d932abe 100644 --- a/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx +++ b/packages/studio/src/components/nle/useCanvasZOrderTimelineMirror.test.tsx @@ -205,12 +205,7 @@ describe("useCanvasZOrderTimelineMirror", () => { // Two consecutive gestures on the same element: each mints its own coalesce // key (gesture sequence), so even an unbounded per-gesture window must never // merge distinct user actions into one undo step. - setStoreElements([ - storeEl("t", 0, 0, 10), - storeEl("b", 1, 0, 10), - storeEl("c", 2, 0, 10), - storeEl("a", 3, 20, 5), // free lane 3 over t's span - ]); + setStoreElements([storeEl("t", 0, 0, 10), storeEl("b", 1, 0, 10)]); const history = makeHistory(); const api = mountMirrorHarness(history); @@ -225,12 +220,14 @@ describe("useCanvasZOrderTimelineMirror", () => { ); const keys: string[] = []; - // Gesture 1: t (lane 0) sent backward past b → lands on the free lane 3. - // Gesture 2: t brought forward past c → back onto the now-free lane 0. - // Both gestures mirror (lane move persists), so each records a z+move pair. + // Gesture 1: t (top) sent backward past b → back-to-back, no free lane in + // the bounded interval → insert immediately below b (t and b swap lanes). + // Gesture 2: t brought forward past b → insert immediately above b (swap + // back). Both gestures mirror (lane move persists), so each records a + // z+move pair. const gestures = [ { action: "send-backward" as const, crossedId: "b" }, - { action: "bring-forward" as const, crossedId: "c" }, + { action: "bring-forward" as const, crossedId: "b" }, ]; for (const { action, crossedId } of gestures) { const coalesceKey = zReorderCoalesceKey(entries, action); diff --git a/packages/studio/src/player/components/timelineClipDragCommit.test.ts b/packages/studio/src/player/components/timelineClipDragCommit.test.ts index fd8fa3922d..244c37b26b 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.test.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.test.ts @@ -13,6 +13,7 @@ import { pushEditHistoryEntry, } from "../../utils/editHistory"; import { normalizeToZones } from "./timelineZones"; +import { resolveZMirrorLaneMove } from "./timelineZMirror"; import type { StackingPatch } from "./timelineStackingSync"; function el( @@ -1125,6 +1126,39 @@ describe("commitZMirrorLaneMove", () => { }); }); + it("END-TO-END one-element step: resolver insertRow renumbers the clip strictly between the two neighbors", async () => { + // 3 stacked back-to-back clips + a free lane beyond the far one. Send t + // (top) backward past b: the resolver must bound at c and produce the + // insert row IMMEDIATELY below b, and commitZMirrorLaneMove's renumber must + // land t strictly between b and c — never on the farther free lane 3. + const t = el("t", 0, 0, 10); + const b = el("b", 1, 0, 10); + const c = el("c", 2, 0, 10); + const far = el("far", 3, 20, 5); // free over t's span, beyond c + const elements = [t, b, c, far]; + const move = resolveZMirrorLaneMove({ + action: "send-backward", + element: t, + elements, + crossedKey: "b", + }); + expect(move).toEqual({ kind: "insert", insertRow: 2 }); + const { onMoveElements, deps } = mirrorDeps(elements, [0, 1, 2, 3]); + const moved = await commitZMirrorLaneMove(t, move!, deps, "z-reorder:send-backward:t"); + expect(moved).toBe(true); + const map = editMap(onMoveElements.mock.calls[0][0]); + // The renumber compacts t's vacated top lane, so the whole set shifts up by + // one while t lands on the b/c boundary — strictly between the two. + expect(map).toEqual({ + b: { start: 0, track: 0 }, + t: { start: 0, track: 1 }, + c: { start: 0, track: 2 }, + far: { start: 20, track: 3 }, + }); + expect(map.b.track).toBeLessThan(map.t.track); + expect(map.t.track).toBeLessThan(map.c.track); + }); + it("resolves false for a refused insert (locked clip would need renumbering)", async () => { // b is locked and sits at/below the insert row, so the whole-set renumber // is refused — no persist call. diff --git a/packages/studio/src/player/components/timelineZMirror.test.ts b/packages/studio/src/player/components/timelineZMirror.test.ts index 3540890321..ca9053277d 100644 --- a/packages/studio/src/player/components/timelineZMirror.test.ts +++ b/packages/studio/src/player/components/timelineZMirror.test.ts @@ -114,10 +114,27 @@ describe("resolveZMirrorLaneMove — bring-forward / send-backward", () => { expect(resolve("send-backward", t, [t, b])).toBeNull(); }); - it("skips an occupied lane and takes the next free one in direction", () => { - // Above neighbor b (lane 2): lane 1 occupied over the span, lane 0 free. - const a = el("a", 0, 30, 5); - const x = el("x", 1, 5, 10); + it("BOUNDED: never steps past the next overlapping element to a farther free lane", () => { + // Above neighbor b (lane 2): lane 1 holds x — the NEXT temporally-overlapping + // same-file element in the direction — and lane 0 is free. A single forward + // step crosses ONE element, so the free lane 0 beyond x is out of reach: + // insert immediately above b instead (row of lane 2 in the ascending order). + const a = el("a", 0, 30, 5); // lane 0 free over t's span — but beyond the bound + const x = el("x", 1, 5, 10); // overlaps t → the exclusive bound + const b = el("b", 2, 0, 10); + const t = el("t", 3, 0, 10); + expect(resolve("bring-forward", t, [a, x, b, t], "b")).toEqual({ + kind: "insert", + insertRow: 2, + }); + }); + + it("OPEN SPACE: with no second overlapping element, skips an occupied lane to the next free one", () => { + // Lane 1's occupant is a FOREIGN-file clip: it occupies the lane (freeness is + // file-agnostic) but is not in the same stacking context, so it does not + // bound the step — the search continues to free lane 0, as before. + const a = el("a", 0, 30, 5); // lane 0 free over t's span + const x = el("x", 1, 5, 10, { sourceFile: "sub.html" }); const b = el("b", 2, 0, 10); const t = el("t", 3, 0, 10); expect(resolve("bring-forward", t, [a, x, b, t], "b")).toEqual({ @@ -136,6 +153,93 @@ describe("resolveZMirrorLaneMove — bring-forward / send-backward", () => { }); }); +describe("resolveZMirrorLaneMove — one-element step bound (forward/backward)", () => { + // Three stacked back-to-back clips (lanes 0/1/2, all overlapping) plus a free + // lane BEYOND the far element — the lane the old resolver would overshoot to. + const threeStackedWithFarFree = () => { + const a = el("a", 0, 0, 10); + const b = el("b", 1, 0, 10); + const c = el("c", 2, 0, 10); + const d = el("d", 3, 20, 5); // lane 3 free over the span — beyond c + return { a, b, c, d, elements: [a, b, c, d] }; + }; + + it("send-backward from the top inserts between elements 1 and 2 — not past element 2", () => { + const { a, elements } = threeStackedWithFarFree(); + // Reference = b (lane 1); next overlap below = c (lane 2) bounds the search; + // no free lane strictly between → insert at the b/c boundary (row 2), NOT + // the farther free lane 3. + expect(resolve("send-backward", a, elements, "b")).toEqual({ kind: "insert", insertRow: 2 }); + }); + + it("bring-forward from the bottom inserts between elements 1 and 2 (symmetric)", () => { + const d = el("d", 0, 20, 5); // lane 0 free over the span — beyond a + const a = el("a", 1, 0, 10); + const b = el("b", 2, 0, 10); + const t = el("t", 3, 0, 10); + // Reference = b (lane 2); next overlap above = a (lane 1) bounds the search; + // no free lane strictly between → insert at the a/b boundary (row 2), NOT + // the farther free lane 0. + expect(resolve("bring-forward", t, [d, a, b, t], "b")).toEqual({ + kind: "insert", + insertRow: 2, + }); + }); + + it("takes a free lane strictly between the reference and the next overlap", () => { + const a = el("a", 0, 0, 10); // second element — the exclusive bound + const gap = el("gap", 1, 20, 5); // lane 1 free over the span, inside the interval + const b = el("b", 2, 0, 10); // crossed reference + const t = el("t", 3, 0, 10); + expect(resolve("bring-forward", t, [a, gap, b, t], "b")).toEqual({ + kind: "move", + displayTrack: 1, + persistTrack: 1, + }); + }); + + it("of several free lanes in the interval, takes the one closest to the reference", () => { + const a = el("a", 0, 0, 10); // bound + const g1 = el("g1", 1, 20, 5); // free, farther from reference + const g2 = el("g2", 2, 20, 5); // free, closest to reference + const b = el("b", 3, 0, 10); // crossed reference + const t = el("t", 4, 0, 10); + expect(resolve("bring-forward", t, [a, g1, g2, b, t], "b")).toEqual({ + kind: "move", + displayTrack: 2, + persistTrack: 2, + }); + }); + + it("no second overlapping element beyond the reference → the zone edge bounds (as today)", () => { + const { t, elements } = stackBelow(); + // Only c overlaps below the reference b... remove c's overlap: reference is + // then the ONLY overlap below; the search runs to the zone edge and takes + // the free lane beyond the neighbor. + const spread = elements.map((e) => (e.id === "c" ? { ...e, start: 20 } : e)); + expect(resolve("send-backward", t, spread, "b")).toEqual({ + kind: "move", + displayTrack: 2, + persistTrack: 2, + }); + }); + + it("bring-to-front is NOT bounded: still moves past the whole overlap set", () => { + const { t, elements } = (() => { + const free = el("free", 0, 20, 5); // free lane beyond the topmost overlap + const a = el("a", 1, 0, 10); + const b = el("b", 2, 0, 10); + const t = el("t", 3, 0, 10); + return { t, elements: [free, a, b, t] }; + })(); + expect(resolve("bring-to-front", t, elements)).toEqual({ + kind: "move", + displayTrack: 0, + persistTrack: 0, + }); + }); +}); + describe("resolveZMirrorLaneMove — bring-to-front / send-to-back", () => { it("bring-to-front moves above the topmost temporally-overlapping clip", () => { const a = el("a", 0, 20, 5); // lane 0 free over t's span diff --git a/packages/studio/src/player/components/timelineZMirror.ts b/packages/studio/src/player/components/timelineZMirror.ts index 33e6d849c6..9208f3265c 100644 --- a/packages/studio/src/player/components/timelineZMirror.ts +++ b/packages/studio/src/player/components/timelineZMirror.ts @@ -21,10 +21,21 @@ import { authoredTrackForLane, sameSourceFile } from "./timelineClipDragCommit"; * ── Locked rules (agreed design — do not re-litigate here) ─────────────────── * - The mirror computes a lane move to ACCOMPANY a z action on a timeline clip; * it never replaces the z patch. - * - Move the clip to the closest track in the action's direction that is FREE - * over the clip's whole [start, start + duration) span; if no free track - * exists in that direction, CREATE one adjacent to the reference neighbor - * (commitTrackInsert semantics). + * - ONE-ELEMENT STEP (bring-forward / send-backward): the z action stepped past + * exactly ONE element — the reference neighbor — so the lane move must too. + * Target = the free lane (whole-span, file-agnostic occupancy, same zone) + * closest to the reference, searched STRICTLY BETWEEN the reference's lane + * and the next temporally-overlapping same-file visual element's lane in the + * direction (exclusive bound). No free lane in that open interval (the common + * back-to-back case) → CREATE one at the boundary immediately beyond the + * reference neighbor (commitTrackInsert semantics) — never scan past the + * second element to a farther free lane, which would overshoot the paint + * order and trip our own zOverride badge. With no second overlapping element + * beyond the reference, the bound is the zone edge: closest free lane beyond + * the neighbor, else insert immediately beyond it. + * - bring-to-front / send-to-back move past the WHOLE overlap set: closest free + * lane beyond the extreme overlap in the direction, else insert adjacent to + * the extreme. * - Direction: bring-forward/front = toward LOWER display lanes (up = above); * send-backward/back = toward HIGHER lanes, but only within the visual zone — * the audio zone is untouched and never crossed (a bottom-of-zone insert lands @@ -118,29 +129,82 @@ export function resolveZMirrorLaneMove(input: ZMirrorInput): ZMirrorLaneMove { const order = displayTrackOrder(elements); const visualLanes = displayTrackOrder(elements.filter((el) => classifyZone(el) === "visual")); - - // Closest free lane strictly beyond the reference, lane-by-lane in direction, - // whole-span freeness, same zone (visual lanes only — never into audio). const refIdx = visualLanes.indexOf(referenceLane); if (refIdx === -1) return null; // reference is not a visual lane — no mirror + + const boundLane = stepBoundLane(action, overlapSet, referenceLane, up); + const lane = closestFreeLane({ + elements, + visualLanes, + refIdx, + up, + boundLane, + start, + end, + selfKey, + }); + if (lane != null) { + // The closest free lane is the clip's OWN lane (possible only when z and + // track had diverged): the clip already sits where the action puts it. + if (lane === element.track) return null; + return { + kind: "move", + displayTrack: lane, + persistTrack: authoredTrackForLane(lane, elements, element), + }; + } + + // No free lane before the bound (or the zone edge) → create one adjacent to + // the reference: the boundary between its lane and the next in direction. + return { kind: "insert", insertRow: order.indexOf(referenceLane) + (up ? 0 : 1) }; +} + +/** + * ONE-ELEMENT-STEP bound (bring-forward / send-backward only): the lane of the + * NEXT temporally-overlapping same-file visual element strictly beyond the + * reference in the direction — the free-lane search may not reach it + * (exclusive). Front/back have no bound (they step past the whole overlap + * set), and neither does a step with no second overlapping element beyond the + * reference (the zone edge bounds instead). + */ +function stepBoundLane( + action: ZMirrorAction, + overlapSet: TimelineElement[], + referenceLane: number, + up: boolean, +): number | null { + if (action !== "bring-forward" && action !== "send-backward") return null; + const beyond = overlapSet + .map((el) => el.track) + .filter((lane) => (up ? lane < referenceLane : lane > referenceLane)); + if (beyond.length === 0) return null; + return (up ? Math.max : Math.min)(...beyond); +} + +/** + * Closest free lane strictly beyond the reference, lane-by-lane in direction, + * whole-span freeness, same zone (visual lanes only — never into audio), + * stopping at the exclusive bound when one applies. Null → no free lane in + * the open interval. + */ +function closestFreeLane(args: { + elements: TimelineElement[]; + visualLanes: number[]; + refIdx: number; + up: boolean; + boundLane: number | null; + start: number; + end: number; + selfKey: string; +}): number | null { + const { elements, visualLanes, refIdx, up, boundLane, start, end, selfKey } = args; const step = up ? -1 : 1; for (let i = refIdx + step; i >= 0 && i < visualLanes.length; i += step) { const lane = visualLanes[i]; - if (isLaneFree(elements, lane, start, end, selfKey)) { - // The closest free lane is the clip's OWN lane (possible only when z and - // track had diverged): the clip already sits where the action puts it. - if (lane === element.track) return null; - return { - kind: "move", - displayTrack: lane, - persistTrack: authoredTrackForLane(lane, elements, element), - }; - } + if (boundLane != null && (up ? lane <= boundLane : lane >= boundLane)) break; + if (isLaneFree(elements, lane, start, end, selfKey)) return lane; } - - // No free lane before the zone edge → create one adjacent to the reference: - // the boundary between the reference lane and the next lane in direction. - return { kind: "insert", insertRow: order.indexOf(referenceLane) + (up ? 0 : 1) }; + return null; } /** From 7fb6e683a3c1db30e1bc738d7b776b8db4e653e7 Mon Sep 17 00:00:00 2001 From: ukimsanov Date: Tue, 14 Jul 2026 00:26:14 -0700 Subject: [PATCH 06/19] feat(studio): permanent gap-menu rows with hover and click-select gap highlights - TrackGapContextMenu always renders both rows; an inapplicable action dims with a tooltip ("No gap here" / lock reason / "No gaps on this track") instead of vanishing into a one-item menu. Width badge only when a gap exists under the pointer. - Hovering an ACTIONABLE row highlights the strip(s) it would close in the timeline: the single gap for Close gap, every current gap (leading included) for Close all gaps. New resolveAllGapIntervals in timelineGaps.ts reports present-state intervals (epsilon-tolerant, overlap-safe), distinct from resolveAllTrackGaps' post-compaction starts. - Click-selecting a single clip paints a quieter tint over its lane's gaps (suppressed for marquee multi-selection and during drags; the gap-menu hover wins on its own lane). Derivation lives in useTimelineGapHighlights with the pure buildTimelineGapStrips exported and unit-tested. - Strips render in TimelineCanvas with the drop-placeholder geometry (row top + clip inset), dashed accent for hover, faint tint for selection. - Timeline.tsx stayed under the 600-line cap by extracting the scroll-viewport plumbing (ResizeObserver width + shortcut-hint sync) into useTimelineScrollViewport, behavior unchanged. --- .../studio/src/player/components/Timeline.tsx | 96 ++++++---------- .../src/player/components/TimelineCanvas.tsx | 29 +++++ .../player/components/TimelineOverlays.tsx | 3 + .../player/components/TrackGapContextMenu.tsx | 68 +++++++----- .../player/components/timelineGaps.test.ts | 27 +++++ .../src/player/components/timelineGaps.ts | 27 +++++ .../useTimelineGapHighlights.test.ts | 104 ++++++++++++++++++ .../components/useTimelineGapHighlights.ts | 92 ++++++++++++++++ .../components/useTimelineScrollViewport.ts | 73 ++++++++++++ .../src/player/components/useTrackGapMenu.ts | 60 ++++++++-- 10 files changed, 484 insertions(+), 95 deletions(-) create mode 100644 packages/studio/src/player/components/useTimelineGapHighlights.test.ts create mode 100644 packages/studio/src/player/components/useTimelineGapHighlights.ts create mode 100644 packages/studio/src/player/components/useTimelineScrollViewport.ts diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index 1d30609a36..01fdfd51d4 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -21,16 +21,13 @@ import { useTimelineEditPinning } from "./useTimelineEditPinning"; import { useTimelineStackingSync } from "./useTimelineStackingSync"; import { useTimelineGeometry } from "./useTimelineGeometry"; import { useTimelineTrackDerivations } from "./useTimelineTrackDerivations"; -import { - GUTTER, - generateTicks, - getTimelineCanvasHeight, - shouldShowTimelineShortcutHint, -} from "./timelineLayout"; +import { GUTTER, generateTicks, getTimelineCanvasHeight } from "./timelineLayout"; +import { useTimelineScrollViewport } from "./useTimelineScrollViewport"; import { STUDIO_PREVIEW_FPS } from "../lib/time"; import { useResolvedTimelineEditCallbacks } from "./useResolvedTimelineEditCallbacks"; import type { TimelineProps } from "./TimelineTypes"; import { useTrackGapMenu } from "./useTrackGapMenu"; +import { useTimelineGapHighlights } from "./useTimelineGapHighlights"; // Re-export pure utilities so existing imports from "./Timeline" still resolve. export { @@ -129,31 +126,12 @@ export const Timeline = memo(function Timeline({ }); const [showPopover, setShowPopover] = useState(false); - const [showShortcutHint, setShowShortcutHint] = useState(true); const [kfContextMenu, setKfContextMenu] = useState(null); const [clipContextMenu, setClipContextMenu] = useState<{ x: number; y: number; element: TimelineElement; } | null>(null); - const [viewportWidth, setViewportWidth] = useState(0); - const roRef = useRef(null); - const shortcutHintRafRef = useRef(0); - - const syncShortcutHintVisibility = useCallback(() => { - const scroll = scrollRef.current; - setShowShortcutHint( - scroll ? shouldShowTimelineShortcutHint(scroll.scrollHeight, scroll.clientHeight) : true, - ); - }, []); - - const scheduleShortcutHintVisibilitySync = useCallback(() => { - if (shortcutHintRafRef.current) cancelAnimationFrame(shortcutHintRafRef.current); - shortcutHintRafRef.current = requestAnimationFrame(() => { - shortcutHintRafRef.current = 0; - syncShortcutHintVisibility(); - }); - }, [syncShortcutHintVisibility]); const setContainerRef = useCallback((el: HTMLDivElement | null) => { containerRef.current = el; @@ -161,31 +139,6 @@ export const Timeline = memo(function Timeline({ // Last horizontal scroll offset, restored across the post-edit iframe reload (pinned zoom). const lastScrollLeftRef = useRef(0); - const setScrollRef = useCallback( - (el: HTMLDivElement | null) => { - if (roRef.current) { - roRef.current.disconnect(); - roRef.current = null; - } - scrollRef.current = el; - if (!el) return; - - const syncScrollViewport = () => { - setViewportWidth(el.clientWidth); - scheduleShortcutHintVisibilitySync(); - }; - - syncScrollViewport(); - roRef.current = new ResizeObserver(syncScrollViewport); - roRef.current.observe(el); - }, - [scheduleShortcutHintVisibilitySync], - ); - - useMountEffect(() => () => { - roRef.current?.disconnect(); - if (shortcutHintRafRef.current) cancelAnimationFrame(shortcutHintRafRef.current); - }); const effectiveDuration = useMemo(() => { const safeDur = Number.isFinite(duration) ? duration : 0; @@ -233,14 +186,21 @@ export const Timeline = memo(function Timeline({ expandedElementsRef, }); - const { gapMenuModel, openGapMenu, dismissGapMenu, closeTrackGap, closeAllTrackGaps } = - useTrackGapMenu({ - tracks, - expandedElementsRef, - trackOrderRef, - onMoveElement: pinnedOnMoveElement, - onMoveElements: pinnedOnMoveElements, - }); + const { + gapMenuModel, + gapHighlight, + setHoveredGapAction, + openGapMenu, + dismissGapMenu, + closeTrackGap, + closeAllTrackGaps, + } = useTrackGapMenu({ + tracks, + expandedElementsRef, + trackOrderRef, + onMoveElement: pinnedOnMoveElement, + onMoveElements: pinnedOnMoveElements, + }); const { draggedClip, @@ -282,7 +242,21 @@ export const Timeline = memo(function Timeline({ return [...trackOrder, draggedClip.previewTrack].sort((a, b) => a - b); }, [draggedClip, trackOrder]); + const laneGapStrips = useTimelineGapHighlights({ + gapHighlight, + tracks, + selectedElementId, + selectedElementIds, + expandedElements, + dragActive: draggedClip?.started === true || resizingClip != null, + }); + const totalH = getTimelineCanvasHeight(displayTrackOrder.length); + const { viewportWidth, showShortcutHint, setScrollRef } = useTimelineScrollViewport(scrollRef, [ + timelineReady, + expandedElements.length, + totalH, + ]); const keyframeCache = usePlayerStore((s) => s.keyframeCache); const selectedKeyframes = usePlayerStore((s) => s.selectedKeyframes); const toggleSelectedKeyframe = usePlayerStore((s) => s.toggleSelectedKeyframe); @@ -391,10 +365,6 @@ export const Timeline = memo(function Timeline({ ); const majorTickInterval = major.length >= 2 ? major[1] - major[0] : effectiveDuration; - useEffect(() => { - syncShortcutHintVisibility(); - }, [syncShortcutHintVisibility, timelineReady, expandedElements.length, totalH]); - const getPreviewElement = useCallback( (element: TimelineElement): TimelineElement => { if ( @@ -477,6 +447,7 @@ export const Timeline = memo(function Timeline({ majorTickInterval={majorTickInterval} rangeSelection={rangeSelection} marqueeRect={marqueeRect} + laneGapStrips={laneGapStrips} theme={theme} displayTrackOrder={displayTrackOrder} trackOrder={trackOrder} @@ -593,6 +564,7 @@ export const Timeline = memo(function Timeline({ onDismissGapContextMenu={dismissGapMenu} onCloseTrackGap={closeTrackGap} onCloseAllTrackGaps={closeAllTrackGaps} + onHoverGapAction={setHoveredGapAction} />
); diff --git a/packages/studio/src/player/components/TimelineCanvas.tsx b/packages/studio/src/player/components/TimelineCanvas.tsx index f706088da9..883a8d6d0c 100644 --- a/packages/studio/src/player/components/TimelineCanvas.tsx +++ b/packages/studio/src/player/components/TimelineCanvas.tsx @@ -23,6 +23,7 @@ import { TimelineClip } from "./TimelineClip"; import { TimelineLanes, type TimelineLaneBaseProps } from "./TimelineLanes"; import { renderClipChildren } from "./timelineClipChildren"; import { useTimelineRevealClip } from "./useTimelineRevealClip"; +import type { TimelineLaneGapStrips } from "./useTimelineGapHighlights"; interface TimelineCanvasProps extends TimelineLaneBaseProps { major: number[]; @@ -37,6 +38,8 @@ interface TimelineCanvasProps extends TimelineLaneBaseProps { /** Playhead is being actively scrubbed — fills the grab-handle head. */ isScrubbing: boolean; playheadRef: React.RefObject; + /** Gap strips: loud on gap-menu-row hover, quiet on the selected clip's lane. */ + laneGapStrips: TimelineLaneGapStrips[]; } export const TimelineCanvas = memo(function TimelineCanvas(props: TimelineCanvasProps) { @@ -131,6 +134,32 @@ export const TimelineCanvas = memo(function TimelineCanvas(props: TimelineCanvas new bottom track comfortably (see TRACKS_BOTTOM_PAD / getTimelineCanvasHeight). */}