feat(editor): track regions — move a region by dragging its block - #334
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
967ea65 to
dc4fa1b
Compare
…t the active tab A song can hold several drum parts now (#339): each type:"drums" arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid target. The region commands and the parts-view drag arming predate that (#334) — kind:'drums' always operated on S.drumTab, and a materialized extra part ('drums-2'…) armed as 'notation' over its empty shell notes[], so dragging its block moved nothing (or, for the primary while another part was active, moved the WRONG part's hits). Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part whose track carries the region → its own .drumTab; arrIdx < 0 stays the legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared _contentList, and parts-view arms kind:'drums' for any drum-arrangement row. Tests drive a NON-active part and pin that the active tab never moves, plus the legacy no-arrIdx fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
…as a placed, movable region (#344) * feat(editor): track regions — place + delete commands (the import-region foundation) PlaceRegionCmd drops freshly-added track content onto the timeline as a BOUNDED, selected, draggable block: it slides the content so its first onset lands on the snapped startBeat (a musical move — beats preserved on a varying grid, mirroring MoveRegionCmd's remap with the constant-tempo fast path) and writes an explicit-length region covering it. Even a bar-1 placement stays bounded — never the implicit default — so the block is a distinct addressable object from birth. DeleteRegionCmd removes exactly the notes/hits a region's end-exclusive window owns plus the region entry, leaving neighbours untouched. Both round-trip: rollback restores a verbatim content snapshot (the beatOf∘timeOf round trip is not bit-reversible), the track's raw `regions` key (including deleting one that was never there), and the prior selection. Node-runnable with no DOM, like the move command. _nextRegionIdPure allocates the next free region:N, counting the implicit default region:1 even when unpersisted so a placed region can never collide with it. This is the command layer for "Add Track from File → placed region" (TRACK-REGIONS-DESIGN.md PR 3); the import front door wires in next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix * fix(editor): region commands act on the drum part the track names, not the active tab A song can hold several drum parts now (#339): each type:"drums" arrangement owns its .drumTab and S.drumTab is only the ACTIVE grid target. The region commands and the parts-view drag arming predate that (#334) — kind:'drums' always operated on S.drumTab, and a materialized extra part ('drums-2'…) armed as 'notation' over its empty shell notes[], so dragging its block moved nothing (or, for the primary while another part was active, moved the WRONG part's hits). Resolution now mirrors the lane silhouette: arrIdx >= 0 names the part whose track carries the region → its own .drumTab; arrIdx < 0 stays the legacy unmaterialized S.drumTab. MoveRegionCmd delegates to the shared _contentList, and parts-view arms kind:'drums' for any drum-arrangement row. Tests drive a NON-active part and pin that the active tab never moves, plus the legacy no-arrIdx fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix * feat(editor): the drum import lands as a placed, selected region (Add Track from File) The R3b payoff: importing a drum part into an existing project now surfaces it in the Tracks view as a SELECTED region, ready to drag. The Add-Drums dialog gains an inline "Place at" choice — Keep source timing (default: the file's own timing, no content motion, no persisted window), Bar 1, or Playhead — the latter two sliding the whole part there as one undoable PlaceRegionCmd (bar-snapped, beat-preserving). placeImportedPartAsRegion (track-session.js) is the orchestrator: it re-normalizes the session so the fresh part's row exists (the import just created the part — its row is synthesized here), resolves the track from the part's arr:<idx> target, then places or just selects. The import flow (arrangement.js) reaches it through the host table — track-session imports arrangement, so the call inverts through the usual seam. _placeAtStartBeatPure (region.js) resolves the dialog choice to a startBeat, converter-free like the other region pures. Create mode keeps replace semantics: the Place-at row hides, the choice is forced to 'keep', and with no materialized arrangement the hook no-ops gracefully. Tests drive the real orchestrator: row synthesis + placement + selection + undo restore, keep-writes-no-window, playhead snapping, and the pure's resolution table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix * feat(editor): Delete removes the selected region block (content + window, undoable) Del/Backspace in the Tracks view deletes the selected region as one DeleteRegionCmd: the notes/hits its end-exclusive window owns plus the window entry, neighbours untouched. parts-view owns the resolution (same kind/arrIdx logic as the drag arming, so a drum part's region always deletes ITS OWN hits, never the active grid's) and input.js's Delete ladder asks it first through the host table — transcription rows only; audio regions stay select-only until the audio-region PR. Falls through cleanly outside the Tracks view / with nothing selected. Tests drive the real handler: delete + undo round-trip, the gate table, and the non-active-drum-part case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix * fix(editor): route region-delete inside the Tracks-overview key gate Runtime verification caught the Del key doing nothing in the Tracks view: onKeyDown's partsViewMode gate returns for every key except the parts toggle, so the region-delete branch further down the Delete ladder was unreachable on the only surface it serves. The gate itself now offers Delete/Backspace to host.partsViewRegionDelete() before ignoring the key — a region op on this surface, not a note-edit on the hidden chart — and the dead ladder branch is folded into it. Verified end-to-end via Playwright against the real host (import a drums MIDI → Place at Bar 1 → region:2 lands selected → drag commits "Moved region:2 later" → Delete commits "Deleted region region:2 and its notes" → undo restores). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix * Harden imported region placement commands * Support legacy drum region placement --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The move step of the track-region layer: grab a track's region block in the Tracks (Parts) view and slide it along the timeline to reposition the track's content, as one undoable, bar-snapped step. Third in the region series — stacked on #333 (strip + selection), which is stacked on #332 (data model). Merge #332 → #333 → this.
This is PR 3a of a split: the charrette's PR 3 was commands + drag and the "Add Track from File" import driver; per the operability seat they're separated. The import driver (3b) is a follow-up and is blocked on a product decision (see below).
What it does
src/region-commands.js(new):MoveRegionCmd. A notation region shifts the arrangement's contained notes; a drum region shiftsS.drumTab's contained hits. Routes throughS.history.exec(not the track-sessioncommit()path, which never bumpseditGen, so coverage/chord/lint memos would go stale).pitchPreservingso it passes the read-only-roll edit lock; the drum variant issongScopelike every drum command._regionRemapPure). A move preserves musical position, not wall-clock seconds — each note's whole interval remaps through the one tempo map (newTime = timeOf(beatOf(oldTime) + dBeat), sustain via its end-beat), mirroringTempoMapCmd. On a steady-tempo song this is just a clean shift; across a tempo change the part re-locks to the destination grid and a sustained note keeps its beat-length. Fast path: a constant/absent tempo collapses to one bit-exact seconds shift (nobeatOf∘timeOfdrift).dBeat === 0is a no-op. Rollback restores a verbatim snapshot (the beat round trip is not bit-reversible), never inverts. (Move semantics reconciled with the rhythm-meter design lane against the seconds-primary reality; audio regions stay constant-dtime for the audio-region PR.)startBeatwith the move (and restores, incl. a move that re-sorts it past a neighbour inregions[]); the implicit default full-span region leavesregions[]untouched, carrying the move purely in its content — so untouched packs stay byte-identical.parts-view.js: mousedown on a hit block arms the drag (transcription rows only — audio blocks still just select, pending the audio-region PR), a bar-snapped dashed ghost previews the drop (Alt = free), and mouseUp commits. Routed through two host hooks so themouse.jsmove/up branches stay cycle-free.Tests & gates
tests/region_move.test.mjs(12): the pure time model (constant/varying tempo, fast path, snap),MoveRegionCmdexec→rollback deep-equality→redo for notation and drums, the bounded-regionstartBeatride + restore, zero-delta no-op, and the realS.history.exec/undo/redo path.routes.pyuntouched (no pytest).Runtime-verified
Served through the real host and driven with Playwright (AC/DC — Back In Black, 9 tracks): dragging the Lead block commits the move (
Moved "region:1" later), only the Lead track moves (Rhythm/Bass untouched), it snaps to bar lines (a sub-bar drag on this tempo-mapped song correctly no-ops), and Undo/Redo revert/re-apply exactly.Follow-up (not this PR)
The "Add Track from File" import driver (drums-only GP into an existing project) is blocked on a product call: drums are a single session-wide
S.drumTab— a project can hold only one drum part, so a new drum track isn't representable without extending the model to multiple drum tabs. Non-drum parts already add as N coexisting tracks. Flagged separately for the maintainer.🤖 Generated with Claude Code