Register alternative keyboard shortcuts for zooming in - #2426
Open
robertobandini wants to merge 2 commits into
Open
robertobandini wants to merge 2 commits into
robertobandini wants to merge 2 commits into
Conversation
The "zoomIn" Electron role defaults to the "CommandOrControl+Plus" accelerator, which Electron resolves to Control, Shift and the key that Windows maps to VKEY_OEM_PLUS. On layouts where "+" is not the shifted "=" (German, Italian and others) that combination cannot be typed, so Zoom In never fires from the keyboard while Zoom Out, whose accelerator needs no Shift, keeps working. Register the missing accelerators as hidden menu items: hidden items are still part of the menu, so Electron registers their accelerators without showing duplicate entries. Zoom In gains "CommandOrControl+=" and the plus of the numeric keypad, Zoom Out the keypad minus. Fixes #1848 Co-authored-by: Roberto Bandini <11294865+robertobandini@users.noreply.github.com>
Member
Author
|
I built this branch on Windows (portable Node 24.18.0, no elevated access needed) and tested the packaged app personally with the Italian QWERTY keyboard layout: I confirm the fix works — Zoom In is now triggered correctly from the keyboard. This covers the "not yet verified on Windows" part of the PR description. As per our usual practice, I'm requesting a review from @dex4er before merging. |
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
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.
Zoom In could not be triggered from the keyboard on layouts where
+is not the shifted=.Root cause
Electron's
zoomInrole defaults to theCommandOrControl+Plusaccelerator. In Electron 42.7.1 (shell/common/keyboard_util.cc) theplustoken maps to{VKEY_OEM_PLUS, '+'}with a shifted character, soaccelerator_util.ccaddsEF_SHIFT_DOWN: the accelerator is literally Ctrl + Shift + VKEY_OEM_PLUS. That only matches keyboards where+is the shifted=(US/UK). On Italian, German, Nordic and similar layouts the key Windows reports asVK_OEM_PLUSis the dedicated+key, pressed without Shift, so the combination never matches; the numeric keypad+(VKEY_ADD) never matches on any layout. Zoom Out is unaffected becauseCommandOrControl+-needs no Shift. Clicking the menu item bypasses accelerator matching, which is why the mouse path works.Fix
An Electron
MenuItemholds a single accelerator, so the alternatives are registered as hidden menu items:GenerateAcceleratorTablewalks the menu model without looking at visibility (it only honoursregisterAccelerator, which istruefor the zoom roles), so avisible: falseitem keeps its shortcut without adding a duplicate entry to the View menu.OsActionMenuItemgains an optionalvisibleflag (MayBeHiddenFromMenu), passed through to the Electron templateCommandOrControl+=(the unshiftedVKEY_OEM_PLUS:+on IT/DE keyboards,=on US ones, also the common Chrome-style shortcut) andCommandOrControl+numaddCommandOrControl+numsubzoom-keyboard-shortcuts.test.ts(one visible item per direction, alias accelerators present), snapshot and DI registration updatedApplies to v2 (
main); the same three source files are byte-identical onv1, so a backport is a clean cherry-pick (the new test needs porting from vitest to jest).Verification
Run locally on macOS (x64) against this branch, replicating the PR workflows:
type:check,build:di(no drift),knip:check,trunk check --all: cleanelectron-builder --macos dir) + integration tests: passed (KinD-dependent tests skipped as in CI)Not yet verified: the actual key presses on Windows. The unit test only proves the Electron menu template carries the accelerators; the matching happens in native code and depends on the keyboard layout. Someone with Windows and an Italian/German layout should check Ctrl +
+(main key), Ctrl + keypad+, Ctrl + keypad-, and on a US layout that Ctrl+Shift+= still works and Ctrl+= now works too. If nobody gets to it, I will do it myself ASAP.Fixes #1848
Generated with Claude Code
| Model:
claude-opus-5(analysis and fix),claude-fable-5(review and verification)