mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 09:16:55 +00:00
fix(studio): dropped timeline assets land where you drop them (#4062)
* fix(studio): dropped timeline assets land where you drop them Dropping an asset, an OS file, or a block onto the timeline always placed the new clip at the current playhead time, discarding the actual drop position; only the track (vertical position) tracked the pointer. Composition drops already used the correct pointer position. Unify all drop kinds onto that same, correct placement path, and drop the now-pointless duration clamp so a drop past the end of the timeline extends it, matching what composition drops already did. * fix(studio): grow the timeline when a drop lands past its end Asset/file and composition drops could place a clip past the root composition's duration without growing it, so the editor showed the clip fine but the render truncated it. Every sibling edit path (block install, move, resize) already grows the duration on the same rule; these two didn't. Also restores the pointer-placement decision's provenance in the drop docblock and PR description, and marks the now-production-dead clamp branch in resolveTimelineAssetDrop so the next reader doesn't assume the default is live. * fix(studio): revert the composition-drop duration extension The composition-drop path already grows the root composition's duration server-side (compositionInsertion.ts, before the response is even returned): this was verified wrong via two independent review passes after being merged in by mistake. The client-side extend added on top of that was redundant, and in edge cases could re-apply a slightly different rounding than the server's own. * fix(studio): remove the dead clampStartToDuration option resolveTimelineAssetDrop had one production caller and it always passed clampStartToDuration: false; the clamped branch and the duration field that only fed it were exercised solely by this file's own tests, not by anything shipping. Drop the option, the branch, and duration/durationRef everywhere they were only feeding it (timelineDragDrop.ts, Timeline.tsx, and the touched test fixtures). No test asserted a drop past duration relying on clamping, so this is behavior-neutral for every existing test.
This commit is contained in:
@@ -0,0 +1,72 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import React, { act } from "react";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { useTimelineAssetDropOps } from "./useTimelineAssetDropOps";
|
||||
import { mountReactHarness } from "./domSelectionTestHarness";
|
||||
|
||||
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
type DropFn = ReturnType<typeof useTimelineAssetDropOps>["handleTimelineAssetDrop"];
|
||||
|
||||
function renderDropHook(
|
||||
sourceContent: string,
|
||||
writeProjectFile: (path: string, content: string, expectedContent?: string) => Promise<void>,
|
||||
) {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue({ ok: true, json: async () => ({ content: sourceContent }) }),
|
||||
);
|
||||
let drop: DropFn | null = null;
|
||||
function Harness() {
|
||||
const { handleTimelineAssetDrop } = useTimelineAssetDropOps({
|
||||
projectIdRef: { current: "project" },
|
||||
activeCompPath: "index.html",
|
||||
timelineElements: [],
|
||||
showToast: vi.fn(),
|
||||
writeProjectFile,
|
||||
recordEdit: vi.fn().mockResolvedValue(undefined),
|
||||
reloadPreview: vi.fn(),
|
||||
uploadProjectFiles: vi.fn().mockResolvedValue([]),
|
||||
});
|
||||
drop = handleTimelineAssetDrop;
|
||||
return null;
|
||||
}
|
||||
mountReactHarness(<Harness />);
|
||||
return () => drop!;
|
||||
}
|
||||
|
||||
describe("useTimelineAssetDropOps handleTimelineAssetDrop", () => {
|
||||
it("grows the root duration when the drop lands past the current end", async () => {
|
||||
const source =
|
||||
'<main data-composition-id="scene" data-duration="10" data-width="1920" data-height="1080"></main>';
|
||||
const writeProjectFile = vi.fn().mockResolvedValue(undefined);
|
||||
const getDrop = renderDropHook(source, writeProjectFile);
|
||||
|
||||
await act(async () => {
|
||||
await getDrop()("clip.mp4", { start: 8, track: 0 }, 5);
|
||||
});
|
||||
|
||||
const [, written] = writeProjectFile.mock.calls[0] as [string, string];
|
||||
expect(written).toContain('data-duration="13"');
|
||||
});
|
||||
|
||||
it("leaves the root duration alone when the drop lands inside the current end", async () => {
|
||||
const source =
|
||||
'<main data-composition-id="scene" data-duration="10" data-width="1920" data-height="1080"></main>';
|
||||
const writeProjectFile = vi.fn().mockResolvedValue(undefined);
|
||||
const getDrop = renderDropHook(source, writeProjectFile);
|
||||
|
||||
await act(async () => {
|
||||
await getDrop()("clip.mp4", { start: 1, track: 0 }, 2);
|
||||
});
|
||||
|
||||
const [, written] = writeProjectFile.mock.calls[0] as [string, string];
|
||||
expect(written).toContain('data-duration="10"');
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,7 @@ import { collectHtmlIds, resolveDroppedAssetDuration } from "../utils/studioHelp
|
||||
import { formatTimelineAttributeNumber } from "./timelineEditingHelpers";
|
||||
import { readFileContent } from "./timelineTimingSync";
|
||||
import { commitTimelineCompositionInsertion } from "../utils/timelineCompositionInsert";
|
||||
import { extendRootDurationInSource } from "../utils/rootDuration";
|
||||
import { usePlayerStore } from "../player";
|
||||
|
||||
interface UseTimelineAssetDropOpsOptions {
|
||||
@@ -89,22 +90,25 @@ export function useTimelineAssetDropOps({
|
||||
);
|
||||
const newElementZIndex = Math.max(1, relevantElements.length + 1);
|
||||
|
||||
const patchedContent = insertTimelineAssetIntoSource(
|
||||
originalContent,
|
||||
buildTimelineAssetInsertHtml({
|
||||
id: newId,
|
||||
hfId: `hf-${generateId()}`,
|
||||
assetPath: resolvedAssetSrc,
|
||||
kind,
|
||||
start: normalizedStart,
|
||||
duration: normalizedDuration,
|
||||
track: placement.track,
|
||||
zIndex: newElementZIndex,
|
||||
geometry: fitTimelineAssetGeometry(
|
||||
null,
|
||||
resolveTimelineAssetCompositionSize(originalContent),
|
||||
),
|
||||
}),
|
||||
const patchedContent = extendRootDurationInSource(
|
||||
insertTimelineAssetIntoSource(
|
||||
originalContent,
|
||||
buildTimelineAssetInsertHtml({
|
||||
id: newId,
|
||||
hfId: `hf-${generateId()}`,
|
||||
assetPath: resolvedAssetSrc,
|
||||
kind,
|
||||
start: normalizedStart,
|
||||
duration: normalizedDuration,
|
||||
track: placement.track,
|
||||
zIndex: newElementZIndex,
|
||||
geometry: fitTimelineAssetGeometry(
|
||||
null,
|
||||
resolveTimelineAssetCompositionSize(originalContent),
|
||||
),
|
||||
}),
|
||||
),
|
||||
normalizedStart + normalizedDuration,
|
||||
);
|
||||
|
||||
await saveProjectFilesWithHistory({
|
||||
|
||||
@@ -166,7 +166,6 @@ describe("Timeline provider boundary", () => {
|
||||
scrollTop: 0,
|
||||
contentOrigin: GUTTER,
|
||||
pixelsPerSecond: 100,
|
||||
duration: 60,
|
||||
trackOrder: [0],
|
||||
},
|
||||
1132,
|
||||
@@ -282,7 +281,6 @@ describe("Timeline provider boundary", () => {
|
||||
scrollTop: 0,
|
||||
contentOrigin: LABEL_COL_W + GUTTER,
|
||||
pixelsPerSecond: 100,
|
||||
duration: 60,
|
||||
trackOrder: [0],
|
||||
},
|
||||
100 + LABEL_COL_W + GUTTER + 1000,
|
||||
@@ -1263,7 +1261,6 @@ describe("resolveTimelineAssetDrop", () => {
|
||||
scrollTop: 0,
|
||||
contentOrigin: GUTTER,
|
||||
pixelsPerSecond: 100,
|
||||
duration: 10,
|
||||
trackHeight: 72,
|
||||
trackOrder: [0, 3, 7],
|
||||
},
|
||||
@@ -1285,7 +1282,6 @@ describe("resolveTimelineAssetDrop", () => {
|
||||
scrollTop: 0,
|
||||
contentOrigin: GUTTER,
|
||||
pixelsPerSecond: 100,
|
||||
duration: 10,
|
||||
trackHeight: 72,
|
||||
trackOrder: [0, 3, 7],
|
||||
},
|
||||
|
||||
@@ -238,7 +238,6 @@ export const Timeline = memo(function Timeline({
|
||||
const assetDrop = useTimelineAssetDrop({
|
||||
scrollRef,
|
||||
ppsRef,
|
||||
durationRef,
|
||||
trackOrderRef,
|
||||
rowGeometryRef,
|
||||
contentOrigin,
|
||||
|
||||
@@ -56,7 +56,6 @@ function renderHarness(
|
||||
api = useTimelineAssetDrop({
|
||||
scrollRef: { current: scroll },
|
||||
ppsRef: { current: 40 },
|
||||
durationRef: { current: 120 },
|
||||
trackOrderRef: { current: tracks },
|
||||
rowGeometryRef: { current: geometry },
|
||||
contentOrigin: 0,
|
||||
@@ -138,7 +137,6 @@ describe("useTimelineAssetDrop", () => {
|
||||
it("drops once on a model row outside the mounted window and appends below the last row", () => {
|
||||
const onAssetDrop = vi.fn();
|
||||
const view = renderHarness(onAssetDrop);
|
||||
usePlayerStore.getState().setCurrentTime(12.5);
|
||||
view.scroll.scrollTop = view.scroll.scrollHeight - view.scroll.clientHeight;
|
||||
const transfer = assetTransfer(JSON.stringify({ path: "/media/hero.mp4" }));
|
||||
|
||||
@@ -148,11 +146,29 @@ describe("useTimelineAssetDrop", () => {
|
||||
});
|
||||
|
||||
expect(onAssetDrop).toHaveBeenCalledTimes(1);
|
||||
expect(onAssetDrop).toHaveBeenCalledWith("/media/hero.mp4", { start: 12.5, track: 100 });
|
||||
// pps=40, clientX=400 -> 10s at the pointer, not the playhead.
|
||||
expect(onAssetDrop).toHaveBeenCalledWith("/media/hero.mp4", { start: 10, track: 100 });
|
||||
expect(view.api.isDragOver).toBe(false);
|
||||
act(() => view.root.unmount());
|
||||
});
|
||||
|
||||
it("places the drop at the pointer x, ignoring the playhead", () => {
|
||||
const onAssetDrop = vi.fn();
|
||||
const view = renderHarness(onAssetDrop);
|
||||
usePlayerStore.getState().setCurrentTime(50);
|
||||
const transfer = assetTransfer(JSON.stringify({ path: "/media/hero.mp4" }));
|
||||
|
||||
act(() => {
|
||||
view.api.handleAssetDragOver(dragEvent(transfer, 80, 100));
|
||||
view.api.handleAssetDrop(dragEvent(transfer, 80, 100));
|
||||
});
|
||||
|
||||
// pps=40, clientX=80 -> 2s, far from the 50s playhead: proves start tracks
|
||||
// the drop position, not usePlayerStore.currentTime.
|
||||
expect(onAssetDrop).toHaveBeenCalledWith("/media/hero.mp4", { start: 2, track: 0 });
|
||||
act(() => view.root.unmount());
|
||||
});
|
||||
|
||||
it("ignores malformed payloads and clears the actor on project reset", () => {
|
||||
const onAssetDrop = vi.fn();
|
||||
const view = renderHarness(onAssetDrop, 1);
|
||||
@@ -190,7 +206,8 @@ describe("useTimelineAssetDrop", () => {
|
||||
});
|
||||
|
||||
expect(onAssetDrop).not.toHaveBeenCalled();
|
||||
expect(onBlockDrop).toHaveBeenCalledExactlyOnceWith("title-card", { start: 0, track: 0 });
|
||||
// pps=40, clientX=400 -> 10s at the pointer.
|
||||
expect(onBlockDrop).toHaveBeenCalledExactlyOnceWith("title-card", { start: 10, track: 0 });
|
||||
act(() => view.root.unmount());
|
||||
});
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@ import {
|
||||
parseTimelineCompositionPayload,
|
||||
TIMELINE_COMPOSITION_MIME,
|
||||
} from "../../utils/timelineCompositionDrop";
|
||||
import { usePlayerStore } from "../store/playerStore";
|
||||
import { resolveTimelineAssetDrop, type TimelineRowGeometry } from "./timelineLayout";
|
||||
import type { TimelineDropCallbacks } from "./timelineCallbacks";
|
||||
import {
|
||||
@@ -15,7 +14,6 @@ import {
|
||||
interface UseTimelineAssetDropOptions extends TimelineDropCallbacks {
|
||||
scrollRef: RefObject<HTMLDivElement | null>;
|
||||
ppsRef: RefObject<number>;
|
||||
durationRef: RefObject<number>;
|
||||
trackOrderRef: RefObject<number[]>;
|
||||
rowGeometryRef: RefObject<TimelineRowGeometry>;
|
||||
contentOrigin: number;
|
||||
@@ -53,11 +51,6 @@ function invokeDropCallback(callback: () => Promise<void> | void): void {
|
||||
}
|
||||
}
|
||||
|
||||
function resolveDropStart(usePointerStart: boolean, pointerStart: number): number {
|
||||
if (usePointerStart) return pointerStart;
|
||||
return Math.max(0, usePlayerStore.getState().currentTime);
|
||||
}
|
||||
|
||||
function applyFileDrop(
|
||||
transfer: DataTransfer,
|
||||
onFileDrop: TimelineDropCallbacks["onFileDrop"],
|
||||
@@ -87,17 +80,14 @@ function applyTypedJsonDrop(
|
||||
}
|
||||
|
||||
/**
|
||||
* Dropping an asset/file/block onto the timeline places it at the PLAYHEAD —
|
||||
* start is the current playhead time, only the track comes from the drop y.
|
||||
* Deliberate product choice (user preference, 2026-07-09): every add lands at
|
||||
* the playhead regardless of drop x, like CapCut's add-to-timeline. External
|
||||
* OS file drops and internal asset drops share this same placement path, so
|
||||
* both land identically.
|
||||
* Dropping an asset/file/block/composition onto the timeline places it at the
|
||||
* exact time and track it was dropped on, like CapCut (pointer placement on
|
||||
* every track but the magnetic main track). Supersedes the prior playhead
|
||||
* decision (#2291); playhead adds stay available, see useAddAssetAtPlayhead.
|
||||
*/
|
||||
export function useTimelineAssetDrop({
|
||||
scrollRef,
|
||||
ppsRef,
|
||||
durationRef,
|
||||
trackOrderRef,
|
||||
rowGeometryRef,
|
||||
contentOrigin,
|
||||
@@ -185,10 +175,10 @@ export function useTimelineAssetDrop({
|
||||
);
|
||||
|
||||
const resolveDropPlacement = useCallback(
|
||||
(clientX: number, clientY: number, usePointerStart = false): TimelinePlacement => {
|
||||
(clientX: number, clientY: number): TimelinePlacement => {
|
||||
const scroll = scrollRef.current;
|
||||
const rect = scroll?.getBoundingClientRect();
|
||||
const pointer = resolveTimelineAssetDrop(
|
||||
return resolveTimelineAssetDrop(
|
||||
{
|
||||
rectLeft: rect?.left ?? 0,
|
||||
rectTop: rect?.top ?? 0,
|
||||
@@ -196,20 +186,14 @@ export function useTimelineAssetDrop({
|
||||
scrollTop: scroll?.scrollTop ?? 0,
|
||||
contentOrigin,
|
||||
pixelsPerSecond: ppsRef.current,
|
||||
duration: durationRef.current,
|
||||
clampStartToDuration: !usePointerStart,
|
||||
rowHeights: rowGeometryRef.current.rowHeights,
|
||||
trackOrder: trackOrderRef.current,
|
||||
},
|
||||
clientX,
|
||||
clientY,
|
||||
);
|
||||
return {
|
||||
start: resolveDropStart(usePointerStart, pointer.start),
|
||||
track: pointer.track,
|
||||
};
|
||||
},
|
||||
[scrollRef, ppsRef, durationRef, trackOrderRef, rowGeometryRef, contentOrigin],
|
||||
[scrollRef, ppsRef, trackOrderRef, rowGeometryRef, contentOrigin],
|
||||
);
|
||||
|
||||
const handleAssetDrop = useCallback(
|
||||
@@ -218,15 +202,15 @@ export function useTimelineAssetDrop({
|
||||
const canCommit = activeDropEpochRef.current === sessionEpoch;
|
||||
clearDropPreview();
|
||||
if (!canCommit) return;
|
||||
const placement = resolveDropPlacement(e.clientX, e.clientY);
|
||||
|
||||
const compositionPayload = parseTimelineCompositionPayload(
|
||||
e.dataTransfer.getData(TIMELINE_COMPOSITION_MIME),
|
||||
);
|
||||
if (compositionPayload && onCompositionDrop) {
|
||||
const placement = resolveDropPlacement(e.clientX, e.clientY, true);
|
||||
invokeDropCallback(() => onCompositionDrop(compositionPayload.sourcePath, placement));
|
||||
return;
|
||||
}
|
||||
const placement = resolveDropPlacement(e.clientX, e.clientY);
|
||||
|
||||
if (applyFileDrop(e.dataTransfer, onFileDrop, placement)) return;
|
||||
if (applyTypedJsonDrop(e.dataTransfer, TIMELINE_ASSET_MIME, "path", onAssetDrop, placement)) {
|
||||
|
||||
@@ -217,7 +217,6 @@ describe("track-area breathing pad y-math", () => {
|
||||
scrollTop: 0,
|
||||
contentOrigin: GUTTER,
|
||||
pixelsPerSecond: 100,
|
||||
duration: 60,
|
||||
rowHeights: baseRows(3),
|
||||
trackOrder: [0, 1, 2],
|
||||
};
|
||||
|
||||
@@ -568,8 +568,6 @@ export function resolveTimelineAssetDrop(
|
||||
scrollTop: number;
|
||||
contentOrigin: number;
|
||||
pixelsPerSecond: number;
|
||||
duration: number;
|
||||
clampStartToDuration?: boolean;
|
||||
rowHeights?: readonly number[];
|
||||
trackOrder: number[];
|
||||
},
|
||||
@@ -584,10 +582,7 @@ export function resolveTimelineAssetDrop(
|
||||
});
|
||||
const contentY = clientY - input.rectTop + input.scrollTop;
|
||||
const pointerStart = Math.round((x / Math.max(input.pixelsPerSecond, 1)) * 100) / 100;
|
||||
const start = Math.max(
|
||||
0,
|
||||
input.clampStartToDuration === false ? pointerStart : Math.min(input.duration, pointerStart),
|
||||
);
|
||||
const start = Math.max(0, pointerStart);
|
||||
// Row from the shared row→y inverse so the top pad is honoured; a drop in the
|
||||
// pad above the first lane floors to row 0, a drop in the bottom pad rounds
|
||||
// past the last lane (getDefaultDroppedTrack then appends a new track).
|
||||
|
||||
Reference in New Issue
Block a user