Add Welcome sidebar toggle shortcuts - #3748
Merged
Merged
Conversation
The Welcome surface is hosted by the same main-window state owners as regular workspaces, so the regression captures the shared shortcut registry and the SidebarState/FileExplorerState mutation path before changing labels or command wiring. Constraint: Welcome is delivered through a main cmux window rather than a separate SwiftUI WelcomeView in this branch. Confidence: medium Scope-risk: narrow Directive: Keep Welcome sidebar shortcuts routed through KeyboardShortcutSettings and the shared sidebar state owners. Tested: Not run locally; regression commit is expected to fail until the implementation commit updates the right-sidebar command exposure. Not-tested: Local xcodebuild/unit test execution intentionally skipped per build restrictions.
Welcome is hosted in the shared main-window surface, so the durable fix is to make the shared shortcut registry and View menu expose explicit left/right sidebar toggle actions. The right-sidebar action keeps the existing raw shortcut id for compatibility while code now names the concept as toggleRightSidebar. Constraint: Cmd+B was already the shared left-sidebar binding; Cmd+Option+B was the existing File Explorer toggle binding and must remain rebindable through KeyboardShortcutSettings. Rejected: Add Welcome-only shortcut handlers | would duplicate the main-window shortcut surface and miss future shared command updates. Confidence: medium Scope-risk: narrow Directive: Do not add Welcome-specific sidebar shortcut definitions; route through KeyboardShortcutSettings.Action and AppDelegate's shared sidebar state owners. Tested: git diff --check; jq empty Resources/Localizable.xcstrings; verified no Swift references remain to .toggleFileExplorer. Not-tested: Local xcodebuild/unit tests intentionally not run; CI will exercise the new regression.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR splits the unified sidebar toggle into distinct left/right actions: renames the enum case, adds left/right localization keys, updates default shortcuts, routes AppDelegate and menu/ContentView commands, adds tests, and updates docs/examples. ChangesLeft/Right Sidebar Shortcut Split
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant KeyboardSettings
participant AppDelegate
participant Window
User->>UI: press Cmd+B or Cmd+Opt+B
UI->>KeyboardSettings: resolve action (toggleSidebar / toggleRightSidebar)
KeyboardSettings->>AppDelegate: deliver action
AppDelegate->>Window: toggleLeftSidebar or toggleRightSidebarInActiveMainWindow(preferredWindow:)
Window->>Window: update SidebarState / FileExplorerState
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (13 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 |
Contributor
Greptile SummarySplits the previously generic "Toggle Sidebar" into explicit Toggle Left Sidebar (⌘B) and Toggle Right Sidebar (⌘⌥B) commands, routing both through the shared AppDelegate window handlers, and preserves the persisted config key
Confidence Score: 5/5Safe to merge — all changes are renaming, relabeling, and routing shortcut actions that were already wired; user-persisted config keys are unchanged. Every production call site that referenced .toggleFileExplorer has been updated to .toggleRightSidebar, which carries rawValue = "toggleFileExplorer" so persisted user bindings round-trip correctly. The new View-menu item follows the identical pattern already used by the existing focusRightSidebar entry. Localization is complete across all 19 locales. No new concurrency primitives, no actor isolation changes, no state duplication. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant NSMenu as View Menu / KeyEquivalent
participant cmuxApp as cmuxApp (SwiftUI)
participant AppDelegate
participant SidebarState
participant FileExplorerState
User->>NSMenu: ⌘B (Toggle Left Sidebar)
NSMenu->>cmuxApp: splitCommandButton action
cmuxApp->>AppDelegate: toggleSidebarInActiveMainWindow()
alt Active window found
AppDelegate->>SidebarState: toggle()
else No active window
cmuxApp->>SidebarState: sidebarState.toggle() (fallback)
end
User->>NSMenu: ⌘⌥B (Toggle Right Sidebar)
NSMenu->>cmuxApp: splitCommandButton action
cmuxApp->>AppDelegate: toggleRightSidebarInActiveMainWindow(preferredWindow:)
alt Active window found
AppDelegate->>FileExplorerState: toggle()
else No active window
cmuxApp->>cmuxApp: NSSound.beep()
end
Reviews (10): Last reviewed commit: "Keep drag suppression tests aligned with..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🤖 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/AppDelegateShortcutRoutingTests.swift`:
- Around line 862-875: The test currently asserts English label literals for
KeyboardShortcutSettings.Action.toggleSidebar.label and
.toggleRightSidebar.label which is locale-dependent; change the assertions to
use the localized value or key-derived identifier instead of hardcoded
strings—e.g., fetch the expected label via the app's localization lookup
(NSLocalizedString with the same localization key the action uses) or compare to
a property that returns the key-derived value (such as a .labelKey or similar
exposed on KeyboardShortcutSettings.Action) so the test is stable across
locales; update the two assertions referencing
KeyboardShortcutSettings.Action.toggleSidebar.label and
KeyboardShortcutSettings.Action.toggleRightSidebar.label accordingly.
In `@web/data/cmux-shortcuts.ts`:
- Line 65: The shortcut entry with id "toggleFileExplorer" uses the modifier
order ["⌥","⌘","B"] which renders as opt+cmd+b and is inconsistent with the rest
of the config; update the combos array for the "toggleFileExplorer" object to
use the normalized modifier order ["⌘","⌥","B"] so it displays as cmd+opt+b,
matching other shortcut entries and generated docs.
🪄 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: 0b9386cc-233e-4cee-af63-ea9da5ace652
📒 Files selected for processing (9)
Resources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/cmuxApp.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/app/[locale]/docs/keyboard-shortcuts/page.tsxweb/data/cmux-shortcuts.ts
The review pass found two quality gaps after the Welcome shortcut wiring: newly-added sidebar command strings did not carry the same locale breadth as surrounding shortcut/menu labels, and the right-sidebar menu fallback read as less best-effort than the left-sidebar path. This keeps the shared shortcut model intact while tightening those edges without changing the persisted toggleFileExplorer config id.\n\nConstraint: Shortcut storage remains backward-compatible through the existing toggleFileExplorer raw config key\nRejected: Add Welcome-only shortcut actions | duplicates the shared sidebar command concept\nConfidence: high\nScope-risk: narrow\nTested: jq empty Resources/Localizable.xcstrings; git diff --check\nNot-tested: Local Xcode/unit test run, per no-local-build workflow
CodeRabbit flagged two real polish issues in the PR surface: localized label assertions were pinned to English literals, and the web shortcut metadata rendered the right-sidebar chord in the wrong modifier order. This keeps the behavioral regression test but makes its label checks locale-aware, and matches the docs' Cmd+Option+B ordering.\n\nConstraint: Do not run local Xcode builds or tests in this workflow\nConfidence: high\nScope-risk: narrow\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local unit tests, per workflow
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The menu handler was re-checking fileExplorerState after the shared AppDelegate toggle had already exhausted that fallback, leaving dead code in the command surface. Keep the menu thin and let the shared command own all best-effort toggle routing. Constraint: Welcome and workspace windows share the same shortcut command surface. Rejected: Duplicate fileExplorerState fallback in cmuxApp.swift | it was unreachable after the shared toggle returned false. Confidence: high Scope-risk: narrow Directive: Keep sidebar menu handlers as thin dispatchers to AppDelegate shortcut actions. Tested: git diff --check Not-tested: Local unit tests not run because this branch avoids local xcodebuild; CI rerun required after push.
The shortcut split replaced generic sidebar labels with explicit left/right labels, and reviewer feedback identified the old catalog entries as orphaned. Remove only those unused keys so the string catalog reflects the active command surface. Constraint: New sidebar shortcut labels remain fully localized through the replacement left/right keys. Rejected: Keep orphaned generic keys | they had no remaining call sites and cluttered the catalog. Confidence: high Scope-risk: narrow Reversibility: clean Tested: jq empty Resources/Localizable.xcstrings; git diff --check; rg no remaining old key references Not-tested: Local unit tests not run because this branch avoids local xcodebuild; CI rerun required after push.
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 92be92e. Configure here.
The right-sidebar action has a Swift case name that intentionally differs from its persisted raw value. Titlebar shortcut hints elsewhere use the raw value, so this keeps UI automation identifiers stable across the rename. Constraint: The toggleRightSidebar action must preserve the existing toggleFileExplorer raw id for user config compatibility. Rejected: Hardcode titlebarShortcutHint.toggleRightSidebar | it diverges from the raw-value convention used by the titlebar hint overlay. Confidence: high Scope-risk: narrow Reversibility: clean Tested: git diff --check; rg no titlebarShortcutHint.toggleRightSidebar literal remains Not-tested: Local unit tests not run because this branch avoids local xcodebuild; CI rerun required after push.
The Welcome terminal output is a user-facing shortcut surface, so the CLI contract now checks that it advertises both sidebar toggle commands through the built cmux binary instead of by inspecting source text. Constraint: Regression test added before the visible Welcome splash fix Confidence: high Scope-risk: narrow Tested: git diff --check -- docs/cli-contract.md Not-tested: CLI probe not run locally; final verification will use PR CI
The shortcut registry and menu routing already expose the shared sidebar toggles, but the first-run Welcome terminal screen keeps a separate static list of common shortcuts. Add the two sidebar rows there so the visible Welcome surface matches the default bindings users see and press. Constraint: Welcome splash is static CLI output separate from KeyboardShortcutSettings Rejected: Generate this splash from KeyboardShortcutSettings in this PR | the bundled CLI and app registry are separate runtimes, and replacing the splash source would broaden a follow-up documentation fix Confidence: high Scope-risk: narrow Directive: Keep the Welcome splash shortcut list in sync when changing first-run default shortcuts Tested: git diff --check -- CLI/cmux.swift docs/cli-contract.md Not-tested: CLI welcome probe not run locally; final verification will use PR CI
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 `@docs/cli-contract.md`:
- Around line 363-364: Update the CLI contract entries for the welcome command
to match the implementation's Title Case strings: replace the lowercase `"Toggle
left sidebar"` and `"Toggle right sidebar"` with `"Toggle Left Sidebar"` and
`"Toggle Right Sidebar"` so the docs/probes align with the actual localized
defaults used in the code (see defaults for the toggle strings in cmuxApp.swift
and KeyboardShortcutSettings.swift and the English values in
Localizable.xcstrings).
🪄 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: 96380bdb-d0ce-4622-bfd9-3ecd74e3f7ea
📒 Files selected for processing (2)
CLI/cmux.swiftdocs/cli-contract.md
CodeRabbit correctly flagged that the new Welcome splash probes should track the visible command labels rather than introducing a separate sentence-case spelling. Use the same Title Case text as the shortcut registry labels while keeping the CLI contract probe on the runtime welcome output. Constraint: Review feedback requested Title Case alignment for the Welcome shortcut rows Confidence: high Scope-risk: narrow Tested: git diff --check -- CLI/cmux.swift docs/cli-contract.md Not-tested: CLI probe not run locally; PR CI will run the built CLI contract
Merged origin/main into the sidebar shortcut branch and resolved the overlapping shortcut surfaces by keeping the shared right-sidebar action name while preserving the persisted toggleFileExplorer config key. Constraint: Build verification must use scripts/reload.sh; local tests are not run in this workspace.\nRejected: Keep origin/main's toggleFileExplorer Swift enum case | it would reintroduce duplicate terminology and break the PR's central shortcut registry shape.\nConfidence: high\nScope-risk: moderate\nDirective: Keep toggleRightSidebar.rawValue equal to toggleFileExplorer for config compatibility.\nTested: python3 -m json.tool Resources/Localizable.xcstrings; git diff --cached --check; ./scripts/reload.sh --tag issue-3745-welcome-sidebar-shortcuts --launch\nNot-tested: Local unit/UI tests per repository policy.
The latest base split drag suppression state from temporary window movability. The unit tests still referenced the removed helper names, so the compile failed before exercising the shortcut regression coverage. Constraint: Local test execution is deferred to CI for this repository. Rejected: Reintroduce the removed helper functions | that would duplicate the current suppression and temporary-movability APIs. Confidence: high Scope-risk: narrow Directive: Keep suppression-depth tests focused on suppression state; use withTemporaryWindowMovableEnabled for movability restoration behavior. Tested: git diff --check Not-tested: Local unit tests, per repository policy; CircleCI will run cmux-unit.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This was referenced May 12, 2026
This was referenced Jul 13, 2026
This branch was successfully deployed
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
1 participant
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.
Closes #3745
Summary
Conflict note
The codebase has no separate SwiftUI WelcomeView; Welcome is delivered in the shared main cmux window via the welcome command. No separate Welcome-only Cmd+B binding was found. The existing shared Cmd+B binding is the left-sidebar toggle and is now labeled that way.
Verification
Local xcodebuild/unit tests were not run per the build restrictions; CI should exercise the regression.
Note
Medium Risk
Changes keyboard shortcut routing and menu/command labels for sidebar toggles; low surface area but could affect existing shortcut bindings and window targeting behavior.
Overview
Adds explicit left vs right sidebar toggle commands across the app:
⌘Bis now labeled as Toggle Left Sidebar and⌘⌥Btoggles the right sidebar via a sharedtoggleRightSidebaraction (while preserving the persisted config idtoggleFileExplorer).Updates localized strings, View menu items, command palette entry/keywords, CLI
cmux welcomeshortcuts output, and web/docs shortcut examples to reflect the split toggles. Adds regression tests asserting the Welcome/main-window surface uses the shared toggle commands and updates shortcut/settings ordering and related test expectations.Reviewed by Cursor Bugbot for commit 5efb2fc. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds explicit left/right sidebar toggle shortcuts to the Welcome surface and across the app via shared main-window commands per #3745. Updates menus, command palette, CLI Welcome splash, web/docs, and tests while keeping existing user bindings.
New Features
toggleFileExplorerwhile the Swift action istoggleRightSidebar; titlebar shortcut hints continue to use the raw value.Refactors
Written for commit 5efb2fc. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization
UI
Documentation
Tests