From 14f34712b25f138be2af09b1006592bcdb767207 Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Sun, 7 Jun 2026 20:01:39 +0300 Subject: [PATCH] =?UTF-8?q?Add=20live-preview=20busy=20indicator=20for=20s?= =?UTF-8?q?low=20renders=20(M6,=20=C2=A704/=C2=A710)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/app/components/LivePreview.module.css | 44 ++++++++++++ src/app/components/LivePreview.test.tsx | 87 +++++++++++++++++++++++ src/app/components/LivePreview.tsx | 69 +++++++++++++++--- src/app/stores/PreviewStore.test.ts | 54 ++++++++++++++ src/app/stores/PreviewStore.ts | 15 ++++ 5 files changed, 261 insertions(+), 8 deletions(-) create mode 100644 src/app/components/LivePreview.test.tsx create mode 100644 src/app/stores/PreviewStore.test.ts diff --git a/src/app/components/LivePreview.module.css b/src/app/components/LivePreview.module.css index f154807..b861919 100644 --- a/src/app/components/LivePreview.module.css +++ b/src/app/components/LivePreview.module.css @@ -18,6 +18,7 @@ } .body { + position: relative; /* establishes stacking context for the busy overlay */ flex: 1 1 auto; min-height: 0; overflow: auto; @@ -88,3 +89,46 @@ white-space: pre-wrap; word-break: break-word; } + +/* + * Busy overlay — non-blocking; floats over the chart body only (arch §10.2; + * spec §04/§10). position:absolute relative to .body's position:relative. + * pointer-events:none ensures the editor and other controls stay fully + * interactive while the overlay is visible. + * + * The spinner animation is a pure CSS transform-based rotation so it respects + * the global prefers-reduced-motion rule in base.css (animation-duration → 0.01ms). + */ +.busyOverlay { + position: absolute; + inset: 0; + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: var(--space-3); + background: color-mix(in srgb, var(--bg) 75%, transparent); + pointer-events: none; +} + +.busySpinner { + display: block; + width: var(--icon-lg); + height: var(--icon-lg); + border: 2px solid var(--border-strong); + border-top-color: var(--accent); + border-radius: 50%; + animation: busySpin 600ms linear infinite; +} + +.busyLabel { + font-size: 12px; + color: var(--text-secondary); + line-height: 1.4; +} + +@keyframes busySpin { + to { + transform: rotate(360deg); + } +} diff --git a/src/app/components/LivePreview.test.tsx b/src/app/components/LivePreview.test.tsx new file mode 100644 index 0000000..71d8182 --- /dev/null +++ b/src/app/components/LivePreview.test.tsx @@ -0,0 +1,87 @@ +/** + * LivePreview — busy overlay guard (spec §04; arch §10.2). + * + * The render pipeline is integration-heavy (vega-embed, IndexedDB, Monaco); the + * busy OVERLAY itself is purely a function of `PreviewStore.busy`. These tests + * set that flag directly and assert the DOM result — no timing, no mocking of the + * async render path. + */ + +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { usePreviewStore } from '../stores/PreviewStore'; +import { useSnippetStore } from '../stores/SnippetStore'; +import { useDatasetStore } from '../stores/DatasetStore'; +import { LivePreview } from './LivePreview'; + +// Heavy services and sub-components the LivePreview wires — not under test here. +vi.mock('../services/chart-renderer', () => ({ + renderSpec: () => Promise.resolve({ destroy() {}, resize() {} }), +})); +vi.mock('./SettingsPopover', () => ({ + SettingsPopover: () => null, + SettingRow: () => null, + RangeControl: () => null, +})); + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + usePreviewStore.setState({ error: null, busy: false }); + useSnippetStore.getState().reset(); + useDatasetStore.getState().reset(); + + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => root.render()); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + usePreviewStore.setState({ error: null, busy: false }); + vi.clearAllMocks(); +}); + +describe('LivePreview busy overlay', () => { + test('does not render the busy overlay when busy=false', () => { + // The overlay element should not be in the DOM at all during normal operation. + expect(container.querySelector('[aria-hidden="true"]')).toBeNull(); + }); + + test('renders the busy overlay when PreviewStore.busy=true', () => { + act(() => usePreviewStore.setState({ busy: true })); + const overlay = container.querySelector('[aria-hidden="true"]'); + expect(overlay).not.toBeNull(); + }); + + test('overlay carries a visible label for sighted users', () => { + act(() => usePreviewStore.setState({ busy: true })); + const label = container.querySelector('[aria-hidden="true"]')?.textContent; + expect(label).toMatch(/rendering/i); + }); + + test('the preview body carries aria-busy=true when busy', () => { + act(() => usePreviewStore.setState({ busy: true })); + // The body element has aria-busy when the store says busy. + const busyEl = container.querySelector('[aria-busy="true"]'); + expect(busyEl).not.toBeNull(); + }); + + test('aria-busy is absent when busy=false (no aria-busy="false" noise)', () => { + // aria-busy="false" is technically valid but needlessly verbose; we omit it. + expect(container.querySelector('[aria-busy]')).toBeNull(); + }); + + test('overlay disappears when busy returns to false', () => { + act(() => usePreviewStore.setState({ busy: true })); + expect(container.querySelector('[aria-hidden="true"]')).not.toBeNull(); + act(() => usePreviewStore.setState({ busy: false })); + expect(container.querySelector('[aria-hidden="true"]')).toBeNull(); + }); +}); diff --git a/src/app/components/LivePreview.tsx b/src/app/components/LivePreview.tsx index 3f2396c..1aa2367 100644 --- a/src/app/components/LivePreview.tsx +++ b/src/app/components/LivePreview.tsx @@ -110,11 +110,14 @@ export function LivePreview() { const lastLoadRef = useRef({ bufferEpoch: -1, editorView }); const error = usePreviewStore((s) => s.error); const setError = usePreviewStore((s) => s.setError); + const busy = usePreviewStore((s) => s.busy); + const setBusy = usePreviewStore((s) => s.setBusy); + + // Busy-indication timer ref: if a render exceeds ~1s we surface a non-blocking + // overlay (arch §10.2 NN/g: >1s owes a busy indication; <1s shows nothing to + // avoid flicker on typical fast renders). + const busyTimerRef = useRef | null>(null); - // TODO (council backfill, doc §10.2): a render that exceeds ~1s owes a - // non-blocking busy indication (overlay + aria-busy), gated by a threshold so - // sub-1s renders show nothing. Deferred — it pairs with the heavier M3 dataset - // renders the budget flags; today's inline-data renders are effectively instant. useEffect(() => { const node = hostRef.current; if (!node) return; @@ -147,6 +150,25 @@ export function LivePreview() { } const mine = ++generationRef.current; + + // Arm the busy indicator: if the render hasn't settled within ~1s, flip the + // flag. Sub-1s renders (the common case) never show the overlay — no flicker + // (arch §10.2; NN/g ≤1s = uninterrupted thought, >1s = noticeably waiting). + if (busyTimerRef.current !== null) clearTimeout(busyTimerRef.current); + busyTimerRef.current = setTimeout(() => { + // Only arm for the current generation; a superseded render doesn't own busy. + if (mine === generationRef.current) setBusy(true); + }, 1000); + + /** Clear busy and the timer unconditionally — called on settle or error. */ + const clearBusy = () => { + if (busyTimerRef.current !== null) { + clearTimeout(busyTimerRef.current); + busyTimerRef.current = null; + } + setBusy(false); + }; + try { const prepared = prepareSpecForRender(parsed, { fitMode, datasets }); const config = chartConfigFor(uiTheme); @@ -165,8 +187,10 @@ export function LivePreview() { } handleRef.current = handle; setError(null); + clearBusy(); } catch (e) { if (mine === generationRef.current) { + clearBusy(); // A missing dataset reference is not a JSON/spec problem, so it gets a // tailored, fixable message instead of the generic syntax hint (council: // GOV.UK error-message + NN/g #9 — name the problem, give the real fix). @@ -187,7 +211,17 @@ export function LivePreview() { }, delay); return () => clearTimeout(timer); - }, [shownText, fitMode, uiTheme, datasets, setError, bufferEpoch, editorView, renderDebounce]); + }, [ + shownText, + fitMode, + uiTheme, + datasets, + setError, + setBusy, + bufferEpoch, + editorView, + renderDebounce, + ]); // Re-fit the chart when its container resizes (e.g. a pane drag). Vega doesn't // observe the element, so we do: one observer on the stable host node for the @@ -205,13 +239,15 @@ export function LivePreview() { return () => ro.disconnect(); }, []); - // Finalize the live view on unmount, and clear the shared error so a stale - // message never outlives this pane. + // Finalize the live view on unmount, and clear the shared error + busy state so + // stale transient state never outlives this pane. useEffect( () => () => { handleRef.current?.destroy(); handleRef.current = null; + if (busyTimerRef.current !== null) clearTimeout(busyTimerRef.current); usePreviewStore.getState().setError(null); + usePreviewStore.getState().setBusy(false); }, [], ); @@ -222,7 +258,12 @@ export function LivePreview() { -
+ {/* + * aria-busy on the chart region tells AT the area is being updated (arch §10.2; + * spec §04). The overlay is a non-blocking sibling inside the relative-positioned + * body; it never covers the header or editor, so editing stays fully interactive. + */} +
{/* Frame is React-owned and carries the fit-sizing class; the inner host is owned by vega-embed (it brands it `.vega-embed` and mutates its classList at runtime), so its className stays static and React never @@ -234,6 +275,18 @@ export function LivePreview() { editor pane's role="alert" (one producer, two subscribers; doc §10.1), so adding one here would double-announce it. */} {error !== null && 
{error}
} + {/* + * Busy overlay: non-blocking, overlays only the chart body, never the header + * or the editor (arch §10.2; spec §04/§10). Shown only after the ~1s threshold + * so sub-1s renders produce no flicker. The spinner animation is suppressed + * under prefers-reduced-motion (base.css *{animation-duration:0.01ms}). + */} + {busy && ( + + )}
); diff --git a/src/app/stores/PreviewStore.test.ts b/src/app/stores/PreviewStore.test.ts new file mode 100644 index 0000000..ed5d361 --- /dev/null +++ b/src/app/stores/PreviewStore.test.ts @@ -0,0 +1,54 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { usePreviewStore } from './PreviewStore'; + +const store = () => usePreviewStore.getState(); + +afterEach(() => { + // Reset to a known clean state between tests so store leaks don't affect order. + store().setError(null); + store().setBusy(false); +}); + +describe('PreviewStore — error slice', () => { + it('starts with null error', () => { + expect(store().error).toBeNull(); + }); + + it('setError stores the provided message', () => { + store().setError('Rendering error: something went wrong.'); + expect(store().error).toBe('Rendering error: something went wrong.'); + }); + + it('setError(null) clears the message', () => { + store().setError('an error'); + store().setError(null); + expect(store().error).toBeNull(); + }); +}); + +describe('PreviewStore — busy slice', () => { + it('starts with busy=false', () => { + expect(store().busy).toBe(false); + }); + + it('setBusy(true) sets busy to true', () => { + store().setBusy(true); + expect(store().busy).toBe(true); + }); + + it('setBusy(false) clears busy', () => { + store().setBusy(true); + store().setBusy(false); + expect(store().busy).toBe(false); + }); + + it('busy and error are independent — setting one does not affect the other', () => { + store().setBusy(true); + store().setError('some error'); + expect(store().busy).toBe(true); + expect(store().error).toBe('some error'); + + store().setBusy(false); + expect(store().error).toBe('some error'); // error unchanged by clearing busy + }); +}); diff --git a/src/app/stores/PreviewStore.ts b/src/app/stores/PreviewStore.ts index 1288f5b..a983215 100644 --- a/src/app/stores/PreviewStore.ts +++ b/src/app/stores/PreviewStore.ts @@ -9,6 +9,11 @@ * Live Preview is the single producer; it writes the current error here and both * panes subscribe. `null` means the current spec rendered cleanly (or is blank). * + * `busy` tracks whether a render is in flight long enough to warrant a visible + * indicator (arch §10.2: >~1s owes a non-blocking busy overlay). LivePreview arms + * a 1 s timer when a render starts and sets `busy = true` only if the render has + * not settled by then; it clears `busy` on settle or error regardless. + * * Kept as its own tiny store rather than folded into the SnippetStore: this is * transient render state, not durable domain data, and it must not be persisted. */ @@ -20,9 +25,19 @@ export interface PreviewState { error: string | null; /** Set (or clear) the current render error. */ setError: (error: string | null) => void; + /** + * True while a render has been in flight for longer than the ~1s NN/g threshold + * (arch §10.2). The LivePreview overlay reads this to show a non-blocking busy + * indication; aria-busy on the preview region mirrors it. + */ + busy: boolean; + /** Set or clear the busy flag. */ + setBusy: (busy: boolean) => void; } export const usePreviewStore = create((set) => ({ error: null, setError: (error) => set({ error }), + busy: false, + setBusy: (busy) => set({ busy }), }));