Repository navigation
Fix browser popover appearance contrast - #9744
Conversation
|
@coderabbitai review |
📝 WalkthroughWalkthroughAdds browser chrome popover color-scheme handling and debug-only downloads-popover appearance capture. Adds an end-to-end UI test that verifies light content and window appearance under dark AppKit appearance. ChangesDownloads Popover Appearance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BrowserDownloadsPopoverContrastUITests
participant DebugApp
participant BrowserPanelView
participant BrowserDownloadsToolbarButton
participant BrowserDownloadsPopoverAppearanceUITestRecorder
participant UITestCaptureSink
BrowserDownloadsPopoverContrastUITests->>DebugApp: launch with dark AppKit and light browser settings
DebugApp->>BrowserPanelView: provide fixture download
BrowserPanelView->>BrowserDownloadsToolbarButton: render downloads popover
BrowserDownloadsToolbarButton->>BrowserDownloadsPopoverAppearanceUITestRecorder: record popover window
BrowserDownloadsPopoverAppearanceUITestRecorder->>UITestCaptureSink: write light window and content appearance
BrowserDownloadsPopoverContrastUITests->>UITestCaptureSink: poll captured JSON results
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 1📝 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 |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 245,275 of the 240,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 25 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
Rate Limit Exceeded
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@coderabbitai review |
Rate Limit Exceeded
|
|
Review refresh after 2c7bc82: @coderabbitai review |
|
|
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 245,275 of the 240,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 25 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxUITests/BrowserDownloadsPopoverContrastUITests.swift`:
- Around line 191-204: The terminateAppProcess method must confirm that the app
exits after process.interrupt() before allowing teardown to continue. Add a
deadline-bounded wait using the process’s actual termination state or equivalent
completion signal, and fail test cleanup if the process remains running when the
bound expires.
- Around line 81-83: Update the wait predicate in the appearance probe test to
require the expected light values for both contentColorScheme and
windowAppearance, rather than only checking that the fields exist. Use the
two-write contract in BrowserDownloadsPopoverAppearanceUITestSupport to ensure
the wait completes on the final snapshot before assertions run.
In `@Sources/Debug/UITests/BrowserDownloadsPopoverAppearanceUITestSupport.swift`:
- Around line 42-48: Remove the Task.yield-based second write from
record(window:contentColorScheme:), leaving a single snapshot capture. Update
WindowAccessor so it invokes the capture only after the popover window has
attached and resolved its appearance, without relying on a later scheduler turn.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5455426d-6f53-4441-93f5-43123a1a7216
📒 Files selected for processing (8)
Sources/Debug/UITests/BrowserDownloadsPopoverAppearanceUITestPresenter.swiftSources/Debug/UITests/BrowserDownloadsPopoverAppearanceUITestRecorder.swiftSources/Debug/UITests/BrowserDownloadsPopoverAppearanceUITestSupport.swiftSources/Panels/BrowserChromePopoverAppearance.swiftSources/Panels/BrowserDownloadsToolbarButton.swiftSources/Panels/BrowserPanelView.swiftcmux.xcodeproj/project.pbxprojcmuxUITests/BrowserDownloadsPopoverContrastUITests.swift
|
Review refresh after 6239b29: @coderabbitai review |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 245,275 of the 240,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 25 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6239b29. Configure here.
|
Review refresh after 981d4d7: @coderabbitai review |
|
|
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 245,275 of the 240,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 25 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
@coderabbitai review |
|
|
Review disposition for non-inline bot output:
All actionable inline CodeRabbit and Cursor threads are fixed, answered, and resolved. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Issue: #9341
Closes #9341
Testing
189ac5b27b; exactly one focused test executed and failed because both the popover SwiftUI environment and_NSPopoverWindow.effectiveAppearanceresolved dark.981d4d78166f721b65c9d78f4a9d2253821fd5ec; exactly one focused test executed with zero failures in 6.514 seconds. Artifact: https://github.com/manaflow-ai/cmux-dev-artifacts/issues/9469../scripts/check-pbxproj.sh../scripts/lint-pbxproj-test-wiring.sh(654 test files).python3 scripts/check-package-resolved-policy.py,git diff --check, and Swift parser checks for all changed Swift files.origin/main(80054fcaca) succeeds without conflicts.xcodebuild test, or local XCUITest was run.Localization audit: no user-facing strings changed; the changed Swift surfaces were checked for newly introduced bare English, and no localization catalog changes are needed.
Warning audit: no warning-budget or file-length-budget file is changed.
Demo Video
Review Trigger
Requested separately after the latest commit:
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Note
Low Risk
Scoped SwiftUI/AppKit appearance wiring and DEBUG-only test probes; no auth, data, or release-path behavior changes beyond popover styling.
Overview
Fixes mixed light/dark contrast in browser toolbar popovers when the app appearance and browser chrome disagree (e.g. dark app with light terminal-derived chrome).
A shared
browserChromePopoverAppearancemodifier appliespreferredColorSchemeto popover content so SwiftUI semantics and the AppKit_NSPopoverWindowmaterial resolve from the same browser-chrome scheme. It is wired on the Downloads, profile, theme, and import-hint popovers; the downloads popover also reads the chrome scheme from its toolbar button.DEBUG-only UI-test hooks auto-present the real downloads popover, inject a fixture download, and record
contentColorScheme,windowAppearance, and window class via anNSViewwindow accessor. A new headless XCUITest launches the binary directly and asserts both schemes stay light under dark app / light browser chrome.Reviewed by Cursor Bugbot for commit 981d4d7. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes mixed light/dark contrast in browser toolbar popovers by resolving both SwiftUI content and the AppKit popover from the browser‑chrome color scheme. Applies to Downloads, profile, theme, and import‑hint popovers and adds a lifecycle‑driven UI test to prevent regressions. Fixes #9341.
browserChromePopoverAppearanceso popover material and semantic colors resolve from the browser‑chrome scheme._NSPopoverWindowand content scheme and refreshes on attach, effective-appearance changes, and SwiftUI updates.Written for commit 981d4d7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes