Adopt macOS 26 Liquid Glass design#2647
Adopt macOS 26 Liquid Glass design#2647rodchristiansen wants to merge 5 commits intomanaflow-ai:mainfrom
Conversation
NavigationSplitView with native glass sidebar, system titlebar, and SwiftUI toolbar on macOS 26. Terminal detail content gets 8pt external padding, 6pt internal content inset, and leading corner rounding (16pt) when the sidebar is visible. Key changes: - NavigationSplitView wraps sidebar + detail for native glass treatment - System titlebar (titlebarAppearsTransparent = false) with SwiftUI .toolbar for bell and new-tab buttons - SplitViewDividerHider clears NSSplitView divider color - Terminal content inset (6pt) keeps text within safe zone - Leading corners rounded at AppKit portal level when sidebar present - Bonsplit tab bar hidden for single-tab panes on macOS 26 - App icon auto-set skipped on macOS 26 (system handles dark variants) - Old titlebar accessory controllers skipped on macOS 26
|
@rodchristiansen is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
📝 WalkthroughWalkthroughmacOS 26+ gated UI changes: system safe-area integration, NavigationSplitView for content/sidebar, toolbar/ titlebar behavior and accessory/popover flow updated, terminal hosting/scroll geometry and layer corner rounding adjusted, bonsplit submodule bumped. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Toolbar as NSToolbar
participant AppDelegate
participant Content as ContentView
participant Accessory as TitlebarAccessory
User->>Toolbar: Click "Notifications" item
Toolbar->>AppDelegate: toolbar action toggleNotificationsPopover
alt macOS >= 26
AppDelegate->>AppDelegate: post Notification.Name("cmux.toggleNotificationsPopover")
AppDelegate-->>Content: Notification received
Content->>Content: set isNotificationsPopoverPresented = true
Content->>User: present popover
else macOS < 26
AppDelegate->>Accessory: titlebarAccessoryController.toggleNotificationsPopover(animated:)
Accessory->>User: toggle accessory popover
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adopts macOS 26 Liquid Glass by switching to
Confidence Score: 4/5Not safe to merge — two P1 regressions ship broken functionality to macOS 26 users. The bell-button no-op and unconditional tab-bar hiding are both present defects on the changed code path requiring fixes before merge. Sources/AppDelegate.swift (broken notifications popover) and Sources/Workspace.swift (unconditional tab bar hide) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[macOS 26 toolbar bell click] --> B[AppDelegate.toggleNotificationsPopover]
B --> C[titlebarAccessoryController.toggleNotificationsPopover]
C --> D{controlsControllers.isEmpty?}
D -- yes macOS 26 --> E[return NO-OP]
D -- no pre-26 --> F[show popover]
G[App launch macOS 26] --> H[skip titlebarAccessoryController.start]
H --> J[controlsControllers stays empty]
K[Window created macOS 26] --> L[attachUpdateAccessory]
L --> M{macOS 26?}
M -- yes --> N[return early no start/attach]
M -- no --> O[start + attach controller]
Reviews (1): Last reviewed commit: "Adopt macOS 26 Liquid Glass design" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func attachUpdateAccessory(to window: NSWindow) { | ||
| if #available(macOS 26.0, *) { | ||
| // On macOS 26, toolbar buttons are native SwiftUI .toolbar items | ||
| // in the NavigationSplitView. Skip the old titlebar accessory controllers. | ||
| return | ||
| } | ||
| titlebarAccessoryController.start() | ||
| titlebarAccessoryController.attach(to: window) | ||
| } |
There was a problem hiding this comment.
attachUpdateAccessory(to:) returns early without calling titlebarAccessoryController.start() or .attach(to:), and applicationDidFinishLaunching also skips .start() on macOS 26 (~line 2601). As a result controlsControllers.allObjects is always empty. When the SwiftUI toolbar bell calls AppDelegate.shared?.toggleNotificationsPopover(animated: true), UpdateTitlebarAccessoryController.toggleNotificationsPopover bails immediately on guard !controllers.isEmpty else { return } — clicking the bell silently does nothing.
Fix: either call titlebarAccessoryController.start() (without attach) so the popover infrastructure is initialized, or route the bell action through a macOS-26-specific popover path that does not depend on controlsControllers.
| let hideTabBar: Bool | ||
| if #available(macOS 26.0, *) { | ||
| hideTabBar = true | ||
| } else { | ||
| hideTabBar = false | ||
| } | ||
| return BonsplitConfiguration.Appearance( | ||
| tabBarHeight: hideTabBar ? 0 : 33, |
There was a problem hiding this comment.
Setting hideTabBar = true on macOS 26 sets tabBarHeight = 0 for every pane, regardless of how many tabs it contains. The PR description says "single-tab panes," but no tab count is checked here. In a split workspace with multiple tabs per pane, all in-pane tab switching UI disappears on macOS 26.
If the goal is to hide the bar only for single-tab panes, the appearance must be updated reactively based on bonsplitController.tabs(inPane:).count rather than as a static launch-time configuration.
| NavigationSplitView(columnVisibility: Binding( | ||
| get: { sidebarState.isVisible ? .all : .detailOnly }, | ||
| set: { newValue in | ||
| let shouldShow = (newValue != .detailOnly) | ||
| if shouldShow != sidebarState.isVisible { | ||
| _ = sidebarState.toggle() | ||
| } | ||
| } | ||
| )) { | ||
| sidebarView | ||
| .navigationSplitViewColumnWidth(min: 120, ideal: sidebarWidth, max: 400) | ||
| } detail: { | ||
| terminalContentWithSidebarDropOverlay | ||
| .padding(8) | ||
| } | ||
| .navigationSplitViewStyle(.prominentDetail) | ||
| .background(SplitViewDividerHider()) | ||
| .toolbar { | ||
| ToolbarItemGroup(placement: .primaryAction) { | ||
| Button { | ||
| _ = AppDelegate.shared?.toggleNotificationsPopover(animated: true) | ||
| } label: { | ||
| Image(systemName: "bell") | ||
| } | ||
| .buttonStyle(.accessoryBarAction) | ||
| .accessibilityIdentifier("toolbar.notifications") | ||
|
|
||
| Button { | ||
| if let appDelegate = AppDelegate.shared { | ||
| if appDelegate.addWorkspaceInPreferredMainWindow(debugSource: "toolbar.newTab") == nil { | ||
| appDelegate.openNewMainWindow(nil) | ||
| } | ||
| } | ||
| } label: { | ||
| Image(systemName: "plus") | ||
| } | ||
| .buttonStyle(.accessoryBarAction) | ||
| .accessibilityIdentifier("toolbar.newTab") | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
navigationSplitViewColumnWidth(min: 120, ideal: sidebarWidth, max: 400) provides an initial hint, but drag events from the native divider are not fed back into sidebarWidth or sidebarState.persistedWidth. Every sidebar toggle or app restart snaps back to the last value written by the old custom-resizer path, ignoring any width the user set via the system divider on macOS 26.
Sources/GhosttyTerminalView.swift
Outdated
| let contentInset: CGFloat | ||
| if #available(macOS 26.0, *) { | ||
| contentInset = 6 | ||
| } else { | ||
| contentInset = 0 | ||
| } | ||
| let scrollFrame = contentInset > 0 ? bounds.insetBy(dx: contentInset, dy: contentInset) : bounds | ||
| _ = setFrameIfNeeded(scrollView, to: scrollFrame) |
There was a problem hiding this comment.
bounds.insetBy(dx: contentInset, dy: contentInset) applies the 6 pt padding on all four sides. Only the leading corners (top-left, bottom-left) are rounded; the right and bottom edges are flat. The right-side and bottom 6 pt waste terminal real estate with no visual justification. Consider applying inset only on the leading and top edges where rounding occurs.
| if #available(macOS 26.0, *) { | ||
| let hasSidebarToLeft = targetFrame.origin.x > 20 | ||
| let desiredRadius: CGFloat = hasSidebarToLeft ? 16 : 0 | ||
| if hostedView.layer?.cornerRadius != desiredRadius { | ||
| hostedView.layer?.cornerRadius = desiredRadius | ||
| hostedView.layer?.maskedCorners = hasSidebarToLeft | ||
| ? [.layerMinXMinYCorner, .layerMinXMaxYCorner] | ||
| : [] | ||
| } | ||
| } |
There was a problem hiding this comment.
let hasSidebarToLeft = targetFrame.origin.x > 20 infers sidebar presence from the terminal frame's x-origin. A window positioned near x = 0 with no sidebar, or a sidebar narrower than 20 pt during animation, would produce incorrect rounding. Deriving this directly from sidebarState.isVisible would be more robust and self-documenting.
There was a problem hiding this comment.
Pull request overview
Updates the macOS UI path to align with macOS 26 “Liquid Glass” conventions by switching to system-native sidebar/titlebar behaviors and adjusting terminal spacing/clipping to match the new visual language.
Changes:
- Adds a macOS 26-specific
NavigationSplitViewlayout with SwiftUI toolbar actions and updated window/titlebar configuration. - Adjusts terminal presentation (external padding, internal content inset, leading-corner rounding) for Liquid Glass spacing/shape.
- Skips legacy titlebar accessory / custom glass paths and icon override logic on macOS 26; hides Bonsplit tab bar on macOS 26.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Workspace.swift | macOS 26-specific Bonsplit appearance change to hide tab bar. |
| Sources/WindowToolbarController.swift | Adds macOS 26 toolbar style + additional toolbar items/actions. |
| Sources/TerminalWindowPortal.swift | Adds macOS 26 leading-corner rounding logic for hosted terminal views. |
| Sources/GhosttyTerminalView.swift | Insets terminal scroll view on macOS 26 to create internal safe padding. |
| Sources/ContentView.swift | Introduces macOS 26 NavigationSplitView path, toolbar buttons, window/titlebar behavior changes, sidebar background changes, and new glass-effect modifiers/utilities. |
| Sources/cmuxApp.swift | Refactors main window content builder; skips custom app icon override behavior on macOS 26. |
| Sources/AppDelegate.swift | Enables system safe area on macOS 26 and skips legacy titlebar accessory startup/attachment on macOS 26. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .padding(.top, effectiveTitlebarPadding) | ||
| .padding(.top, { | ||
| if #available(macOS 26.0, *) { | ||
| return 0 // Native glass titlebar handles spacing via safe area |
There was a problem hiding this comment.
On macOS 26 this sets terminalContent top padding to 0 assuming the system safe area will keep content below the titlebar, but ContentView later applies .ignoresSafeArea() to the root view. With safe areas ignored, the terminal content can end up under the titlebar and reintroduce the drag/interaction issues this padding previously avoided. Consider making the safe-area ignoring conditional (e.g., only for the sidebar/background) or keep an appropriate top inset for the detail content on macOS 26.
| return 0 // Native glass titlebar handles spacing via safe area | |
| // Keep the terminal content below the titlebar even if an ancestor view | |
| // ignores safe areas; relying on the system safe area here can place | |
| // interactive content under the titlebar. | |
| return effectiveTitlebarPadding |
Sources/ContentView.swift
Outdated
| Image(systemName: "bell") | ||
| } | ||
| .buttonStyle(.accessoryBarAction) | ||
| .accessibilityIdentifier("toolbar.notifications") | ||
|
|
||
| Button { | ||
| if let appDelegate = AppDelegate.shared { | ||
| if appDelegate.addWorkspaceInPreferredMainWindow(debugSource: "toolbar.newTab") == nil { | ||
| appDelegate.openNewMainWindow(nil) | ||
| } | ||
| } | ||
| } label: { | ||
| Image(systemName: "plus") | ||
| } | ||
| .buttonStyle(.accessoryBarAction) | ||
| .accessibilityIdentifier("toolbar.newTab") |
There was a problem hiding this comment.
.buttonStyle(.accessoryBarAction) is a macOS 26 SwiftUI API; wrapping it only in if #available(macOS 26.0, *) is not sufficient for builds using older SDKs/toolchains (this repo’s Package.swift is swift-tools-version 5.9). To preserve backward build compatibility, gate these calls with a compiler/SDK availability check (similar to the #if compiler(>=6.2) guards used elsewhere) or provide a fallback button style.
Sources/ContentView.swift
Outdated
| // Override the divider color to clear | ||
| splitView.setValue(NSColor.clear, forKey: "dividerColor") |
There was a problem hiding this comment.
splitView.setValue(NSColor.clear, forKey: "dividerColor") relies on KVC to mutate an AppKit property and can raise an Objective‑C exception at runtime if the key isn’t settable on some OS versions. Prefer a supported API (e.g., setting a public dividerColor if writable) or guard the KVC path with a selector/responds(to:) check and fail safely.
| // Override the divider color to clear | |
| splitView.setValue(NSColor.clear, forKey: "dividerColor") | |
| // Override the divider color to clear when the Objective-C setter exists. | |
| let setDividerColorSelector = NSSelectorFromString("setDividerColor:") | |
| if splitView.responds(to: setDividerColorSelector) { | |
| splitView.perform(setDividerColorSelector, with: NSColor.clear) | |
| } |
Sources/ContentView.swift
Outdated
| /// Finds NSSplitView(s) inside NavigationSplitView and hides dividers | ||
| /// by installing a custom delegate that draws nothing. | ||
| @available(macOS 26.0, *) | ||
| private struct SplitViewDividerHider: NSViewRepresentable { | ||
| func makeNSView(context: Context) -> SplitViewDividerHiderView { | ||
| SplitViewDividerHiderView() | ||
| } | ||
|
|
||
| func updateNSView(_ nsView: SplitViewDividerHiderView, context: Context) { | ||
| nsView.scheduleHide() | ||
| } | ||
| } | ||
|
|
||
| @available(macOS 26.0, *) | ||
| final class SplitViewDividerHiderView: NSView { | ||
| private var installed = false | ||
|
|
There was a problem hiding this comment.
The SplitViewDividerHider docs say it hides dividers “by installing a custom delegate”, but the implementation only walks subviews and tweaks properties (no delegate is installed). Also installed is declared but unused, suggesting an intended one-time install that isn’t implemented. Please update the comment to match behavior and either remove installed or use it to prevent repeated recursive patching on every SwiftUI update.
| if #available(macOS 26.0, *) { | ||
| // On macOS 26 without fullSizeContentView, the system titlebar | ||
| // handles drag natively. | ||
| window.isMovable = true | ||
| window.isMovableByWindowBackground = false | ||
| } else { | ||
| // Keep window immovable; the sidebar's WindowDragHandleView handles | ||
| // drag-to-move via performDrag with temporary movable override. | ||
| // isMovableByWindowBackground=true breaks tab reordering, and | ||
| // isMovable=true blocks clicks on sidebar buttons in minimal mode. | ||
| window.isMovableByWindowBackground = false | ||
| window.isMovable = false | ||
| } | ||
| window.styleMask.insert(.fullSizeContentView) | ||
|
|
There was a problem hiding this comment.
This comment says “On macOS 26 without fullSizeContentView, the system titlebar handles drag natively”, but the code unconditionally inserts .fullSizeContentView right after. Either make the style-mask change conditional for macOS 26 or update the comment, since it currently contradicts the actual behavior and makes debugging titlebar/safe-area interactions harder.
Sources/ContentView.swift
Outdated
| /// Fills the background with the terminal's current background color, | ||
| /// staying in sync with theme changes. Used behind NavigationSplitView | ||
| /// so the sidebar's rounded corners blend seamlessly with the window. | ||
| @available(macOS 26.0, *) | ||
| private struct TerminalBackgroundFill: View { | ||
| @State private var bgColor = Color(nsColor: GhosttyBackgroundTheme.currentColor()) | ||
|
|
||
| var body: some View { | ||
| bgColor | ||
| .onReceive(NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)) { _ in | ||
| bgColor = Color(nsColor: GhosttyBackgroundTheme.currentColor()) | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
TerminalBackgroundFill is documented as “Used behind NavigationSplitView”, but it isn’t referenced anywhere in ContentView (search only finds its definition). If it’s no longer needed, consider removing it; if it is needed to blend the sidebar corners, wire it into the macOS 26 NavigationSplitView branch so the code matches the comment.
| /// Fills the background with the terminal's current background color, | |
| /// staying in sync with theme changes. Used behind NavigationSplitView | |
| /// so the sidebar's rounded corners blend seamlessly with the window. | |
| @available(macOS 26.0, *) | |
| private struct TerminalBackgroundFill: View { | |
| @State private var bgColor = Color(nsColor: GhosttyBackgroundTheme.currentColor()) | |
| var body: some View { | |
| bgColor | |
| .onReceive(NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)) { _ in | |
| bgColor = Color(nsColor: GhosttyBackgroundTheme.currentColor()) | |
| } | |
| } | |
| } |
| let hideTabBar: Bool | ||
| if #available(macOS 26.0, *) { | ||
| hideTabBar = true | ||
| } else { | ||
| hideTabBar = false | ||
| } | ||
| return BonsplitConfiguration.Appearance( | ||
| tabBarHeight: hideTabBar ? 0 : 33, | ||
| splitButtonTooltips: Self.currentSplitButtonTooltips(), |
There was a problem hiding this comment.
On macOS 26 hideTabBar is always set to true, which forces tabBarHeight to 0 for all panes. The PR description mentions hiding the tab bar only for single-tab panes; if multi-tab panes should still show a tab bar, this needs conditional logic based on actual tab count/state rather than a blanket OS check.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
Sources/TerminalWindowPortal.swift (1)
1504-1516: Corner radius updates may animate unexpectedly during geometry sync.The surrounding frame/bounds updates are wrapped in a
CATransactionwith disabled actions (lines 1489–1499), but the corner radius changes are applied after the transaction commits. This could cause implicit Core Animation transitions when toggling sidebar visibility, potentially resulting in visual flicker during rapid split/resize operations.Consider wrapping the corner radius update in its own transaction with disabled actions, or moving it inside the existing transaction block:
🔧 Suggested fix
CATransaction.commit() if geometryChanged { hostedView.reconcileGeometryNow() hostedView.refreshSurfaceNow(reason: "portal.frameChange") } // On macOS 26, round the terminal's leading corners when a sidebar // is visible to its left, matching the NavigationSplitView glass shape. if `#available`(macOS 26.0, *) { let hasSidebarToLeft = targetFrame.origin.x > 20 let desiredRadius: CGFloat = hasSidebarToLeft ? 16 : 0 if hostedView.layer?.cornerRadius != desiredRadius { + CATransaction.begin() + CATransaction.setDisableActions(true) hostedView.layer?.cornerRadius = desiredRadius hostedView.layer?.maskedCorners = hasSidebarToLeft ? [.layerMinXMinYCorner, .layerMinXMaxYCorner] : [] + CATransaction.commit() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalWindowPortal.swift` around lines 1504 - 1516, The corner radius and maskedCorners updates on hostedView.layer (the block checking hasSidebarToLeft and setting hostedView.layer?.cornerRadius / hostedView.layer?.maskedCorners) must be performed inside a CATransaction with actions disabled (or moved into the existing CATransaction that updates frame/bounds) to prevent implicit animations during geometry sync; wrap the radius/maskedCorners assignment in a CATransaction.begin()/setDisableActions(true)/commit() pair (or relocate those lines into the existing disabled-action transaction) so changes to cornerRadius and maskedCorners aren’t implicitly animated.Sources/WindowToolbarController.swift (1)
230-236: Remove unnecessary_ =assignments.Both
toggle()andtoggleNotificationsPopover(animated:)returnVoid, so the discard assignments are unnecessary.✨ Suggested cleanup
`@objc` private func toggleSidebarAction() { - _ = AppDelegate.shared?.sidebarState?.toggle() + AppDelegate.shared?.sidebarState?.toggle() } `@objc` private func toggleNotificationsAction() { - _ = AppDelegate.shared?.toggleNotificationsPopover(animated: true) + AppDelegate.shared?.toggleNotificationsPopover(animated: true) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/WindowToolbarController.swift` around lines 230 - 236, The discard assignments are unnecessary: remove the `_ =` when calling AppDelegate.shared?.sidebarState?.toggle() inside toggleSidebarAction() and when calling AppDelegate.shared?.toggleNotificationsPopover(animated: true) inside toggleNotificationsAction() so each method is invoked directly (i.e., call sidebarState?.toggle() and toggleNotificationsPopover(animated: true) without assigning their Void results).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 2601-2605: The notifications popover is never initialized on macOS
26 because titlebarAccessoryController.start() is skipped; call
titlebarAccessoryController.start() for macOS 26 so
UpdateTitlebarAccessoryController populates controlsControllers and the popover
helpers (toggleNotificationsPopover / showNotificationsPopover /
dismissNotificationsPopoverIfShown) work; specifically, remove or modify the
availability check around titlebarAccessoryController.start() in AppDelegate so
start() always runs (or add a macOS26-specific branch that calls start()), and
ensure Start/UpdateTitlebarAccessoryController safely no-ops if
platform-specific internals differ so
WindowToolbarController.toggleNotificationsAction ->
AppDelegate.toggleNotificationsPopover() works on macOS 26.
In `@Sources/ContentView.swift`:
- Around line 2901-2922: The toolbar's image-only Buttons (the bell and plus in
the ToolbarItemGroup) lack localized action names for VoiceOver; replace each
Button's label Image block with a localized Label using String(localized:
"toolbar.notifications", defaultValue: "Notifications") and String(localized:
"toolbar.newTab", defaultValue: "New Tab") (or similar keys), then apply
.labelStyle(.iconOnly) to keep the icon-only appearance; ensure the same
accessibilityIdentifier values ("toolbar.notifications" and "toolbar.newTab")
remain and add corresponding keys to Resources/Localizable.xcstrings with
English and Japanese translations.
- Around line 2884-2895: The native NavigationSplitView configuration only
writes the preferred width into sidebarWidth (via
navigationSplitViewColumnWidth) but never updates
sidebarWidth/sidebarState.persistedWidth when the user drags the native divider,
causing the custom resizer monitor (which reads
sidebarWidth/sidebarState.persistedWidth) to drift; fix this by observing the
actual rendered sidebar width (e.g., with a GeometryReader or .background
preference) inside the sidebarView and, when its width changes beyond a small
threshold, write the new value back into sidebarWidth and
sidebarState.persistedWidth (and persist via sidebarState.save/persist method if
one exists), ensuring updates are debounced/throttled to avoid tight update
loops and only performed when shouldShow/sidebarState.isVisible matches the
split state.
- Around line 15458-15472: The current approach in
hideDividers()/patchSplitViews(in:) uses KVC to set a read-only NSSplitView
property and indiscriminately mutates every split view in the window; instead,
create a focused custom NSSplitView subclass (e.g., TransparentDividerSplitView)
that overrides dividerColor (or drawDivider(in:)) to return/paint clear, then
replace only the specific NavigationSplitView's split view instance with this
subclass (or ensure the NavigationSplitView uses it) rather than recursing from
window.contentView; update hideDividers to target the navigation split view or
its container and remove the setValue(_:forKey:) KVC call and broad traversal.
- Around line 3487-3506: The code sets
window.styleMask.insert(.fullSizeContentView) unconditionally, which contradicts
the macOS 26+ branch that expects the system titlebar to handle dragging without
fullSizeContentView; either make the insertion conditional (only insert
.fullSizeContentView in the else branch for pre-macOS 26) or update the comment
above the macOS 26+ branch to explicitly explain why fullSizeContentView is
still required there; locate the availability checks and the unconditional call
to window.styleMask.insert(.fullSizeContentView) and change it to conditional
insertion tied to the macOS 26 availability or revise the comment near the macOS
26 block to describe the actual layout/drag implications when
.fullSizeContentView is present.
In `@Sources/WindowToolbarController.swift`:
- Around line 193-222: Replace the hard-coded UI strings in the toolbar item
construction with localized strings using String(localized:defaultValue:), e.g.
for items identified by sidebarToggleIdentifier, notificationsIdentifier, and
newTabIdentifier update item.label, item.toolTip, and NSImage(...
accessibilityDescription:) to use String(localized: "TOOLBAR_SIDEBAR_LABEL",
defaultValue: "Sidebar") (and similar keys for tooltip and accessibility text
for each item); ensure the code changes occur in the toolbar item creation block
(the method that builds NSToolbarItem instances and sets item.target/action like
toggleSidebarAction, toggleNotificationsAction, newTabAction) and add the
corresponding keys and translations into Resources/Localizable.xcstrings for
English and Japanese.
In `@Sources/Workspace.swift`:
- Around line 6781-6789: The code unconditionally sets tabBarHeight to 0 on
macOS 26 which hides tabs for all panes; change the helper so tabBarHeight is
computed per-pane (or accept a parameter like tabCount or isSingleTab) and only
return 0 when the pane has a single tab on macOS 26. Update the
BonsplitConfiguration.Appearance creation logic (the call site that currently
uses Self.currentSplitButtonTooltips()) to pass the pane-specific flag or count,
and alter the helper that currently sets hideTabBar to instead check both OS
version and whether the pane is single-tab before returning tabBarHeight 0; keep
the default 33 for multi-tab panes.
---
Nitpick comments:
In `@Sources/TerminalWindowPortal.swift`:
- Around line 1504-1516: The corner radius and maskedCorners updates on
hostedView.layer (the block checking hasSidebarToLeft and setting
hostedView.layer?.cornerRadius / hostedView.layer?.maskedCorners) must be
performed inside a CATransaction with actions disabled (or moved into the
existing CATransaction that updates frame/bounds) to prevent implicit animations
during geometry sync; wrap the radius/maskedCorners assignment in a
CATransaction.begin()/setDisableActions(true)/commit() pair (or relocate those
lines into the existing disabled-action transaction) so changes to cornerRadius
and maskedCorners aren’t implicitly animated.
In `@Sources/WindowToolbarController.swift`:
- Around line 230-236: The discard assignments are unnecessary: remove the `_ =`
when calling AppDelegate.shared?.sidebarState?.toggle() inside
toggleSidebarAction() and when calling
AppDelegate.shared?.toggleNotificationsPopover(animated: true) inside
toggleNotificationsAction() so each method is invoked directly (i.e., call
sidebarState?.toggle() and toggleNotificationsPopover(animated: true) without
assigning their Void results).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b13684ed-2261-47a7-9ab5-3bfe6eab8317
📒 Files selected for processing (8)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/TerminalWindowPortal.swiftSources/WindowToolbarController.swiftSources/Workspace.swiftSources/cmuxApp.swiftvendor/bonsplit
| .toolbar { | ||
| ToolbarItemGroup(placement: .primaryAction) { | ||
| Button { | ||
| _ = AppDelegate.shared?.toggleNotificationsPopover(animated: true) | ||
| } label: { | ||
| Image(systemName: "bell") | ||
| } | ||
| .buttonStyle(.accessoryBarAction) | ||
| .accessibilityIdentifier("toolbar.notifications") | ||
|
|
||
| Button { | ||
| if let appDelegate = AppDelegate.shared { | ||
| if appDelegate.addWorkspaceInPreferredMainWindow(debugSource: "toolbar.newTab") == nil { | ||
| appDelegate.openNewMainWindow(nil) | ||
| } | ||
| } | ||
| } label: { | ||
| Image(systemName: "plus") | ||
| } | ||
| .buttonStyle(.accessoryBarAction) | ||
| .accessibilityIdentifier("toolbar.newTab") | ||
| } |
There was a problem hiding this comment.
Give the new toolbar icons localized action names.
These are image-only buttons, so VoiceOver/help text falls back to the SF Symbol name (or no useful action name). Please switch them to localized Labels and keep the icon-only appearance with .labelStyle(.iconOnly).
Possible fix
.toolbar {
ToolbarItemGroup(placement: .primaryAction) {
Button {
_ = AppDelegate.shared?.toggleNotificationsPopover(animated: true)
} label: {
- Image(systemName: "bell")
+ Label(
+ String(localized: "toolbar.notifications.button", defaultValue: "Notifications"),
+ systemImage: "bell"
+ )
}
+ .labelStyle(.iconOnly)
+ .help(String(localized: "toolbar.notifications.button", defaultValue: "Notifications"))
.buttonStyle(.accessoryBarAction)
.accessibilityIdentifier("toolbar.notifications")
Button {
if let appDelegate = AppDelegate.shared {
if appDelegate.addWorkspaceInPreferredMainWindow(debugSource: "toolbar.newTab") == nil {
appDelegate.openNewMainWindow(nil)
}
}
} label: {
- Image(systemName: "plus")
+ Label(
+ String(localized: "toolbar.newWorkspace.button", defaultValue: "New Workspace"),
+ systemImage: "plus"
+ )
}
+ .labelStyle(.iconOnly)
+ .help(String(localized: "toolbar.newWorkspace.button", defaultValue: "New Workspace"))
.buttonStyle(.accessoryBarAction)
.accessibilityIdentifier("toolbar.newTab")
}
}As per coding guidelines All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") for every string shown in the UI. Keys must go in Resources/Localizable.xcstrings with translations for all supported languages (English and Japanese).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 2901 - 2922, The toolbar's image-only
Buttons (the bell and plus in the ToolbarItemGroup) lack localized action names
for VoiceOver; replace each Button's label Image block with a localized Label
using String(localized: "toolbar.notifications", defaultValue: "Notifications")
and String(localized: "toolbar.newTab", defaultValue: "New Tab") (or similar
keys), then apply .labelStyle(.iconOnly) to keep the icon-only appearance;
ensure the same accessibilityIdentifier values ("toolbar.notifications" and
"toolbar.newTab") remain and add corresponding keys to
Resources/Localizable.xcstrings with English and Japanese translations.
| if #available(macOS 26.0, *) { | ||
| window.titlebarAppearsTransparent = false | ||
| window.titlebarSeparatorStyle = .none | ||
| } else { | ||
| window.titlebarAppearsTransparent = true | ||
| } | ||
| if #available(macOS 26.0, *) { | ||
| // On macOS 26 without fullSizeContentView, the system titlebar | ||
| // handles drag natively. | ||
| window.isMovable = true | ||
| window.isMovableByWindowBackground = false | ||
| } else { | ||
| // Keep window immovable; the sidebar's WindowDragHandleView handles | ||
| // drag-to-move via performDrag with temporary movable override. | ||
| // isMovableByWindowBackground=true breaks tab reordering, and | ||
| // isMovable=true blocks clicks on sidebar buttons in minimal mode. | ||
| window.isMovableByWindowBackground = false | ||
| window.isMovable = false | ||
| } | ||
| window.styleMask.insert(.fullSizeContentView) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
On macOS, what layout effect does NSWindow.StyleMask.fullSizeContentViewhave whentitlebarAppearsTransparent is false, and does it still extend content into the titlebar area?
💡 Result:
NSWindow.StyleMask.fullSizeContentView makes the window’s contentView take the entire window size (including the region normally reserved for the titlebar/toolbar), and you’re expected to use contentLayoutRect / contentLayoutGuide if you want your UI to start below the titlebar/toolbar area. [1]
If titlebarAppearsTransparent is false (the default), the titlebar/toolbar is still drawn on top of that extended content area (typically blurring/obscuring what’s underneath), so your content is still extended into the titlebar region, but it won’t appear “behind” the titlebar as unobscured content. [1][2]
🏁 Script executed:
# Check lines around 2679-2684 to understand the "zero-top-padding macOS 26 path"
sed -n '2670,2700p' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 1428
🏁 Script executed:
# Check the full context of the window configuration section (3487-3506 and surrounding)
sed -n '3480,3520p' Sources/ContentView.swiftRepository: manaflow-ai/cmux
Length of output: 2128
🏁 Script executed:
# Search for all fullSizeContentView references to understand how it's used conditionally
rg 'fullSizeContentView' Sources/ContentView.swift -B 2 -A 2Repository: manaflow-ai/cmux
Length of output: 866
Remove fullSizeContentView from the macOS 26+ path, or update the comment to reflect actual behavior.
The comment at line 3494 says "On macOS 26 without fullSizeContentView, the system titlebar handles drag natively," but line 3506 unconditionally inserts fullSizeContentView for all OS versions. This contradiction is inconsistent with the zero-top-padding design at lines 2679-2684 (which relies on the native glass titlebar to handle safe-area spacing). When fullSizeContentView is active, the contentView extends into the titlebar region, and the layout depends entirely on SwiftUI respecting contentLayoutRect—a fragile dependency that conflicts with the stated intent of letting the macOS 26 native titlebar handle the layout natively.
Either conditionally skip .fullSizeContentView for macOS 26+, or clarify the comment to explain why the native titlebar path still requires extended content bounds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 3487 - 3506, The code sets
window.styleMask.insert(.fullSizeContentView) unconditionally, which contradicts
the macOS 26+ branch that expects the system titlebar to handle dragging without
fullSizeContentView; either make the insertion conditional (only insert
.fullSizeContentView in the else branch for pre-macOS 26) or update the comment
above the macOS 26+ branch to explicitly explain why fullSizeContentView is
still required there; locate the availability checks and the unconditional call
to window.styleMask.insert(.fullSizeContentView) and change it to conditional
insertion tied to the macOS 26 availability or revise the comment near the macOS
26 block to describe the actual layout/drag implications when
.fullSizeContentView is present.
| private func hideDividers() { | ||
| guard let window else { return } | ||
| patchSplitViews(in: window.contentView) | ||
| } | ||
|
|
||
| private func patchSplitViews(in view: NSView?) { | ||
| guard let view else { return } | ||
| if let splitView = view as? NSSplitView { | ||
| splitView.dividerStyle = .thin | ||
| // Override the divider color to clear | ||
| splitView.setValue(NSColor.clear, forKey: "dividerColor") | ||
| } | ||
| for subview in view.subviews { | ||
| patchSplitViews(in: subview) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does AppKit's NSSplitViewpublicly expose adividerColorproperty orsetDividerColor:selector on macOS, and issetValue(_:forKey: "dividerColor") a supported customization path?
💡 Result:
- Public API: On macOS,
NSSplitViewpublicly exposesdividerColoronly as a getter (-dividerColor/var dividerColor: NSColor { get }). Apple’s docs explicitly describe it as something you override to change divider drawing, and the header is shown as readonly. [1], [2] setDividerColor:selector: Apple’s publicNSSplitViewAPI does not document or declare asetDividerColor:setter (i.e.,dividerColoris not a public settable property in AppKit). [1], [2]setValue(_:forKey: "dividerColor"): Using KVC to force-set"dividerColor"is not a supported customization path (it’s not part of the documented API contract, can break across macOS releases, and may fail depending on implementation details). The supported approaches are overridingdividerColorand/ordrawDivider(in:). [1], [3]
Sources:
[1] Apple “NSSplitView Class Reference” PDF (Leopard ADC mirror)
[2] Header excerpt showing @property (readonly, copy) NSColor *dividerColor (blog quoting NSSplitView.h)
[3] Stack Overflow discussions demonstrating the supported pattern of overriding dividerColor / drawDivider(in:) rather than setting it directly
Don't use KVC to set dividerColor; override it instead. Also, scope is too broad.
Line 15468's setValue(_:forKey: "dividerColor") attempts to set a read-only property via KVC. NSSplitView.dividerColor is documented as readonly in AppKit and has no public setter. This unsupported customization path will break across macOS versions and may silently fail or throw at runtime.
Additionally, line 15460 starts from window.contentView, so this recursively patches every NSSplitView in the window, not just the NavigationSplitView shell dividers.
Fix: Create a custom NSSplitView subclass and override dividerColor (to return .clear) or drawDivider(in:) (to skip drawing). Apply it only to the navigation split view that needs the change, not the entire window hierarchy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 15458 - 15472, The current approach
in hideDividers()/patchSplitViews(in:) uses KVC to set a read-only NSSplitView
property and indiscriminately mutates every split view in the window; instead,
create a focused custom NSSplitView subclass (e.g., TransparentDividerSplitView)
that overrides dividerColor (or drawDivider(in:)) to return/paint clear, then
replace only the specific NavigationSplitView's split view instance with this
subclass (or ensure the NavigationSplitView uses it) rather than recursing from
window.contentView; update hideDividers to target the navigation split view or
its container and remove the setValue(_:forKey:) KVC call and broad traversal.
| if #available(macOS 26.0, *) { | ||
| if itemIdentifier == sidebarToggleIdentifier { | ||
| let item = NSToolbarItem(itemIdentifier: itemIdentifier) | ||
| item.image = NSImage(systemSymbolName: "sidebar.left", accessibilityDescription: "Toggle Sidebar") | ||
| item.label = "Sidebar" | ||
| item.toolTip = "Toggle Sidebar" | ||
| item.target = self | ||
| item.action = #selector(toggleSidebarAction) | ||
| return item | ||
| } | ||
|
|
||
| if itemIdentifier == notificationsIdentifier { | ||
| let item = NSToolbarItem(itemIdentifier: itemIdentifier) | ||
| item.image = NSImage(systemSymbolName: "bell", accessibilityDescription: "Notifications") | ||
| item.label = "Notifications" | ||
| item.toolTip = "Show Notifications" | ||
| item.target = self | ||
| item.action = #selector(toggleNotificationsAction) | ||
| return item | ||
| } | ||
|
|
||
| if itemIdentifier == newTabIdentifier { | ||
| let item = NSToolbarItem(itemIdentifier: itemIdentifier) | ||
| item.image = NSImage(systemSymbolName: "plus", accessibilityDescription: "New Workspace") | ||
| item.label = "New Workspace" | ||
| item.toolTip = "New Workspace" | ||
| item.target = self | ||
| item.action = #selector(newTabAction) | ||
| return item | ||
| } |
There was a problem hiding this comment.
User-facing strings must be localized.
The toolbar item labels, tooltips, and accessibility descriptions are bare string literals. As per coding guidelines, all UI strings must use String(localized:defaultValue:).
🌐 Proposed fix for localization
if itemIdentifier == sidebarToggleIdentifier {
let item = NSToolbarItem(itemIdentifier: itemIdentifier)
- item.image = NSImage(systemSymbolName: "sidebar.left", accessibilityDescription: "Toggle Sidebar")
- item.label = "Sidebar"
- item.toolTip = "Toggle Sidebar"
+ item.image = NSImage(systemSymbolName: "sidebar.left", accessibilityDescription: String(localized: "toolbar.sidebar.accessibilityDescription", defaultValue: "Toggle Sidebar"))
+ item.label = String(localized: "toolbar.sidebar.label", defaultValue: "Sidebar")
+ item.toolTip = String(localized: "toolbar.sidebar.tooltip", defaultValue: "Toggle Sidebar")
item.target = self
item.action = `#selector`(toggleSidebarAction)
return item
}
if itemIdentifier == notificationsIdentifier {
let item = NSToolbarItem(itemIdentifier: itemIdentifier)
- item.image = NSImage(systemSymbolName: "bell", accessibilityDescription: "Notifications")
- item.label = "Notifications"
- item.toolTip = "Show Notifications"
+ item.image = NSImage(systemSymbolName: "bell", accessibilityDescription: String(localized: "toolbar.notifications.accessibilityDescription", defaultValue: "Notifications"))
+ item.label = String(localized: "toolbar.notifications.label", defaultValue: "Notifications")
+ item.toolTip = String(localized: "toolbar.notifications.tooltip", defaultValue: "Show Notifications")
item.target = self
item.action = `#selector`(toggleNotificationsAction)
return item
}
if itemIdentifier == newTabIdentifier {
let item = NSToolbarItem(itemIdentifier: itemIdentifier)
- item.image = NSImage(systemSymbolName: "plus", accessibilityDescription: "New Workspace")
- item.label = "New Workspace"
- item.toolTip = "New Workspace"
+ item.image = NSImage(systemSymbolName: "plus", accessibilityDescription: String(localized: "toolbar.newWorkspace.accessibilityDescription", defaultValue: "New Workspace"))
+ item.label = String(localized: "toolbar.newWorkspace.label", defaultValue: "New Workspace")
+ item.toolTip = String(localized: "toolbar.newWorkspace.tooltip", defaultValue: "New Workspace")
item.target = self
item.action = `#selector`(newTabAction)
return item
}Remember to add the new keys to Resources/Localizable.xcstrings with translations for all supported languages (English and Japanese).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/WindowToolbarController.swift` around lines 193 - 222, Replace the
hard-coded UI strings in the toolbar item construction with localized strings
using String(localized:defaultValue:), e.g. for items identified by
sidebarToggleIdentifier, notificationsIdentifier, and newTabIdentifier update
item.label, item.toolTip, and NSImage(... accessibilityDescription:) to use
String(localized: "TOOLBAR_SIDEBAR_LABEL", defaultValue: "Sidebar") (and similar
keys for tooltip and accessibility text for each item); ensure the code changes
occur in the toolbar item creation block (the method that builds NSToolbarItem
instances and sets item.target/action like toggleSidebarAction,
toggleNotificationsAction, newTabAction) and add the corresponding keys and
translations into Resources/Localizable.xcstrings for English and Japanese.
| let hideTabBar: Bool | ||
| if #available(macOS 26.0, *) { | ||
| hideTabBar = true | ||
| } else { | ||
| hideTabBar = false | ||
| } | ||
| return BonsplitConfiguration.Appearance( | ||
| tabBarHeight: hideTabBar ? 0 : 33, | ||
| splitButtonTooltips: Self.currentSplitButtonTooltips(), |
There was a problem hiding this comment.
Avoid globally hiding the tab bar on macOS 26.
Line 6788 sets tabBarHeight to 0 for all panes on macOS 26, which hides tabs even when a pane has multiple tabs. The PR objective calls for hiding only single-tab panes, so this can regress multi-tab navigation.
💡 Suggested adjustment in this helper
- let hideTabBar: Bool
- if `#available`(macOS 26.0, *) {
- hideTabBar = true
- } else {
- hideTabBar = false
- }
+ // Keep global appearance stable here. Single-tab hiding should be applied
+ // at the pane/tab-rendering layer where tab counts are available.
+ let hideTabBar = false
return BonsplitConfiguration.Appearance(
tabBarHeight: hideTabBar ? 0 : 33,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 6781 - 6789, The code unconditionally
sets tabBarHeight to 0 on macOS 26 which hides tabs for all panes; change the
helper so tabBarHeight is computed per-pane (or accept a parameter like tabCount
or isSingleTab) and only return 0 when the pane has a single tab on macOS 26.
Update the BonsplitConfiguration.Appearance creation logic (the call site that
currently uses Self.currentSplitButtonTooltips()) to pass the pane-specific flag
or count, and alter the helper that currently sets hideTabBar to instead check
both OS version and whether the pane is single-tab before returning tabBarHeight
0; keep the default 33 for multi-tab panes.
- Fix bell button no-op: start titlebarAccessoryController on macOS 26 so notifications popover infrastructure is available - Add tab bar design comment: bonsplit shows tabs when pane.tabs > 1 - Replace dividerColor KVC with safe responds(to:)/perform selector - Add accessibility labels to toolbar buttons (bell, new tab) - Localize all WindowToolbarController toolbar item strings - Add comment about NavigationSplitView width persistence limitation - Move corner radius update inside CATransaction to prevent flicker - Replace magic number 20 with named sidebarDetectionThreshold constant - Remove unused TerminalBackgroundFill struct - Fix SplitViewDividerHider doc comment and remove unused property - Add comment explaining uniform 6pt content inset design
…macOS 26 - Fix bell button no-op: use SwiftUI .popover on macOS 26 instead of the broken AppKit titlebar accessory path; bridge keyboard shortcut via NotificationCenter - Move pane action buttons (terminal, browser, split right/down) from bonsplit tab bar to native toolbar as a ControlGroup on macOS 26 - Hide bonsplit split buttons when tab bar is hidden (macOS 26) - Hide old HiddenTitlebarSidebarControlsView on macOS 26 - Remove system sidebar toggle chevron (.toolbar(removing: .sidebarToggle))
- Fix asymmetric inset: only inset leading/top edges where corners are rounded, preserving terminal real estate on right/bottom - Fix misleading comment about fullSizeContentView on macOS 26 - Update SplitViewDividerHider doc to match actual implementation - Clarify hideTabBar comment: PaneContainerView conditionally shows tab bar for multi-tab panes - Improve sidebar detection heuristic comment explaining why frame-based approach is used (AppKit view cannot access SwiftUI SidebarState) - Add GeometryReader to persist sidebar width on macOS 26 when user drags the native NavigationSplitView divider
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Sources/ContentView.swift (1)
15475-15520:⚠️ Potential issue | 🟠 MajorAvoid private selector dispatch on
NSSplitView.Line 15511 uses
setDividerColor:via runtime selector dispatch. This is not a public API—the macOS SDK documents only adividerColorgetter onNSSplitViewand does not expose a public setter. Using undocumented selectors is brittle and can break across SDK/runtime updates.Instead, keep only public APIs (like
dividerStyle). If visual hiding is still needed, mask with a SwiftUI/AppKit overlay as a safer fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 15475 - 15520, The code uses a private ObjC selector NSSelectorFromString("setDividerColor:") inside patchSplitViews(in:) to call setDividerColor: on NSSplitView; remove that selector dispatch and keep only the public API change (splitView.dividerStyle = .thin) in SplitViewDividerHiderView.patchSplitViews(in:). If you still need to visually hide dividers, implement a public-API fallback: add a safe masking helper (e.g., addDividerMask(to:) called from patchSplitViews) that inserts an NSView overlay or adjusts the split view’s layer (wantsLayer = true and layer properties) to cover the divider area, and remove the NSSelectorFromString/perform(...) block entirely.
🧹 Nitpick comments (3)
Sources/GhosttyTerminalView.swift (1)
9118-9134: Consider tying the leading inset to the same sidebar-presence condition used for corner rounding.
Sources/TerminalWindowPortal.swiftonly rounds leading corners when a sidebar is detected; keeping this inset always-on may leave unnecessary gutter when the sidebar is hidden. A shared condition (or propagated flag) would keep geometry behavior aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 9118 - 9134, The leading inset is always applied in GhosttyTerminalView despite corner rounding only happening when the sidebar is visible; update the inset calculation to apply the inset only when the same "sidebar visible" condition used in TerminalWindowPortal.swift is true (propagate or query that flag into GhosttyTerminalView), e.g. compute inset = (isSidebarVisible ? 6 : 0) before building scrollFrame, and then call setFrameIfNeeded(scrollView, to: scrollFrame) as before so geometry and corner rounding remain in sync; refer to the scrollFrame/inset calculation and the call to setFrameIfNeeded(scrollView, to:) when locating the change.Sources/WindowToolbarController.swift (1)
230-232: Minor: Unnecessary discard assignment.
SidebarState.toggle()returnsVoid, so the_ =prefix is superfluous.♻️ Suggested simplification
`@objc` private func toggleSidebarAction() { - _ = AppDelegate.shared?.sidebarState?.toggle() + AppDelegate.shared?.sidebarState?.toggle() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/WindowToolbarController.swift` around lines 230 - 232, The toggleSidebarAction method contains an unnecessary discard assignment; remove the "_ =" so the call simply invokes AppDelegate.shared?.sidebarState?.toggle() because SidebarState.toggle() returns Void. Update the toggleSidebarAction function to call SidebarState.toggle() without assigning its result (targeting the invocation in toggleSidebarAction and the SidebarState.toggle call).Sources/AppDelegate.swift (1)
2601-2605: Decouple the macOS 26 notifications bridge from legacy accessory startup.
titlebarAccessoryController.start()now runs even thoughattachUpdateAccessory(to:)never attaches on macOS 26. That still keeps the legacy controller’s observer/startup lifecycle alive after the UI moved to the SwiftUI toolbar/popover path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 2601 - 2605, The legacy accessory controller is being started unconditionally via titlebarAccessoryController.start() even on macOS 26 where attachUpdateAccessory(to:) is not used; stop starting the legacy controller in that case to avoid keeping its observers/lifecycle alive. Update the startup logic so titlebarAccessoryController.start() is only called when the legacy accessory path will be attached (e.g., guard the call behind the same OS-version or feature check used by attachUpdateAccessory(to:) — or add a predicate like shouldUseLegacyAccessory() — and call start() only when that predicate is true), leaving the SwiftUI toolbar/popover path on macOS 26 untouched. Ensure references to titlebarAccessoryController and attachUpdateAccessory(to:) are adjusted consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 10168-10176: The macOS 26 bridge only posts a global toggle and
drops the originating window, causing desync with titlebarAccessoryController;
change the NotificationCenter bridge used by
toggleNotificationsPopover(animated:anchorView:) to include identifying info
(e.g., the originating NSWindow or a unique window identifier in userInfo) so
the SwiftUI presenter has a deterministic target, and then route
showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and
isNotificationsPopoverShown() to use the same macOS 26 notification/presenter
path instead of calling titlebarAccessoryController directly (keep the legacy
controller path for <macOS26). Locate and update the symbols
toggleNotificationsPopover(animated:anchorView:),
showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and
isNotificationsPopoverShown() to post/observe and query the unified presenter
notification (using userInfo keys) so all entry points target the same presenter
on macOS 26.
In `@Sources/ContentView.swift`:
- Around line 2967-2969: The global notification handler in ContentView
(.onReceive(NotificationCenter.default.publisher(for:
AppDelegate.toggleNotificationsPopoverNotification))) unconditionally toggles
isNotificationsPopoverPresented causing all windows to change; change the
onReceive closure to first cast notification.object to an NSWindow and only
toggle when that window matches this view’s hosting window (i.e., compare
notification.object as? NSWindow to this ContentView’s window/hosting NSWindow),
and update AppDelegate.toggleNotificationsPopoverNotification posts to include
the NSWindow as the notification object so the subscriber can perform the match.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 9123-9130: The inset calculation in GhosttyTerminalView (inside
the macOS 26.0 availability branch where scrollFrame is computed) can produce
negative width/height when bounds are smaller than the hardcoded inset; fix this
by clamping computed width and height to non-negative values (e.g., use
max(bounds.width - inset, 0) and max(bounds.height - inset, 0)) and adjust the
x/y inset application to not exceed the bounds (e.g., compute effectiveInset =
min(inset, bounds.width) for x and min(inset, bounds.height) for y) before
assigning scrollFrame so the resulting CGRect never has negative dimensions.
---
Duplicate comments:
In `@Sources/ContentView.swift`:
- Around line 15475-15520: The code uses a private ObjC selector
NSSelectorFromString("setDividerColor:") inside patchSplitViews(in:) to call
setDividerColor: on NSSplitView; remove that selector dispatch and keep only the
public API change (splitView.dividerStyle = .thin) in
SplitViewDividerHiderView.patchSplitViews(in:). If you still need to visually
hide dividers, implement a public-API fallback: add a safe masking helper (e.g.,
addDividerMask(to:) called from patchSplitViews) that inserts an NSView overlay
or adjusts the split view’s layer (wantsLayer = true and layer properties) to
cover the divider area, and remove the NSSelectorFromString/perform(...) block
entirely.
---
Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Around line 2601-2605: The legacy accessory controller is being started
unconditionally via titlebarAccessoryController.start() even on macOS 26 where
attachUpdateAccessory(to:) is not used; stop starting the legacy controller in
that case to avoid keeping its observers/lifecycle alive. Update the startup
logic so titlebarAccessoryController.start() is only called when the legacy
accessory path will be attached (e.g., guard the call behind the same OS-version
or feature check used by attachUpdateAccessory(to:) — or add a predicate like
shouldUseLegacyAccessory() — and call start() only when that predicate is true),
leaving the SwiftUI toolbar/popover path on macOS 26 untouched. Ensure
references to titlebarAccessoryController and attachUpdateAccessory(to:) are
adjusted consistently.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 9118-9134: The leading inset is always applied in
GhosttyTerminalView despite corner rounding only happening when the sidebar is
visible; update the inset calculation to apply the inset only when the same
"sidebar visible" condition used in TerminalWindowPortal.swift is true
(propagate or query that flag into GhosttyTerminalView), e.g. compute inset =
(isSidebarVisible ? 6 : 0) before building scrollFrame, and then call
setFrameIfNeeded(scrollView, to: scrollFrame) as before so geometry and corner
rounding remain in sync; refer to the scrollFrame/inset calculation and the call
to setFrameIfNeeded(scrollView, to:) when locating the change.
In `@Sources/WindowToolbarController.swift`:
- Around line 230-232: The toggleSidebarAction method contains an unnecessary
discard assignment; remove the "_ =" so the call simply invokes
AppDelegate.shared?.sidebarState?.toggle() because SidebarState.toggle() returns
Void. Update the toggleSidebarAction function to call SidebarState.toggle()
without assigning its result (targeting the invocation in toggleSidebarAction
and the SidebarState.toggle call).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04b9fbdf-fc7b-405a-a08d-2f294a14cb52
📒 Files selected for processing (7)
Sources/AppDelegate.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/TerminalWindowPortal.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/WindowToolbarController.swiftSources/Workspace.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/Update/UpdateTitlebarAccessory.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/TerminalWindowPortal.swift
| /// Notification posted on macOS 26 to toggle the SwiftUI notifications popover. | ||
| static let toggleNotificationsPopoverNotification = Notification.Name("cmux.toggleNotificationsPopover") | ||
|
|
||
| func toggleNotificationsPopover(animated: Bool = true, anchorView: NSView? = nil) { | ||
| if #available(macOS 26.0, *) { | ||
| NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil) | ||
| return | ||
| } | ||
| titlebarAccessoryController.toggleNotificationsPopover(animated: animated, anchorView: anchorView) |
There was a problem hiding this comment.
Scope and complete the macOS 26 notifications presenter.
This bridge only emits a global toggle and drops the originating window. The SwiftUI side therefore has no deterministic target window, and the explicit show/dismiss/state entry points in this file (showNotificationsPopoverFromMenuBar, dismissNotificationsPopoverIfShown, isNotificationsPopoverShown) still talk to titlebarAccessoryController. On macOS 26 that can desync menu-bar/Escape behavior from the popover that is actually visible.
🛠️ Suggested direction
/// Notification posted on macOS 26 to toggle the SwiftUI notifications popover.
static let toggleNotificationsPopoverNotification = Notification.Name("cmux.toggleNotificationsPopover")
+ static let showNotificationsPopoverNotification = Notification.Name("cmux.showNotificationsPopover")
+ static let dismissNotificationsPopoverNotification = Notification.Name("cmux.dismissNotificationsPopover")
+ static let notificationsPopoverWindowIdUserInfoKey = "windowId"
func toggleNotificationsPopover(animated: Bool = true, anchorView: NSView? = nil) {
if `#available`(macOS 26.0, *) {
- NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil)
+ guard
+ let window = anchorView?.window ?? NSApp.keyWindow ?? NSApp.mainWindow,
+ let windowId = mainWindowId(for: window)
+ else { return }
+ NotificationCenter.default.post(
+ name: Self.toggleNotificationsPopoverNotification,
+ object: window,
+ userInfo: [Self.notificationsPopoverWindowIdUserInfoKey: windowId]
+ )
return
}
titlebarAccessoryController.toggleNotificationsPopover(animated: animated, anchorView: anchorView)
}Then route showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and isNotificationsPopoverShown() through the same macOS 26 presenter/observer path instead of the legacy accessory controller.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 10168 - 10176, The macOS 26 bridge
only posts a global toggle and drops the originating window, causing desync with
titlebarAccessoryController; change the NotificationCenter bridge used by
toggleNotificationsPopover(animated:anchorView:) to include identifying info
(e.g., the originating NSWindow or a unique window identifier in userInfo) so
the SwiftUI presenter has a deterministic target, and then route
showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and
isNotificationsPopoverShown() to use the same macOS 26 notification/presenter
path instead of calling titlebarAccessoryController directly (keep the legacy
controller path for <macOS26). Locate and update the symbols
toggleNotificationsPopover(animated:anchorView:),
showNotificationsPopoverFromMenuBar(), dismissNotificationsPopoverIfShown(), and
isNotificationsPopoverShown() to post/observe and query the unified presenter
notification (using userInfo keys) so all entry points target the same presenter
on macOS 26.
| .onReceive(NotificationCenter.default.publisher(for: AppDelegate.toggleNotificationsPopoverNotification)) { _ in | ||
| isNotificationsPopoverPresented.toggle() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether toggleNotificationsPopoverNotification posts are window-scoped.
rg -n --type=swift -C3 'toggleNotificationsPopoverNotification|post\(\s*name:\s*AppDelegate\.toggleNotificationsPopoverNotification'Repository: manaflow-ai/cmux
Length of output: 1580
Scope notifications popover toggles to the intended window.
Line 2967 subscribes to a global notification and unconditionally toggles local state. In multi-window sessions, posting one notification toggles all ContentView instances' popovers simultaneously.
Suggested fix
- .onReceive(NotificationCenter.default.publisher(for: AppDelegate.toggleNotificationsPopoverNotification)) { _ in
- isNotificationsPopoverPresented.toggle()
- }
+ .onReceive(NotificationCenter.default.publisher(for: AppDelegate.toggleNotificationsPopoverNotification)) { notification in
+ if let targetWindow = notification.object as? NSWindow {
+ guard targetWindow === observedWindow else { return }
+ }
+ isNotificationsPopoverPresented.toggle()
+ }Also update AppDelegate.swift line 10173 to post the notification with the window object:
- NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil)
+ NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: NSApp.keyWindow)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 2967 - 2969, The global notification
handler in ContentView (.onReceive(NotificationCenter.default.publisher(for:
AppDelegate.toggleNotificationsPopoverNotification))) unconditionally toggles
isNotificationsPopoverPresented causing all windows to change; change the
onReceive closure to first cast notification.object to an NSWindow and only
toggle when that window matches this view’s hosting window (i.e., compare
notification.object as? NSWindow to this ContentView’s window/hosting NSWindow),
and update AppDelegate.toggleNotificationsPopoverNotification posts to include
the NSWindow as the notification object so the subscriber can perform the match.
| if #available(macOS 26.0, *) { | ||
| let inset: CGFloat = 6 | ||
| scrollFrame = CGRect( | ||
| x: bounds.origin.x + inset, | ||
| y: bounds.origin.y, | ||
| width: bounds.width - inset, | ||
| height: bounds.height - inset | ||
| ) |
There was a problem hiding this comment.
Clamp inset geometry to avoid negative frame sizes during transient layout.
When bounds are smaller than the 6pt inset (e.g., during attach/detach or live transitions), this can produce negative width/height and unstable scroll geometry.
Proposed fix
if `#available`(macOS 26.0, *) {
let inset: CGFloat = 6
+ let xInset = min(inset, max(0, bounds.width))
+ let topInset = min(inset, max(0, bounds.height))
scrollFrame = CGRect(
- x: bounds.origin.x + inset,
+ x: bounds.origin.x + xInset,
y: bounds.origin.y,
- width: bounds.width - inset,
- height: bounds.height - inset
+ width: max(0, bounds.width - xInset),
+ height: max(0, bounds.height - topInset)
)
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if #available(macOS 26.0, *) { | |
| let inset: CGFloat = 6 | |
| scrollFrame = CGRect( | |
| x: bounds.origin.x + inset, | |
| y: bounds.origin.y, | |
| width: bounds.width - inset, | |
| height: bounds.height - inset | |
| ) | |
| if `#available`(macOS 26.0, *) { | |
| let inset: CGFloat = 6 | |
| let xInset = min(inset, max(0, bounds.width)) | |
| let topInset = min(inset, max(0, bounds.height)) | |
| scrollFrame = CGRect( | |
| x: bounds.origin.x + xInset, | |
| y: bounds.origin.y, | |
| width: max(0, bounds.width - xInset), | |
| height: max(0, bounds.height - topInset) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 9123 - 9130, The inset
calculation in GhosttyTerminalView (inside the macOS 26.0 availability branch
where scrollFrame is computed) can produce negative width/height when bounds are
smaller than the hardcoded inset; fix this by clamping computed width and height
to non-negative values (e.g., use max(bounds.width - inset, 0) and
max(bounds.height - inset, 0)) and adjust the x/y inset application to not
exceed the bounds (e.g., compute effectiveInset = min(inset, bounds.width) for x
and min(inset, bounds.height) for y) before assigning scrollFrame so the
resulting CGRect never has negative dimensions.
…s on macOS 26 - Use flexible frame for sidebar in NavigationSplitView so rows extend full width instead of floating centered with gaps - Skip attaching legacy TitlebarControlsAccessoryViewController on macOS 26 since NavigationSplitView toolbar provides its own controls - Add unread notification badge to the new toolbar bell button
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit eba5d8c. Configure here.
| if #available(macOS 26.0, *) { | ||
| NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil) | ||
| return | ||
| } |
There was a problem hiding this comment.
Notifications popover state queries broken on macOS 26
High Severity
On macOS 26, dismissNotificationsPopoverIfShown() and isNotificationsPopoverShown() incorrectly delegate to titlebarAccessoryController. As the notifications popover is SwiftUI-managed on this OS, these functions don't reflect its actual state, breaking Escape key dismissal and allowing typing to leak into the terminal.
Reviewed by Cursor Bugbot for commit eba5d8c. Configure here.
| appDelegate.openNewMainWindow(nil) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
WindowToolbarController is never instantiated — dead code
Low Severity
The WindowToolbarController is never instantiated, making its new macOS 26 toolbar items and their associated action methods dead code. The application's macOS 26 toolbar functionality is already provided by SwiftUI's .toolbar in ContentView.
Reviewed by Cursor Bugbot for commit eba5d8c. Configure here.
| func toggleNotificationsPopover(animated: Bool = true, anchorView: NSView? = nil) { | ||
| if #available(macOS 26.0, *) { | ||
| NotificationCenter.default.post(name: Self.toggleNotificationsPopoverNotification, object: nil) | ||
| return |
There was a problem hiding this comment.
Notification popover broadcast toggles all windows simultaneously
Medium Severity
On macOS 26, toggleNotificationsPopover() posts toggleNotificationsPopoverNotification to NotificationCenter.default with no window-scoping information. Every ContentView instance subscribes via .onReceive and toggles its own isNotificationsPopoverPresented state. In a multi-window setup, triggering the notification shortcut or fullscreen controls bell button causes ALL windows' popovers to toggle simultaneously instead of only the active window's.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit eba5d8c. Configure here.
Summary
.toolbarfor bell and new-tab buttons with.accessoryBarActionstyleKnown limitations
Test plan
🤖 Generated with Claude Code
Summary by cubic
Adopts macOS 26 Liquid Glass with NavigationSplitView, a native system titlebar, and a SwiftUI toolbar for pane actions, notifications (with unread badge), and new tab. Refines terminal spacing/corners, hides split dividers, fills sidebar row width, and removes legacy titlebar/sidebar UI.
New Features
.popoverwith an unread badge and a NotificationCenter toggle, and new tab; localized labels and accessibility IDs; start the accessory controller on all versions for popover support, but skip attaching legacy titlebar accessories on macOS 26.NSSplitViewdividers and prevent corner-radius flicker.bonsplittab bar and split buttons for single-tab panes; hide legacy sidebar controls on macOS 26; let the system handle app icon variants.Dependencies
bonsplitsubmodule.Written for commit eba5d8c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Note
Medium Risk
Medium risk due to widespread window/layout changes gated on
macOS 26(safe area, titlebar, sidebar/terminal geometry) plus a new private-selector hack to hideNSSplitViewdividers that could regress on OS updates.Overview
Updates macOS 26 behavior to adopt native Liquid Glass: switches the main layout to
NavigationSplitView, relies on system safe areas/titlebar rendering, and replaces legacy titlebar overlay/accessory UI with SwiftUI.toolbarcontrols (including a SwiftUI notifications.popovertoggled viaNotificationCenter).Adjusts window/terminal presentation on macOS 26 (movability, transparent titlebar handling, disables custom
NSGlassEffectViewinsertion, adds outer/inner padding and leading-corner rounding when the sidebar is present, and hides split dividers via anNSSplitViewpatch), while keeping pre-26 code paths using the existing custom glass/titlebar implementations. Also adds subtle glass hover/active effects in the sidebar, hides the bonsplit tab bar for single-tab panes on macOS 26, and skips custom app icon appearance overrides on macOS 26.Reviewed by Cursor Bugbot for commit eba5d8c. Bugbot is set up for automated code reviews on this repo. Configure here.