fix(studio): remove keyframe dragging from the timeline (#1763)

Dragging timeline keyframe diamonds was unreliable — clip<->tween percentage
remapping, an optimistic-hold workaround, and an intermittent no-op/revert when
the GSAP session lagged the drag (its own comments document the flakiness). Remove
the drag interaction entirely: diamonds still display, click-to-seek, and offer the
context menu (add/remove/ease) — keyframe timing is edited via the playhead + panel,
which are deterministic. Deletes the keyframe-move plan module + its wiring through
TimelineClipDiamonds -> TimelineCanvas -> Timeline -> TimelineEditContext.
This commit is contained in:
Miguel Ángel
2026-06-27 11:33:14 -04:00
committed by GitHub
parent 2d3b19d255
commit cf8f8aa568
8 changed files with 4 additions and 448 deletions
@@ -20,7 +20,6 @@ import {
type KeyframeDiamondContextMenuState,
} from "./KeyframeDiamondContextMenu";
import { useTimelineClipDrag } from "./useTimelineClipDrag";
import { snapKeyframePctToBeat } from "./timelineEditing";
import { ClipContextMenu } from "./ClipContextMenu";
import {
GUTTER,
@@ -87,7 +86,6 @@ export const Timeline = memo(function Timeline({
onDeleteKeyframe,
onDeleteAllKeyframes,
onChangeKeyframeEase,
onMoveKeyframe,
} = useResolvedTimelineEditCallbacks({
onMoveElement: onMoveElementOverride,
onResizeElement: onResizeElementOverride,
@@ -481,19 +479,6 @@ export const Timeline = memo(function Timeline({
onShiftClickKeyframe={(elId, pct) => {
toggleSelectedKeyframe(`${elId}:${pct}`);
}}
onDragKeyframe={(el, oldPct, newPct) => {
onMoveKeyframe?.(el, oldPct, newPct);
}}
onSnapKeyframePct={(el, pct) =>
snapKeyframePctToBeat(el, pct, adjustedBeatAnalysis?.beatTimes, pps)
}
onPickKeyframeElement={(el) => {
const elKey = el.key ?? el.id;
if (selectedElementId !== elKey) {
setSelectedElementId(elKey);
onSelectElement?.(el);
}
}}
onContextMenuKeyframe={(e, elId, pct) => {
const el = expandedElements.find((x) => (x.key ?? x.id) === elId);
if (el) {
@@ -91,11 +91,6 @@ interface TimelineCanvasProps {
currentTime: number;
onClickKeyframe?: (element: TimelineElement, percentage: number) => void;
onShiftClickKeyframe?: (elementId: string, percentage: number) => void;
onDragKeyframe?: (element: TimelineElement, oldPct: number, newPct: number) => void;
/** Snap a keyframe's clip-relative % to the nearest beat (returns unchanged when none in range). */
onSnapKeyframePct?: (element: TimelineElement, pct: number) => number;
/** Select the element when a keyframe drag starts (loads its GSAP session). */
onPickKeyframeElement?: (element: TimelineElement) => void;
onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void;
onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void;
beatAnalysis?: MusicBeatAnalysis | null;
@@ -143,9 +138,6 @@ export const TimelineCanvas = memo(function TimelineCanvas({
currentTime,
onClickKeyframe,
onShiftClickKeyframe,
onDragKeyframe,
onSnapKeyframePct,
onPickKeyframeElement,
onContextMenuKeyframe,
onContextMenuClip,
beatAnalysis,
@@ -446,11 +438,6 @@ export const TimelineCanvas = memo(function TimelineCanvas({
selectedKeyframes={selectedKeyframes}
onClickKeyframe={(pct) => onClickKeyframe?.(previewElement, pct)}
onShiftClickKeyframe={onShiftClickKeyframe}
onDragKeyframe={(oldPct, newPct) =>
onDragKeyframe?.(previewElement, oldPct, newPct)
}
snapPct={(pct) => onSnapKeyframePct?.(previewElement, pct) ?? pct}
onPickForDrag={() => onPickKeyframeElement?.(previewElement)}
onContextMenuKeyframe={onContextMenuKeyframe}
/>
)}
@@ -1,4 +1,4 @@
import { memo, useEffect, useRef, useState } from "react";
import { memo } from "react";
import { BEAT_BAND_H } from "./BeatStrip";
interface KeyframeEntry {
@@ -28,15 +28,7 @@ interface TimelineClipDiamondsProps {
selectedKeyframes: Set<string>;
onClickKeyframe?: (percentage: number) => void;
onShiftClickKeyframe?: (elementId: string, percentage: number) => void;
onDragKeyframe?: (percentage: number, newPercentage: number) => void;
onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void;
/** Snap a clip-relative percentage to the nearest beat (returns it unchanged
* when no beat is within range). Drives live beat-snapping while dragging. */
snapPct?: (percentage: number) => number;
/** Select this element when a keyframe drag begins, so its GSAP session is
* loaded by the time the move commits (diamonds render on unselected clips
* too, and a drag suppresses the selecting click). */
onPickForDrag?: () => void;
}
const DIAMOND_RATIO = 0.8;
@@ -59,54 +51,8 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
selectedKeyframes,
onClickKeyframe,
onShiftClickKeyframe,
onDragKeyframe,
onContextMenuKeyframe,
snapPct,
onPickForDrag,
}: TimelineClipDiamondsProps) {
// Live drag: which keyframe (by original %) is being dragged and its current
// (beat-snapped) %, so the diamond + its connecting lines follow the cursor.
const dragRef = useRef<{ origPct: number; pct: number; moved: boolean } | null>(null);
const [drag, setDrag] = useState<{ origPct: number; pct: number } | null>(null);
// Commit through the latest callback, not the one captured at pointer-down:
// selecting the element on drag-start loads its GSAP session asynchronously,
// and the commit must use the closure that sees the loaded session.
const onDragKeyframeRef = useRef(onDragKeyframe);
onDragKeyframeRef.current = onDragKeyframe;
// Optimistic hold: after a commit, keep the diamond at the dropped position
// until the cache reflects the change (the file round-trip rewrites
// keyframesData), so it doesn't flash back to the old spot in between.
const pendingRef = useRef(false);
const pendingHeldPctRef = useRef<number | null>(null);
const pendingTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
// Cleanup for an in-flight drag's document listeners, so an unmount mid-drag
// (clip deleted, comp switch, zoom-out → early return) doesn't leak them.
const dragCleanupRef = useRef<(() => void) | null>(null);
useEffect(() => {
if (!pendingRef.current) return;
// Only release the optimistic hold once the cache actually reflects the
// committed position (a keyframe near the held %). An unrelated cache
// rebuild (e.g. elementCount change) rebuilds keyframesData with the SAME
// percentages — releasing then would flash the diamond back to the old spot.
const held = pendingHeldPctRef.current;
if (held != null && !keyframesData.keyframes.some((k) => Math.abs(k.percentage - held) < 0.3)) {
return;
}
pendingRef.current = false;
pendingHeldPctRef.current = null;
if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current);
setDrag(null);
}, [keyframesData]);
useEffect(
() => () => {
clearTimeout(pendingTimerRef.current ?? undefined);
dragCleanupRef.current?.();
},
[],
);
if (clipWidthPx < 20) return null;
// When the beat strip occupies the top band, shrink the diamonds and center
@@ -129,79 +75,13 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
}
};
const handlePointerDown = (e: React.PointerEvent, pct: number) => {
if (e.button !== 0) return;
e.stopPropagation();
// Ignore a new drag while a prior drop is still settling: `pct` comes from
// props (the pre-drop position) but the diamond is held at its dropped spot
// via effPct(), so a re-grab would track from a stale origin and commit
// against the wrong tween. The hold clears on the cache round-trip (≤2s).
if (pendingRef.current) return;
// Select the element up front so its GSAP session loads during the drag and
// the commit (which resolves the animation from the selection) isn't a no-op.
onPickForDrag?.();
const startX = e.clientX;
dragRef.current = { origPct: pct, pct, moved: false };
const handleMove = (me: PointerEvent) => {
const d = dragRef.current;
if (!d) return;
const dx = me.clientX - startX;
// 4px dead zone so a click doesn't register as a drag.
if (!d.moved && Math.abs(dx) <= 4) return;
d.moved = true;
const rawPct = Math.max(0, Math.min(100, pct + (dx / clipWidthPx) * 100));
const snapped = snapPct ? snapPct(rawPct) : rawPct;
d.pct = snapped;
setDrag({ origPct: pct, pct: snapped });
};
const handleUp = () => {
document.removeEventListener("pointermove", handleMove);
document.removeEventListener("pointerup", handleUp);
dragCleanupRef.current = null;
const d = dragRef.current;
dragRef.current = null;
const willCommit = !!(d && d.moved && Math.abs(d.pct - d.origPct) > 0.5);
if (willCommit && d) {
// Hold the dropped position optimistically; the effect clears it once the
// cache round-trip lands (fallback timeout in case it never does).
pendingRef.current = true;
pendingHeldPctRef.current = d.pct;
setDrag({ origPct: d.origPct, pct: d.pct });
if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current);
pendingTimerRef.current = setTimeout(() => {
pendingRef.current = false;
pendingHeldPctRef.current = null;
setDrag(null);
}, 2000);
onDragKeyframeRef.current?.(d.origPct, d.pct);
} else {
setDrag(null);
}
};
dragCleanupRef.current = () => {
document.removeEventListener("pointermove", handleMove);
document.removeEventListener("pointerup", handleUp);
};
document.addEventListener("pointermove", handleMove);
document.addEventListener("pointerup", handleUp);
};
const effPct = (p: number): number => (drag && drag.origPct === p ? drag.pct : p);
return (
<div className="absolute inset-0" style={{ zIndex: 3, pointerEvents: "none" }}>
{sorted.map((kf, i) => {
if (i === 0) return null;
const prev = sorted[i - 1]!;
const x1 = Math.max(
0,
Math.min(clipWidthPx, (effPct(prev.percentage) / 100) * clipWidthPx),
);
const x2 = Math.max(0, Math.min(clipWidthPx, (effPct(kf.percentage) / 100) * clipWidthPx));
const x1 = Math.max(0, Math.min(clipWidthPx, (prev.percentage / 100) * clipWidthPx));
const x2 = Math.max(0, Math.min(clipWidthPx, (kf.percentage / 100) * clipWidthPx));
if (x2 - x1 < 1) return null;
return (
<div
@@ -227,7 +107,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
// is the clip's left edge (the diamond's left half overflows, which the
// overflow-visible clip shows) — NOT shifted fully inside. No clamp, or
// boundary keyframes (0% / 100%) would render off-center.
const leftPx = (effPct(kf.percentage) / 100) * clipWidthPx - half;
const leftPx = (kf.percentage / 100) * clipWidthPx - half;
const kfKey = `${elementId}:${kf.percentage}`;
const isKfSelected = selectedKeyframes.has(kfKey);
const atPlayhead = isSelected && Math.abs(kf.percentage - currentPercentage) < 0.5;
@@ -252,7 +132,6 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
padding: 0,
}}
onClick={(e) => handleClick(e, kf.percentage)}
onPointerDown={(e) => handlePointerDown(e, kf.percentage)}
onContextMenu={(e) => {
e.preventDefault();
e.stopPropagation();
@@ -39,6 +39,5 @@ export interface TimelineEditCallbacks {
onDeleteKeyframe?: (elementId: string, percentage: number) => void;
onDeleteAllKeyframes?: (elementId: string) => void;
onChangeKeyframeEase?: (elementId: string, percentage: number, ease: string) => void;
onMoveKeyframe?: (element: TimelineElement, oldPct: number, newPct: number) => void;
onToggleKeyframeAtPlayhead?: (element: TimelineElement) => void;
}