Themes#16
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds custom dashboard themes, updates theme selection and application to handle them, expands theme-specific CSS and styling, and changes the sidebar Settings item to stay active on nested routes. ChangesCustom Theme System
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/components/theme-provider.tsx (1)
3-141: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winWire Sonner to the dashboard theme context
apps/dashboard/src/components/ui/sonner.tsxstill readsuseThemefromnext-themes, butapps/dashboard/src/main.tsxonly provides the localThemeProvider. Without anext-themesprovider, the toaster will stay on the default/system theme instead of following the custom palettes.🤖 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 `@apps/dashboard/src/components/theme-provider.tsx` around lines 3 - 141, The Sonner toaster is still using the wrong theme source, so it won’t follow the dashboard’s custom palette state. Update the theme wiring in the Sonner component (the `sonner.tsx` `useTheme` usage) to consume the local `ThemeProvider`/`useTheme` from `theme-provider.tsx` instead of `next-themes`. Make sure the toaster reads the current `theme` value and maps custom themes through the existing `DARK_THEMES` and `LIGHT_THEMES` handling so it stays in sync with `ThemeProvider`.
🧹 Nitpick comments (4)
apps/dashboard/src/components/sidebar/app-sidebar.tsx (1)
133-133: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider boundary check to avoid matching sibling routes.
startsWith(settingsUrl)will also match any route withsettingsUrlas a literal prefix (e.g. a hypothetical/sites/$id/settings-generalroute), not just nested paths. Given current routes, this is unlikely to cause an issue, but a trailing-slash check would be more robust.💡 Optional boundary-safe check
- isActive={pathname.startsWith(settingsUrl)} + isActive={pathname === settingsUrl || pathname.startsWith(`${settingsUrl}/`)}🤖 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 `@apps/dashboard/src/components/sidebar/app-sidebar.tsx` at line 133, The active-state check in the app sidebar is too broad because `pathname.startsWith(settingsUrl)` can match sibling routes that only share the same prefix. Update the `isActive` logic in `app-sidebar` to use a boundary-safe path check for `settingsUrl`, so it only matches the settings route and its nested children; keep the change localized to the `isActive` prop where the current `pathname.startsWith(settingsUrl)` is used.apps/dashboard/src/styles.css (2)
93-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComment understates the light-theme set.
This says
.theme-lightis for "Citrus Sherbet" only, but there are 6 light palettes (citrus-sherbet,desert-khaki,sunrise,hanami,cotton-candy,mint-apple), matchingLIGHT_THEMESintheme-provider.tsx. Same drift as the dark-theme comment there — worth updating both together.🤖 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 `@apps/dashboard/src/styles.css` around lines 93 - 97, Update the theme documentation comment in styles.css so `.theme-light` reflects all light palettes, not just Citrus Sherbet; it should match the `LIGHT_THEMES` set used by `ThemeProvider` in theme-provider.tsx. Also check the corresponding dark-theme note in the same area and keep both comments aligned with the actual palette lists so the marker-class behavior is described accurately.
9-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCustom-variant expansion is correct; Stylelint warning here is a known false positive.
@custom-variantis Tailwind v4 syntax that Stylelint's CSS parser doesn't recognize (scss/at-rule-no-unknown). Since this pattern already existed and is just being extended to include.theme-dark, consider adding@custom-variantto Stylelint'signoreAtRulesto silence this recurring false positive.🤖 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 `@apps/dashboard/src/styles.css` around lines 9 - 12, The `@custom-variant` in `styles.css` is valid Tailwind v4 syntax, but Stylelint flags it as an unknown at-rule. Update the Stylelint configuration to add `custom-variant` to `ignoreAtRules` so the `dark` variant definition on `.theme-dark` is no longer reported as a false positive. Keep the existing `@custom-variant dark` usage unchanged and make the lint rule exemption apply globally.Source: Linters/SAST tools
apps/dashboard/src/components/theme-provider.tsx (1)
88-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale comment: only lists a handful of the actual dark themes.
The block comment above
DARK_THEMES/.theme-darkusage (and the mirrored comment instyles.csslines 93-97) says custom dark themes are "Crimson Moon, Sepia, Midnight Blurple, Forest, Dusk" butDARK_THEMESnow contains 13 entries (alsoblurple-twilight,dusk,aurora,sunset,mars,retro-storm,under-the-sea,strawberry-lemonade,neon-nights). Worth updating the comment to avoid confusing future maintainers about which themes are dark vs. light.🤖 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 `@apps/dashboard/src/components/theme-provider.tsx` around lines 88 - 115, Update the stale theme documentation in theme-provider so it matches the current DARK_THEMES list and the mirrored note in styles.css. The comment around the theme handling in theme-provider and the CSS comment should be revised to mention all current dark themes, using the existing DARK_THEMES and theme-dark usage as the source of truth, so maintainers are not misled by the outdated short list.
🤖 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 `@apps/dashboard/src/components/theme-toggle.tsx`:
- Around line 45-67: The theme selector is using a plain DropdownMenuItem with a
manual aria-checked state, but the checked behavior should be handled by radio
menu semantics. Update renderItem in theme-toggle.tsx to use
DropdownMenuRadioGroup with DropdownMenuRadioItem instead of DropdownMenuItem,
and move the selected state from aria-checked/onClick into the radio item’s
value/change handling while keeping the active styling and Check icon tied to
the current theme.
---
Outside diff comments:
In `@apps/dashboard/src/components/theme-provider.tsx`:
- Around line 3-141: The Sonner toaster is still using the wrong theme source,
so it won’t follow the dashboard’s custom palette state. Update the theme wiring
in the Sonner component (the `sonner.tsx` `useTheme` usage) to consume the local
`ThemeProvider`/`useTheme` from `theme-provider.tsx` instead of `next-themes`.
Make sure the toaster reads the current `theme` value and maps custom themes
through the existing `DARK_THEMES` and `LIGHT_THEMES` handling so it stays in
sync with `ThemeProvider`.
---
Nitpick comments:
In `@apps/dashboard/src/components/sidebar/app-sidebar.tsx`:
- Line 133: The active-state check in the app sidebar is too broad because
`pathname.startsWith(settingsUrl)` can match sibling routes that only share the
same prefix. Update the `isActive` logic in `app-sidebar` to use a boundary-safe
path check for `settingsUrl`, so it only matches the settings route and its
nested children; keep the change localized to the `isActive` prop where the
current `pathname.startsWith(settingsUrl)` is used.
In `@apps/dashboard/src/components/theme-provider.tsx`:
- Around line 88-115: Update the stale theme documentation in theme-provider so
it matches the current DARK_THEMES list and the mirrored note in styles.css. The
comment around the theme handling in theme-provider and the CSS comment should
be revised to mention all current dark themes, using the existing DARK_THEMES
and theme-dark usage as the source of truth, so maintainers are not misled by
the outdated short list.
In `@apps/dashboard/src/styles.css`:
- Around line 93-97: Update the theme documentation comment in styles.css so
`.theme-light` reflects all light palettes, not just Citrus Sherbet; it should
match the `LIGHT_THEMES` set used by `ThemeProvider` in theme-provider.tsx. Also
check the corresponding dark-theme note in the same area and keep both comments
aligned with the actual palette lists so the marker-class behavior is described
accurately.
- Around line 9-12: The `@custom-variant` in `styles.css` is valid Tailwind v4
syntax, but Stylelint flags it as an unknown at-rule. Update the Stylelint
configuration to add `custom-variant` to `ignoreAtRules` so the `dark` variant
definition on `.theme-dark` is no longer reported as a false positive. Keep the
existing `@custom-variant dark` usage unchanged and make the lint rule exemption
apply globally.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68162ea7-c3c2-46e7-b341-acaf161f7302
📒 Files selected for processing (4)
apps/dashboard/src/components/sidebar/app-sidebar.tsxapps/dashboard/src/components/theme-provider.tsxapps/dashboard/src/components/theme-toggle.tsxapps/dashboard/src/styles.css
…or theme selection
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Style / UI