From 6b4870246ce7d9c4f259c1f714c1467c97c0df29 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 8 Jul 2026 17:44:48 -0400 Subject: [PATCH] fix(studio): dropping an overlapping clip on a lane restacks it, not a no-op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A vertical drag onto a lane whose clips overlap the dragged clip in time used to resolve to the "nearest valid lane", which included the clip's own lane — so a small drag snapped straight back and felt like the editor refused the move. Overlapping clips can't share a row (a row is 1-D in time), but the drag should still restack, never reject. Now a conflicting onto-drop converts to an edge insertion adjacent to the target by drag direction (up -> above, down -> below): the clip lands on its own lane just in front of / behind the target, with its time unchanged. Non-overlapping drops still join the lane. Deletes the nearest-valid-placement search (~90 lines) this replaces. --- .../components/timelineLayerDrag.test.ts | 50 +++++++++- .../player/components/timelineLayerDrag.ts | 98 ++----------------- 2 files changed, 55 insertions(+), 93 deletions(-) diff --git a/packages/studio/src/player/components/timelineLayerDrag.test.ts b/packages/studio/src/player/components/timelineLayerDrag.test.ts index 4d129b6ff..b325095a2 100644 --- a/packages/studio/src/player/components/timelineLayerDrag.test.ts +++ b/packages/studio/src/player/components/timelineLayerDrag.test.ts @@ -129,7 +129,7 @@ describe("resolveTimelineLayerZIndexChanges", () => { }); describe("resolveTimelineLayerStackingMove", () => { - it("snaps a blocked onto-lane drop to the nearest new lane instead of committing the conflict", () => { + it("places an upward onto-lane drop above the overlapping target lane", () => { const front = element({ id: "front", zIndex: 10, start: 0, duration: 2 }); const dragged = element({ id: "dragged", zIndex: 1, start: 0.5, duration: 1 }); const layers = [layer("front", 10, [front]), layer("dragged", 1, [dragged])]; @@ -139,7 +139,7 @@ describe("resolveTimelineLayerStackingMove", () => { element: dragged, layers, layerOrder: layers.map((item) => item.id), - trackDeltaRaw: -1, + trackDeltaRaw: -0.8, }), ).toEqual({ previewLayerId: "preview:dragged:above:front", @@ -151,4 +151,50 @@ describe("resolveTimelineLayerStackingMove", () => { }, }); }); + + it("places a downward onto-lane drop below the overlapping target lane", () => { + const dragged = element({ id: "dragged", zIndex: 10, start: 0.5, duration: 1 }); + const back = element({ id: "back", zIndex: 1, start: 0, duration: 2 }); + const layers = [layer("dragged", 10, [dragged]), layer("back", 1, [back])]; + + expect( + resolveTimelineLayerStackingMove({ + element: dragged, + layers, + layerOrder: layers.map((item) => item.id), + trackDeltaRaw: 0.8, + }), + ).toEqual({ + previewLayerId: "preview:dragged:below:back", + previewLayerIndex: 2, + stackingReorder: { + contextKey: "root", + placement: { type: "below", layerId: "back" }, + zIndexChanges: [{ key: "dragged", zIndex: 0 }], + }, + }); + }); + + it("keeps a non-overlapping onto-lane drop joined to the target lane", () => { + const front = element({ id: "front", zIndex: 10, start: 0, duration: 1 }); + const dragged = element({ id: "dragged", zIndex: 1, start: 2, duration: 1 }); + const layers = [layer("front", 10, [front]), layer("dragged", 1, [dragged])]; + + expect( + resolveTimelineLayerStackingMove({ + element: dragged, + layers, + layerOrder: layers.map((item) => item.id), + trackDeltaRaw: -0.8, + }), + ).toEqual({ + previewLayerId: "front", + previewLayerIndex: 0, + stackingReorder: { + contextKey: "root", + placement: { type: "onto", layerId: "front" }, + zIndexChanges: [{ key: "dragged", zIndex: 10 }], + }, + }); + }); }); diff --git a/packages/studio/src/player/components/timelineLayerDrag.ts b/packages/studio/src/player/components/timelineLayerDrag.ts index f718fb542..75e4d4c1b 100644 --- a/packages/studio/src/player/components/timelineLayerDrag.ts +++ b/packages/studio/src/player/components/timelineLayerDrag.ts @@ -296,96 +296,6 @@ function resolveDragPlacement( : null; } -function buildInsertionPlacement( - layers: readonly StackingTimelineLayer[], - insertionIndex: number, -): TimelineLayerDropPlacement | null { - const first = layers[0]; - const last = layers[layers.length - 1]; - if (!first || !last) return null; - if (insertionIndex <= 0) return { type: "above", layerId: first.id }; - if (insertionIndex >= layers.length) return { type: "below", layerId: last.id }; - const before = layers[insertionIndex - 1]; - const after = layers[insertionIndex]; - return before && after - ? { type: "between", beforeLayerId: before.id, afterLayerId: after.id } - : null; -} - -interface ValidPlacementCandidate { - placement: TimelineLayerDropPlacement; - position: number; - kind: "onto" | "insert"; -} - -function comparePlacementCandidates(input: { - targetPosition: number; - currentIndex: number; - a: ValidPlacementCandidate; - b: ValidPlacementCandidate; -}): number { - const distanceA = Math.abs(input.a.position - input.targetPosition); - const distanceB = Math.abs(input.b.position - input.targetPosition); - if (distanceA !== distanceB) return distanceA - distanceB; - - const direction = Math.sign(input.targetPosition - input.currentIndex); - if (direction !== 0) { - const biasA = - direction < 0 - ? input.a.position <= input.targetPosition - : input.a.position >= input.targetPosition; - const biasB = - direction < 0 - ? input.b.position <= input.targetPosition - : input.b.position >= input.targetPosition; - if (biasA !== biasB) return biasA ? -1 : 1; - } - - if (input.a.kind !== input.b.kind) return input.a.kind === "onto" ? -1 : 1; - return input.a.position - input.b.position; -} - -function resolveNearestValidPlacement(input: { - layers: readonly StackingTimelineLayer[]; - element: TimelineStackingElement; - draggedKey: string; - targetPosition: number; - currentIndex: number; -}): TimelineLayerDropPlacement | null { - const candidates: ValidPlacementCandidate[] = []; - - input.layers.forEach((layer, index) => { - if (!layerConflictsWithElement(layer, input.element, input.draggedKey)) { - candidates.push({ - placement: { type: "onto", layerId: layer.id }, - position: index, - kind: "onto", - }); - } - }); - - for (let insertionIndex = 0; insertionIndex <= input.layers.length; insertionIndex += 1) { - const placement = buildInsertionPlacement(input.layers, insertionIndex); - if (!placement) continue; - candidates.push({ - placement, - position: insertionIndex - 0.5, - kind: "insert", - }); - } - - return ( - candidates.sort((a, b) => - comparePlacementCandidates({ - targetPosition: input.targetPosition, - currentIndex: input.currentIndex, - a, - b, - }), - )[0]?.placement ?? null - ); -} - function resolveLaneAwareDragPlacement(input: { layers: readonly StackingTimelineLayer[]; element: TimelineStackingElement; @@ -398,7 +308,13 @@ function resolveLaneAwareDragPlacement(input: { const target = findLayer(input.layers, input.placement.layerId); if (!target) return null; if (!layerConflictsWithElement(target, input.element, input.draggedKey)) return input.placement; - return resolveNearestValidPlacement(input); + if (input.targetPosition < input.currentIndex) { + return { type: "above", layerId: target.id }; + } + if (input.targetPosition > input.currentIndex) { + return { type: "below", layerId: target.id }; + } + return input.placement; } function getPreviewLayerId(