Repository navigation
Fix dock icon not auto-switching with system dark mode - #1928
Conversation
The automatic icon mode relied on the asset catalog to handle appearance-based icon selection, but the compiled Assets.car does not include dark variant renditions for AppIcon. This meant setting applicationIconImage to nil had no effect — the icon stayed on the light variant regardless of system appearance. Replace the nil-reset approach with an active KVO observer on NSApp.effectiveAppearance that programmatically swaps between AppIconDark and AppIconLight images when in automatic mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR adds a singleton Changes
Sequence Diagram(s)sequenceDiagram
participant Settings as AppIconSettings
participant Observer as AppIconAppearanceObserver
participant App as NSApp (NSApplication)
participant Icons as AppIconAssets
Settings->>Observer: startObserving() when mode == .automatic
Observer->>App: read effectiveAppearance
Observer->>Icons: choose AppIconLight or AppIconDark
Icons-->>Observer: selected icon image
Observer->>Settings: apply icon on main queue
Note right of App: On appearance change (KVO)
App->>Observer: effectiveAppearance changed
Observer->>Icons: reselect appropriate icon
Icons-->>Observer: new icon image
Observer->>Settings: reapply icon on main queue
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 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 Tip Migrating from UI to YAML configuration.Use the |
Greptile SummaryThis PR fixes dock icon appearance not following the system dark/light mode by replacing the previous Key changes in
Minor issues found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppDelegate
participant AppIconSettings
participant AppIconAppearanceObserver
participant NSApp
AppDelegate->>AppIconSettings: applyIcon(resolvedMode)
AppIconSettings->>AppIconAppearanceObserver: startObserving()
AppIconAppearanceObserver->>NSApp: effectiveAppearance.bestMatch(...)
AppIconAppearanceObserver->>NSApp: applicationIconImage = AppIconDark/Light
AppIconAppearanceObserver->>NSApp: observe(\.effectiveAppearance)
Note over NSApp: System appearance changes
NSApp-->>AppIconAppearanceObserver: KVO callback fires
AppIconAppearanceObserver->>AppIconAppearanceObserver: DispatchQueue.main.async
AppIconAppearanceObserver->>NSApp: applicationIconImage = AppIconDark/Light
User->>AppIconSettings: applyIcon(.light)
AppIconSettings->>AppIconAppearanceObserver: stopObserving()
AppIconAppearanceObserver->>AppIconAppearanceObserver: observation.invalidate() + nil
AppIconSettings->>NSApp: applicationIconImage = AppIconLight
Last reviewed commit: "Fix dock icon not au..." |
| observation = NSApp.observe(\.effectiveAppearance, options: [.new]) { [weak self] _, _ in | ||
| DispatchQueue.main.async { | ||
| self?.applyIconForCurrentAppearance() | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale async block can override explicit icon selection
When the user switches from .automatic to .light or .dark, stopObserving() invalidates the KVO token, but any DispatchQueue.main.async block that was already enqueued from a prior KVO callback will still run after the explicit icon has been set.
Concrete sequence:
- System appearance changes → KVO fires synchronously on main thread →
DispatchQueue.main.async { applyIconForCurrentAppearance() }is enqueued - Before the queued block runs, user picks "Light" →
stopObserving()→applicationIconImage = lightIcon - The enqueued block now runs → overwrites with the appearance-based icon, ignoring the user's explicit choice
A simple guard using a flag (or checking the current mode before applying) prevents the stale dispatch from firing:
func stopObserving() {
observation?.invalidate()
observation = nil
}
private func applyIconForCurrentAppearance() {
guard observation != nil else { return } // drop stale async calls
let isDark = NSApp.effectiveAppearance.bestMatch(from: [.darkAqua, .aqua]) == .darkAqua
let imageName = isDark ? "AppIconDark" : "AppIconLight"
if let icon = NSImage(named: imageName) {
NSApplication.shared.applicationIconImage = icon
}
}| final class AppIconAppearanceObserver: NSObject { | ||
| static let shared = AppIconAppearanceObserver() |
There was a problem hiding this comment.
Singleton missing
private init()
AppIconAppearanceObserver exposes static let shared but has no private init(), so callers outside this file can still create independent instances (e.g. AppIconAppearanceObserver()) that each set up their own KVO observation without going through the shared lifecycle. Adding a private init() makes the singleton intent explicit and prevents accidental misuse.
| final class AppIconAppearanceObserver: NSObject { | |
| static let shared = AppIconAppearanceObserver() | |
| final class AppIconAppearanceObserver: NSObject { | |
| static let shared = AppIconAppearanceObserver() | |
| private var observation: NSKeyValueObservation? | |
| private override init() { super.init() } |
| func startObserving() { | ||
| applyIconForCurrentAppearance() | ||
| guard observation == nil else { return } | ||
| observation = NSApp.observe(\.effectiveAppearance, options: [.new]) { [weak self] _, _ in |
There was a problem hiding this comment.
.new option is unused overhead
options: [.new] asks KVO to allocate and populate the NSKeyValueObservedChange with the new value, but the closure's second parameter is _ — it's never read. Using options: [] eliminates the unnecessary allocation on every appearance change.
| observation = NSApp.observe(\.effectiveAppearance, options: [.new]) { [weak self] _, _ in | |
| observation = NSApp.observe(\.effectiveAppearance, options: []) { [weak self] _, _ in |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/cmuxApp.swift`:
- Around line 3709-3712: The NSApp appearance observer's async callback can run
after stopObserving() clears observation, so modify the closure created in
observation = NSApp.observe(\.effectiveAppearance...) to early-return if the
observer was removed: inside the DispatchQueue.main.async block, check that self
is non-nil and that self.observation != nil before calling
applyIconForCurrentAppearance(); this prevents stale appearance updates from
overriding a manually-set fixed icon and ties the update to the observer's
lifetime.
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/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:3711">
P2: Queued async appearance updates can run after leaving automatic mode and override manually selected light/dark icon.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Add guard in async callback to skip stale updates after stopObserving() - Add private init() to prevent external instantiation of singleton - Remove unused .new KVO option Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thank you for the contribution! |
|
Thank you for the contribution! |
) * Fix dock icon not auto-switching with system dark mode The automatic icon mode relied on the asset catalog to handle appearance-based icon selection, but the compiled Assets.car does not include dark variant renditions for AppIcon. This meant setting applicationIconImage to nil had no effect — the icon stayed on the light variant regardless of system appearance. Replace the nil-reset approach with an active KVO observer on NSApp.effectiveAppearance that programmatically swaps between AppIconDark and AppIconLight images when in automatic mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review feedback: fix async race and harden singleton - Add guard in async callback to skip stale updates after stopObserving() - Add private init() to prevent external instantiation of singleton - Remove unused .new KVO option Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: CHE-3 <schumannzheng@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
applicationIconImage = nil, expecting the asset catalog to handle dark/light switching — but the compiledAssets.cardoesn't include dark variant renditions forAppIcon, so the icon stays light regardless of system appearanceNSApp.effectiveAppearancethat programmatically swaps betweenAppIconDarkandAppIconLightimages when in automatic modeRoot Cause
The
AppIcon.appiconset/Contents.jsoncorrectly defines dark variants withluminosity: darkappearance, but the compiledAssets.carstrips these — allAppIconentries haveAppearance: none. TheAppIconDarkandAppIconLightimagesets exist separately but aren't linked as appearance variants of the mainAppIcon.Test plan
🤖 Generated with Claude Code
Summary by cubic
Fixes the dock icon not switching with macOS dark mode by observing system appearance and swapping between
AppIconDarkandAppIconLightin Automatic mode. The observer starts/stops with icon mode changes, avoids stale async updates, and applies the correct icon on launch while keeping manual Light/Dark fixed.NSApp.effectiveAppearanceto swapAppIconDark/AppIconLightin Automatic mode.stopObserving(), enforce a singleton with a private init, and remove the unused KVO option.Written for commit fb5b763. Summary will update on new commits.
Summary by CodeRabbit