Sign up
View stargazers13Sign up

fix(desktop): handle macos command-plus zoom input#18439

merged7
main
GitHub fork: fix/macos-command-plus
opened14d agobyMmaria
4 files changed
View on GitHubTwo-way sync enabledPR last imported 6d ago

macOS Command-plus can miss the app zoom accelerator. Add a hidden shifted-Equal alias and direct app zoom to the registered main window, so a focused popup cannot receive app zoom. Keep the existing preview zoom reapplication and renderer shortcut paths.

Rebuilt @maria-rcks’s focused fix from #18439 on current main. Removed the duplicate numpad-plus alias because main now supplies it on every platform. Reduced the original test expansions to two existing behavioral cases; removed the before-input no-op matrix because it did not prove renderer/native ordering. No new test cases were added (45 → 45).

Validation:

  • Base production with the retained regressions: 43 passed, 2 failed (missing shifted-Equal and zooming a focused popup).
  • Rebuilt implementation: all 45 focused menu/window tests passed.
  • Two existing preview-manager tests passed: guest shortcut isolation and stored preview zoom reapplication.
  • Desktop typecheck, scoped lint, formatting, and diff checks passed.

Independent adversarial reviews: Claude claude-opus-5-5 and Codex gpt-6-sol, one clean round with no concrete findings. Both require native macOS acceptance before an end-to-end claim.

The tests use mocked Electron windows on headless Linux. ELECTRON_OVERRIDE_DIST_PATH bypassed the package’s failed binary download for those mocked tests; no Electron process was launched. Native macOS key dispatch, one physical keypress producing one zoom step, keyboard-layout behavior, renderer/native menu ordering, and live guest zoom remain unverified. A native macOS pass and before/after interaction recording are still required before calling those claims verified. Windows, iOS, Android, and Electron desktop were not tested live. Web, mobile, provider, and wire behavior have no source changes.

Tests: Codex gpt-6-astra (high). Implementation: Codex gpt-6.1-sol (high). Maintainer coordination: gpt-6.1-sol through the Codex harness in T3 Code.

Checks

21 successful · 11 skipped
All checks passed
59ce1e7

Added 4 commits

  • fix(desktop): handle macos command-plus zoom input641419f
  • fix(desktop): preserve preview zoom shortcut handlingc320540
  • style(desktop): format zoom menu regression test06152eb
  • chore: merge main7bd7fa3
MmacroscopeappBot11d ago
Synced from GitHub

Approvability

Verdict: Approved at 3e40442

Macroscope's review found this PR approvable — This is a narrowly scoped desktop bug fix that adds a macOS command-plus alias and keeps zooming directed at the registered main window instead of auxiliary windows. The production changes are isolated and accompanied by focused tests, with no schema, deployment, security, or static-analysis impact.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot11d ago
Synced from GitHub

Review in Change Stack →

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 03535c29-b746-4c99-a6d4-427c22084f4a
📥 Commits

Reviewing files that changed from the base of the PR and between 7bd7fa3c694c693c2208d8097abcb3b3fd37ed67 and 59ce1e730aebaddcc5b345f46ed7766734f13f96.

📒 Files selected for processing: 4
  • apps/desktop/src/window/DesktopApplicationMenu.test.ts
  • apps/desktop/src/window/DesktopApplicationMenu.ts
  • apps/desktop/src/window/DesktopWindow.test.ts
  • apps/desktop/src/window/DesktopWindow.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 35073c9d-df03-4815-973c-c925a795c5b3
📥 Commits

Reviewing files that changed from the base of the PR and between 348f217a853dd4a23d7fa4834dedb05f545c2075 and 7bd7fa3c694c693c2208d8097abcb3b3fd37ed67.

📒 Files selected for processing: 4
  • apps/desktop/src/window/DesktopApplicationMenu.test.ts
  • apps/desktop/src/window/DesktopApplicationMenu.ts
  • apps/desktop/src/window/DesktopWindow.test.ts
  • apps/desktop/src/window/DesktopWindow.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The desktop View menu adds two macOS Zoom In accelerators. Main-window zoom now targets the registered main window rather than the focused window. Tests cover accelerator routing and zoom behavior when a popup has focus.

Changes: Desktop Zoom Routing

Layer / File(s)Summary
Route zoom to the main window
apps/desktop/src/window/DesktopWindow.ts, apps/desktop/src/window/DesktopWindow.test.ts		zoomMain selects the registered main window. Tests check that zoom-like inputs do not change the main window or popup zoom when the popup has focus.
Add macOS zoom shortcuts
apps/desktop/src/window/DesktopApplicationMenu.ts, apps/desktop/src/window/DesktopApplicationMenu.test.ts		The View menu adds hidden CmdOrCtrl+Shift+= and CmdOrCtrl+numadd Zoom In entries on macOS. Tests check platform-specific accelerators and dispatch.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 7bd7f

No demonstrated issue blocks merging after normal checks. Native macOS shortcut behavior remains to be validated.

### Architecture Summary

Architecture risk: 🔵 Low · up to 7bd7f

The change affects 1 system.

Changed systems: apps/desktop

Architecture concerns No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/desktop (service) was modified; 4 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/desktop/src/window/DesktopApplicationMenu.test.ts: Expanded the zoom-routing test from one default-platform accelerator check to per-platform, per-accelerator checks. It now expects two accelerators on Linux and Windows, four on macOS, the same accelerator list on each platform’s Zoom In items, exactly one visible item, and zoom-in dispatch when each matching item is clicked. The existing checks for absent zoom, reload, and devtools roles remain.
  • observed — Modified behavior in apps/desktop/src/window/DesktopApplicationMenu.ts: Adds two hidden macOS-only Zoom In accelerators, CmdOrCtrl+Shift+= and CmdOrCtrl+numadd, both wired to the existing zoom-in handler; other platforms add neither entry.
  • observed — Modified behavior in apps/desktop/src/window/DesktopWindow.test.ts: Renames the test to describe independent preview zoom and adds a fake popup window alongside the main window.
  • observed — Modified behavior in apps/desktop/src/window/DesktopWindow.test.ts: Configures the test layer’s focused window as the popup while retaining the main window as the zoom target.
Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the problem, implementation, tests, limitations, and unverified native behavior. It does not provide the required explicit maintainer approval evidence, and it does not includ…Add a Scope and approval section with the triaged issue or discussion and the explicit maintainer approval comment. Add before/after screenshots and a short interaction recording if motion or interaction details require it. Keep the stated …
Passed Passed checks: 3 passed
Check nameStatusExplanation
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Title checkPassedThe title clearly identifies the desktop macOS command-plus zoom fix and matches the primary change.
Full details: Description check

Explanation

The description explains the problem, implementation, tests, limitations, and unverified native behavior. It does not provide the required explicit maintainer approval evidence, and it does not include the required before/after UI evidence or interaction recording.

Resolution

Add a Scope and approval section with the triaged issue or discussion and the explicit maintainer approval comment. Add before/after screenshots and a short interaction recording if motion or interaction details require it. Keep the stated native macOS and CI limitations until those checks pass.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

JJulius Marminge7d ago

Taking over this one as maintainer for the composer-chat-09 batch. I will preserve the focused macOS zoom fix, reconcile the aliases with current main, and push follow-up commits here. Thanks @maria-rcks.

Added 2 commits

  • Merge remote-tracking branch 'origin/main' into fix/macos-command-plus69c00b7
  • fix(desktop): keep macOS zoom aliases unique on current main3e40442

Added 1 commit

  • Merge remote-tracking branch 'origin/main' into l25/1843959ce1e7