From 7652dcf5e8740b076a26ab814489594dd5ed4d7e Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 17:55:05 +1000 Subject: [PATCH 1/4] feat(diffshub): persist display preferences DiffsHub display controls previously reset between visits, so repeat review sessions had to reapply the same view, wrap, line-number, background, indicator, and collapse preferences. The display state lived only in ReviewUI component state. This stores a versioned, locally validated preference payload, reuses the existing browser storage boundary for theme persistence, and hydrates the viewer only after preferences are loaded. Closes #851. --- apps/diffshub/components/ReviewUI.tsx | 90 +++++++- apps/diffshub/components/themeController.ts | 32 +-- apps/diffshub/lib/browserStorage.ts | 16 ++ apps/diffshub/lib/displayPreferences.ts | 201 ++++++++++++++++++ .../lib/test/displayPreferences.test.ts | 153 +++++++++++++ 5 files changed, 459 insertions(+), 33 deletions(-) create mode 100644 apps/diffshub/lib/browserStorage.ts create mode 100644 apps/diffshub/lib/displayPreferences.ts create mode 100644 apps/diffshub/lib/test/displayPreferences.test.ts diff --git a/apps/diffshub/components/ReviewUI.tsx b/apps/diffshub/components/ReviewUI.tsx index 49d586e2e..ed996000c 100644 --- a/apps/diffshub/components/ReviewUI.tsx +++ b/apps/diffshub/components/ReviewUI.tsx @@ -6,6 +6,7 @@ import { type ColorMode } from '@pierre/theming'; import { useThemeController } from '@pierre/theming/react'; import { type ReactNode, + type SetStateAction, useCallback, useEffect, useRef, @@ -24,6 +25,10 @@ import { themeController, } from '@/components/themeController'; import { preloadAvatars } from '@/lib/annotation'; +import { + type DiffsHubDisplayPreferences, + useDiffsHubDisplayPreferences, +} from '@/lib/displayPreferences'; import { removeSavedCommentSidebarEntry } from '@/lib/removeSavedCommentSidebarEntry'; import type { DarkThemeName, LightThemeName } from '@/lib/themeNames'; import type { @@ -54,15 +59,23 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { useEffect(preloadAvatars, []); const isWorkerPoolReadyOrDisable = useIsWorkerPoolReadyOrDisabled(); - const [diffStyle, setDiffStyle] = useState<'split' | 'unified'>('split'); - const [collapseMode, setCollapseMode] = useState<'expanded' | 'collapsed'>( - 'expanded' - ); const [fileTreeOverlayOpen, setFileTreeOverlayOpen] = useState(false); - const [overflow, setOverflow] = useState<'wrap' | 'scroll'>('scroll'); - const [showBackgrounds, setShowBackgrounds] = useState(true); - const [diffIndicators, setDiffIndicators] = useState('bars'); - const [lineNumbers, setLineNumbers] = useState(true); + const [forceUnifiedDiffStyle, setForceUnifiedDiffStyle] = useState(false); + const { + displayPreferences, + displayPreferencesHydrated, + updateDisplayPreferences, + } = useDiffsHubDisplayPreferences(); + const { + collapseMode, + diffIndicators, + lineNumbers, + overflow, + showBackgrounds, + } = displayPreferences; + const diffStyle = forceUnifiedDiffStyle + ? 'unified' + : displayPreferences.diffStyle; // All theming state — color mode and the light/dark theme-name picks — lives // in the single @pierre/theming controller (the same instance the app-wide // ThemeProvider is bound to). Reading it here means picking Auto/Light/Dark @@ -145,7 +158,7 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { useEffect(() => { const mediaQuery = window.matchMedia('(max-width: 767px)'); const updateMobileState = (matches: boolean) => { - setDiffStyle(matches ? 'unified' : 'split'); + setForceUnifiedDiffStyle(matches); if (!matches) setFileTreeOverlayOpen(false); }; const handleChange = (event: MediaQueryListEvent) => { @@ -156,6 +169,57 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { mediaQuery.addEventListener('change', handleChange); return () => mediaQuery.removeEventListener('change', handleChange); }, []); + const updateDisplayPreference = useCallback( + ( + key: Key, + value: SetStateAction + ) => { + updateDisplayPreferences((previous) => { + const previousValue = previous[key]; + const nextValue = + typeof value === 'function' + ? ( + value as (current: typeof previousValue) => typeof previousValue + )(previousValue) + : value; + return { + ...previous, + [key]: nextValue, + }; + }); + }, + [updateDisplayPreferences] + ); + const setDiffStyle = useCallback( + (value: SetStateAction<'split' | 'unified'>) => { + updateDisplayPreference('diffStyle', value); + }, + [updateDisplayPreference] + ); + const setDiffIndicators = useCallback( + (value: SetStateAction) => { + updateDisplayPreference('diffIndicators', value); + }, + [updateDisplayPreference] + ); + const setLineNumbers = useCallback( + (value: SetStateAction) => { + updateDisplayPreference('lineNumbers', value); + }, + [updateDisplayPreference] + ); + const setOverflow = useCallback( + (value: SetStateAction<'wrap' | 'scroll'>) => { + updateDisplayPreference('overflow', value); + }, + [updateDisplayPreference] + ); + const setShowBackgrounds = useCallback( + (value: SetStateAction) => { + updateDisplayPreference('showBackgrounds', value); + }, + [updateDisplayPreference] + ); const handleSelectTreeItem = useCallback((itemId: string) => { setFileTreeOverlayOpen(false); const viewer = viewerRef.current; @@ -177,9 +241,12 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { }, []); const handleToggleCollapseMode = useCallback(() => { const next = collapseMode === 'expanded' ? 'collapsed' : 'expanded'; - setCollapseMode(next); + updateDisplayPreferences((previous) => ({ + ...previous, + collapseMode: next, + })); applyCollapseModeToLoaded(next); - }, [applyCollapseModeToLoaded, collapseMode]); + }, [applyCollapseModeToLoaded, collapseMode, updateDisplayPreferences]); const handleCommentSaved = useCallback( (comment: DiffsHubSavedCommentEvent) => { setCommentSections((prev) => @@ -227,6 +294,7 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { // first batch of files against the wrong palette. const viewerAvailable = isWorkerPoolReadyOrDisable && + displayPreferencesHydrated && themesHydrated && (loadState === 'ready' || (loadState === 'streaming' && initialItems.length > 0)); diff --git a/apps/diffshub/components/themeController.ts b/apps/diffshub/components/themeController.ts index cb5d8f545..277b5a5a3 100644 --- a/apps/diffshub/components/themeController.ts +++ b/apps/diffshub/components/themeController.ts @@ -1,6 +1,10 @@ import { createThemeController, type ThemePersistence } from '@pierre/theming'; import { docsThemeCatalog } from './themeCatalog'; +import { + readBrowserStorageKey, + writeBrowserStorageKey, +} from '@/lib/browserStorage'; export { docsThemeCatalog } from './themeCatalog'; @@ -23,30 +27,14 @@ const MODE_KEY = 'theme'; const LIGHT_THEME_KEY = 'diffshub-light-theme'; const DARK_THEME_KEY = 'diffshub-dark-theme'; -function readKey(key: string): string | null { - try { - return globalThis.localStorage?.getItem(key) ?? null; - } catch { - return null; - } -} - -function writeKey(key: string, value: string): void { - try { - globalThis.localStorage?.setItem(key, value); - } catch { - // Storage may be unavailable (private mode / denied) — non-fatal. - } -} - // Maps the controller's selection onto the app's three storage keys: mode as a // plain `light`/`dark`/`system` string under `theme` (what the bootstrap script // reads), and the theme names under the diffshub-prefixed keys. const docsPersistence: ThemePersistence = { load() { - const mode = readKey(MODE_KEY); - const light = readKey(LIGHT_THEME_KEY); - const dark = readKey(DARK_THEME_KEY); + const mode = readBrowserStorageKey(MODE_KEY); + const light = readBrowserStorageKey(LIGHT_THEME_KEY); + const dark = readBrowserStorageKey(DARK_THEME_KEY); if (mode == null && light == null && dark == null) return null; const validMode = mode === 'light' || mode === 'dark' || mode === 'system' @@ -59,9 +47,9 @@ const docsPersistence: ThemePersistence = { }; }, save(selection) { - writeKey(MODE_KEY, selection.mode); - writeKey(LIGHT_THEME_KEY, selection.lightThemeName); - writeKey(DARK_THEME_KEY, selection.darkThemeName); + writeBrowserStorageKey(MODE_KEY, selection.mode); + writeBrowserStorageKey(LIGHT_THEME_KEY, selection.lightThemeName); + writeBrowserStorageKey(DARK_THEME_KEY, selection.darkThemeName); }, }; diff --git a/apps/diffshub/lib/browserStorage.ts b/apps/diffshub/lib/browserStorage.ts new file mode 100644 index 000000000..b5976965c --- /dev/null +++ b/apps/diffshub/lib/browserStorage.ts @@ -0,0 +1,16 @@ +export function readBrowserStorageKey(key: string): string | null { + try { + return globalThis.localStorage?.getItem(key) ?? null; + } catch { + return null; + } +} + +export function writeBrowserStorageKey(key: string, value: string): void { + try { + globalThis.localStorage?.setItem(key, value); + } catch { + // Storage may be unavailable (private mode / denied) or full. Callers keep + // their in-memory state, so persistence failure is non-fatal. + } +} diff --git a/apps/diffshub/lib/displayPreferences.ts b/apps/diffshub/lib/displayPreferences.ts new file mode 100644 index 000000000..39683baaa --- /dev/null +++ b/apps/diffshub/lib/displayPreferences.ts @@ -0,0 +1,201 @@ +import type { DiffIndicators } from '@pierre/diffs'; +import { useCallback, useEffect, useState } from 'react'; + +import { + readBrowserStorageKey, + writeBrowserStorageKey, +} from './browserStorage'; + +export type DiffsHubCollapseMode = 'expanded' | 'collapsed'; +export type DiffsHubDiffStyle = 'split' | 'unified'; +export type DiffsHubOverflow = 'wrap' | 'scroll'; + +export interface DiffsHubDisplayPreferences { + collapseMode: DiffsHubCollapseMode; + diffIndicators: DiffIndicators; + diffStyle: DiffsHubDiffStyle; + lineNumbers: boolean; + overflow: DiffsHubOverflow; + showBackgrounds: boolean; +} + +export const DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES = { + collapseMode: 'expanded', + diffIndicators: 'bars', + diffStyle: 'split', + lineNumbers: true, + overflow: 'scroll', + showBackgrounds: true, +} satisfies DiffsHubDisplayPreferences; + +const COLLAPSE_MODE_VALUES = [ + 'expanded', + 'collapsed', +] satisfies readonly DiffsHubCollapseMode[]; +const DIFF_INDICATOR_VALUES = [ + 'bars', + 'classic', + 'none', +] satisfies readonly DiffIndicators[]; +const DIFF_STYLE_VALUES = [ + 'split', + 'unified', +] satisfies readonly DiffsHubDiffStyle[]; +const OVERFLOW_VALUES = [ + 'wrap', + 'scroll', +] satisfies readonly DiffsHubOverflow[]; + +const DISPLAY_PREFERENCES_STORAGE_KEY = 'diffshub.displayPreferences.v1'; +const DISPLAY_PREFERENCES_STORAGE_VERSION = 1; + +interface StoredDisplayPreferences { + preferences: DiffsHubDisplayPreferences; + version: typeof DISPLAY_PREFERENCES_STORAGE_VERSION; +} + +interface UseDiffsHubDisplayPreferencesResult { + displayPreferences: DiffsHubDisplayPreferences; + displayPreferencesHydrated: boolean; + updateDisplayPreferences( + update: (previous: DiffsHubDisplayPreferences) => DiffsHubDisplayPreferences + ): void; +} + +export function useDiffsHubDisplayPreferences(): UseDiffsHubDisplayPreferencesResult { + const [displayPreferences, setDisplayPreferences] = + useState(DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES); + const [displayPreferencesHydrated, setDisplayPreferencesHydrated] = + useState(false); + + useEffect(() => { + setDisplayPreferences(readDiffsHubDisplayPreferences()); + setDisplayPreferencesHydrated(true); + }, []); + + const updateDisplayPreferences = useCallback( + ( + update: ( + previous: DiffsHubDisplayPreferences + ) => DiffsHubDisplayPreferences + ) => { + setDisplayPreferences((previous) => { + const next = update(previous); + writeDiffsHubDisplayPreferences(next); + return next; + }); + }, + [] + ); + + return { + displayPreferences, + displayPreferencesHydrated, + updateDisplayPreferences, + }; +} + +export function readDiffsHubDisplayPreferences(): DiffsHubDisplayPreferences { + const rawValue = readBrowserStorageKey(DISPLAY_PREFERENCES_STORAGE_KEY); + if (rawValue == null) { + return DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES; + } + + let parsedValue: unknown; + try { + parsedValue = JSON.parse(rawValue); + } catch { + return DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES; + } + + return parseStoredDisplayPreferences(parsedValue); +} + +export function writeDiffsHubDisplayPreferences( + preferences: DiffsHubDisplayPreferences +): void { + const storedPreferences = { + preferences, + version: DISPLAY_PREFERENCES_STORAGE_VERSION, + } satisfies StoredDisplayPreferences; + + writeBrowserStorageKey( + DISPLAY_PREFERENCES_STORAGE_KEY, + JSON.stringify(storedPreferences) + ); +} + +function parseStoredDisplayPreferences( + value: unknown +): DiffsHubDisplayPreferences { + if ( + getObjectProperty(value, 'version') !== DISPLAY_PREFERENCES_STORAGE_VERSION + ) { + return DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES; + } + + return parseDisplayPreferences(getObjectProperty(value, 'preferences')); +} + +function parseDisplayPreferences(value: unknown): DiffsHubDisplayPreferences { + return { + collapseMode: parseStringChoice( + getObjectProperty(value, 'collapseMode'), + COLLAPSE_MODE_VALUES, + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.collapseMode + ), + diffIndicators: parseStringChoice( + getObjectProperty(value, 'diffIndicators'), + DIFF_INDICATOR_VALUES, + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.diffIndicators + ), + diffStyle: parseStringChoice( + getObjectProperty(value, 'diffStyle'), + DIFF_STYLE_VALUES, + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.diffStyle + ), + lineNumbers: parseBoolean( + getObjectProperty(value, 'lineNumbers'), + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.lineNumbers + ), + overflow: parseStringChoice( + getObjectProperty(value, 'overflow'), + OVERFLOW_VALUES, + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.overflow + ), + showBackgrounds: parseBoolean( + getObjectProperty(value, 'showBackgrounds'), + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.showBackgrounds + ), + }; +} + +function parseStringChoice( + value: unknown, + choices: readonly Value[], + fallback: Value +): Value { + if (typeof value !== 'string') { + return fallback; + } + + for (const choice of choices) { + if (choice === value) { + return choice; + } + } + + return fallback; +} + +function parseBoolean(value: unknown, fallback: boolean): boolean { + return typeof value === 'boolean' ? value : fallback; +} + +function getObjectProperty(value: unknown, property: string): unknown { + if (value == null || typeof value !== 'object' || Array.isArray(value)) { + return undefined; + } + + return Object.getOwnPropertyDescriptor(value, property)?.value; +} diff --git a/apps/diffshub/lib/test/displayPreferences.test.ts b/apps/diffshub/lib/test/displayPreferences.test.ts new file mode 100644 index 000000000..d6daa46d9 --- /dev/null +++ b/apps/diffshub/lib/test/displayPreferences.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, test } from 'bun:test'; + +import { + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES, + type DiffsHubDisplayPreferences, + readDiffsHubDisplayPreferences, + writeDiffsHubDisplayPreferences, +} from '../displayPreferences'; + +const STORAGE_KEY = 'diffshub.displayPreferences.v1'; + +class MemoryStorage implements Storage { + private readonly values = new Map(); + + get length(): number { + return this.values.size; + } + + clear(): void { + this.values.clear(); + } + + getItem(key: string): string | null { + return this.values.get(key) ?? null; + } + + key(index: number): string | null { + return Array.from(this.values.keys())[index] ?? null; + } + + removeItem(key: string): void { + this.values.delete(key); + } + + setItem(key: string, value: string): void { + this.values.set(key, value); + } +} + +class ThrowingStorage extends MemoryStorage { + override getItem(): string | null { + throw new Error('storage unavailable'); + } + + override setItem(): void { + throw new Error('storage unavailable'); + } +} + +describe('DiffsHub display preferences', () => { + test('falls back to defaults when storage is empty or unavailable', () => { + withLocalStorage(new MemoryStorage(), () => { + expect(readDiffsHubDisplayPreferences()).toEqual( + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES + ); + }); + withLocalStorage(new ThrowingStorage(), () => { + expect(readDiffsHubDisplayPreferences()).toEqual( + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES + ); + }); + }); + + test('round-trips validated display preferences', () => { + const storage = new MemoryStorage(); + const preferences: DiffsHubDisplayPreferences = { + collapseMode: 'collapsed', + diffIndicators: 'classic', + diffStyle: 'unified', + lineNumbers: false, + overflow: 'wrap', + showBackgrounds: false, + }; + + withLocalStorage(storage, () => { + writeDiffsHubDisplayPreferences(preferences); + + expect(readDiffsHubDisplayPreferences()).toEqual(preferences); + }); + }); + + test('ignores malformed stored preferences per field', () => { + const storage = new MemoryStorage(); + storage.setItem( + STORAGE_KEY, + JSON.stringify({ + preferences: { + collapseMode: 'closed', + diffIndicators: 'classic', + diffStyle: 'side-by-side', + lineNumbers: false, + overflow: 'wrap', + showBackgrounds: 'no', + }, + version: 1, + }) + ); + + withLocalStorage(storage, () => { + expect(readDiffsHubDisplayPreferences()).toEqual({ + ...DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES, + diffIndicators: 'classic', + lineNumbers: false, + overflow: 'wrap', + }); + }); + }); + + test('ignores incompatible storage versions', () => { + const storage = new MemoryStorage(); + storage.setItem( + STORAGE_KEY, + JSON.stringify({ + preferences: { + collapseMode: 'collapsed', + diffIndicators: 'none', + diffStyle: 'unified', + lineNumbers: false, + overflow: 'wrap', + showBackgrounds: false, + }, + version: 2, + }) + ); + + withLocalStorage(storage, () => { + expect(readDiffsHubDisplayPreferences()).toEqual( + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES + ); + }); + }); +}); + +function withLocalStorage(storage: Storage, callback: () => void): void { + const descriptor = Object.getOwnPropertyDescriptor( + globalThis, + 'localStorage' + ); + Object.defineProperty(globalThis, 'localStorage', { + configurable: true, + value: storage, + }); + + try { + callback(); + } finally { + if (descriptor == null) { + Reflect.deleteProperty(globalThis, 'localStorage'); + } else { + Object.defineProperty(globalThis, 'localStorage', descriptor); + } + } +} From d3b37100fcd98f97db5d4ddf8134f133767c7041 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 18:18:47 +1000 Subject: [PATCH 2/4] test(diffshub): cover persisted display preferences --- apps/diffshub/components/ReviewUI.tsx | 1 + apps/diffshub/components/usePatchLoader.ts | 7 + apps/diffshub/lib/displayPreferences.ts | 18 +- .../test/reviewDisplayPreferences.test.tsx | 450 ++++++++++++++++++ 4 files changed, 469 insertions(+), 7 deletions(-) create mode 100644 apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx diff --git a/apps/diffshub/components/ReviewUI.tsx b/apps/diffshub/components/ReviewUI.tsx index ed996000c..2d66f5f6b 100644 --- a/apps/diffshub/components/ReviewUI.tsx +++ b/apps/diffshub/components/ReviewUI.tsx @@ -150,6 +150,7 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { } = usePatchLoader({ collapseMode, domain, + enabled: displayPreferencesHydrated, onLoadStart: handlePatchLoadStart, path, viewerRef, diff --git a/apps/diffshub/components/usePatchLoader.ts b/apps/diffshub/components/usePatchLoader.ts index 4da2aa0ef..4a5fe6279 100644 --- a/apps/diffshub/components/usePatchLoader.ts +++ b/apps/diffshub/components/usePatchLoader.ts @@ -56,6 +56,7 @@ const GENERIC_PATCH_LOAD_ERROR_MESSAGE = interface UsePatchLoaderOptions { collapseMode: 'expanded' | 'collapsed'; domain?: string; + enabled: boolean; onLoadStart(): void; path: string; viewerRef: RefObject | null>; @@ -80,6 +81,7 @@ interface UsePatchLoaderResult { export function usePatchLoader({ collapseMode, domain, + enabled, onLoadStart, path, viewerRef, @@ -212,6 +214,10 @@ export function usePatchLoader({ ); useEffect(() => { + if (!enabled) { + return; + } + const patchRequestKey = domain == null || domain === '' ? path : `${domain}${path}`; const patchSearchParams = new URLSearchParams({ path }); @@ -488,6 +494,7 @@ export function usePatchLoader({ }; }, [ domain, + enabled, loadAttempt, onLoadStart, path, diff --git a/apps/diffshub/lib/displayPreferences.ts b/apps/diffshub/lib/displayPreferences.ts index 39683baaa..6ae39fd91 100644 --- a/apps/diffshub/lib/displayPreferences.ts +++ b/apps/diffshub/lib/displayPreferences.ts @@ -1,5 +1,5 @@ import type { DiffIndicators } from '@pierre/diffs'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { readBrowserStorageKey, @@ -65,11 +65,16 @@ interface UseDiffsHubDisplayPreferencesResult { export function useDiffsHubDisplayPreferences(): UseDiffsHubDisplayPreferencesResult { const [displayPreferences, setDisplayPreferences] = useState(DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES); + const displayPreferencesRef = useRef( + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES + ); const [displayPreferencesHydrated, setDisplayPreferencesHydrated] = useState(false); useEffect(() => { - setDisplayPreferences(readDiffsHubDisplayPreferences()); + const storedPreferences = readDiffsHubDisplayPreferences(); + displayPreferencesRef.current = storedPreferences; + setDisplayPreferences(storedPreferences); setDisplayPreferencesHydrated(true); }, []); @@ -79,11 +84,10 @@ export function useDiffsHubDisplayPreferences(): UseDiffsHubDisplayPreferencesRe previous: DiffsHubDisplayPreferences ) => DiffsHubDisplayPreferences ) => { - setDisplayPreferences((previous) => { - const next = update(previous); - writeDiffsHubDisplayPreferences(next); - return next; - }); + const next = update(displayPreferencesRef.current); + displayPreferencesRef.current = next; + setDisplayPreferences(next); + writeDiffsHubDisplayPreferences(next); }, [] ); diff --git a/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx b/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx new file mode 100644 index 000000000..76580b091 --- /dev/null +++ b/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx @@ -0,0 +1,450 @@ +/** @jsxImportSource react */ + +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + test, +} from 'bun:test'; +import { JSDOM, VirtualConsole } from 'jsdom'; +import { + AppRouterContext, + type AppRouterInstance, +} from 'next/dist/shared/lib/app-router-context.shared-runtime'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; + +const DISPLAY_PREFERENCES_STORAGE_KEY = 'diffshub.displayPreferences.v1'; +const MOBILE_MEDIA_QUERY = '(max-width: 767px)'; +const PATCH_TEXT = `diff --git a/src/example.ts b/src/example.ts +index 1111111..2222222 100644 +--- a/src/example.ts ++++ b/src/example.ts +@@ -1,3 +1,3 @@ + export function example() { +- return 1; ++ return 2; + } +`; + +const originalGlobals = { + CSSStyleSheet: Reflect.get(globalThis, 'CSSStyleSheet'), + Image: Reflect.get(globalThis, 'Image'), + cancelAnimationFrame: Reflect.get(globalThis, 'cancelAnimationFrame'), + customElements: Reflect.get(globalThis, 'customElements'), + document: Reflect.get(globalThis, 'document'), + fetch: Reflect.get(globalThis, 'fetch'), + getComputedStyle: Reflect.get(globalThis, 'getComputedStyle'), + HTMLButtonElement: Reflect.get(globalThis, 'HTMLButtonElement'), + HTMLDivElement: Reflect.get(globalThis, 'HTMLDivElement'), + HTMLElement: Reflect.get(globalThis, 'HTMLElement'), + HTMLStyleElement: Reflect.get(globalThis, 'HTMLStyleElement'), + HTMLTemplateElement: Reflect.get(globalThis, 'HTMLTemplateElement'), + IntersectionObserver: Reflect.get(globalThis, 'IntersectionObserver'), + IS_REACT_ACT_ENVIRONMENT: Reflect.get( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }, + 'IS_REACT_ACT_ENVIRONMENT' + ), + localStorage: Reflect.get(globalThis, 'localStorage'), + matchMedia: Reflect.get(globalThis, 'matchMedia'), + MutationObserver: Reflect.get(globalThis, 'MutationObserver'), + navigator: Reflect.get(globalThis, 'navigator'), + requestAnimationFrame: Reflect.get(globalThis, 'requestAnimationFrame'), + ResizeObserver: Reflect.get(globalThis, 'ResizeObserver'), + ShadowRoot: Reflect.get(globalThis, 'ShadowRoot'), + SVGElement: Reflect.get(globalThis, 'SVGElement'), + window: Reflect.get(globalThis, 'window'), +}; + +const virtualConsole = new VirtualConsole(); +virtualConsole.on('jsdomError', (error) => { + if ('type' in error && error.type === 'css parsing') { + return; + } + + console.error(error); +}); + +const dom = new JSDOM('', { + pretendToBeVisual: true, + url: 'http://localhost', + virtualConsole, +}); + +let mobileMatches = false; +type MediaListener = + | EventListenerOrEventListenerObject + | ((this: MediaQueryList, event: MediaQueryListEvent) => unknown); +let mediaListeners = new Map< + MediaListener, + (event: MediaQueryListEvent) => void +>(); + +class MockResizeObserver { + observe(_target: Element): void {} + unobserve(_target: Element): void {} + disconnect(): void {} +} + +class MockIntersectionObserver { + observe(_target: Element): void {} + unobserve(_target: Element): void {} + disconnect(): void {} + takeRecords(): IntersectionObserverEntry[] { + return []; + } +} + +class MockCSSStyleSheet { + replaceSync(_cssText: string): void {} +} + +beforeAll(() => { + Object.assign(globalThis, { + CSSStyleSheet: MockCSSStyleSheet, + cancelAnimationFrame: dom.window.cancelAnimationFrame.bind(dom.window), + customElements: dom.window.customElements, + document: dom.window.document, + fetch: fetchPatch, + getComputedStyle: dom.window.getComputedStyle.bind(dom.window), + HTMLButtonElement: dom.window.HTMLButtonElement, + HTMLDivElement: dom.window.HTMLDivElement, + HTMLElement: dom.window.HTMLElement, + HTMLStyleElement: dom.window.HTMLStyleElement, + HTMLTemplateElement: dom.window.HTMLTemplateElement, + Image: dom.window.Image, + IntersectionObserver: MockIntersectionObserver, + localStorage: dom.window.localStorage, + matchMedia, + MutationObserver: dom.window.MutationObserver, + navigator: dom.window.navigator, + requestAnimationFrame: dom.window.requestAnimationFrame.bind(dom.window), + ResizeObserver: MockResizeObserver, + ShadowRoot: dom.window.ShadowRoot, + SVGElement: dom.window.SVGElement, + window: dom.window, + }); + ( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } + ).IS_REACT_ACT_ENVIRONMENT = true; + Object.assign(dom.window, { + CSSStyleSheet: MockCSSStyleSheet, + IntersectionObserver: MockIntersectionObserver, + matchMedia, + ResizeObserver: MockResizeObserver, + }); +}); + +beforeEach(() => { + mediaListeners = new Map(); + mobileMatches = false; + localStorage.clear(); + document.body.textContent = ''; +}); + +afterAll(() => { + for (const [key, value] of Object.entries(originalGlobals)) { + if (value === undefined) { + Reflect.deleteProperty(globalThis, key); + } else { + Object.assign(globalThis, { [key]: value }); + } + } + dom.window.close(); +}); + +describe('ReviewUI display preferences', () => { + test('uses unified view on mobile without overwriting the stored desktop split preference', async () => { + mobileMatches = true; + writeStoredPreferences({ + collapseMode: 'expanded', + diffIndicators: 'bars', + diffStyle: 'split', + lineNumbers: true, + overflow: 'scroll', + showBackgrounds: true, + }); + + const rendered = await renderReviewUI(); + + try { + await waitFor(() => { + expect( + querySelectorDeep(rendered.container, 'pre[data-diff]')?.getAttribute( + 'data-diff-type' + ) + ).toBe('single'); + }); + expect(readStoredDiffStyle()).toBe('split'); + + await act(async () => { + setMobileMatches(false); + await flushReact(); + }); + + await waitFor(() => { + expect( + querySelectorDeep(rendered.container, 'pre[data-diff]')?.getAttribute( + 'data-diff-type' + ) + ).toBe('split'); + }); + + const toggle = await waitForElement( + rendered.container, + 'button[title="Switch to unified view"]' + ); + await act(async () => { + toggle.click(); + await flushReact(); + }); + + await waitFor(() => { + expect(readStoredDiffStyle()).toBe('unified'); + }); + } finally { + await cleanup(rendered); + } + }); + + test('uses stored collapsed mode for initially loaded diff items', async () => { + writeStoredPreferences({ + collapseMode: 'collapsed', + diffIndicators: 'bars', + diffStyle: 'split', + lineNumbers: true, + overflow: 'scroll', + showBackgrounds: true, + }); + + const rendered = await renderReviewUI(); + + try { + await waitFor(() => { + expect( + rendered.container.querySelector('button[aria-label="Expand diff"]') + ).not.toBeNull(); + }); + } finally { + await cleanup(rendered); + } + }); +}); + +interface StoredPreferences { + collapseMode: 'expanded' | 'collapsed'; + diffIndicators: 'bars' | 'classic' | 'none'; + diffStyle: 'split' | 'unified'; + lineNumbers: boolean; + overflow: 'wrap' | 'scroll'; + showBackgrounds: boolean; +} + +interface RenderedReviewUI { + container: HTMLDivElement; + root: Root; +} + +async function renderReviewUI(): Promise { + const container = document.createElement('div'); + document.body.append(container); + const { ReviewUI } = await import('../../components/ReviewUI'); + let root: Root | undefined; + + await act(async () => { + root = createRoot(container); + root.render( + + + + ); + await flushReact(); + }); + + if (root == null) { + throw new Error('ReviewUI root was not created'); + } + return { container, root }; +} + +async function cleanup({ container, root }: RenderedReviewUI): Promise { + await act(async () => { + root.unmount(); + await flushReact(); + }); + container.remove(); +} + +function writeStoredPreferences(preferences: StoredPreferences): void { + localStorage.setItem( + DISPLAY_PREFERENCES_STORAGE_KEY, + JSON.stringify({ preferences, version: 1 }) + ); +} + +function readStoredDiffStyle(): string | undefined { + const rawValue = localStorage.getItem(DISPLAY_PREFERENCES_STORAGE_KEY); + if (rawValue == null) { + return undefined; + } + + return ( + JSON.parse(rawValue) as { + preferences?: { diffStyle?: string }; + } + ).preferences?.diffStyle; +} + +function fetchPatch(): Promise { + return Promise.resolve({ + body: null, + ok: true, + text: () => Promise.resolve(PATCH_TEXT), + } as Response); +} + +const testRouter: AppRouterInstance = { + back() {}, + forward() {}, + prefetch() {}, + push() {}, + refresh() {}, + replace() {}, +}; + +function matchMedia(query: string): MediaQueryList { + const matches = query === MOBILE_MEDIA_QUERY ? mobileMatches : false; + let mediaQueryList: MediaQueryList; + + mediaQueryList = { + addEventListener( + _type: 'change', + listener: EventListenerOrEventListenerObject | null + ) { + if (listener == null) { + return; + } + mediaListeners.set(listener, (event) => { + if (typeof listener === 'function') { + listener(event); + } else { + listener.handleEvent(event); + } + }); + }, + addListener( + listener: + | ((this: MediaQueryList, event: MediaQueryListEvent) => unknown) + | null + ) { + if (listener == null) { + return; + } + mediaListeners.set(listener, (event) => { + listener.call(mediaQueryList, event); + }); + }, + dispatchEvent() { + return true; + }, + matches, + media: query, + onchange: null, + removeEventListener( + _type: 'change', + listener: EventListenerOrEventListenerObject | null + ) { + if (listener != null) { + mediaListeners.delete(listener); + } + }, + removeListener( + listener: + | ((this: MediaQueryList, event: MediaQueryListEvent) => unknown) + | null + ) { + if (listener != null) { + mediaListeners.delete(listener); + } + }, + } as MediaQueryList; + + return mediaQueryList; +} + +function setMobileMatches(matches: boolean): void { + mobileMatches = matches; + const event = { matches, media: MOBILE_MEDIA_QUERY } as MediaQueryListEvent; + for (const listener of mediaListeners.values()) { + listener(event); + } +} + +async function waitFor(assertion: () => void | Promise): Promise { + const startedAt = Date.now(); + let lastError: unknown; + + while (Date.now() - startedAt < 2_000) { + try { + await assertion(); + return; + } catch (error) { + lastError = error; + } + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + await flushReact(); + }); + } + + throw lastError; +} + +async function waitForElement( + container: ParentNode, + selector: string +): Promise { + let element: ElementType | null = null; + await waitFor(() => { + element = container.querySelector(selector); + expect(element).not.toBeNull(); + }); + if (element == null) { + throw new Error(`Expected to find element matching ${selector}`); + } + return element; +} + +function querySelectorDeep( + root: ParentNode, + selector: string +): ElementType | null { + const directMatch = root.querySelector(selector); + if (directMatch != null) { + return directMatch; + } + + for (const element of root.querySelectorAll('*')) { + const shadowMatch = + element.shadowRoot == null + ? null + : querySelectorDeep(element.shadowRoot, selector); + if (shadowMatch != null) { + return shadowMatch; + } + } + + return null; +} + +async function flushReact(): Promise { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); +} From e1e2d3832172f130bee155bbb93c7a4c1de68805 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Fri, 19 Jun 2026 18:01:34 +1000 Subject: [PATCH 3/4] feat(diffshub): add code font preference DiffsHub now has a persisted display preference payload, so the code font can live beside the other viewer settings instead of adding another storage boundary. This adds Berkeley, Geist Mono, and system monospace choices, applies the selected family through the existing --diffs-font-family hook, and validates stored font values with the same per-field preference parser. Closes #852. --- apps/diffshub/components/DiffsHubHeader.tsx | 95 ++++- apps/diffshub/components/DiffsHubViewer.tsx | 21 +- apps/diffshub/components/ReviewUI.tsx | 12 + apps/diffshub/lib/displayPreferences.ts | 324 ++++++++++++++++++ .../lib/test/displayPreferences.test.ts | 81 +++++ .../test/reviewDisplayPreferences.test.tsx | 212 ++++++++++++ 6 files changed, 742 insertions(+), 3 deletions(-) diff --git a/apps/diffshub/components/DiffsHubHeader.tsx b/apps/diffshub/components/DiffsHubHeader.tsx index 6b7445b37..da3252d99 100644 --- a/apps/diffshub/components/DiffsHubHeader.tsx +++ b/apps/diffshub/components/DiffsHubHeader.tsx @@ -23,6 +23,7 @@ import { type Dispatch, memo, type SetStateAction, + useCallback, useLayoutEffect, useMemo, useRef, @@ -44,6 +45,12 @@ import { import { Switch } from '@/components/Switch'; import { docsThemeCatalog } from '@/components/themeCatalog'; import { cn } from '@/lib/cn'; +import { + DIFFS_HUB_CODE_FONT_OPTIONS, + type DiffsHubCodeFont, + isDiffsHubDefaultCodeFont, + resolveCodeFontInput, +} from '@/lib/displayPreferences'; import { diffshubChromeMapping } from '@/lib/theme/diffshubChromeMapping'; import { getDropdownThemeStyle } from '@/lib/theme/dropdownChromeStyle'; @@ -55,6 +62,7 @@ const SETTING_ROW_CLASS = interface HeaderProps { className?: string; + codeFont: DiffsHubCodeFont; collapseMode: 'expanded' | 'collapsed'; colorMode: ColorMode; darkThemeName: DarkThemeName; @@ -68,6 +76,7 @@ interface HeaderProps { overflow: 'wrap' | 'scroll'; onToggleCollapseMode(): void; onToggleFileTreeOverlay(): void; + setCodeFont: Dispatch>; setColorMode(mode: ColorMode): void; setDarkThemeName(name: DarkThemeName): void; setDiffIndicators: Dispatch>; @@ -81,6 +90,7 @@ interface HeaderProps { export const DiffsHubHeader = memo(function DiffsHubHeader({ className, + codeFont, collapseMode, colorMode, darkThemeName, @@ -94,6 +104,7 @@ export const DiffsHubHeader = memo(function DiffsHubHeader({ overflow, onToggleCollapseMode, onToggleFileTreeOverlay, + setCodeFont, setColorMode, setDarkThemeName, setDiffIndicators, @@ -105,6 +116,23 @@ export const DiffsHubHeader = memo(function DiffsHubHeader({ showBackgrounds, }: HeaderProps) { const [currentUrl, setCurrentUrl] = useState(initialUrl); + const codeFontSelection = codeFont.kind === 'default' ? 'default' : 'custom'; + const customCodeFontFamily = + codeFont.kind === 'default' + ? '' + : (codeFont.input ?? + (codeFont.kind === 'system' ? 'System monospace' : codeFont.family)); + const focusCustomCodeFontInput = useCallback( + (input: HTMLInputElement | null) => { + if (input == null || document.activeElement === input) { + return; + } + + input.focus({ preventScroll: true }); + input.setSelectionRange(input.value.length, input.value.length); + }, + [] + ); // Only show the external-link button when the input still reflects the // committed URL — otherwise we'd be pointing at a draft the user is editing. const showExternalLink = currentUrl === initialUrl; @@ -238,7 +266,7 @@ export const DiffsHubHeader = memo(function DiffsHubHeader({ +
+
+ Code font + { + if (isDiffsHubDefaultCodeFont(value)) { + setCodeFont({ + kind: 'default', + }); + return; + } + + if (value === 'custom') { + setCodeFont((previous) => + previous.kind !== 'default' + ? previous + : { + family: '', + input: '', + kind: 'custom', + } + ); + } + }} + > + {DIFFS_HUB_CODE_FONT_OPTIONS.map((option) => ( + + {option.label} + + ))} + + Custom + + +
+ {codeFont.kind !== 'default' && ( + { + setCodeFont( + resolveCodeFontInput(currentTarget.value) ?? { + family: '', + input: currentTarget.value, + kind: 'custom', + } + ); + }} + onKeyDown={(event) => event.stopPropagation()} + /> + )} +
e.preventDefault()} diff --git a/apps/diffshub/components/DiffsHubViewer.tsx b/apps/diffshub/components/DiffsHubViewer.tsx index 218673f81..c9f5be6e2 100644 --- a/apps/diffshub/components/DiffsHubViewer.tsx +++ b/apps/diffshub/components/DiffsHubViewer.tsx @@ -12,7 +12,14 @@ import { } from '@pierre/diffs'; import { type CodeViewHandle, useStableCallback } from '@pierre/diffs/react'; import { IconChevronSm } from '@pierre/icons'; -import { memo, type RefObject, useMemo, useRef, useState } from 'react'; +import { + type CSSProperties, + memo, + type RefObject, + useMemo, + useRef, + useState, +} from 'react'; import { DraftAnnotation } from './DraftAnnotation'; import { ExampleAnnotation } from './ExampleAnnotation'; @@ -63,6 +70,7 @@ interface ActiveDraftComment { interface DiffsHubViewerProps { className?: string; + codeFontFamily: string; diffStyle: 'split' | 'unified'; onCommentDeleted(comment: DiffsHubDeletedCommentEvent): void; onCommentSaved(comment: DiffsHubSavedCommentEvent): void; @@ -80,6 +88,7 @@ interface DiffsHubViewerProps { export const DiffsHubViewer = memo(function DiffsHubViewer({ className, + codeFontFamily, diffStyle, onCommentDeleted, onCommentSaved, @@ -107,6 +116,14 @@ export const DiffsHubViewer = memo(function DiffsHubViewer({ () => buildAnnotationThemeStyle(themeChromeStyle), [themeChromeStyle] ); + const viewerStyle = useMemo( + () => + ({ + ...(annotationThemeStyle ?? {}), + '--diffs-font-family': codeFontFamily, + }) satisfies CSSProperties & { '--diffs-font-family': string }, + [annotationThemeStyle, codeFontFamily] + ); const handleSetSelection = useStableCallback( (selection: CodeViewLineSelection | null) => { @@ -471,7 +488,7 @@ export const DiffsHubViewer = memo(function DiffsHubViewer({ 'cv-scrollbar relative h-full min-h-0 min-w-0 flex-1 overflow-y-auto overflow-x-clip overscroll-contain border-b border-border w-full [contain:strict] [overflow-anchor:none] [will-change:scroll-position] md:border-b-0 [&_diffs-container]:overflow-clip [&_diffs-container]:[contain:layout_paint_style] [&_diffs-container]:shadow-[0_-1px_0_var(--diffshub-diff-separator,var(--color-border-opaque)),0_1px_0_var(--diffshub-diff-separator,var(--color-border-opaque))]' )} options={options} - style={annotationThemeStyle} + style={viewerStyle} selectedLines={selectedLines} onSelectedLinesChange={handleSetSelection} renderAnnotation={renderCommentAnnotation} diff --git a/apps/diffshub/components/ReviewUI.tsx b/apps/diffshub/components/ReviewUI.tsx index 2d66f5f6b..a77e9ebf4 100644 --- a/apps/diffshub/components/ReviewUI.tsx +++ b/apps/diffshub/components/ReviewUI.tsx @@ -27,6 +27,7 @@ import { import { preloadAvatars } from '@/lib/annotation'; import { type DiffsHubDisplayPreferences, + getDiffsHubCodeFontFamily, useDiffsHubDisplayPreferences, } from '@/lib/displayPreferences'; import { removeSavedCommentSidebarEntry } from '@/lib/removeSavedCommentSidebarEntry'; @@ -68,6 +69,7 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { } = useDiffsHubDisplayPreferences(); const { collapseMode, + codeFont, diffIndicators, lineNumbers, overflow, @@ -76,6 +78,7 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { const diffStyle = forceUnifiedDiffStyle ? 'unified' : displayPreferences.diffStyle; + const codeFontFamily = getDiffsHubCodeFontFamily(codeFont); // All theming state — color mode and the light/dark theme-name picks — lives // in the single @pierre/theming controller (the same instance the app-wide // ThemeProvider is bound to). Reading it here means picking Auto/Light/Dark @@ -197,6 +200,12 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { }, [updateDisplayPreference] ); + const setCodeFont = useCallback( + (value: SetStateAction) => { + updateDisplayPreference('codeFont', value); + }, + [updateDisplayPreference] + ); const setDiffIndicators = useCallback( (value: SetStateAction) => { updateDisplayPreference('diffIndicators', value); @@ -304,6 +313,7 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { [ + normalizeFontQuery(alias), + compactFontKey(alias), + ]) +); + +interface KnownInstalledCodeFont { + aliases: readonly string[]; + family: string; +} + +const KNOWN_INSTALLED_CODE_FONTS = [ + { + aliases: [ + 'jetbrains', + 'jetbrains mono', + 'jetbrains-mono', + 'jetbrainsmono', + 'jb mono', + 'jbmono', + ], + family: 'JetBrains Mono', + }, + { + aliases: ['fira', 'fira code', 'fira-code', 'firacode'], + family: 'Fira Code', + }, + { + aliases: ['cascadia', 'cascadia code', 'cascadia-code', 'cascadiacode'], + family: 'Cascadia Code', + }, + { + aliases: [ + 'ibm plex', + 'ibm plex mono', + 'ibmplexmono', + 'plex mono', + 'plexmono', + ], + family: 'IBM Plex Mono', + }, + { + aliases: [ + 'adobe source code pro', + 'source code pro', + 'source-code-pro', + 'sourcecodepro', + ], + family: 'Source Code Pro', + }, + { + aliases: ['roboto mono', 'roboto-mono', 'robotomono'], + family: 'Roboto Mono', + }, + { + aliases: ['sf mono', 'sf-mono', 'sfmono'], + family: 'SF Mono', + }, +] satisfies readonly KnownInstalledCodeFont[]; +const FONT_ALIAS_INDEX = buildFontAliasIndex(KNOWN_INSTALLED_CODE_FONTS); + +export const DIFFS_HUB_CODE_FONT_OPTIONS = [ + { + fontFamily: DEFAULT_CODE_FONT_FAMILY, + label: 'Default', + value: 'default', + }, +] satisfies readonly DiffsHubCodeFontOption[]; + const COLLAPSE_MODE_VALUES = [ 'expanded', 'collapsed', @@ -129,6 +234,69 @@ export function writeDiffsHubDisplayPreferences( ); } +export function getDiffsHubCodeFontFamily(font: DiffsHubCodeFont): string { + switch (font.kind) { + case 'default': + return DEFAULT_CODE_FONT_FAMILY; + case 'system': + return SYSTEM_CODE_FONT_FAMILY; + case 'custom': + return ( + getCustomCodeFontFamilyName(font.family) ?? + getDiffsHubCodeFontFamily( + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.codeFont + ) + ); + } +} + +export function getCustomCodeFontFamily(family: string): string | null { + const resolvedFont = resolveCodeFontInput(family); + if (resolvedFont == null) { + return null; + } + + return resolvedFont.kind === 'custom' + ? getCustomCodeFontFamilyName(resolvedFont.family) + : getDiffsHubCodeFontFamily(resolvedFont); +} + +export function resolveCustomCodeFontFamilyName(input: string): string | null { + const resolvedFont = resolveCodeFontInput(input); + return resolvedFont?.kind === 'custom' ? resolvedFont.family : null; +} + +export function resolveCodeFontInput(input: string): DiffsHubCodeFont | null { + const cleanedInput = cleanCustomCodeFontFamilyInput(input); + if (cleanedInput == null) { + return null; + } + + const normalizedInput = normalizeFontQuery(cleanedInput); + const compactInput = compactFontKey(cleanedInput); + if ( + SYSTEM_MONO_ALIAS_KEYS.has(normalizedInput) || + SYSTEM_MONO_ALIAS_KEYS.has(compactInput) + ) { + return { + input, + kind: 'system', + }; + } + + const knownFont = + FONT_ALIAS_INDEX.get(normalizedInput) ?? FONT_ALIAS_INDEX.get(compactInput); + return { + family: knownFont?.family ?? cleanedInput, + input, + kind: 'custom', + }; +} + +export function isDiffsHubDefaultCodeFont(value: string): value is 'default' { + return value === 'default'; +} + function parseStoredDisplayPreferences( value: unknown ): DiffsHubDisplayPreferences { @@ -148,6 +316,10 @@ function parseDisplayPreferences(value: unknown): DiffsHubDisplayPreferences { COLLAPSE_MODE_VALUES, DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.collapseMode ), + codeFont: parseCodeFont( + getObjectProperty(value, 'codeFont'), + DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.codeFont + ), diffIndicators: parseStringChoice( getObjectProperty(value, 'diffIndicators'), DIFF_INDICATOR_VALUES, @@ -192,6 +364,48 @@ function parseStringChoice( return fallback; } +function parseCodeFont( + value: unknown, + fallback: DiffsHubCodeFont +): DiffsHubCodeFont { + const kind = getObjectProperty(value, 'kind'); + if (kind === 'default') { + return { + kind: 'default', + }; + } + + if (kind === 'system') { + const input = cleanCustomCodeFontFamilyInput( + getObjectProperty(value, 'input') + ); + return input == null + ? { + kind: 'system', + } + : { + input, + kind: 'system', + }; + } + + if (kind === 'preset') { + const presetValue = getObjectProperty(value, 'value'); + return presetValue === 'default' ? { kind: 'default' } : fallback; + } + + if (kind === 'custom') { + const customInput = + cleanCustomCodeFontFamilyInput(getObjectProperty(value, 'input')) ?? + cleanCustomCodeFontFamilyInput(getObjectProperty(value, 'family')); + if (customInput != null) { + return resolveCodeFontInput(customInput) ?? fallback; + } + } + + return fallback; +} + function parseBoolean(value: unknown, fallback: boolean): boolean { return typeof value === 'boolean' ? value : fallback; } @@ -203,3 +417,113 @@ function getObjectProperty(value: unknown, property: string): unknown { return Object.getOwnPropertyDescriptor(value, property)?.value; } + +function cleanCustomCodeFontFamilyInput(value: unknown): string | null { + if (typeof value !== 'string') { + return null; + } + + const unquotedValue = removeWrappingQuotes( + removeControlCharacters(value.normalize('NFKC')).trim() + ); + const cleanedValue = unquotedValue.replaceAll(/\s+/g, ' ').trim(); + + if ( + cleanedValue.length === 0 || + cleanedValue.length > MAX_CUSTOM_CODE_FONT_LENGTH || + cleanedValue.includes(',') + ) { + return null; + } + + return cleanedValue; +} + +function getCustomCodeFontFamilyName(family: string): string | null { + const fontFamilyName = cleanCustomCodeFontFamilyInput(family); + if (fontFamilyName == null) { + return null; + } + + return `${quoteCSSString(fontFamilyName)}, ${CUSTOM_CODE_FONT_FALLBACK}`; +} + +function buildFontAliasIndex( + fonts: readonly KnownInstalledCodeFont[] +): ReadonlyMap { + const index = new Map(); + for (const font of fonts) { + indexFontAlias(index, font, font.family); + for (const alias of font.aliases) { + indexFontAlias(index, font, alias); + } + } + return index; +} + +function indexFontAlias( + index: Map, + font: KnownInstalledCodeFont, + alias: string +): void { + for (const key of [normalizeFontQuery(alias), compactFontKey(alias)]) { + if (key.length === 0) { + continue; + } + + const existing = index.get(key); + if (existing != null && existing.family !== font.family) { + throw new Error( + `Font alias collision for "${key}": "${existing.family}" and "${font.family}"` + ); + } + index.set(key, font); + } +} + +function normalizeFontQuery(value: string): string { + return value + .normalize('NFKC') + .replaceAll(/([a-z])([A-Z])/g, '$1 $2') + .toLowerCase() + .replaceAll(/['’]/g, '') + .replaceAll(/[^a-z0-9]+/g, ' ') + .trim() + .replaceAll(/\s+/g, ' '); +} + +function compactFontKey(value: string): string { + return normalizeFontQuery(value).replaceAll(/\s+/g, ''); +} + +function removeWrappingQuotes(value: string): string { + if (value.length < 2) { + return value; + } + + const firstCharacter = value[0]; + const lastCharacter = value.at(-1); + if ( + (firstCharacter === '"' && lastCharacter === '"') || + (firstCharacter === "'" && lastCharacter === "'") + ) { + return value.slice(1, -1); + } + + return value; +} + +function quoteCSSString(value: string): string { + return `"${value.replaceAll(/["\\]/g, '\\$&')}"`; +} + +function removeControlCharacters(value: string): string { + let result = ''; + for (const character of value) { + const code = character.charCodeAt(0); + if (code > 0x1f && code !== 0x7f) { + result += character; + } + } + return result; +} diff --git a/apps/diffshub/lib/test/displayPreferences.test.ts b/apps/diffshub/lib/test/displayPreferences.test.ts index d6daa46d9..3b4a0b912 100644 --- a/apps/diffshub/lib/test/displayPreferences.test.ts +++ b/apps/diffshub/lib/test/displayPreferences.test.ts @@ -3,11 +3,16 @@ import { describe, expect, test } from 'bun:test'; import { DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES, type DiffsHubDisplayPreferences, + getCustomCodeFontFamily, + getDiffsHubCodeFontFamily, readDiffsHubDisplayPreferences, writeDiffsHubDisplayPreferences, } from '../displayPreferences'; const STORAGE_KEY = 'diffshub.displayPreferences.v1'; +const SYSTEM_CODE_FONT_FAMILY = + 'ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace'; +const CUSTOM_CODE_FONT_FALLBACK = `var(--font-berkeley-mono), ${SYSTEM_CODE_FONT_FAMILY}`; class MemoryStorage implements Storage { private readonly values = new Map(); @@ -65,6 +70,11 @@ describe('DiffsHub display preferences', () => { const storage = new MemoryStorage(); const preferences: DiffsHubDisplayPreferences = { collapseMode: 'collapsed', + codeFont: { + family: 'JetBrains Mono', + input: 'jetbrains', + kind: 'custom', + }, diffIndicators: 'classic', diffStyle: 'unified', lineNumbers: false, @@ -86,6 +96,10 @@ describe('DiffsHub display preferences', () => { JSON.stringify({ preferences: { collapseMode: 'closed', + codeFont: { + kind: 'legacy', + value: 'system-mono', + }, diffIndicators: 'classic', diffStyle: 'side-by-side', lineNumbers: false, @@ -113,6 +127,10 @@ describe('DiffsHub display preferences', () => { JSON.stringify({ preferences: { collapseMode: 'collapsed', + codeFont: { + input: 'system', + kind: 'system', + }, diffIndicators: 'none', diffStyle: 'unified', lineNumbers: false, @@ -129,6 +147,69 @@ describe('DiffsHub display preferences', () => { ); }); }); + + test('formats custom installed font names as a quoted font-family fallback stack', () => { + for (const input of [ + 'jetbrains', + 'jetbrainsmono', + 'jetbrains mono', + 'jetbrains-mono', + 'JetBrainsMono', + 'JETBRAINS MONO', + ]) { + expect(getCustomCodeFontFamily(input)).toBe( + `"JetBrains Mono", ${CUSTOM_CODE_FONT_FALLBACK}` + ); + } + + expect(getCustomCodeFontFamily('Vendor "Mono" \\ Preview\n')).toBe( + `"Vendor \\"Mono\\" \\\\ Preview", ${CUSTOM_CODE_FONT_FALLBACK}` + ); + + expect(getCustomCodeFontFamily('sourcecodepro')).toBe( + `"Source Code Pro", ${CUSTOM_CODE_FONT_FALLBACK}` + ); + + for (const input of [ + 'system', + 'monospace', + 'system mono', + 'system monospace', + 'ui monospace', + ]) { + expect(getCustomCodeFontFamily(input)).toBe(SYSTEM_CODE_FONT_FAMILY); + } + + expect( + getDiffsHubCodeFontFamily({ + family: 'Commit Mono', + input: 'Commit Mono', + kind: 'custom', + }) + ).toBe(`"Commit Mono", ${CUSTOM_CODE_FONT_FALLBACK}`); + + expect( + getDiffsHubCodeFontFamily({ + input: 'monospace', + kind: 'system', + }) + ).toBe(SYSTEM_CODE_FONT_FAMILY); + + expect(getCustomCodeFontFamily('JetBrains Mono, serif')).toBeNull(); + expect(getCustomCodeFontFamily('x'.repeat(81))).toBeNull(); + + expect( + getDiffsHubCodeFontFamily({ + family: 'JetBrains Mono, serif', + input: 'JetBrains Mono, serif', + kind: 'custom', + }) + ).toBe( + getDiffsHubCodeFontFamily(DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.codeFont) + ); + + expect(getCustomCodeFontFamily(' ')).toBeNull(); + }); }); function withLocalStorage(storage: Storage, callback: () => void): void { diff --git a/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx b/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx index 76580b091..5da90dec7 100644 --- a/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx +++ b/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx @@ -31,6 +31,9 @@ index 1111111..2222222 100644 const originalGlobals = { CSSStyleSheet: Reflect.get(globalThis, 'CSSStyleSheet'), + CustomEvent: Reflect.get(globalThis, 'CustomEvent'), + Element: Reflect.get(globalThis, 'Element'), + Event: Reflect.get(globalThis, 'Event'), Image: Reflect.get(globalThis, 'Image'), cancelAnimationFrame: Reflect.get(globalThis, 'cancelAnimationFrame'), customElements: Reflect.get(globalThis, 'customElements'), @@ -40,6 +43,7 @@ const originalGlobals = { HTMLButtonElement: Reflect.get(globalThis, 'HTMLButtonElement'), HTMLDivElement: Reflect.get(globalThis, 'HTMLDivElement'), HTMLElement: Reflect.get(globalThis, 'HTMLElement'), + HTMLInputElement: Reflect.get(globalThis, 'HTMLInputElement'), HTMLStyleElement: Reflect.get(globalThis, 'HTMLStyleElement'), HTMLTemplateElement: Reflect.get(globalThis, 'HTMLTemplateElement'), IntersectionObserver: Reflect.get(globalThis, 'IntersectionObserver'), @@ -51,6 +55,7 @@ const originalGlobals = { matchMedia: Reflect.get(globalThis, 'matchMedia'), MutationObserver: Reflect.get(globalThis, 'MutationObserver'), navigator: Reflect.get(globalThis, 'navigator'), + Node: Reflect.get(globalThis, 'Node'), requestAnimationFrame: Reflect.get(globalThis, 'requestAnimationFrame'), ResizeObserver: Reflect.get(globalThis, 'ResizeObserver'), ShadowRoot: Reflect.get(globalThis, 'ShadowRoot'), @@ -72,6 +77,16 @@ const dom = new JSDOM('', { url: 'http://localhost', virtualConsole, }); +const originalHTMLElementPrototype = { + attachEvent: Object.getOwnPropertyDescriptor( + dom.window.HTMLElement.prototype, + 'attachEvent' + ), + detachEvent: Object.getOwnPropertyDescriptor( + dom.window.HTMLElement.prototype, + 'detachEvent' + ), +}; let mobileMatches = false; type MediaListener = @@ -101,9 +116,45 @@ class MockCSSStyleSheet { replaceSync(_cssText: string): void {} } +function defineHTMLElementEventShim( + property: keyof typeof originalHTMLElementPrototype +): void { + const isAttachEvent = property === 'attachEvent'; + Object.defineProperty(dom.window.HTMLElement.prototype, property, { + configurable: true, + value( + this: HTMLElement, + eventName: string, + listener: EventListenerOrEventListenerObject + ) { + const type = eventName.startsWith('on') ? eventName.slice(2) : eventName; + if (isAttachEvent) { + this.addEventListener(type, listener); + } else { + this.removeEventListener(type, listener); + } + }, + }); +} + +function restoreHTMLElementEventShim( + property: keyof typeof originalHTMLElementPrototype +): void { + const descriptor = originalHTMLElementPrototype[property]; + if (descriptor == null) { + Reflect.deleteProperty(dom.window.HTMLElement.prototype, property); + return; + } + + Object.defineProperty(dom.window.HTMLElement.prototype, property, descriptor); +} + beforeAll(() => { Object.assign(globalThis, { CSSStyleSheet: MockCSSStyleSheet, + CustomEvent: dom.window.CustomEvent, + Element: dom.window.Element, + Event: dom.window.Event, cancelAnimationFrame: dom.window.cancelAnimationFrame.bind(dom.window), customElements: dom.window.customElements, document: dom.window.document, @@ -112,6 +163,7 @@ beforeAll(() => { HTMLButtonElement: dom.window.HTMLButtonElement, HTMLDivElement: dom.window.HTMLDivElement, HTMLElement: dom.window.HTMLElement, + HTMLInputElement: dom.window.HTMLInputElement, HTMLStyleElement: dom.window.HTMLStyleElement, HTMLTemplateElement: dom.window.HTMLTemplateElement, Image: dom.window.Image, @@ -120,6 +172,7 @@ beforeAll(() => { matchMedia, MutationObserver: dom.window.MutationObserver, navigator: dom.window.navigator, + Node: dom.window.Node, requestAnimationFrame: dom.window.requestAnimationFrame.bind(dom.window), ResizeObserver: MockResizeObserver, ShadowRoot: dom.window.ShadowRoot, @@ -135,6 +188,8 @@ beforeAll(() => { matchMedia, ResizeObserver: MockResizeObserver, }); + defineHTMLElementEventShim('attachEvent'); + defineHTMLElementEventShim('detachEvent'); }); beforeEach(() => { @@ -152,6 +207,8 @@ afterAll(() => { Object.assign(globalThis, { [key]: value }); } } + restoreHTMLElementEventShim('attachEvent'); + restoreHTMLElementEventShim('detachEvent'); dom.window.close(); }); @@ -231,9 +288,104 @@ describe('ReviewUI display preferences', () => { await cleanup(rendered); } }); + + test('hydrates, applies, and persists the custom code font preference', async () => { + writeStoredPreferences({ + codeFont: { + family: 'jetbrains', + kind: 'custom', + }, + collapseMode: 'expanded', + diffIndicators: 'bars', + diffStyle: 'split', + lineNumbers: true, + overflow: 'scroll', + showBackgrounds: true, + }); + + const rendered = await renderReviewUI(); + + try { + await waitFor(() => { + expect(readViewerFontFamily(rendered.container)).toBe( + '"JetBrains Mono", var(--font-berkeley-mono), ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace' + ); + }); + + const settingsButton = await waitForElement( + rendered.container, + 'button[aria-label="Display settings"]' + ); + await act(async () => { + pointerClick(settingsButton); + await flushReact(); + }); + + const customButton = await waitForElement( + document, + 'button[title="custom"]' + ); + await act(async () => { + customButton.click(); + await flushReact(); + }); + + const customInput = await waitForElement( + document, + 'input[placeholder="JetBrains Mono"]' + ); + expect(document.activeElement).toBe(customInput); + + await act(async () => { + setInputValue(customInput, 'sourcecodepro'); + await flushReact(); + }); + + await waitFor(() => { + expect(readStoredCodeFont()).toEqual({ + family: 'Source Code Pro', + input: 'sourcecodepro', + kind: 'custom', + }); + expect(readViewerFontFamily(rendered.container)).toBe( + '"Source Code Pro", var(--font-berkeley-mono), ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace' + ); + }); + + await act(async () => { + setInputValue(customInput, 'monospace'); + await flushReact(); + }); + + await waitFor(() => { + expect(readStoredCodeFont()).toEqual({ + input: 'monospace', + kind: 'system', + }); + expect(readViewerFontFamily(rendered.container)).toBe( + 'ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace' + ); + }); + } finally { + await cleanup(rendered); + } + }); }); interface StoredPreferences { + codeFont?: + | { + kind: 'default'; + } + | { + input?: string; + kind: 'system'; + } + | { + family: string; + input?: string; + kind: 'custom'; + }; collapseMode: 'expanded' | 'collapsed'; diffIndicators: 'bars' | 'classic' | 'none'; diffStyle: 'split' | 'unified'; @@ -300,6 +452,47 @@ function readStoredDiffStyle(): string | undefined { ).preferences?.diffStyle; } +function readStoredCodeFont(): unknown { + const rawValue = localStorage.getItem(DISPLAY_PREFERENCES_STORAGE_KEY); + if (rawValue == null) { + return undefined; + } + + return ( + JSON.parse(rawValue) as { + preferences?: { codeFont?: unknown }; + } + ).preferences?.codeFont; +} + +function readViewerFontFamily(container: ParentNode): string | null { + return ( + container + .querySelector('[style*="--diffs-font-family"]') + ?.style.getPropertyValue('--diffs-font-family') ?? null + ); +} + +function setInputValue(input: HTMLInputElement, value: string): void { + const valueSetter = Object.getOwnPropertyDescriptor( + dom.window.HTMLInputElement.prototype, + 'value' + )?.set; + if (valueSetter == null) { + throw new Error('Expected HTMLInputElement.value setter to exist'); + } + + valueSetter.call(input, value); + const propertyChangeEvent = new dom.window.Event('propertychange', { + bubbles: true, + }); + Object.defineProperty(propertyChangeEvent, 'propertyName', { + value: 'value', + }); + input.dispatchEvent(propertyChangeEvent); + input.dispatchEvent(new dom.window.Event('input', { bubbles: true })); +} + function fetchPatch(): Promise { return Promise.resolve({ body: null, @@ -421,6 +614,25 @@ async function waitForElement( return element; } +function pointerClick(element: HTMLElement): void { + const pointerDown = new dom.window.MouseEvent('pointerdown', { + bubbles: true, + button: 0, + buttons: 1, + cancelable: true, + }); + Object.defineProperty(pointerDown, 'pointerType', { value: 'mouse' }); + element.dispatchEvent(pointerDown); + element.dispatchEvent( + new dom.window.MouseEvent('pointerup', { + bubbles: true, + button: 0, + cancelable: true, + }) + ); + element.click(); +} + function querySelectorDeep( root: ParentNode, selector: string From 134e7cf4ac592180e34222b73855f3cf5d2d3a94 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Sat, 20 Jun 2026 03:35:16 +1000 Subject: [PATCH 4/4] refactor(diffshub): simplify code font preference state --- apps/diffshub/components/DiffsHubHeader.tsx | 38 +++--- apps/diffshub/components/ReviewUI.tsx | 66 +---------- apps/diffshub/lib/displayPreferences.ts | 110 ++++++++++++------ .../lib/test/displayPreferences.test.ts | 7 +- .../test/reviewDisplayPreferences.test.tsx | 12 +- 5 files changed, 102 insertions(+), 131 deletions(-) diff --git a/apps/diffshub/components/DiffsHubHeader.tsx b/apps/diffshub/components/DiffsHubHeader.tsx index da3252d99..0c0a74590 100644 --- a/apps/diffshub/components/DiffsHubHeader.tsx +++ b/apps/diffshub/components/DiffsHubHeader.tsx @@ -20,9 +20,7 @@ import { type ColorMode } from '@pierre/theming'; import Link from 'next/link'; import { type CSSProperties, - type Dispatch, memo, - type SetStateAction, useCallback, useLayoutEffect, useMemo, @@ -49,7 +47,6 @@ import { DIFFS_HUB_CODE_FONT_OPTIONS, type DiffsHubCodeFont, isDiffsHubDefaultCodeFont, - resolveCodeFontInput, } from '@/lib/displayPreferences'; import { diffshubChromeMapping } from '@/lib/theme/diffshubChromeMapping'; import { getDropdownThemeStyle } from '@/lib/theme/dropdownChromeStyle'; @@ -76,15 +73,15 @@ interface HeaderProps { overflow: 'wrap' | 'scroll'; onToggleCollapseMode(): void; onToggleFileTreeOverlay(): void; - setCodeFont: Dispatch>; + setCodeFont(value: DiffsHubCodeFont): void; setColorMode(mode: ColorMode): void; setDarkThemeName(name: DarkThemeName): void; - setDiffIndicators: Dispatch>; - setDiffStyle: Dispatch>; + setDiffIndicators(value: DiffIndicators): void; + setDiffStyle(value: 'split' | 'unified'): void; setLightThemeName(name: LightThemeName): void; - setLineNumbers: Dispatch>; - setOverflow: Dispatch>; - setShowBackgrounds: Dispatch>; + setLineNumbers(value: boolean): void; + setOverflow(value: 'wrap' | 'scroll'): void; + setShowBackgrounds(value: boolean): void; showBackgrounds: boolean; } @@ -118,10 +115,7 @@ export const DiffsHubHeader = memo(function DiffsHubHeader({ const [currentUrl, setCurrentUrl] = useState(initialUrl); const codeFontSelection = codeFont.kind === 'default' ? 'default' : 'custom'; const customCodeFontFamily = - codeFont.kind === 'default' - ? '' - : (codeFont.input ?? - (codeFont.kind === 'system' ? 'System monospace' : codeFont.family)); + codeFont.kind === 'default' ? '' : codeFont.input; const focusCustomCodeFontInput = useCallback( (input: HTMLInputElement | null) => { if (input == null || document.activeElement === input) { @@ -322,11 +316,10 @@ export const DiffsHubHeader = memo(function DiffsHubHeader({ } if (value === 'custom') { - setCodeFont((previous) => - previous.kind !== 'default' - ? previous + setCodeFont( + codeFont.kind !== 'default' + ? codeFont : { - family: '', input: '', kind: 'custom', } @@ -360,13 +353,10 @@ export const DiffsHubHeader = memo(function DiffsHubHeader({ spellCheck={false} value={customCodeFontFamily} onChange={({ currentTarget }) => { - setCodeFont( - resolveCodeFontInput(currentTarget.value) ?? { - family: '', - input: currentTarget.value, - kind: 'custom', - } - ); + setCodeFont({ + input: currentTarget.value, + kind: 'custom', + }); }} onKeyDown={(event) => event.stopPropagation()} /> diff --git a/apps/diffshub/components/ReviewUI.tsx b/apps/diffshub/components/ReviewUI.tsx index a77e9ebf4..1db36d77f 100644 --- a/apps/diffshub/components/ReviewUI.tsx +++ b/apps/diffshub/components/ReviewUI.tsx @@ -1,12 +1,10 @@ 'use client'; -import { type DiffIndicators } from '@pierre/diffs'; import { type CodeViewHandle, useWorkerPool } from '@pierre/diffs/react'; import { type ColorMode } from '@pierre/theming'; import { useThemeController } from '@pierre/theming/react'; import { type ReactNode, - type SetStateAction, useCallback, useEffect, useRef, @@ -26,7 +24,6 @@ import { } from '@/components/themeController'; import { preloadAvatars } from '@/lib/annotation'; import { - type DiffsHubDisplayPreferences, getDiffsHubCodeFontFamily, useDiffsHubDisplayPreferences, } from '@/lib/displayPreferences'; @@ -65,6 +62,12 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { const { displayPreferences, displayPreferencesHydrated, + setCodeFont, + setDiffIndicators, + setDiffStyle, + setLineNumbers, + setOverflow, + setShowBackgrounds, updateDisplayPreferences, } = useDiffsHubDisplayPreferences(); const { @@ -173,63 +176,6 @@ function ReviewUIInner({ domain, initialUrl, path }: ReviewUIProps) { mediaQuery.addEventListener('change', handleChange); return () => mediaQuery.removeEventListener('change', handleChange); }, []); - const updateDisplayPreference = useCallback( - ( - key: Key, - value: SetStateAction - ) => { - updateDisplayPreferences((previous) => { - const previousValue = previous[key]; - const nextValue = - typeof value === 'function' - ? ( - value as (current: typeof previousValue) => typeof previousValue - )(previousValue) - : value; - return { - ...previous, - [key]: nextValue, - }; - }); - }, - [updateDisplayPreferences] - ); - const setDiffStyle = useCallback( - (value: SetStateAction<'split' | 'unified'>) => { - updateDisplayPreference('diffStyle', value); - }, - [updateDisplayPreference] - ); - const setCodeFont = useCallback( - (value: SetStateAction) => { - updateDisplayPreference('codeFont', value); - }, - [updateDisplayPreference] - ); - const setDiffIndicators = useCallback( - (value: SetStateAction) => { - updateDisplayPreference('diffIndicators', value); - }, - [updateDisplayPreference] - ); - const setLineNumbers = useCallback( - (value: SetStateAction) => { - updateDisplayPreference('lineNumbers', value); - }, - [updateDisplayPreference] - ); - const setOverflow = useCallback( - (value: SetStateAction<'wrap' | 'scroll'>) => { - updateDisplayPreference('overflow', value); - }, - [updateDisplayPreference] - ); - const setShowBackgrounds = useCallback( - (value: SetStateAction) => { - updateDisplayPreference('showBackgrounds', value); - }, - [updateDisplayPreference] - ); const handleSelectTreeItem = useCallback((itemId: string) => { setFileTreeOverlayOpen(false); const viewer = viewerRef.current; diff --git a/apps/diffshub/lib/displayPreferences.ts b/apps/diffshub/lib/displayPreferences.ts index 043282859..c431143bb 100644 --- a/apps/diffshub/lib/displayPreferences.ts +++ b/apps/diffshub/lib/displayPreferences.ts @@ -12,12 +12,7 @@ export type DiffsHubCodeFont = kind: 'default'; } | { - input?: string; - kind: 'system'; - } - | { - family: string; - input?: string; + input: string; kind: 'custom'; }; export type DiffsHubDiffStyle = 'split' | 'unified'; @@ -162,11 +157,26 @@ interface StoredDisplayPreferences { interface UseDiffsHubDisplayPreferencesResult { displayPreferences: DiffsHubDisplayPreferences; displayPreferencesHydrated: boolean; + setCodeFont(value: DiffsHubDisplayPreferences['codeFont']): void; + setDiffIndicators(value: DiffIndicators): void; + setDiffStyle(value: DiffsHubDiffStyle): void; + setLineNumbers(value: boolean): void; + setOverflow(value: DiffsHubOverflow): void; + setShowBackgrounds(value: boolean): void; updateDisplayPreferences( update: (previous: DiffsHubDisplayPreferences) => DiffsHubDisplayPreferences ): void; } +type ResolvedCodeFontInput = + | { + kind: 'system'; + } + | { + family: string; + kind: 'custom'; + }; + export function useDiffsHubDisplayPreferences(): UseDiffsHubDisplayPreferencesResult { const [displayPreferences, setDisplayPreferences] = useState(DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES); @@ -196,10 +206,64 @@ export function useDiffsHubDisplayPreferences(): UseDiffsHubDisplayPreferencesRe }, [] ); + const updateDisplayPreference = useCallback( + ( + key: Key, + value: DiffsHubDisplayPreferences[Key] + ) => { + updateDisplayPreferences((previous) => ({ + ...previous, + [key]: value, + })); + }, + [updateDisplayPreferences] + ); + const setCodeFont = useCallback( + (value: DiffsHubDisplayPreferences['codeFont']) => { + updateDisplayPreference('codeFont', value); + }, + [updateDisplayPreference] + ); + const setDiffIndicators = useCallback( + (value: DiffIndicators) => { + updateDisplayPreference('diffIndicators', value); + }, + [updateDisplayPreference] + ); + const setDiffStyle = useCallback( + (value: DiffsHubDiffStyle) => { + updateDisplayPreference('diffStyle', value); + }, + [updateDisplayPreference] + ); + const setLineNumbers = useCallback( + (value: boolean) => { + updateDisplayPreference('lineNumbers', value); + }, + [updateDisplayPreference] + ); + const setOverflow = useCallback( + (value: DiffsHubOverflow) => { + updateDisplayPreference('overflow', value); + }, + [updateDisplayPreference] + ); + const setShowBackgrounds = useCallback( + (value: boolean) => { + updateDisplayPreference('showBackgrounds', value); + }, + [updateDisplayPreference] + ); return { displayPreferences, displayPreferencesHydrated, + setCodeFont, + setDiffIndicators, + setDiffStyle, + setLineNumbers, + setOverflow, + setShowBackgrounds, updateDisplayPreferences, }; } @@ -238,11 +302,9 @@ export function getDiffsHubCodeFontFamily(font: DiffsHubCodeFont): string { switch (font.kind) { case 'default': return DEFAULT_CODE_FONT_FAMILY; - case 'system': - return SYSTEM_CODE_FONT_FAMILY; case 'custom': return ( - getCustomCodeFontFamilyName(font.family) ?? + getCustomCodeFontFamily(font.input) ?? getDiffsHubCodeFontFamily( DEFAULT_DIFFS_HUB_DISPLAY_PREFERENCES.codeFont ) @@ -258,15 +320,10 @@ export function getCustomCodeFontFamily(family: string): string | null { return resolvedFont.kind === 'custom' ? getCustomCodeFontFamilyName(resolvedFont.family) - : getDiffsHubCodeFontFamily(resolvedFont); + : SYSTEM_CODE_FONT_FAMILY; } -export function resolveCustomCodeFontFamilyName(input: string): string | null { - const resolvedFont = resolveCodeFontInput(input); - return resolvedFont?.kind === 'custom' ? resolvedFont.family : null; -} - -export function resolveCodeFontInput(input: string): DiffsHubCodeFont | null { +function resolveCodeFontInput(input: string): ResolvedCodeFontInput | null { const cleanedInput = cleanCustomCodeFontFamilyInput(input); if (cleanedInput == null) { return null; @@ -279,7 +336,6 @@ export function resolveCodeFontInput(input: string): DiffsHubCodeFont | null { SYSTEM_MONO_ALIAS_KEYS.has(compactInput) ) { return { - input, kind: 'system', }; } @@ -288,7 +344,6 @@ export function resolveCodeFontInput(input: string): DiffsHubCodeFont | null { FONT_ALIAS_INDEX.get(normalizedInput) ?? FONT_ALIAS_INDEX.get(compactInput); return { family: knownFont?.family ?? cleanedInput, - input, kind: 'custom', }; } @@ -375,20 +430,6 @@ function parseCodeFont( }; } - if (kind === 'system') { - const input = cleanCustomCodeFontFamilyInput( - getObjectProperty(value, 'input') - ); - return input == null - ? { - kind: 'system', - } - : { - input, - kind: 'system', - }; - } - if (kind === 'preset') { const presetValue = getObjectProperty(value, 'value'); return presetValue === 'default' ? { kind: 'default' } : fallback; @@ -399,7 +440,10 @@ function parseCodeFont( cleanCustomCodeFontFamilyInput(getObjectProperty(value, 'input')) ?? cleanCustomCodeFontFamilyInput(getObjectProperty(value, 'family')); if (customInput != null) { - return resolveCodeFontInput(customInput) ?? fallback; + return { + input: customInput, + kind: 'custom', + }; } } diff --git a/apps/diffshub/lib/test/displayPreferences.test.ts b/apps/diffshub/lib/test/displayPreferences.test.ts index 3b4a0b912..08c2d8de6 100644 --- a/apps/diffshub/lib/test/displayPreferences.test.ts +++ b/apps/diffshub/lib/test/displayPreferences.test.ts @@ -71,7 +71,6 @@ describe('DiffsHub display preferences', () => { const preferences: DiffsHubDisplayPreferences = { collapseMode: 'collapsed', codeFont: { - family: 'JetBrains Mono', input: 'jetbrains', kind: 'custom', }, @@ -129,7 +128,7 @@ describe('DiffsHub display preferences', () => { collapseMode: 'collapsed', codeFont: { input: 'system', - kind: 'system', + kind: 'custom', }, diffIndicators: 'none', diffStyle: 'unified', @@ -182,7 +181,6 @@ describe('DiffsHub display preferences', () => { expect( getDiffsHubCodeFontFamily({ - family: 'Commit Mono', input: 'Commit Mono', kind: 'custom', }) @@ -191,7 +189,7 @@ describe('DiffsHub display preferences', () => { expect( getDiffsHubCodeFontFamily({ input: 'monospace', - kind: 'system', + kind: 'custom', }) ).toBe(SYSTEM_CODE_FONT_FAMILY); @@ -200,7 +198,6 @@ describe('DiffsHub display preferences', () => { expect( getDiffsHubCodeFontFamily({ - family: 'JetBrains Mono, serif', input: 'JetBrains Mono, serif', kind: 'custom', }) diff --git a/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx b/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx index 5da90dec7..6debe644b 100644 --- a/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx +++ b/apps/diffshub/lib/test/reviewDisplayPreferences.test.tsx @@ -292,7 +292,7 @@ describe('ReviewUI display preferences', () => { test('hydrates, applies, and persists the custom code font preference', async () => { writeStoredPreferences({ codeFont: { - family: 'jetbrains', + input: 'jetbrains', kind: 'custom', }, collapseMode: 'expanded', @@ -343,7 +343,6 @@ describe('ReviewUI display preferences', () => { await waitFor(() => { expect(readStoredCodeFont()).toEqual({ - family: 'Source Code Pro', input: 'sourcecodepro', kind: 'custom', }); @@ -360,7 +359,7 @@ describe('ReviewUI display preferences', () => { await waitFor(() => { expect(readStoredCodeFont()).toEqual({ input: 'monospace', - kind: 'system', + kind: 'custom', }); expect(readViewerFontFamily(rendered.container)).toBe( 'ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace' @@ -378,12 +377,7 @@ interface StoredPreferences { kind: 'default'; } | { - input?: string; - kind: 'system'; - } - | { - family: string; - input?: string; + input: string; kind: 'custom'; }; collapseMode: 'expanded' | 'collapsed';