Conversation
📝 WalkthroughWalkthroughThe PR adds persisted light, dark, and system appearance settings. The settings window updates live, and settings controls now use adaptive colors. English and Simplified Chinese documentation and localization include the new appearance options. ChangesAppearance theming
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsView
participant AppearanceStore
participant SettingsWindowController
participant macOS
SettingsView->>AppearanceStore: update selected appearance
AppearanceStore->>SettingsWindowController: publish appearance change
SettingsWindowController->>macOS: set window appearance
macOS-->>SettingsView: render selected color scheme
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 788155fcef
ℹ️ 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".
| .background(Color(red: 0.020, green: 0.020, blue: 0.027)) | ||
| .preferredColorScheme(.dark) | ||
| .background(IslandColor.settingsBackground) | ||
| .preferredColorScheme(appearanceStore.appearance.colorScheme) |
There was a problem hiding this comment.
Apply the selected appearance to the island
When the user selects Light, or selects System while macOS is in light mode, this modifier only changes the Settings view. The repository-wide usage search shows AppearanceStore is observed only here and by SettingsWindowController; IslandRootView still renders a black silhouette with fixed white content and never observes the preference. Consequently, the advertised appearance setting has no effect on the island itself after Settings is closed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@README.md`:
- Around line 187-188: Update the README feature description to replace
“light/dark/system appearance” with the clearer wording “light, dark, or system
appearance,” leaving the surrounding list items unchanged.
In `@Sources/Model/AppearanceStore.swift`:
- Around line 25-33: Update IslandWindowController to observe
AppearanceStore.shared, matching the subscription pattern used by
SettingsWindowController, and apply the selected appearance to the island
NSWindow: nil for .system, .darkAqua for .dark, and .aqua for .light. Ensure
changes are applied to the borderless panel window while preserving the existing
IslandRootView setup.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2743bc7a-fccb-4cc7-b69a-6249b5893dfb
📒 Files selected for processing (16)
README.mdREADME.zh-CN.mdResources/en.lproj/Localizable.stringsResources/zh-Hans.lproj/Localizable.stringsSources/Model/AppearanceStore.swiftSources/Theme/Colors.swiftSources/Views/Settings/BrandHeader.swiftSources/Views/Settings/ChartStylePicker.swiftSources/Views/Settings/CostStylePicker.swiftSources/Views/Settings/SegmentedControl.swiftSources/Views/Settings/SettingsFooter.swiftSources/Views/Settings/SettingsRow.swiftSources/Views/Settings/SettingsToggle.swiftSources/Views/Settings/StyleTile.swiftSources/Views/SettingsView.swiftSources/Views/SettingsWindowController.swift
| light/dark/system appearance, Always show usage, Low Power Mode, | ||
| configurable limit alerts, and Sparkle update controls. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a clear list for appearance modes.
Write light, dark, or system appearance instead of light/dark/system appearance. This improves readability and removes the slash compound.
Proposed wording
- light/dark/system appearance, Always show usage, Low Power Mode,
+ light, dark, or system appearance, Always show usage, Low Power Mode,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| light/dark/system appearance, Always show usage, Low Power Mode, | |
| configurable limit alerts, and Sparkle update controls. | |
| light, dark, or system appearance, Always show usage, Low Power Mode, | |
| configurable limit alerts, and Sparkle update controls. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~187-~187: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...k/system appearance, Always show usage, Low Power Mode, configurable limit alerts, and ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for 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.
In `@README.md` around lines 187 - 188, Update the README feature description to
replace “light/dark/system appearance” with the clearer wording “light, dark, or
system appearance,” leaving the surrounding list items unchanged.
Source: Linters/SAST tools
| @MainActor | ||
| final class AppearanceStore: ObservableObject { | ||
| static let shared = AppearanceStore() | ||
| static let key = "MacIsland.appearance" | ||
|
|
||
| @Published var appearance: AppAppearance { | ||
| didSet { | ||
| UserDefaults.standard.set(appearance.rawValue, forKey: Self.key) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'AppearanceStore\.shared|preferredColorScheme|NSApp\.appearance|NSApplication\.shared\.appearance|NSAppearance\(named:' \
SourcesRepository: ericjypark/codex-island
Length of output: 2837
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked Swift files:\n'
git ls-files 'Sources/*.swift' 'Sources/**/*.swift' | sed -n '1,120p'
printf '\nAppearanceStore reference summary:\n'
rg -n 'AppearanceStore|AppAppearance|NSAppearance|window\.appearance|appearance =|\.colorScheme|`@MainActor`' Sources
printf '\nMain app files:\n'
for f in $(git ls-files 'Sources/*.swift' 'Sources/**/*.swift' | grep -E 'App|Entry|main|AppDelegate|Scene' || true); do
echo "--- $f"
wc -l "$f"
doneRepository: ericjypark/codex-island
Length of output: 8190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- Sources/App.swift ---\n'
cat -n Sources/App.swift
printf '\n--- Sources/Window/IslandWindowController.swift ---\n'
wc -l Sources/Window/IslandWindowController.swift
sed -n '1,280p' Sources/Window/IslandWindowController.swift | cat -n
printf '\n--- Sources/Views/IslandRootView.swift ---\n'
wc -l Sources/Views/IslandRootView.swift
sed -n '1,240p' Sources/Views/IslandRootView.swift | cat -nRepository: ericjypark/codex-island
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Sources/App.swift ---'
cat -n Sources/App.swift
printf '%s\n' ''
printf '%s\n' '--- Sources/Window/IslandWindowController.swift ---'
wc -l Sources/Window/IslandWindowController.swift
sed -n '1,280p' Sources/Window/IslandWindowController.swift | cat -n
printf '%s\n' ''
printf '%s\n' '--- Sources/Views/IslandRootView.swift ---'
wc -l Sources/Views/IslandRootView.swift
sed -n '1,240p' Sources/Views/IslandRootView.swift | cat -nRepository: ericjypark/codex-island
Length of output: 28505
Wire AppearanceStore into IslandWindowController and apply it to the island window.
IslandWindowController hosts IslandRootView and controls the island window, but it does not observe AppearanceStore.shared. Add that subscription like SettingsWindowController and apply nil for .system, .darkAqua for .dark, and .aqua for .light to the island NSWindow; rely on .preferredColorScheme alone updates the settings SwiftUI scene, not the borderless panel.
🤖 Prompt for 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.
In `@Sources/Model/AppearanceStore.swift` around lines 25 - 33, Update
IslandWindowController to observe AppearanceStore.shared, matching the
subscription pattern used by SettingsWindowController, and apply the selected
appearance to the island NSWindow: nil for .system, .darkAqua for .dark, and
.aqua for .light. Ensure changes are applied to the borderless panel window
while preserving the existing IslandRootView setup.
What
Adds Light, Dark, and System appearance choices to Settings and applies the selected appearance consistently across the island and settings window.
Why
Users should be able to use CodexIsland in a daytime-friendly light appearance or let it follow macOS automatically.
Validation
Split from #67 following maintainer feedback.
Summary by CodeRabbit
New Features
Documentation