Skip to content

fix(desktop): support JIS keyboard zoom shortcut - #38904

Draft
katanumahotori wants to merge 1 commit into
NousResearch:mainfrom
katanumahotori:codex/jis-zoom-shortcut
Draft

fix(desktop): support JIS keyboard zoom shortcut#38904
katanumahotori wants to merge 1 commit into
NousResearch:mainfrom
katanumahotori:codex/jis-zoom-shortcut

Conversation

@katanumahotori

Copy link
Copy Markdown

Summary

Adds JIS/Japanese keyboard-friendly zoom-in handling to the desktop zoom shortcut fallback.

The existing desktop zoom shortcut handler (from #37894) rejects all shifted inputs. On JIS keyboards, typing + commonly requires Shift, and Electron can surface that physical key path as ; or : depending on the layout/input state. This means Windows users on Japanese keyboards can still miss the zoom-in shortcut even though Ctrl+- and Ctrl+0 work.

This PR keeps the existing zoom behavior and only broadens zoom-in recognition:

  • keep requiring Ctrl/Cmd
  • keep ignoring Alt-modified shortcuts
  • keep Ctrl+=, Ctrl++, and numpad Add as zoom-in
  • add shifted ; / : as zoom-in for JIS/Japanese keyboards
  • keep zoom-out as unshifted -
  • keep reset as unshifted 0

Related

Follow-up to #37894 and related to #37917 / #37619. I also checked for existing Japanese/JIS-specific issues or PRs and did not find a duplicate.

Validation

  • node --check apps/desktop/electron/main.cjs
  • Built the Windows desktop package locally with python -m hermes_cli.main desktop --build-only
  • Confirmed apps/desktop/release/win-unpacked/Hermes.exe exists and launches on Windows

Co-Authored-By: OpenAI <support@openai.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 4, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the Shift guard in the zoom fallback. The premise still exists on current main: apps/desktop/electron/main.ts:4809 returns before handling any Shift-modified shortcut.

Problems

  • The PR edits apps/desktop/electron/main.cjs, but that file was renamed to apps/desktop/electron/main.ts by 39d09453f; current main has no tracked main.cjs. The patch therefore needs a manual port to main.ts:4806.
  • The added JIS/numpad key paths have no focused test coverage. apps/desktop/electron/zoom.test.ts currently covers helpers and wiring, not before-input-event key/modifier recognition.

Suggested changes

  • Port the widened zoom-in predicate to apps/desktop/electron/main.ts, retaining setAndPersistZoomLevel at lines 4817-4823.
  • Add coverage for shifted ;/:, =, +, Add, and rejected shifted reset/zoom-out combinations.

Automated hermes-sweeper review.

const key = input.key
if (key === '0') {
const key = String(input.key || '')
const isZoomReset = key === '0' && !input.shift

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main no longer tracks main.cjs: commit 39d09453f renamed it to apps/desktop/electron/main.ts. Please port this predicate to the current installZoomShortcuts handler there, preserving its setAndPersistZoomLevel calls.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants