Skip to content

fix(app): register every chord that means zoom, not just the canonical one - #131

Draft
gennadiryan wants to merge 1 commit into
local/amicodefrom
fix/zoom-keybinds-amicode-266
Draft

fix(app): register every chord that means zoom, not just the canonical one#131
gennadiryan wants to merge 1 commit into
local/amicodefrom
fix/zoom-keybinds-amicode-266

Conversation

@gennadiryan

Copy link
Copy Markdown
Member

Important

Problem

Cmd/Ctrl +/- did not zoom inside the Amicode webview. For anyone on a layout where "=" is a shifted key, zoom was unreachable entirely; for everyone else it worked only via one specific keystroke out of the several that mean "zoom in".

Approach

Register every chord that physically means zoom, rather than only the canonical one. Key reason: nothing was wrong with the zoom implementation — keybind dispatch is an exact key-plus-modifier-mask lookup with no fallback, and the registered chords covered one of four real cases.

Approaches Considered

  • Widen the registered chord sets (chosen) — multi-binding is already supported by the parser and the keymap builder; this is configuration, not machinery.
  • Normalize shifted punctuation during matching so "+" folds onto "=" — fixes zoom and silently changes matching for every other keybind in the app; far too broad for the problem.
  • Add a fallback pass when an exact lookup misses — reintroduces ambiguity into a lookup that is deliberately exact, and would make chord collisions harder to reason about.

Scope

  • In: the zoom-in and zoom-out chord sets; first test coverage for zoom, including a seam assertion that the app registers those sets.
  • Out: zoom reset, which stays a single chord — "0" is unshifted on every layout we ship to, and widening it would start swallowing unrelated chords. The desktop build, which zooms through its menu accelerator and is unaffected. The matcher itself, unchanged.

Assumptions / Open Qs

  • Assumes the four cases below are exhaustive for the layouts we ship to. If a layout produces some other key for zoom-in, it needs adding to the same list — the fix is extensible by construction.

What was failing

Keybind dispatch builds a signature from the normalized key and a modifier mask, then does a map lookup with no fallback. Only mod+= and mod+- were registered.

What a user actually presses:

Keystroke Arrives as Registered before
Ctrl/Cmd + = (US main row) =, no shift yes
Ctrl/Cmd + Plus (US) — physically shift+= + → "plus", with shift no
Numpad Ctrl/Cmd + + + → "plus", no shift no
Ctrl/Cmd + = on DE/FR/Nordic — = is shifted =, with shift no

The reported keystroke missed on both the key name and the shift bit. The bare chord worked only on a US layout via the main-row = — which is why it survived: it works for whoever happens to try it that one way.

What changed

  • Zoom in: mod+=,mod+shift+=,mod+plus,mod+shift+plus
  • Zoom out: mod+-,mod+shift+_
  • Zoom reset: unchanged

Multi-binding needed no new machinery — the parser already splits on comma and the keymap builder already loops every parsed bind. Tooltips are unaffected: the display helper renders only the first chord, so users still see the canonical one.

Verification

  • packages/app unit suite: 828 passing, 0 failing.
  • New: six tests over zoom, which previously had none — no test anywhere referenced the zoom commands or their chords.
  • One of them asserts the narrow chord still misses Ctrl+Plus, so anyone tempted to revert the widening gets told why not.

Key Decisions

  • Widen the data, not the matcher. The exactness of the lookup is a feature; loosening it to fix zoom would change matching semantics app-wide.
  • Reset stays single. Widening a chord set has a cost — every added chord is one the app now swallows. 0 needs no help.
  • The tests assert the chord set and the registration. Asserting only the set would reproduce the failure mode that caused this bug: a correct mechanism with a wrong integration. The seam test reads the registration site and requires the exact strings.

Constraints & Invariants

  • The keybind matcher and its normalization are untouched.
  • Desktop behaviour is untouched — it zooms through the Electron menu accelerator, and registering these chords there would double-fire against its own handler. The commands remain gated to the web platform.

Source

Closes harmoniqs/amicode#266.

Found during a bug sweep of the amicode ↔ vendored-opencode seam; related seam work is harmoniqs/amicode#243.

Notes

Zoom is only reachable in the web/webview host, and the in-app command palette is the sole non-keyboard route to it. Inside the Amicode webview the palette shortcut is intercepted and forwarded to the editor's own palette, so before this change a user on a shifted-= layout had no way to zoom at all.


Addendum — remote GUI retest (2026-08-07)

A remote GUI retest of this branch showed the zoom chords still unresponsive:
the widened registration did not fix the symptom. This PR is therefore not
ready to merge
, and does not close harmoniqs/amicode#266.

What the failure indicates: keybind dispatch itself is exact-lookup with no
fallback, and the registration seam test here proves the widened chord sets
reach the keymap builder — so the remaining doubt is at the event layer
(which key values the host delivers inside the webview, and whether the chords
are intercepted before dispatch). The follow-up should capture the actual key
values and modifier masks the webview receives for those physical chords on
the failing host, rather than widening registration further. The test harness
added here remains the right seam to assert against once the real key values
are known.

…l one

Cmd/Ctrl +/- zoom did nothing in the amicode webview.

The zoom commands, the CSS-zoom implementation and the platform signal were all
present and correct. Only the keybind strings were wrong. Dispatch is an exact
(normalized-key, modifier-mask) lookup with no fallback, and only "mod+=" and
"mod+-" were registered.

Ctrl+Plus on a US layout is physically Ctrl+Shift+"=", which arrives as key "+"
(normalized "plus") carrying the shift bit — matching neither the key nor the
mask of a bare "mod+=". The numpad's "+" arrives unshifted. On layouts where "="
is itself shifted (DE/FR/Nordic) even the canonical chord carries shift. The
bare chord therefore worked only on a US layout via the main-row "=", which is
why it survived: it works for whoever happens to try it that one way.

Widens zoom-in to "mod+=,mod+shift+=,mod+plus,mod+shift+plus" and zoom-out to
"mod+-,mod+shift+_". Reset stays a single chord — "0" is unshifted everywhere we
ship, and widening it would start swallowing unrelated chords. Multi-binding is
already supported (parseKeybind splits on comma; the keymap builder loops every
parsed bind), and tooltips are unaffected since displayKeybind renders only the
first. Desktop is untouched: it zooms through the Electron menu accelerator.

Adds the first test coverage zoom has ever had, including a seam assertion that
layout.tsx registers these exact sets — the mechanism being right while its one
integration was wrong is how this shipped, and a test of the chord sets alone
would reproduce that.

Closes harmoniqs/amicode#266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Cmd/Ctrl +/- zoom keybindings do not work in Amicode

1 participant