Skip to content

fix(ui): make punctuation hotkeys layout-independent#9174

Open
DustyShoe wants to merge 11 commits into
invoke-ai:mainfrom
DustyShoe:fix(UI)-hotkeys-layout-independent-punctuation
Open

fix(ui): make punctuation hotkeys layout-independent#9174
DustyShoe wants to merge 11 commits into
invoke-ai:mainfrom
DustyShoe:fix(UI)-hotkeys-layout-independent-punctuation

Conversation

@DustyShoe

Copy link
Copy Markdown
Collaborator

Summary

Fix punctuation-based hotkeys in invokeai/frontend/web so they use layout-independent physical key identifiers instead of layout-dependent characters. This keeps canvas tool width shortcuts and related punctuation bindings working when a non-English keyboard layout is active. The change centralizes hotkey normalization and display formatting, canonicalizes legacy persisted bindings, and adds focused test coverage.

Also stabilize a flaky timeout assertion in invokeai/frontend/web/src/features/ui/layouts/navigation-api.test.ts so the required frontend build passes reliably under full-suite load.

Related Issues / Discussions

N/A

QA Instructions

  1. Open the canvas with a non-English keyboard layout active.
  2. Select the brush or eraser tool.
  3. Press the physical [ and ] keys and confirm the tool width changes.
  4. Open the hotkeys modal and confirm the bindings still display as [ and ].

Merge Plan

None.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • ❗Changes to a redux slice have a corresponding migration
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions Bot added the frontend PRs that change frontend files label May 14, 2026
@lstein lstein self-assigned this May 30, 2026
@lstein lstein added the 6.14.x label May 30, 2026
@lstein lstein moved this to 6.14.x Theme: USER EXPERIENCE in Invoke - Community Roadmap May 30, 2026
@lstein

lstein commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Code review

Overview. The core mechanism is sound — react-hotkeys-hook 4.5.0 matches the new tokens (bracketleft, bracketright, period) via mapKey(event.code), so the headline fix (brackets working on non-US layouts) genuinely works, and the fake-timers rewrite of navigation-api.test.ts is strictly tighter than the old real-timer assertion with no coverage loss. However, review found real regressions for keyboards where punctuation glyphs live on different physical keys (AltGr layouts), plus several design-level issues. Findings 1–3 share one root cause: glyph→token rewriting that assumes the glyph came from the US-layout physical key.

