fix(studio): start timeline drops on an empty lane at 0 and snap to clip edges (#4811)

* fix(studio): start timeline drops on an empty lane at 0 and snap to clip edges

A file, asset, block or composition dropped on an empty lane, a new lane, or an empty timeline now starts at 0 instead of the pointer time. On a lane with clips the start snaps to the nearest clip edge within TIMELINE_SNAP_PX. The drop preview and the committed drop share the placement.

* fix(studio): keep the drop file's comment share flat

* fix(studio): a drop snaps to clip edges only while the timeline magnet is on
This commit is contained in:
Miguel Ángel
2026-10-01 03:42:15 +00:00
committed by GitHub
parent 6ba35c9e3b
commit af314cbc3d
3 changed files with 91 additions and 13 deletions
@@ -4,7 +4,7 @@ import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi, type Mock } from "vitest";
import { TIMELINE_ASSET_MIME, TIMELINE_BLOCK_MIME } from "../../utils/timelineAssetDrop";
import { usePlayerStore } from "../store/playerStore";
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
import { createTimelineRowGeometry } from "./timelineLayout";
import { resolveDropInsertRow, useTimelineAssetDrop } from "./timelineDragDrop";
import { getTimelineRowTop, TRACK_H } from "./timelineLayout";
@@ -39,10 +39,19 @@ function assetTransfer(payload: string): DropTransfer {
};
}
// One far-off clip per lane, so a drop keeps its pointer time instead of an empty lane's 0.
const FAR_CLIPS: TimelineElement[] = Array.from({ length: 100 }, (_, track) => ({
id: `far-${track}`,
tag: "div",
start: 900,
duration: 1,
track,
}));
function renderHarness(
onAssetDrop: Mock,
sessionEpoch = 1,
options: { onBlockDrop?: Mock; strict?: boolean } = {},
options: { onBlockDrop?: Mock; strict?: boolean; elements?: TimelineElement[] } = {},
) {
const tracks = Array.from({ length: 100 }, (_, index) => index);
const geometry = createTimelineRowGeometry(
@@ -60,6 +69,7 @@ function renderHarness(
scrollRef: { current: scroll },
ppsRef: { current: 40 },
trackOrderRef: { current: tracks },
elementsRef: { current: options.elements ?? FAR_CLIPS },
rowGeometryRef: { current: geometry },
contentOrigin: 0,
sessionEpoch: epoch,
@@ -149,8 +159,8 @@ describe("useTimelineAssetDrop", () => {
});
expect(onAssetDrop).toHaveBeenCalledTimes(1);
// pps=40, clientX=400 -> 10s at the pointer, not the playhead.
expect(onAssetDrop).toHaveBeenCalledWith("/media/hero.mp4", { start: 10, track: 100 });
// The appended lane is empty, so the drop starts at 0.
expect(onAssetDrop).toHaveBeenCalledWith("/media/hero.mp4", { start: 0, track: 100 });
expect(view.api.isDragOver).toBe(false);
act(() => view.root.unmount());
});
@@ -256,6 +266,52 @@ describe("useTimelineAssetDrop", () => {
});
});
describe("useTimelineAssetDrop start", () => {
function dropAt(elements: TimelineElement[], clientX: number) {
const onAssetDrop = vi.fn();
const view = renderHarness(onAssetDrop, 1, { elements });
const transfer = assetTransfer(JSON.stringify({ path: "assets/a.mp4" }));
act(() => view.api.handleAssetDragOver(dragEvent(transfer, clientX, ROW0_MID_Y)));
const preview = view.api.dropPreview;
act(() => view.api.handleAssetDrop(dragEvent(transfer, clientX, ROW0_MID_Y)));
act(() => view.root.unmount());
return { preview, committed: onAssetDrop.mock.calls[0]?.[1] };
}
it("starts a drop on an empty timeline at 0, in the preview and the commit", () => {
// pps=40, clientX=80 -> 2s at the pointer.
const { preview, committed } = dropAt([], 80);
expect(preview).toEqual({ start: 0, track: 0 });
expect(committed).toEqual(preview);
});
it("starts a drop on an empty lane at 0 while other lanes hold clips", () => {
const { committed } = dropAt([{ id: "c", tag: "div", start: 0, duration: 5, track: 1 }], 80);
expect(committed).toEqual({ start: 0, track: 0 });
});
it("snaps a drop within TIMELINE_SNAP_PX of a clip edge onto that edge", () => {
const lane: TimelineElement[] = [{ id: "c", tag: "div", start: 0, duration: 5, track: 0 }];
// The clip ends at x=200; 6 px past it is 5.15s, inside the 8 px radius.
const { preview, committed } = dropAt(lane, 206);
expect(preview).toEqual({ start: 5, track: 0 });
expect(committed).toEqual(preview);
// 12 px past it is outside the radius and keeps the pointer time.
expect(dropAt(lane, 212).committed).toEqual({ start: 5.3, track: 0 });
});
it("keeps the pointer time with the magnet off, and still starts an empty lane at 0", () => {
usePlayerStore.getState().setTimelineSnapEnabled(false);
try {
const lane: TimelineElement[] = [{ id: "c", tag: "div", start: 0, duration: 5, track: 0 }];
expect(dropAt(lane, 206).committed).toEqual({ start: 5.15, track: 0 });
expect(dropAt([], 80).committed).toEqual({ start: 0, track: 0 });
} finally {
usePlayerStore.getState().setTimelineSnapEnabled(true);
}
});
});
describe("resolveDropInsertRow", () => {
const rows = [TRACK_H, TRACK_H, TRACK_H];
it("stays on the row at the boundary between two rows", () => {
@@ -302,7 +358,7 @@ describe("useTimelineAssetDrop new-track drops", () => {
expect(onAssetDrop).toHaveBeenCalledWith(
"assets/a.png",
expect.objectContaining({
start: 10,
start: 0,
insertRow: 0,
trackOrder: Array.from({ length: 100 }, (_, index) => index),
}),
@@ -10,6 +10,8 @@ import {
type TimelineRowGeometry,
} from "./timelineLayout";
import { resolveInsertRow } from "./timelineCollision";
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
import { collectTimelineSnapTargets, snapTimelineTime, TIMELINE_SNAP_PX } from "./timelineSnapping";
import type { TimelineDropCallbacks, TimelineDropPlacement } from "./timelineCallbacks";
import {
applyTimelineAutoScrollStep,
@@ -20,6 +22,7 @@ interface UseTimelineAssetDropOptions extends TimelineDropCallbacks {
scrollRef: RefObject<HTMLDivElement | null>;
ppsRef: RefObject<number>;
trackOrderRef: RefObject<number[]>;
elementsRef: RefObject<readonly TimelineElement[]>;
rowGeometryRef: RefObject<TimelineRowGeometry>;
contentOrigin: number;
sessionEpoch: number;
@@ -65,20 +68,36 @@ export function resolveDropInsertRow(
return resolveInsertRow(rowFloat, trackCount);
}
function alignDropStart(
placement: TimelineDropPlacement,
elements: readonly TimelineElement[],
pixelsPerSecond: number,
): TimelineDropPlacement {
const laneHasClips =
placement.insertRow == null && elements.some((el) => el.track === placement.track);
if (!laneHasClips) return { ...placement, start: 0 };
if (!usePlayerStore.getState().timelineSnapEnabled) return placement;
const targets = collectTimelineSnapTargets({ elements, playheadTime: null, beatTimes: [] });
const threshold = TIMELINE_SNAP_PX / Math.max(pixelsPerSecond, 1);
return { ...placement, start: snapTimelineTime(placement.start, targets, threshold).time };
}
function placeDrop(
geometry: Parameters<typeof resolveTimelineAssetDrop>[0],
elements: readonly TimelineElement[],
clientX: number,
clientY: number,
// Blocks and compositions are written by their own installers, which only take a track.
canInsertTrack: boolean,
): TimelineDropPlacement {
const placement = resolveTimelineAssetDrop(geometry, clientX, clientY);
if (!canInsertTrack) return placement;
const contentY = clientY - geometry.rectTop + geometry.scrollTop;
const insertRow = resolveDropInsertRow(contentY, geometry.rowHeights, geometry.trackOrder.length);
return insertRow == null
? placement
: { ...placement, insertRow, trackOrder: geometry.trackOrder };
const insertRow = canInsertTrack
? resolveDropInsertRow(contentY, geometry.rowHeights, geometry.trackOrder.length)
: null;
const aimed =
insertRow == null ? placement : { ...placement, insertRow, trackOrder: geometry.trackOrder };
return alignDropStart(aimed, elements, geometry.pixelsPerSecond);
}
function invokeDropCallback(callback: () => Promise<void> | void): void {
@@ -118,14 +137,15 @@ function applyTypedJsonDrop(
}
/**
* Dropping an asset/file/block/composition onto the timeline places it on the row
* and at the time it was dropped on; an asset moves to that row's nearest free time.
* Dropping an asset/file/block/composition places it on the row at 0 on an empty lane, else at
* the pointer snapped to a near clip edge; an asset moves to that row's nearest free time.
* Supersedes the prior playhead decision (#2291); see useAddAssetAtPlayhead.
*/
export function useTimelineAssetDrop({
scrollRef,
ppsRef,
trackOrderRef,
elementsRef,
rowGeometryRef,
contentOrigin,
onFileDrop,
@@ -197,12 +217,13 @@ export function useTimelineAssetDrop({
rowHeights: rowGeometryRef.current.rowHeights,
trackOrder: trackOrderRef.current,
},
elementsRef.current,
clientX,
clientY,
canInsertTrack,
);
},
[scrollRef, ppsRef, trackOrderRef, rowGeometryRef, contentOrigin],
[scrollRef, ppsRef, trackOrderRef, elementsRef, rowGeometryRef, contentOrigin],
);
const handleAssetDragOver = useCallback(
@@ -229,6 +229,7 @@ export function useTimelineProviderState({
scrollRef,
ppsRef,
trackOrderRef,
elementsRef: timelineElementsRef,
rowGeometryRef,
contentOrigin,
onFileDrop: pinnedOnFileDrop,