fix(editor): don't leave the entry landing over a chart loaded via editSong - #164
Conversation
…itSong
`window.editSong` calls `showScreen('plugin-editor')` and `loadCDLC(filename)`
in the same tick. The screen activation arms the entry landing on a
`setTimeout(…, 80)` (and `…, 120` when the screen is already active at init),
and `_editorMaybeShowStartLanding` asks "is anything loaded?" only when that
timer fires. A load still fetching at +80ms therefore loses the race, and
nothing ever took the landing down again — its own buttons were the only
`remove()` call sites. A large feedpak finished loading underneath a
`fixed inset-0 z-50` overlay.
That would be merely cosmetic if the overlay were inert. It is not:
canvas.addEventListener('mousedown', onMouseDown); // canvas
_globalListeners.add(document, 'mousemove', onMouseMove); // document
`mousemove` fires straight through any overlay, so `canvas.style.cursor` still
becomes `ew-resize` over a note's right edge, while `mousedown` is swallowed and
the drag never starts. Cursor arms, nothing grabs.
Both entrances to `editSong` are real user flows: the library song-card's "Open
in editor" action, and the 3D highway's "Edit region" return path
(`returnToEditorFromHighway`).
Fix: count loads in flight. A load in flight suppresses the landing; starting a
load dismisses a landing already up. A counter rather than a flag because two
loads can overlap — a fast failure would otherwise clear a guard a slower load
still needs held (Codex).
Verified headlessly through `editSong` on a 33MB feedpak: before, the landing
survives the load, `elementFromPoint` over a note returns
`DIV#editor-start-landing`, and a click selects nothing; after, the canvas is
topmost and the click selects. An empty editor still offers the landing, so the
harness also fails a "fix" that just deletes the feature.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an ChangesLanding Overlay Race Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant editSong
participant loadCDLC
participant editorMaybeShowStartLanding
User->>editSong: open song from library/highway
editSong->>loadCDLC: loadCDLC(filename)
loadCDLC->>loadCDLC: _editorLoadsInFlight++
editSong->>editorMaybeShowStartLanding: showScreen triggers check
editorMaybeShowStartLanding->>editorMaybeShowStartLanding: return early (loads in flight > 0)
loadCDLC->>loadCDLC: finally: _editorLoadsInFlight--
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)src/main.jsast-grep timed out on this file Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes a race where the editor’s “start landing” overlay could remain on top of the canvas when a song is opened via window.editSong (or similar flows), preventing mousedown interactions from reaching the canvas.
Changes:
- Track in-flight editor loads via
_editorLoadsInFlightand suppress showing the start landing while a load is running. - Dismiss any existing start landing overlay when a load begins, ensuring it can’t persist over an active editor session.
- Document the issue and fix in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main.js | Adds an in-flight load counter, removes the landing overlay on load start, and prevents landing display while loads are running. |
| CHANGELOG.md | Records the bug scenario and the behavioral change (landing suppressed during loads; dismissed on load start). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Opening a song through the library card’s “Open in editor” action, or returning from the 3D highway’s “Edit region” button, could finish the load underneath a full-screen overlay that silently ate every canvas click.
The race
window.editSongnavigates and loads in the same tick:The screen activation arms
_editorMaybeShowStartLandingon asetTimeout(…, 80)(and…, 120when the screen is already active atinit()). That function asks “is anything loaded?” only when it fires — so a load still fetching at +80 ms loses the race. And nothing ever took the landing down again: its own buttons were the onlyremove()call sites.Why it wasn’t merely cosmetic
mousemovefires straight through any overlay, socanvas.style.cursorstill turnsew-resizeover a note’s right edge.mousedownis bound to the canvas, so it is swallowed and the drag never starts. Cursor arms, nothing grabs — with abg-black/70scrim sitting there to explain it, if you happened to look.Fix
Count loads in flight (
_editorLoadsInFlight). A load in flight suppresses the landing; starting a load dismisses a landing already up.A counter rather than a boolean because two loads can overlap —
editSongtwice, or a fast failure alongside a slow fetch — and the first to settle would otherwise clear a guard the second still needs held. Codex caught that on the first pass.Verification
A headless harness drives the real flow (
window.editSongon a 33 MB feedpak) and asserts both halves. It fails onmainand passes here:elementFromPointover a noteDIV#editor-start-landingCANVAS#editor-canvasonMouseDownThe first row matters: it fails a “fix” that just deletes the landing.
node --test88/88,npm run lint0 errors (10 pre-existing warnings), Codex clean.Not fixed here
The
mousedown-on-canvas /mousemove-on-document asymmetry is the amplifier — any element above the canvas produces the same “cursor works, drag doesn’t” signature. Movingmousedowntodocumentwould change click-outside semantics, so it is left alone; the real defence is not leaving overlays around, which is what this PR does.🤖 Generated with Claude Code
Summary by CodeRabbit