Skip to content

feat(studio): multi-clip drag preview math#2197

Open
ukimsanov wants to merge 1 commit into
studio-dnd/pr05-timeline-snappingfrom
studio-dnd/pr06-multidrag-preview
Open

feat(studio): multi-clip drag preview math#2197
ukimsanov wants to merge 1 commit into
studio-dnd/pr05-timeline-snappingfrom
studio-dnd/pr06-multidrag-preview

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

What

new pure module timelineMultiDragPreview — group-drag passenger offsets and clamped group deltas (isMultiDragActive, multiDragDeltaSeconds, multiDragPassengerOffsetPx, clampGroupMoveDelta) with tests.

Why

the group-drag math, standalone and DOM-free.

How

new files only; consumed later by TimelineLanes.

Test plan

bunx vitest run timelineMultiDragPreview.test.ts; tsc --noEmit; fallow audit clean.


Stack position 6/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary TEMP(studio-dnd) fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.

ukimsanov commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

This was referenced Jul 11, 2026
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 4622bb9 to d0a6ab1 Compare July 11, 2026 00:55
@ukimsanov ukimsanov force-pushed the studio-dnd/pr05-timeline-snapping branch 2 times, most recently from 7312dd0 to 7da8833 Compare July 11, 2026 01:15
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from d0a6ab1 to 206097c Compare July 11, 2026 01:15

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga

@vanceingalls vanceingalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: LGTM (green) — rigid group, no shearing

A concise study in "the formation moves as one". Every passenger inherits the same delta; the leftmost member sets the floor; the grabbed clip stops with the group rather than out-running it. That is the correct model.

The pointed queries

  • Group-clamping by the leftmost element (no shearing). packages/studio/src/player/components/timelineMultiDragPreview.ts:100–:106clampGroupMoveDelta reads minStart = Math.min(...memberStarts) and bounds rawDelta ≥ -minStart. The test at :113–:116 nails the invariant: grabbed clip at 10, passenger at 1, clampGroupMoveDelta(-8, [10, 1, 4]) === -1. The passenger — not the grabbed clip — sets the floor. This is precisely the "group shears if you per-element-clamp" bug the module documents at :90–:98.
  • Individual snap participation vs group delta. The module's own docstring at :2–:20 is explicit: the preview start (draggedPreviewStart) is ALREADY group-clamped upstream (updateDraggedClipPreview runs clampGroupMoveDelta before setting it), so the delta this module returns is by construction the clamped delta. Passengers inherit it via multiDragPassengerOffsetPx (:74–:82). Snap participation happens for the grabbed clip at the upstream layer; the group moves rigidly by its outcome. That's the correct sequencing.
  • Unit consistency. multiDragDeltaSeconds returns seconds (:52–:55); multiDragPassengerOffsetPx converts to pixels via explicit × pps at :81 and treats Number.isFinite(pps) ? pps : 0 (guards NaN input, tested at :86–:88). Clean boundary.
  • Edge cases exercised. Zero-delta when inactive (:49), leftward negative delta (:45), NaN pps → 0 offset (:87), already-at-0 forbids further leftward (:118–:122), empty formation returns raw delta (:124).

One observation (not a blocker)

  • Line :103 uses Math.min(...memberStarts) — for extremely large selections (10k+ elements) this can spread-args the call stack. Timeline selections are single-digit-to-dozens in practice, so purely theoretical. Would memberStarts.reduce((a, b) => a < b ? a : b) be more defensive? Yes. Necessary here? No.

R1 by Via

@ukimsanov ukimsanov force-pushed the studio-dnd/pr05-timeline-snapping branch from 7da8833 to 85079ea Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 206097c to 1ed5c27 Compare July 11, 2026 02:56
@ukimsanov ukimsanov force-pushed the studio-dnd/pr05-timeline-snapping branch from 85079ea to f441a84 Compare July 11, 2026 08:18
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 1ed5c27 to f2fbc25 Compare July 11, 2026 08:18
@ukimsanov ukimsanov marked this pull request as ready for review July 11, 2026 09:06
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from f2fbc25 to 6730438 Compare July 11, 2026 10:49
@ukimsanov ukimsanov force-pushed the studio-dnd/pr05-timeline-snapping branch from 25372cc to a399c01 Compare July 11, 2026 12:51
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 6730438 to 7f34c4e Compare July 11, 2026 12:51
What: new pure module timelineMultiDragPreview — group-drag passenger
offsets and clamped group deltas (isMultiDragActive, multiDragDeltaSeconds,
multiDragPassengerOffsetPx, clampGroupMoveDelta) with tests.

Why: the group-drag math, standalone and DOM-free.

How: new files only; consumed later by TimelineLanes.

Test plan: bunx vitest run timelineMultiDragPreview.test.ts; tsc --noEmit;
fallow audit clean.
@ukimsanov ukimsanov force-pushed the studio-dnd/pr05-timeline-snapping branch from a399c01 to ff3ddb7 Compare July 11, 2026 21:08
@ukimsanov ukimsanov force-pushed the studio-dnd/pr06-multidrag-preview branch from 7f34c4e to 4a2773c Compare July 11, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants