Add appearance settings with themes and accent colors#1550
Add appearance settings with themes and accent colors#1550juliusmarminge wants to merge 5 commits intomainfrom
Conversation
- Move theme control into a dedicated appearance settings panel - Add built-in themes, accent overrides, and desktop IPC sync - Migrate settings and update components to use unified appearance state
|
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)
Comment |
github-actions
bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function suppressTransitions(fn: () => void) { | ||
| document.documentElement.classList.add("no-transitions"); | ||
| fn(); | ||
| // Force reflow so the no-transitions class takes effect before removal | ||
| // oxlint-disable-next-line no-unused-expressions | ||
| document.documentElement.offsetHeight; | ||
| requestAnimationFrame(() => { | ||
| document.documentElement.classList.remove("no-transitions"); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟢 Low hooks/useAppearance.ts:29
In suppressTransitions, if fn() throws an exception, the requestAnimationFrame callback that removes the no-transitions class is never scheduled, leaving the class on document.documentElement permanently. This disables all CSS transitions for the rest of the session. Consider wrapping fn() in a try/finally to ensure the cleanup always runs.
function suppressTransitions(fn: () => void) {
document.documentElement.classList.add("no-transitions");
- fn();
- // Force reflow so the no-transitions class takes effect before removal
- // oxlint-disable-next-line no-unused-expressions
- document.documentElement.offsetHeight;
- requestAnimationFrame(() => {
- document.documentElement.classList.remove("no-transitions");
- });
+ try {
+ fn();
+ // Force reflow so the no-transitions class takes effect before removal
+ // oxlint-disable-next-line no-unused-expressions
+ document.documentElement.offsetHeight;
+ } finally {
+ requestAnimationFrame(() => {
+ document.documentElement.classList.remove("no-transitions");
+ });
+ }
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/hooks/useAppearance.ts around lines 29-38:
In `suppressTransitions`, if `fn()` throws an exception, the `requestAnimationFrame` callback that removes the `no-transitions` class is never scheduled, leaving the class on `document.documentElement` permanently. This disables all CSS transitions for the rest of the session. Consider wrapping `fn()` in a try/finally to ensure the cleanup always runs.
Evidence trail:
apps/web/src/hooks/useAppearance.ts lines 27-35 at REVIEWED_COMMIT: The `suppressTransitions` function adds `no-transitions` class, calls `fn()`, then schedules cleanup via `requestAnimationFrame`. There is no try/finally block, so if `fn()` throws, the cleanup code never executes.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Effect cleanup removes shared global style element
- Removed the useEffect cleanup that called removeThemeTokens(), since the shared global <style> element should persist across component unmounts and the FOUC-prevention code plus remaining mounted components' effects handle reapplication.
- ✅ Fixed: Theme migration unreachable without old settings key
- Moved the t3code:theme localStorage migration block before the OLD_SETTINGS_KEY early-return guard so it runs independently regardless of whether old-format settings exist.
- ✅ Fixed: Redundant appearance update before full settings reset
- Removed the redundant updateSettings() call that set appearance defaults immediately before resetSettings(), which already applies all defaults including the same appearance values.
Or push these changes by commenting:
@cursor push 57ebd82da6
Preview (57ebd82da6)
diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx
--- a/apps/web/src/components/settings/SettingsPanels.tsx
+++ b/apps/web/src/components/settings/SettingsPanels.tsx
@@ -424,7 +424,7 @@
export function useSettingsRestore(onRestored?: () => void) {
const settings = useSettings();
- const { updateSettings, resetSettings } = useUpdateSettings();
+ const { resetSettings } = useUpdateSettings();
const isGitWritingModelDirty = !Equal.equals(
settings.textGenerationModelSelection ?? null,
@@ -487,10 +487,9 @@
);
if (!confirmed) return;
- updateSettings({ colorMode: "system", activeThemeId: "t3code", accentHue: null });
resetSettings();
onRestored?.();
- }, [changedSettingLabels, onRestored, resetSettings, updateSettings]);
+ }, [changedSettingLabels, onRestored, resetSettings]);
return {
changedSettingLabels,
diff --git a/apps/web/src/hooks/useAppearance.ts b/apps/web/src/hooks/useAppearance.ts
--- a/apps/web/src/hooks/useAppearance.ts
+++ b/apps/web/src/hooks/useAppearance.ts
@@ -12,7 +12,7 @@
import { useCallback, useEffect, useMemo, useSyncExternalStore } from "react";
import type { ColorMode } from "@t3tools/contracts/settings";
import type { DesktopAppearance } from "@t3tools/contracts";
-import { applyThemeTokens, findThemeById, removeThemeTokens, BUILT_IN_THEMES } from "~/lib/themes";
+import { applyThemeTokens, findThemeById, BUILT_IN_THEMES } from "~/lib/themes";
import { useSettings, useUpdateSettings } from "./useSettings";
// ── Constants ────────────────────────────────────────────────────
@@ -138,8 +138,6 @@
// Sync to Electron
syncDesktopAppearance({ mode: colorMode, themeId: activeThemeId, accentHue });
-
- return () => removeThemeTokens();
}, [resolvedTheme, activeTheme, accentHue, colorMode, activeThemeId]);
const setColorMode = useCallback(
diff --git a/apps/web/src/hooks/useSettings.ts b/apps/web/src/hooks/useSettings.ts
--- a/apps/web/src/hooks/useSettings.ts
+++ b/apps/web/src/hooks/useSettings.ts
@@ -235,10 +235,9 @@
export function migrateLocalSettingsToServer(): void {
if (typeof window === "undefined") return;
- const raw = localStorage.getItem(OLD_SETTINGS_KEY);
- if (!raw) return;
-
- // Migrate old t3code:theme localStorage key to server colorMode
+ // Migrate old t3code:theme localStorage key to server colorMode.
+ // This runs independently of OLD_SETTINGS_KEY since the old useTheme hook
+ // wrote t3code:theme separately.
try {
const oldTheme = localStorage.getItem("t3code:theme");
if (oldTheme === "light" || oldTheme === "dark" || oldTheme === "system") {
@@ -250,6 +249,9 @@
// Best-effort — don't block startup
}
+ const raw = localStorage.getItem(OLD_SETTINGS_KEY);
+ if (!raw) return;
+
try {
const old = JSON.parse(raw);
if (!Predicate.isObject(old)) return;
apps/web/src/hooks/useAppearance.ts
Outdated
| // Sync to Electron | ||
| syncDesktopAppearance({ mode: colorMode, themeId: activeThemeId, accentHue }); | ||
|
|
||
| return () => removeThemeTokens(); |
There was a problem hiding this comment.
Effect cleanup removes shared global style element
High Severity
The useEffect cleanup calls removeThemeTokens(), which removes a shared global <style> element by ID. Since multiple components (ChatView, DiffPanel, DiffWorkerPoolProvider, ChatMarkdown, AppearanceSettingsPanel) each call useAppearance(), each gets its own effect cleanup. When any of these components unmounts (e.g., navigating from chat to settings), the cleanup deletes the shared style element. The root component's effect doesn't re-run because its dependencies haven't changed, so theme token CSS overrides are permanently lost until a dependency changes. Non-default themes and accent colors break on navigation.
Additional Locations (1)
| } | ||
| } catch { | ||
| // Best-effort — don't block startup | ||
| } |
There was a problem hiding this comment.
Theme migration unreachable without old settings key
Medium Severity
The t3code:theme localStorage migration is placed after if (!raw) return; on line 239, where raw reads from OLD_SETTINGS_KEY. Since t3code:theme was written independently by the now-deleted useTheme hook, users whose old settings were already migrated (removing OLD_SETTINGS_KEY) or who never had old-format settings will hit the early return. Their theme preference in t3code:theme is never migrated to the server's colorMode setting, silently losing the preference.
| if (!confirmed) return; | ||
|
|
||
| setTheme("system"); | ||
| updateSettings({ colorMode: "system", activeThemeId: "t3code", accentHue: null }); |
There was a problem hiding this comment.
Redundant appearance update before full settings reset
Low Severity
updateSettings({ colorMode: "system", activeThemeId: "t3code", accentHue: null }) is called immediately before resetSettings(), which internally calls updateSettings(DEFAULT_UNIFIED_SETTINGS) — containing the same appearance defaults. This fires two separate server RPCs with overlapping content when one suffices.
- Write theme CSS variables at the intermediate token level so Tailwind picks them up - Call `useAppearance()` before the root route can return early to satisfy Hooks rules
- Apply theme overrides as inline CSS variables - Add diff-specific tokens and color-blind diff palette - Update swatches and diff stats to use new semantic colors
github-actions
bot
added
size:XXL
- Replace single-theme settings with light and dark theme libraries - Persist custom theme definitions and add import, copy, duplicate, and reset flows - Update shared appearance contracts, UI, and tests
| const baseId = slugifyThemeId(`${themeDocument.id}-copy`); | ||
| const nextId = ensureUniqueThemeId(baseId, existingThemeIds); | ||
| const nextName = | ||
| nextId === `${themeDocument.id}-copy` | ||
| ? `${themeDocument.name} Copy` | ||
| : `${themeDocument.name} Copy ${nextId.split("-").at(-1)}`; |
There was a problem hiding this comment.
🟢 Low settings/appearance.logic.ts:57
The condition nextId === \${themeDocument.id}-copy`compares against the rawthemeDocument.id, but nextIdis derived fromslugifyThemeId(`${themeDocument.id}-copy`). If themeDocument.idcontains uppercase letters or spaces (e.g., "Codex Dark"), the comparison fails becausenextIdis "codex-dark-copy" while the right side is "Codex Dark-copy". This causesnextName` to incorrectly include the numeric suffix ("Copy 2") even when no deduplication was needed, instead of the plain "Copy".
| const baseId = slugifyThemeId(`${themeDocument.id}-copy`); | |
| const nextId = ensureUniqueThemeId(baseId, existingThemeIds); | |
| const nextName = | |
| nextId === `${themeDocument.id}-copy` | |
| ? `${themeDocument.name} Copy` | |
| : `${themeDocument.name} Copy ${nextId.split("-").at(-1)}`; | |
| const nextId = ensureUniqueThemeId(baseId, existingThemeIds); | |
| const nextName = | |
| nextId === baseId | |
| ? `${themeDocument.name} Copy` | |
| : `${themeDocument.name} Copy ${nextId.split("-").at(-1)}`; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/settings/appearance.logic.ts around lines 57-62:
The condition `nextId === \`${themeDocument.id}-copy\`` compares against the raw `themeDocument.id`, but `nextId` is derived from `slugifyThemeId(\`${themeDocument.id}-copy\``). If `themeDocument.id` contains uppercase letters or spaces (e.g., "Codex Dark"), the comparison fails because `nextId` is "codex-dark-copy" while the right side is "Codex Dark-copy". This causes `nextName` to incorrectly include the numeric suffix ("Copy 2") even when no deduplication was needed, instead of the plain "Copy".
Evidence trail:
apps/web/src/components/settings/appearance.logic.ts lines 8-14 (toKebabCase function), lines 16-18 (slugifyThemeId), lines 57-62 (createDuplicateTheme with the buggy comparison). The comparison on line 59 `nextId === \`${themeDocument.id}-copy\`` compares a slugified nextId against raw themeDocument.id without slugification.
| function parseHexColor(hex: string): Rgb { | ||
| const normalized = hex.replace("#", ""); | ||
| const expanded = | ||
| normalized.length === 8 ? normalized.slice(0, 6) : normalized.length === 6 ? normalized : ""; | ||
| if (!expanded) { | ||
| throw new Error(`Invalid hex color: ${hex}`); | ||
| } | ||
| return { | ||
| r: Number.parseInt(expanded.slice(0, 2), 16), | ||
| g: Number.parseInt(expanded.slice(2, 4), 16), | ||
| b: Number.parseInt(expanded.slice(4, 6), 16), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🟢 Low appearance/derive.ts:112
parseHexColor throws on valid 3-character shorthand hex colors like #fff. When normalized.length is 3, the expanded variable is set to empty string, causing the error throw. If any theme color uses shorthand notation, theme derivation will crash.
function parseHexColor(hex: string): Rgb {
const normalized = hex.replace("#", "");
const expanded =
- normalized.length === 8 ? normalized.slice(0, 6) : normalized.length === 6 ? normalized : "";
+ normalized.length === 8 ? normalized.slice(0, 6) : normalized.length === 6 ? normalized : normalized.length === 3 ? normalized.split("").map(c => c + c).join("") : "";
if (!expanded) {
throw new Error(`Invalid hex color: ${hex}`);
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/shared/src/appearance/derive.ts around lines 112-124:
`parseHexColor` throws on valid 3-character shorthand hex colors like `#fff`. When `normalized.length` is 3, the `expanded` variable is set to empty string, causing the error throw. If any theme color uses shorthand notation, theme derivation will crash.
Evidence trail:
packages/shared/src/appearance/derive.ts lines 112-123 at REVIEWED_COMMIT - The `parseHexColor` function only handles 6-char and 8-char hex colors. The ternary expression `normalized.length === 8 ? normalized.slice(0, 6) : normalized.length === 6 ? normalized : ""` sets `expanded` to empty string for 3-char inputs, triggering the error throw at line 117.
- Set built-in theme sidebar borders to `transparent` - Update token derivation tests to assert the new sidebar border value
1 participant
Closes #1284
Closes #1535
Closes #1545
Closes #418
Closes #464
Closes #254
Closes #1279
Closes #233
Closes #400
Closes #1183
Summary
useAppearanceflow that syncs theme state, applies tokens, and prevents FOUC.Testing
bun fmtbun lintbun typecheckbun run testHigh Contrast
Color Blind
Configurable Accent
Settings (NEEDS A LOT OF WORK STILL)
Note
Medium Risk
Moves theme control into server-authoritative settings and adds cross-surface token injection plus new Electron IPC, which could cause regressions in initial render (FOUC) and dark-mode synchronization across web/desktop.
Overview
Adds a dedicated Appearance settings page to manage
colorMode(system/light/dark), select a built-in theme, and override the accent hue, and wires it into the settings navigation/routes.Replaces the legacy
useThemeflow with a unifieduseAppearancehook that applies the.darkclass and injects theme CSS tokens (with a localStorage cache to reduce FOUC), updates consumers (chat markdown, diff panel, diff worker pool) to readresolvedThemefrom appearance, and updates “restore defaults” to reset the new appearance fields.Extends contracts and Electron bridging by adding
DesktopAppearanceplus a newdesktop:set-appearanceIPC (setThemedeprecated) so the web UI can sync richer appearance state to the desktop shell (currently used to setnativeTheme.themeSource).Written by Cursor Bugbot for commit f4ab7eb. This will update automatically on new commits. Configure here.
Note
Add appearance settings with theme and accent color controls
/settings/appearanceroute with a settings panel for color mode (light/dark/system), built-in theme selection, and accent color customization (presets + hex picker).useAppearanceto replace the deleteduseThemehook; it drives.darkclass and CSS custom property injection, caches values in localStorage to prevent FOUC, and syncs to Electron viadesktopBridge.setAppearance.themes.tsmodule defining token types, three built-in themes (t3code,high-contrast,color-blind), accent presets, and helpers to inject/remove CSS variables.ServerSettingswithcolorMode,activeThemeId, andaccentHuefields; migrates the legacyt3code:themelocalStorage key tocolorModeon startup.Macroscope summarized f4ab7eb.