fix(favicon): make favicon badge a0 instead of a1#2717
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated ReactiveFavicon's badgeColor to use the same theme color key ('a0') as accentColor, replacing the previous distinct key ('a1'). All other component functionality remains unchanged. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
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 `@js/app/packages/app/component/ReactiveFavicon.tsx`:
- Around line 15-16: badgeColor is a redundant reactive binding mirroring
accentColor; remove the duplicate useReactiveColorString('a0') assignment (the
badgeColor declaration) and change the call site in updateFavicon to pass
accentColor() for the badge color argument (and replace any other uses of
badgeColor with accentColor or accentColor()); ensure you remove the extra
signal subscription and any unused import/variable for badgeColor.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce1d6cbc-c0bf-4d4c-abc7-66f0305163f7
📒 Files selected for processing (1)
js/app/packages/app/component/ReactiveFavicon.tsx
| const accentColor = useReactiveColorString('a0'); | ||
| const badgeColor = useReactiveColorString('a1'); | ||
| const badgeColor = useReactiveColorString('a0'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant duplicate reactive color binding.
badgeColor now resolves to exactly the same reactive string as accentColor. Consider dropping the second binding and passing accentColor() for both arguments in updateFavicon (line 61) to avoid the extra signal subscriptions and the misleading impression that the two colors can diverge.
♻️ Proposed simplification
- const accentColor = useReactiveColorString('a0');
- const badgeColor = useReactiveColorString('a0');
+ const accentColor = useReactiveColorString('a0');
@@
- updateFavicon(accentColor(), badgeColor(), showNotificationBadge());
+ updateFavicon(accentColor(), accentColor(), showNotificationBadge());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/ReactiveFavicon.tsx` around lines 15 - 16,
badgeColor is a redundant reactive binding mirroring accentColor; remove the
duplicate useReactiveColorString('a0') assignment (the badgeColor declaration)
and change the call site in updateFavicon to pass accentColor() for the badge
color argument (and replace any other uses of badgeColor with accentColor or
accentColor()); ensure you remove the extra signal subscription and any unused
import/variable for badgeColor.
No description provided.