feat(ui): add theme system with AMOLED Strix theme#2750
Open
RAFSuNX wants to merge 1 commit intoseerr-team:developfrom
Open
feat(ui): add theme system with AMOLED Strix theme#2750RAFSuNX wants to merge 1 commit intoseerr-team:developfrom
RAFSuNX wants to merge 1 commit intoseerr-team:developfrom
Conversation
Add a CSS custom property-based theming system that allows users to switch between the default dark theme and an AMOLED-optimized pure black theme with violet accents. The approach overrides Tailwind's gray and indigo color scales at the CSS variable level, so all existing utility classes automatically adapt to the active theme without any component-level branching. Hardcoded rgba() gradient values in inline styles have been replaced with CSS variable equivalents to ensure full theme coverage. New files: - ThemeContext provider with localStorage persistence - useTheme hook - Appearance settings page (profile + user routes) Modified files: - globals.css: CSS custom properties for both themes, scrollbar and sidebar gradient now use variables - tailwind.config.js: gray/indigo scales mapped to CSS variables - _app.tsx: ThemeProvider wrapper - _document.tsx: inline script to prevent theme flash on load - 11 components: replaced hardcoded rgba/hex gradient values with CSS variable equivalents Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a theme system with localStorage persistence and CSS variable-driven color management. It adds theme infrastructure (context, hooks, initialization), refactors hardcoded gradient overlays to use CSS variables, creates a user-facing appearance settings interface, and updates styling configuration to support multiple themes. Changes
Sequence DiagramsequenceDiagram
participant Browser
participant _document
participant _app
participant ThemeProvider
participant UserAppearanceSettings
participant ThemeContext
participant localStorage
Browser->>_document: Initial page load
_document->>localStorage: Read 'seerr-theme' from storage
_document->>Browser: Set data-theme attribute on <html>
Browser->>_app: Render app
_app->>ThemeProvider: Mount with children
ThemeProvider->>ThemeContext: Initialize theme state
ThemeProvider->>Browser: Render theme-aware UI
Browser->>UserAppearanceSettings: User visits settings
UserAppearanceSettings->>ThemeContext: Call useTheme()
ThemeContext-->>UserAppearanceSettings: Return current theme
UserAppearanceSettings->>Browser: Render theme selector
Browser->>UserAppearanceSettings: User clicks theme option
UserAppearanceSettings->>ThemeContext: Call setTheme(newThemeId)
ThemeContext->>localStorage: Update 'seerr-theme'
ThemeContext->>Browser: Set data-theme attribute
Browser->>Browser: Re-render with new theme CSS variables
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![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
🧹 Nitpick comments (3)
src/pages/_document.tsx (1)
18-22: Inline script correctly prevents theme flash on page load.The
dangerouslySetInnerHTMLstatic analysis warning is a false positive here—the script content is a static string literal, not user input, so there's no XSS risk.The script correctly uses the same storage key (
'seerr-theme') and attribute name ('data-theme') asThemeContext.tsx, ensuring consistency.Optional hardening: The script doesn't validate the localStorage value against known theme IDs before applying it. If a user manually sets an invalid value, the CSS will gracefully fall back to
:rootdefaults, but you could add validation for defense-in-depth:♻️ Optional: Add theme validation
- __html: `(function(){try{var t=localStorage.getItem('seerr-theme');if(t)document.documentElement.setAttribute('data-theme',t)}catch(e){}})()`, + __html: `(function(){try{var t=localStorage.getItem('seerr-theme');if(t&&(t==='default'||t==='amoled-strix'))document.documentElement.setAttribute('data-theme',t)}catch(e){}})()`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/_document.tsx` around lines 18 - 22, This inline script in _document.tsx is a false positive for dangerouslySetInnerHTML because the content is a static string; leave the script as-is to prevent theme flash (it uses the 'seerr-theme' key and sets 'data-theme'), or if you prefer defence-in-depth update the script to validate the stored value against your known theme IDs (the same whitelist used in ThemeContext.tsx, e.g. the allowedThemes/THEME_IDS constant) before calling setAttribute; alternatively add a short comment above the script explaining it is static and safe to suppress the lint warning.src/hooks/useTheme.ts (1)
1-6: Consider adding a guard for usage outside ThemeProvider.The hook assumes it's always used within a
ThemeProvider. While the provider is at the app root, a defensive check could provide clearer error messages during development if the context is accidentally used in isolation (e.g., tests, Storybook).♻️ Optional: Add context validation
import { ThemeContext } from '@app/context/ThemeContext'; import { useContext } from 'react'; -const useTheme = () => useContext(ThemeContext); +const useTheme = () => { + const context = useContext(ThemeContext); + if (!context) { + throw new Error('useTheme must be used within a ThemeProvider'); + } + return context; +}; export default useTheme;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useTheme.ts` around lines 1 - 6, The useTheme hook currently returns ThemeContext value without checking whether it's undefined; modify useTheme to call useContext(ThemeContext), store the result in a local variable, and if the result is null/undefined throw a clear error (e.g. "useTheme must be used within a ThemeProvider") so consumers (tests/Storybook) get a helpful message; finally return the validated context value. Reference: useTheme and ThemeContext (and mention ThemeProvider in the error text).src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx (1)
27-36: UseThemeId-keyed mapping for compile-time exhaustiveness.Typing
themeNamesasRecord<string, ...>allows incomplete mappings that won't surface until runtime. UseRecord<ThemeId, ...>instead so missing theme entries fail at compile time. TheThemeIdtype is already defined inThemeContextas'default' | 'amoled-strix'and matches the actual themes.This change also enables removing the optional chaining at lines 135-140 since type safety guarantees
infois always present.♻️ Proposed typing refactor
-import { THEMES } from '@app/context/ThemeContext'; +import { THEMES, type ThemeId } from '@app/context/ThemeContext'; - const themeNames: Record<string, { name: string; description: string }> = { + const themeNames: Record<ThemeId, { name: string; description: string }> = { default: { name: intl.formatMessage(messages.themeDefaultName), description: intl.formatMessage(messages.themeDefaultDescription), }, 'amoled-strix': { name: intl.formatMessage(messages.themeAmoledStrixName), description: intl.formatMessage(messages.themeAmoledStrixDescription), }, };Then update the optional chaining at lines 135–140:
- {info?.name} + {info.name}- {info?.description} + {info.description}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx` around lines 27 - 36, Change the themeNames map to be keyed by the ThemeId union so missing themes fail at compile time: replace Record<string, {name: string; description: string}> with Record<ThemeId, {name: string; description: string}>, import ThemeId from ThemeContext if needed, and keep the same entries for 'default' and 'amoled-strix'; then remove the optional chaining where you read this map (the code that currently uses themeNames?.[...] or info? checks) because ThemeId typing guarantees the entry exists. Ensure all usages of themeNames (e.g., where info is read) compile after switching to Record<ThemeId,...>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/context/ThemeContext.tsx`:
- Around line 53-57: The setTheme callback currently calls
localStorage.setItem(STORAGE_KEY, next) without protection; wrap that write in a
try/catch inside setTheme (the function that calls setThemeState(next) and
document.documentElement.setAttribute('data-theme', next)) so storage errors
(private mode/quota/disabled) won’t throw—on error, swallow or log it via
console.warn/processLogger and continue to apply the theme to state and
document; keep STORAGE_KEY and setThemeState usage intact and ensure
document.documentElement.setAttribute still runs even if storage fails.
---
Nitpick comments:
In `@src/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx`:
- Around line 27-36: Change the themeNames map to be keyed by the ThemeId union
so missing themes fail at compile time: replace Record<string, {name: string;
description: string}> with Record<ThemeId, {name: string; description: string}>,
import ThemeId from ThemeContext if needed, and keep the same entries for
'default' and 'amoled-strix'; then remove the optional chaining where you read
this map (the code that currently uses themeNames?.[...] or info? checks)
because ThemeId typing guarantees the entry exists. Ensure all usages of
themeNames (e.g., where info is read) compile after switching to
Record<ThemeId,...>.
In `@src/hooks/useTheme.ts`:
- Around line 1-6: The useTheme hook currently returns ThemeContext value
without checking whether it's undefined; modify useTheme to call
useContext(ThemeContext), store the result in a local variable, and if the
result is null/undefined throw a clear error (e.g. "useTheme must be used within
a ThemeProvider") so consumers (tests/Storybook) get a helpful message; finally
return the validated context value. Reference: useTheme and ThemeContext (and
mention ThemeProvider in the error text).
In `@src/pages/_document.tsx`:
- Around line 18-22: This inline script in _document.tsx is a false positive for
dangerouslySetInnerHTML because the content is a static string; leave the script
as-is to prevent theme flash (it uses the 'seerr-theme' key and sets
'data-theme'), or if you prefer defence-in-depth update the script to validate
the stored value against your known theme IDs (the same whitelist used in
ThemeContext.tsx, e.g. the allowedThemes/THEME_IDS constant) before calling
setAttribute; alternatively add a short comment above the script explaining it
is static and safe to suppress the lint warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 581cc65e-f9c9-489c-9780-3419b2d3b14d
📒 Files selected for processing (21)
src/components/Blocklist/index.tsxsrc/components/CollectionDetails/index.tsxsrc/components/Common/ImageFader/index.tsxsrc/components/Common/Modal/index.tsxsrc/components/IssueDetails/index.tsxsrc/components/IssueList/IssueItem/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/RequestCard/index.tsxsrc/components/RequestList/RequestItem/index.tsxsrc/components/TitleCard/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsxsrc/components/UserProfile/UserSettings/index.tsxsrc/context/ThemeContext.tsxsrc/hooks/useTheme.tssrc/pages/_app.tsxsrc/pages/_document.tsxsrc/pages/profile/settings/appearance.tsxsrc/pages/users/[userId]/settings/appearance.tsxsrc/styles/globals.csstailwind.config.js
Comment on lines
+53
to
+57
| const setTheme = useCallback((next: ThemeId) => { | ||
| setThemeState(next); | ||
| localStorage.setItem(STORAGE_KEY, next); | ||
| document.documentElement.setAttribute('data-theme', next); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify read path is guarded while write path is not
rg -n -C3 "localStorage\\.(getItem|setItem)" src/context/ThemeContext.tsxRepository: seerr-team/seerr
Length of output: 627
Guard localStorage.setItem in setTheme to avoid runtime crashes in storage-restricted environments.
The read path is protected with try-catch, but the write path is unguarded. This inconsistency can cause theme switching to fail when localStorage throws (private browsing, quota exceeded, disabled storage, etc.).
🛡️ Proposed reliability fix
const setTheme = useCallback((next: ThemeId) => {
setThemeState(next);
- localStorage.setItem(STORAGE_KEY, next);
+ try {
+ localStorage.setItem(STORAGE_KEY, next);
+ } catch {
+ // localStorage unavailable
+ }
document.documentElement.setAttribute('data-theme', next);
}, []);📝 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.
Suggested change
| const setTheme = useCallback((next: ThemeId) => { | |
| setThemeState(next); | |
| localStorage.setItem(STORAGE_KEY, next); | |
| document.documentElement.setAttribute('data-theme', next); | |
| }, []); | |
| const setTheme = useCallback((next: ThemeId) => { | |
| setThemeState(next); | |
| try { | |
| localStorage.setItem(STORAGE_KEY, next); | |
| } catch { | |
| // localStorage unavailable | |
| } | |
| document.documentElement.setAttribute('data-theme', next); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/context/ThemeContext.tsx` around lines 53 - 57, The setTheme callback
currently calls localStorage.setItem(STORAGE_KEY, next) without protection; wrap
that write in a try/catch inside setTheme (the function that calls
setThemeState(next) and document.documentElement.setAttribute('data-theme',
next)) so storage errors (private mode/quota/disabled) won’t throw—on error,
swallow or log it via console.warn/processLogger and continue to apply the theme
to state and document; keep STORAGE_KEY and setThemeState usage intact and
ensure document.documentElement.setAttribute still runs even if storage fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a CSS custom property-based theming system that allows users to switch between the default dark theme and an AMOLED-optimized pure black theme (AMOLED Strix) with violet accents, via a new Appearance settings page.
How it works
The core idea is zero component-level branching. Instead of rendering alternate UI trees per theme (which bloats the diff and hurts maintainability), this PR:
:root(default) and[data-theme='amoled-strix']gray-*andindigo-*scales intailwind.config.jsto reference these CSS variablesbg-gray-800,text-gray-400,border-gray-700, etc.) automatically adapts to the active themeThe only component changes are replacing ~11 hardcoded
rgba()gradient inline styles with CSS variable equivalents (e.g.,rgba(17, 24, 39, 0.47)→rgb(var(--gray-900) / 0.47)), since inline styles can't be reached by Tailwind.New files
src/context/ThemeContext.tsx— ThemeProvider withlocalStoragepersistence anddata-themeHTML attributesrc/hooks/useTheme.ts— simpleuseContextwrappersrc/components/UserProfile/UserSettings/UserAppearanceSettings/index.tsx— theme picker with visual previewssrc/pages/profile/settings/appearance.tsx+src/pages/users/[userId]/settings/appearance.tsx— page routesModified files
src/styles/globals.css— CSS custom properties for both themes; scrollbar and sidebar gradient now use variablestailwind.config.js— gray/indigo color scales mapped to CSS variablessrc/pages/_app.tsx—ThemeProviderwrappersrc/pages/_document.tsx— inline script to prevent theme flash on page loadrgba()/hex gradient values with CSS variable equivalentsAI Disclosure
This PR was authored with significant assistance from Claude Code (Anthropic's CLI tool). All code was reviewed and tested by the contributor. The approach and architecture decisions were made collaboratively.
How Has This Been Tested?
docker build --build-arg COMMIT_TAG=... -t seerr .) completes successfullylocalStoragedevelopbranch (CSS variables resolve to the same Tailwind v3 defaults)_document.tsxChecklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Release Notes
New Features
Style