Repository navigation
fix(studio): a clip drag released outside the window is cancelled - #4646
Conversation
Pointer capture delivered the release to the timeline even outside the window, so the move or trim committed wherever its preview was last drawn. A release outside the window now cancels the gesture, the same as Escape.
Fade handles run their own gesture, so they now use the same outside-window check as clip moves and trims, moved into one shared module.
759b4cb to
ae23d76
Compare
somanshreddy
left a comment
There was a problem hiding this comment.
Reviewed at ae23d76044bf989d059519fa8062ec0540af581b. Approving. The implementation is four lines and the boundary is exactly pinned. One question I'd want answered before this reaches users, about the horizontal edges specifically.
The boundary is precise and tested
return x < 0 || y < 0 || x >= window.innerWidth || y >= window.innerHeight;Off-by-one here is the obvious failure, so I mutated >= to > and two tests fail immediately:
× cancels a drag released on the window's right edge and leaves the clip where it was
× cancels a drag released on the window's bottom edge and leaves the clip where it was
innerWidth - 1 is the last valid pixel and commits; innerWidth is outside and cancels. Both directions pinned, 7/7 at this head.
I also checked the placement of the new check, since it runs before claimActiveGesture and so sees pointers that the normal path would classify as "ignored". That's safe: handleWindowPointerCancel does its own pointerMatchesGesture(event) test before cancelling, and the blocked-clip branch compares pointerId. A second pointer releasing outside the window can't cancel someone else's active drag.
1. [question, non-blocking] Cancel is applied to all four edges, but the horizontal ones are a designed interaction zone
The motivating bug is a clip dragged off the top landing on another track. Cancelling there is plainly right — there's no valid drop target above the timeline, so committing "wherever the preview was last drawn" is nonsense.
The left and right edges aren't the same, because edge auto-scroll exists and timelineClipDragTypes.ts:50 describes it precisely:
Edge auto-scroll moves the content under a stationary pointer, so the trim math folds (current − origin) scrollLeft into the pointer x
So holding the pointer at the horizontal edge is not an accident, it's the documented way to scroll the timeline mid-drag. The gesture that ends that workflow is: press against the edge, wait while content scrolls, release. If the pointer is sitting at clientX >= innerWidth at that moment, the edit is now silently discarded — and whether it lands on innerWidth - 1 or innerWidth isn't something the user controls, especially on a fast drag or a trackpad flick.
The "last pixel still commits" mitigation doesn't help here, because the auto-scroll workflow deliberately parks the pointer at the boundary rather than one pixel inside it.
What I can't determine from the code: whether this actually bites depends on whether the timeline pane reaches the window edge. Auto-scroll is container-relative (getBoundingClientRect on the scroll element in timelineClipDragPreview.ts), so if the pane is inset behind a side panel, the auto-scroll trigger zone sits comfortably inside innerWidth and nothing cancels. If the pane runs full-width at the bottom — which is the usual Studio layout — the two coincide and the workflow terminates in a discard. I'd need to run Studio to say which, and I couldn't.
If it does coincide, the asymmetry suggests the remedy: cancel on the vertical axis, where there is genuinely no drop target, and clamp on the horizontal, where the drag math already folds scrollLeft and a clamped x is meaningful. That keeps the fix for the bug you're actually fixing without taking the auto-scroll workflow with it.
Not blocking, because the old behaviour was worse in the case you set out to fix and nothing here regresses the vertical case. But it's worth a deliberate answer rather than discovering it from a bug report about "my clip move sometimes doesn't stick."
2. [nit] A non-matching pointer now reaches the suppression cleanup
Routing outside-releases through handleWindowPointerCancel changes what happens for pointers that aren't the gesture's. In the inside path, claimActiveGesture returns "ignored" and the handler returns before touching click suppression. In the outside path the tail still runs:
const blocked = blockedClipRef.current;
if (blocked && blocked.pointerId !== event.pointerId) return;
blockedClipRef.current = null;
if (suppressClickRef.current) clearSuppressedClick();With no blocked clip the guard doesn't return, so an unrelated pointer released outside the window clears a pending click suppression that the old path would have preserved — and the post-drag click it was meant to swallow can then fire. Needs a second pointer, so it's niche on desktop and more plausible on touch. An early if (!pointerMatchesGesture(event) && !blockedClipRef.current) return; would restore the previous behaviour.
Consistency with #4645
Worth noting these two land together: #4645 makes the picture layer take no pointer input so edge presses reach the trim handles, and this one governs where those drags may be released. The fade handles are treated correctly in both — outside the pointer-events: none layer there, and routed through the same shared releasedOutsideWindow here rather than keeping their own copy.
Code merit only — no merge or queue action from me.
What a user can do now
Abandon a clip drag by letting go outside the window. A clip move, a trim or a fade-handle drag released outside the window is cancelled and the clip goes back to how it was, the same as pressing Escape. Before this change, the drag committed to wherever its preview was last drawn, so a clip dragged off the top of the window landed on another track.
Why it happened
The timeline captures the pointer when a drag starts, so the release reaches it even when it happens outside the window. The release handler committed any release from the gesture's pointer without looking at where it happened. Only Escape, a cancelled pointer and a lost pointer capture cancelled a drag.
The fix
Clip moves, single and group trims, and the blocked-edit gesture on a locked clip all end in one release handler in
timelineClipDragGestureLifecycle.ts. A release outside the window's bounds now goes through the same path as a cancelled pointer, which discards the preview and commits nothing. A release inside the window, up to its last pixel, still commits as before.Fade handles run their own gesture in
TimelineClipFades.tsx. Its release now uses the same outside-window check, moved into one shared module (timelinePointerRelease.ts): a fade drag released outside the window puts the fade back as it was and saves nothing, and a release inside still saves.Tests
timelineClipDragGestureLifecycle.test.ts:TimelineClipFades.test.tsx:Two existing fade tests released at negative x as a stand-in for "far left"; they now release inside the window, since a release outside cancels.
The cancel cases, clip and fade, fail on main:
Also run: the
useTimelineClipDrag*,timelineClipDrag*,TimelineClipandTimelineClipFadestests, plus format, lint and the studio typecheck.Before
Studio on main with a small fixture: the Bar clip (start 1 s, second row) is dragged 2 s right and up, then released above the top of the window. The move commits: Bar lands on a new top row at 0 s and pushes Title down.
release-outside-before.mp4
After
Same fixture and gesture on this branch: after the release above the window, Bar is back at 1 s on its own row and nothing was saved.
release-outside-after.mp4
Not covered
Keyframe retimes, automation-lane point drags and beat drags have their own pointer handling and still save when released outside the window. They are a follow-up.