Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
48 changes: 43 additions & 5 deletions Sources/App/SettingsWindowFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import os
@MainActor
enum SettingsWindowFactory {
private nonisolated static let log = Logger(subsystem: "com.cmuxterm.app", category: "Settings")
static let toolbarIdentifier = NSToolbar.Identifier("cmux.settings.toolbar")

/// `onContentAppear` is invoked from the hosted content's `onAppear`, so
/// the presenter that owns this window learns when the content's
Expand All @@ -29,15 +30,39 @@ enum SettingsWindowFactory {
let hostingController = NSHostingController(
rootView: SettingsWindowHostRoot(onContentAppear: onContentAppear)
)
// Bridge SwiftUI's navigation title, the sidebar toggle, and the
// search field into the AppKit window's titlebar/toolbar.
hostingController.sceneBridgingOptions = [.toolbars, .title]
// The AppKit owner configures the toolbar explicitly below; bridge only
// SwiftUI's navigation title so an implicit scene toolbar can never
// replace or remove the Settings chrome contract.
hostingController.sceneBridgingOptions = [.title]
let window = SettingsHostWindow(contentViewController: hostingController)
window.styleMask = [.titled, .closable, .miniaturizable, .resizable]
window.styleMask = [.titled, .closable, .miniaturizable, .resizable, .fullSizeContentView]
window.title = String(localized: "settings.title", defaultValue: "Settings")
configureChrome(on: window)
window.setContentSize(NSSize(width: 980, height: 680))
return window
}

/// Establishes the complete modern Settings chrome invariant at
/// construction time. The AppKit-owned window must also own its toolbar;
/// `sceneBridgingOptions` only forwards toolbar content explicitly declared
/// by a hosted SwiftUI view and otherwise leaves a bare legacy titlebar.
private static func configureChrome(on window: SettingsHostWindow) {
window.titlebarAppearsTransparent = true
window.titleVisibility = .hidden
window.titlebarSeparatorStyle = .none

let toolbar = NSToolbar(identifier: toolbarIdentifier)
toolbar.allowsUserCustomization = false
toolbar.autosavesConfiguration = false
toolbar.displayMode = .iconOnly
toolbar.insertItem(withItemIdentifier: .toggleSidebar, at: 0)
Comment thread
austinywang marked this conversation as resolved.
Resolved
if let sidebarToggle = toolbar.items.first(where: { $0.itemIdentifier == .toggleSidebar }) {
sidebarToggle.target = window
sidebarToggle.action = #selector(SettingsHostWindow.toggleSidebar(_:))
}
window.toolbar = toolbar
window.toolbarStyle = .unifiedCompact
}
}

extension SettingsWindowPresenter {
Expand All @@ -49,9 +74,16 @@ extension SettingsWindowPresenter {
/// main actor and warn under strict concurrency.
static func handleSidebarToggleIfSettingsWindowIsKey(keyWindow: NSWindow?) -> Bool {
guard keyWindow?.identifier?.rawValue == windowIdentifier else { return false }
NotificationCenter.default.post(name: SettingsWindowRoot.sidebarToggleRequestName, object: nil)
requestSidebarToggle()
return true
}

/// Shared mutation path for the toolbar item and the app's Toggle Left
/// Sidebar command. The SwiftUI split view owns the visibility state and
/// consumes this request in ``SettingsWindowRoot``.
static func requestSidebarToggle() {
NotificationCenter.default.post(name: SettingsWindowRoot.sidebarToggleRequestName, object: nil)
}
}

/// Settings window class that records the moment close teardown begins, so
Expand All @@ -62,6 +94,12 @@ extension SettingsWindowPresenter {
class SettingsHostWindow: NSWindow {
private(set) var isClosingSettingsWindow = false

/// Receives the standard AppKit toolbar item's `toggleSidebar:` action and
/// forwards it through the same route as the app menu command.
@objc func toggleSidebar(_ sender: Any?) {
SettingsWindowPresenter.requestSidebarToggle()
}

override func close() {
isClosingSettingsWindow = true
super.close()
Expand Down
4 changes: 4 additions & 0 deletions cmux.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources
D10000000000000000A00016 /* SettingsSidebarBetaBehaviorUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000A00017 /* SettingsSidebarBetaBehaviorUITests.swift */; };
D10000000000000000A00014 /* SettingsTerminalBehaviorUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000A00015 /* SettingsTerminalBehaviorUITests.swift */; };
D10000000000000000A00010 /* SettingsUITestSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000A00011 /* SettingsUITestSupport.swift */; };
D80100010000000000000001 /* SettingsWindowChromeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D80100010000000000000002 /* SettingsWindowChromeTests.swift */; };
D36090030000000000000001 /* SettingsWindowFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = D36090030000000000000002 /* SettingsWindowFactory.swift */; };
D37777030000000000000001 /* SettingsWindowGeometry.swift in Sources */ = {isa = PBXBuildFile; fileRef = D37777030000000000000002 /* SettingsWindowGeometry.swift */; };
D37777070000000000000001 /* SettingsWindowNavigationDelivery.swift in Sources */ = {isa = PBXBuildFile; fileRef = D37777070000000000000002 /* SettingsWindowNavigationDelivery.swift */; };
Expand Down Expand Up @@ -3058,6 +3059,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
D10000000000000000A00017 /* SettingsSidebarBetaBehaviorUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsSidebarBetaBehaviorUITests.swift; sourceTree = "<group>"; };
D10000000000000000A00015 /* SettingsTerminalBehaviorUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsTerminalBehaviorUITests.swift; sourceTree = "<group>"; };
D10000000000000000A00011 /* SettingsUITestSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsUITestSupport.swift; sourceTree = "<group>"; };
D80100010000000000000002 /* SettingsWindowChromeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsWindowChromeTests.swift; sourceTree = "<group>"; };
D36090030000000000000002 /* SettingsWindowFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = App/SettingsWindowFactory.swift; sourceTree = "<group>"; };
D37777030000000000000002 /* SettingsWindowGeometry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = App/SettingsWindowGeometry.swift; sourceTree = "<group>"; };
D37777070000000000000002 /* SettingsWindowNavigationDelivery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = App/SettingsWindowNavigationDelivery.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5120,6 +5122,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
D37777050000000000000002 /* AppDelegatePresentPreferencesWindowTests.swift */,
D37777020000000000000002 /* SettingsWindowNavigationRoutingTests.swift */,
D37777060000000000000002 /* SettingsWindowPresentationContractTests.swift */,
D80100010000000000000002 /* SettingsWindowChromeTests.swift */,
D36090010000000000000004 /* SettingsWindowPresenterTests.swift */,
D36090010000000000000006 /* CmuxMainWindowConstrainFrameTests.swift */,
D36090020000000000000006 /* CmuxMainWindowFullScreenCapabilityTests.swift */,
Expand Down Expand Up @@ -7461,6 +7464,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
F5000000A1B2C3D4E5F60718 /* SessionPersistenceTests.swift in Sources */,
583A675AA1224E8D82A44883 /* SetAutoTitleSocketTests.swift in Sources */,
A50019B2 /* SettingsSearchIndexTests.swift in Sources */,
D80100010000000000000001 /* SettingsWindowChromeTests.swift in Sources */,
D37777020000000000000001 /* SettingsWindowNavigationRoutingTests.swift in Sources */,
D37777010000000000000001 /* SettingsWindowOpenRegressionTests.swift in Sources */,
D37777060000000000000001 /* SettingsWindowPresentationContractTests.swift in Sources */,
Expand Down
50 changes: 50 additions & 0 deletions cmuxTests/SettingsWindowChromeTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import AppKit
import Testing

#if canImport(cmux_DEV)
@testable import cmux_DEV
#elseif canImport(cmux)
@testable import cmux
#endif

#if DEBUG
extension SettingsWindowSharedStateSuites {
/// Window-construction coverage for the modern Settings chrome contract
/// (https://github.com/manaflow-ai/cmux/issues/8010).
@MainActor
@Suite(.serialized)
struct SettingsWindowChromeTests {
@Test func factoryBuildsModernUnifiedChromeWithWorkingSidebarToggle() throws {
let window = SettingsWindowFactory.makeSettingsWindow(onContentAppear: {})
defer {
window.contentViewController = nil
window.contentView = nil
window.close()
}

#expect(window.styleMask.contains(.fullSizeContentView))
#expect(window.toolbarStyle == .unifiedCompact)
#expect(window.titlebarAppearsTransparent)
#expect(window.titleVisibility == .hidden)
#expect(window.titlebarSeparatorStyle == .none)

let toolbar = try #require(window.toolbar)
#expect(!toolbar.allowsUserCustomization)
#expect(!toolbar.autosavesConfiguration)
#expect(toolbar.displayMode == .iconOnly)

let sidebarToggle = try #require(
toolbar.items.first { $0.itemIdentifier == .toggleSidebar }
)
#expect(sidebarToggle.target === window)
#expect(sidebarToggle.action == #selector(NSSplitViewController.toggleSidebar(_:)))

let recorder = SettingsSidebarToggleRecorder()
defer { recorder.stopObserving() }
let action = try #require(sidebarToggle.action)
#expect(NSApp.sendAction(action, to: sidebarToggle.target, from: sidebarToggle))
#expect(recorder.receivedCount == 1)
}
}
}
#endif
2 changes: 1 addition & 1 deletion cmuxTests/SettingsWindowNavigationRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ private final class SettingsNavigationTargetRecorder: NSObject {
/// `SettingsWindowRoot.sidebarToggleRequestName`) so the test target does not
/// depend on package-symbol visibility, which differs across toolchains.
@MainActor
private final class SettingsSidebarToggleRecorder: NSObject {
final class SettingsSidebarToggleRecorder: NSObject {
private(set) var receivedCount = 0

override init() {
Expand Down
Loading