feat(editor): standard notation in the score view (View ▸ Score staff) - #274
Closed
ChrisBeWithYou wants to merge 6 commits into
Closed
feat(editor): standard notation in the score view (View ▸ Score staff)#274ChrisBeWithYou wants to merge 6 commits into
ChrisBeWithYou wants to merge 6 commits into
Conversation
The live score view grows a staff preference: tablature only (default,
per the v1 call), standard notation only, or both staves together —
same generated alphaTex either way, alphaTab derives pitch from
tuning + fret, so notation comes free of new generation code.
- tab-view-live.js: _scoreStaffProfilePure maps the preference to
alphaTab's StaveProfile key (unknown/legacy stored values degrade to
tab, never throw into the renderer); the api tracks the staff it was
built with and rebuilds on change (staveProfile is construction-time);
editorSetTabViewStaff validates + persists (localStorage, a reading
preference like loop-snap) and ENTERS the view when it's off.
- menu-bar.js: a Score-staff radio trio in the View menu, same ctx/
dispatch pattern as the loop-snap trio; checkmarks resolve at open.
- state.js: S.tabViewStaff ('tab' default).
Live-verified on :8001 (AC/DC pak): all three staves engrave (tab
frets / treble-clef pitches with accidentals / braced grand system),
click-to-select works under 'both' (3 notes), the real View-menu radio
renders + dispatches + enters the view, preference persists. 148/148,
lint baseline, new tab_view_staff suite + menu-model radio pin.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…feat/editor-notation-staff
…build
The Score-staff switch made _ensureApi rebuild the alphaTab api on the
SAME mount node (staveProfile is a construction-time setting). But
_destroyApi only tore down alphaTab's own api — it never removed our
capture-phase mousedown fallback, which is our own closure. So every
staff toggle stacked another live listener on the surviving mount, and
each subsequent beat click fired select() / editorSeekToTime() / status
updates once per accumulated listener.
Root cause: the manual mount.addEventListener('mousedown', ..., true) in
_ensureApi had no matching removeEventListener. Name the handler, drop it
in _destroyApi before nulling _apiMount. In PR #273 this path never fired
on a surviving node (rebuild happened only on a NEW mount, GC'd the old
listener), so the leak is specific to the staff-switch guard this PR adds.
Regression test (tests/tab_view_staff.test.mjs): three staff switches on
one mount keep exactly one mousedown listener; fails pre-fix (2 !== 1).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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
The standard notation follow-up to #273: the live score view gains View ▸ Score staff — a radio picking what the engraving shows: tablature only (the default, per the v1 call), standard notation only, or both staves together. The pick is a per-browser reading preference (persisted like the loop-snap mode), clicking a beat selects its notes under every staff choice, and choosing a staff while the score view is off enters it — choosing what to read implies wanting to read.
No new generation code: the same alphaTex drives all three (alphaTab derives pitch from tuning + fret), so notation-only shows real pitches with accidentals and "both" engraves the braced grand system.
How
src/tab-view-live.js—_scoreStaffProfilePuremaps the preference to alphaTab'sStaveProfileenum key (unknown/legacy stored values degrade totab, never throw into the renderer). The api now tracks the staff it was built with and rebuilds on change (staveProfileis a construction-time setting).editorSetTabViewStaffvalidates + persists + arms a re-render;editorTabViewStafffeeds the menu checkmarks.src/menu-bar.js— a Score-staff radio trio in the View menu, exactly the loop-snap trio's ctx/dispatch pattern (a ctx withoutscoreStaffrenders unchecked, older callers never crash).src/state.js—S.tabViewStaff, default'tab'.Testing
tests/tab_view_staff.test.mjs(6 cases, fails on main): the enum mapping + degradation, restore-on-load from localStorage, setter validate/persist/no-re-persist, and the enters-the-view behavior.tests/menu_model.test.mjsgains a radio pin: three rows, exactly the active staff checked, dispatch payloads, unchecked degradation for older ctx callers.🤖 Generated with Claude Code
https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q