Repository navigation
Add browser.autoFocusMode: auto-enter Browser Focus Mode on pane focus - #5904
erik-hunter wants to merge 21 commits into
Conversation
|
To use Codex here, create a Codex account and connect to github. |
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds an "Auto Focus Mode" setting for the browser that automatically enters Browser Focus Mode when a browser pane gains focus. The feature spans setting declaration, persistence infrastructure, UI, runtime activation, search indexing, and validation tests across multiple modules and configuration layers. ChangesBrowser Auto Focus Mode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 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 |
|
@codex review |
@erik-hunter I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 20 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
Greptile SummaryThis PR adds a
Confidence Score: 4/5The new feature is off by default and the core activation guard is well-tested; the outstanding concern is in the command-palette interaction path flagged in earlier review rounds. The activation logic, suppression model, and all explicit-exit paths are correctly wired. The two AppDelegate command-palette clear sites (postCommandPaletteRequest line 5245 and setCommandPaletteVisible line 5403) still call setFocusedBrowserFocusModeActive(false, …) without suppressAutoFocusUntilFocusGain: true, and activateAutoFocusModeIfNeeded checks only committed-visible palette state rather than the broader isCommandPaletteEffectivelyVisible (which includes isPendingOpen). Both gaps were flagged in previous review rounds and remain open. Sources/AppDelegate.swift — the two command-palette focus-mode clear sites at lines 5245 and 5403. Important Files Changed
Reviews (20): Last reviewed commit: "Merge origin/main into browser-auto-focu..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 11800-11825: The tests currently call
setBrowserFocusModeActive(...) directly which bypasses the real panel-focus
entrypoint and the auto-focus gating; update the tests created with
makeBrowserFocusModeHarness() so they simulate a real panel focus event by
invoking the actual focus-change entrypoint (e.g., handlePanelFocusChange or
whatever method routes panel focus events into the panel) instead of calling
setBrowserFocusModeActive directly, keep toggling
BrowserAutoFocusModeSettings.enabledKey via UserDefaults to verify the positive
path (autoFocusMode on activates via the real entrypoint) and add a negative
case with autoFocusMode off asserting no activation, and for the
redundant-activation test call the same real entrypoint twice to ensure it does
not clear or crash while panel.isBrowserFocusModeActive remains true.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 428-440: Extract the nonisolated enum BrowserAutoFocusModeSettings
(including its static properties enabledKey and defaultEnabled and methods
isEnabled and isEnabled(defaults:)) out of BrowserPanel.swift into a new
dedicated settings file/module alongside other settings persistence helpers;
move the type to that file, update any imports/visibility so callers of
BrowserAutoFocusModeSettings and its members continue to compile, and remove it
from BrowserPanel.swift so the panel file only contains runtime/panel code.
Ensure the new file is part of the app target or a shared settings module as
appropriate and run tests/compile to fix any references.
In `@Sources/SettingsNavigation.swift`:
- Line 400: The new "browser.autoFocusMode" search entry lacks an anchor mapping
in the settingsPathAnchorIDs dictionary, so anchorID(forSettingsPath:) can't
resolve it; add a key/value pair for "browser.autoFocusMode" in the same
settingsPathAnchorIDs dictionary alongside the other browser.* entries (use the
same anchor ID string style used for the other browser keys so jump/highlight
behavior works correctly), then run a quick search/jump test to confirm anchor
resolution for the new Auto Focus Mode setting.
🪄 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: a97f3a5a-fe6e-40aa-9779-0634b35866ad
📒 Files selected for processing (10)
Packages/CmuxSettings/Sources/CmuxSettings/Keys/BrowserCatalogSection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/BrowserSection.swiftResources/Localizable.xcstringsSources/CmuxSettingsJSONPathSupport.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/SettingsNavigation.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftweb/data/cmux.schema.json
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge; the new setting is off by default and the activation path is a small, well-isolated addition to an existing focus-change handler. The implementation follows established patterns throughout. Two minor issues were found: spurious state transitions when a blank-tab pane gains focus (address bar auto-focus fires first, then focus mode activates, then gets cleared in the next SwiftUI cycle), and two tests that mutate UserDefaults.standard rather than an injected suite. Neither affects the primary use case (loaded pages like VS Code Web) or the off-by-default baseline. Sources/Panels/BrowserPanelView.swift — the ordering of autoFocusOmnibarIfBlank() relative to the focus-mode activation block; cmuxTests/AppDelegateShortcutRoutingTests.swift — the two tests that write to UserDefaults.standard Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant BrowserPanelView
participant BrowserPanel
participant NotificationCenter
User->>BrowserPanelView: Focus browser pane
BrowserPanelView->>BrowserPanelView: handlePanelFocusChange(true)
BrowserPanelView->>BrowserPanelView: autoFocusOmnibarIfBlank() [no-op if page loaded]
alt autoFocusModeEnabled and not isBrowserFocusModeActive
BrowserPanelView->>BrowserPanel: setBrowserFocusModeActive(true, focusWebView: false)
BrowserPanel->>BrowserPanel: "isBrowserFocusModeActive = true"
BrowserPanel->>NotificationCenter: post(browserFocusModeStateDidChange)
end
User->>BrowserPanelView: Press Cmd+N / Cmd+F
BrowserPanelView->>BrowserPanel: route shortcut to web view (focus mode active)
User->>BrowserPanelView: Double-Escape
BrowserPanel->>BrowserPanel: clearBrowserFocusMode(reason: escape)
BrowserPanel->>NotificationCenter: post(browserFocusModeStateDidChange)
Note over BrowserPanelView: stays off until next pane focus
User->>BrowserPanelView: Unfocus browser pane
BrowserPanelView->>BrowserPanel: clearBrowserFocusMode(reason: panelFocus.onChange.unfocused)
BrowserPanel->>BrowserPanel: "isBrowserFocusModeActive = false"
Reviews (2): Last reviewed commit: "Add auto-focus mode setting for webView ..." | Re-trigger Greptile |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
@erik-hunter I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 20 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
1 similar comment
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedReview finished.
|
|
This is great! I love the idea! |
|
Addressed the CodeRabbit pre-merge i18n summary in 0bab3c1: web/data/cmux.schema.json now includes descriptionKey for browser.autoFocusMode, and docs.configuration.schemaDescriptions.browser.autoFocusMode is present in all 20 web/messages locale catalogs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
# Conflicts: # .github/swift-file-length-budget.tsv
Resolved conflicts in: - .github/swift-file-length-budget.tsv: merged budgets, taking max per file - Resources/Localizable.xcstrings: kept blendMode.* and askWhereToSaveDownloads* from main, inserted autoFocusMode keys in alphabetical order - cmuxTests/AppDelegateShortcutRoutingTests.swift: kept both auto-focus tests and new Cmd+I tests from main - web/messages/en.json, web/messages/ja.json: kept both autoFocusMode and askWhereToSaveDownloads schema description entries
|
Deployment failed with the following error: |
There was a problem hiding this comment.
2 issues found across 36 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/messages/da.json">
<violation number="1" location="web/messages/da.json:830">
P2: The Danish translation for `browser.autoFocusMode` uses "browserfane" (browser tab/surface) instead of the correct "browserpanel" or equivalent pane terminology. The English source states the feature activates "when a browser pane gains focus" — a pane is a split region that may contain multiple surfaces/tabs. The Danish text "skift til en browserfane" implies activation on tab switches, which understates/misstates the actual pane-focus behavior and could confuse users about when Focus Mode triggers. Consider aligning with the English "pane" semantics (e.g. "når et browserpanel får fokus").</violation>
</file>
<file name="Sources/Panels/BrowserPanelView.swift">
<violation number="1" location="Sources/Panels/BrowserPanelView.swift:409">
P2: If `browser.autoFocusMode` is toggled while the browser pane is already focused (for example, via `cmux.json` live reload), auto-focus mode will not activate until another transition such as blur/refocus or address-bar interaction occurs. The file already uses `.onChange(of:)` for other `@AppStorage` values like `browserThemeModeRaw`, so consider adding a matching handler for `autoFocusModeEnabled` that calls `reconcileFocusedPanelState` when the pane is focused:
```swift
.onChange(of: autoFocusModeEnabled) { _ in
if isFocused {
reconcileFocusedPanelState(reason: "autoFocusMode.settingChanged", isPanelFocused: true)
}
}
This ensures live-reloaded settings take effect immediately for focused panes rather than waiting for the next focus transition.
</details>
<sub>Reply with feedback, questions, or to request a fix.<br /><br />[Re-trigger cubic](https://www.cubic.dev/action/re-review/pr/manaflow-ai/cmux/5904/ai_pr_review_1782944750293_ed178d45-4fa6-4581-bc87-030dd32b1731?returnTo=https%3A%2F%2Fgithub.com%2Fmanaflow-ai%2Fcmux%2Fpull%2F5904)</sub>
<!-- cubic:review-post:ai_pr_review_1782944750293_ed178d45-4fa6-4581-bc87-030dd32b1731:1c665650df9361c7de97b6c9d01315981ed12b01:045a87e8-bd4b-4843-9fca-699ff82b43b5 -->
| "customSearchEngineName": "Visningsnavn brugt når defaultSearchEngine er custom.", | ||
| "customSearchEngineURLTemplate": "Søge-URL brugt når defaultSearchEngine er custom. Medtag forespørgselspladsholderen eller %s for den kodede søgning. Uden en pladsholder tilføjer cmux q= til URLen." | ||
| "customSearchEngineURLTemplate": "Søge-URL brugt når defaultSearchEngine er custom. Medtag forespørgselspladsholderen eller %s for den kodede søgning. Uden en pladsholder tilføjer cmux q= til URLen.", | ||
| "autoFocusMode": "Når slået til, aktiverer skift til en browserfane automatisk fokustilstand, så Cmd-genveje når siden i stedet for cmux." |
There was a problem hiding this comment.
P2: The Danish translation for browser.autoFocusMode uses "browserfane" (browser tab/surface) instead of the correct "browserpanel" or equivalent pane terminology. The English source states the feature activates "when a browser pane gains focus" — a pane is a split region that may contain multiple surfaces/tabs. The Danish text "skift til en browserfane" implies activation on tab switches, which understates/misstates the actual pane-focus behavior and could confuse users about when Focus Mode triggers. Consider aligning with the English "pane" semantics (e.g. "når et browserpanel får fokus").
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/messages/da.json, line 830:
<comment>The Danish translation for `browser.autoFocusMode` uses "browserfane" (browser tab/surface) instead of the correct "browserpanel" or equivalent pane terminology. The English source states the feature activates "when a browser pane gains focus" — a pane is a split region that may contain multiple surfaces/tabs. The Danish text "skift til en browserfane" implies activation on tab switches, which understates/misstates the actual pane-focus behavior and could confuse users about when Focus Mode triggers. Consider aligning with the English "pane" semantics (e.g. "når et browserpanel får fokus").</comment>
<file context>
@@ -826,7 +826,8 @@
"customSearchEngineName": "Visningsnavn brugt når defaultSearchEngine er custom.",
- "customSearchEngineURLTemplate": "Søge-URL brugt når defaultSearchEngine er custom. Medtag forespørgselspladsholderen eller %s for den kodede søgning. Uden en pladsholder tilføjer cmux q= til URLen."
+ "customSearchEngineURLTemplate": "Søge-URL brugt når defaultSearchEngine er custom. Medtag forespørgselspladsholderen eller %s for den kodede søgning. Uden en pladsholder tilføjer cmux q= til URLen.",
+ "autoFocusMode": "Når slået til, aktiverer skift til en browserfane automatisk fokustilstand, så Cmd-genveje når siden i stedet for cmux."
},
"workspaceGroups": {
</file context>
| "autoFocusMode": "Når slået til, aktiverer skift til en browserfane automatisk fokustilstand, så Cmd-genveje når siden i stedet for cmux." | |
| + "autoFocusMode": "Når slået til, aktiverer det automatisk browserfokustilstand, når et browserpanel får fokus, så Cmd-genveje når siden i stedet for cmux." |
| @AppStorage(BrowserProfilePopoverDebugSettings.verticalPaddingKey) | ||
| private var browserProfilePopoverVerticalPaddingRaw = BrowserProfilePopoverDebugSettings.defaultVerticalPadding | ||
| @AppStorage(BrowserThemeSettings.modeKey) private var browserThemeModeRaw = BrowserThemeSettings.defaultMode.rawValue | ||
| @AppStorage(BrowserAutoFocusModeSettings.enabledKey) private var autoFocusModeEnabled = BrowserAutoFocusModeSettings.defaultEnabled |
There was a problem hiding this comment.
P2: If browser.autoFocusMode is toggled while the browser pane is already focused (for example, via cmux.json live reload), auto-focus mode will not activate until another transition such as blur/refocus or address-bar interaction occurs. The file already uses .onChange(of:) for other @AppStorage values like browserThemeModeRaw, so consider adding a matching handler for autoFocusModeEnabled that calls reconcileFocusedPanelState when the pane is focused:
.onChange(of: autoFocusModeEnabled) { _ in
if isFocused {
reconcileFocusedPanelState(reason: "autoFocusMode.settingChanged", isPanelFocused: true)
}
}This ensures live-reloaded settings take effect immediately for focused panes rather than waiting for the next focus transition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Panels/BrowserPanelView.swift, line 409:
<comment>If `browser.autoFocusMode` is toggled while the browser pane is already focused (for example, via `cmux.json` live reload), auto-focus mode will not activate until another transition such as blur/refocus or address-bar interaction occurs. The file already uses `.onChange(of:)` for other `@AppStorage` values like `browserThemeModeRaw`, so consider adding a matching handler for `autoFocusModeEnabled` that calls `reconcileFocusedPanelState` when the pane is focused:
```swift
.onChange(of: autoFocusModeEnabled) { _ in
if isFocused {
reconcileFocusedPanelState(reason: "autoFocusMode.settingChanged", isPanelFocused: true)
}
}
This ensures live-reloaded settings take effect immediately for focused panes rather than waiting for the next focus transition.
@@ -406,6 +406,7 @@ struct BrowserPanelView: View { @AppStorage(BrowserProfilePopoverDebugSettings.verticalPaddingKey) private var browserProfilePopoverVerticalPaddingRaw = BrowserProfilePopoverDebugSettings.defaultVerticalPadding @AppStorage(BrowserThemeSettings.modeKey) private var browserThemeModeRaw = BrowserThemeSettings.defaultMode.rawValue + @AppStorage(BrowserAutoFocusModeSettings.enabledKey) private var autoFocusModeEnabled = BrowserAutoFocusModeSettings.defaultEnabled @AppStorage(BrowserImportHintSettings.variantKey) private var browserImportHintVariantRaw = BrowserImportHintSettings.defaultVariant.rawValue @AppStorage(BrowserImportHintSettings.showOnBlankTabsKey) private var showBrowserImportHintOnBlankTabs = BrowserImportHintSettings.defaultShowOnBlankTabs ```|
This opt-in browser focus setting remains distinct and live; leaving it open with the two review findings called out for follow-up. |
Summary
browser.autoFocusModesetting (off by default). When enabled, focusing a browser pane automatically activates Browser Focus Mode, so Cmd-shortcuts (Cmd+N, Cmd+F, Cmd+P, …) reach the embedded page instead of being captured by cmux. Focus mode still clears when the pane loses focus, and double-Escape still exits it; with the setting on, it re-arms on the next pane focus.Wiring follows the existing settings patterns end to end:
BrowserCatalogSectionkey +cmux.schema.jsonentry + JSON path registration +parseBrowserSectionparsing (browser.autoFocusModein~/.config/cmux/cmux.json, live-reloaded)BrowserAutoFocusModeSettingsenum +@AppStoragehook inBrowserPanelView.handlePanelFocusChange, activating via the existingsetBrowserFocusModeActive(_:reason:focusWebView:)path withfocusWebView: false(the pane is already gaining focus, so no explicit first-responder request is needed)Resources/Localizable.xcstringsTesting
./scripts/reload.sh --tag auto-focus-mode) and dogfooded against a code-server pane: with the toggle on, entering the pane auto-enables Focus Mode and Cmd+N/Cmd+F reach VS Code Web; leaving the pane clears it; double-Escape exits as before. Verified the toggle in Settings → Browser and thebrowser.autoFocusModekey incmux.json(live reload) both drive the behavior, and that everything is unchanged with the setting off (default).AppDelegateShortcutRoutingTests: settings default/roundtrip with injectedUserDefaults, focus-mode activation throughsetBrowserFocusModeActivewithfocusWebView: false, and idempotence on redundant activation. Note: thehandlePanelFocusChangeview-layer wiring itself isn't reachable from unit tests; that path was validated manually in the dev build. Localization audit: parsedLocalizable.xcstringsand confirmed both new keys (settings.browser.autoFocusMode,.subtitle) have translated entries for all 20 locales.Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Screen.Recording.2026-06-11.at.1.48.02.PM.mov
Review Trigger (Copy/Paste as PR comment)
Checklist
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a new
browser.autoFocusModesetting that auto‑enters Browser Focus Mode when a browser pane gains focus so Cmd‑shortcuts go to the page instead of cmux. Off by default; focuses the web view on auto‑activation; exits on pane blur or double‑Escape; skips blank tabs and when the Command Palette is open; restores after the palette closes.New Features
browser.autoFocusMode(default false). Toggle in Settings → Browser or set in~/.config/cmux/cmux.jsonatbrowser.autoFocusMode(live reload). Indexed in settings search; added to the generated config template and schema; localized across all supported locales.setBrowserFocusModeActive(..., focusWebView: true); returns toggle result to callers for correct feedback.Bug Fixes
Written for commit 1c66565. Summary will update on new commits.
Summary by CodeRabbit