Honor Ghostty background-opacity across all cmux chrome#667
Merged
lawrencecchen merged 2 commits intomainfrom Mar 1, 2026
Merged
Honor Ghostty background-opacity across all cmux chrome#667lawrencecchen merged 2 commits intomainfrom
lawrencecchen merged 2 commits intomainfrom
Conversation
Parse background-opacity from Ghostty config and propagate it through the entire chrome pipeline: bonsplit tab bar (via RRGGBBAA hex), browser panel/omnibar, titlebar, empty panel, and window background. Decouple glass effect from sidebar blend mode — bgGlassEnabled now defaults to false so opacity works independently. Add GhosttyBackgroundTheme helper for consistent color+opacity resolution across all UI surfaces. Fixes #263
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds background-opacity support and centralizes Ghostty background color handling. Introduces GhosttyBackgroundTheme, opacity-aware chrome color generation, new titlebar layer view, translucency decision flow (clear vs. glass fallback), view-hierarchy transparency helper, and corresponding tests and config parsing. Changes
sequenceDiagram
participant Config as GhosttyConfig
participant App as Workspace/Resolve
participant Theme as GhosttyBackgroundTheme
participant Content as ContentView
participant Window as NSWindow
Config->>App: parse background-opacity
App->>Theme: request currentColor(opacity)
Theme->>App: color with clamped alpha
App->>Content: apply chrome/background (hex with opacity)
Content->>Content: evaluate shouldForceTransparentHosting / shouldApplyWindowGlassFallback
alt Force transparent hosting
Content->>Window: set isOpaque = false, background = near-clear
Content->>Content: makeViewHierarchyTransparent(rootView)
else Apply glass fallback / opaque
Content->>Window: apply glass layer or themed opaque background
end
Window->>Window: render with opacity-aware layers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/cmuxApp.swift (1)
2376-2423:⚠️ Potential issue | 🟡 MinorReset action now conflicts with the new
bgGlassEnableddefault.Line 2376 defaults
bgGlassEnabledtofalse, but the reset path still forces it totrue(Line 2422). This makes “Reset” re-enable glass unexpectedly.Proposed fix
Button("Reset") { bgGlassTintHex = "#000000" bgGlassTintOpacity = 0.03 bgGlassMaterial = "hudWindow" - bgGlassEnabled = true + bgGlassEnabled = false updateWindowGlassTint() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/cmuxApp.swift` around lines 2376 - 2423, The Reset button currently forces bgGlassEnabled = true while the `@AppStorage` declaration sets bgGlassEnabled default to false, causing Reset to unexpectedly re-enable glass; update the Reset action in the Button closure (where bgGlassTintHex, bgGlassTintOpacity, bgGlassMaterial, bgGlassEnabled, and updateWindowGlassTint() are set) so bgGlassEnabled is reset to the same default (false) or, better, read a single source-of-truth default constant and assign that value instead of hardcoding true.Sources/ContentView.swift (1)
2434-2456:⚠️ Potential issue | 🟠 MajorAdd a true “disable” path for translucency/glass state.
Line 2441 only applies the “on” state. When opacity returns to 1.0 or glass is disabled, window state is not restored, and the view hierarchy from Line 2520 remains force-cleared. This can leave stale translucency and break the expected “opacity=1.0 matches prior behavior” flow.
🔧 Suggested fix
- if shouldForceTransparentHosting { + if shouldForceTransparentHosting { window.isOpaque = false // Keep the window clear whenever translucency is active. Relying only on // terminal focus-driven updates can leave stale opaque window fills. window.backgroundColor = NSColor.white.withAlphaComponent(0.001) // Configure contentView hierarchy for transparency. if let contentView = window.contentView { makeViewHierarchyTransparent(contentView) } + } else { + window.isOpaque = true + window.backgroundColor = .windowBackgroundColor + // Also restore content hierarchy backgrounds/opacity here. } if shouldApplyWindowGlassFallback { // Apply liquid glass effect to the window with tint from settings let tintColor = (NSColor(hex: bgGlassTintHex) ?? .black).withAlphaComponent(bgGlassTintOpacity) WindowGlassEffect.apply(to: window, tintColor: tintColor) + } else { + WindowGlassEffect.remove(from: window) }Also,
WindowGlassEffect.remove(from:)should actually detach fallback views, not only clear associated-object references.Also applies to: 2520-2528
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 2434 - 2456, The code only sets up translucency when shouldForceTransparentHosting is true but never restores the prior state when opacity returns to 1.0 or glass is disabled; update the branch logic so when shouldForceTransparentHosting is false you revert changes: set window.isOpaque = true, restore a non-transparent backgroundColor (e.g., remove the near-clear alpha or reset to the prior window background), and call a new or existing inverse of makeViewHierarchyTransparent (e.g., makeViewHierarchyOpaque or remove transparency on window.contentView) to restore the view hierarchy; also ensure WindowGlassEffect.remove(from:) truly detaches any fallback/translucency views from the window (not just clearing associated-object refs) and call WindowGlassEffect.remove(from: window) when shouldApplyWindowGlassFallback becomes false so fallback views are removed.
🧹 Nitpick comments (6)
Sources/Panels/BrowserPanel.swift (1)
7-52: Consider removing now-duplicated background resolver logic fromBrowserPanel.With
GhosttyBackgroundThemeintroduced on Line 7, the olderBrowserPanelprivate helpers (clampedGhosttyBackgroundOpacity,resolvedGhosttyBackgroundColor,resolvedBrowserChromeBackgroundColor) are overlapping paths and can drift. Prefer a single resolver path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanel.swift` around lines 7 - 52, The BrowserPanel still contains duplicate background-resolver helpers (clampedGhosttyBackgroundOpacity, resolvedGhosttyBackgroundColor, resolvedBrowserChromeBackgroundColor) that overlap with the new GhosttyBackgroundTheme utilities; remove those private helper methods from BrowserPanel and update any usages to call GhosttyBackgroundTheme.color(from:), GhosttyBackgroundTheme.color(backgroundColor:opacity:), or GhosttyBackgroundTheme.currentColor() as appropriate so there is a single canonical resolver path (search for the helper names in BrowserPanel and replace their callers with the matching GhosttyBackgroundTheme API).Sources/GhosttyTerminalView.swift (1)
2377-2393: Cache the clear-window decision once per call for readability.The same predicate is evaluated multiple times in one method; storing it once improves clarity and keeps logging/state branches tightly coupled.
♻️ Proposed cleanup
applySurfaceBackground() let color = effectiveBackgroundColor() - if cmuxShouldUseClearWindowBackground(for: color.alphaComponent) { + let useClearWindowBackground = cmuxShouldUseClearWindowBackground(for: color.alphaComponent) + if useClearWindowBackground { window.backgroundColor = cmuxTransparentWindowBaseColor() window.isOpaque = false } else { window.backgroundColor = color window.isOpaque = color.alphaComponent >= 1.0 } if GhosttyApp.shared.backgroundLogEnabled { - let signature = "\(cmuxShouldUseClearWindowBackground(for: color.alphaComponent) ? "transparent" : color.hexString()):\(String(format: "%.3f", color.alphaComponent))" + let signature = "\(useClearWindowBackground ? "transparent" : color.hexString()):\(String(format: "%.3f", color.alphaComponent))" if signature != lastLoggedWindowBackgroundSignature { lastLoggedWindowBackgroundSignature = signature @@ GhosttyApp.shared.logBackground( - "window background applied tab=\(tabId?.uuidString ?? "unknown") surface=\(terminalSurface?.id.uuidString ?? "unknown") source=\(source) override=\(overrideHex) default=\(defaultHex) transparent=\(cmuxShouldUseClearWindowBackground(for: color.alphaComponent)) color=\(color.hexString()) opacity=\(String(format: "%.3f", color.alphaComponent))" + "window background applied tab=\(tabId?.uuidString ?? "unknown") surface=\(terminalSurface?.id.uuidString ?? "unknown") source=\(source) override=\(overrideHex) default=\(defaultHex) transparent=\(useClearWindowBackground) color=\(color.hexString()) opacity=\(String(format: "%.3f", color.alphaComponent))" ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 2377 - 2393, Cache the result of cmuxShouldUseClearWindowBackground(for: color.alphaComponent) in a local Bool (e.g., let useClearBackground = cmuxShouldUseClearWindowBackground(for: color.alphaComponent)) at the start of the block and use that variable everywhere instead of calling the predicate repeatedly: use it to choose window.backgroundColor and window.isOpaque, to build the signature checked/assigned to lastLoggedWindowBackgroundSignature, and to populate the "transparent=" field in the GhosttyApp.shared.logBackground call; keep all other logic (backgroundColor override, hex strings, opacity formatting, and lastLoggedWindowBackgroundSignature comparison) unchanged.cmuxTests/CmuxWebViewKeyEquivalentTests.swift (1)
1306-1329: Consolidate overlapping BrowserPanel opacity tests.Line 1306 adds valuable
NSNumberpayload coverage, but the assertion body is almost identical to the existing test around Line 1249. Consider parameterizing the payload type (DoublevsNSNumber) through a shared helper to reduce duplicate maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift` around lines 1306 - 1329, The new test testBrowserPanelRefreshesUnderPageBackgroundColorWithGhosttyOpacity duplicates assertion logic from the existing similar test; refactor by extracting a shared helper that accepts the notification payload (allowing both Double and NSNumber for GhosttyNotificationKey.backgroundOpacity) and performs the NotificationCenter.post + underPageBackgroundColor assertions for BrowserPanel; reuse this helper from both testBrowserPanelRefreshesUnderPageBackgroundColorWithGhosttyOpacity and the earlier test to remove duplicated assertion code while keeping coverage for .ghosttyDefaultBackgroundDidChange and GhosttyNotificationKey keys.Sources/WorkspaceContentView.swift (1)
407-407:EmptyPanelViewmay not update when background opacity changes.Unlike
BrowserPanelViewwhich has its own.onReceivefor.ghosttyDefaultBackgroundDidChange,EmptyPanelViewcomputes the background inline without any reactive update mechanism. Since it's rendered inside BonsplitView's content closure, parent state changes inWorkspaceContentViewmay not reliably trigger re-evaluation of these closures.If a user changes
background-opacityin Ghostty config while an empty panel is visible, it might not update until the view is recreated.Consider adding a similar pattern to
EmptyPanelView:♻️ Add reactive background update to EmptyPanelView
struct EmptyPanelView: View { `@ObservedObject` var workspace: Workspace let paneId: PaneID `@AppStorage`(KeyboardShortcutSettings.Action.newSurface.defaultsKey) private var newSurfaceShortcutData = Data() `@AppStorage`(KeyboardShortcutSettings.Action.openBrowser.defaultsKey) private var openBrowserShortcutData = Data() + `@State` private var ghosttyBackgroundGeneration: Int = 0 + + private var backgroundColor: Color { + _ = ghosttyBackgroundGeneration + return Color(nsColor: GhosttyBackgroundTheme.currentColor()) + } // ... existing code ... var body: some View { VStack(spacing: 16) { // ... existing content ... } .frame(maxWidth: .infinity, maxHeight: .infinity) - .background(Color(nsColor: GhosttyBackgroundTheme.currentColor())) + .background(backgroundColor) + .onReceive(NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)) { _ in + ghosttyBackgroundGeneration &+= 1 + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/WorkspaceContentView.swift` at line 407, EmptyPanelView's inline background using GhosttyBackgroundTheme.currentColor() won't react to background-opacity changes; update EmptyPanelView in WorkspaceContentView to subscribe to Ghostty's background-change notifications (the same publisher used by BrowserPanelView, e.g. NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)) and call a view update (e.g. store a `@State/`@Published color or opacity and set it in the onReceive) so the .background(Color(nsColor: GhosttyBackgroundTheme.currentColor())) modifier is re-evaluated when the notification fires; locate EmptyPanelView and add an onReceive that updates a local state backing the background color/opacity.Sources/Panels/BrowserPanelView.swift (1)
740-742: Consider: WebView placeholder background does not apply opacity.Line 741 uses
browserChromeBackgroundColor(without opacity) while the address bar now usesbrowserChromeBackground(with opacity). WhenshouldRenderWebViewis false, the placeholder area will be opaque while the address bar above is translucent, creating a visual inconsistency.If this is intentional (e.g., placeholder should remain opaque), a brief comment would clarify. Otherwise, consider using
browserChromeBackgroundhere as well.💡 Optional: Use opacity-aware color for consistency
} else { - Color(nsColor: browserChromeBackgroundColor) + browserChromeBackground .contentShape(Rectangle())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanelView.swift` around lines 740 - 742, When shouldRenderWebView is false the placeholder uses browserChromeBackgroundColor (opaque) while the address bar uses the opacity-aware browserChromeBackground, creating a visual mismatch; update the placeholder in BrowserPanelView (the branch that currently creates Color(nsColor: browserChromeBackgroundColor)) to use browserChromeBackground instead so the same opacity is applied, or if opacity difference is intentional add a brief comment explaining why the opaque browserChromeBackgroundColor is required.Sources/Workspace.swift (1)
1072-1078: Consider de-emphasizing the alpha-derived overload to avoid future opacity regressions.This overload is convenient, but deriving opacity from
backgroundColor.alphaComponentcan silently ignore external opacity sources if reused casually. Consider marking it as transitional/deprecated or adding a doc warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 1072 - 1078, The shorthand overload applyGhosttyChrome(backgroundColor: NSColor, reason: String = "unspecified") should be de-emphasized to avoid accidental opacity regressions: either annotate it as deprecated (e.g. with `@available`(*, deprecated, message: "...")) pointing callers to use applyGhosttyChrome(backgroundColor:backgroundOpacity:reason:) or add a clear doc comment above the overload warning that alphaComponent-derived opacity may be incorrect and callers should prefer the explicit backgroundOpacity parameter; update any internal call sites to use the explicit overload and run tests to catch regressions.
🤖 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/GhosttyConfig.swift`:
- Around line 152-155: The "background-opacity" parsing currently assigns any
Double(value) to backgroundOpacity; change this to validate the parsed Double
for finiteness and clamp it into the [0.0, 1.0] range before assigning. In the
switch case handling "background-opacity" (where you currently call
Double(value) and set backgroundOpacity), parse to a Double, ensure it is finite
(not NaN or infinite), then clamp with min/max (or equivalent) to 0.0/1.0; if
the value is non-finite or cannot be parsed, leave backgroundOpacity unchanged
(or handle via existing error path) so invalid values do not propagate.
In `@Sources/Workspace.swift`:
- Around line 1012-1013: The code currently treats alpha values >= 0.999 as
opaque; change the logic so any alpha less than 1.0 is preserved — update the
includeAlpha computation (themedColor.alphaComponent < 0.999) to use a strict
comparison against 1.0 (e.g., includeAlpha = themedColor.alphaComponent < 1.0 or
an equivalent epsilon-aware check) so themedColor.alphaComponent is passed
correctly to hexString(includeAlpha:) and near‑opaque configured opacities are
not dropped.
In `@vendor/bonsplit`:
- Line 1: The parent repo's submodule pointer for bonsplit points to commit
335facd9fd1d81a3c71fea69345af30f7e3601f9 which is not present on the bonsplit
remote main; push that commit (or fast-forward main to include it) to the
bonsplit remote main branch, verify the commit exists on remote, then update and
commit the submodule pointer in the parent repository (the bonsplit submodule
entry) so it references the now-pushed commit.
---
Outside diff comments:
In `@Sources/cmuxApp.swift`:
- Around line 2376-2423: The Reset button currently forces bgGlassEnabled = true
while the `@AppStorage` declaration sets bgGlassEnabled default to false, causing
Reset to unexpectedly re-enable glass; update the Reset action in the Button
closure (where bgGlassTintHex, bgGlassTintOpacity, bgGlassMaterial,
bgGlassEnabled, and updateWindowGlassTint() are set) so bgGlassEnabled is reset
to the same default (false) or, better, read a single source-of-truth default
constant and assign that value instead of hardcoding true.
In `@Sources/ContentView.swift`:
- Around line 2434-2456: The code only sets up translucency when
shouldForceTransparentHosting is true but never restores the prior state when
opacity returns to 1.0 or glass is disabled; update the branch logic so when
shouldForceTransparentHosting is false you revert changes: set window.isOpaque =
true, restore a non-transparent backgroundColor (e.g., remove the near-clear
alpha or reset to the prior window background), and call a new or existing
inverse of makeViewHierarchyTransparent (e.g., makeViewHierarchyOpaque or remove
transparency on window.contentView) to restore the view hierarchy; also ensure
WindowGlassEffect.remove(from:) truly detaches any fallback/translucency views
from the window (not just clearing associated-object refs) and call
WindowGlassEffect.remove(from: window) when shouldApplyWindowGlassFallback
becomes false so fallback views are removed.
---
Nitpick comments:
In `@cmuxTests/CmuxWebViewKeyEquivalentTests.swift`:
- Around line 1306-1329: The new test
testBrowserPanelRefreshesUnderPageBackgroundColorWithGhosttyOpacity duplicates
assertion logic from the existing similar test; refactor by extracting a shared
helper that accepts the notification payload (allowing both Double and NSNumber
for GhosttyNotificationKey.backgroundOpacity) and performs the
NotificationCenter.post + underPageBackgroundColor assertions for BrowserPanel;
reuse this helper from both
testBrowserPanelRefreshesUnderPageBackgroundColorWithGhosttyOpacity and the
earlier test to remove duplicated assertion code while keeping coverage for
.ghosttyDefaultBackgroundDidChange and GhosttyNotificationKey keys.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2377-2393: Cache the result of
cmuxShouldUseClearWindowBackground(for: color.alphaComponent) in a local Bool
(e.g., let useClearBackground = cmuxShouldUseClearWindowBackground(for:
color.alphaComponent)) at the start of the block and use that variable
everywhere instead of calling the predicate repeatedly: use it to choose
window.backgroundColor and window.isOpaque, to build the signature
checked/assigned to lastLoggedWindowBackgroundSignature, and to populate the
"transparent=" field in the GhosttyApp.shared.logBackground call; keep all other
logic (backgroundColor override, hex strings, opacity formatting, and
lastLoggedWindowBackgroundSignature comparison) unchanged.
In `@Sources/Panels/BrowserPanel.swift`:
- Around line 7-52: The BrowserPanel still contains duplicate
background-resolver helpers (clampedGhosttyBackgroundOpacity,
resolvedGhosttyBackgroundColor, resolvedBrowserChromeBackgroundColor) that
overlap with the new GhosttyBackgroundTheme utilities; remove those private
helper methods from BrowserPanel and update any usages to call
GhosttyBackgroundTheme.color(from:),
GhosttyBackgroundTheme.color(backgroundColor:opacity:), or
GhosttyBackgroundTheme.currentColor() as appropriate so there is a single
canonical resolver path (search for the helper names in BrowserPanel and replace
their callers with the matching GhosttyBackgroundTheme API).
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 740-742: When shouldRenderWebView is false the placeholder uses
browserChromeBackgroundColor (opaque) while the address bar uses the
opacity-aware browserChromeBackground, creating a visual mismatch; update the
placeholder in BrowserPanelView (the branch that currently creates
Color(nsColor: browserChromeBackgroundColor)) to use browserChromeBackground
instead so the same opacity is applied, or if opacity difference is intentional
add a brief comment explaining why the opaque browserChromeBackgroundColor is
required.
In `@Sources/Workspace.swift`:
- Around line 1072-1078: The shorthand overload
applyGhosttyChrome(backgroundColor: NSColor, reason: String = "unspecified")
should be de-emphasized to avoid accidental opacity regressions: either annotate
it as deprecated (e.g. with `@available`(*, deprecated, message: "...")) pointing
callers to use applyGhosttyChrome(backgroundColor:backgroundOpacity:reason:) or
add a clear doc comment above the overload warning that alphaComponent-derived
opacity may be incorrect and callers should prefer the explicit
backgroundOpacity parameter; update any internal call sites to use the explicit
overload and run tests to catch regressions.
In `@Sources/WorkspaceContentView.swift`:
- Line 407: EmptyPanelView's inline background using
GhosttyBackgroundTheme.currentColor() won't react to background-opacity changes;
update EmptyPanelView in WorkspaceContentView to subscribe to Ghostty's
background-change notifications (the same publisher used by BrowserPanelView,
e.g. NotificationCenter.default.publisher(for:
.ghosttyDefaultBackgroundDidChange)) and call a view update (e.g. store a
`@State/`@Published color or opacity and set it in the onReceive) so the
.background(Color(nsColor: GhosttyBackgroundTheme.currentColor())) modifier is
re-evaluated when the notification fires; locate EmptyPanelView and add an
onReceive that updates a local state backing the background color/opacity.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Sources/ContentView.swiftSources/GhosttyConfig.swiftSources/GhosttyTerminalView.swiftSources/Panels/BrowserPanel.swiftSources/Panels/BrowserPanelView.swiftSources/Workspace.swiftSources/WorkspaceContentView.swiftSources/cmuxApp.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.swiftcmuxTests/GhosttyConfigTests.swiftvendor/bonsplit
Comment on lines
+152
to
+155
| case "background-opacity": | ||
| if let opacity = Double(value) { | ||
| backgroundOpacity = opacity | ||
| } |
There was a problem hiding this comment.
Validate and clamp parsed background-opacity range.
Line 153 currently accepts any Double. Clamp to [0.0, 1.0] (and reject non-finite values) to prevent invalid opacity state from propagating.
🔧 Proposed fix
case "background-opacity":
- if let opacity = Double(value) {
- backgroundOpacity = opacity
+ if let opacity = Double(value), opacity.isFinite {
+ backgroundOpacity = min(1.0, max(0.0, opacity))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyConfig.swift` around lines 152 - 155, The "background-opacity"
parsing currently assigns any Double(value) to backgroundOpacity; change this to
validate the parsed Double for finiteness and clamp it into the [0.0, 1.0] range
before assigning. In the switch case handling "background-opacity" (where you
currently call Double(value) and set backgroundOpacity), parse to a Double,
ensure it is finite (not NaN or infinite), then clamp with min/max (or
equivalent) to 0.0/1.0; if the value is non-finite or cannot be parsed, leave
backgroundOpacity unchanged (or handle via existing error path) so invalid
values do not propagate.
Comment on lines
+1012
to
+1013
| let includeAlpha = themedColor.alphaComponent < 0.999 | ||
| return themedColor.hexString(includeAlpha: includeAlpha) |
There was a problem hiding this comment.
Don’t drop near-opaque configured opacity values.
At Line 1012, themedColor.alphaComponent < 0.999 treats values in [0.999, 1.0) as fully opaque (no alpha in hex), which can violate exact background-opacity intent.
🔧 Proposed fix
- let includeAlpha = themedColor.alphaComponent < 0.999
+ let includeAlpha = themedColor.alphaComponent < 1.0📝 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.
Suggested change
| let includeAlpha = themedColor.alphaComponent < 0.999 | |
| return themedColor.hexString(includeAlpha: includeAlpha) | |
| let includeAlpha = themedColor.alphaComponent < 1.0 | |
| return themedColor.hexString(includeAlpha: includeAlpha) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 1012 - 1013, The code currently treats
alpha values >= 0.999 as opaque; change the logic so any alpha less than 1.0 is
preserved — update the includeAlpha computation (themedColor.alphaComponent <
0.999) to use a strict comparison against 1.0 (e.g., includeAlpha =
themedColor.alphaComponent < 1.0 or an equivalent epsilon-aware check) so
themedColor.alphaComponent is passed correctly to hexString(includeAlpha:) and
near‑opaque configured opacities are not dropped.
| @@ -1 +1 @@ | |||
| Subproject commit c4b8f5cc3def0a44c1c3634d4f358a66fd956606 | |||
| Subproject commit 335facd9fd1d81a3c71fea69345af30f7e3601f9 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the bonsplit submodule commit is on the remote main branch
cd vendor/bonsplit 2>/dev/null || {
echo "Error: vendor/bonsplit directory not found"
exit 1
}
# Get current remote URL
echo "Remote URL:"
git config --get remote.origin.url
# Fetch latest from remote
echo -e "\nFetching origin/main..."
git fetch origin main 2>&1 | head -20
# Check if commit exists locally
echo -e "\nChecking if commit 335facd9fd1d81a3c71fea69345af30f7e3601f9 is known locally..."
if git cat-file -t 335facd9fd1d81a3c71fea69345af30f7e3601f9 2>/dev/null | grep -q commit; then
echo "✓ Commit exists locally"
else
echo "✗ Commit does NOT exist locally"
fi
# Check if commit is ancestor of origin/main
echo -e "\nChecking if commit is on origin/main..."
if git merge-base --is-ancestor 335facd9fd1d81a3c71fea69345af30f7e3601f9 origin/main 2>/dev/null; then
echo "✓ Commit 335facd9fd1d81a3c71fea69345af30f7e3601f9 IS on origin/main"
exit 0
else
echo "✗ Commit 335facd9fd1d81a3c71fea69345af30f7e3601f9 is NOT an ancestor of origin/main"
# Try to show what's on origin/main
echo -e "\nLatest commit on origin/main:"
git log origin/main -1 --oneline 2>/dev/null || echo "Could not retrieve"
exit 1
fiRepository: manaflow-ai/cmux
Length of output: 566
Submodule commit must be pushed to remote main before updating the parent repository pointer.
The new bonsplit commit 335facd9fd1d81a3c71fea69345af30f7e3601f9 does not exist on the remote main branch. Per the established workflow, submodule commits must be pushed to the remote main branch before updating the pointer in the parent repository. Without this, the submodule reference will be broken for other developers who clone the repository.
Action required: Push the commit to the bonsplit remote main branch, then update the parent repository pointer to that commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/bonsplit` at line 1, The parent repo's submodule pointer for bonsplit
points to commit 335facd9fd1d81a3c71fea69345af30f7e3601f9 which is not present
on the bonsplit remote main; push that commit (or fast-forward main to include
it) to the bonsplit remote main branch, verify the commit exists on remote, then
update and commit the submodule pointer in the parent repository (the bonsplit
submodule entry) so it references the now-pushed commit.
Contributor
Greptile SummaryThis PR successfully implements configurable background opacity across all cmux chrome surfaces by parsing Key changes:
The implementation is architecturally sound with proper separation of concerns, consistent opacity clamping, and thoughtful prevention of visual artifacts from layered translucency. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Ghostty Config<br/>background-opacity] --> B[GhosttyConfig.parse]
B --> C[GhosttyApp.refreshDefaultBackground<br/>clamps opacity 0-1]
C --> D[GhosttyApp.shared.defaultBackgroundOpacity]
D --> E[GhosttyBackgroundTheme Helper]
E --> F[Titlebar Background]
E --> G[Browser Panel Chrome]
E --> H[Empty Panel Background]
D --> I[Workspace.bonsplitChromeHex]
I --> J{opacity < 0.999?}
J -->|Yes| K[RRGGBBAA hex format]
J -->|No| L[RRGGBB hex format]
K --> M[Bonsplit Tab Bar]
L --> M
D --> N{opacity < 0.999?}
N -->|Yes| O[Window: transparent<br/>Panel: clear background]
N -->|No| P[Window: opaque<br/>Panel: colored background]
style E fill:#e1f5ff
style I fill:#e1f5ff
style O fill:#fff4e1
style P fill:#fff4e1
Last reviewed commit: abab8ef |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Additional Comments (1)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abab8ef9de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| next.backgroundColor = resolvedBackground | ||
| // Use the runtime opacity from the Ghostty engine, which may differ from the | ||
| // file-level value parsed by GhosttyConfig.load(). | ||
| next.backgroundOpacity = defaultBackgroundOpacity() |
There was a problem hiding this comment.
This now stores Ghostty's runtime backgroundOpacity, but the subsequent refresh gating still only compares background hex values, so an opacity-only update (for example changing background-opacity and reloading config without changing color) skips onThemeRefreshRequest. In that scenario the root ContentView never bumps titlebarThemeGeneration, leaving titlebar/window chrome at stale opacity until some unrelated UI state change causes a rerender.
Useful? React with 👍 / 👎.
Use CALayer-level opacity for the titlebar background instead of SwiftUI Color alpha, matching the terminal's Metal compositing path. Account for the double alpha stacking in the terminal area (Bonsplit container bg + Ghostty renderer) so the titlebar visually matches. Also fix opacity-only config changes not triggering titlebar refresh on Cmd+Shift+, reload.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56b9f3a173
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2478
to
+2481
| // Browser-focused workspaces may not have an active terminal panel to refresh | ||
| // the NSWindow background. Keep opaque theme changes applied here as well. | ||
| window.backgroundColor = currentThemeBackground | ||
| window.isOpaque = currentThemeBackground.alphaComponent >= 0.999 |
There was a problem hiding this comment.
This fallback path does not actually run on subsequent theme/opacity updates, so browser-focused workspaces can keep a stale NSWindow background after background-opacity changes. refreshGhosttyAppearanceConfig already skips terminalPanel.applyWindowBackgroundIfActive() when no terminal is focused, and this code relies on WindowAccessor to handle that case, but WindowAccessor defaults to dedupeByWindow = true and short-circuits callbacks unless the window instance changes (Sources/WindowAccessor.swift, guard on lastWindow !== window), so the newly added browser fallback is effectively one-shot.
Useful? React with 👍 / 👎.
This was referenced Mar 2, 2026
This was referenced Mar 10, 2026
This was referenced Mar 18, 2026
This was referenced Mar 27, 2026
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
) * Honor Ghostty background-opacity across all cmux chrome Parse background-opacity from Ghostty config and propagate it through the entire chrome pipeline: bonsplit tab bar (via RRGGBBAA hex), browser panel/omnibar, titlebar, empty panel, and window background. Decouple glass effect from sidebar blend mode — bgGlassEnabled now defaults to false so opacity works independently. Add GhosttyBackgroundTheme helper for consistent color+opacity resolution across all UI surfaces. Fixes manaflow-ai#263 * Titlebar and chrome opacity matches terminal background-opacity Use CALayer-level opacity for the titlebar background instead of SwiftUI Color alpha, matching the terminal's Metal compositing path. Account for the double alpha stacking in the terminal area (Bonsplit container bg + Ghostty renderer) so the titlebar visually matches. Also fix opacity-only config changes not triggering titlebar refresh on Cmd+Shift+, reload.
ohikouta
added a commit
to ohikouta/cmux
that referenced
this pull request
Apr 3, 2026
…arency Ghostty's `background-opacity` setting becomes ineffective when using native macOS fullscreen because the desktop background is removed. This adds support for `macos-non-native-fullscreen` config option (matching Ghostty's own implementation) which expands the window to fill the screen without entering a native fullscreen Space, keeping the desktop visible behind the transparent terminal. Also clamps `background-opacity` config values to 0.0-1.0 range and re-applies window transparency state on native fullscreen exit. Addresses coderabbitai review feedback from PR manaflow-ai#667. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
background-opacityfrom Ghostty config and propagate it through bonsplit tab bar (RRGGBBAA hex via bonsplit PR Release v1.17.0 #14), browser panel/omnibar, titlebar, empty panel, and window backgroundGhosttyBackgroundThemehelper for consistent color+opacity resolution across all UI surfacesbgGlassEnableddefaults tofalseso opacity works independentlyTest plan
background-opacity = 0.8in Ghostty config, launch cmux — terminal, tab bar, browser chrome, and titlebar should all be translucentCloses #263
Supersedes #274
Summary by CodeRabbit
New Features
Behavior Changes
Tests