Uh oh!
There was an error while loading. Please reload this page.
feat(web): add theme gallery and custom palettes - #2501
Conversation
@an2tha is attempting to deploy a commit to the Ping Labs Team on Vercel. A member of the Team first needs to authorize it. |
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
| const isDark = | ||
| theme === "dark" || | ||
| theme === "dracula" || | ||
| theme === "one-dark" || | ||
| theme === "oled-dark" || | ||
| theme === "nord" || | ||
| theme === "tokyo-night" || | ||
| theme === "gruvbox-dark" || | ||
| (theme === "system" && getSystemDark()); |
There was a problem hiding this comment.
🟠 Highhooks/useTheme.ts:365
When theme === "custom", the isDark check (lines 365-373) omits "custom" so isDark evaluates to false. This skips adding the "dark" class (lines 380-381) and causes syncDesktopTheme to receive "light" instead of "dark". However, useTheme reports resolvedTheme: "dark" for custom themes (line 439), so the DOM classes and desktop sync are inconsistent with the hook's return value.
- const isDark =- theme === "dark" ||- theme === "dracula" ||- theme === "one-dark" ||- theme === "oled-dark" ||- theme === "nord" ||- theme === "tokyo-night" ||- theme === "gruvbox-dark" ||- (theme === "system" && getSystemDark());+ const isDark =+ theme === "dark" ||+ theme === "dracula" ||+ theme === "one-dark" ||+ theme === "oled-dark" ||+ theme === "nord" ||+ theme === "tokyo-night" ||+ theme === "gruvbox-dark" ||+ theme === "custom" ||+ (theme === "system" && getSystemDark());🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/hooks/useTheme.ts around lines 365-373:
When `theme === "custom"`, the `isDark` check (lines 365-373) omits `"custom"` so `isDark` evaluates to `false`. This skips adding the `"dark"` class (lines 380-381) and causes `syncDesktopTheme` to receive `"light"` instead of `"dark"`. However, `useTheme` reports `resolvedTheme: "dark"` for custom themes (line 439), so the DOM classes and desktop sync are inconsistent with the hook's return value.
Evidence trail:
apps/web/src/hooks/useTheme.ts lines 365-373 (isDark check omits 'custom'), line 375 (isBaseTheme check), lines 378-383 (else branch — dark class not added when isDark is false), line 404 (syncDesktopTheme receives 'light'), lines 436-440 (resolvedTheme returns 'dark' for custom theme with comment '// All custom themes are dark for now'). Reviewed at commit REVIEWED_COMMIT.
| window.removeEventListener("storage", handleStorage); | ||
| }; | ||
| } | ||
| const systemDark = useMemo(() => getSystemDark(), []); |
There was a problem hiding this comment.
🟡 Mediumhooks/useTheme.ts:434
systemDark is captured once on mount via useMemo(..., []) and never updates. When the system preference changes, resolvedTheme still returns the old value (e.g., "light" after switching to dark) until the component remounts. Consumers relying on resolvedTheme will see stale data.
- const systemDark = useMemo(() => getSystemDark(), []);+ const [systemDark, setSystemDark] = useState(() => getSystemDark());++ useEffect(() => {+ const mq = window.matchMedia(MEDIA_QUERY);+ const handleChange = () => setSystemDark(mq.matches);+ mq.addEventListener("change", handleChange);+ return () => mq.removeEventListener("change", handleChange);+ }, []);🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/hooks/useTheme.ts around line 434:
`systemDark` is captured once on mount via `useMemo(..., [])` and never updates. When the system preference changes, `resolvedTheme` still returns the old value (e.g., `"light"` after switching to dark) until the component remounts. Consumers relying on `resolvedTheme` will see stale data.
Evidence trail:
apps/web/src/hooks/useTheme.ts lines 434 (useMemo with empty deps), 436-440 (resolvedTheme depending on systemDark), 455-462 (useEffect handles visual theme change but doesn't update systemDark), 464 (stale resolvedTheme returned to consumers)
ApprovabilityVerdict: Needs human review New feature adding theme gallery and custom CSS capability. Multiple unresolved review comments identify bugs: custom CSS is unreachable from the UI, the 'custom' theme has incorrect dark mode handling, and resolvedTheme becomes stale on OS preference changes. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1f2724f. Configure here.
| } | ||
| if (theme === "custom" && customCSS) { | ||
| fullCSS += `\n.custom {\n${customCSS}\n}`; | ||
| } |
There was a problem hiding this comment.
Custom CSS never applied due to unreachable condition
High Severity
The customCSS is only injected into the page when theme === "custom", but the "custom" theme value is deliberately excluded from BUILT_IN_THEME_OPTIONS and has no selectable UI option. This means the Custom CSS textarea in ThemesPanel is entirely non-functional — users can type CSS but it will never be applied regardless of which theme they select.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1f2724f. Configure here.
| if (theme === "system") return systemDark ? "dark" : "light"; | ||
| if (theme === "light") return "light"; | ||
| return "dark"; // All custom themes are dark for now | ||
| }, [theme, systemDark]); |
There was a problem hiding this comment.
Stale resolvedTheme when OS theme changes
Medium Severity
systemDark is computed once on mount with useMemo(() => getSystemDark(), []) and never updates. When the OS switches between light/dark mode, the DOM updates correctly via the media query listener, but resolvedTheme remains stale. Components consuming resolvedTheme (e.g., ChatMarkdown for syntax highlighting themes, ChatView) won't re-render with the correct value.
Reviewed by Cursor Bugbot for commit 1f2724f. Configure here.
| }, | ||
| "dependencies": { | ||
| "@t3tools/monorepo": "." | ||
| }, |
There was a problem hiding this comment.
Self-referential dependency in root package.json
Low Severity
The root package.json (named @t3tools/monorepo) declares itself as a dependency with "@t3tools/monorepo": ".". This self-referential dependency serves no purpose and appears to be accidentally committed, possibly from a debugging session or tooling mishap.
Reviewed by Cursor Bugbot for commit 1f2724f. Configure here.
| }, []); | ||
| useEffect(() => { | ||
| applyTheme(theme, customCSS, true); | ||
| }, [theme, customCSS]); |
There was a problem hiding this comment.
Theme flash on load due to removed synchronous initialization
Medium Severity
The old code had a synchronous applyTheme(getStored()) at module load that read the theme from localStorage before React rendered, preventing a flash of wrong theme. This was removed with no replacement. Now the theme is only applied inside a useEffect (which runs after the first paint), and the settings are loaded asynchronously via hydrateClientSettings. Users with a non-system theme will see a brief flash of the default "system" theme on every page load until hydration completes.
Reviewed by Cursor Bugbot for commit 1f2724f. Configure here.
Note
Medium Risk
Adds new persisted settings (
theme,customCSS) and rewrites client-side theme application to inject CSS and toggle root classes, which can impact global styling across the app and desktop bridge theme syncing.Overview
Adds a dedicated Themes settings section with a gallery-style picker (including multiple built-in dark themes) and a Custom CSS editor, wired to persisted client settings.
Extends
ClientSettings/patch schema to storethemeandcustomCSS, updates tests to include these fields, and refactorsuseThemeto read/write via settings, apply theme classes deterministically, inject theme CSS into a<style>tag, and sync browser/desktop chrome colors. Also updates base CSS to use:root.darkfor dark variables and adds a new/settings/themesroute + sidebar nav item.Reviewed by Cursor Bugbot for commit 1f2724f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add theme gallery and custom CSS palette editor to settings
/settings/themespage with a visual grid of clickable theme previews (system, light, dark, dracula, one-dark, oled-dark, nord, tokyo-night, gruvbox-dark, custom) and removes theme selection from the General settings panel.ThemePreviewcards that render representative color tokens for each theme; selecting a theme applies it immediately via class toggling on<html>and CSS injection.themeandcustomCSSinClientSettingsSchema(with defaults"system"and"") instead of localStorage; theuseThemehook reads/writes viauseSettings/useUpdateSettings.:root.darkclass selector instead of a Tailwind@variant darkblock, which may affect any custom CSS that relied on the old variant.📊 Macroscope summarized 85c6016. 6 files reviewed, 2 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues