-
-
Notifications
You must be signed in to change notification settings - Fork 995
Add cleaner titlebar visibility options #1327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
b77d7c2
ca6f83f
cdcfa19
c8d561c
7385a01
d5cf95a
6b281dc
3e75be2
cf3a132
6244e49
d7d6c60
945881b
cc51646
dc394ad
9ecefc5
239ec1d
a82fa63
18d2d5f
5cd4b30
67f2bda
afd2d21
278a2ff
5c42da4
f1e0abe
8d96c97
ae0a4e3
b61698c
87ee59f
39d8cc2
3a41872
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1931,8 +1931,7 @@ struct ContentView: View { | |
| .frame(width: sidebarWidth) | ||
| } | ||
|
|
||
| /// 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. | ||
| /// Space at top of content area reserved for the custom workspace titlebar. | ||
| @State private var titlebarPadding: CGFloat = 32 | ||
|
|
||
| private var terminalContent: some View { | ||
|
|
@@ -1941,56 +1940,59 @@ struct ContentView: View { | |
| let selectedWorkspaceId = tabManager.selectedTabId | ||
| let retiringWorkspaceId = self.retiringWorkspaceId | ||
|
|
||
| return ZStack { | ||
| return ZStack(alignment: .top) { | ||
| ZStack { | ||
| ForEach(mountedWorkspaces) { tab in | ||
| let isSelectedWorkspace = selectedWorkspaceId == tab.id | ||
| let isRetiringWorkspace = retiringWorkspaceId == tab.id | ||
| let shouldPrimeInBackground = tabManager.pendingBackgroundWorkspaceLoadIds.contains(tab.id) | ||
| // Keep the retiring workspace visible during handoff, but never input-active. | ||
| // Allowing both selected+retiring workspaces to be input-active lets the | ||
| // old workspace steal first responder (notably with WKWebView), which can | ||
| // delay handoff completion and make browser returns feel laggy. | ||
| let isInputActive = isSelectedWorkspace | ||
| let isVisible = isSelectedWorkspace || isRetiringWorkspace | ||
| let portalPriority = isSelectedWorkspace ? 2 : (isRetiringWorkspace ? 1 : 0) | ||
| WorkspaceContentView( | ||
| workspace: tab, | ||
| isWorkspaceVisible: isVisible, | ||
| isWorkspaceInputActive: isInputActive, | ||
| workspacePortalPriority: portalPriority, | ||
| onThemeRefreshRequest: { reason, eventId, source, payloadHex in | ||
| scheduleTitlebarThemeRefreshFromWorkspace( | ||
| workspaceId: tab.id, | ||
| reason: reason, | ||
| backgroundEventId: eventId, | ||
| backgroundSource: source, | ||
| notificationPayloadHex: payloadHex | ||
| ) | ||
| ZStack { | ||
| ForEach(mountedWorkspaces) { tab in | ||
| let isSelectedWorkspace = selectedWorkspaceId == tab.id | ||
| let isRetiringWorkspace = retiringWorkspaceId == tab.id | ||
| let shouldPrimeInBackground = tabManager.pendingBackgroundWorkspaceLoadIds.contains(tab.id) | ||
| // Keep the retiring workspace visible during handoff, but never input-active. | ||
| // Allowing both selected+retiring workspaces to be input-active lets the | ||
| // old workspace steal first responder (notably with WKWebView), which can | ||
| // delay handoff completion and make browser returns feel laggy. | ||
| let isInputActive = isSelectedWorkspace | ||
| let isVisible = isSelectedWorkspace || isRetiringWorkspace | ||
| let portalPriority = isSelectedWorkspace ? 2 : (isRetiringWorkspace ? 1 : 0) | ||
| WorkspaceContentView( | ||
| workspace: tab, | ||
| isWorkspaceVisible: isVisible, | ||
| isWorkspaceInputActive: isInputActive, | ||
| workspacePortalPriority: portalPriority, | ||
| onThemeRefreshRequest: { reason, eventId, source, payloadHex in | ||
| scheduleTitlebarThemeRefreshFromWorkspace( | ||
| workspaceId: tab.id, | ||
| reason: reason, | ||
| backgroundEventId: eventId, | ||
| backgroundSource: source, | ||
| notificationPayloadHex: payloadHex | ||
| ) | ||
| } | ||
| ) | ||
| .opacity(isVisible ? 1 : 0) | ||
| .allowsHitTesting(isSelectedWorkspace) | ||
| .accessibilityHidden(!isVisible) | ||
| .zIndex(isSelectedWorkspace ? 2 : (isRetiringWorkspace ? 1 : 0)) | ||
| .task(id: shouldPrimeInBackground ? tab.id : nil) { | ||
| await primeBackgroundWorkspaceIfNeeded(workspaceId: tab.id) | ||
| } | ||
| ) | ||
| .opacity(isVisible ? 1 : 0) | ||
| .allowsHitTesting(isSelectedWorkspace) | ||
| .accessibilityHidden(!isVisible) | ||
| .zIndex(isSelectedWorkspace ? 2 : (isRetiringWorkspace ? 1 : 0)) | ||
| .task(id: shouldPrimeInBackground ? tab.id : nil) { | ||
| await primeBackgroundWorkspaceIfNeeded(workspaceId: tab.id) | ||
| } | ||
| } | ||
| .opacity(sidebarSelectionState.selection == .tabs ? 1 : 0) | ||
| .allowsHitTesting(sidebarSelectionState.selection == .tabs) | ||
| .accessibilityHidden(sidebarSelectionState.selection != .tabs) | ||
|
|
||
| NotificationsPage(selection: $sidebarSelectionState.selection) | ||
| .opacity(sidebarSelectionState.selection == .notifications ? 1 : 0) | ||
| .allowsHitTesting(sidebarSelectionState.selection == .notifications) | ||
| .accessibilityHidden(sidebarSelectionState.selection != .notifications) | ||
|
Comment on lines
+1956
to
+2001
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gate
Suggested fix- return ZStack(alignment: .top) {
+ let showsWorkspacePage = sidebarSelectionState.selection == .tabs
+ return ZStack(alignment: .top) {
ZStack {
ZStack {
ForEach(mountedWorkspaces) { tab in
let isSelectedWorkspace = selectedWorkspaceId == tab.id
let isRetiringWorkspace = retiringWorkspaceId == tab.id
let shouldPrimeInBackground = tabManager.pendingBackgroundWorkspaceLoadIds.contains(tab.id)
@@
- let isInputActive = isSelectedWorkspace
- let isVisible = isSelectedWorkspace || isRetiringWorkspace
- let portalPriority = isSelectedWorkspace ? 2 : (isRetiringWorkspace ? 1 : 0)
+ let isInputActive = showsWorkspacePage && isSelectedWorkspace
+ let isVisible = showsWorkspacePage && (isSelectedWorkspace || isRetiringWorkspace)
+ let portalPriority = isVisible ? (isSelectedWorkspace ? 2 : 1) : 0
@@
- .opacity(sidebarSelectionState.selection == .tabs ? 1 : 0)
- .allowsHitTesting(sidebarSelectionState.selection == .tabs)
- .accessibilityHidden(sidebarSelectionState.selection != .tabs)
+ .opacity(showsWorkspacePage ? 1 : 0)
+ .allowsHitTesting(showsWorkspacePage)
+ .accessibilityHidden(!showsWorkspacePage)Based on learnings, 🤖 Prompt for AI Agents |
||
| } | ||
| .opacity(sidebarSelectionState.selection == .tabs ? 1 : 0) | ||
| .allowsHitTesting(sidebarSelectionState.selection == .tabs) | ||
| .accessibilityHidden(sidebarSelectionState.selection != .tabs) | ||
| .padding(.top, titlebarPadding) | ||
|
|
||
| NotificationsPage(selection: $sidebarSelectionState.selection) | ||
| .opacity(sidebarSelectionState.selection == .notifications ? 1 : 0) | ||
| .allowsHitTesting(sidebarSelectionState.selection == .notifications) | ||
| .accessibilityHidden(sidebarSelectionState.selection != .notifications) | ||
| } | ||
| .padding(.top, titlebarPadding) | ||
| .overlay(alignment: .top) { | ||
| // Titlebar overlay is only over terminal content, not the sidebar. | ||
| customTitlebar | ||
| if showWorkspaceTitlebar { | ||
cubic-dev-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Titlebar overlay is only over terminal content, not the sidebar. | ||
| customTitlebar | ||
|
Comment on lines
+2005
to
+2007
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When this branch omits Useful? React with 👍 / 👎. |
||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
|
|
@@ -2007,6 +2009,8 @@ struct ContentView: View { | |
| @AppStorage("bgGlassTintHex") private var bgGlassTintHex = "#000000" | ||
| @AppStorage("bgGlassTintOpacity") private var bgGlassTintOpacity = 0.03 | ||
| @AppStorage("bgGlassEnabled") private var bgGlassEnabled = false | ||
| @AppStorage(WorkspaceTitlebarSettings.showTitlebarKey) | ||
| private var showWorkspaceTitlebar = WorkspaceTitlebarSettings.defaultShowTitlebar | ||
| @AppStorage("debugTitlebarLeadingExtra") private var debugTitlebarLeadingExtra: Double = 0 | ||
|
|
||
| @State private var titlebarLeadingInset: CGFloat = 12 | ||
|
|
@@ -2623,7 +2627,7 @@ struct ContentView: View { | |
| removeSidebarResizerPointerMonitor() | ||
| }) | ||
|
|
||
| view = AnyView(view.background(WindowAccessor { [sidebarBlendMode, bgGlassEnabled, bgGlassTintHex, bgGlassTintOpacity] window in | ||
| view = AnyView(view.background(WindowAccessor { [sidebarBlendMode, bgGlassEnabled, bgGlassTintHex, bgGlassTintOpacity, showWorkspaceTitlebar] window in | ||
| window.identifier = NSUserInterfaceItemIdentifier(windowIdentifier) | ||
| window.titlebarAppearsTransparent = true | ||
| // Do not make the entire background draggable; it interferes with drag gestures | ||
|
|
@@ -2646,10 +2650,11 @@ struct ContentView: View { | |
| } | ||
| } | ||
|
|
||
| // Keep content below the titlebar so drags on Bonsplit's tab bar don't | ||
| // get interpreted as window drags. | ||
| // Reserve space for the custom titlebar only when it is enabled. When hidden, the | ||
| // top Bonsplit tab bar moves into this strip and its empty space gets an explicit | ||
| // drag handle overlay instead. | ||
| let computedTitlebarHeight = window.frame.height - window.contentLayoutRect.height | ||
| let nextPadding = max(28, min(72, computedTitlebarHeight)) | ||
| let nextPadding = showWorkspaceTitlebar ? max(28, min(72, computedTitlebarHeight)) : 0 | ||
| if abs(titlebarPadding - nextPadding) > 0.5 { | ||
|
Comment on lines
2669
to
2671
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎. |
||
| DispatchQueue.main.async { | ||
| titlebarPadding = nextPadding | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 8792
🏁 Script executed:
rg -n "enum ChromeControlsVisibilityMode" -A 20 --type swiftRepository: manaflow-ai/cmux
Length of output: 1632
🏁 Script executed:
rg -n "extension ChromeControlsVisibilityMode" -A 15 --type swiftRepository: manaflow-ai/cmux
Length of output: 1056
Add separate mode display name keys for
paneTabBarControlsor explicitly document intentional key sharing.The
paneTabBarControlspicker usesChromeControlsVisibilityMode.displayName, which hardcodes localization keys fromsettings.app.titlebarControls.alwaysandsettings.app.titlebarControls.hover(lines 3047-3049 in cmuxApp.swift). While both controls share the same enum, this creates implicit coupling.Consider either:
settings.app.paneTabBarControls.alwaysandsettings.app.paneTabBarControls.hoverkeys to the localization file for independent control, ordisplayNameimplementation to make the key sharing explicit and documented in code.Currently, changes to titlebar mode labels would unintentionally affect pane tab bar mode labels.
🤖 Prompt for AI Agents