feat(web): add light theme with OS preference detection#24
Open
perler wants to merge 2 commits intoes6kr:mainfrom
Open
feat(web): add light theme with OS preference detection#24perler wants to merge 2 commits intoes6kr:mainfrom
perler wants to merge 2 commits intoes6kr:mainfrom
Conversation
Adds a GitHub Light color scheme alongside the existing dark theme. Theme defaults to OS preference (prefers-color-scheme) and can be manually toggled via a header button that cycles light/dark/system. Choice persists in localStorage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Work Server <work@server.local>
Signed-off-by: Work Server <work@server.local>
📝 WalkthroughWalkthroughThis pull request implements a comprehensive theme management system for the web application. It adds light theme CSS variables, creates a theme store with preference persistence and system theme detection, integrates theme management into the main layout with a toggle button, and updates the root HTML element with language and default theme attributes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Layout as +layout.svelte
participant Store as theme.ts Store
participant DOM as document
participant Browser as Browser API
participant Storage as localStorage
User->>Layout: Page loads
Layout->>Store: initTheme()
Store->>Storage: getStoredPreference()
activate Store
Store->>Browser: matchMedia('prefers-color-scheme')
Browser-->>Store: OS theme preference
Store->>DOM: applyTheme(effectiveTheme)
Store->>Browser: addEventListener (OS theme changes)
deactivate Store
User->>Layout: Click theme toggle button
Layout->>Store: toggleTheme()
Store->>Store: Update themePreference store
Store->>DOM: applyTheme(newTheme)
Store->>Storage: Persist preference to localStorage
Layout->>Layout: Re-render with new theme (reactive update)
DOM-->>User: Visual theme change applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/lib/stores/theme.ts`:
- Around line 20-57: The effectiveTheme derived store never reacts to OS changes
because initTheme only updates the DOM; create a readable/writable store (e.g.,
systemTheme) that holds getSystemTheme() and update it on the matchMedia
'change' event, then change effectiveTheme to derive from both themePreference
and systemTheme (e.g., effectiveTheme = derived([themePreference, systemTheme],
([$pref, $system]) => $pref === 'system' ? $system : $pref)); update initTheme
to initialize and update the systemTheme store on startup and in the mql change
handler, leave applyTheme and persistence logic intact.
In `@packages/web/src/routes/`+layout.svelte:
- Around line 331-367: The theme toggle button currently uses only title for
accessibility; update the button that calls toggleTheme (the element with
onclick={toggleTheme} and using {$themePreference} and {$effectiveTheme}) to
include an appropriate aria-label (e.g., "Toggle theme: {$themePreference}
({$effectiveTheme})") and add aria-pressed (true when dark, false when light) to
expose state to assistive tech; ensure the label reflects the current state and
keep the existing title if desired for hover tooltips.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
packages/web/src/app.csspackages/web/src/app.htmlpackages/web/src/lib/stores/theme.tspackages/web/src/routes/+layout.svelte
Comment on lines
+20
to
+57
| export const effectiveTheme = derived(themePreference, ($pref) => { | ||
| if ($pref === 'system') return getSystemTheme() | ||
| return $pref | ||
| }) | ||
|
|
||
| const applyTheme = (theme: EffectiveTheme) => { | ||
| if (typeof document !== 'undefined') { | ||
| document.documentElement.dataset.theme = theme | ||
| } | ||
| } | ||
|
|
||
| export const initTheme = () => { | ||
| // Apply initial theme | ||
| const pref = getStoredPreference() | ||
| const initial = pref === 'system' ? getSystemTheme() : pref | ||
| applyTheme(initial) | ||
|
|
||
| // Listen for OS theme changes | ||
| const mql = window.matchMedia('(prefers-color-scheme: light)') | ||
| const handleChange = () => { | ||
| let currentPref: ThemePreference = 'system' | ||
| themePreference.subscribe((v) => (currentPref = v))() | ||
| if (currentPref === 'system') { | ||
| applyTheme(getSystemTheme()) | ||
| } | ||
| } | ||
| mql.addEventListener('change', handleChange) | ||
|
|
||
| // Subscribe to store changes | ||
| const unsub = effectiveTheme.subscribe((theme) => { | ||
| applyTheme(theme) | ||
| }) | ||
|
|
||
| // Persist preference changes | ||
| const unsubPref = themePreference.subscribe((pref) => { | ||
| if (typeof localStorage !== 'undefined') { | ||
| localStorage.setItem('theme', pref) | ||
| } |
There was a problem hiding this comment.
effectiveTheme doesn’t update on OS theme changes.
When OS theme flips while preference is system, you update the DOM but the store value (used by the icon/tooltip) stays stale. Consider tracking system theme in a store and deriving from it.
🧩 Suggested fix
-import { writable, derived } from 'svelte/store'
+import { writable, derived } from 'svelte/store'
@@
export const themePreference = writable<ThemePreference>(getStoredPreference())
-export const effectiveTheme = derived(themePreference, ($pref) => {
- if ($pref === 'system') return getSystemTheme()
- return $pref
-})
+const systemTheme = writable<EffectiveTheme>(getSystemTheme())
+
+export const effectiveTheme = derived(
+ [themePreference, systemTheme],
+ ([$pref, $system]) => ($pref === 'system' ? $system : $pref)
+)
@@
export const initTheme = () => {
// Apply initial theme
const pref = getStoredPreference()
const initial = pref === 'system' ? getSystemTheme() : pref
+ systemTheme.set(getSystemTheme())
applyTheme(initial)
@@
const mql = window.matchMedia('(prefers-color-scheme: light)')
const handleChange = () => {
- let currentPref: ThemePreference = 'system'
- themePreference.subscribe((v) => (currentPref = v))()
- if (currentPref === 'system') {
- applyTheme(getSystemTheme())
- }
+ systemTheme.set(getSystemTheme())
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/stores/theme.ts` around lines 20 - 57, The
effectiveTheme derived store never reacts to OS changes because initTheme only
updates the DOM; create a readable/writable store (e.g., systemTheme) that holds
getSystemTheme() and update it on the matchMedia 'change' event, then change
effectiveTheme to derive from both themePreference and systemTheme (e.g.,
effectiveTheme = derived([themePreference, systemTheme], ([$pref, $system]) =>
$pref === 'system' ? $system : $pref)); update initTheme to initialize and
update the systemTheme store on startup and in the mql change handler, leave
applyTheme and persistence logic intact.
Comment on lines
+331
to
+367
| <button | ||
| onclick={toggleTheme} | ||
| class="p-1.5 rounded-md text-gh-text-secondary hover:text-gh-text hover:bg-gh-border-subtle transition-colors" | ||
| title="Theme: {$themePreference} ({$effectiveTheme})" | ||
| > | ||
| {#if $effectiveTheme === 'light'} | ||
| <!-- Sun icon --> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| stroke-width="2" | ||
| d="M12 3v1m0 16v1m9-9h-1M4 12H3m15.364 6.364l-.707-.707M6.343 6.343l-.707-.707m12.728 0l-.707.707M6.343 17.657l-.707.707M16 12a4 4 0 11-8 0 4 4 0 018 0z" | ||
| /> | ||
| </svg> | ||
| {:else if $themePreference === 'system'} | ||
| <!-- Monitor icon --> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| stroke-width="2" | ||
| d="M9.75 17L9 20l-1 1h8l-1-1-.75-3M3 13h18M5 17h14a2 2 0 002-2V5a2 2 0 00-2-2H5a2 2 0 00-2 2v10a2 2 0 002 2z" | ||
| /> | ||
| </svg> | ||
| {:else} | ||
| <!-- Moon icon --> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| stroke-width="2" | ||
| d="M20.354 15.354A9 9 0 018.646 3.646 9.003 9.003 0 0012 21a9.003 9.003 0 008.354-5.646z" | ||
| /> | ||
| </svg> | ||
| {/if} | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label for the icon-only theme toggle.
Screen readers won’t get a meaningful label from title. Add aria-label (and optionally aria-pressed) so the control is discoverable.
♿ Suggested fix
- <button
+ <button
onclick={toggleTheme}
class="p-1.5 rounded-md text-gh-text-secondary hover:text-gh-text hover:bg-gh-border-subtle transition-colors"
title="Theme: {$themePreference} ({$effectiveTheme})"
+ aria-label="Toggle theme"
>📝 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
| <button | |
| onclick={toggleTheme} | |
| class="p-1.5 rounded-md text-gh-text-secondary hover:text-gh-text hover:bg-gh-border-subtle transition-colors" | |
| title="Theme: {$themePreference} ({$effectiveTheme})" | |
| > | |
| {#if $effectiveTheme === 'light'} | |
| <!-- Sun icon --> | |
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| stroke-width="2" | |
| d="M12 3v1m0 16v1m9-9h-1M4 12H3m15.364 6.364l-.707-.707M6.343 6.343l-.707-.707m12.728 0l-.707.707M6.343 17.657l-.707.707M16 12a4 4 0 11-8 0 4 4 0 018 0z" | |
| /> | |
| </svg> | |
| {:else if $themePreference === 'system'} | |
| <!-- Monitor icon --> | |
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| stroke-width="2" | |
| d="M9.75 17L9 20l-1 1h8l-1-1-.75-3M3 13h18M5 17h14a2 2 0 002-2V5a2 2 0 00-2-2H5a2 2 0 00-2 2v10a2 2 0 002 2z" | |
| /> | |
| </svg> | |
| {:else} | |
| <!-- Moon icon --> | |
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| stroke-width="2" | |
| d="M20.354 15.354A9 9 0 018.646 3.646 9.003 9.003 0 0012 21a9.003 9.003 0 008.354-5.646z" | |
| /> | |
| </svg> | |
| {/if} | |
| </button> | |
| <button | |
| onclick={toggleTheme} | |
| class="p-1.5 rounded-md text-gh-text-secondary hover:text-gh-text hover:bg-gh-border-subtle transition-colors" | |
| title="Theme: {$themePreference} ({$effectiveTheme})" | |
| aria-label="Toggle theme" | |
| > | |
| {`#if` $effectiveTheme === 'light'} | |
| <!-- Sun icon --> | |
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| stroke-width="2" | |
| d="M12 3v1m0 16v1m9-9h-1M4 12H3m15.364 6.364l-.707-.707M6.343 6.343l-.707-.707m12.728 0l-.707.707M6.343 17.657l-.707.707M16 12a4 4 0 11-8 0 4 4 0 018 0z" | |
| /> | |
| </svg> | |
| {:else if $themePreference === 'system'} | |
| <!-- Monitor icon --> | |
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| stroke-width="2" | |
| d="M9.75 17L9 20l-1 1h8l-1-1-.75-3M3 13h18M5 17h14a2 2 0 002-2V5a2 2 0 00-2-2H5a2 2 0 00-2 2v10a2 2 0 002 2z" | |
| /> | |
| </svg> | |
| {:else} | |
| <!-- Moon icon --> | |
| <svg class="w-5 h-5" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| stroke-linecap="round" | |
| stroke-linejoin="round" | |
| stroke-width="2" | |
| d="M20.354 15.354A9 9 0 018.646 3.646 9.003 9.003 0 0012 21a9.003 9.003 0 008.354-5.646z" | |
| /> | |
| </svg> | |
| {/if} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/routes/`+layout.svelte around lines 331 - 367, The theme
toggle button currently uses only title for accessibility; update the button
that calls toggleTheme (the element with onclick={toggleTheme} and using
{$themePreference} and {$effectiveTheme}) to include an appropriate aria-label
(e.g., "Toggle theme: {$themePreference} ({$effectiveTheme})") and add
aria-pressed (true when dark, false when light) to expose state to assistive
tech; ensure the label reflects the current state and keep the existing title if
desired for hover tooltips.
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.
Summary
prefers-color-schememedia queryonMountcleanupTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style