Skip to content

Fix notification accent propagation after #15445 - #16388

Closed
austinywang wants to merge 1 commit into
mainfrom
fix/15445-notification-accent
Closed

austinywang wants to merge 1 commit into
mainfrom
fix/15445-notification-accent

Conversation

@austinywang

@austinywang austinywang commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

The notification titlebar and popover views now receive the configured cmux accent instead of falling back to the default blue. This follow-up completes the two actionable review findings from merged PR #15445 by injecting the accent environment at the titlebar and both notification popover hosting roots, and by keeping the popover row and badge reads environment-backed.

Related: #15445

Validation:

  • python3 scripts/swift_file_length_budget.py
  • python3 scripts/verify-local.py --only swift-syntax --swift-changed origin/main
  • CI compile admission and package tests on the current head

Changelog

Fixed: notification unread indicators now follow the configured cmux accent in titlebar and popover hosts.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes the notification titlebar and popover to use the configured cmux accent color instead of falling back to the default blue. This completes the remaining review findings from #15445 by injecting the accent environment into the notification popover view and keeping the popover row and badge reads environment-backed.

Written for commit a8b68ad. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 3 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b8243651-b9af-4041-950b-b1867123f92a

📥 Commits

Reviewing files that changed from the base of the PR and between 8a5b39c and a8b68ad.

📒 Files selected for processing (1)
  • Sources/Update/UpdateTitlebarAccessory.swift
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on a8b68ad1ba (run 36822910592 attempt 1): 1 code.

Job Verdict Why
macos / macOS compile admission code a compile error
Matched log lines
macos / macOS compile admission: /tmp/cmux-ci/src/Sources/Update/NotificationPopoverRow.swift:100:18: error: instance method 'fill(_:style:)' requires that 'CmuxAccentColor' conform to 'ShapeStyle'

Not re-run automatically: macos / macOS compile admission is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@austinywang

Copy link
Copy Markdown
Contributor Author

Coordinating the #15488 main full-suite repair in #16386: the current compile failure is now Sources/Update/NotificationPopoverRow.swift:100, where .fill(notification.isRead ? Color.clear : cmuxAccent) passes CmuxAccentColor instead of Color. Hosted evidence: https://github.com/manaflow-ai/cmux/actions/runs/36822667693/job/110241598135. Please include the missing .color projection in this notification repair. #16386 already has a green Bonsplit package lane and will incorporate this repair from main, avoiding a duplicate notification/updater change.

@austinywang

Copy link
Copy Markdown
Contributor Author

The focused browser compile PR #16395 confirms the optional URL error is fixed; its only remaining compile failure is the duplicate cmuxAccent declaration at UpdateTitlebarAccessory.swift:986,989. The same main run still needs NotificationPopoverRow.swift:100 to use cmuxAccent.color. Please rebase/update #16388 with both notification fixes so main and #16395 can turn green.

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this onto current main breaks the compile. Main already declares @Environment(\.cmuxAccentColor) private var cmuxAccent in NotificationsPopoverView (after keyboardShortcutSettingsObserver, Sources/Update/UpdateTitlebarAccessory.swift:2366). This PR adds the same property on the line after the struct header, and git merges both without a conflict. git merge-tree origin/main on this head gives two cmuxAccent declarations at lines 2364 and 2367, which is an invalid redeclaration. The other hunk only removes a blank line. Main already applies .cmuxAccentColorEnvironment() at the three hosting roots (lines 2023, 2266 and 3140), so this PR looks fully superseded.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Closing: main already declares cmuxAccent in NotificationsPopoverView and applies cmuxAccentColorEnvironment at the hosting roots (2c33e92).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants