refactor(editor): move Tab preview to src/tab-preview.js (R2, step 35) - #187
Merged
Conversation
Extract the read-only Tab preview (convert the saved pack of the current fretted part to GP5 and engrave it with a CDN-loaded alphaTab, render-only) out of the src/main.js monolith into a new native ES module. This is a self-contained leaf (0 deps into main.js, imports only S) AND the prerequisite for a clean keyboard-layer cut next: the keyboard command dispatcher calls _editorShowTabPreview and onKeyDown calls _tabPreviewKeyPolicyPure, so landing tab-preview as its own module turns those into plain module imports instead of awkward host hooks for a pure function. - src/tab-preview.js: 3 window handlers (re-attached by main.js) + the 2 symbols the keyboard layer still needs exported back. The @pure:tab-preview block is kept export-free (a trailing export statement) so the JS suite's new-Function slice stays valid. - tests/tab_preview.test.js + tab_preview_race.test.js retargeted to slice from src/tab-preview.js. main.js drops ~178 lines. 36 modules. Verified: 90/90 JS suites pass, ESLint gate clean (0 errors), strict no-undef clean on tab-preview.js, Codex preflight clean. New headless harness: 3 handlers bound, editorShowTabPreview opens the modal + sets the title + runs the render path (status populated), editorHideTabPreview closes it, zero page errors. Negative-checked by dropping a re-attach. 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 (4)
📝 WalkthroughWalkthroughTab Preview functionality is extracted from ChangesTab Preview module extraction
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
byrongamatos
added a commit
that referenced
this pull request
Jul 10, 2026
…188) * refactor(editor): move the input layer to src/input.js (R2, step 36) Extract the entire remaining "Mouse interactions" banner — the keyboard shortcut dispatch (onKeyDown), all the _editorX command implementations, the bookmarks/duplicate/section-cmds command clusters, the shortcut panel, and the canvas context menu (onContextMenu/showSectionMenu) — out of the src/main.js monolith into a new native ES module. The banner is now empty and gone. Enabled by landing tab-preview first (#187): the two tab-preview couplings (_editorShowTabPreview, _tabPreviewKeyPolicyPure) are now plain module imports. - src/input.js (~1240 lines): imports across ~20 modules; 7 symbols exported back (onKeyDown + onContextMenu, wired onto the canvas by main.js init(); _editorCurrentNoteIndices/_editorSeekToTime/_editorSnapStepSeconds host hooks; _bookmarks + the editorWaveformVisible live binding, read by the draw path). - 5 new host hooks for the composition-root symbols the commands call: editorCycleViewMode, editorMovePart, editorToggleKeyHighlight, editorTogglePartsView, tempoResolvedMeasureIdx. - The 4 @pure blocks (shortcut-panel-hint, bookmarks, duplicate, section-cmds) and the command classes inside them stay export-free (internal), so the JS suite's new-Function slices stay valid. - Removed 72 now-dead main.js imports (used only by the moved layer) — strict no-undef confirmed no over-removal. - 4 tests retargeted to slice from input.js (bookmarks, duplicate_selection, section_undo, roll_position_cycle). main.js drops ~1243 lines. 37 modules. Verified: 90/90 JS suites pass, ESLint gate clean (0 errors), strict no-undef clean on input.js, Codex preflight 0 regressions (init() attachment + editorWaveformVisible live binding confirmed). New headless harness drives REAL input: right-click opens the context menu (onContextMenu), the shortcut panel toggles, editorRunShortcutCommand routes 'toggleKeyHighlight'/'togglePartsView' into their new host hooks (distinct status each), and a real Shift+A keypress routes through onKeyDown to the parts-view command — zero page errors. The two new hooks + onKeyDown negative-checked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(input): CodeRabbit review on #188 — inert measure default, section-menu XSS, tempo-mode delete guard - host.js: tempoResolvedMeasureIdx default 0 -> -1 so an unwired hook is inert (callers apply the time-signature edit only on >= 0). This one is a regression from this PR's new hook. - input.js showSectionMenu: build the buttons as DOM nodes with textContent instead of interpolating the user-authored section name into innerHTML — removes a stored-XSS sink. (Pre-existing, moved verbatim; hardened here.) - input.js Delete key: add the !S.tempoMapMode guard (matching duplicate/copy/ paste) so a stale selection can't delete hidden notes while Tempo Map mode is active. (Pre-existing, moved verbatim.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Step 35 of the R2 module-extraction refactor: move the read-only Tab preview out of
src/main.jsinto a new native ES modulesrc/tab-preview.js.Convert the saved pack of the current fretted part to GP5 and engrave it with a CDN-loaded alphaTab (render-only, no synth) — a modal proofreading lens whose key policy blocks every editor shortcut behind it.
Why now (prerequisite for the keyboard layer)
This is a self-contained leaf (0 deps into main.js, imports only
S), and it's the prerequisite for a clean keyboard-command-layer cut next: the keyboard shortcut dispatcher calls_editorShowTabPreviewandonKeyDowncalls_tabPreviewKeyPolicyPure. Landing tab-preview as its own module first turns those into plain module→module imports instead of awkward host hooks (one of them is a pure function). Same pattern as landinghost.jsbefore the command-class steps.How the seam is drawn
window.editor*TabPreviewhandlers (re-attached by main.js) + the 2 symbols the keyboard layer still needs exported back (_editorShowTabPreview,_tabPreviewKeyPolicyPure).@pure:tab-previewblock is kept export-free (a trailingexport { … }statement) so the JS suite'snew Functionslice of that block stays valid.tests/tab_preview.test.js+tab_preview_race.test.jsretargeted to slice fromsrc/tab-preview.js.main.js drops ~178 lines. 36 modules.
Verification
no-undefclean ontab-preview.js.verify_tab_preview.py): 3 handlers bound,editorShowTabPreviewopens the modal + sets the title + runs the render path (status populated — the engraving itself needs the alphaTab CDN + Tab View plugin, out of headless scope),editorHideTabPreviewcloses it, zero page errors. Negative-checked by dropping a re-attach.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes