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/2] 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/2] 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(); +}