Findings (most severe first)

  1. hotkeyStrings.ts:59normalizeHotkeyKey rewrites punctuation glyphs to physical-key tokens without knowing which physical key produced them, retargeting recorded hotkeys to the wrong key.
    A German QWERTZ user records AltGr+8, which types [ with e.code='Digit8'. Digit8 isn't in HOTKEY_KEY_ALIASES_BY_CODE, so the glyph falls through getHotkeyKeyFromEvent and is then rewritten to bracketleft by normalizeHotkeyKey. The saved hotkey never fires on AltGr+8 (the library sees keyCode '8' / pressedKey '[') and instead fires on the physical BracketLeft key — the ü key. Pre-PR the stored literal [ matched via event.key and worked.

  2. useHotkeyData.ts:66 — read-time canonicalization silently rebinds legacy persisted glyph bindings recorded on non-US layouts to a different physical key.
    Same mechanism as Not working on windows ( no readline ) but pyreadline is avalilable #1 on the persistence path: a pre-PR custom [ binding that a QWERTZ user triggered via AltGr+8 is rewritten to bracketleft on every load, so after upgrading, their working chord stops firing and the ü key starts triggering the action — with no notice, since the modal still displays [.

  3. hotkeyStrings.ts:90canonicalizeHotkeyString misses legacy shifted glyphs (>, <, {, :), so conflict detection misses equivalents and one keystroke fires two hotkeys.
    The pre-PR recorder stored shift+> for Shift+Period. That string survives canonicalization; a post-PR recording of Shift+Period on another action produces shift+period; conflictMap.get('shift+period') can't hit shift+>, so both save, and one Shift+. keydown fires the legacy binding via pressedKey '>' and the new one via keyCode 'period'. Narrow (requires a pre-existing shifted-punctuation custom binding) but fully reproducible.

  4. hotkeyStrings.ts:97 — display and modal search use a hardcoded US glyph table, so the exact users this PR targets see keycap labels that don't exist on their keyboard.
    A QWERTZ user sees "Increment tool width: ]" but the triggering keycap is labeled +; ЙЦУКЕН users see [ for the х key; the HotkeysModal search filter matches only the US glyph via platformKeys. navigator.keyboard.getLayoutMap() would resolve the real glyphs as a Chromium-only progressive enhancement, with the US table as fallback.

  5. HotkeyListItem.tsx:171 — the recorder unconditionally coerces the 11 punctuation codes to physical tokens, discarding the letter the user typed.
    An AZERTY user pressing M (e.code='Semicolon') records semicolon and the modal displays ; for a keycap labeled M; a Russian user intending to bind the letter х gets bracketleft, which conflicts with the tool-width default and hard-disables the save button. Bindings still fire on the same physical key, so the harm is display accuracy and intent rather than function — and the conflict block is arguably correct since the keys physically collide — but it's worth a deliberate decision.

  6. useHotkeyData.ts:66 — legacy bindings are canonicalized on every rebuild forever instead of via the repo's established one-time persist migration.
    hotkeysSlice already has _version: 1 and a wired persistConfig.migrate hook, and upscaleSlice.ts shows the exact v1→v2 convention. A migration mapping customHotkeys values through canonicalizeHotkeyString would retire the per-read shim (the PR checklist's migration box is unchecked).

  7. hotkeyStrings.ts:11 — the same 11-key punctuation correspondence is hand-maintained in four tables, and normalizeHotkeyKey allocates a fresh ~17-entry object per call.
    Code→token (HOTKEY_KEY_ALIASES_BY_CODE), token→glyph (HOTKEY_KEY_DISPLAY_ALIASES), plus byte-identical glyph→token blocks in both the mac and non-mac branches of normalizeHotkeyKey. Adding a key (e.g. IntlBackslash) means editing four places, and a missed edit silently breaks record/display for that key. The allocation runs ~294× per buildHotkeysData, which is memoized per component across ~91 useRegisteredHotkeys call sites. Deriving all maps from one {code, token, glyph} table at module scope fixes both.

  8. useHotkeyData.ts:105 — physical-key tokens only match via event.code, so bracket hotkeys stop firing for events with an empty code (synthetic new KeyboardEvent('keydown', {key:'['}) from test automation, low-fidelity remote-desktop clients). Pre-PR the literal [ matched via the event.key path. Mainstream real/virtual keyboards populate code, so this is low impact — mostly a test-automation concern.

Verified non-issues

  • Numpad punctuation recording produces bindings the numpad can't trigger, but this was equally broken pre-PR (the library maps a registered . to period anyway) — not a regression.
  • The IS_MAC_OS move/re-export resolves cleanly with no import cycle, and no other consumer renders raw hotkey strings.
  • Reusing the library's useRecordHotkeys instead of the bespoke recorder was considered and rejected: it never emits mod, doesn't remove keys on keyup, and its mapKey isn't safely deep-importable (no runtime dist/parseHotkeys.js).

Suggested direction: apply the glyph→token mapping only when e.code confirms the physical key (i.e., rely on getHotkeyKeyFromEvent and drop the glyph entries from normalizeHotkeyKey's general path), and do legacy canonicalization as a hotkeysSlice v1→v2 persist migration. That addresses findings 1, 2, 3, and 6 together.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.14.x frontend PRs that change frontend files

Projects

Status: 6.14.x Theme: USER EXPERIENCE

Development

Successfully merging this pull request may close these issues.

2 participants