Repository navigation
Add global hotkey overlay panel for #2758 - #4026
austinywang wants to merge 24 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an NSPanel-backed SwiftUI global hotkey overlay summonable from any app (including full-screen), threads a session-aware main-window role through ContentView/AppDelegate to exclude the panel from session restore, and updates localization, web/docs, project wiring, and tests. ChangesGlobal Hotkey Panel Feature
Sequence DiagramsequenceDiagram
participant AppDelegate
participant GlobalHotkeyPanelController
participant GlobalHotkeyPanel
participant GlobalHotkeyPanelContentState
AppDelegate->>GlobalHotkeyPanelController: toggle()
GlobalHotkeyPanelController->>GlobalHotkeyPanel: ensurePanel() / apply configuration
GlobalHotkeyPanelController->>GlobalHotkeyPanel: show() / makeKeyAndOrderFront
GlobalHotkeyPanelController->>GlobalHotkeyPanelContentState: scheduleConfigLoadAfterFirstDisplay()
GlobalHotkeyPanel->>GlobalHotkeyPanelController: cancelOperation -> onCancel -> hide()
GlobalHotkeyPanelController->>GlobalHotkeyPanel: hide() / orderOut
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR replaces the "hide all cmux windows" global hotkey behavior with a dedicated floating overlay panel (
Confidence Score: 5/5Safe to merge — all lifecycle boundaries (session restore, close-quit prompts, window cycling, focus restore) are correctly isolated for the overlay role. The overlay panel is registered via the ObjectIdentifier-keyed mainWindowContexts dict, so setActiveMainWindow and restoreActiveMainWindowAfterHiding work correctly despite the non-standard identifier. The isSessionRestorable=false role gates out startup session restore, session snapshot, and close-warning paths. Actor isolation is explicit. Escape is handled through a single cancelOperation path. Locale coverage is complete across all 20 locales. Five new unit tests cover the critical contract surfaces. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppDelegate
participant Controller as GlobalHotkeyPanelController
participant Panel as GlobalHotkeyPanel
participant PrevWindow as Previous Standard Window
User->>AppDelegate: toggleApplicationVisibilityFromGlobalHotkey()
AppDelegate->>Controller: toggle()
alt Panel not yet created
Controller->>Panel: create NSPanel (nonactivating, canJoinAllSpaces, fullScreenAuxiliary)
Controller->>AppDelegate: registerMainWindow(panel, role: .globalHotkeyPanel)
end
alt Panel hidden
Controller->>Panel: orderFrontRegardless() + makeKey()
Controller->>AppDelegate: setActiveMainWindow(panel)
else Panel visible
Controller->>Panel: orderOut(nil)
Controller->>AppDelegate: restoreActiveMainWindowAfterHiding(panel)
AppDelegate->>PrevWindow: activateMainWindowContext(prevVisibleRestorable)
end
User->>Panel: Press Escape
Panel->>Controller: cancelOperation → onCancel() → hide()
Reviews (13): Last reviewed commit: "fix: normalize project file" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Addressed Greptile's summary-only startup-restore note in 727e36d: registerMainWindow now only runs startup restore and follow-up registration snapshot saves for session-restorable roles, so the global hotkey panel cannot consume the one-shot startup restore path. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Resources/Localizable.xcstrings (1)
72652-72734:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix missing locales for updated globalHotkey strings in Resources/Localizable.xcstrings (lines ~72652–72734)
The updated keys
settings.globalHotkey.enable,settings.globalHotkey.enable.subtitleOn,settings.globalHotkey.note, andsettings.globalHotkey.shortcutonly includeen,ja, andkolocalizations, but the catalog already supports additional locales:ar,bs,da,de,es,fr,it,km,nb,pl,pt-BR,ru,th,tr,uk,zh-Hans,zh-Hant. Add proper translations (no placeholder/machine markers) for all missing locales.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Resources/Localizable.xcstrings` around lines 72652 - 72734, The new localization entries for the keys settings.globalHotkey.enable, settings.globalHotkey.enable.subtitleOn, settings.globalHotkey.note, and settings.globalHotkey.shortcut currently only include en/ja/ko; update Resources/Localizable.xcstrings to add the missing locale blocks for ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant for each of those four keys, supplying proper translations (no placeholders or machine-marked values) consistent with existing stringUnit structure so each localization has state and value fields like the other languages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxTests/GlobalHotkeyPanelControllerTests.swift`:
- Around line 50-51: The two failing assertions in
GlobalHotkeyPanelControllerTests should use inclusive boundaries: replace
XCTAssertGreaterThan(frame.width, screenFrame.width * 0.85) and
XCTAssertGreaterThan(frame.height, screenFrame.height * 0.75) with
XCTAssertGreaterThanOrEqual so the min-size contract accepts exact 85%/75%
values; update both assertions (the ones referencing frame.width/frame.height
and screenFrame.width/screenFrame.height) to XCTAssertGreaterThanOrEqual with
the same right-hand expressions.
In `@Resources/Localizable.xcstrings`:
- Around line 72600-72622: The new localization key globalHotkey.window.title
only includes en/ja/ko but must include every locale the catalog supports; add
entries for the missing locales (ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR,
ru, th, tr, uk, zh-Hans, zh-Hant) under the same key in
Resources/Localizable.xcstrings, set each stringUnit.state to "translated" (or
appropriate state) and provide the correct localized value for each locale (or
placeholder translation if necessary) so the key has a localization object for
each supported locale.
---
Outside diff comments:
In `@Resources/Localizable.xcstrings`:
- Around line 72652-72734: The new localization entries for the keys
settings.globalHotkey.enable, settings.globalHotkey.enable.subtitleOn,
settings.globalHotkey.note, and settings.globalHotkey.shortcut currently only
include en/ja/ko; update Resources/Localizable.xcstrings to add the missing
locale blocks for ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr, uk,
zh-Hans, and zh-Hant for each of those four keys, supplying proper translations
(no placeholders or machine-marked values) consistent with existing stringUnit
structure so each localization has state and value fields like the other
languages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8baef861-8885-4b0f-98b8-e3e821a9b3b9
📒 Files selected for processing (12)
README.mdResources/Localizable.xcstringsSources/App/GlobalHotkeyPanelController.swiftSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SettingsNavigation.swiftSources/SettingsSearchAliases.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/GlobalHotkeyPanelControllerTests.swiftweb/data/cmux-shortcuts.ts
|
Addressed the latest Greptile localization note in ea3c98a: globalHotkey.window.title now has entries for all locales currently present in the catalog, including ar, nb, zh-Hans, zh-Hant, and km. |
|
Addressed CodeRabbit's outside-diff localization request in 454d53c: the global hotkey section, enable subtitles, note, shortcut label, window title, and related global-hotkey search aliases now cover every locale present in the catalog. Also applied the inclusive layout assertion suggestion from the inline test comment. |
|
Follow-up for the full internationalization feedback: commit 13f0b38 adds all 20 web routing locales for the hotkey shortcut description/note in web/data/cmux-shortcuts.ts and updates both docs consumers to prefer locale-specific shortcut text with English fallback. I also rechecked the globalHotkey xcstrings keys against the catalog locales and the tagged reload build passes. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Follow-up for the CodeRabbit Swift file/package boundary failure: commit d23ff6f splits |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/`[locale]/docs/configuration/page.tsx:
- Around line 11-15: Confirm that showHideAllWindows in
web/data/cmux-shortcuts.ts indeed contains description and note entries for all
20 locales (no code change required for that shortcut), then remove the
duplicated localeAliases constant from both page.tsx and keyboard-shortcuts.tsx
and extract it into a single shared module (exporting the const localeAliases:
Record<string,string>), update both files to import localeAliases from that
shared module, and ensure the shared export preserves the same mappings
("nb":"no","zh-Hans":"zh-CN","zh-Hant":"zh-TW") and type so consumers continue
to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58aaacdf-b7b8-454a-b0e2-569aa4a967e6
📒 Files selected for processing (3)
web/app/[locale]/docs/configuration/page.tsxweb/app/[locale]/keyboard-shortcuts.tsxweb/data/cmux-shortcuts.ts
|
Follow-up for the CodeRabbit concurrent/main-thread config-load failure: commit 86d0182 removes the synchronous |
Stale automated review: locale alias duplication was addressed by centralizing localizedShortcutText in web/data/cmux-shortcuts.ts, and the current CodeRabbit check is passing.
|
What's holding this PR? |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 383a62f. Configure here.
|
Status update: merge conflicts with What is holding the PR now is pending CI/review-bot completion on the current head: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
I've been using this for a while (pulled it + rebased on main) and it works great. A few things I've noticed (that I fixed locally):
These all might be because of rebase conflicts and how I resolved, though. |
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
4 participants
Summary
Closes #2758
Testing
git diff --checkbun --cwd web --eval '<shortcut locale coverage check>'jqcoverage check for allglobalHotkeyxcstrings locales./scripts/reload.sh --tag issue-2758-global-hotkey-overlayDemo Video
Checklist
Note
Medium Risk
Changes global hotkey behavior and main-window registration/focus routing; mistakes could affect session restore or active-window targeting, though the overlay role is explicitly isolated and covered by new tests.
Overview
The system-wide hotkey no longer toggles all cmux windows; it shows or hides a dedicated hotkey overlay built as a non-activating
NSPanelthat can appear over full-screen Spaces.Runtime: New
GlobalHotkeyPanelControllerhosts fullContentViewin a floating panel (canJoinAllSpaces,fullScreenAuxiliary, stable idcmux.hotkeyPanel). Hiding the overlay restores the previously active session-restorable main window context.MainWindowContextRole.globalHotkeyPanelexcludes the panel from session snapshots, window cycling, and “last window” quit prompts.Copy & docs: Settings, README, web shortcuts, and
Localizable.xcstringsrename the action from “show/hide all windows” to hotkey window / overlay; web shortcut data gainslocalizedShortcutTextwith broader locale fallbacks.Tests:
GlobalHotkeyPanelControllerTestscover panel configuration, layout, config load, and focus/close-warning behavior with the new role.Reviewed by Cursor Bugbot for commit 0738d27. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a system‑wide hotkey overlay that floats above any app, including full‑screen Spaces. Implements #2758 by toggling a non‑activating panel, restoring focus on hide, and updating shortcut text and localization across app and web.
New Features
GlobalHotkeyPanelController/Configuration;.nonactivatingPanelwith.canJoinAllSpacesand.fullScreenAuxiliary, top‑anchored and screen‑aware; ESC hides it.MainWindowContextRole.globalHotkeyPanel(non‑restorable) and passes the role throughContentView.localizedShortcutTextwith locale alias fallbacks.Bug Fixes
cmux.hotkeyPanelas an auxiliary window; excluded from session restore, cycling, and close warnings.Written for commit 0738d27. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests