Keybinds 3/4: migrate Space and Escape without clobbering the host's built-ins #39

Activity

1. cchacs

   added a parent issue on Oct 2, 2026

   • Hardcoded keybinds in onKeyDown bypass window.registerShortcut #10
2. cchacs

   added

   enhancementNew feature or requestNew feature or request

   on Oct 2, 2026
3. pullfrog commented on Oct 2, 2026

   

   pullfrogBot

   on Oct 2, 2026 – with Pullfrog

   Last edited by pullfrog

   More actions

   Plan — Keybinds 3/4: migrate Space and Escape without clobbering the host built-ins (#39)

   Context

   #38 (PR #65) already built the bridge this issue extends:

   • src/shortcut-registry.js — registerEditorShortcuts() feature-detects window.registerShortcut, registers the five editing keys under plugin-editor, re-checks the onKeyDown guards inside each handler, rolls back atomically on a partial failure, and unregisters on teardown. src/shortcut-state.js holds editorShortcutState.registered, which input.js reads to stand its fallback down.
   • src/main.js registers in init() and unregisters in _teardownScreenSoft().

   Where the two keys live today (src/input.js):

   • Space — input.js:2053-2056: if (e.key === ' ' && !typing) { preventDefault(); return _editorRunEofCommand('togglePlay'); }. It sits after the read-only lens gates and the tool-palette gate, and before the _recState === 'recording' guard (:2064) — so it is swallowed by a lens, but live mid-take to finalize the recording (window.editorTogglePlay routes to editorStopRecordMidi, main.js:1698-1712).
   • Escape — an ordered ladder of eight owners: tab-preview lens, user-guide lens, context/section menu, tool palette (:1922, :1940, :1962, :1972), tap-tempo run, suggested-fit ghosts, barline multi-selection, note/drum selection (:2219, :2223, :2234, :2253). When none consumes it, onKeyDown returns without preventDefault(), leaving Escape to the host's back-to-library. main.js:1003-1032 (add-note/load/palette), src/tempo-zones.js:278 and src/anchor-resolve.js:395 own Escape on their own listeners.

   Note the CHANGELOG already lists Space as a registry command for the plugin's own ? panel / command palette (#43 entry, CHANGELOG.md:24-30). That is a distinct surface from the Host ? panel this issue targets — don't treat it as already done.

   Decisions to confirm before coding

   1. Scope is plugin-editor for both keys, not player. The 1/4 table recommended player for Space; Keybinds 2/4: migrate the editing keys (Delete, Backspace, G, F, K) to registerShortcut #38 already corrected Delete from player to plugin-editor because player is a distinct host screen, not a generic transport scope. The editor's Space drives the editor's own startPlayback/stopPlayback (src/audio.js), not the host player's song playback — a player-scoped registration would fire it on the host player screen, violating "no longer work outside its screen". plugin-editor is the narrowest correct scope for both.
   2. Escape's fall-through is load-bearing. The registered handler must preventDefault() only when it actually consumed the key, or the editor starts eating the host's back-to-library from the editor screen. This is the structural half of the collision resolution and the part easiest to regress.
   3. Space must not reuse _editorShortcutAllowed as-is. That guard bails on _recState === 'recording' (right for Delete/drum); Space needs the screen/typing/lens/palette guard minus the recording check so a take still finalizes. Add a parameter or a dedicated guard rather than weakening the shared one.
   4. Escape must not use _editorShortcutAllowed either. It bails when the tab-preview / user-guide lenses are open — but Escape's job in those states is to close them. Escape needs its own guard (screen active + not a typing target), with the lens/menu/palette owners inside the ladder.
   5. Key token + host dispatch semantics are unverified (carried from 1/4's open questions, still unanswered in this repo — no vendored core). Confirm against the installed host: (a) the key value for Space (' ' vs 'Space'; the plugin's own registry uses 'Space'); (b) whether a plugin-editor registration suppresses or co-fires with the host's own Space/Escape; (c) whether the host honors our non-consumption (no preventDefault) by running its own handler; (d) whether Space is an Electron menu accelerator (then preventDefault() is powerless and both toggle). If Space co-fires irreconcilably, the documented fallback is to drop the plugin's Space registration and record why in the PR.

   Milestones

   M1 — Extract the shared handlers (no behavior change). Mirror #38's _editorDeleteSelection.

   • Add exported _editorEscape(e) in src/input.js: the eight owners in today's order, including the recording gate between the tool palette and the tap-tempo owners, returning true only when it consumed the key (never preventDefault internally).
   • Add exported _editorTogglePlayShortcut(e) (typing/lens/palette/screen guard, recording allowed) that runs _editorRunEofCommand('togglePlay') and returns handled.
   • Rewire onKeyDown to call both in the same positions, gated on !editorShortcutState.registered, so the fallback path is equivalent when the host API is absent.

   M2 — Register Space + Escape.

   • Extend _SHORTCUT_SPECS with Escape ("Close the top layer, or clear the selection") and Space ("Play / pause"), both plugin-editor.
   • _onEscapeShortcut: bail on modifier chords; bail (return without acting) when the host-owned transient modals — add-note / load / command-palette — are open, so it cannot clear the selection behind them; otherwise run _editorEscape and preventDefault() only on true.
   • _onSpaceShortcut: bail on modifier chords + the lens/palette/typing/screen guards; run the toggle.
   • The existing atomic rollback and teardown unregister generalize (the _registered array is per-spec).

   M3 — Stand the onKeyDown branches down.

   • Space branch: when editorShortcutState.registered, return without acting.
   • Escape ladder: when registered, return after the screen-active check so the host handler is the only path.
   • Leave the main.js listener and the tempo-zones.js / anchor-resolve.js Escape listeners untouched — Keybinds 4/4: remove the document-level key listener and verify the help panel #40 owns removing the document listener and reconciling those.

   M4 — Tests (tests/keybind_registration.test.mjs, plus updates).

   • Registration: seven specs total; Space/Escape scoped plugin-editor with descriptions (update the current five-key assertion).
   • Space: typing target no-ops; a lens open no-ops; mid-take fires the finalize; preventDefault only when it acted.
   • Escape collision cases from 1/4: each owner in order; the overlapping "suggestion ghost and barlines selected" case resolves to the ghost owner (a case Keybinds 2/4: migrate the editing keys (Delete, Backspace, G, F, K) to registerShortcut #38 had to reason about); the no-owner case does not preventDefault (host keeps back-to-library); selection intact when the context menu owns it; add-note/load/palette open do not clear the selection.
   • Fallback: with the host API absent, onKeyDown still toggles play and runs the full ladder; update tests/user_guide_gate.test.mjs, tests/escape_deselect_menu.test.mjs, tests/tab_preview.test.js, tests/click_tools.test.mjs if the early-return gating changes the path they exercise.

   M5 — Docs. Add a CHANGELOG.md [Unreleased] ▸ Added entry (top of the section): Space/Escape now register with the Host shortcut service and surface in the global ? panel, with the in-editor behavior and guards preserved; keep it distinct from the existing in-plugin-panel Space entry.

   Acceptance-criteria mapping

   • Space/Escape behave as before in-editor, host built-ins unchanged elsewhere → plugin-editor scope + preserved guards + Escape fall-through (M1–M3).
   • No double-handling → editorShortcutState.registered stand-down + modifier-bailing handlers + the host-semantics verification (M2/M3, Decision 5).
   • Tests cover the 1/4 collision cases → M4.

   Out of scope

   The other keys (#38, done); removing the document listener / final ? verification (#40).

   Risks

   • The host-dispatch questions (Decision 5) are the reason this issue is called the riskier one; they gate whether a naive Space registration double-toggles. Verify before shipping, and fall back per the audit if the host cannot suppress its own binding.
   • _editorEscape must keep the recording gate at its current position, or Escape gains new behavior mid-take.

     ^{｜ Implement plan ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using big-pickle (free) ｜ 𝕏}
4. get-flashbacks

   deleted a comment from pullfrog on Oct 9, 2026
5. pullfrog

   added a commit that references this issue on Oct 9, 2026

   refactor(input): split _editorEscape into rung helpers for CodeFactor…

   ...

   f4fe84a
6. cchacs

   closed this as completedin #66on Oct 9, 2026
7. cchacs

   added 2 commits that reference this issue on Oct 9, 2026

   feat(editor): register Space/Escape with the host shortcut service (#39…

   ...

   7e60c5c

   refactor(input): split _editorEscape into rung helpers for CodeFactor…

   ...

   973bad0