feat(ui): Add dark mode support with light/dark/system themes #30
Conversation
- Darken all dark palette backgrounds by 30-40% for proper contrast - Add dark theme overrides for workflow-specific CSS variables - Fix all heading elements (h1, h2, h3) to use --palette-foreground - Replace hardcoded 'background: white' with --palette-bg-subtle - Fix workflow navigation step visibility with borders and opacity - Update all button backgrounds to use CSS variables - Fix Themis components: ArcSection, CourseOverview, ModuleCard, etc. - Fix Theia components: ExportButton, ExportConfigModal - Ensure all text colors use semantic CSS variables for theme adaptation All workflows (Metis, Themis, Tethys, Theia, Rhea) now have excellent contrast in both light and dark modes with no hardcoded colors. Resolves remaining dark mode contrast issues.
SummaryExcellent dark mode implementation with a well-architected dual-dimension palette system. The code is clean, type-safe, and accessibility-focused with WCAG AA compliance. The architecture is solid with proper SSR handling and localStorage persistence. Minor suggestions include adding tests, reducing CSS variable duplication, and enhancing error handling for edge cases. Key Strengths: Comprehensive documentation, zero breaking changes, proper accessibility attributes, SSR-safe implementation 🎯 Code Quality & Best PracticesStrengths1. Type Safety ✅
2. Architecture ✅
3. SSR Compatibility ✅
4. Documentation ✅
5. Accessibility ✅
Suggestions1. CSS Variable Duplication The generated CSS in [data-palette="metis"],
[data-palette="metis"][data-theme="light"] { ... }Consider simplifying to just use 2. Component Prop Syntax The ThemeSelector component uses Svelte 4 export syntax: <!-- Current: no props needed, which is fine -->Since you're using Svelte 5 runes ( 3. Color Naming Consistency In 4. Error Handling in CSS Generation The
🐛 Potential IssuesCritical IssuesNone found ✅ Medium Priority1. Initial Theme Flash The theme is applied in <!-- Add inline script in app.html to set theme before content loads -->
<script>
const theme = localStorage.getItem('theme-preference') || 'system';
const systemTheme = window.matchMedia('(prefers-color-scheme: dark)').matches ? 'dark' : 'light';
const effective = theme === 'system' ? systemTheme : theme;
document.body.setAttribute('data-theme', effective);
</script>2. Tab Index Pattern in ThemeSelector
This creates a tab trap pattern. Consider using roving tabindex with arrow key navigation (standard for radio groups):
3. Legacy Browser Support
Low Priority1. Missing Cleanup in Layout
(() => {
const cleanup = initSystemThemeDetection();
return cleanup; // Svelte 5 effect cleanup
});While not critical (event listener persists for app lifetime), it's best practice. 2. Palette Generation Script Truncation The diff shows const tethysPaletteDark = parsePaletteFile(join(palette
// ^^^ Appears incomplete⚡ Performance ConsiderationsStrengths1. CSS Custom Properties ✅
2. Efficient State Management ✅
3. Tree-Shakable Palettes ✅
Suggestions1. CSS File Size Generated CSS is ~121 lines added for dark themes. For 5 workflows × 2 themes × ~10 variables, this is reasonable but consider:
2. localStorage Subscription
store.subscribe(value => {
saveToLocalStorage(key, value, serialize);
});This writes the default value even if nothing changed. Consider: let initialized = false;
store.subscribe(value => {
if (!initialized) {
initialized = true;
return;
}
saveToLocalStorage(key, value, serialize);
});3. Event Listener Optimization The 🔒 Security ConcernsOverall AssessmentNo significant security issues found ✅ Observations1. localStorage Usage ✅
2. CSS Injection ✅
3. Type Safety ✅
Best Practices1. Consider CSP Compatibility If your app uses Content Security Policy, ensure inline styles are allowed or move to external CSS. Current implementation uses CSS variables in external files, so this should be fine. 2. localStorage Quota While theme preference is tiny (~20 bytes), consider handling try {
localStorage.setItem(key, serialized);
} catch (e) {
if (e.name === 'QuotaExceededError') {
// Handle gracefully - theme still works, just not persisted
console.warn('localStorage full, theme preference not saved');
}
}Already handled generically in ✅ Test CoverageCurrent StateNo tests found for new code ❌ Only existing test: Recommended Tests1. Theme Store Tests // src/lib/stores/themeStore.test.ts
import { describe, it, expect, beforeEach } from 'vitest';
import { get } from 'svelte/store';
import { themePreference, effectiveTheme, toggleTheme, cycleThemePreference } from './themeStore';
describe('themeStore', () => {
beforeEach(() => {
themePreference.set('system');
localStorage.clear();
});
it('defaults to system preference', () => {
expect(get(themePreference)).toBe('system');
});
it('toggleTheme switches between light and dark', () => {
themePreference.set('light');
toggleTheme();
expect(get(themePreference)).toBe('dark');
toggleTheme();
expect(get(themePreference)).toBe('light');
});
it('cycleThemePreference rotates through all options', () => {
themePreference.set('light');
cycleThemePreference();
expect(get(themePreference)).toBe('dark');
cycleThemePreference();
expect(get(themePreference)).toBe('system');
cycleThemePreference();
expect(get(themePreference)).toBe('light');
});
it('effectiveTheme resolves system to actual theme', () => {
// Mock matchMedia
window.matchMedia = () => ({ matches: true } as MediaQueryList);
themePreference.set('system');
expect(get(effectiveTheme)).toBe('dark');
});
});2. Palette Loader Tests // src/lib/utils/palette/paletteLoader.test.ts
import { describe, it, expect } from 'vitest';
import { getWorkflowPalette, generateCSSVariables } from './paletteLoader';
describe('paletteLoader', () => {
it('loads light palette by default', () => {
const palette = getWorkflowPalette('metis');
expect(palette.colours.background.primary.main).toBeDefined();
});
it('loads dark palette when specified', () => {
const lightPalette = getWorkflowPalette('metis', 'light');
const darkPalette = getWorkflowPalette('metis', 'dark');
expect(lightPalette).not.toEqual(darkPalette);
});
it('generates valid CSS variables', () => {
const vars = generateCSSVariables('metis', 'dark');
expect(vars['--palette-primary']).toMatch(/^#[0-9a-fA-F]{8}$/);
expect(vars['--palette-foreground']).toBeDefined();
});
});3. ThemeSelector Component Tests // src/lib/components/ui/ThemeSelector.test.ts
import { describe, it, expect } from 'vitest';
import { render, fireEvent } from '@testing-library/svelte';
import ThemeSelector from './ThemeSelector.svelte';
describe('ThemeSelector', () => {
it('renders all theme options', () => {
const { getByText } = render(ThemeSelector);
expect(getByText('Light')).toBeInTheDocument();
expect(getByText('Dark')).toBeInTheDocument();
expect(getByText('System')).toBeInTheDocument();
});
it('applies active class to selected theme', () => {
const { getByText } = render(ThemeSelector);
const lightButton = getByText('Light');
expect(lightButton).toHaveClass('active');
});
it('changes theme on click', async () => {
const { getByText } = render(ThemeSelector);
const darkButton = getByText('Dark');
await fireEvent.click(darkButton);
expect(darkButton).toHaveClass('active');
});
it('handles keyboard navigation', async () => {
const { getByText } = render(ThemeSelector);
const darkButton = getByText('Dark');
await fireEvent.keyDown(darkButton, { key: 'Enter' });
expect(darkButton).toHaveClass('active');
});
});4. Visual Regression Tests Consider adding visual regression tests with Playwright or Chromatic to ensure:
Testing Checklist
ConclusionThis is a high-quality PR that introduces dark mode in a maintainable, accessible, and performant way. The architecture is well-designed for future extensibility (custom themes, per-workflow themes, etc.). Recommended Actions Before Merge:
Ready to merge after addressing the keyboard navigation issue (#2 in Potential Issues). Great work on the comprehensive documentation and thorough implementation! 🎉 |
Overview
Implements complete dark mode support for Rhea with user-selectable themes (Light/Dark/System). All workflows maintain their unique colour identities while adapting seamlessly to both light and dark themes.
Task: Rhea 1a2b - Add Dark Mode to UI
Features
✅ User-selectable themes - Light, dark, or system preference
✅ System preference detection - Automatically respects OS dark mode setting
✅ Persistent preference - localStorage saves user choice across sessions
✅ Workflow-specific palettes - Each workflow maintains identity in both themes
✅ WCAG AA compliant - All colour combinations meet 4.5:1 contrast requirements
✅ Zero breaking changes - Fully backward compatible
✅ Professional UI - Text-only theme selector
Implementation
Architecture
Dual-dimension palette system:
Key Components
Changes
Testing
✅ Build successful - No errors
✅ All workflows tested - Metis, Themis, Tethys, Theia, Rhea
✅ Theme switching - Instant transitions
✅ System preference - Detects OS setting
✅ Persistence - Survives page reload
✅ Contrast verified - WCAG AA compliant
Documentation
docs/dev/features/dark-mode.mddocs/dev/ref/palettes/README.md