From f7475ec7c45eef4c711d96daf469a850eea5f084 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 20:19:51 -0400 Subject: [PATCH 01/16] feat(studio): a pure Premiere style placement rule with overwrite and insert --- .../components/timelinePlacement.test.ts | 73 +++++++++++++ .../player/components/timelinePlacement.ts | 102 ++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 packages/studio/src/player/components/timelinePlacement.test.ts create mode 100644 packages/studio/src/player/components/timelinePlacement.ts diff --git a/packages/studio/src/player/components/timelinePlacement.test.ts b/packages/studio/src/player/components/timelinePlacement.test.ts new file mode 100644 index 0000000000..84ac880777 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacement.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { placeClip, type PlacementClip, type PlacementMode } from "./timelinePlacement"; + +const clips: PlacementClip[] = [ + { key: "a", start: 0, duration: 4 }, + { key: "b", start: 4, duration: 4 }, +]; +const place = ( + start: number, + duration: number, + mode: PlacementMode = "overwrite", + on: PlacementClip[] = clips, +) => placeClip({ clips: on, track: 1, start, duration, mode }); + +describe("placeClip overwrite", () => { + it("leaves an abutting neighbour alone", () => { + expect(place(8, 2).cuts).toEqual([]); + }); + + it("removes a clip the drop fully covers", () => { + expect(place(3.5, 5).cuts).toEqual([ + { kind: "trim-tail", key: "a", duration: 3.5 }, + { kind: "remove", key: "b" }, + ]); + }); + + it("trims the tail of a clip the drop starts inside", () => { + expect(place(6, 4).cuts).toEqual([{ kind: "trim-tail", key: "b", duration: 2 }]); + }); + + it("trims the head of a clip the drop ends inside and reports the source shift", () => { + expect(place(2, 3).cuts).toEqual([ + { kind: "trim-tail", key: "a", duration: 2 }, + { kind: "trim-head", key: "b", start: 5, duration: 3, sourceShift: 1 }, + ]); + }); + + it("splits a clip the drop lands in the middle of", () => { + expect(place(1, 2, "overwrite", [clips[0]]).cuts).toEqual([ + { kind: "split", key: "a", headDuration: 1, tail: { start: 3, duration: 1, sourceShift: 3 } }, + ]); + }); + + it("never changes the track and shifts nothing", () => { + const r = place(2, 3); + expect(r.track).toBe(1); + expect(r.shifts).toEqual([]); + }); + + it("clamps a negative start to zero", () => { + expect(place(-1, 2).start).toBe(0); + }); +}); + +describe("placeClip insert", () => { + it("pushes every clip at or after the drop point by the clip length", () => { + expect(place(4, 2, "insert").shifts).toEqual([{ key: "b", start: 6 }]); + }); + + it("splits a clip straddling the drop point and pushes its tail past the new clip", () => { + const r = place(6, 2, "insert"); + expect(r.cuts).toEqual([ + { kind: "split", key: "b", headDuration: 2, tail: { start: 8, duration: 2, sourceShift: 2 } }, + ]); + expect(r.shifts).toEqual([]); + }); + + it("leaves clips wholly before the drop point alone", () => { + const r = place(8, 2, "insert"); + expect(r.cuts).toEqual([]); + expect(r.shifts).toEqual([]); + }); +}); diff --git a/packages/studio/src/player/components/timelinePlacement.ts b/packages/studio/src/player/components/timelinePlacement.ts new file mode 100644 index 0000000000..524f102cf9 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacement.ts @@ -0,0 +1,102 @@ +export interface PlacementClip { + key: string; + start: number; + duration: number; +} + +export type PlacementMode = "overwrite" | "insert"; + +/** What placing a clip does to a clip already on the target track. Times are final, in seconds. */ +export type PlacementCut = + | { kind: "remove"; key: string } + | { kind: "trim-head"; key: string; start: number; duration: number; sourceShift: number } + | { kind: "trim-tail"; key: string; duration: number } + | { + kind: "split"; + key: string; + headDuration: number; + tail: { start: number; duration: number; sourceShift: number }; + }; + +export interface PlacementShift { + key: string; + start: number; +} + +export interface PlacementResult { + track: number; + start: number; + cuts: PlacementCut[]; + shifts: PlacementShift[]; +} + +export interface PlaceClipInput { + /** Clips already on the target track, the dragged clip excluded. */ + clips: readonly PlacementClip[]; + track: number; + /** Already snapped; this function never snaps. */ + start: number; + duration: number; + mode: PlacementMode; +} + +const overlaps = (a0: number, a1: number, b0: number, b1: number) => a0 < b1 && b0 < a1; + +/** + * Premiere's drop rules: an overwrite cuts away the range the clip covers, + * an insert splits a straddled clip at the drop point and pushes what follows. + * Nothing moves to another track and nothing is left hidden. + */ +export function placeClip({ + clips, + track, + start, + duration, + mode, +}: PlaceClipInput): PlacementResult { + const from = Math.max(0, start); + const to = from + duration; + const cuts: PlacementCut[] = []; + const shifts: PlacementShift[] = []; + + for (const clip of clips) { + const end = clip.start + clip.duration; + if (mode === "insert") { + if (clip.start >= from) { + shifts.push({ key: clip.key, start: clip.start + duration }); + } else if (end > from) { + cuts.push({ + kind: "split", + key: clip.key, + headDuration: from - clip.start, + tail: { start: to, duration: end - from, sourceShift: from - clip.start }, + }); + } + continue; + } + if (!overlaps(from, to, clip.start, end)) continue; + const headKept = clip.start < from; + const tailKept = end > to; + if (headKept && tailKept) { + cuts.push({ + kind: "split", + key: clip.key, + headDuration: from - clip.start, + tail: { start: to, duration: end - to, sourceShift: to - clip.start }, + }); + } else if (headKept) { + cuts.push({ kind: "trim-tail", key: clip.key, duration: from - clip.start }); + } else if (tailKept) { + cuts.push({ + kind: "trim-head", + key: clip.key, + start: to, + duration: end - to, + sourceShift: to - clip.start, + }); + } else { + cuts.push({ kind: "remove", key: clip.key }); + } + } + return { track, start: from, cuts, shifts }; +} From 118aeea3f334209ae1990a9de63b86b4769bbad5 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:13:44 -0400 Subject: [PATCH 02/16] feat(studio): dragging a clip over another overwrites it, alt inserts, one undo --- packages/studio/src/App.tsx | 1 + .../studio/src/components/EditorShell.tsx | 2 + .../src/components/nle/TimelinePane.tsx | 8 +- .../nle/useTimelineEditCallbacks.ts | 6 + .../src/contexts/TimelineEditContext.tsx | 1 + packages/studio/src/hooks/useRazorSplit.ts | 25 +- .../studio/src/hooks/useTimelineDeleteOps.ts | 54 +- .../studio/src/hooks/useTimelineEditing.ts | 42 +- .../player/components/timelineCallbacks.ts | 5 +- .../components/timelineClipDragCommit.ts | 24 + .../timelineClipDragGestureLifecycle.ts | 13 +- .../components/timelineClipDragPreview.ts | 7 +- .../components/timelineCollision.test.ts | 43 ++ .../player/components/timelineCollision.ts | 6 + .../player/components/timelineGroupEditing.ts | 2 +- .../timelinePlacementCommit.test.ts | 510 ++++++++++++++++++ .../components/timelinePlacementCommit.ts | 236 ++++++++ .../player/components/useTimelineClipDrag.ts | 5 + .../studio/src/utils/razorSplitTransaction.ts | 11 +- 19 files changed, 956 insertions(+), 45 deletions(-) create mode 100644 packages/studio/src/player/components/timelinePlacementCommit.test.ts create mode 100644 packages/studio/src/player/components/timelinePlacementCommit.ts diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index cd696541c1..5152f9ac08 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -525,6 +525,7 @@ export function StudioApp() { handleTimelineElementSplit={timelineEditing.handleTimelineElementSplit} handleRazorSplit={timelineEditing.handleRazorSplit} handleRazorSplitAll={timelineEditing.handleRazorSplitAll} + placementOps={timelineEditing.placementOps} onCopyClip={handleCopy} onPasteClip={handlePaste} onDuplicateClip={handleDuplicate} diff --git a/packages/studio/src/components/EditorShell.tsx b/packages/studio/src/components/EditorShell.tsx index f7b2b9295b..69eab0ac8c 100644 --- a/packages/studio/src/components/EditorShell.tsx +++ b/packages/studio/src/components/EditorShell.tsx @@ -94,6 +94,7 @@ export function EditorShell({ handleTimelineElementSplit, handleRazorSplit, handleRazorSplitAll, + placementOps, onCopyClip, onPasteClip, onDuplicateClip, @@ -151,6 +152,7 @@ export function EditorShell({ handleTimelineElementSplit, handleRazorSplit, handleRazorSplitAll, + placementOps, }); return ( diff --git a/packages/studio/src/components/nle/TimelinePane.tsx b/packages/studio/src/components/nle/TimelinePane.tsx index ed1dbf4e8d..d03af819c1 100644 --- a/packages/studio/src/components/nle/TimelinePane.tsx +++ b/packages/studio/src/components/nle/TimelinePane.tsx @@ -40,6 +40,8 @@ export function forwardRebasedTimelineMoveElements( ); } +type TimelineResizeCommitOptions = { coalesceKey?: string; coalesceMs?: number }; + type TimelineResizeChange = { element: TimelineElement; start: number; @@ -49,10 +51,10 @@ type TimelineResizeChange = { export function forwardRebasedTimelineResizeElements( changes: TimelineResizeChange[], - options: { coalesceKey?: string } | undefined, + options: TimelineResizeCommitOptions | undefined, onResizeElements: ( changes: TimelineResizeChange[], - options?: { coalesceKey?: string }, + options?: TimelineResizeCommitOptions, ) => Promise | void, ) { return onResizeElements( @@ -222,7 +224,7 @@ export function TimelinePane({ duration: number; playbackStart?: number; }>, - options?: { coalesceKey?: string }, + options?: TimelineResizeCommitOptions, ) => { if (!onResizeElements) return; if (changes.some(({ element }) => element.expandedParentStart !== undefined)) { diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts index 788b56282b..7f68419901 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts @@ -20,6 +20,7 @@ import { splitTimelineElementKey, } from "../../player/lib/timelineElementHelpers"; import type { TimelineKeyframeTarget } from "../../player/components/timelineKeyframeIdentity"; +import type { PlacementOps } from "../../player/components/timelinePlacementCommit"; export interface TimelineEditCallbackDeps { handleTimelineElementMove: ( @@ -46,6 +47,8 @@ export interface TimelineEditCallbackDeps { handleTimelineElementSplit: (element: TimelineElement, splitTime: number) => Promise | void; handleRazorSplit: (element: TimelineElement, splitTime: number) => Promise | void; handleRazorSplitAll: (splitTime: number) => Promise | void; + /** Split and remove writes for a clip drop that overwrites a neighbour. */ + placementOps?: PlacementOps; /** C1's ungrouped-track FX pointer — same auto-grouping write B6's carve uses. */ handleGroupClips?: ( clipIds: readonly string[], @@ -124,6 +127,7 @@ export function useTimelineEditCallbacks({ handleTimelineElementSplit, handleRazorSplit, handleRazorSplitAll, + placementOps, handleGroupClips, setElementFxAttribute, }: TimelineEditCallbackDeps): TimelineEditCallbacks { @@ -207,6 +211,7 @@ export function useTimelineEditCallbacks({ onMoveElements: handleTimelineElementsMove, onResizeElement: handleTimelineElementResize, onResizeElements: handleTimelineGroupResize, + onPlacementOps: placementOps, onToggleTrackHidden: handleToggleTrackHidden, onSetAudioGroupAttributeLive: setAudioGroupAttribute.setLive, onSetAudioGroupAttributeQuiet: setAudioGroupAttribute.setQuiet, @@ -405,6 +410,7 @@ export function useTimelineEditCallbacks({ handleTimelineElementsMove, handleTimelineElementResize, handleTimelineGroupResize, + placementOps, handleToggleTrackHidden, setAudioGroupAttribute, handleGroupClips, diff --git a/packages/studio/src/contexts/TimelineEditContext.tsx b/packages/studio/src/contexts/TimelineEditContext.tsx index 4bdedcfbbc..d22bfc0d88 100644 --- a/packages/studio/src/contexts/TimelineEditContext.tsx +++ b/packages/studio/src/contexts/TimelineEditContext.tsx @@ -37,6 +37,7 @@ export function TimelineEditProvider({ value.onMoveElement, value.onMoveElements, value.onResizeElement, + value.onPlacementOps, value.onToggleTrackHidden, value.onSetAudioGroupAttributeLive, value.onSetAudioGroupAttributeQuiet, diff --git a/packages/studio/src/hooks/useRazorSplit.ts b/packages/studio/src/hooks/useRazorSplit.ts index 91bc44b2cf..9bd1fe902f 100644 --- a/packages/studio/src/hooks/useRazorSplit.ts +++ b/packages/studio/src/hooks/useRazorSplit.ts @@ -6,6 +6,8 @@ import { trackStudioRazorSplit } from "../telemetry/events"; import { canSplitElementAt, selectSplittableElements } from "../utils/timelineElementSplit"; import { buildAtomicCutIntents, runAtomicCutTransaction } from "../utils/razorSplitTransaction"; import type { RecordEditInput } from "./timelineEditingHelpers"; +import type { PlacementFold } from "../player/components/timelinePlacementCommit"; +import { getStudioSaveErrorMessage } from "../utils/studioSaveDiagnostics"; interface UseRazorSplitOptions { projectId: string | null; @@ -50,7 +52,12 @@ export function useRazorSplit({ }, [forceReloadSdkSession, reloadPreview]); const runCut = useCallback( - async (elements: readonly TimelineElement[], splitTime: number, mode: "single" | "all") => { + async ( + elements: readonly TimelineElement[], + splitTime: number, + mode: "single" | "all", + fold?: PlacementFold, + ) => { const pid = projectIdRef.current; if (!pid || elements.length === 0) return; const intents = buildAtomicCutIntents(elements, splitTime, activeCompPath); @@ -64,6 +71,7 @@ export function useRazorSplit({ projectId: pid, intents, label, + ...fold, writeProjectFile, recordEdit, observeProjectFileVersion, @@ -114,6 +122,19 @@ export function useRazorSplit({ [isRecordingRef, runCut, showToast], ); + /** One cut inside a clip drop: recorded under the drop's fold key, true once it landed. */ + const handlePlacementSplit = useCallback( + async (element: TimelineElement, splitTime: number, fold: PlacementFold) => { + try { + return (await runCut([element], splitTime, "single", fold)) !== undefined; + } catch (error) { + showToast(getStudioSaveErrorMessage(error), "error"); + return false; + } + }, + [runCut, showToast], + ); + const handleRazorSplitAll = useCallback( async (splitTime: number) => { if (isRecordingRef?.current) { @@ -135,5 +156,5 @@ export function useRazorSplit({ [isRecordingRef, runCut, showToast], ); - return { handleRazorSplit, handleRazorSplitAll }; + return { handleRazorSplit, handleRazorSplitAll, handlePlacementSplit }; } diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.ts b/packages/studio/src/hooks/useTimelineDeleteOps.ts index 5c0a297dd7..a1d2178bab 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.ts +++ b/packages/studio/src/hooks/useTimelineDeleteOps.ts @@ -16,6 +16,7 @@ import { resolveMainTrackDeleteRippleShifts, resolveShiftedElements, } from "../player/components/timelineGapCommit"; +import type { PlacementFold } from "../player/components/timelinePlacementCommit"; import type { TimelineGroupCommitOptions, TimelineGroupMoveChange, @@ -78,17 +79,17 @@ export function useTimelineDeleteOps({ // named in words instead. const rippleNoticeShownRef = useRef(false); // fallow-ignore-next-line complexity - const handleTimelineElementsDelete = useCallback( + const deleteTimelineElements = useCallback( // fallow-ignore-next-line complexity - async (selection: TimelineElement[]) => { + async (selection: TimelineElement[], overwrite?: PlacementFold): Promise => { if (isRecordingRef?.current) { showToast("Cannot edit timeline while recording", "error"); - return; + return false; } const pid = projectIdRef.current; if (!pid) throw new Error("No active project"); const [element] = selection; - if (!element) return; + if (!element) return false; const label = selection.length === 1 ? getTimelineElementLabel(element) : `${selection.length} clips`; @@ -151,7 +152,8 @@ export function useTimelineDeleteOps({ // Shared with the ripple move below so a folded ripple is one undo // step with the delete, not two (editHistory.ts coalesces by key + // window across separate recordEdit calls, not by label). - const coalesceKey = `main-track-ripple-delete:${deleteGestureSeq++}`; + const coalesceKey = + overwrite?.coalesceKey ?? `main-track-ripple-delete:${deleteGestureSeq++}`; const deleteHistoryLabel = "Delete timeline clip"; try { await saveProjectFilesWithHistory({ @@ -159,6 +161,7 @@ export function useTimelineDeleteOps({ label: deleteHistoryLabel, kind: "timeline", coalesceKey, + coalesceMs: overwrite?.coalesceMs, files: { [targetPath]: patchedContent }, readFile: async () => originalContent, // remove-element already wrote the removal, so disk holds THAT — not the @@ -179,11 +182,14 @@ export function useTimelineDeleteOps({ // first (resolveMainTrackDeleteRippleShifts — off, no main-track clip // deleted, already gapless, or a locked survivor all resolve to null), // then the one write, folded into the delete's undo entry above. - const rippleShifts = resolveMainTrackDeleteRippleShifts( - survivors, - sameFile, - usePlayerStore.getState().rippleEditEnabled, - ); + // An overwrite never ripples: the dropped clip fills the span it removed. + const rippleShifts = overwrite + ? null + : resolveMainTrackDeleteRippleShifts( + survivors, + sameFile, + usePlayerStore.getState().rippleEditEnabled, + ); let rippleApplied: TimelineGroupMoveChange[] | null = null; let rippleFailed = false; if (rippleShifts) { @@ -212,13 +218,15 @@ export function useTimelineDeleteOps({ } usePlayerStore.getState().setElements(applyRippleShifts(survivors, rippleApplied)); - usePlayerStore.getState().setSelectedElementId(null); - usePlayerStore.getState().setSelectedElementIds(new Set()); + if (!overwrite) { + usePlayerStore.getState().setSelectedElementId(null); + usePlayerStore.getState().setSelectedElementIds(new Set()); + } forceReloadSdkSession?.(); reloadPreview(); // A failed ripple already showed its own toast above; the user did one // thing (delete), so they get one message, not this generic follow-up too. - if (!rippleFailed) { + if (!rippleFailed && !overwrite) { showToast( `Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`, "info", @@ -232,9 +240,11 @@ export function useTimelineDeleteOps({ "info", ); } + return true; } catch (error) { const message = error instanceof Error ? error.message : "Failed to delete timeline clip"; showToast(message); + return false; } }, [ @@ -252,13 +262,25 @@ export function useTimelineDeleteOps({ ], ); + const handleTimelineElementsDelete = useCallback( + async (selection: TimelineElement[]) => { + await deleteTimelineElements(selection); + }, + [deleteTimelineElements], + ); + /** Single-clip delete — the context menu and clip chrome path. */ const handleTimelineElementDelete = useCallback( async (element: TimelineElement) => { - await handleTimelineElementsDelete([element]); + await deleteTimelineElements([element]); }, - [handleTimelineElementsDelete], + [deleteTimelineElements], ); - return { handleTimelineElementsDelete, handleTimelineElementDelete }; + return { + handleTimelineElementsDelete, + handleTimelineElementDelete, + /** The delete a clip drop uses to remove an overwritten clip: no ripple, folded into its undo step. */ + deleteTimelineElements, + }; } diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index ab6b37df18..cba4f45eb6 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -1,7 +1,8 @@ // fallow-ignore-file complexity -import { useCallback, useRef } from "react"; +import { useCallback, useMemo, useRef } from "react"; import type { TimelineElement } from "../player"; import { useRazorSplit } from "./useRazorSplit"; +import type { PlacementOps } from "../player/components/timelinePlacementCommit"; import { useTimelineAssetDropOps } from "./useTimelineAssetDropOps"; import { applyTimelineStackingReorder, @@ -402,19 +403,20 @@ export function useTimelineEditing({ isRecordingRef, }); - const { handleTimelineElementsDelete, handleTimelineElementDelete } = useTimelineDeleteOps({ - projectIdRef, - activeCompPath, - timelineElements, - showToast, - writeProjectFile, - recordEdit, - reloadPreview, - isRecordingRef, - forceReloadSdkSession, - previewIframeRef, - handleTimelineGroupMove: groupEditing.handleTimelineGroupMove, - }); + const { handleTimelineElementsDelete, handleTimelineElementDelete, deleteTimelineElements } = + useTimelineDeleteOps({ + projectIdRef, + activeCompPath, + timelineElements, + showToast, + writeProjectFile, + recordEdit, + reloadPreview, + isRecordingRef, + forceReloadSdkSession, + previewIframeRef, + handleTimelineGroupMove: groupEditing.handleTimelineGroupMove, + }); const { handleTimelineAssetDrop, handleTimelineFileDrop, handleTimelineCompositionDrop } = useTimelineAssetDropOps({ @@ -433,7 +435,7 @@ export function useTimelineEditing({ const handleBlockedTimelineEdit = useBlockedTimelineEditToast(showToast); - const { handleRazorSplit, handleRazorSplitAll } = useRazorSplit({ + const { handleRazorSplit, handleRazorSplitAll, handlePlacementSplit } = useRazorSplit({ projectId, activeCompPath, showToast, @@ -445,7 +447,17 @@ export function useTimelineEditing({ forceReloadSdkSession, }); + const placementOps = useMemo( + () => ({ + split: handlePlacementSplit, + remove: deleteTimelineElements, + toast: (message) => showToast(message, "error"), + }), + [handlePlacementSplit, deleteTimelineElements, showToast], + ); + return { + placementOps, handleTimelineElementMove, handleTimelineElementResize, handleToggleTrackHidden, diff --git a/packages/studio/src/player/components/timelineCallbacks.ts b/packages/studio/src/player/components/timelineCallbacks.ts index ad206205e6..a4e29ff3a1 100644 --- a/packages/studio/src/player/components/timelineCallbacks.ts +++ b/packages/studio/src/player/components/timelineCallbacks.ts @@ -5,6 +5,7 @@ import type { TimelineMoveOperation } from "../../hooks/timelineMoveAdapter"; import type { BlockedTimelineEditIntent } from "./timelineEditing"; import type { PropertyGroupName } from "@hyperframes/core/gsap-parser"; import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity"; +import type { PlacementOps } from "./timelinePlacementCommit"; export interface TimelinePropertyGroupKeyframeToggle { animationId: string; @@ -54,6 +55,8 @@ export interface TimelineEditCallbacks { operation?: TimelineMoveOperation, coalesceMs?: number, ) => Promise | void; + /** Split and remove writes a clip drop needs when it cuts a neighbour (one shared undo step). */ + onPlacementOps?: PlacementOps; onResizeElement?: ( element: TimelineElement, updates: Pick, @@ -65,7 +68,7 @@ export interface TimelineEditCallbacks { duration: number; playbackStart?: number; }>, - options?: { coalesceKey?: string }, + options?: { coalesceKey?: string; coalesceMs?: number }, ) => Promise | void; /** * `displayNumber` is the row the CLICKED control announced. It travels with diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index e8bcc409f3..9321df432b 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -18,6 +18,8 @@ import { import { runLaneZGesture } from "../../components/nle/zLaneGesture"; import { refreshAfterDurableLaneMove } from "./timelineLaneMoveRefresh"; import { authoredTrackForLane } from "./timelineAuthoredTrack"; +import { commitPlacementDrop, type PlacementOps } from "./timelinePlacementCommit"; +import type { TimelineEditCallbacks } from "./timelineCallbacks"; type StartTrack = Pick; export interface TimelineMoveEdit { @@ -76,6 +78,12 @@ export interface DragCommitDeps { onStackingPatches?: (patches: StackingPatch[], coalesceKey?: string) => Promise | void; /** Converge the preview manifest after the complete lane + z transaction. */ refreshAfterLaneMove?: () => void; + /** Trims for a drop that cuts a neighbour. */ + onResizeElements?: TimelineEditCallbacks["onResizeElements"]; + /** Split and remove writes for a drop that cuts a neighbour. */ + placementOps?: PlacementOps; + /** Alt or Cmd held at pointer-up: push what follows instead of overwriting. */ + insertMode?: boolean; } const keyOf = (e: TimelineElement) => e.key ?? e.id; @@ -240,6 +248,22 @@ export function commitDraggedClipMove(rawDrag: DraggedClipState, deps: DragCommi const isVertical = isInsert || aimTrack !== drag.element.track; const multi = resolveMultiSelection(drag, deps); + if (!isInsert && !multi) { + const mode = deps.insertMode ? "insert" : "overwrite"; + const move = (edits: TimelineMoveEdit[], fold: { coalesceKey: string; coalesceMs: number }) => + refreshAfterDurableLaneMove( + persistMoveEdits( + edits, + deps, + fold.coalesceKey, + edits.some((e) => e.updates.track !== e.element.track) ? "lane-reorder" : "timing", + fold.coalesceMs, + ), + deps, + ); + if (commitPlacementDrop(drag, deps, mode, move)) return; + } + // ── Pure time-move (dragged clip keeps its lane, no insert) ───────────────── if (!isInsert && !laneChanged) { const delta = drag.previewStart - drag.element.start; diff --git a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.ts b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.ts index 674b1f360a..2fd737ed1b 100644 --- a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.ts +++ b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.ts @@ -74,6 +74,7 @@ interface TimelineClipDragGestureLifecycleInput { ((patches: StackingPatch[]) => Promise | void) | undefined >; refreshAfterLaneMoveRef: RefObject<(() => void) | undefined>; + onPlacementOpsRef: RefObject; } export function mountTimelineClipDragGestureLifecycle({ @@ -109,6 +110,7 @@ export function mountTimelineClipDragGestureLifecycle({ readZIndexRef, onStackingPatchesRef, refreshAfterLaneMoveRef, + onPlacementOpsRef, }: TimelineClipDragGestureLifecycleInput): () => void { const clearSuppressedClick = () => { requestAnimationFrame(() => { @@ -277,7 +279,7 @@ export function mountTimelineClipDragGestureLifecycle({ if (blocked.started) clearSuppressedClick(); }; - const commitDragPointerUp = (drag: DraggedClipState) => { + const commitDragPointerUp = (drag: DraggedClipState, insertMode: boolean) => { if (!drag.started) return; suppressClickRef.current = true; clearSuppressedClick(); @@ -291,6 +293,9 @@ export function mountTimelineClipDragGestureLifecycle({ readZIndex: readZIndexRef.current, onStackingPatches: onStackingPatchesRef.current, refreshAfterLaneMove: refreshAfterLaneMoveRef.current, + onResizeElements: onResizeElementsRef.current, + placementOps: onPlacementOpsRef.current, + insertMode, }); }; @@ -333,12 +338,12 @@ export function mountTimelineClipDragGestureLifecycle({ return claimed; }; - const commitClaimedGesture = (gesture: TimelineGestureCommit) => { + const commitClaimedGesture = (gesture: TimelineGestureCommit, insertMode: boolean) => { try { if (gesture.kind === "resize" && gesture.resize) { commitResizePointerUp(gesture.resize, gesture.groupResize); } else if (gesture.kind === "drag" && gesture.drag) { - commitDragPointerUp(gesture.drag); + commitDragPointerUp(gesture.drag, insertMode); } } finally { const lifecycle = lifecycleRef.current; @@ -352,7 +357,7 @@ export function mountTimelineClipDragGestureLifecycle({ const claimed = claimActiveGesture(event); if (claimed === "ignored") return; if (claimed) { - commitClaimedGesture(claimed); + commitClaimedGesture(claimed, event.altKey || event.metaKey); return; } const blocked = blockedClipRef.current; diff --git a/packages/studio/src/player/components/timelineClipDragPreview.ts b/packages/studio/src/player/components/timelineClipDragPreview.ts index a53f7e8764..51362a745b 100644 --- a/packages/studio/src/player/components/timelineClipDragPreview.ts +++ b/packages/studio/src/player/components/timelineClipDragPreview.ts @@ -104,7 +104,9 @@ function resolveDropPlacement( desiredTrack: number, ctx: DragPreviewContext, ): { track: number; insertRow: number | null } { - const { scroll, trackOrder, rowHeights, elements } = ctx; + const { scroll, trackOrder, rowHeights, elements, selectedKeys } = ctx; + const dragKey = drag.element.key ?? drag.element.id; + const isGroupDrag = selectedKeys.size > 1 && selectedKeys.has(dragKey); // rowFloat = the pointer's position in track-heights from the top lane; a // near-boundary hover requests a deliberate new-track insert. Uses the // shared row→y inverse so the top breathing pad is subtracted consistently. @@ -134,9 +136,10 @@ function resolveDropPlacement( deliberateInsertRow: rawInsertRow, start: previewStart, duration: drag.element.duration, - dragKey: drag.element.key ?? drag.element.id, + dragKey, isAudio: isAudioTimelineElement(drag.element), preferInsertAbove, + relocateOnOverlap: isGroupDrag, }); } diff --git a/packages/studio/src/player/components/timelineCollision.test.ts b/packages/studio/src/player/components/timelineCollision.test.ts index 328c875708..4ae48d5e99 100644 --- a/packages/studio/src/player/components/timelineCollision.test.ts +++ b/packages/studio/src/player/components/timelineCollision.test.ts @@ -285,6 +285,7 @@ describe("resolveZoneDropPlacement (the whole drop decision, no same-track overl duration: 2, dragKey: "x", isAudio: false, + relocateOnOverlap: true, }; it("lands on the aimed track when it is free at that time", () => { @@ -516,3 +517,45 @@ describe("resolveMainTrackDropStart (magnetic first clip on an empty main track) expect(resolveMainTrackDropStart([], 1, 0, true, 7)).toBe(7); }); }); + +describe("resolveZoneDropPlacement overwrite (a single clip does not relocate)", () => { + const base = { + order: [0, 1, 2, 3], + audioTracks: new Set([3]), + deliberateInsertRow: null as number | null, + start: 2, + duration: 2, + dragKey: "x", + isAudio: false, + }; + + it("stays on the aimed lane over an overlapping clip", () => { + expect( + resolveZoneDropPlacement({ ...base, elements: [el("a", 1, 0, 5)], desiredTrack: 1 }), + ).toEqual({ track: 1, insertRow: null }); + }); + + it("stays on the aimed lane when every lane is occupied", () => { + const full = [el("a", 0, 0, 9), el("b", 1, 0, 9), el("c", 2, 0, 9)]; + expect(resolveZoneDropPlacement({ ...base, elements: full, desiredTrack: 1 })).toEqual({ + track: 1, + insertRow: null, + }); + }); + + it("still honours a deliberate new-track insert", () => { + expect( + resolveZoneDropPlacement({ + ...base, + elements: [el("a", 1, 0, 5)], + desiredTrack: 1, + deliberateInsertRow: 1, + }), + ).toEqual({ track: 1, insertRow: 1 }); + }); + + it("still creates a track for an aim that is not a real lane", () => { + const result = resolveZoneDropPlacement({ ...base, elements: [], desiredTrack: -1 }); + expect(result.insertRow).not.toBeNull(); + }); +}); diff --git a/packages/studio/src/player/components/timelineCollision.ts b/packages/studio/src/player/components/timelineCollision.ts index ab24667610..23a6e4b99c 100644 --- a/packages/studio/src/player/components/timelineCollision.ts +++ b/packages/studio/src/player/components/timelineCollision.ts @@ -84,6 +84,7 @@ function outOfRangeZoneInsertRow( return desired < Math.min(...zoneTracks) ? zoneTop : zoneBottom; } +// fallow-ignore-next-line complexity export function resolveZoneDropPlacement(input: { order: number[]; audioTracks: ReadonlySet; @@ -95,6 +96,8 @@ export function resolveZoneDropPlacement(input: { dragKey: string; isAudio: boolean; preferInsertAbove?: boolean; + /** Group drags keep the old bump-to-a-free-lane rule; a single clip overwrites in place. */ + relocateOnOverlap?: boolean; }): { track: number; insertRow: number | null } { const { order, audioTracks, elements, desiredTrack, deliberateInsertRow } = input; const { start, duration, dragKey, isAudio, preferInsertAbove } = input; @@ -108,6 +111,9 @@ export function resolveZoneDropPlacement(input: { } const desired = clampTrackToZone(desiredTrack, order, audioRow, isAudio); + if (!input.relocateOnOverlap && order.includes(desired)) { + return { track: desired, insertRow: null }; + } const zoneTracks = order.filter((t) => audioTracks.has(t) === isAudio); const placement = resolvePlacement({ elements, diff --git a/packages/studio/src/player/components/timelineGroupEditing.ts b/packages/studio/src/player/components/timelineGroupEditing.ts index e5ec36d7ca..18e8f073be 100644 --- a/packages/studio/src/player/components/timelineGroupEditing.ts +++ b/packages/studio/src/player/components/timelineGroupEditing.ts @@ -181,7 +181,7 @@ function elementKey(element: TimelineElement): string { return element.key ?? element.id; } -function hasSourcePlaybackOffset(element: TimelineElement): boolean { +export function hasSourcePlaybackOffset(element: TimelineElement): boolean { const tag = element.tag.toLowerCase(); return element.kind === "composition" || tag === "audio" || tag === "video"; } diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts new file mode 100644 index 0000000000..2b1f4e6823 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -0,0 +1,510 @@ +import { describe, expect, it, vi } from "vitest"; +import type { TimelineElement } from "../store/playerStore"; +import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; +import type { DraggedClipState } from "./timelineClipDragTypes"; +import type { PlacementResult } from "./timelinePlacement"; +import { + buildPlacementSteps, + commitPlacementDrop, + placementRefusal, + runPlacementSteps, + type PlacementFold, + type PlacementOps, + type PlacementResizeChange, +} from "./timelinePlacementCommit"; +import { + buildEditHistoryEntry, + createEmptyEditHistory, + hashEditHistoryContent, + pushEditHistoryEntry, + undoEditHistory, + type EditHistoryState, +} from "../../utils/editHistory"; + +function clip(id: string, start: number, duration: number, extra: Partial = {}) { + return { id, domId: id, tag: "video", start, duration, track: 1, ...extra } as TimelineElement; +} + +const dragged = clip("d", 20, 2); +const draggedEdit = (start: number): TimelineMoveEdit => ({ + element: dragged, + updates: { start, track: 1 }, +}); +const result = (r: Partial): PlacementResult => ({ + track: 1, + start: 0, + cuts: [], + shifts: [], + ...r, +}); + +describe("buildPlacementSteps", () => { + const a = clip("a", 0, 4); + const b = clip("b", 4, 4, { playbackStart: 1, playbackRate: 2 }); + + it("removes a covered clip and then moves the dragged clip", () => { + const steps = buildPlacementSteps({ + result: result({ start: 0, cuts: [{ kind: "remove", key: "a" }] }), + mode: "overwrite", + laneClips: [a], + draggedEdit: draggedEdit(0), + }); + expect(steps).toEqual([ + { kind: "remove", elements: [a] }, + { kind: "move", edits: [draggedEdit(0)] }, + ]); + }); + + it("trims a tail without touching its start", () => { + const steps = buildPlacementSteps({ + result: result({ start: 2, cuts: [{ kind: "trim-tail", key: "a", duration: 2 }] }), + mode: "overwrite", + laneClips: [a], + draggedEdit: draggedEdit(2), + }); + expect(steps[0]).toEqual({ + kind: "resize", + changes: [{ element: a, start: 0, duration: 2 }], + }); + }); + + it("trims a head and moves its source in-point by the cut at the clip's own rate", () => { + const steps = buildPlacementSteps({ + result: result({ + start: 2, + cuts: [{ kind: "trim-head", key: "b", start: 6, duration: 2, sourceShift: 2 }], + }), + mode: "overwrite", + laneClips: [b], + draggedEdit: draggedEdit(4), + }); + // 1s in-point + 2s of timeline cut at 2x rate = 5s into the source. + expect(steps[0]).toEqual({ + kind: "resize", + changes: [{ element: b, start: 6, duration: 2, playbackStart: 5 }], + }); + }); + + it("leaves the playback start of a clip with no source alone", () => { + const text = clip("t", 4, 4, { tag: "div" }); + const steps = buildPlacementSteps({ + result: result({ + cuts: [{ kind: "trim-head", key: "t", start: 6, duration: 2, sourceShift: 2 }], + }), + mode: "overwrite", + laneClips: [text], + draggedEdit: draggedEdit(4), + }); + expect(steps[0]).toMatchObject({ + changes: [{ start: 6, duration: 2, playbackStart: undefined }], + }); + }); + + it("splits at the end of the drop range, then trims the original with its post-split length", () => { + const steps = buildPlacementSteps({ + result: result({ + start: 1, + cuts: [ + { + kind: "split", + key: "a", + headDuration: 1, + tail: { start: 3, duration: 1, sourceShift: 3 }, + }, + ], + }), + mode: "overwrite", + laneClips: [a], + draggedEdit: draggedEdit(1), + }); + expect(steps).toEqual([ + { kind: "split", element: a, at: 3 }, + { + kind: "resize", + changes: [{ element: { ...a, duration: 3 }, start: 0, duration: 1 }], + }, + { kind: "move", edits: [draggedEdit(1)] }, + ]); + }); + + it("insert pushes later clips before the dragged clip lands", () => { + const steps = buildPlacementSteps({ + result: result({ start: 4, shifts: [{ key: "b", start: 6 }] }), + mode: "insert", + laneClips: [b], + draggedEdit: draggedEdit(4), + }); + expect(steps).toEqual([ + { kind: "move", edits: [{ element: b, updates: { start: 6, track: 1 } }] }, + { kind: "move", edits: [draggedEdit(4)] }, + ]); + }); + + it("insert into a straddled clip pushes it, splits at the pushed drop end, moves the head back", () => { + const steps = buildPlacementSteps({ + result: result({ + start: 6, + cuts: [ + { + kind: "split", + key: "b", + headDuration: 2, + tail: { start: 8, duration: 2, sourceShift: 2 }, + }, + ], + }), + mode: "insert", + laneClips: [b], + draggedEdit: draggedEdit(6), + }); + const pushed = { ...b, start: 6 }; + expect(steps).toEqual([ + { kind: "move", edits: [{ element: b, updates: { start: 6, track: 1 } }] }, + { kind: "split", element: pushed, at: 8 }, + { + kind: "move", + edits: [ + { element: { ...pushed, duration: 2 }, updates: { start: 4, track: 1 } }, + draggedEdit(6), + ], + }, + ]); + }); +}); + +describe("placementRefusal", () => { + const cutA = result({ start: 0, cuts: [{ kind: "remove", key: "a" }] }); + + it("allows a clean overwrite", () => { + expect(placementRefusal(cutA, "overwrite", [clip("a", 0, 4)])).toBeNull(); + }); + + it("refuses when a clip that would be cut is locked", () => { + expect( + placementRefusal(cutA, "overwrite", [clip("a", 0, 4, { timelineLocked: true })]), + ).toMatch(/locked or expanded/); + }); + + it("refuses when a clip that would be cut is an expanded child", () => { + expect( + placementRefusal(cutA, "overwrite", [clip("a", 0, 4, { expandedParentStart: 2 })]), + ).toMatch(/locked or expanded/); + }); + + it("refuses when a clip that would be pushed is locked", () => { + const push = result({ start: 0, shifts: [{ key: "a", start: 2 }] }); + expect(placementRefusal(push, "insert", [clip("a", 0, 4, { timelineLocked: true })])).toMatch( + /locked or expanded/, + ); + }); + + it("refuses a split closer to a clip edge than the split epsilon", () => { + const split = result({ + start: 0, + cuts: [ + { + kind: "split", + key: "a", + headDuration: 3.98, + tail: { start: 3.99, duration: 0.01, sourceShift: 3.99 }, + }, + ], + }); + expect(placementRefusal(split, "overwrite", [clip("a", 0, 4)])).toMatch(/split/); + }); +}); + +interface Doc { + [id: string]: { start: number; duration: number; playbackStart?: number }; +} + +/** A tiny stand-in for the project file plus the history recorder, driven by the real reducer. */ +function createFakeProject(initial: Doc) { + let doc: Doc = structuredClone(initial); + let history: EditHistoryState = createEmptyEditHistory(); + let clock = 0; + const serialize = (d: Doc) => + JSON.stringify(Object.entries(d).sort(([x], [y]) => (x < y ? -1 : 1))); + const record = (mutate: () => void, label: string, fold: PlacementFold) => { + const before = serialize(doc); + mutate(); + history = pushEditHistoryEntry( + history, + buildEditHistoryEntry({ + id: `e${clock}`, + projectId: "p", + label, + kind: "timeline", + coalesceKey: fold.coalesceKey, + coalesceMs: fold.coalesceMs, + now: (clock += 1000), + files: { "index.html": { before, after: serialize(doc) } }, + }), + ); + }; + const current = (element: TimelineElement) => { + const found = doc[element.id]; + if (!found) throw new Error(`${element.id} is not in the document`); + // The element a step hands over must describe what the document holds right now. + expect({ start: element.start, duration: element.duration }).toEqual({ + start: found.start, + duration: found.duration, + }); + return found; + }; + const ops: PlacementOps = { + split: async (element, at, fold) => { + record( + () => { + const target = current(element); + const end = target.start + target.duration; + doc[`${element.id}-split`] = { + start: at, + duration: end - at, + playbackStart: (target.playbackStart ?? 0) + (at - target.start), + }; + target.duration = at - target.start; + }, + "split", + fold, + ); + return true; + }, + remove: async (elements, fold) => { + record( + () => { + for (const element of elements) { + current(element); + delete doc[element.id]; + } + }, + "delete", + fold, + ); + return true; + }, + toast: vi.fn(), + }; + const resize = async (changes: PlacementResizeChange[], fold: PlacementFold) => { + record( + () => { + for (const change of changes) { + const target = current(change.element); + target.start = change.start; + target.duration = change.duration; + if (change.playbackStart != null) target.playbackStart = change.playbackStart; + } + }, + "resize", + fold, + ); + }; + const move = async (edits: TimelineMoveEdit[], fold: PlacementFold) => { + record( + () => { + for (const edit of edits) { + const found = doc[edit.element.id]; + if (found) found.start = edit.updates.start; + else + doc[edit.element.id] = { start: edit.updates.start, duration: edit.element.duration }; + } + }, + "move", + fold, + ); + return true; + }; + return { + ops, + resize, + move, + doc: () => doc, + history: () => history, + undo: () => { + const undone = undoEditHistory( + history, + { "index.html": hashEditHistoryContent(serialize(doc)) }, + 0, + ); + if (!undone.ok) throw new Error(`undo refused: ${undone.reason}`); + return undone.filesToWrite["index.html"]; + }, + serializeInitial: () => serialize(initial), + }; +} + +function dragOf(element: TimelineElement, previewStart: number, previewTrack = 1) { + return { + element, + previewStart, + previewTrack, + insertRow: null, + } as unknown as DraggedClipState; +} + +function depsOf(elements: TimelineElement[], project: ReturnType) { + return { + elements, + trackOrder: [1], + updateElement: vi.fn(), + placementOps: project.ops, + onResizeElements: project.resize, + } as unknown as DragCommitDeps; +} + +describe("commitPlacementDrop: one undo step for the move and every cut", () => { + // Lane 1: a [0,4) and b [4,8) video (b reads its source from 1s at rate 1), dragged d [20,22). + const a = clip("a", 0, 4); + const b = clip("b", 4, 4, { playbackStart: 1 }); + const d = clip("d", 20, 2); + const start: Doc = { + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4, playbackStart: 1 }, + d: { start: 20, duration: 2 }, + }; + + async function drop(at: number, mode: "overwrite" | "insert" = "overwrite", lane = [a, b]) { + const project = createFakeProject(start); + const placed = commitPlacementDrop( + dragOf(d, at), + depsOf([...lane, d], project), + mode, + project.move, + ); + expect(placed).not.toBeNull(); + await placed; + return project; + } + + it("trimmed head: the clip underneath starts later and reads its source later", async () => { + const project = await drop(4); + // d [4,6) leaves a alone and trims b to [6,8), reading the source from 3s. + expect(project.doc()).toEqual({ + a: { start: 0, duration: 4 }, + b: { start: 6, duration: 2, playbackStart: 3 }, + d: { start: 4, duration: 2 }, + }); + expect(project.history().undo).toHaveLength(1); + expect(project.undo()).toBe(project.serializeInitial()); + }); + + it("removed: a clip the drop fully covers is deleted and the drop lands", async () => { + const project = createFakeProject({ ...start, d: { start: 20, duration: 6 } }); + const long = clip("d", 20, 6); + await commitPlacementDrop( + dragOf(long, 4), + depsOf([a, b, long], project), + "overwrite", + project.move, + ); + expect(project.doc()).toEqual({ + a: { start: 0, duration: 4 }, + d: { start: 4, duration: 6 }, + }); + expect(project.history().undo).toHaveLength(1); + expect(project.undo()).toBe(project.serializeInitial()); + }); + + it("trimmed tail: the drop over a tail shortens the clip underneath", async () => { + const project = await drop(2); + // d [2,4): a loses [2,4) so it ends at 2. + expect(project.doc().a).toEqual({ start: 0, duration: 2 }); + expect(project.doc().d).toEqual({ start: 2, duration: 2 }); + expect(project.history().undo).toHaveLength(1); + expect(project.undo()).toBe(project.serializeInitial()); + }); + + it("split: a drop inside a clip leaves a head and a tail, one undo removes the tail too", async () => { + const project = await drop(1, "overwrite", [a]); + // d [1,3) inside a [0,4): head [0,1), tail [3,4) reading the source from 3s. + expect(project.doc()).toEqual({ + a: { start: 0, duration: 1 }, + "a-split": { start: 3, duration: 1, playbackStart: 3 }, + d: { start: 1, duration: 2 }, + }); + expect(project.history().undo).toHaveLength(1); + expect(project.undo()).toBe(project.serializeInitial()); + }); + + it("insert: what follows is pushed right and the straddled clip is split around the drop", async () => { + const project = await drop(6, "insert", [b]); + // d [6,8) into b [4,8): head [4,6), tail [8,10) reading the source from 3s. + expect(project.doc()).toEqual({ + b: { start: 4, duration: 2, playbackStart: 1 }, + "b-split": { start: 8, duration: 2, playbackStart: 3 }, + d: { start: 6, duration: 2 }, + }); + expect(project.history().undo).toHaveLength(1); + expect(project.undo()).toBe(project.serializeInitial()); + }); + + it("refuses the whole drop when a clip it would cut is locked, writing nothing", async () => { + const project = createFakeProject(start); + const locked = clip("b", 4, 4, { timelineLocked: true }); + await commitPlacementDrop( + dragOf(d, 4), + depsOf([a, locked, d], project), + "overwrite", + project.move, + ); + expect(project.history().undo).toHaveLength(0); + expect(project.doc()).toEqual(start); + expect(project.ops.toast).toHaveBeenCalledWith(expect.stringMatching(/locked or expanded/)); + }); + + it("leaves a drop that touches nothing to the plain move paths", () => { + const project = createFakeProject(start); + expect( + commitPlacementDrop(dragOf(d, 10), depsOf([a, b, d], project), "overwrite", project.move), + ).toBeNull(); + }); + + it("leaves a drop that did not move to the plain move paths", () => { + const project = createFakeProject(start); + expect( + commitPlacementDrop(dragOf(d, 20), depsOf([a, b, d], project), "overwrite", project.move), + ).toBeNull(); + }); +}); + +describe("runPlacementSteps failures", () => { + const steps = [ + { kind: "split", element: clip("a", 0, 4), at: 2 }, + { kind: "move", edits: [] }, + ] as const; + + it("names a partial apply and stops when a later step fails", async () => { + const toast = vi.fn(); + await runPlacementSteps(steps, { + ops: { split: async () => true, remove: async () => true, toast }, + resize: vi.fn(), + move: async () => false, + }); + expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); + }); + + it("says nothing extra when the first step fails, since nothing was applied", async () => { + const toast = vi.fn(); + const move = vi.fn(async () => true); + await runPlacementSteps(steps, { + ops: { split: async () => false, remove: async () => true, toast }, + resize: vi.fn(), + move, + }); + expect(toast).not.toHaveBeenCalled(); + expect(move).not.toHaveBeenCalled(); + }); + + it("treats a rejected step as a failure", async () => { + const toast = vi.fn(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + await runPlacementSteps(steps, { + ops: { split: async () => true, remove: async () => true, toast }, + resize: vi.fn(), + move: async () => { + throw new Error("write failed"); + }, + }); + expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); + }); +}); diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts new file mode 100644 index 0000000000..dc0e6c1d91 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -0,0 +1,236 @@ +import type { TimelineElement } from "../store/playerStore"; +import type { DraggedClipState } from "./timelineClipDragTypes"; +import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; +import { canMoveTimelineElement } from "./timelineAuthoredMoveTarget"; +import { authoredTrackForLane } from "./timelineAuthoredTrack"; +import { round3 } from "./timelineGaps"; +import { hasSourcePlaybackOffset } from "./timelineGroupEditing"; +import { + placeClip, + type PlacementMode, + type PlacementResult, + type PlacementShift, +} from "./timelinePlacement"; +import { canSplitElementAt } from "../../utils/timelineElementSplit"; + +/** One shared history key and an unbounded window: every write of a drop is one undo step. */ +export interface PlacementFold { + coalesceKey: string; + coalesceMs: number; +} + +export interface PlacementResizeChange { + element: TimelineElement; + start: number; + duration: number; + playbackStart?: number; +} + +/** The two writes a drop needs beyond move and resize; both must record with the fold key. */ +export interface PlacementOps { + split: (element: TimelineElement, at: number, fold: PlacementFold) => Promise; + remove: (elements: TimelineElement[], fold: PlacementFold) => Promise; + toast: (message: string) => void; +} + +export type PlacementStep = + | { kind: "split"; element: TimelineElement; at: number } + | { kind: "remove"; elements: TimelineElement[] } + | { kind: "resize"; changes: PlacementResizeChange[] } + | { kind: "move"; edits: TimelineMoveEdit[] }; + +const keyOf = (e: TimelineElement) => e.key ?? e.id; + +const moveEdit = (element: TimelineElement, start: number): TimelineMoveEdit => ({ + element, + updates: { start, track: element.track }, +}); + +/** Where a trimmed-in clip starts reading its source: later by the cut time, at its own rate. */ +function trimmedPlaybackStart(el: TimelineElement, sourceShift: number): number | undefined { + if (!hasSourcePlaybackOffset(el)) return el.playbackStart; + return round3((el.playbackStart ?? 0) + sourceShift * (el.playbackRate ?? 1)); +} + +interface PlacementPlanInput { + result: PlacementResult; + mode: PlacementMode; + laneClips: readonly TimelineElement[]; + draggedEdit: TimelineMoveEdit; +} + +/** + * Placement to ordered writes, needing no new clip id: an overwrite splits at the drop end then + * trims the original; an insert pushes the straddled clip, splits it, then moves the head back. + */ +export function buildPlacementSteps({ + result, + mode, + laneClips, + draggedEdit, +}: PlacementPlanInput): PlacementStep[] { + const byKey = new Map(laneClips.map((e) => [keyOf(e), e])); + const clip = (key: string): TimelineElement => { + const found = byKey.get(key); + if (!found) throw new Error(`Placement referenced ${key}, which is not on the target lane`); + return found; + }; + const shiftEdits = result.shifts.map((s: PlacementShift) => + moveEdit(clip(s.key), round3(s.start)), + ); + const splits: PlacementStep[] = []; + const removes: TimelineElement[] = []; + const resizes: PlacementResizeChange[] = []; + const settle: TimelineMoveEdit[] = []; + + for (const cut of result.cuts) { + const el = clip(cut.key); + switch (cut.kind) { + case "remove": + removes.push(el); + break; + case "trim-tail": + resizes.push({ element: el, start: el.start, duration: round3(cut.duration) }); + break; + case "trim-head": + resizes.push({ + element: el, + start: round3(cut.start), + duration: round3(cut.duration), + playbackStart: trimmedPlaybackStart(el, cut.sourceShift), + }); + break; + case "split": { + const at = round3(cut.tail.start); + if (mode === "overwrite") { + splits.push({ kind: "split", element: el, at }); + resizes.push({ + element: { ...el, duration: round3(at - el.start) }, + start: el.start, + duration: round3(cut.headDuration), + }); + break; + } + const pushed = { ...el, start: round3(el.start + (at - result.start)) }; + shiftEdits.push(moveEdit(el, pushed.start)); + splits.push({ kind: "split", element: pushed, at }); + settle.push(moveEdit({ ...pushed, duration: round3(cut.headDuration) }, el.start)); + break; + } + } + } + + const steps: PlacementStep[] = []; + if (shiftEdits.length > 0) steps.push({ kind: "move", edits: shiftEdits }); + steps.push(...splits); + if (removes.length > 0) steps.push({ kind: "remove", elements: removes }); + if (resizes.length > 0) steps.push({ kind: "resize", changes: resizes }); + steps.push({ kind: "move", edits: [...settle, draggedEdit] }); + return steps; +} + +/** Reason the whole drop must be refused, or null. Nothing is written when this is set. */ +export function placementRefusal( + result: PlacementResult, + mode: PlacementMode, + laneClips: readonly TimelineElement[], +): string | null { + const byKey = new Map(laneClips.map((e) => [keyOf(e), e])); + const touched = [...result.cuts, ...result.shifts].map((c) => byKey.get(c.key)); + if (touched.some((el) => !el || !canMoveTimelineElement(el) || el.expandedParentStart != null)) { + return "Cannot overwrite a locked or expanded clip"; + } + const unsplittable = result.cuts.some((cut) => { + if (cut.kind !== "split") return false; + const at = mode === "overwrite" ? cut.tail.start : result.start; + const el = byKey.get(cut.key); + return !el || !canSplitElementAt(el, at); + }); + return unsplittable ? "Cannot split a clip at the drop point" : null; +} + +let placementGestureSeq = 0; + +interface PlacementRunner { + ops: PlacementOps; + resize: (changes: PlacementResizeChange[], fold: PlacementFold) => Promise | void; + move: (edits: TimelineMoveEdit[], fold: PlacementFold) => Promise; +} + +function runStep(step: PlacementStep, fold: PlacementFold, run: PlacementRunner) { + switch (step.kind) { + case "split": + return run.ops.split(step.element, step.at, fold); + case "remove": + return run.ops.remove(step.elements, fold); + case "resize": + return run.resize(step.changes, fold); + case "move": + return run.move(step.edits, fold); + } +} + +/** Each step awaits the previous: the history fold needs every write to start from the last one's output. */ +export async function runPlacementSteps( + steps: readonly PlacementStep[], + run: PlacementRunner, +): Promise { + const fold: PlacementFold = { + coalesceKey: `clip-overwrite:${placementGestureSeq++}`, + coalesceMs: Number.POSITIVE_INFINITY, + }; + for (const [index, step] of steps.entries()) { + let applied = false; + try { + applied = (await runStep(step, fold, run)) !== false; + } catch (error) { + console.error("[Timeline] Overwrite step failed", error); + } + if (applied) continue; + if (index > 0) run.ops.toast("Overwrite partly applied, Undo restores it"); + return; + } +} + +/** Commit a single-clip drop onto its lane's clips; null when it touches none, so plain moves apply. */ +export function commitPlacementDrop( + drag: DraggedClipState, + deps: DragCommitDeps, + mode: PlacementMode, + move: PlacementRunner["move"], +): Promise | null { + const { placementOps, onResizeElements, elements } = deps; + const dragKey = keyOf(drag.element); + const laneChanged = drag.previewTrack !== drag.element.track; + if (!placementOps || !onResizeElements) return null; + if (!laneChanged && drag.previewStart === drag.element.start) return null; + + const laneClips = elements.filter((e) => e.track === drag.previewTrack && keyOf(e) !== dragKey); + const result = placeClip({ + clips: laneClips.map((e) => ({ key: keyOf(e), start: e.start, duration: e.duration })), + track: drag.previewTrack, + start: drag.previewStart, + duration: drag.element.duration, + mode, + }); + if (result.cuts.length === 0 && result.shifts.length === 0) return null; + + const refusal = placementRefusal(result, mode, laneClips); + if (refusal) { + placementOps.toast(refusal); + return Promise.resolve(); + } + const draggedEdit: TimelineMoveEdit = { + element: drag.element, + updates: { start: result.start, track: drag.previewTrack }, + ...(laneChanged + ? { persistTrack: authoredTrackForLane(drag.previewTrack, elements, drag.element) } + : {}), + }; + const steps = buildPlacementSteps({ result, mode, laneClips, draggedEdit }); + return runPlacementSteps(steps, { + ops: placementOps, + resize: (changes, fold) => onResizeElements(changes, fold), + move, + }); +} diff --git a/packages/studio/src/player/components/useTimelineClipDrag.ts b/packages/studio/src/player/components/useTimelineClipDrag.ts index 7c49eda117..3b84594762 100644 --- a/packages/studio/src/player/components/useTimelineClipDrag.ts +++ b/packages/studio/src/player/components/useTimelineClipDrag.ts @@ -1,5 +1,6 @@ import { useRef, useState, useCallback, useMemo, useEffect } from "react"; import { useMountEffect } from "../../hooks/useMountEffect"; +import { useTimelineEditContextOptional } from "../../contexts/TimelineEditContext"; import { applyTimelineAutoScrollStep, resolveTimelineAutoScrollLoopAction, @@ -243,6 +244,9 @@ export function useTimelineClipDrag({ // It owns a projection only; canonical store timing changes at commit. const groupResizeRef = useRef(null); + const { onPlacementOps } = useTimelineEditContextOptional(); + const onPlacementOpsRef = useRef(onPlacementOps); + onPlacementOpsRef.current = onPlacementOps; const onMoveElementRef = useRef(onMoveElement); onMoveElementRef.current = onMoveElement; const onMoveElementsRef = useRef(onMoveElements); @@ -418,6 +422,7 @@ export function useTimelineClipDrag({ useMountEffect(() => mountTimelineClipDragGestureLifecycle({ onStackingPatchesRef, + onPlacementOpsRef, refreshAfterLaneMoveRef, readZIndexRef, onBlockedEditAttemptRef, diff --git a/packages/studio/src/utils/razorSplitTransaction.ts b/packages/studio/src/utils/razorSplitTransaction.ts index 77054df21c..e1e04a4555 100644 --- a/packages/studio/src/utils/razorSplitTransaction.ts +++ b/packages/studio/src/utils/razorSplitTransaction.ts @@ -166,6 +166,9 @@ interface RunAtomicCutInput { projectId: string; intents: CutFileIntent[]; label: string; + /** Folds this cut into the undo entry of the writes that share the key. */ + coalesceKey?: string; + coalesceMs?: number; writeProjectFile: ProjectFileWriter; recordEdit: (input: RecordEditInput) => Promise; observeProjectFileVersion?: (path: string, version: string | null) => void; @@ -181,7 +184,13 @@ export function runAtomicCutTransaction(input: RunAtomicCutInput): Promise [file.path, { before: file.before, after: file.after }]), ); try { - await input.recordEdit({ label: input.label, kind: "timeline", files: snapshots }); + await input.recordEdit({ + label: input.label, + kind: "timeline", + coalesceKey: input.coalesceKey, + coalesceMs: input.coalesceMs, + files: snapshots, + }); } catch (error) { try { await rollbackUnrecordedCut(result.files, input.writeProjectFile); From 079976e5b745308f9917d89a68f13d4dc0ff10cb Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:16:29 -0400 Subject: [PATCH 03/16] test(studio): overwrite fold tests keep untouched lane clips in the expected document --- .../src/player/components/timelinePlacementCommit.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index 2b1f4e6823..cb011c3f63 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -418,6 +418,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => const project = await drop(1, "overwrite", [a]); // d [1,3) inside a [0,4): head [0,1), tail [3,4) reading the source from 3s. expect(project.doc()).toEqual({ + b: { start: 4, duration: 4, playbackStart: 1 }, a: { start: 0, duration: 1 }, "a-split": { start: 3, duration: 1, playbackStart: 3 }, d: { start: 1, duration: 2 }, @@ -430,6 +431,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => const project = await drop(6, "insert", [b]); // d [6,8) into b [4,8): head [4,6), tail [8,10) reading the source from 3s. expect(project.doc()).toEqual({ + a: { start: 0, duration: 4 }, b: { start: 4, duration: 2, playbackStart: 1 }, "b-split": { start: 8, duration: 2, playbackStart: 3 }, d: { start: 6, duration: 2 }, From 96d545547f202dc381ebc1f7902e88a1c426f3bd Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:21:12 -0400 Subject: [PATCH 04/16] fix(studio): a wrong-zone aim still creates a track and a group drag keeps relocating --- .../player/components/timelineClipDragPreview.test.ts | 10 +++++++++- .../studio/src/player/components/timelineCollision.ts | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/studio/src/player/components/timelineClipDragPreview.test.ts b/packages/studio/src/player/components/timelineClipDragPreview.test.ts index 346770f9d0..c954f29c56 100644 --- a/packages/studio/src/player/components/timelineClipDragPreview.test.ts +++ b/packages/studio/src/player/components/timelineClipDragPreview.test.ts @@ -168,7 +168,7 @@ describe("computeDragPreview — plain horizontal drag never arms a phantom inse expect(next.previewTrack).toBe(0); }); - it("uses the expanded row midpoint when choosing the side for an automatic insert", () => { + it("uses the expanded row midpoint when choosing the side for a group drag's automatic insert", () => { const rowHeights = [TRACK_H + 2 * LANE_H, TRACK_H]; const dragged = clip("dragged", 0, 0, 1, 3); const occupied = [dragged, clip("block-0", 0, 0, 1, 2), clip("block-1", 1, 0, 1, 1)]; @@ -194,8 +194,16 @@ describe("computeDragPreview — plain horizontal drag never arms a phantom inse const next = computeDragPreview(drag, 0, clientY, { ...ctx(rowHeights, occupied), trackOrder: [0, 1], + selectedKeys: new Set(["dragged", "block-1"]), }); expect(next.insertRow).toBe(0); + // A single clip dropped on the same occupied lanes stays put and overwrites instead. + const single = computeDragPreview(drag, 0, clientY, { + ...ctx(rowHeights, occupied), + trackOrder: [0, 1], + }); + expect(single.insertRow).toBeNull(); + expect(single.previewTrack).toBe(0); }); }); diff --git a/packages/studio/src/player/components/timelineCollision.ts b/packages/studio/src/player/components/timelineCollision.ts index 23a6e4b99c..eb459c1d68 100644 --- a/packages/studio/src/player/components/timelineCollision.ts +++ b/packages/studio/src/player/components/timelineCollision.ts @@ -111,10 +111,10 @@ export function resolveZoneDropPlacement(input: { } const desired = clampTrackToZone(desiredTrack, order, audioRow, isAudio); - if (!input.relocateOnOverlap && order.includes(desired)) { + const zoneTracks = order.filter((t) => audioTracks.has(t) === isAudio); + if (!input.relocateOnOverlap && zoneTracks.includes(desired)) { return { track: desired, insertRow: null }; } - const zoneTracks = order.filter((t) => audioTracks.has(t) === isAudio); const placement = resolvePlacement({ elements, desiredTrack: desired, From 67dd38882185265773d196aaa8da6bae473d8736 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:27:28 -0400 Subject: [PATCH 05/16] test(studio): alt at pointer-up drives an insert through the drag lifecycle --- .../timelineClipDragGestureLifecycle.test.ts | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts index 74b7c2e085..d389ff92e9 100644 --- a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts +++ b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts @@ -89,6 +89,7 @@ describe("timeline clip drag gesture lifecycle", () => { readZIndexRef: { current: undefined }, onStackingPatchesRef: { current: undefined }, refreshAfterLaneMoveRef: { current: undefined }, + onPlacementOpsRef: { current: undefined }, }); const sourceRow = document.createElement("div"); @@ -109,4 +110,99 @@ describe("timeline clip drag gesture lifecycle", () => { dispose(); expect(cancelGestureRef.current()).toBe(false); }); + it("reads Alt at pointer-up as insert mode: what follows is pushed, then the clip lands", async () => { + const dragged: TimelineElement = { + id: "d", + domId: "d", + tag: "div", + start: 0, + duration: 2, + track: 0, + }; + const later: TimelineElement = { + id: "b", + domId: "b", + tag: "div", + start: 5, + duration: 4, + track: 0, + }; + const drag = { + pointerId: 0, + element: dragged, + originClientX: 0, + originClientY: 0, + originScrollLeft: 0, + originScrollTop: 0, + pointerClientX: 0, + pointerClientY: 0, + pointerOffsetX: 0, + pointerOffsetY: 0, + previewStart: 3, + previewTrack: 0, + desiredTrack: 0, + insertRow: null, + snapTime: null, + snapType: null, + started: true, + } as DraggedClipState; + const draggedClipRef = { current: drag as DraggedClipState | null }; + const setDraggedClip = (next: SetStateAction) => { + draggedClipRef.current = typeof next === "function" ? next(draggedClipRef.current) : next; + }; + const onMoveElements = vi.fn(async () => undefined); + const dispose = mountTimelineClipDragGestureLifecycle({ + lifecycleRef: { + current: { kind: "drag", phase: "active", pointerId: null, sessionEpoch: 0 }, + }, + sessionEpochRef: { current: 0 }, + cancelGestureRef: { current: () => false }, + scrollRef: { current: null }, + draggedClipRef, + resizingClipRef: { current: null }, + blockedClipRef: { current: null }, + groupResizeRef: { current: null }, + suppressClickRef: { current: false }, + gestureSelectedKeysRef: { current: new Set() }, + elementsRef: { current: [dragged, later] }, + trackOrderRef: { current: [0] }, + setDraggedClipState: setDraggedClip, + setResizingClipState: () => {}, + setShowPopover: () => {}, + setRangeSelectionRef: { current: null }, + applyResizePointerRef: { current: () => {} }, + syncClipDragAutoScrollRef: { current: () => {} }, + stopClipDragAutoScrollRef: { current: () => {} }, + updateDraggedClipPreviewRef: { current: (previous: DraggedClipState) => previous }, + publishDraggedClip: setDraggedClip, + updateElement: vi.fn(), + onMoveElementRef: { current: vi.fn() }, + onMoveElementsRef: { current: onMoveElements }, + onResizeElementRef: { current: undefined }, + onResizeElementsRef: { current: vi.fn() }, + onBlockedEditAttemptRef: { current: undefined }, + readZIndexRef: { current: undefined }, + onStackingPatchesRef: { current: undefined }, + refreshAfterLaneMoveRef: { current: undefined }, + onPlacementOpsRef: { + current: { + split: vi.fn(async () => true), + remove: vi.fn(async () => true), + toast: vi.fn(), + }, + }, + }); + + window.dispatchEvent(new MouseEvent("pointerup", { altKey: true })); + await vi.waitFor(() => expect(onMoveElements).toHaveBeenCalledTimes(2)); + // The clip after the drop moves 5 -> 7 (drop is 2s long), then the dragged clip lands at 3. + const [pushed, landed] = onMoveElements.mock.calls as unknown as Array< + [Array<{ element: TimelineElement; updates: { start: number } }>, string] + >; + expect(pushed[0].map((e) => [e.element.id, e.updates.start])).toEqual([["b", 7]]); + expect(landed[0].map((e) => [e.element.id, e.updates.start])).toEqual([["d", 3]]); + expect(pushed[1]).toMatch(/^clip-overwrite:/); + expect(landed[1]).toBe(pushed[1]); + dispose(); + }); }); From e8295b4f5ab735d708305f17b76eaf839b64fb4d Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:30:50 -0400 Subject: [PATCH 06/16] test(studio): cut and delete record under the drop's fold key, delete does not ripple --- .../src/hooks/useTimelineDeleteOps.test.tsx | 23 ++++++++++++++++++- .../src/utils/razorSplitTransaction.test.ts | 20 ++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx index 682cb5e829..de265a1e0b 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx +++ b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx @@ -58,6 +58,7 @@ describe("useTimelineDeleteOps: ripple undo label", () => { function mountDeleteHarness(overrides: { handleTimelineGroupMove: DeleteOpsOptions["handleTimelineGroupMove"]; showToast?: DeleteOpsOptions["showToast"]; + recordEdit?: DeleteOpsOptions["recordEdit"]; }) { const elements = [el("hf-a", 0, 2), el("hf-b", 2, 2), el("hf-c", 4, 2)]; let hook: ReturnType | null = null; @@ -68,7 +69,7 @@ describe("useTimelineDeleteOps: ripple undo label", () => { timelineElements: elements, showToast: overrides.showToast ?? vi.fn(), writeProjectFile: vi.fn().mockResolvedValue(undefined), - recordEdit: vi.fn().mockResolvedValue(undefined), + recordEdit: overrides.recordEdit ?? vi.fn().mockResolvedValue(undefined), reloadPreview: vi.fn(), previewIframeRef: { current: null }, handleTimelineGroupMove: overrides.handleTimelineGroupMove, @@ -116,4 +117,24 @@ describe("useTimelineDeleteOps: ripple undo label", () => { expect.objectContaining({ suppressFailureToast: true }), ); }); + + it("an overwrite delete records under the drop's fold key and never ripples the main track", async () => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + const recordEdit = vi.fn().mockResolvedValue(undefined); + const { b, getHook } = mountDeleteHarness({ handleTimelineGroupMove, recordEdit }); + let deleted: boolean | undefined; + + await act(async () => { + deleted = await getHook().deleteTimelineElements([b], { + coalesceKey: "clip-overwrite:3", + coalesceMs: Number.POSITIVE_INFINITY, + }); + }); + + expect(deleted).toBe(true); + expect(handleTimelineGroupMove).not.toHaveBeenCalled(); + expect(recordEdit).toHaveBeenCalledWith( + expect.objectContaining({ coalesceKey: "clip-overwrite:3", coalesceMs: Infinity }), + ); + }); }); diff --git a/packages/studio/src/utils/razorSplitTransaction.test.ts b/packages/studio/src/utils/razorSplitTransaction.test.ts index 9e4a2dfebd..aca4ab1beb 100644 --- a/packages/studio/src/utils/razorSplitTransaction.test.ts +++ b/packages/studio/src/utils/razorSplitTransaction.test.ts @@ -141,6 +141,26 @@ describe("runAtomicCutTransaction", () => { expect(result).toMatchObject({ splitCount: 1, syncFailed: false }); }); + it("records the cut under the caller's fold key so it merges into one undo step", async () => { + installCutServer(); + const recordEdit = vi.fn().mockResolvedValue(undefined); + + await runAtomicCutTransaction({ + projectId: "p1", + intents: buildAtomicCutIntents([element()], 2, "index.html"), + label: "Split timeline clip", + coalesceKey: "clip-overwrite:7", + coalesceMs: Number.POSITIVE_INFINITY, + writeProjectFile: vi.fn(), + recordEdit, + synchronize: vi.fn(), + }); + + expect(recordEdit).toHaveBeenCalledWith( + expect.objectContaining({ coalesceKey: "clip-overwrite:7", coalesceMs: Infinity }), + ); + }); + it("CAS-restores durable bytes when history registration fails", async () => { installCutServer(); const writeProjectFile = vi.fn().mockResolvedValue(undefined); From 45ffaf6bf1c8f1bda1a4fcb043d01629e2c448cd Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:54:22 -0400 Subject: [PATCH 07/16] test(studio): the overwrite delete test removes a clip so history records it --- packages/studio/src/hooks/useTimelineDeleteOps.test.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx index de265a1e0b..16c6f3c922 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx +++ b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx @@ -122,6 +122,14 @@ describe("useTimelineDeleteOps: ripple undo label", () => { const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); const recordEdit = vi.fn().mockResolvedValue(undefined); const { b, getHook } = mountDeleteHarness({ handleTimelineGroupMove, recordEdit }); + const withoutB = html.replace(/
]*><\/div>\n/, ""); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const content = String(input).includes("/remove-element/") ? withoutB : html; + return new Response(JSON.stringify({ changed: true, content }), { status: 200 }); + }), + ); let deleted: boolean | undefined; await act(async () => { From 3dc7b9bc5087a258dae2627a4a409b92b465ece5 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 21:59:08 -0400 Subject: [PATCH 08/16] refactor(studio): drop a redundant type annotation in the placement commit --- .../src/player/components/timelinePlacementCommit.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index dc0e6c1d91..f76e840eeb 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -5,12 +5,7 @@ import { canMoveTimelineElement } from "./timelineAuthoredMoveTarget"; import { authoredTrackForLane } from "./timelineAuthoredTrack"; import { round3 } from "./timelineGaps"; import { hasSourcePlaybackOffset } from "./timelineGroupEditing"; -import { - placeClip, - type PlacementMode, - type PlacementResult, - type PlacementShift, -} from "./timelinePlacement"; +import { placeClip, type PlacementMode, type PlacementResult } from "./timelinePlacement"; import { canSplitElementAt } from "../../utils/timelineElementSplit"; /** One shared history key and an unbounded window: every write of a drop is one undo step. */ @@ -75,9 +70,7 @@ export function buildPlacementSteps({ if (!found) throw new Error(`Placement referenced ${key}, which is not on the target lane`); return found; }; - const shiftEdits = result.shifts.map((s: PlacementShift) => - moveEdit(clip(s.key), round3(s.start)), - ); + const shiftEdits = result.shifts.map((s) => moveEdit(clip(s.key), round3(s.start))); const splits: PlacementStep[] = []; const removes: TimelineElement[] = []; const resizes: PlacementResizeChange[] = []; From 22fd4e0729eb17a3e125c8f444fc2416b4a1d6a1 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 22:06:31 -0400 Subject: [PATCH 09/16] chore(studio): shorten the placement rule comment to the four line limit --- packages/studio/src/player/components/timelinePlacement.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/studio/src/player/components/timelinePlacement.ts b/packages/studio/src/player/components/timelinePlacement.ts index 524f102cf9..63d9b14ad5 100644 --- a/packages/studio/src/player/components/timelinePlacement.ts +++ b/packages/studio/src/player/components/timelinePlacement.ts @@ -43,9 +43,8 @@ export interface PlaceClipInput { const overlaps = (a0: number, a1: number, b0: number, b1: number) => a0 < b1 && b0 < a1; /** - * Premiere's drop rules: an overwrite cuts away the range the clip covers, - * an insert splits a straddled clip at the drop point and pushes what follows. - * Nothing moves to another track and nothing is left hidden. + * Premiere's drop rules: an overwrite cuts away the range the clip covers, an insert splits + * a straddled clip at the drop point and pushes what follows. Nothing changes track or hides. */ export function placeClip({ clips, From 8e597ccbb7d35551148b8518d509106b52f02581 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 18 Sep 2026 23:12:36 -0400 Subject: [PATCH 10/16] refactor(studio): reuse existing overlap, identity and resize types in the placement rule Drops the unread track echo from placeClip and renames its result so it stops shadowing the collision PlacementResult. Types the drag move fold as PlacementFold. --- .../components/timelineClipDragCommit.ts | 8 +++++-- .../player/components/timelineCollision.ts | 4 ++-- .../components/timelinePlacement.test.ts | 5 ++-- .../player/components/timelinePlacement.ts | 20 +++++----------- .../timelinePlacementCommit.test.ts | 9 ++++--- .../components/timelinePlacementCommit.ts | 24 +++++++------------ 6 files changed, 28 insertions(+), 42 deletions(-) diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index 9321df432b..6b7f615e60 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -18,7 +18,11 @@ import { import { runLaneZGesture } from "../../components/nle/zLaneGesture"; import { refreshAfterDurableLaneMove } from "./timelineLaneMoveRefresh"; import { authoredTrackForLane } from "./timelineAuthoredTrack"; -import { commitPlacementDrop, type PlacementOps } from "./timelinePlacementCommit"; +import { + commitPlacementDrop, + type PlacementFold, + type PlacementOps, +} from "./timelinePlacementCommit"; import type { TimelineEditCallbacks } from "./timelineCallbacks"; type StartTrack = Pick; @@ -250,7 +254,7 @@ export function commitDraggedClipMove(rawDrag: DraggedClipState, deps: DragCommi if (!isInsert && !multi) { const mode = deps.insertMode ? "insert" : "overwrite"; - const move = (edits: TimelineMoveEdit[], fold: { coalesceKey: string; coalesceMs: number }) => + const move = (edits: TimelineMoveEdit[], fold: PlacementFold) => refreshAfterDurableLaneMove( persistMoveEdits( edits, diff --git a/packages/studio/src/player/components/timelineCollision.ts b/packages/studio/src/player/components/timelineCollision.ts index eb459c1d68..4c879971f8 100644 --- a/packages/studio/src/player/components/timelineCollision.ts +++ b/packages/studio/src/player/components/timelineCollision.ts @@ -100,7 +100,7 @@ export function resolveZoneDropPlacement(input: { relocateOnOverlap?: boolean; }): { track: number; insertRow: number | null } { const { order, audioTracks, elements, desiredTrack, deliberateInsertRow } = input; - const { start, duration, dragKey, isAudio, preferInsertAbove } = input; + const { start, duration, dragKey, isAudio, preferInsertAbove, relocateOnOverlap } = input; const audioRow = order.findIndex((t) => audioTracks.has(t)); if ( @@ -112,7 +112,7 @@ export function resolveZoneDropPlacement(input: { const desired = clampTrackToZone(desiredTrack, order, audioRow, isAudio); const zoneTracks = order.filter((t) => audioTracks.has(t) === isAudio); - if (!input.relocateOnOverlap && zoneTracks.includes(desired)) { + if (!relocateOnOverlap && zoneTracks.includes(desired)) { return { track: desired, insertRow: null }; } const placement = resolvePlacement({ diff --git a/packages/studio/src/player/components/timelinePlacement.test.ts b/packages/studio/src/player/components/timelinePlacement.test.ts index 84ac880777..5591991440 100644 --- a/packages/studio/src/player/components/timelinePlacement.test.ts +++ b/packages/studio/src/player/components/timelinePlacement.test.ts @@ -10,7 +10,7 @@ const place = ( duration: number, mode: PlacementMode = "overwrite", on: PlacementClip[] = clips, -) => placeClip({ clips: on, track: 1, start, duration, mode }); +) => placeClip({ clips: on, start, duration, mode }); describe("placeClip overwrite", () => { it("leaves an abutting neighbour alone", () => { @@ -41,9 +41,8 @@ describe("placeClip overwrite", () => { ]); }); - it("never changes the track and shifts nothing", () => { + it("shifts nothing", () => { const r = place(2, 3); - expect(r.track).toBe(1); expect(r.shifts).toEqual([]); }); diff --git a/packages/studio/src/player/components/timelinePlacement.ts b/packages/studio/src/player/components/timelinePlacement.ts index 63d9b14ad5..dc8d47ef10 100644 --- a/packages/studio/src/player/components/timelinePlacement.ts +++ b/packages/studio/src/player/components/timelinePlacement.ts @@ -1,3 +1,5 @@ +import { timeRangesOverlap } from "./timelineCollision"; + export interface PlacementClip { key: string; start: number; @@ -23,8 +25,7 @@ export interface PlacementShift { start: number; } -export interface PlacementResult { - track: number; +export interface PlaceClipResult { start: number; cuts: PlacementCut[]; shifts: PlacementShift[]; @@ -33,26 +34,17 @@ export interface PlacementResult { export interface PlaceClipInput { /** Clips already on the target track, the dragged clip excluded. */ clips: readonly PlacementClip[]; - track: number; /** Already snapped; this function never snaps. */ start: number; duration: number; mode: PlacementMode; } -const overlaps = (a0: number, a1: number, b0: number, b1: number) => a0 < b1 && b0 < a1; - /** * Premiere's drop rules: an overwrite cuts away the range the clip covers, an insert splits * a straddled clip at the drop point and pushes what follows. Nothing changes track or hides. */ -export function placeClip({ - clips, - track, - start, - duration, - mode, -}: PlaceClipInput): PlacementResult { +export function placeClip({ clips, start, duration, mode }: PlaceClipInput): PlaceClipResult { const from = Math.max(0, start); const to = from + duration; const cuts: PlacementCut[] = []; @@ -73,7 +65,7 @@ export function placeClip({ } continue; } - if (!overlaps(from, to, clip.start, end)) continue; + if (!timeRangesOverlap(from, to, clip.start, end)) continue; const headKept = clip.start < from; const tailKept = end > to; if (headKept && tailKept) { @@ -97,5 +89,5 @@ export function placeClip({ cuts.push({ kind: "remove", key: clip.key }); } } - return { track, start: from, cuts, shifts }; + return { start: from, cuts, shifts }; } diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index cb011c3f63..24ab9977d2 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it, vi } from "vitest"; +import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; import type { TimelineElement } from "../store/playerStore"; import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; import type { DraggedClipState } from "./timelineClipDragTypes"; -import type { PlacementResult } from "./timelinePlacement"; +import type { PlaceClipResult } from "./timelinePlacement"; import { buildPlacementSteps, commitPlacementDrop, @@ -10,7 +11,6 @@ import { runPlacementSteps, type PlacementFold, type PlacementOps, - type PlacementResizeChange, } from "./timelinePlacementCommit"; import { buildEditHistoryEntry, @@ -30,8 +30,7 @@ const draggedEdit = (start: number): TimelineMoveEdit => ({ element: dragged, updates: { start, track: 1 }, }); -const result = (r: Partial): PlacementResult => ({ - track: 1, +const result = (r: Partial): PlaceClipResult => ({ start: 0, cuts: [], shifts: [], @@ -285,7 +284,7 @@ function createFakeProject(initial: Doc) { }, toast: vi.fn(), }; - const resize = async (changes: PlacementResizeChange[], fold: PlacementFold) => { + const resize = async (changes: TimelineGroupResizeChange[], fold: PlacementFold) => { record( () => { for (const change of changes) { diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index f76e840eeb..225eb548b8 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -5,7 +5,9 @@ import { canMoveTimelineElement } from "./timelineAuthoredMoveTarget"; import { authoredTrackForLane } from "./timelineAuthoredTrack"; import { round3 } from "./timelineGaps"; import { hasSourcePlaybackOffset } from "./timelineGroupEditing"; -import { placeClip, type PlacementMode, type PlacementResult } from "./timelinePlacement"; +import { placeClip, type PlacementMode, type PlaceClipResult } from "./timelinePlacement"; +import { getTimelineElementIdentity as keyOf } from "../lib/timelineElementHelpers"; +import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; import { canSplitElementAt } from "../../utils/timelineElementSplit"; /** One shared history key and an unbounded window: every write of a drop is one undo step. */ @@ -14,13 +16,6 @@ export interface PlacementFold { coalesceMs: number; } -export interface PlacementResizeChange { - element: TimelineElement; - start: number; - duration: number; - playbackStart?: number; -} - /** The two writes a drop needs beyond move and resize; both must record with the fold key. */ export interface PlacementOps { split: (element: TimelineElement, at: number, fold: PlacementFold) => Promise; @@ -31,11 +26,9 @@ export interface PlacementOps { export type PlacementStep = | { kind: "split"; element: TimelineElement; at: number } | { kind: "remove"; elements: TimelineElement[] } - | { kind: "resize"; changes: PlacementResizeChange[] } + | { kind: "resize"; changes: TimelineGroupResizeChange[] } | { kind: "move"; edits: TimelineMoveEdit[] }; -const keyOf = (e: TimelineElement) => e.key ?? e.id; - const moveEdit = (element: TimelineElement, start: number): TimelineMoveEdit => ({ element, updates: { start, track: element.track }, @@ -48,7 +41,7 @@ function trimmedPlaybackStart(el: TimelineElement, sourceShift: number): number } interface PlacementPlanInput { - result: PlacementResult; + result: PlaceClipResult; mode: PlacementMode; laneClips: readonly TimelineElement[]; draggedEdit: TimelineMoveEdit; @@ -73,7 +66,7 @@ export function buildPlacementSteps({ const shiftEdits = result.shifts.map((s) => moveEdit(clip(s.key), round3(s.start))); const splits: PlacementStep[] = []; const removes: TimelineElement[] = []; - const resizes: PlacementResizeChange[] = []; + const resizes: TimelineGroupResizeChange[] = []; const settle: TimelineMoveEdit[] = []; for (const cut of result.cuts) { @@ -124,7 +117,7 @@ export function buildPlacementSteps({ /** Reason the whole drop must be refused, or null. Nothing is written when this is set. */ export function placementRefusal( - result: PlacementResult, + result: PlaceClipResult, mode: PlacementMode, laneClips: readonly TimelineElement[], ): string | null { @@ -146,7 +139,7 @@ let placementGestureSeq = 0; interface PlacementRunner { ops: PlacementOps; - resize: (changes: PlacementResizeChange[], fold: PlacementFold) => Promise | void; + resize: (changes: TimelineGroupResizeChange[], fold: PlacementFold) => Promise | void; move: (edits: TimelineMoveEdit[], fold: PlacementFold) => Promise; } @@ -201,7 +194,6 @@ export function commitPlacementDrop( const laneClips = elements.filter((e) => e.track === drag.previewTrack && keyOf(e) !== dragKey); const result = placeClip({ clips: laneClips.map((e) => ({ key: keyOf(e), start: e.start, duration: e.duration })), - track: drag.previewTrack, start: drag.previewStart, duration: drag.element.duration, mode, From 1be08257adfde0e57367303b1f1445d5a50c3c20 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 19 Sep 2026 00:19:49 -0400 Subject: [PATCH 11/16] fix(studio): a drop toasts on total failure and refuses a too-thin trim --- .../components/timelinePlacementCommit.test.ts | 14 ++++++++++++-- .../components/timelinePlacementCommit.ts | 17 ++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index 24ab9977d2..6ac4207c85 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -211,6 +211,16 @@ describe("placementRefusal", () => { }); expect(placementRefusal(split, "overwrite", [clip("a", 0, 4)])).toMatch(/split/); }); + + it("refuses a trim that would leave a sliver thinner than the split epsilon", () => { + const trim = result({ start: 0, cuts: [{ kind: "trim-tail", key: "a", duration: 0.01 }] }); + expect(placementRefusal(trim, "overwrite", [clip("a", 0, 4)])).toMatch(/thin/); + }); + + it("allows a trim that leaves at least the split epsilon", () => { + const trim = result({ start: 0, cuts: [{ kind: "trim-tail", key: "a", duration: 0.03 }] }); + expect(placementRefusal(trim, "overwrite", [clip("a", 0, 4)])).toBeNull(); + }); }); interface Doc { @@ -484,7 +494,7 @@ describe("runPlacementSteps failures", () => { expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); }); - it("says nothing extra when the first step fails, since nothing was applied", async () => { + it("toasts a plain failure, not a partial-apply message, when the first step fails", async () => { const toast = vi.fn(); const move = vi.fn(async () => true); await runPlacementSteps(steps, { @@ -492,7 +502,7 @@ describe("runPlacementSteps failures", () => { resize: vi.fn(), move, }); - expect(toast).not.toHaveBeenCalled(); + expect(toast).toHaveBeenCalledWith("Overwrite failed, nothing changed"); expect(move).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index 225eb548b8..05e4146eaa 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -8,7 +8,7 @@ import { hasSourcePlaybackOffset } from "./timelineGroupEditing"; import { placeClip, type PlacementMode, type PlaceClipResult } from "./timelinePlacement"; import { getTimelineElementIdentity as keyOf } from "../lib/timelineElementHelpers"; import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; -import { canSplitElementAt } from "../../utils/timelineElementSplit"; +import { canSplitElementAt, SPLIT_BOUNDARY_EPSILON_S } from "../../utils/timelineElementSplit"; /** One shared history key and an unbounded window: every write of a drop is one undo step. */ export interface PlacementFold { @@ -132,7 +132,14 @@ export function placementRefusal( const el = byKey.get(cut.key); return !el || !canSplitElementAt(el, at); }); - return unsplittable ? "Cannot split a clip at the drop point" : null; + if (unsplittable) return "Cannot split a clip at the drop point"; + // A trim below the split epsilon would leave a sliver too thin to select or re-split. + const tooThin = result.cuts.some( + (cut) => + (cut.kind === "trim-head" || cut.kind === "trim-tail") && + cut.duration < SPLIT_BOUNDARY_EPSILON_S, + ); + return tooThin ? "Cannot trim a clip that thin" : null; } let placementGestureSeq = 0; @@ -173,7 +180,11 @@ export async function runPlacementSteps( console.error("[Timeline] Overwrite step failed", error); } if (applied) continue; - if (index > 0) run.ops.toast("Overwrite partly applied, Undo restores it"); + run.ops.toast( + index > 0 + ? "Overwrite partly applied, Undo restores it" + : "Overwrite failed, nothing changed", + ); return; } } From 339cdaf6ed598e27276a97ecf8be42946ccc9f87 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 19 Sep 2026 05:03:35 -0400 Subject: [PATCH 12/16] fix(studio): a drop never shows the timeline an older state A clip drop holds preview manifests while it is in flight, writes the whole placement result to the timeline store in one update, and reloads the preview once at the end. Before, a manifest arriving mid-drop resurrected a clip the drop had just removed, and the drag preview fell back to the old state until the first step landed. Adds a test that mounts the real player hook and asserts the sequence of store states after a drop never returns to an older one. --- .../studio/src/hooks/useRazorSplit.test.ts | 59 ++++- packages/studio/src/hooks/useRazorSplit.ts | 38 +-- .../src/hooks/useTimelineDeleteOps.test.tsx | 39 ++- .../studio/src/hooks/useTimelineDeleteOps.ts | 5 +- .../studio/src/hooks/useTimelineEditing.ts | 3 +- .../components/timelineClipDragCommit.ts | 18 +- .../timelineClipDragGestureLifecycle.test.ts | 1 + .../timelinePlacementCommit.test.ts | 249 +++++++----------- .../components/timelinePlacementCommit.ts | 128 +++++++-- .../timelinePlacementMonotonic.test.ts | 155 +++++++++++ .../timelinePlacementTestHarness.ts | 162 ++++++++++++ .../src/player/hooks/useTimelinePlayer.ts | 7 +- .../src/player/lib/timelineManifestHold.ts | 16 ++ 13 files changed, 678 insertions(+), 202 deletions(-) create mode 100644 packages/studio/src/player/components/timelinePlacementMonotonic.test.ts create mode 100644 packages/studio/src/player/components/timelinePlacementTestHarness.ts create mode 100644 packages/studio/src/player/lib/timelineManifestHold.ts diff --git a/packages/studio/src/hooks/useRazorSplit.test.ts b/packages/studio/src/hooks/useRazorSplit.test.ts index 5d230fabb2..73f05109c4 100644 --- a/packages/studio/src/hooks/useRazorSplit.test.ts +++ b/packages/studio/src/hooks/useRazorSplit.test.ts @@ -9,6 +9,7 @@ import { createPersistentEditHistoryStore } from "./usePersistentEditHistory"; import { createEmptyEditHistory } from "../utils/editHistory"; import type { EditHistoryStorageAdapter } from "../utils/editHistoryStorage"; import { createSplitFetchMock, mountProbe } from "./useRazorSplit.testHelpers"; +import type { PlacementFold } from "../player/components/timelinePlacementCommit"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -49,11 +50,19 @@ interface SplitRequest { type SingleSplit = (element: TimelineElement, splitTime: number) => Promise; type SplitAll = (splitTime: number) => Promise; +type PlacementSplit = ( + element: TimelineElement, + splitTime: number, + fold: PlacementFold, +) => Promise; interface Harness { splitRequests: SplitRequest[]; singleRef: { current: SingleSplit | undefined }; allRef: { current: SplitAll | undefined }; + placementRef: { current: PlacementSplit | undefined }; + reloadPreview: ReturnType; + forceReloadSdkSession: ReturnType; root: ReturnType; } @@ -76,9 +85,12 @@ function mountRazorSplit(): Harness { const singleRef: { current: SingleSplit | undefined } = { current: undefined }; const allRef: { current: SplitAll | undefined } = { current: undefined }; + const placementRef: { current: PlacementSplit | undefined } = { current: undefined }; + const reloadPreview = vi.fn(); + const forceReloadSdkSession = vi.fn(); function Component() { - const { handleRazorSplit, handleRazorSplitAll } = useRazorSplit({ + const { handleRazorSplit, handleRazorSplitAll, handlePlacementSplit } = useRazorSplit({ projectId: "p1", activeCompPath: ROOT_FILE, showToast: () => {}, @@ -86,15 +98,25 @@ function mountRazorSplit(): Harness { disk[path] = content; }, recordEdit: async () => {}, - reloadPreview: () => {}, + reloadPreview, + forceReloadSdkSession, }); singleRef.current = handleRazorSplit; allRef.current = handleRazorSplitAll; + placementRef.current = handlePlacementSplit; return null; } const root = mountProbe(Component); - return { splitRequests, singleRef, allRef, root }; + return { + splitRequests, + singleRef, + allRef, + placementRef, + reloadPreview, + forceReloadSdkSession, + root, + }; } afterEach(() => { @@ -162,6 +184,37 @@ describe("useRazorSplit — sub-comp coordinate rebasing", () => { }); }); +// A drop reloads the preview once, at its end; an interior split must not reload on its own. +describe("useRazorSplit — preview reload folded into a drop", () => { + let harness: Harness; + beforeEach(() => { + harness = mountRazorSplit(); + }); + afterEach(() => { + act(() => harness.root.unmount()); + }); + + it("skips reloading the preview for a split folded into a drop, but still refreshes the sdk session", async () => { + await act(async () => { + await harness.placementRef.current!(rootElement, 4, { + coalesceKey: "clip-overwrite:1", + coalesceMs: Number.POSITIVE_INFINITY, + }); + }); + + expect(harness.reloadPreview).not.toHaveBeenCalled(); + expect(harness.forceReloadSdkSession).toHaveBeenCalledTimes(1); + }); + + it("still reloads the preview for a plain (non-drop) split", async () => { + await act(async () => { + await harness.singleRef.current!(rootElement, 4); + }); + + expect(harness.reloadPreview).toHaveBeenCalledTimes(1); + }); +}); + // ── Bug 1: split must resync the SDK session so undo isn't refused ──────────── const memoryStorage = (): EditHistoryStorageAdapter => { diff --git a/packages/studio/src/hooks/useRazorSplit.ts b/packages/studio/src/hooks/useRazorSplit.ts index 9bd1fe902f..ba1b41d67b 100644 --- a/packages/studio/src/hooks/useRazorSplit.ts +++ b/packages/studio/src/hooks/useRazorSplit.ts @@ -36,20 +36,28 @@ export function useRazorSplit({ const projectIdRef = useRef(projectId); projectIdRef.current = projectId; - const synchronize = useCallback(() => { - let failure: unknown; - try { - forceReloadSdkSession?.(); - } catch (error) { - failure = error; - } - try { - reloadPreview(); - } catch (error) { - failure ??= error; - } - if (failure) throw failure; - }, [forceReloadSdkSession, reloadPreview]); + // skipPreviewReload: true when this cut is one step folded into a drop, whose + // own single reload (after every step lands) replaces this one — otherwise the + // preview would show this step's DOM before the next step makes it stale. + const synchronize = useCallback( + (skipPreviewReload: boolean) => { + let failure: unknown; + try { + forceReloadSdkSession?.(); + } catch (error) { + failure = error; + } + if (!skipPreviewReload) { + try { + reloadPreview(); + } catch (error) { + failure ??= error; + } + } + if (failure) throw failure; + }, + [forceReloadSdkSession, reloadPreview], + ); const runCut = useCallback( async ( @@ -75,7 +83,7 @@ export function useRazorSplit({ writeProjectFile, recordEdit, observeProjectFileVersion, - synchronize, + synchronize: () => synchronize(Boolean(fold)), }); trackStudioRazorSplit({ mode, count: result.splitCount }); if (result.syncFailed) { diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx index 16c6f3c922..e71b974875 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx +++ b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx @@ -59,6 +59,8 @@ describe("useTimelineDeleteOps: ripple undo label", () => { handleTimelineGroupMove: DeleteOpsOptions["handleTimelineGroupMove"]; showToast?: DeleteOpsOptions["showToast"]; recordEdit?: DeleteOpsOptions["recordEdit"]; + reloadPreview?: DeleteOpsOptions["reloadPreview"]; + forceReloadSdkSession?: DeleteOpsOptions["forceReloadSdkSession"]; }) { const elements = [el("hf-a", 0, 2), el("hf-b", 2, 2), el("hf-c", 4, 2)]; let hook: ReturnType | null = null; @@ -70,7 +72,8 @@ describe("useTimelineDeleteOps: ripple undo label", () => { showToast: overrides.showToast ?? vi.fn(), writeProjectFile: vi.fn().mockResolvedValue(undefined), recordEdit: overrides.recordEdit ?? vi.fn().mockResolvedValue(undefined), - reloadPreview: vi.fn(), + reloadPreview: overrides.reloadPreview ?? vi.fn(), + forceReloadSdkSession: overrides.forceReloadSdkSession, previewIframeRef: { current: null }, handleTimelineGroupMove: overrides.handleTimelineGroupMove, }); @@ -145,4 +148,38 @@ describe("useTimelineDeleteOps: ripple undo label", () => { expect.objectContaining({ coalesceKey: "clip-overwrite:3", coalesceMs: Infinity }), ); }); + + // A drop reloads the preview once, at its end; an interior remove must not reload on its own. + it("skips reloading the preview for an overwrite delete, but still refreshes the sdk session", async () => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + const reloadPreview = vi.fn(); + const forceReloadSdkSession = vi.fn(); + const { b, getHook } = mountDeleteHarness({ + handleTimelineGroupMove, + reloadPreview, + forceReloadSdkSession, + }); + + await act(async () => { + await getHook().deleteTimelineElements([b], { + coalesceKey: "clip-overwrite:4", + coalesceMs: Number.POSITIVE_INFINITY, + }); + }); + + expect(reloadPreview).not.toHaveBeenCalled(); + expect(forceReloadSdkSession).toHaveBeenCalledTimes(1); + }); + + it("still reloads the preview for a plain (non-drop) delete", async () => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + const reloadPreview = vi.fn(); + const { b, getHook } = mountDeleteHarness({ handleTimelineGroupMove, reloadPreview }); + + await act(async () => { + await getHook().handleTimelineElementDelete(b); + }); + + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.ts b/packages/studio/src/hooks/useTimelineDeleteOps.ts index a1d2178bab..9b015611fa 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.ts +++ b/packages/studio/src/hooks/useTimelineDeleteOps.ts @@ -223,7 +223,10 @@ export function useTimelineDeleteOps({ usePlayerStore.getState().setSelectedElementIds(new Set()); } forceReloadSdkSession?.(); - reloadPreview(); + // Folded into a drop: the drop's own single reload (after every step + // lands) replaces this one, so an interior remove never shows the + // preview a state the next step is about to make stale. + if (!overwrite) reloadPreview(); // A failed ripple already showed its own toast above; the user did one // thing (delete), so they get one message, not this generic follow-up too. if (!rippleFailed && !overwrite) { diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index cba4f45eb6..063291f2ef 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -452,8 +452,9 @@ export function useTimelineEditing({ split: handlePlacementSplit, remove: deleteTimelineElements, toast: (message) => showToast(message, "error"), + reloadPreview, }), - [handlePlacementSplit, deleteTimelineElements, showToast], + [handlePlacementSplit, deleteTimelineElements, showToast, reloadPreview], ); return { diff --git a/packages/studio/src/player/components/timelineClipDragCommit.ts b/packages/studio/src/player/components/timelineClipDragCommit.ts index 6b7f615e60..71dc85e770 100644 --- a/packages/studio/src/player/components/timelineClipDragCommit.ts +++ b/packages/studio/src/player/components/timelineClipDragCommit.ts @@ -117,6 +117,8 @@ export function persistMoveEdits( coalesceKey?: string, operation: TimelineMoveOperation = "timing", coalesceMs?: number, + /** False when the caller already wrote the end state to the store (a placement drop does). */ + updateStore = true, ): Promise { if (edits.length === 0) return Promise.resolve(true); const { updateElement, onMoveElement, onMoveElements } = deps; @@ -150,7 +152,7 @@ export function persistMoveEdits( writtenTrack == null ? e.updates : { ...e.updates, authoredTrack: writtenTrack }, ); }; - for (const e of edits) applyEdit(e); + if (updateStore) for (const e of edits) applyEdit(e); // The store above gets DISPLAY lanes; the file below gets the authored-space // track when one was resolved (see TimelineMoveEdit.persistTrack). const persistEdits = edits.map((e) => @@ -167,6 +169,7 @@ export function persistMoveEdits( // restore the preview manifest's pre-gesture lane. Reassert the durable // result after persistence, but only while this remains the latest // optimistic gesture so an older save can never clobber a newer drag. + if (!updateStore) return true; for (const e of edits) { const key = keyOf(e.element); if (isLatestTimelineOptimisticGesture(updateElement, revision, key)) applyEdit(e); @@ -174,9 +177,15 @@ export function persistMoveEdits( return true; }, (error) => { - for (const p of prev) { - if (isLatestTimelineOptimisticGesture(updateElement, revision, p.key)) { - updateElement(p.key, { start: p.start, track: p.track, authoredTrack: p.authoredTrack }); + if (updateStore) { + for (const p of prev) { + if (isLatestTimelineOptimisticGesture(updateElement, revision, p.key)) { + updateElement(p.key, { + start: p.start, + track: p.track, + authoredTrack: p.authoredTrack, + }); + } } } console.error("[Timeline] Failed to persist clip edits", error); @@ -262,6 +271,7 @@ export function commitDraggedClipMove(rawDrag: DraggedClipState, deps: DragCommi fold.coalesceKey, edits.some((e) => e.updates.track !== e.element.track) ? "lane-reorder" : "timing", fold.coalesceMs, + false, ), deps, ); diff --git a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts index d389ff92e9..2e7540e193 100644 --- a/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts +++ b/packages/studio/src/player/components/timelineClipDragGestureLifecycle.test.ts @@ -189,6 +189,7 @@ describe("timeline clip drag gesture lifecycle", () => { split: vi.fn(async () => true), remove: vi.fn(async () => true), toast: vi.fn(), + reloadPreview: vi.fn(), }, }, }); diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index 6ac4207c85..c6ad394cf7 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -1,29 +1,13 @@ import { describe, expect, it, vi } from "vitest"; -import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; -import type { TimelineElement } from "../store/playerStore"; -import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; -import type { DraggedClipState } from "./timelineClipDragTypes"; +import type { TimelineMoveEdit } from "./timelineClipDragCommit"; import type { PlaceClipResult } from "./timelinePlacement"; import { buildPlacementSteps, commitPlacementDrop, placementRefusal, runPlacementSteps, - type PlacementFold, - type PlacementOps, } from "./timelinePlacementCommit"; -import { - buildEditHistoryEntry, - createEmptyEditHistory, - hashEditHistoryContent, - pushEditHistoryEntry, - undoEditHistory, - type EditHistoryState, -} from "../../utils/editHistory"; - -function clip(id: string, start: number, duration: number, extra: Partial = {}) { - return { id, domId: id, tag: "video", start, duration, track: 1, ...extra } as TimelineElement; -} +import { clip, createFakeProject, depsOf, dragOf, type Doc } from "./timelinePlacementTestHarness"; const dragged = clip("d", 20, 2); const draggedEdit = (start: number): TimelineMoveEdit => ({ @@ -223,144 +207,6 @@ describe("placementRefusal", () => { }); }); -interface Doc { - [id: string]: { start: number; duration: number; playbackStart?: number }; -} - -/** A tiny stand-in for the project file plus the history recorder, driven by the real reducer. */ -function createFakeProject(initial: Doc) { - let doc: Doc = structuredClone(initial); - let history: EditHistoryState = createEmptyEditHistory(); - let clock = 0; - const serialize = (d: Doc) => - JSON.stringify(Object.entries(d).sort(([x], [y]) => (x < y ? -1 : 1))); - const record = (mutate: () => void, label: string, fold: PlacementFold) => { - const before = serialize(doc); - mutate(); - history = pushEditHistoryEntry( - history, - buildEditHistoryEntry({ - id: `e${clock}`, - projectId: "p", - label, - kind: "timeline", - coalesceKey: fold.coalesceKey, - coalesceMs: fold.coalesceMs, - now: (clock += 1000), - files: { "index.html": { before, after: serialize(doc) } }, - }), - ); - }; - const current = (element: TimelineElement) => { - const found = doc[element.id]; - if (!found) throw new Error(`${element.id} is not in the document`); - // The element a step hands over must describe what the document holds right now. - expect({ start: element.start, duration: element.duration }).toEqual({ - start: found.start, - duration: found.duration, - }); - return found; - }; - const ops: PlacementOps = { - split: async (element, at, fold) => { - record( - () => { - const target = current(element); - const end = target.start + target.duration; - doc[`${element.id}-split`] = { - start: at, - duration: end - at, - playbackStart: (target.playbackStart ?? 0) + (at - target.start), - }; - target.duration = at - target.start; - }, - "split", - fold, - ); - return true; - }, - remove: async (elements, fold) => { - record( - () => { - for (const element of elements) { - current(element); - delete doc[element.id]; - } - }, - "delete", - fold, - ); - return true; - }, - toast: vi.fn(), - }; - const resize = async (changes: TimelineGroupResizeChange[], fold: PlacementFold) => { - record( - () => { - for (const change of changes) { - const target = current(change.element); - target.start = change.start; - target.duration = change.duration; - if (change.playbackStart != null) target.playbackStart = change.playbackStart; - } - }, - "resize", - fold, - ); - }; - const move = async (edits: TimelineMoveEdit[], fold: PlacementFold) => { - record( - () => { - for (const edit of edits) { - const found = doc[edit.element.id]; - if (found) found.start = edit.updates.start; - else - doc[edit.element.id] = { start: edit.updates.start, duration: edit.element.duration }; - } - }, - "move", - fold, - ); - return true; - }; - return { - ops, - resize, - move, - doc: () => doc, - history: () => history, - undo: () => { - const undone = undoEditHistory( - history, - { "index.html": hashEditHistoryContent(serialize(doc)) }, - 0, - ); - if (!undone.ok) throw new Error(`undo refused: ${undone.reason}`); - return undone.filesToWrite["index.html"]; - }, - serializeInitial: () => serialize(initial), - }; -} - -function dragOf(element: TimelineElement, previewStart: number, previewTrack = 1) { - return { - element, - previewStart, - previewTrack, - insertRow: null, - } as unknown as DraggedClipState; -} - -function depsOf(elements: TimelineElement[], project: ReturnType) { - return { - elements, - trackOrder: [1], - updateElement: vi.fn(), - placementOps: project.ops, - onResizeElements: project.resize, - } as unknown as DragCommitDeps; -} - describe("commitPlacementDrop: one undo step for the move and every cut", () => { // Lane 1: a [0,4) and b [4,8) video (b reads its source from 1s at rate 1), dragged d [20,22). const a = clip("a", 0, 4); @@ -487,9 +333,10 @@ describe("runPlacementSteps failures", () => { it("names a partial apply and stops when a later step fails", async () => { const toast = vi.fn(); await runPlacementSteps(steps, { - ops: { split: async () => true, remove: async () => true, toast }, + ops: { split: async () => true, remove: async () => true, toast, reloadPreview: vi.fn() }, resize: vi.fn(), move: async () => false, + applyToStore: vi.fn(), }); expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); }); @@ -498,9 +345,10 @@ describe("runPlacementSteps failures", () => { const toast = vi.fn(); const move = vi.fn(async () => true); await runPlacementSteps(steps, { - ops: { split: async () => false, remove: async () => true, toast }, + ops: { split: async () => false, remove: async () => true, toast, reloadPreview: vi.fn() }, resize: vi.fn(), move, + applyToStore: vi.fn(), }); expect(toast).toHaveBeenCalledWith("Overwrite failed, nothing changed"); expect(move).not.toHaveBeenCalled(); @@ -510,12 +358,95 @@ describe("runPlacementSteps failures", () => { const toast = vi.fn(); vi.spyOn(console, "error").mockImplementation(() => undefined); await runPlacementSteps(steps, { - ops: { split: async () => true, remove: async () => true, toast }, + ops: { split: async () => true, remove: async () => true, toast, reloadPreview: vi.fn() }, resize: vi.fn(), move: async () => { throw new Error("write failed"); }, + applyToStore: vi.fn(), }); expect(toast).toHaveBeenCalledWith("Overwrite partly applied, Undo restores it"); }); }); + +// A drop reloads the preview exactly once, after every step lands, never once per step. +describe("runPlacementSteps: one reload per drop, after every step lands", () => { + it("reloads exactly once, after the move step, when the drop contains a remove", async () => { + const calls: string[] = []; + const reloadPreview = vi.fn(() => calls.push("reload")); + const remove = vi.fn(async () => { + calls.push("remove"); + return true; + }); + const move = vi.fn(async () => { + calls.push("move"); + return true; + }); + await runPlacementSteps( + [ + { kind: "remove", elements: [clip("a", 0, 4)] }, + { kind: "move", edits: [] }, + ], + { + ops: { split: async () => true, remove, toast: vi.fn(), reloadPreview }, + resize: vi.fn(), + move, + applyToStore: vi.fn(), + }, + ); + expect(calls).toEqual(["remove", "move", "reload"]); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); + + it("reloads exactly once when the drop contains a split", async () => { + const reloadPreview = vi.fn(); + const split = vi.fn(async () => true); + await runPlacementSteps( + [ + { kind: "split", element: clip("a", 0, 4), at: 2 }, + { kind: "move", edits: [] }, + ], + { + ops: { split, remove: async () => true, toast: vi.fn(), reloadPreview }, + resize: vi.fn(), + move: async () => true, + applyToStore: vi.fn(), + }, + ); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); + + it("never reloads a move/resize-only drop (a plain trim stays flash-free)", async () => { + const reloadPreview = vi.fn(); + await runPlacementSteps( + [ + { kind: "resize", changes: [] }, + { kind: "move", edits: [] }, + ], + { + ops: { split: async () => true, remove: async () => true, toast: vi.fn(), reloadPreview }, + resize: vi.fn(async () => undefined), + move: async () => true, + applyToStore: vi.fn(), + }, + ); + expect(reloadPreview).not.toHaveBeenCalled(); + }); + + it("reloads once when a step failed, because the store was already told the end state", async () => { + const reloadPreview = vi.fn(); + await runPlacementSteps( + [ + { kind: "remove", elements: [clip("a", 0, 4)] }, + { kind: "move", edits: [] }, + ], + { + ops: { split: async () => true, remove: async () => false, toast: vi.fn(), reloadPreview }, + resize: vi.fn(), + move: async () => true, + applyToStore: vi.fn(), + }, + ); + expect(reloadPreview).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index 05e4146eaa..f3a51300df 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -1,4 +1,5 @@ -import type { TimelineElement } from "../store/playerStore"; +import { usePlayerStore, type TimelineElement } from "../store/playerStore"; +import { holdTimelineManifests } from "../lib/timelineManifestHold"; import type { DraggedClipState } from "./timelineClipDragTypes"; import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; import { canMoveTimelineElement } from "./timelineAuthoredMoveTarget"; @@ -21,6 +22,8 @@ export interface PlacementOps { split: (element: TimelineElement, at: number, fold: PlacementFold) => Promise; remove: (elements: TimelineElement[], fold: PlacementFold) => Promise; toast: (message: string) => void; + /** The one full-reload point for a drop; split/remove skip their own while folded. */ + reloadPreview: () => void; } export type PlacementStep = @@ -148,6 +151,8 @@ interface PlacementRunner { ops: PlacementOps; resize: (changes: TimelineGroupResizeChange[], fold: PlacementFold) => Promise | void; move: (edits: TimelineMoveEdit[], fold: PlacementFold) => Promise; + /** Puts the drop's end state in the store before any write starts. */ + applyToStore: (steps: readonly PlacementStep[]) => void; } function runStep(step: PlacementStep, fold: PlacementFold, run: PlacementRunner) { @@ -163,7 +168,87 @@ function runStep(step: PlacementStep, fold: PlacementFold, run: PlacementRunner) } } -/** Each step awaits the previous: the history fold needs every write to start from the last one's output. */ +/** The clip a split will add, under a stand-in id until the reload reports the real one. */ +function pendingSplitTail(el: TimelineElement, at: number): TimelineElement { + const playbackStart = hasSourcePlaybackOffset(el) + ? round3((el.playbackStart ?? 0) + (at - el.start) * (el.playbackRate ?? 1)) + : el.playbackStart; + return { + ...el, + id: `${el.id}~tail`, + key: `${keyOf(el)}~tail`, + domId: undefined, + start: at, + duration: round3(el.start + el.duration - at), + playbackStart, + }; +} + +function moveUpdate(edit: TimelineMoveEdit): Partial { + const written = + edit.persistTrack ?? + (edit.updates.track !== edit.element.track ? edit.updates.track : undefined); + return written == null ? edit.updates : { ...edit.updates, authoredTrack: written }; +} + +function resizeUpdate(change: TimelineGroupResizeChange): Partial { + return { + start: change.start, + duration: change.duration, + ...(change.playbackStart != null ? { playbackStart: change.playbackStart } : {}), + }; +} + +/** What one step changes in the store: keyed updates, removed keys, and clips it adds. */ +function stepEffects(step: PlacementStep): { + updates: Array<[string, Partial]>; + removed: string[]; + added: TimelineElement[]; +} { + switch (step.kind) { + case "remove": + return { updates: [], removed: step.elements.map(keyOf), added: [] }; + case "move": + return { + updates: step.edits.map((edit) => [keyOf(edit.element), moveUpdate(edit)]), + removed: [], + added: [], + }; + case "resize": + return { + updates: step.changes.map((change) => [keyOf(change.element), resizeUpdate(change)]), + removed: [], + added: [], + }; + case "split": + return { + updates: [[keyOf(step.element), { duration: round3(step.at - step.element.start) }]], + removed: [], + added: [pendingSplitTail(step.element, step.at)], + }; + } +} + +/** The drop's end state, written to the store in one go so no in-between state is ever shown. */ +function applyPlacementToStore(steps: readonly PlacementStep[]): void { + const removed = new Set(); + const updates = new Map>(); + const tails: TimelineElement[] = []; + for (const effects of steps.map(stepEffects)) { + for (const key of effects.removed) removed.add(key); + for (const [key, next] of effects.updates) updates.set(key, { ...updates.get(key), ...next }); + tails.push(...effects.added); + } + const { elements, setElements } = usePlayerStore.getState(); + setElements([ + ...elements + .filter((el) => !removed.has(keyOf(el))) + .map((el) => ({ ...el, ...updates.get(keyOf(el)) })), + ...tails, + ]); +} + +/** Steps run in order, each from the last write; preview manifests stay held until the final reload. */ export async function runPlacementSteps( steps: readonly PlacementStep[], run: PlacementRunner, @@ -172,20 +257,32 @@ export async function runPlacementSteps( coalesceKey: `clip-overwrite:${placementGestureSeq++}`, coalesceMs: Number.POSITIVE_INFINITY, }; - for (const [index, step] of steps.entries()) { - let applied = false; - try { - applied = (await runStep(step, fold, run)) !== false; - } catch (error) { - console.error("[Timeline] Overwrite step failed", error); + const release = holdTimelineManifests(); + try { + run.applyToStore(steps); + for (const [index, step] of steps.entries()) { + let applied = false; + try { + applied = (await runStep(step, fold, run)) !== false; + } catch (error) { + console.error("[Timeline] Overwrite step failed", error); + } + if (applied) continue; + run.ops.toast( + index > 0 + ? "Overwrite partly applied, Undo restores it" + : "Overwrite failed, nothing changed", + ); + // The store was told the drop's end state; only a reload puts back what disk holds. + run.ops.reloadPreview(); + return; + } + // remove/split write to disk directly and skip their own reload while folded here. + if (steps.some((step) => step.kind === "remove" || step.kind === "split")) { + run.ops.reloadPreview(); } - if (applied) continue; - run.ops.toast( - index > 0 - ? "Overwrite partly applied, Undo restores it" - : "Overwrite failed, nothing changed", - ); - return; + } finally { + release(); } } @@ -228,5 +325,6 @@ export function commitPlacementDrop( ops: placementOps, resize: (changes, fold) => onResizeElements(changes, fold), move, + applyToStore: applyPlacementToStore, }); } diff --git a/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts new file mode 100644 index 0000000000..89ffacdc56 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts @@ -0,0 +1,155 @@ +// @vitest-environment happy-dom + +import React, { act, useEffect } from "react"; +import { createRoot } from "react-dom/client"; +import { runtimeProtocolMetadata } from "@hyperframes/core/runtime/protocol"; +import { afterEach, describe, expect, it } from "vitest"; +import { useTimelinePlayer } from "../hooks/useTimelinePlayer"; +import { usePlayerStore } from "../store/playerStore"; +import { commitPlacementDrop } from "./timelinePlacementCommit"; +import { clip, createFakeProject, depsOf, dragOf } from "./timelinePlacementTestHarness"; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +function Harness() { + useTimelinePlayer(); + useEffect(() => undefined); + return null; +} + +function mountPlayer() { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => root.render(React.createElement(Harness))); + return () => act(() => root.unmount()); +} + +const manifestClip = (id: string, start: number, duration: number) => ({ + id, + label: id, + start, + duration, + track: 1, + kind: "element", + tagName: "div", + compositionId: null, + parentCompositionId: null, + compositionSrc: null, + assetUrl: null, +}); + +// The preview iframe keeps the pre-drop DOM until the drop's one reload, so its manifest lists the +// clip the drop already removed. It used to land mid-drop and put that clip back on the timeline. +function postStalePreviewManifest() { + window.dispatchEvent( + new MessageEvent("message", { + data: { + source: "hf-preview", + type: "timeline", + clips: [manifestClip("a", 0, 4), manifestClip("b", 4, 4), manifestClip("d", 20, 6)], + durationInFrames: 900, + fps: 30, + ...runtimeProtocolMetadata(30), + }, + }), + ); +} + +function recordStoreStates( + format: (el: { id: string; start: number; duration: number }) => string, +) { + const states: string[] = []; + const stop = usePlayerStore.subscribe((state) => + states.push(state.elements.map(format).join(" ")), + ); + return { states, stop }; +} + +afterEach(() => { + document.body.innerHTML = ""; + usePlayerStore.getState().reset(); +}); + +describe("a drop's timeline states never go back to an older one", () => { + it("keeps the removed clip gone when the old preview posts its manifest mid-drop", async () => { + const unmount = mountPlayer(); + const a = clip("a", 0, 4); + const b = clip("b", 4, 4); + const covering = clip("d", 20, 6); + const preDrop = [a, b, covering]; + usePlayerStore.getState().setElements(preDrop); + const project = createFakeProject({ + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4 }, + d: { start: 20, duration: 6 }, + }); + const remove = project.ops.remove; + project.ops.remove = async (elements, fold) => { + const done = await remove(elements, fold); + act(() => postStalePreviewManifest()); + return done; + }; + const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}`); + + await commitPlacementDrop( + dragOf(covering, 4), + depsOf(preDrop, project), + "overwrite", + project.move, + ); + stop(); + + expect(states.length).toBeGreaterThan(0); + expect(states.every((state) => !state.includes("b@"))).toBe(true); + expect(states.at(-1)).toContain("d@4"); + unmount(); + }); + + it("shows a split's head, the dropped clip and the new tail in the first state after the drop", async () => { + const a = clip("a", 0, 4); + const b = clip("b", 4, 4, { playbackStart: 1 }); + const dropped = clip("d", 20, 2); + const lane = [a, b, dropped]; + usePlayerStore.getState().setElements(lane); + const project = createFakeProject({ + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4, playbackStart: 1 }, + d: { start: 20, duration: 2 }, + }); + const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}+${el.duration}`); + + await commitPlacementDrop(dragOf(dropped, 1), depsOf(lane, project), "overwrite", project.move); + stop(); + + // d [1,3) inside a [0,4): head [0,1), tail [3,4), all in the very first store write. + expect(states[0]).toBe("a@0+1 b@4+4 d@1+2 a~tail@3+1"); + }); + + it("shows an insert's pushed clips, split head and tail in the first state after the drop", async () => { + const a = clip("a", 0, 4); + const b = clip("b", 4, 4); + const dropped = clip("d", 20, 2); + const lane = [a, b, dropped]; + usePlayerStore.getState().setElements(lane); + const project = createFakeProject({ + a: { start: 0, duration: 4 }, + b: { start: 4, duration: 4 }, + d: { start: 20, duration: 2 }, + }); + const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}+${el.duration}`); + + await commitPlacementDrop(dragOf(dropped, 2), depsOf(lane, project), "insert", project.move); + stop(); + + // d [2,4) inserted into a [0,4): head [0,2), tail [4,6), b pushed to [6,10). + expect(states[0]).toBe("a@0+2 b@6+4 d@2+2 a~tail@4+2"); + }); + + it("takes the preview's manifest again once the drop is over", () => { + const unmount = mountPlayer(); + act(() => postStalePreviewManifest()); + expect(usePlayerStore.getState().elements.map((el) => el.id)).toEqual(["a", "b", "d"]); + unmount(); + }); +}); diff --git a/packages/studio/src/player/components/timelinePlacementTestHarness.ts b/packages/studio/src/player/components/timelinePlacementTestHarness.ts new file mode 100644 index 0000000000..14fd74f834 --- /dev/null +++ b/packages/studio/src/player/components/timelinePlacementTestHarness.ts @@ -0,0 +1,162 @@ +import { expect, vi } from "vitest"; +import type { TimelineGroupResizeChange } from "../../hooks/useTimelineGroupEditing"; +import type { TimelineElement } from "../store/playerStore"; +import type { DragCommitDeps, TimelineMoveEdit } from "./timelineClipDragCommit"; +import type { DraggedClipState } from "./timelineClipDragTypes"; +import type { PlacementFold, PlacementOps } from "./timelinePlacementCommit"; +import { + buildEditHistoryEntry, + createEmptyEditHistory, + hashEditHistoryContent, + pushEditHistoryEntry, + undoEditHistory, + type EditHistoryState, +} from "../../utils/editHistory"; + +export function clip( + id: string, + start: number, + duration: number, + extra: Partial = {}, +) { + return { id, domId: id, tag: "video", start, duration, track: 1, ...extra } as TimelineElement; +} + +export interface Doc { + [id: string]: { start: number; duration: number; playbackStart?: number }; +} + +/** A tiny stand-in for the project file plus the history recorder, driven by the real reducer. */ +export function createFakeProject(initial: Doc) { + let doc: Doc = structuredClone(initial); + let history: EditHistoryState = createEmptyEditHistory(); + let clock = 0; + const serialize = (d: Doc) => + JSON.stringify(Object.entries(d).sort(([x], [y]) => (x < y ? -1 : 1))); + const record = (mutate: () => void, label: string, fold: PlacementFold) => { + const before = serialize(doc); + mutate(); + history = pushEditHistoryEntry( + history, + buildEditHistoryEntry({ + id: `e${clock}`, + projectId: "p", + label, + kind: "timeline", + coalesceKey: fold.coalesceKey, + coalesceMs: fold.coalesceMs, + now: (clock += 1000), + files: { "index.html": { before, after: serialize(doc) } }, + }), + ); + }; + const current = (element: TimelineElement) => { + const found = doc[element.id]; + if (!found) throw new Error(`${element.id} is not in the document`); + // The element a step hands over must describe what the document holds right now. + expect({ start: element.start, duration: element.duration }).toEqual({ + start: found.start, + duration: found.duration, + }); + return found; + }; + const ops: PlacementOps = { + split: async (element, at, fold) => { + record( + () => { + const target = current(element); + const end = target.start + target.duration; + doc[`${element.id}-split`] = { + start: at, + duration: end - at, + playbackStart: (target.playbackStart ?? 0) + (at - target.start), + }; + target.duration = at - target.start; + }, + "split", + fold, + ); + return true; + }, + remove: async (elements, fold) => { + record( + () => { + for (const element of elements) { + current(element); + delete doc[element.id]; + } + }, + "delete", + fold, + ); + return true; + }, + toast: vi.fn(), + reloadPreview: vi.fn(), + }; + const resize = async (changes: TimelineGroupResizeChange[], fold: PlacementFold) => { + record( + () => { + for (const change of changes) { + const target = current(change.element); + target.start = change.start; + target.duration = change.duration; + if (change.playbackStart != null) target.playbackStart = change.playbackStart; + } + }, + "resize", + fold, + ); + }; + const move = async (edits: TimelineMoveEdit[], fold: PlacementFold) => { + record( + () => { + for (const edit of edits) { + const found = doc[edit.element.id]; + if (found) found.start = edit.updates.start; + else + doc[edit.element.id] = { start: edit.updates.start, duration: edit.element.duration }; + } + }, + "move", + fold, + ); + return true; + }; + return { + ops, + resize, + move, + doc: () => doc, + history: () => history, + undo: () => { + const undone = undoEditHistory( + history, + { "index.html": hashEditHistoryContent(serialize(doc)) }, + 0, + ); + if (!undone.ok) throw new Error(`undo refused: ${undone.reason}`); + return undone.filesToWrite["index.html"]; + }, + serializeInitial: () => serialize(initial), + }; +} + +export function dragOf(element: TimelineElement, previewStart: number, previewTrack = 1) { + return { + element, + previewStart, + previewTrack, + insertRow: null, + } as unknown as DraggedClipState; +} + +export function depsOf(elements: TimelineElement[], project: ReturnType) { + return { + elements, + trackOrder: [1], + updateElement: vi.fn(), + placementOps: project.ops, + onResizeElements: project.resize, + } as unknown as DragCommitDeps; +} diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index 7794be1b04..f7fca7faed 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -40,6 +40,7 @@ import { normalizeToZones } from "../components/timelineZones"; import { applyPreviewAudioFlags, setPreviewPlaybackRate } from "../lib/timelineIframeHelpers"; import { scrubMusicAtSeek, stopScrubPreviewAudio } from "../lib/playbackScrub"; import { hasTimelinePerformanceFixtureLease } from "../lib/timelinePerformanceFixture"; +import { timelineManifestsHeld } from "../lib/timelineManifestHold"; import { applyCachedSourceDurations, probeMissingSourceDurations } from "../lib/mediaProbe"; import { shouldResumeForwardPlaybackAfterSeek, shouldStopAfterSeek } from "../lib/playbackSeek"; import { applyPreviewVariablesToUrl } from "../../hooks/previewVariablesStore"; @@ -64,13 +65,13 @@ export function useTimelinePlayer() { const { setIsPlaying, setCurrentTime, setDuration, requestTimelineReady, setElements } = usePlayerStore.getState(); - // The fixture lease belongs at this shared synchronization boundary so every - // iframe discovery path has the same owner for deciding whether it may write. + // The fixture lease and the drop's manifest hold belong at this shared synchronization + // boundary so every iframe discovery path has the same owner for deciding whether it may write. const syncTimelineElements = useCallback( // The lease guard adds one deliberate branch at the shared synchronization boundary. // fallow-ignore-next-line complexity (elements: TimelineElement[], nextDuration?: number) => { - if (hasTimelinePerformanceFixtureLease()) return; + if (hasTimelinePerformanceFixtureLease() || timelineManifestsHeld()) return; const state = usePlayerStore.getState(); const resolvedDuration = nextDuration ?? state.duration; // applyCachedSourceDurations re-applies the cached probe duration: re-derived diff --git a/packages/studio/src/player/lib/timelineManifestHold.ts b/packages/studio/src/player/lib/timelineManifestHold.ts new file mode 100644 index 0000000000..d1e0c5b739 --- /dev/null +++ b/packages/studio/src/player/lib/timelineManifestHold.ts @@ -0,0 +1,16 @@ +let holds = 0; + +/** While held, clip manifests from the preview are ignored; release() lets the next one in. */ +export function holdTimelineManifests(): () => void { + holds += 1; + let released = false; + return () => { + if (released) return; + released = true; + holds -= 1; + }; +} + +export function timelineManifestsHeld(): boolean { + return holds > 0; +} From 68090ee15f7ce3ade0a004c0704c1fcc762a30d4 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 19 Sep 2026 07:01:19 -0400 Subject: [PATCH 13/16] fix(studio): editing a clip while playing keeps the playhead running The soft reload after a timing edit re-seeked the preview to the store's current time, which while playing is the last seek and not the playhead. The preview jumped back and stopped, with the button still showing Pause. It now re-seeks to the live player time and keeps playing when the player is running. --- packages/studio/src/utils/gsapSoftReload.test.ts | 8 ++++++++ packages/studio/src/utils/gsapSoftReload.ts | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/utils/gsapSoftReload.test.ts b/packages/studio/src/utils/gsapSoftReload.test.ts index c2178e5f85..cea072baa7 100644 --- a/packages/studio/src/utils/gsapSoftReload.test.ts +++ b/packages/studio/src/utils/gsapSoftReload.test.ts @@ -105,6 +105,14 @@ describe("applySoftReload", () => { expect(contentWindow.__hfStudioManualEditsApply).toHaveBeenCalled(); }); + it("keeps the live playhead and playback running when the reload lands while playing", () => { + // The store's currentTime is the last seek, not the playhead, while playing. + const { iframe, contentWindow } = buildMockIframe(); + contentWindow.__player.isPlaying = () => true; + applySoftReloadFinalization(iframe, 0.5); + expect(contentWindow.__player.seek).toHaveBeenCalledWith(2.0, { keepPlaying: true }); + }); + it("seeks to the caller-supplied currentTime override instead of the iframe's own __player.getTime()", () => { // Regression: the iframe's raw __player.getTime() (2.0 here, per the mock) // can desync from the studio's authoritative scrub position — e.g. a diff --git a/packages/studio/src/utils/gsapSoftReload.ts b/packages/studio/src/utils/gsapSoftReload.ts index 71b624276d..547caa0a2a 100644 --- a/packages/studio/src/utils/gsapSoftReload.ts +++ b/packages/studio/src/utils/gsapSoftReload.ts @@ -3,7 +3,11 @@ import { applyAuthoredInlineOpacity, readStampedAuthoredOpacity } from "./author type IframeWindow = Window & { __timelines?: Record void; pause?: () => void }>; - __player?: { getTime?: () => number; seek?: (t: number) => void }; + __player?: { + getTime?: () => number; + seek?: (t: number, options?: { keepPlaying?: boolean }) => void; + isPlaying?: () => boolean; + }; __hfForceTimelineRebind?: () => void; __hfSuppressSceneMutations?: (fn: () => T) => T; __hfStudioManualEditsApply?: () => void; @@ -195,7 +199,13 @@ export interface SoftReloadOptions { * no-op if the timeline already reports being at that time internally. */ function finalizeSoftReload(win: IframeWindow, currentTime: number): void { - win.__player?.seek?.(currentTime); + // While playing the store's time is the last seek, not the playhead: re-seek to the live time and keep playing. + const player = win.__player; + if (player?.isPlaying?.() === true) { + player.seek?.(player.getTime?.() ?? currentTime, { keepPlaying: true }); + } else { + player?.seek?.(currentTime); + } win.__hfForceTimelineRebind?.(); win.__hfStudioManualEditsApply?.(); } From 46e916b81da692521673bde904526ebe00ce1174 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 19 Sep 2026 07:53:53 -0400 Subject: [PATCH 14/16] refactor(studio): share test setup in the placement drop tests Fold the repeated runPlacementSteps deps, the one-undo-step check and the two first-state drop tests into helpers. Shorten one narrating comment. --- .../timelinePlacementCommit.test.ts | 115 +++++++----------- .../timelinePlacementMonotonic.test.ts | 27 ++-- .../src/player/hooks/useTimelinePlayer.ts | 3 +- 3 files changed, 54 insertions(+), 91 deletions(-) diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index c6ad394cf7..5760289a48 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -218,6 +218,11 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => d: { start: 20, duration: 2 }, }; + function expectOneUndoStep(project: ReturnType) { + expect(project.history().undo).toHaveLength(1); + expect(project.undo()).toBe(project.serializeInitial()); + } + async function drop(at: number, mode: "overwrite" | "insert" = "overwrite", lane = [a, b]) { const project = createFakeProject(start); const placed = commitPlacementDrop( @@ -239,8 +244,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => b: { start: 6, duration: 2, playbackStart: 3 }, d: { start: 4, duration: 2 }, }); - expect(project.history().undo).toHaveLength(1); - expect(project.undo()).toBe(project.serializeInitial()); + expectOneUndoStep(project); }); it("removed: a clip the drop fully covers is deleted and the drop lands", async () => { @@ -256,8 +260,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => a: { start: 0, duration: 4 }, d: { start: 4, duration: 6 }, }); - expect(project.history().undo).toHaveLength(1); - expect(project.undo()).toBe(project.serializeInitial()); + expectOneUndoStep(project); }); it("trimmed tail: the drop over a tail shortens the clip underneath", async () => { @@ -265,8 +268,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => // d [2,4): a loses [2,4) so it ends at 2. expect(project.doc().a).toEqual({ start: 0, duration: 2 }); expect(project.doc().d).toEqual({ start: 2, duration: 2 }); - expect(project.history().undo).toHaveLength(1); - expect(project.undo()).toBe(project.serializeInitial()); + expectOneUndoStep(project); }); it("split: a drop inside a clip leaves a head and a tail, one undo removes the tail too", async () => { @@ -278,8 +280,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => "a-split": { start: 3, duration: 1, playbackStart: 3 }, d: { start: 1, duration: 2 }, }); - expect(project.history().undo).toHaveLength(1); - expect(project.undo()).toBe(project.serializeInitial()); + expectOneUndoStep(project); }); it("insert: what follows is pushed right and the straddled clip is split around the drop", async () => { @@ -291,8 +292,7 @@ describe("commitPlacementDrop: one undo step for the move and every cut", () => "b-split": { start: 8, duration: 2, playbackStart: 3 }, d: { start: 6, duration: 2 }, }); - expect(project.history().undo).toHaveLength(1); - expect(project.undo()).toBe(project.serializeInitial()); + expectOneUndoStep(project); }); it("refuses the whole drop when a clip it would cut is locked, writing nothing", async () => { @@ -370,83 +370,60 @@ describe("runPlacementSteps failures", () => { }); // A drop reloads the preview exactly once, after every step lands, never once per step. +function reloadDeps( + over: { split?: () => Promise; remove?: () => Promise } = {}, +) { + const reloadPreview = vi.fn(); + const ops = { + split: over.split ?? (async () => true), + remove: over.remove ?? (async () => true), + toast: vi.fn(), + reloadPreview, + }; + return { + reloadPreview, + deps: { ops, resize: vi.fn(), move: async () => true, applyToStore: vi.fn() }, + }; +} +const removeA = { kind: "remove", elements: [clip("a", 0, 4)] } as const; +const emptyMove = { kind: "move", edits: [] } as const; + describe("runPlacementSteps: one reload per drop, after every step lands", () => { it("reloads exactly once, after the move step, when the drop contains a remove", async () => { const calls: string[] = []; - const reloadPreview = vi.fn(() => calls.push("reload")); - const remove = vi.fn(async () => { - calls.push("remove"); - return true; - }); - const move = vi.fn(async () => { - calls.push("move"); - return true; + const { reloadPreview, deps } = reloadDeps({ + remove: async () => { + calls.push("remove"); + return true; + }, }); - await runPlacementSteps( - [ - { kind: "remove", elements: [clip("a", 0, 4)] }, - { kind: "move", edits: [] }, - ], - { - ops: { split: async () => true, remove, toast: vi.fn(), reloadPreview }, - resize: vi.fn(), - move, - applyToStore: vi.fn(), + reloadPreview.mockImplementation(() => calls.push("reload")); + await runPlacementSteps([removeA, emptyMove], { + ...deps, + move: async () => { + calls.push("move"); + return true; }, - ); + }); expect(calls).toEqual(["remove", "move", "reload"]); expect(reloadPreview).toHaveBeenCalledTimes(1); }); it("reloads exactly once when the drop contains a split", async () => { - const reloadPreview = vi.fn(); - const split = vi.fn(async () => true); - await runPlacementSteps( - [ - { kind: "split", element: clip("a", 0, 4), at: 2 }, - { kind: "move", edits: [] }, - ], - { - ops: { split, remove: async () => true, toast: vi.fn(), reloadPreview }, - resize: vi.fn(), - move: async () => true, - applyToStore: vi.fn(), - }, - ); + const { reloadPreview, deps } = reloadDeps(); + await runPlacementSteps([{ kind: "split", element: clip("a", 0, 4), at: 2 }, emptyMove], deps); expect(reloadPreview).toHaveBeenCalledTimes(1); }); it("never reloads a move/resize-only drop (a plain trim stays flash-free)", async () => { - const reloadPreview = vi.fn(); - await runPlacementSteps( - [ - { kind: "resize", changes: [] }, - { kind: "move", edits: [] }, - ], - { - ops: { split: async () => true, remove: async () => true, toast: vi.fn(), reloadPreview }, - resize: vi.fn(async () => undefined), - move: async () => true, - applyToStore: vi.fn(), - }, - ); + const { reloadPreview, deps } = reloadDeps(); + await runPlacementSteps([{ kind: "resize", changes: [] }, emptyMove], deps); expect(reloadPreview).not.toHaveBeenCalled(); }); it("reloads once when a step failed, because the store was already told the end state", async () => { - const reloadPreview = vi.fn(); - await runPlacementSteps( - [ - { kind: "remove", elements: [clip("a", 0, 4)] }, - { kind: "move", edits: [] }, - ], - { - ops: { split: async () => true, remove: async () => false, toast: vi.fn(), reloadPreview }, - resize: vi.fn(), - move: async () => true, - applyToStore: vi.fn(), - }, - ); + const { reloadPreview, deps } = reloadDeps({ remove: async () => false }); + await runPlacementSteps([removeA, emptyMove], deps); expect(reloadPreview).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts index 89ffacdc56..736c05070e 100644 --- a/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts +++ b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts @@ -106,7 +106,7 @@ describe("a drop's timeline states never go back to an older one", () => { unmount(); }); - it("shows a split's head, the dropped clip and the new tail in the first state after the drop", async () => { + async function firstStateAfterDrop(mode: "overwrite" | "insert", at: number) { const a = clip("a", 0, 4); const b = clip("b", 4, 4, { playbackStart: 1 }); const dropped = clip("d", 20, 2); @@ -118,32 +118,19 @@ describe("a drop's timeline states never go back to an older one", () => { d: { start: 20, duration: 2 }, }); const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}+${el.duration}`); - - await commitPlacementDrop(dragOf(dropped, 1), depsOf(lane, project), "overwrite", project.move); + await commitPlacementDrop(dragOf(dropped, at), depsOf(lane, project), mode, project.move); stop(); + return states[0]; + } + it("shows a split's head, the dropped clip and the new tail in the first state after the drop", async () => { // d [1,3) inside a [0,4): head [0,1), tail [3,4), all in the very first store write. - expect(states[0]).toBe("a@0+1 b@4+4 d@1+2 a~tail@3+1"); + expect(await firstStateAfterDrop("overwrite", 1)).toBe("a@0+1 b@4+4 d@1+2 a~tail@3+1"); }); it("shows an insert's pushed clips, split head and tail in the first state after the drop", async () => { - const a = clip("a", 0, 4); - const b = clip("b", 4, 4); - const dropped = clip("d", 20, 2); - const lane = [a, b, dropped]; - usePlayerStore.getState().setElements(lane); - const project = createFakeProject({ - a: { start: 0, duration: 4 }, - b: { start: 4, duration: 4 }, - d: { start: 20, duration: 2 }, - }); - const { states, stop } = recordStoreStates((el) => `${el.id}@${el.start}+${el.duration}`); - - await commitPlacementDrop(dragOf(dropped, 2), depsOf(lane, project), "insert", project.move); - stop(); - // d [2,4) inserted into a [0,4): head [0,2), tail [4,6), b pushed to [6,10). - expect(states[0]).toBe("a@0+2 b@6+4 d@2+2 a~tail@4+2"); + expect(await firstStateAfterDrop("insert", 2)).toBe("a@0+2 b@6+4 d@2+2 a~tail@4+2"); }); it("takes the preview's manifest again once the drop is over", () => { diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index f7fca7faed..aab453cfa0 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -65,8 +65,7 @@ export function useTimelinePlayer() { const { setIsPlaying, setCurrentTime, setDuration, requestTimelineReady, setElements } = usePlayerStore.getState(); - // The fixture lease and the drop's manifest hold belong at this shared synchronization - // boundary so every iframe discovery path has the same owner for deciding whether it may write. + // Every iframe discovery path funnels through here, so the fixture lease and manifest hold live here. const syncTimelineElements = useCallback( // The lease guard adds one deliberate branch at the shared synchronization boundary. // fallow-ignore-next-line complexity From 8e706c46346b375814191c9ef5153f88532649f5 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 19 Sep 2026 08:28:38 -0400 Subject: [PATCH 15/16] fix(studio): an overwrite drop no longer rewrites the timeline from an old copy The remove step of a drop rebuilt the timeline from the clips it saw when the gesture began, so the dragged clip flashed back at its old position a moment after release. A delete folded into a drop now leaves the store to the drop, which already wrote the end state. A drop onto a split tail that the reload has not reported yet is refused, since that clip only exists under a stand-in id. --- .../src/hooks/useTimelineDeleteOps.test.tsx | 23 ++++++++++++++++++- .../studio/src/hooks/useTimelineDeleteOps.ts | 3 ++- .../timelinePlacementCommit.test.ts | 5 ++++ .../components/timelinePlacementCommit.ts | 10 ++++++-- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx index e71b974875..5b8a3629b7 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx +++ b/packages/studio/src/hooks/useTimelineDeleteOps.test.tsx @@ -2,7 +2,7 @@ import { act } from "react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { TimelineElement } from "../player"; +import { usePlayerStore, type TimelineElement } from "../player"; import { applyRippleShifts, useTimelineDeleteOps } from "./useTimelineDeleteOps"; import { installReactActEnvironment, mountReactHarness } from "./domSelectionTestHarness"; @@ -182,4 +182,25 @@ describe("useTimelineDeleteOps: ripple undo label", () => { expect(reloadPreview).toHaveBeenCalledTimes(1); }); + + it("an overwrite delete leaves the store to the drop, a plain delete rewrites it", async () => { + const handleTimelineGroupMove = vi.fn().mockResolvedValue(undefined); + const dropEndState = [el("hf-a", 1, 1), el("hf-c", 6, 2)]; + const { b, getHook } = mountDeleteHarness({ handleTimelineGroupMove }); + + usePlayerStore.getState().setElements(dropEndState); + await act(async () => { + await getHook().deleteTimelineElements([b], { + coalesceKey: "clip-overwrite:5", + coalesceMs: Number.POSITIVE_INFINITY, + }); + }); + expect(usePlayerStore.getState().elements).toEqual(dropEndState); + + await act(async () => { + await getHook().handleTimelineElementDelete(b); + }); + expect(usePlayerStore.getState().elements.map((e) => e.id)).toEqual(["hf-a", "hf-c"]); + expect(usePlayerStore.getState().elements).not.toEqual(dropEndState); + }); }); diff --git a/packages/studio/src/hooks/useTimelineDeleteOps.ts b/packages/studio/src/hooks/useTimelineDeleteOps.ts index 9b015611fa..67c5c9bdd1 100644 --- a/packages/studio/src/hooks/useTimelineDeleteOps.ts +++ b/packages/studio/src/hooks/useTimelineDeleteOps.ts @@ -217,8 +217,9 @@ export function useTimelineDeleteOps({ } } - usePlayerStore.getState().setElements(applyRippleShifts(survivors, rippleApplied)); if (!overwrite) { + // A folded delete leaves the store to the drop, which already wrote its end state. + usePlayerStore.getState().setElements(applyRippleShifts(survivors, rippleApplied)); usePlayerStore.getState().setSelectedElementId(null); usePlayerStore.getState().setSelectedElementIds(new Set()); } diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index 5760289a48..355525a482 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -168,6 +168,11 @@ describe("placementRefusal", () => { ).toMatch(/locked or expanded/); }); + it("refuses a clip still standing in for a split tail the reload has not reported", () => { + const cutTail = result({ start: 0, cuts: [{ kind: "remove", key: "a~tail" }] }); + expect(placementRefusal(cutTail, "overwrite", [clip("a~tail", 0, 4)])).toMatch(/previous edit/); + }); + it("refuses when a clip that would be cut is an expanded child", () => { expect( placementRefusal(cutA, "overwrite", [clip("a", 0, 4, { expandedParentStart: 2 })]), diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index f3a51300df..70992a0329 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -32,6 +32,9 @@ export type PlacementStep = | { kind: "resize"; changes: TimelineGroupResizeChange[] } | { kind: "move"; edits: TimelineMoveEdit[] }; +/** Marks the stand-in id of a split tail that the reload has not reported yet. */ +const PENDING_TAIL_SUFFIX = "~tail"; + const moveEdit = (element: TimelineElement, start: number): TimelineMoveEdit => ({ element, updates: { start, track: element.track }, @@ -129,6 +132,9 @@ export function placementRefusal( if (touched.some((el) => !el || !canMoveTimelineElement(el) || el.expandedParentStart != null)) { return "Cannot overwrite a locked or expanded clip"; } + if (touched.some((el) => el && keyOf(el).endsWith(PENDING_TAIL_SUFFIX))) { + return "Wait for the previous edit to finish"; + } const unsplittable = result.cuts.some((cut) => { if (cut.kind !== "split") return false; const at = mode === "overwrite" ? cut.tail.start : result.start; @@ -175,8 +181,8 @@ function pendingSplitTail(el: TimelineElement, at: number): TimelineElement { : el.playbackStart; return { ...el, - id: `${el.id}~tail`, - key: `${keyOf(el)}~tail`, + id: `${el.id}${PENDING_TAIL_SUFFIX}`, + key: `${keyOf(el)}${PENDING_TAIL_SUFFIX}`, domId: undefined, start: at, duration: round3(el.start + el.duration - at), From 95e3404c7b773ff27936085fe4e8f5897999bd3f Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 19 Sep 2026 09:23:52 -0400 Subject: [PATCH 16/16] fix(studio): mark a not-yet-reloaded split tail with a field, not an id suffix The drop refusal matched clips whose key ended in "~tail", so an author id ending that way could never be overwritten. The stand-in now carries an explicit awaitingReload flag, and the dragged clip is checked too, so dragging a stand-in is refused with the same message. --- .../timelinePlacementCommit.test.ts | 22 +++++++++++++++++-- .../components/timelinePlacementCommit.ts | 16 ++++++++------ .../timelinePlacementMonotonic.test.ts | 6 +++++ .../src/player/store/timelineElement.ts | 2 ++ 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/studio/src/player/components/timelinePlacementCommit.test.ts b/packages/studio/src/player/components/timelinePlacementCommit.test.ts index 355525a482..9b8ca509ff 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.test.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.test.ts @@ -169,8 +169,26 @@ describe("placementRefusal", () => { }); it("refuses a clip still standing in for a split tail the reload has not reported", () => { - const cutTail = result({ start: 0, cuts: [{ kind: "remove", key: "a~tail" }] }); - expect(placementRefusal(cutTail, "overwrite", [clip("a~tail", 0, 4)])).toMatch(/previous edit/); + const standIn = clip("a", 0, 4, { awaitingReload: true }); + const cutBoth = result({ + start: 0, + cuts: [ + { kind: "remove", key: "a" }, + { kind: "remove", key: "b" }, + ], + }); + expect(placementRefusal(cutBoth, "overwrite", [standIn, clip("b", 4, 4)])).toMatch( + /previous edit/, + ); + }); + + it("refuses dragging a stand-in itself, and allows an author id that ends in ~tail", () => { + const standIn = clip("t", 8, 2, { awaitingReload: true }); + expect(placementRefusal(cutA, "overwrite", [clip("a", 0, 4)], standIn)).toMatch( + /previous edit/, + ); + const cutAuthored = result({ start: 0, cuts: [{ kind: "remove", key: "a~tail" }] }); + expect(placementRefusal(cutAuthored, "overwrite", [clip("a~tail", 0, 4)])).toBeNull(); }); it("refuses when a clip that would be cut is an expanded child", () => { diff --git a/packages/studio/src/player/components/timelinePlacementCommit.ts b/packages/studio/src/player/components/timelinePlacementCommit.ts index 70992a0329..cafb7b4dbc 100644 --- a/packages/studio/src/player/components/timelinePlacementCommit.ts +++ b/packages/studio/src/player/components/timelinePlacementCommit.ts @@ -32,8 +32,7 @@ export type PlacementStep = | { kind: "resize"; changes: TimelineGroupResizeChange[] } | { kind: "move"; edits: TimelineMoveEdit[] }; -/** Marks the stand-in id of a split tail that the reload has not reported yet. */ -const PENDING_TAIL_SUFFIX = "~tail"; +const WAIT_FOR_RELOAD = "Wait for the previous edit to finish"; const moveEdit = (element: TimelineElement, start: number): TimelineMoveEdit => ({ element, @@ -126,14 +125,16 @@ export function placementRefusal( result: PlaceClipResult, mode: PlacementMode, laneClips: readonly TimelineElement[], + dragged?: TimelineElement, ): string | null { const byKey = new Map(laneClips.map((e) => [keyOf(e), e])); + if (dragged?.awaitingReload) return WAIT_FOR_RELOAD; const touched = [...result.cuts, ...result.shifts].map((c) => byKey.get(c.key)); if (touched.some((el) => !el || !canMoveTimelineElement(el) || el.expandedParentStart != null)) { return "Cannot overwrite a locked or expanded clip"; } - if (touched.some((el) => el && keyOf(el).endsWith(PENDING_TAIL_SUFFIX))) { - return "Wait for the previous edit to finish"; + if (touched.some((el) => el?.awaitingReload)) { + return WAIT_FOR_RELOAD; } const unsplittable = result.cuts.some((cut) => { if (cut.kind !== "split") return false; @@ -181,8 +182,9 @@ function pendingSplitTail(el: TimelineElement, at: number): TimelineElement { : el.playbackStart; return { ...el, - id: `${el.id}${PENDING_TAIL_SUFFIX}`, - key: `${keyOf(el)}${PENDING_TAIL_SUFFIX}`, + id: `${el.id}~tail`, + key: `${keyOf(el)}~tail`, + awaitingReload: true, domId: undefined, start: at, duration: round3(el.start + el.duration - at), @@ -314,7 +316,7 @@ export function commitPlacementDrop( }); if (result.cuts.length === 0 && result.shifts.length === 0) return null; - const refusal = placementRefusal(result, mode, laneClips); + const refusal = placementRefusal(result, mode, laneClips, drag.element); if (refusal) { placementOps.toast(refusal); return Promise.resolve(); diff --git a/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts index 736c05070e..7e5e0ddc09 100644 --- a/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts +++ b/packages/studio/src/player/components/timelinePlacementMonotonic.test.ts @@ -128,6 +128,12 @@ describe("a drop's timeline states never go back to an older one", () => { expect(await firstStateAfterDrop("overwrite", 1)).toBe("a@0+1 b@4+4 d@1+2 a~tail@3+1"); }); + it("marks the new tail as awaiting the reload so a second drop on it is refused", async () => { + await firstStateAfterDrop("overwrite", 1); + const tail = usePlayerStore.getState().elements.find((el) => el.id === "a~tail"); + expect(tail?.awaitingReload).toBe(true); + }); + it("shows an insert's pushed clips, split head and tail in the first state after the drop", async () => { // d [2,4) inserted into a [0,4): head [0,2), tail [4,6), b pushed to [6,10). expect(await firstStateAfterDrop("insert", 2)).toBe("a@0+2 b@6+4 d@2+2 a~tail@4+2"); diff --git a/packages/studio/src/player/store/timelineElement.ts b/packages/studio/src/player/store/timelineElement.ts index 09532bdd58..bfb377db6a 100644 --- a/packages/studio/src/player/store/timelineElement.ts +++ b/packages/studio/src/player/store/timelineElement.ts @@ -83,6 +83,8 @@ export interface TimelineElement { * the child's local (sourceFile-relative) time. Works at any nesting depth. */ expandedParentStart?: number; + /** A clip a drop wrote to the store that the next preview reload has not reported yet. */ + awaitingReload?: true; expandedHostKey?: string; }