diff --git a/packages/review-editor/components/ReviewHeaderMenu.tsx b/packages/review-editor/components/ReviewHeaderMenu.tsx index ffdaf002b..0f0b9eabe 100644 --- a/packages/review-editor/components/ReviewHeaderMenu.tsx +++ b/packages/review-editor/components/ReviewHeaderMenu.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react'; +import React from 'react'; import { ActionMenu, ActionMenuDivider, @@ -6,6 +6,8 @@ import { ActionMenuSectionLabel, } from '@plannotator/ui/components/ActionMenu'; import { useTheme } from '@plannotator/ui/components/ThemeProvider'; +import { THEME_MODES } from '@plannotator/ui/components/themeModes'; +import { isThemeModeAvailable } from '@plannotator/ui/utils/themeRegistry'; import { MenuVersionSection } from '@plannotator/ui/components/MenuVersionSection'; import { ReviewAgentsIcon } from '@plannotator/ui/components/ReviewAgentsIcon'; import { TextShimmer } from '@plannotator/ui/components/TextShimmer'; @@ -44,15 +46,13 @@ export const ReviewHeaderMenu: React.FC = ({ origin, isWSL = false, }) => { - const { theme, resolvedMode, setTheme } = useTheme(); - const activeTheme = useMemo<'light' | 'dark'>(() => { - return theme === 'system' ? resolvedMode : theme; - }, [resolvedMode, theme]); + const { theme, setTheme, colorTheme } = useTheme(); const showUpdateDot = !!updateInfo?.updateAvailable && !updateInfo.dismissed; return ( ( - ))} + {THEME_MODES.map(({ id, label, Icon }) => { + const available = isThemeModeAvailable(colorTheme, id); + return ( + + ); + })} @@ -213,19 +220,6 @@ const ExportIcon = () => ( ); -const SunIcon = () => ( - - - - -); - -const MoonIcon = () => ( - - - -); - const FileTreeMenuIcon = () => ( diff --git a/packages/ui/HANDOFF.md b/packages/ui/HANDOFF.md index f2ef292d7..c39e031ed 100644 --- a/packages/ui/HANDOFF.md +++ b/packages/ui/HANDOFF.md @@ -194,6 +194,7 @@ We deliberately did **not** restructure the exports map in this PR (move-don't-r | `components/CommentPopover` | Anchor capture + comment entry. Ask-AI UI renders only if you pass `onAskAI`. | | `components/AnnotationPanel` | Renders from your annotation state; no fetches of its own. | | `components/ThemeProvider` | Color-mode context. | +| `theme-modes` (`THEME_MODES`, `Mode`) | The supported Light/Dark/System catalog and mode type. `Mode` also remains exported from `components/ThemeProvider` for compatibility with existing consumers. | | `components/ImageThumbnail` / `getImageSrc` | Routes through `imageSrcResolver`. | | `components/AttachmentsButton` | Routes through `uploadTransport`. | | Seam-backed hooks: `useAnnotationHighlighter`, `useAnnotationDraft`, `useCodeAnnotationDraft`, `useExternalAnnotations`, `useFileBrowser` | Their network access goes through the seams in the catalog above. | diff --git a/packages/ui/components/ActionMenu.test.tsx b/packages/ui/components/ActionMenu.test.tsx new file mode 100644 index 000000000..d6b56f9bf --- /dev/null +++ b/packages/ui/components/ActionMenu.test.tsx @@ -0,0 +1,53 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { ActionMenu } from './ActionMenu'; + +const hasDom = typeof document !== 'undefined'; +let root: Root | null = null; +let host: HTMLElement | null = null; + +async function renderMenu(panelWidth?: 'default' | 'wide'): Promise { + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + await act(async () => { + root!.render( + } + > + {() =>
Panel
} + , + ); + }); + const trigger = host.querySelector('button'); + if (!trigger) throw new Error('ActionMenu trigger did not render'); + await act(async () => trigger.click()); + const panel = Array.from(host.querySelectorAll('div')).find(element => + element.textContent === 'Panel' && element.classList.contains('absolute') + ); + if (!panel) throw new Error('ActionMenu panel did not open'); + return panel; +} + +describe('ActionMenu panel geometry', () => { + afterEach(async () => { + if (root) await act(async () => root!.unmount()); + root = null; + host?.remove(); + host = null; + }); + + test.skipIf(!hasDom)('keeps the default width for unrelated consumers', async () => { + const panel = await renderMenu(); + expect(panel.classList.contains('w-56')).toBe(true); + expect(panel.classList.contains('w-64')).toBe(false); + }); + + test.skipIf(!hasDom)('offers an opt-in wide panel for three-mode pickers', async () => { + const panel = await renderMenu('wide'); + expect(panel.classList.contains('w-64')).toBe(true); + expect(panel.classList.contains('w-56')).toBe(false); + }); +}); diff --git a/packages/ui/components/ActionMenu.tsx b/packages/ui/components/ActionMenu.tsx index 3ca1f8883..cb57f6dd8 100644 --- a/packages/ui/components/ActionMenu.tsx +++ b/packages/ui/components/ActionMenu.tsx @@ -3,6 +3,7 @@ import React, { useEffect, useRef, useState } from 'react'; interface ActionMenuProps { className?: string; panelClassName?: string; + panelWidth?: 'default' | 'wide'; renderTrigger: (props: { isOpen: boolean; toggleMenu: () => void; @@ -13,6 +14,7 @@ interface ActionMenuProps { export const ActionMenu: React.FC = ({ className, panelClassName, + panelWidth = 'default', renderTrigger, children, }) => { @@ -50,7 +52,9 @@ export const ActionMenu: React.FC = ({ })} {isOpen && ( -
+
{children({ closeMenu: () => setIsOpen(false) })}
)} diff --git a/packages/ui/components/ModeToggle.tsx b/packages/ui/components/ModeToggle.tsx index 8119d4fc3..e35baeae8 100644 --- a/packages/ui/components/ModeToggle.tsx +++ b/packages/ui/components/ModeToggle.tsx @@ -1,8 +1,10 @@ import React, { useState, useRef, useEffect } from 'react'; import { useTheme } from './ThemeProvider'; +import { THEME_MODES } from './themeModes'; +import { isThemeModeAvailable } from '../utils/themeRegistry'; export function ModeToggle() { - const { theme, setTheme } = useTheme(); + const { theme, setTheme, colorTheme } = useTheme(); const [isOpen, setIsOpen] = useState(false); const dropdownRef = useRef(null); @@ -48,19 +50,26 @@ export function ModeToggle() { {isOpen && (
- {(['light', 'dark', 'system'] as const).map((t) => ( - - ))} + {THEME_MODES.map(({ id, label }) => { + const available = isThemeModeAvailable(colorTheme, id); + return ( + + ); + })}
)}
diff --git a/packages/ui/components/PlanHeaderMenu.tsx b/packages/ui/components/PlanHeaderMenu.tsx index 2855ecfde..2abb6bc10 100644 --- a/packages/ui/components/PlanHeaderMenu.tsx +++ b/packages/ui/components/PlanHeaderMenu.tsx @@ -6,7 +6,8 @@ import { ActionMenuSectionLabel, } from './ActionMenu'; import { useTheme } from './ThemeProvider'; -import { SunIcon, MoonIcon, SystemIcon } from './icons/themeIcons'; +import { THEME_MODES } from './themeModes'; +import { isThemeModeAvailable } from '../utils/themeRegistry'; import { ReviewAgentsIcon } from './ReviewAgentsIcon'; import { MenuVersionSection } from './MenuVersionSection'; import { TextShimmer } from './TextShimmer'; @@ -58,7 +59,7 @@ export const PlanHeaderMenu: React.FC = ({ bearConfigured, octarineConfigured, }) => { - const { theme, setTheme } = useTheme(); + const { theme, setTheme, colorTheme } = useTheme(); const showUpdateDot = !!updateInfo?.updateAvailable && !updateInfo.dismissed; @@ -67,6 +68,7 @@ export const PlanHeaderMenu: React.FC = ({ return ( ( - ))} + {THEME_MODES.map(({ id, label, Icon }) => { + const available = isThemeModeAvailable(colorTheme, id); + return ( + + ); + })} @@ -296,4 +305,3 @@ const NoteIcon = () => ( ); - diff --git a/packages/ui/components/ThemeProvider.test.tsx b/packages/ui/components/ThemeProvider.test.tsx new file mode 100644 index 000000000..816d1afcc --- /dev/null +++ b/packages/ui/components/ThemeProvider.test.tsx @@ -0,0 +1,232 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import React, { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { resetStorageBackend, setStorageBackend } from '../utils/storage'; +import { BUILT_IN_THEMES } from '../utils/themeRegistry'; +import { THEME_MODES, isThemeMode, parseThemeMode } from './themeModes'; +import { ThemeProvider, useTheme } from './ThemeProvider'; +import { ThemeTab } from './ThemeTab'; + +const hasDom = typeof document !== 'undefined'; + +let root: Root | null = null; +let host: HTMLElement | null = null; +let currentTheme: ReturnType | null = null; +let stored = new Map(); +let originalMatchMediaDescriptor: PropertyDescriptor | undefined; + +function Probe() { + currentTheme = useTheme(); + return null; +} + +function themeState(): ReturnType { + if (!currentTheme) throw new Error('ThemeProvider is not mounted'); + return currentTheme; +} + +function installMatchMedia(initialMatches: boolean) { + let matches = initialMatches; + const listeners = new Set<(event: MediaQueryListEvent) => void>(); + const media = '(prefers-color-scheme: light)'; + const query = { + get matches() { + return matches; + }, + media, + onchange: null, + addListener(listener: (event: MediaQueryListEvent) => void) { + listeners.add(listener); + }, + removeListener(listener: (event: MediaQueryListEvent) => void) { + listeners.delete(listener); + }, + addEventListener(_type: string, listener: EventListenerOrEventListenerObject) { + listeners.add(listener as (event: MediaQueryListEvent) => void); + }, + removeEventListener(_type: string, listener: EventListenerOrEventListenerObject) { + listeners.delete(listener as (event: MediaQueryListEvent) => void); + }, + dispatchEvent() { + return true; + }, + } as MediaQueryList; + + Object.defineProperty(window, 'matchMedia', { + configurable: true, + value: () => query, + }); + + return { + setMatches(nextMatches: boolean) { + matches = nextMatches; + const event = { matches, media } as MediaQueryListEvent; + for (const listener of listeners) listener(event); + }, + }; +} + +async function mountTheme(children?: React.ReactNode): Promise { + host = document.createElement('div'); + document.body.appendChild(host); + root = createRoot(host); + await act(async () => { + root!.render( + + + {children} + , + ); + }); +} + +async function unmountTheme(): Promise { + if (root) { + await act(async () => root!.unmount()); + } + root = null; + host?.remove(); + host = null; + currentTheme = null; +} + +describe('theme mode catalog', () => { + test('is the one Light/Dark/System source and parses persisted input', () => { + expect(THEME_MODES.map(({ id }) => id)).toEqual(['light', 'dark', 'system']); + expect(isThemeMode('system')).toBe(true); + expect(isThemeMode('sepia')).toBe(false); + expect(parseThemeMode('light', 'dark')).toBe('light'); + expect(parseThemeMode('sepia', 'dark')).toBe('dark'); + }); +}); + +describe('ThemeProvider', () => { + beforeEach(() => { + if (hasDom) { + originalMatchMediaDescriptor = Object.getOwnPropertyDescriptor(window, 'matchMedia'); + } + stored = new Map(); + setStorageBackend({ + getItem: key => stored.get(key) ?? null, + setItem: (key, value) => { + stored.set(key, value); + }, + removeItem: key => { + stored.delete(key); + }, + }); + }); + + afterEach(async () => { + if (hasDom) { + await unmountTheme(); + document.documentElement.className = ''; + if (originalMatchMediaDescriptor) { + Object.defineProperty(window, 'matchMedia', originalMatchMediaDescriptor); + } else { + Reflect.deleteProperty(window, 'matchMedia'); + } + originalMatchMediaDescriptor = undefined; + } + resetStorageBackend(); + }); + + test.skipIf(!hasDom)('persists System and follows live OS changes across reloads', async () => { + stored.set('plannotator-theme', 'system'); + stored.set('plannotator-color-theme', 'plannotator'); + const media = installMatchMedia(false); + + await mountTheme(); + expect(themeState().mode).toBe('system'); + expect(themeState().preferredMode).toBe('dark'); + expect(themeState().resolvedMode).toBe('dark'); + + await act(async () => media.setMatches(true)); + expect(themeState().mode).toBe('system'); + expect(themeState().preferredMode).toBe('light'); + expect(themeState().resolvedMode).toBe('light'); + expect(document.documentElement.classList.contains('light')).toBe(true); + expect(stored.get('plannotator-theme')).toBe('system'); + + await unmountTheme(); + media.setMatches(false); + await mountTheme(); + expect(themeState().mode).toBe('system'); + expect(themeState().resolvedMode).toBe('dark'); + }); + + test.skipIf(!hasDom)('keeps System coherent and normalizes explicit modes for constrained palettes', async () => { + stored.set('plannotator-theme', 'system'); + stored.set('plannotator-color-theme', 'andromeeda'); + const media = installMatchMedia(true); + + await mountTheme(); + expect(themeState().mode).toBe('system'); + expect(themeState().preferredMode).toBe('light'); + expect(themeState().resolvedMode).toBe('dark'); + expect(document.documentElement.classList.contains('theme-andromeeda')).toBe(true); + expect(document.documentElement.classList.contains('light')).toBe(false); + + await act(async () => media.setMatches(false)); + expect(themeState().mode).toBe('system'); + expect(themeState().preferredMode).toBe('dark'); + expect(themeState().resolvedMode).toBe('dark'); + + await act(async () => themeState().setColorTheme('kanagawa-lotus')); + expect(themeState().mode).toBe('system'); + expect(themeState().resolvedMode).toBe('light'); + expect(stored.get('plannotator-theme')).toBe('system'); + + await act(async () => themeState().setMode('dark')); + expect(themeState().mode).toBe('light'); + expect(themeState().resolvedMode).toBe('light'); + expect(stored.get('plannotator-theme')).toBe('light'); + + await act(async () => themeState().setColorTheme('andromeeda')); + expect(themeState().mode).toBe('dark'); + expect(themeState().resolvedMode).toBe('dark'); + expect(stored.get('plannotator-theme')).toBe('dark'); + + await act(async () => { + themeState().setColorTheme('plannotator'); + themeState().setMode('light'); + themeState().setColorTheme('andromeeda'); + themeState().setMode('light'); + }); + expect(themeState().mode).toBe('dark'); + expect(themeState().resolvedMode).toBe('dark'); + expect(stored.get('plannotator-theme')).toBe('dark'); + }); + + test.skipIf(!hasDom)('repairs invalid persisted modes before exposing state', async () => { + stored.set('plannotator-theme', 'sepia'); + stored.set('plannotator-color-theme', 'andromeeda'); + installMatchMedia(true); + + await mountTheme(); + expect(themeState().mode).toBe('dark'); + expect(themeState().resolvedMode).toBe('dark'); + expect(stored.get('plannotator-theme')).toBe('dark'); + }); + + test.skipIf(!hasDom)('previews a constrained palette using the mode it actually renders', async () => { + stored.set('plannotator-theme', 'system'); + stored.set('plannotator-color-theme', 'tinacious'); + installMatchMedia(false); + + await mountTheme(); + expect(themeState().preferredMode).toBe('dark'); + expect(themeState().resolvedMode).toBe('light'); + + const paletteButton = Array.from(host!.querySelectorAll('button')).find(button => + button.textContent?.includes('Tinacious') + ); + if (!paletteButton) throw new Error('Tinacious palette preview did not render'); + const swatches = paletteButton.querySelectorAll('.rounded-full'); + const palette = BUILT_IN_THEMES.find(theme => theme.id === 'tinacious'); + if (!palette) throw new Error('Tinacious palette is not registered'); + + expect(swatches[3]?.style.backgroundColor).toBe(palette.colors.light.background); + expect(swatches[3]?.style.backgroundColor).not.toBe(palette.colors.dark.background); + }); +}); diff --git a/packages/ui/components/ThemeProvider.tsx b/packages/ui/components/ThemeProvider.tsx index 6fc8c096d..210993471 100644 --- a/packages/ui/components/ThemeProvider.tsx +++ b/packages/ui/components/ThemeProvider.tsx @@ -1,8 +1,15 @@ -import { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react'; +import { createContext, useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'; import { storage } from '../utils/storage'; -import { BUILT_IN_THEMES, type ThemeInfo } from '../utils/themeRegistry'; +import { + BUILT_IN_THEMES, + normalizeThemeMode, + resolveThemeMode, + type ThemeInfo, +} from '../utils/themeRegistry'; +import { parseThemeMode, type Mode } from './themeModes'; -export type Mode = 'dark' | 'light' | 'system'; +// Kept here because published consumers already import Mode from ThemeProvider. +export type { Mode } from './themeModes'; type ThemeProviderState = { // Mode (dark/light/system) — backward-compatible with old "theme" API @@ -10,6 +17,7 @@ type ThemeProviderState = { setTheme: (mode: Mode) => void; mode: Mode; setMode: (mode: Mode) => void; + preferredMode: 'dark' | 'light'; resolvedMode: 'dark' | 'light'; // Color theme (palette) colorTheme: string; @@ -22,29 +30,18 @@ const ThemeProviderContext = createContext({ setTheme: () => null, mode: 'dark', setMode: () => null, + preferredMode: 'dark', resolvedMode: 'dark', colorTheme: 'plannotator', setColorTheme: () => null, availableThemes: BUILT_IN_THEMES, }); -/** Resolve the class string for a theme + mode combination */ -function resolveThemeClasses(themeId: string, effectiveMode: 'dark' | 'light'): string { - const themeInfo = BUILT_IN_THEMES.find(t => t.id === themeId); - const modeSupport = themeInfo?.modeSupport ?? 'both'; - - let applyLight = effectiveMode === 'light'; - if (modeSupport === 'dark-only') applyLight = false; - if (modeSupport === 'light-only') applyLight = true; - - return `theme-${themeId}${applyLight ? ' light' : ''}`; -} - /** Sync theme classes on without stripping non-theme classes (e.g. transitions-ready). */ -function applyThemeClasses(themeId: string, effectiveMode: 'dark' | 'light'): void { +function applyThemeClasses(themeId: string, resolvedMode: 'dark' | 'light'): void { const el = document.documentElement; const themeClass = `theme-${themeId}`; - const wantLight = resolveThemeClasses(themeId, effectiveMode).includes(' light'); + const wantLight = resolvedMode === 'light'; if (el.classList.contains(themeClass) && el.classList.contains('light') === wantLight) return; @@ -77,18 +74,23 @@ export function ThemeProvider({ storageKey = 'plannotator-theme', colorThemeStorageKey = 'plannotator-color-theme', }: ThemeProviderProps) { - const [mode, setModeState] = useState( - () => (storage.getItem(storageKey) as Mode) || defaultTheme - ); - const [colorTheme, setColorThemeState] = useState( () => storage.getItem(colorThemeStorageKey) || defaultColorTheme ); + const [mode, setModeState] = useState(() => { + const storedMode = parseThemeMode(storage.getItem(storageKey), defaultTheme); + return normalizeThemeMode(colorTheme, storedMode); + }); + const colorThemeRef = useRef(colorTheme); + const modeRef = useRef(mode); + const [systemIsLight, setSystemIsLight] = useState(getSystemIsLight); - // Compute resolved mode once — consumers use this instead of re-querying matchMedia - const resolvedMode: 'dark' | 'light' = mode === 'system' ? (systemIsLight ? 'light' : 'dark') : mode; + // Keep the OS-resolved preference separate from the mode the palette can render. + const preferredMode: 'dark' | 'light' = + mode === 'system' ? (systemIsLight ? 'light' : 'dark') : mode; + const resolvedMode = resolveThemeMode(colorTheme, preferredMode); // [P3 fix] Apply theme class synchronously during initialization to prevent // flash of unstyled content. CSS tokens live under .theme-* selectors, so @@ -126,25 +128,40 @@ export function ThemeProvider({ }, [mode]); const setMode = useCallback((newMode: Mode) => { - storage.setItem(storageKey, newMode); - setModeState(newMode); + const normalizedMode = normalizeThemeMode(colorThemeRef.current, newMode); + modeRef.current = normalizedMode; + storage.setItem(storageKey, normalizedMode); + setModeState(normalizedMode); }, [storageKey]); const setColorTheme = useCallback((newTheme: string) => { + const normalizedMode = normalizeThemeMode(newTheme, modeRef.current); + colorThemeRef.current = newTheme; storage.setItem(colorThemeStorageKey, newTheme); + if (normalizedMode !== modeRef.current) { + modeRef.current = normalizedMode; + storage.setItem(storageKey, normalizedMode); + setModeState(normalizedMode); + } setColorThemeState(newTheme); - }, [colorThemeStorageKey]); + }, [colorThemeStorageKey, storageKey]); + + // Repair invalid or incompatible values left by older versions at the boundary. + useEffect(() => { + if (storage.getItem(storageKey) !== mode) storage.setItem(storageKey, mode); + }, [mode, storageKey]); const value = useMemo(() => ({ theme: mode, setTheme: setMode, mode, setMode, + preferredMode, resolvedMode, colorTheme, setColorTheme, availableThemes: BUILT_IN_THEMES, - }), [mode, resolvedMode, colorTheme, setMode, setColorTheme]); + }), [mode, preferredMode, resolvedMode, colorTheme, setMode, setColorTheme]); return ( diff --git a/packages/ui/components/ThemeTab.tsx b/packages/ui/components/ThemeTab.tsx index b3d9cd72f..7152905b8 100644 --- a/packages/ui/components/ThemeTab.tsx +++ b/packages/ui/components/ThemeTab.tsx @@ -1,6 +1,7 @@ import React from 'react'; -import { useTheme, type Mode } from './ThemeProvider'; -import { SunIcon, MoonIcon, SystemIcon } from './icons/themeIcons'; +import { useTheme } from './ThemeProvider'; +import { THEME_MODES } from './themeModes'; +import { isThemeModeAvailable, resolveThemeMode } from '../utils/themeRegistry'; interface ThemeTabProps { onPreview?: () => void; @@ -8,7 +9,14 @@ interface ThemeTabProps { } export const ThemeTab: React.FC = ({ onPreview, compact }) => { - const { mode, setMode, colorTheme, setColorTheme, availableThemes, resolvedMode } = useTheme(); + const { + mode, + setMode, + colorTheme, + setColorTheme, + availableThemes, + preferredMode, + } = useTheme(); return (
@@ -16,36 +24,26 @@ export const ThemeTab: React.FC = ({ onPreview, compact }) => {
{!compact && }
- {(['dark', 'light', 'system'] as Mode[]).map(m => { - const isActive = mode === m; + {THEME_MODES.map(({ id, label, Icon }) => { + const available = isThemeModeAvailable(colorTheme, id); return ( ); })} @@ -86,10 +84,9 @@ export const ThemeTab: React.FC = ({ onPreview, compact }) => {
{availableThemes.map(theme => { const isSelected = colorTheme === theme.id; - const colors = theme.colors[resolvedMode]; - const modeUnavailable = - (resolvedMode === 'light' && theme.modeSupport === 'dark-only') || - (resolvedMode === 'dark' && theme.modeSupport === 'light-only'); + const previewMode = resolveThemeMode(theme.id, preferredMode); + const colors = theme.colors[previewMode]; + const modeUnavailable = !isThemeModeAvailable(theme.id, preferredMode); return (