fix(noctalia): preserve app icon colors - #1695
Conversation
|
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request updates Noctalia bar widget configurations to explicitly disable icon colorization for the ActiveWindow and Tray widgets, then adds corresponding test assertions to verify these settings are correctly applied in the shell configuration. ChangesWidget Icon Colorization Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Mesa DescriptionTL;DRFixes Noctalia tray icon colors by enabling What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request updates the Tray widget configuration in config/noctalia/default.nix by setting colorizeIcons to false. Correspondingly, the test specification in spec/noctalia_bar_spec.sh has been updated to verify this new configuration and improve the tray block matching logic. I have no feedback to provide.
84158a9 to
4bbf999
Compare
Co-authored-by: Codex <noreply@openai.com>
4bbf999 to
003f344
Compare
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 `@config/noctalia/default.nix`:
- Around line 85-89: The Tray section currently sets colorizeIcons = false while
the PR intends to colorize tray icons; update the Tray block (id = "Tray") to
set colorizeIcons = true and keep drawerEnabled = false so tray icons are
colorized but the drawer remains disabled; locate the Tray stanza containing id
= "Tray", change colorizeIcons to true and leave drawerEnabled unchanged.
In `@spec/noctalia_bar_spec.sh`:
- Around line 12-15: The spec "It 'shows tray items inline instead of hiding
them in the drawer'" asserts the wrong value for tray icon colorization —
replace the expectation for the string 'colorizeIcons = false;' with
'colorizeIcons = true;' so the test matches the PR's intent to colorize tray
icons; keep the existing 'drawerEnabled = false;' assertion as-is and ensure the
grep/awk target that captures the Tray block (the same block that yields
'drawerEnabled' and 'colorizeIcons') remains unchanged.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: baf7e0c7-8a95-4f7e-9caa-8cd0e89931a9
📒 Files selected for processing (2)
config/noctalia/default.nixspec/noctalia_bar_spec.sh
| { | ||
| id = "Tray"; | ||
| colorizeIcons = false; | ||
| drawerEnabled = false; | ||
| } |
There was a problem hiding this comment.
Tray icon colorization is inverted relative to this PR objective.
Line 87 sets colorizeIcons = false, but this PR’s stated goal is to colorize tray icons while keeping the drawer disabled.
Suggested fix
widgets.right = [
{
id = "Tray";
- colorizeIcons = false;
+ colorizeIcons = true;
drawerEnabled = false;
}📝 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.
| { | |
| id = "Tray"; | |
| colorizeIcons = false; | |
| drawerEnabled = false; | |
| } | |
| { | |
| id = "Tray"; | |
| colorizeIcons = true; | |
| drawerEnabled = false; | |
| } |
🤖 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 `@config/noctalia/default.nix` around lines 85 - 89, The Tray section currently
sets colorizeIcons = false while the PR intends to colorize tray icons; update
the Tray block (id = "Tray") to set colorizeIcons = true and keep drawerEnabled
= false so tray icons are colorized but the drawer remains disabled; locate the
Tray stanza containing id = "Tray", change colorizeIcons to true and leave
drawerEnabled unchanged.
| It 'shows tray items inline instead of hiding them in the drawer' | ||
| When run bash -c "awk '/id = \"Tray\";/{in_tray=1} in_tray && /drawerEnabled = false;/{print; exit}' '$CONFIG'" | ||
| When run bash -c "awk '/id = \"Tray\";/{in_tray=1} in_tray && /^ }/{exit} in_tray{print}' '$CONFIG'" | ||
| The output should include 'colorizeIcons = false;' | ||
| The output should include 'drawerEnabled = false;' |
There was a problem hiding this comment.
Tray spec asserts the wrong target behavior.
Line 14 expects colorizeIcons = false;, which contradicts the PR objective to colorize tray icons. This test will enforce the wrong config.
Suggested fix
It 'shows tray items inline instead of hiding them in the drawer'
When run bash -c "awk '/id = \"Tray\";/{in_tray=1} in_tray && /^ }/{exit} in_tray{print}' '$CONFIG'"
-The output should include 'colorizeIcons = false;'
+The output should include 'colorizeIcons = true;'
The output should include 'drawerEnabled = false;'
End📝 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.
| It 'shows tray items inline instead of hiding them in the drawer' | |
| When run bash -c "awk '/id = \"Tray\";/{in_tray=1} in_tray && /drawerEnabled = false;/{print; exit}' '$CONFIG'" | |
| When run bash -c "awk '/id = \"Tray\";/{in_tray=1} in_tray && /^ }/{exit} in_tray{print}' '$CONFIG'" | |
| The output should include 'colorizeIcons = false;' | |
| The output should include 'drawerEnabled = false;' | |
| It 'shows tray items inline instead of hiding them in the drawer' | |
| When run bash -c "awk '/id = \"Tray\";/{in_tray=1} in_tray && /^ }/{exit} in_tray{print}' '$CONFIG'" | |
| The output should include 'colorizeIcons = true;' | |
| The output should include 'drawerEnabled = false;' |
🤖 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 `@spec/noctalia_bar_spec.sh` around lines 12 - 15, The spec "It 'shows tray
items inline instead of hiding them in the drawer'" asserts the wrong value for
tray icon colorization — replace the expectation for the string 'colorizeIcons =
false;' with 'colorizeIcons = true;' so the test matches the PR's intent to
colorize tray icons; keep the existing 'drawerEnabled = false;' assertion as-is
and ensure the grep/awk target that captures the Tray block (the same block that
yields 'drawerEnabled' and 'colorizeIcons') remains unchanged.
Changes
Tray.colorizeIcons = falseso tray icons do not use the themed shader.ActiveWindow.colorizeIcons = falseso the active-window app icon keeps its source colors instead of grayscale.Testing
shellspec spec/noctalia_bar_spec.shnix-instantiate --parse config/noctalia/default.nix >/dev/nullgit diff --checkGenerated with Codex.