Repository navigation
Add Match Terminal Background sidebar setting - #2293
Conversation
Adds a toggle in Settings > Sidebar Appearance that makes the sidebar use the same background color and transparency as the terminal area. Uses layer-level opacity on a fully opaque background color (the same technique as TitlebarLayerBackground) with effective opacity formula `1 - (1-alpha)^2` to account for the terminal's two stacked semi-transparent layers (Bonsplit chrome + Ghostty Metal surface). Also adds a 1px trailing border derived from the terminal chrome color, matching the bonsplit tab bar separator logic.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new "Match Terminal Background" persisted setting, localized strings, settings/debug toggles, conditional sidebar layout/backdrop rendering in ContentView, a native NSView-backed sidebar background component, and a 1px trailing sidebar border. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsView
participant AppStorage as Storage
participant ContentView
participant SidebarNSView as SidebarTerminalBackgroundView
participant Ghostty as GhosttyApp.shared
User->>SettingsView: Toggle "Match Terminal Background"
SettingsView->>Storage: write sidebarMatchTerminalBackground = true
Storage-->>ContentView: AppStorage binding updates
ContentView->>Ghostty: read defaultBackgroundOpacity & color
ContentView->>SidebarNSView: create/update with color + opacity
SidebarNSView->>SidebarNSView: set CALayer backgroundColor & opacity
ContentView->>ContentView: switch layout/backdrop path and render trailing border
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 adds a "Match Terminal Background" toggle under Settings > Sidebar Appearance. When enabled, the sidebar uses the same background color and transparency as the Ghostty terminal by replicating its two-layer compositing model ( Key changes:
Confidence Score: 4/5Safe to merge after fixing the stale border color on theme change in SidebarTrailingBorder. One P1 defect: SidebarTrailingBorder lacks a .ghosttyDefaultBackgroundDidChange subscription, so the border color is not updated when the Ghostty theme changes, even though the PR description explicitly claims live theme reactivity. The backdrop itself is correctly wired. Everything else (settings UI, localization, reset, layout switch) looks correct. Sources/ContentView.swift — specifically SidebarTrailingBorder around line 13771. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[SidebarBackdrop.body] --> B{matchTerminalBackground?}
B -->|Yes| C[Compute effective opacity]
C --> D[SidebarTerminalBackgroundView]
D --> E[onReceive ghosttyDefaultBackgroundDidChange - triggers re-render OK]
B -->|No| F[Existing blur / visual effect path]
G[VerticalTabsSidebar] --> H[background SidebarBackdrop]
G --> I[overlay SidebarTrailingBorder]
I --> J{matchTerminalBackground?}
J -->|Yes| K[Rectangle 1px - chromeSeparatorColor static call]
K --> L[No notification subscription - border color goes STALE]
J -->|No| M[EmptyView]
N[Ghostty theme change] -->|ghosttyDefaultBackgroundDidChange| E
N -.->|missing link| L
Reviews (1): Last reviewed commit: "Add "Match Terminal Background" sidebar ..." | Re-trigger Greptile |
| /// 1px trailing border on the sidebar, derived from the terminal chrome background | ||
| /// using the same logic as bonsplit's TabBarColors.nsColorSeparator: | ||
| /// dark bg → lighten RGB by 0.16 at 0.36 alpha; light bg → darken by 0.12 at 0.26 alpha. | ||
| private struct SidebarTrailingBorder: View { | ||
| @AppStorage("sidebarMatchTerminalBackground") private var matchTerminalBackground = false | ||
|
|
||
| var body: some View { | ||
| if matchTerminalBackground { | ||
| Rectangle() | ||
| .fill(Color(nsColor: Self.chromeSeparatorColor())) | ||
| .frame(width: 1) | ||
| .ignoresSafeArea() | ||
| } | ||
| } |
There was a problem hiding this comment.
SidebarTrailingBorder border color doesn't update on Ghostty theme change
chromeSeparatorColor() is a static call that reads GhosttyBackgroundTheme.currentColor() at render time, but SidebarTrailingBorder has only one reactive dependency: @AppStorage("sidebarMatchTerminalBackground"). SwiftUI will not re-render this view when the Ghostty theme changes, so the border color will become stale after a theme switch.
Compare with SidebarBackdrop: it has @State private var terminalBackgroundColor and an .onReceive(NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)) that mutates that state to force a re-render. SidebarTrailingBorder has no equivalent mechanism.
The test plan item "Change Ghostty themes, verify sidebar updates live" would verify the backdrop updates (because SidebarBackdrop is correctly wired) but would miss the stale border.
The fix is the same pattern used in SidebarBackdrop:
private struct SidebarTrailingBorder: View {
@AppStorage("sidebarMatchTerminalBackground") private var matchTerminalBackground = false
@State private var separatorColor: NSColor = chromeSeparatorColor()
var body: some View {
if matchTerminalBackground {
Rectangle()
.fill(Color(nsColor: separatorColor))
.frame(width: 1)
.ignoresSafeArea()
.onReceive(
NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)
) { _ in
separatorColor = Self.chromeSeparatorColor()
}
}
}
...
}| /// fully opaque layer color + layer-level opacity. Also clears non-transparent | ||
| /// ancestor hosting view layers that SwiftUI may have added after the initial | ||
| /// makeViewHierarchyTransparent pass, preventing them from tinting the fill. | ||
| private struct SidebarTerminalBackgroundView: NSViewRepresentable { | ||
| let backgroundColor: NSColor | ||
| let opacity: CGFloat | ||
|
|
||
| func makeNSView(context: Context) -> NSView { | ||
| let view = NSView() | ||
| view.wantsLayer = true | ||
| view.layer?.backgroundColor = backgroundColor.withAlphaComponent(1.0).cgColor | ||
| view.layer?.opacity = Float(opacity) | ||
| return view | ||
| } | ||
|
|
There was a problem hiding this comment.
Inaccurate doc comment on
SidebarTerminalBackgroundView
The doc comment states: "Also clears non-transparent ancestor hosting view layers that SwiftUI may have added after the initial makeViewHierarchyTransparent pass, preventing them from tinting the fill."
However, the implementation is byte-for-byte identical to TitlebarLayerBackground (lines 309–324) and performs no such ancestor-layer clearing — it only sets layer.backgroundColor and layer.opacity on the view's own layer. Either the described clearing logic is missing from this implementation, or the comment was copy-pasted and describes intended (but unimplemented) behavior. The comment should be corrected to avoid misleading future maintainers.
Additionally, since SidebarTerminalBackgroundView and TitlebarLayerBackground are identical, consider consolidating them into a single shared private struct (e.g., LayerBackgroundView) to reduce duplication.
There was a problem hiding this comment.
The doc comment is accurate for the current implementation. It describes the layer color + opacity technique, which matches what the code does.
— Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58bd0ba247
ℹ️ 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".
| Text("Sidebar Appearance") | ||
| .font(.headline) | ||
|
|
||
| Toggle("Match Terminal Background", isOn: $matchTerminalBackground) |
There was a problem hiding this comment.
Localize the new sidebar debug toggle label
SidebarDebugView introduces Toggle("Match Terminal Background", ...) as a hardcoded English string, which violates the localization rule in /workspace/cmux/AGENTS.md (all user-facing strings must use localization keys). In non-English locales (for example Japanese), this control will remain untranslated and creates an immediate localization regression in the settings UI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already fixed in 315d670 - the debug toggle uses String(localized:) with a proper key.
— Claude Code
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/Localizable.xcstrings`:
- Around line 83201-83234: Add the missing locale entries for both
"settings.sidebarAppearance.matchTerminalBackground" and
"settings.sidebarAppearance.matchTerminalBackground.subtitle" to match the
project's existing pattern: include the full set of locales (ar, bs, da, de, en,
es, fr, it, ja, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, zh-Hant), copying
the English "value" for any locales that should use an English fallback and mark
their "stringUnit.state" as "translated"; ensure existing Japanese entries
remain unchanged and all new locale blocks follow the same structure as other
settings.sidebarAppearance.* keys in the catalog.
In `@Sources/cmuxApp.swift`:
- Line 2923: Replace the bare string in the SwiftUI Toggle with a localized
string: instead of Toggle("Match Terminal Background", isOn:
$matchTerminalBackground) use String(localized:
"settings.matchTerminalBackground", defaultValue: "Match Terminal Background")
for the label; update the Toggle call that references matchTerminalBackground
accordingly and add the corresponding key ("settings.matchTerminalBackground")
to your localization resources. Ensure you modify the Toggle invocation in
cmuxApp.swift where matchTerminalBackground is used so all user-facing text
follows the String(localized:..., defaultValue:...) pattern.
In `@Sources/ContentView.swift`:
- Around line 13775-13803: The separator's color isn't refreshed when Ghostty
theme changes because the Rectangle only depends on `@AppStorage`
matchTerminalBackground; add a listener for .ghosttyDefaultBackgroundDidChange
(the same notification used by the sibling backdrop) and force the separator to
recompute by updating a small `@State` trigger that's referenced by the Rectangle
(or by changing the view's id) so chromeSeparatorColor() is re-evaluated when
the notification fires; update the body around matchTerminalBackground/Rectangle
to include an onReceive(NotificationCenter.default.publisher(for:
.ghosttyDefaultBackgroundDidChange)) { ... } that flips the state trigger.
- Around line 2605-2609: The toggle that flips layout (computed as
useWithinWindow using sidebarBlendMode and sidebarMatchTerminalBackground) can
change portal-hosted surface frames without triggering a geometry resync; after
computing useWithinWindow and before switching into the HStack/ZStack branches,
call the same portal geometry resync hook used for sidebar-width changes (the
explicit resync invoked elsewhere for width updates) so portal frames are
updated immediately when sidebarBlendMode or sidebarMatchTerminalBackground
changes; locate the resync call used for width changes and invoke it here
(adjacent to useWithinWindow) to schedule a portal geometry resync.
🪄 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: 50bc75e0-cc6c-4e48-8376-3d8ee87e4b0d
📒 Files selected for processing (3)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/cmuxApp.swift
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:13774">
P2: Make the trailing border observe Ghostty background changes so its color updates immediately with theme switches.</violation>
<violation number="2" location="Sources/ContentView.swift:13808">
P3: The doc comment says this view clears non-transparent ancestor hosting layers, but the implementation only updates its own layer color/opacity. Update the comment or implement the missing ancestor-clearing behavior so the docs match runtime behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Add @State + .onReceive(.ghosttyDefaultBackgroundDidChange) to SidebarTrailingBorder so the separator color recomputes when the Ghostty theme changes, matching the pattern used in SidebarBackdrop.
|
Fixed the stale border color issue flagged by Greptile in c2ce66a. Added |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:13784">
P2: Refresh `separatorColor` when the border view appears; otherwise toggling “Match Terminal Background” on after a theme change can show an out-of-date border color.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Localize the debug panel toggle label (Codex P1) - Add .onAppear to SidebarTrailingBorder for initial color (Cubic P2) - Fix stale doc comment on SidebarTerminalBackgroundView (Cubic P3)
* Add "Match Terminal Background" sidebar setting Adds a toggle in Settings > Sidebar Appearance that makes the sidebar use the same background color and transparency as the terminal area. Uses layer-level opacity on a fully opaque background color (the same technique as TitlebarLayerBackground) with effective opacity formula `1 - (1-alpha)^2` to account for the terminal's two stacked semi-transparent layers (Bonsplit chrome + Ghostty Metal surface). Also adds a 1px trailing border derived from the terminal chrome color, matching the bonsplit tab bar separator logic. * Fix sidebar border color not updating on theme change Add @State + .onReceive(.ghosttyDefaultBackgroundDidChange) to SidebarTrailingBorder so the separator color recomputes when the Ghostty theme changes, matching the pattern used in SidebarBackdrop. * Address review comments: localize debug toggle, fix separator refresh - Localize the debug panel toggle label (Codex P1) - Add .onAppear to SidebarTrailingBorder for initial color (Cubic P2) - Fix stale doc comment on SidebarTerminalBackgroundView (Cubic P3) --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Summary
1 - (1-alpha)²effective opacity) matching the terminal's two stacked semi-transparent layers (Bonsplit chrome + Ghostty Metal surface).ghosttyDefaultBackgroundDidChangenotificationresetAllSettings()and en/ja localizationTest plan
background-opacity < 1.0, verify sidebar background matches terminalSummary by cubic
Adds a "Match Terminal Background" sidebar setting that uses the terminal’s background color and effective opacity for a seamless look, plus a 1px trailing border that matches terminal chrome. Updates apply live on theme/background changes.
New Features
Bug Fixes
.ghosttyDefaultBackgroundDidChange.Written for commit 315d670. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization