Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 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 replaces a simple opaque-color fallback for the split-button backdrop with a proper alpha-compositing step, pre-blending the tab bar chrome color over the window background to avoid a double-compositing artifact. The intent is sound, but the implementation picks the wrong color source in the default (no custom chrome) case.
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TabBarView body] --> B{Custom chrome color set?}
B -- Yes --> C[chrome = chromeBackgroundColor]
B -- No --> D_bar[barFill path:\nfallback = windowBackgroundColor]
B -- No --> D_bg[bg path:\nfallback = textBackgroundColor ⚠️]
C --> E[alpha = isFocused ? fg.alpha : fg.alpha * 0.95]
D_bar --> F[barFill = windowBackgroundColor.opacity 0.95 or 1.0]
D_bg --> G[fg = textBackgroundColor\nalpha = 1.0 * 0.95 = 0.95]
E --> H[Composite over NSColor.windowBackgroundColor]
G --> H
H --> I[bg = sRGB flat opaque color]
F --> J[.background barFill applied to tab bar]
I --> K[Used for split-button backdrop gradient]
J --> L[Actual rendered bar color]
K -. "Should match" .-> L
I -. "Mismatch in default theme ⚠️" .-> L
Reviews (1): Last reviewed commit: "Pre-composite button backdrop to match t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let chrome = TabBarColors.nsColorPaneBackground(for: appearance) | ||
| let winBg = NSColor.windowBackgroundColor | ||
| guard let fg = chrome.usingColorSpace(.sRGB), | ||
| let bk = winBg.usingColorSpace(.sRGB) else { | ||
| return Color(nsColor: chrome.withAlphaComponent(1.0)) | ||
| } | ||
| let a = isFocused ? fg.alphaComponent : fg.alphaComponent * 0.95 |
There was a problem hiding this comment.
barFill
The comment states this should "match what .background(barFill) looks like over the window background," but the two code paths use different TabBarColors functions with different fallback colors:
barFill(intabBarBackground) callsTabBarColors.barBackground(for: appearance)→effectiveBackgroundColor(fallback: .windowBackgroundColor)bghere callsTabBarColors.nsColorPaneBackground(for: appearance)→effectiveBackgroundColor(fallback: .textBackgroundColor)
When no custom chrome color is configured (the default theme), these diverge:
barFillresolves toNSColor.windowBackgroundColor(~RGB 0.93, 0.93, 0.93 in Light mode)chrome/fghere resolves toNSColor.textBackgroundColor(~RGB 1.0, 1.0, 1.0 in Light mode — white)
With isFocused = true, a = 1.0, so the composite collapses to just fg (white), while the actual barFill is a light gray. With isFocused = false, a = 0.95, producing a near-white that still doesn't match the gray windowBackgroundColor backdrop.
The fix is to use the same NSColor that barBackground(for:) wraps. Since there's currently no nsColorBarBackground(for:) helper, you can inline it:
let chrome = TabBarColors.nsColorPaneBackground(for: appearance)
// Use the same fallback as barBackground(for:) so fg matches barFill
let effectiveFg = chrome.alphaComponent > 0
? chrome
: NSColor.windowBackgroundColorOr, preferably, add a static func nsColorBarBackground(for:) -> NSColor helper to TabBarColors that mirrors barBackground(for:) but returns an NSColor, and call that instead of nsColorPaneBackground(for:) on line 194.
| // window background. Avoids double-compositing mismatch. | ||
| let bg: Color = { | ||
| let chrome = TabBarColors.nsColorPaneBackground(for: appearance) | ||
| let winBg = NSColor.windowBackgroundColor |
There was a problem hiding this comment.
NSColor.windowBackgroundColor is a semantic, not actual, color
NSColor.windowBackgroundColor is the system's default window background swatch — it does not read the actual rendered pixels behind this view. If the tab bar sits over a window using vibrancy, a NSVisualEffectView, or a custom material, the real composited background could differ from this constant, meaning the pre-composite will still be slightly off.
A more accurate approach would be to read the window's backgroundColor property from NSApp.mainWindow (or the host window captured via TabBarHostWindowReader), which you already have plumbed in on lines 235-240. This would give the actual per-window background color rather than the system semantic value.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
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/Bonsplit/Internal/Views/TabBarView.swift">
<violation number="1" location="Sources/Bonsplit/Internal/Views/TabBarView.swift:194">
P2: The pre-composite backdrop uses `nsColorPaneBackground`, but the tab bar itself uses `barBackground`. Their fallback colors differ, so the new blended backdrop can still mismatch the actual bar color in default appearance.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // matches what .background(barFill) looks like over the | ||
| // window background. Avoids double-compositing mismatch. | ||
| let bg: Color = { | ||
| let chrome = TabBarColors.nsColorPaneBackground(for: appearance) |
There was a problem hiding this comment.
P2: The pre-composite backdrop uses nsColorPaneBackground, but the tab bar itself uses barBackground. Their fallback colors differ, so the new blended backdrop can still mismatch the actual bar color in default appearance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Bonsplit/Internal/Views/TabBarView.swift, line 194:
<comment>The pre-composite backdrop uses `nsColorPaneBackground`, but the tab bar itself uses `barBackground`. Their fallback colors differ, so the new blended backdrop can still mismatch the actual bar color in default appearance.</comment>
<file context>
@@ -187,7 +187,24 @@ struct TabBarView: View {
+ // matches what .background(barFill) looks like over the
+ // window background. Avoids double-compositing mismatch.
+ let bg: Color = {
+ let chrome = TabBarColors.nsColorPaneBackground(for: appearance)
+ let winBg = NSColor.windowBackgroundColor
+ guard let fg = chrome.usingColorSpace(.sRGB),
</file context>
1 participant
Summary by cubic
Pre-composites the tab bar split-button backdrop to an opaque color that matches the bar when blended over the window background. This removes double-compositing color mismatches and keeps the backdrop consistent across focus changes.
Written for commit 2db183f. Summary will update on new commits.