Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7131,6 +7131,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
let sidebarSelectionState = SidebarSelectionState(
selection: sessionWindowSnapshot?.sidebar.selection.sidebarSelection ?? .tabs
)

// Seed the per-window Bonsplit tab-bar leading inset before ContentView first
// renders. The initial workspace is created inside TabManager.init, at which
// point there is no source workspace or prior window inset to inherit from, so
// applyCreationChromeInheritance returns early and leaves the Bonsplit inset
// at 0 — which is wrong in minimal mode with the sidebar collapsed, where the
// native traffic lights need an 80pt reserved strip on the tab bar. Without
// this seed, the first-frame layout can mispaint in the new window until
// ContentView.onAppear eventually runs syncTrafficLightInset (#2737).
let initialTabBarLeadingInset: CGFloat =
(WorkspacePresentationModeSettings.isMinimal() && !sidebarState.isVisible)
? 80
: 0
tabManager.syncWorkspaceTabBarLeadingInset(initialTabBarLeadingInset)
let notificationStore = TerminalNotificationStore.shared

let cmuxConfigStore = CmuxConfigStore()
Expand Down
40 changes: 31 additions & 9 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2771,6 +2771,9 @@ struct ContentView: View {
/// Space at top of content area for the titlebar. This must be at least the actual titlebar
/// height; otherwise controls like Bonsplit tab dragging can be interpreted as window drags.
@State private var titlebarPadding: CGFloat = 32
/// SwiftUI WindowGroup windows can still report a titlebar safe area; manually created
/// main windows use MainWindowHostingView and report zero.
@State private var hostingSafeAreaTop: CGFloat = 0
@AppStorage(WorkspacePresentationModeSettings.modeKey)
private var workspacePresentationMode = WorkspacePresentationModeSettings.defaultMode.rawValue

Expand All @@ -2779,10 +2782,23 @@ struct ContentView: View {
}

private var effectiveTitlebarPadding: CGFloat {
if isMinimalMode {
return isFullScreen ? 0 : -titlebarPadding
}
return titlebarPadding
Self.effectiveTitlebarPadding(
isMinimalMode: isMinimalMode,
isFullScreen: isFullScreen,
titlebarPadding: titlebarPadding,
hostingSafeAreaTop: hostingSafeAreaTop
)
}

static func effectiveTitlebarPadding(
isMinimalMode: Bool,
isFullScreen: Bool,
titlebarPadding: CGFloat,
hostingSafeAreaTop: CGFloat
) -> CGFloat {
guard isMinimalMode else { return titlebarPadding }
guard !isFullScreen else { return 0 }
return -max(0, min(titlebarPadding, hostingSafeAreaTop))
}

private var terminalContent: some View {
Expand Down Expand Up @@ -2994,11 +3010,7 @@ struct ContentView: View {

private func syncTrafficLightInset() {
let inset: CGFloat = (isMinimalMode && !sidebarState.isVisible && !isFullScreen) ? 80 : 0
for tab in tabManager.tabs {
if tab.bonsplitController.configuration.appearance.tabBarLeadingInset != inset {
tab.bonsplitController.configuration.appearance.tabBarLeadingInset = inset
}
}
tabManager.syncWorkspaceTabBarLeadingInset(inset)
}

private func updateTitlebarText() {
Expand Down Expand Up @@ -3732,6 +3744,10 @@ struct ContentView: View {
syncTrafficLightInset()
})

view = AnyView(view.onChange(of: tabManager.tabs.map(\.id)) { _ in
syncTrafficLightInset()
})
Comment on lines +3747 to +3749

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inconsistent onChange closure arity

The new handler uses the single-argument (old/deprecated) onChange form while the immediately-preceding handler on line 3727 uses the two-argument (oldValue, newValue) form that is consistent with the rest of this function. Both work at runtime, but mixing the two APIs may generate a deprecation warning in newer Xcode versions and is inconsistent with the surrounding code.

Suggested change
view = AnyView(view.onChange(of: tabManager.tabs.map(\.id)) { _ in
syncTrafficLightInset()
})
view = AnyView(view.onChange(of: tabManager.tabs.map(\.id)) { _, _ in
syncTrafficLightInset()
})


view = AnyView(view.onChange(of: sidebarState.persistedWidth) { newValue in
let sanitized = normalizedSidebarWidth(newValue)
if abs(newValue - sanitized) > 0.5 {
Expand Down Expand Up @@ -3785,11 +3801,17 @@ struct ContentView: View {
// get interpreted as window drags.
let computedTitlebarHeight = window.frame.height - window.contentLayoutRect.height
let nextPadding = max(28, min(72, computedTitlebarHeight))
let nextSafeAreaTop = max(0, window.contentView?.safeAreaInsets.top ?? 0)
if abs(titlebarPadding - nextPadding) > 0.5 {
DispatchQueue.main.async {
titlebarPadding = nextPadding
}
}
if abs(hostingSafeAreaTop - nextSafeAreaTop) > 0.5 {
DispatchQueue.main.async {
hostingSafeAreaTop = nextSafeAreaTop
}
}
#if DEBUG
if ProcessInfo.processInfo.environment["CMUX_UI_TEST_MODE"] == "1" {
UpdateLogStore.shared.append("ui test window accessor: id=\(windowIdentifier) visible=\(window.isVisible)")
Expand Down
22 changes: 18 additions & 4 deletions Sources/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,7 @@ class TabManager: ObservableObject {
private var workspaceCycleCooldownTask: Task<Void, Never>?
private var pendingWorkspaceUnfocusTarget: (tabId: UUID, panelId: UUID)?
private var sidebarSelectedWorkspaceIds: Set<UUID> = []
private var currentWindowTabBarLeadingInset: CGFloat?
private var closeConfirmationInFlight = false
var confirmCloseHandler: ((String, String, Bool) -> Bool)?
private struct WorkspaceCreationTabSnapshot {
Expand Down Expand Up @@ -1943,13 +1944,26 @@ class TabManager: ObservableObject {
to newWorkspace: Workspace,
from sourceWorkspace: Workspace?
) {
guard let sourceWorkspace else { return }
// Sidebar-toggle relayout updates the live Bonsplit leading inset so minimal-mode
// workspaces reserve traffic-light space. New workspaces need that same inset
// copied immediately because creation itself does not trigger the resync path.
let inheritedLeadingInset = sourceWorkspace.bonsplitController.configuration.appearance.tabBarLeadingInset
if newWorkspace.bonsplitController.configuration.appearance.tabBarLeadingInset != inheritedLeadingInset {
newWorkspace.bonsplitController.configuration.appearance.tabBarLeadingInset = inheritedLeadingInset
let inheritedLeadingInset = currentWindowTabBarLeadingInset
?? sourceWorkspace?.bonsplitController.configuration.appearance.tabBarLeadingInset
guard let inheritedLeadingInset else { return }
Comment on lines +1950 to +1952

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 First-workspace creation with uninitialised cache

If currentWindowTabBarLeadingInset is still nil (i.e. syncWorkspaceTabBarLeadingInset has not yet been called by ContentView) and sourceWorkspace is also nil, inheritedLeadingInset resolves to nil and the guard exits — leaving the new workspace with whatever default inset its BonsplitController happened to have. In practice the run-loop round-trip before workspace creation means the value is already populated, but there's no longer a hard guarantee from the type system. The old code had the same hole (guard let sourceWorkspace else { return }), so this is not a regression; just worth a comment clarifying the invariant.

applyTabBarLeadingInset(inheritedLeadingInset, to: newWorkspace)
}

func syncWorkspaceTabBarLeadingInset(_ inset: CGFloat) {
let normalizedInset = max(0, inset)
currentWindowTabBarLeadingInset = normalizedInset
for tab in tabs {
applyTabBarLeadingInset(normalizedInset, to: tab)
}
}

private func applyTabBarLeadingInset(_ inset: CGFloat, to workspace: Workspace) {
if workspace.bonsplitController.configuration.appearance.tabBarLeadingInset != inset {
workspace.bonsplitController.configuration.appearance.tabBarLeadingInset = inset
}
}

Expand Down
158 changes: 158 additions & 0 deletions cmuxTests/AppDelegateShortcutRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1598,6 +1598,164 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
)
}

func testMinimalModeTitlebarPaddingOnlyCancelsHostingSafeArea() {
XCTAssertEqual(
ContentView.effectiveTitlebarPadding(
isMinimalMode: false,
isFullScreen: false,
titlebarPadding: 32,
hostingSafeAreaTop: 0
),
32,
accuracy: 0.5,
"Standard mode should keep reserving titlebar space above terminal content"
)

XCTAssertEqual(
ContentView.effectiveTitlebarPadding(
isMinimalMode: true,
isFullScreen: true,
titlebarPadding: 32,
hostingSafeAreaTop: 32
),
0,
accuracy: 0.5,
"Fullscreen minimal mode should not offset for a titlebar"
)

XCTAssertEqual(
ContentView.effectiveTitlebarPadding(
isMinimalMode: true,
isFullScreen: false,
titlebarPadding: 32,
hostingSafeAreaTop: 0
),
0,
accuracy: 0.5,
"Manually hosted minimal windows already have zero safe area, so the Bonsplit strip must not be pulled offscreen"
)

XCTAssertEqual(
ContentView.effectiveTitlebarPadding(
isMinimalMode: true,
isFullScreen: false,
titlebarPadding: 32,
hostingSafeAreaTop: 28
),
-28,
accuracy: 0.5,
"SwiftUI WindowGroup windows still need their native titlebar safe area cancelled"
)
}

func testMinimalModeCollapsedSidebarResyncsTrafficLightInsetAfterNewWorkspaceCreation() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

let defaults = UserDefaults.standard
let savedMode = defaults.object(forKey: WorkspacePresentationModeSettings.modeKey)
defaults.set(WorkspacePresentationModeSettings.Mode.minimal.rawValue, forKey: WorkspacePresentationModeSettings.modeKey)
defer {
restoreDefaultsValue(savedMode, forKey: WorkspacePresentationModeSettings.modeKey, defaults: defaults)
}

let snapshot = SessionWindowSnapshot(
frame: nil,
display: nil,
tabManager: SessionTabManagerSnapshot(selectedWorkspaceIndex: nil, workspaces: []),
sidebar: SessionSidebarSnapshot(isVisible: false, selection: .tabs, width: nil)
)
let windowId = appDelegate.createMainWindow(sessionWindowSnapshot: snapshot)
defer { closeWindow(withId: windowId) }

guard let manager = appDelegate.tabManagerFor(windowId: windowId) else {
XCTFail("Expected tab manager for created window")
return
}

RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1))

XCTAssertEqual(appDelegate.sidebarVisibility(windowId: windowId), false)

guard let sourceWorkspace = manager.selectedWorkspace else {
XCTFail("Expected selected workspace")
return
}

// Recreate the regression shape: the window chrome state says minimal +
// collapsed sidebar, but the selected workspace's live Bonsplit inset is stale.
sourceWorkspace.bonsplitController.configuration.appearance.tabBarLeadingInset = 0

guard let newWorkspaceId = appDelegate.addWorkspaceInPreferredMainWindow(debugSource: "test.issue2737") else {
XCTFail("Expected workspace creation to route to the test window")
return
}

RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1))

guard let newWorkspace = manager.tabs.first(where: { $0.id == newWorkspaceId }) else {
XCTFail("Expected new workspace in test window")
return
}

XCTAssertEqual(
newWorkspace.bonsplitController.configuration.appearance.tabBarLeadingInset,
80,
accuracy: 0.5,
"New minimal-mode workspaces should reserve traffic-light space immediately even when the source workspace inset is stale"
)
}

func testMinimalModeCollapsedSidebarSeedsTrafficLightInsetOnNewWindowCreation() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
return
}

let defaults = UserDefaults.standard
let savedMode = defaults.object(forKey: WorkspacePresentationModeSettings.modeKey)
defaults.set(WorkspacePresentationModeSettings.Mode.minimal.rawValue, forKey: WorkspacePresentationModeSettings.modeKey)
defer {
restoreDefaultsValue(savedMode, forKey: WorkspacePresentationModeSettings.modeKey, defaults: defaults)
}

// Simulate the new-window flow: createMainWindow with a snapshot that forces
// sidebar collapsed. The initial workspace is created inside TabManager.init,
// before ContentView.onAppear can run syncTrafficLightInset — so the seed in
// createMainWindow is what protects the first render.
let snapshot = SessionWindowSnapshot(
frame: nil,
display: nil,
tabManager: SessionTabManagerSnapshot(selectedWorkspaceIndex: nil, workspaces: []),
sidebar: SessionSidebarSnapshot(isVisible: false, selection: .tabs, width: nil)
)
let windowId = appDelegate.createMainWindow(sessionWindowSnapshot: snapshot)
defer { closeWindow(withId: windowId) }

guard let manager = appDelegate.tabManagerFor(windowId: windowId) else {
XCTFail("Expected tab manager for created window")
return
}

XCTAssertEqual(appDelegate.sidebarVisibility(windowId: windowId), false)

guard let initialWorkspace = manager.selectedWorkspace else {
XCTFail("Expected selected workspace in fresh window")
return
}

// No RunLoop spin before reading the inset — the seed must be applied by the
// time createMainWindow returns, not lazily after onAppear runs.
XCTAssertEqual(
initialWorkspace.bonsplitController.configuration.appearance.tabBarLeadingInset,
80,
accuracy: 0.5,
"New minimal-mode windows with collapsed sidebar should reserve traffic-light space on the initial workspace before first render"
)
}

func testAttachUpdateAccessoryRemovesTitlebarAccessoryWhenMinimalModeEnabled() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
Expand Down
Loading