From 411bfbc6c260974bae478ce77995a128150be8a2 Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Fri, 5 Jun 2026 10:45:41 +0300 Subject: [PATCH] Implement fit-mode rendering contract with container sizing and pane re-fit --- docs/IMPLEMENTATION-PLAN.md | 38 +++++- .../05-rendering-theming-preview.md | 74 +++++++++-- .../architecture/08-vega-editor-techniques.md | 40 +++--- src/app/components/LivePreview.module.css | 108 +++++++++++++++- src/app/components/LivePreview.tsx | 119 +++++++++++++++--- src/app/infrastructure/settings-store.test.ts | 28 ++++- src/app/infrastructure/settings-store.ts | 21 ++++ src/app/orchestration/preferences.ts | 26 ++++ src/app/services/chart-renderer.ts | 18 +++ src/app/stores/AppStore.ts | 7 ++ src/core/rendering.test.ts | 75 ++++++++++- src/core/rendering.ts | 65 ++++++++-- src/main.tsx | 6 + 13 files changed, 552 insertions(+), 73 deletions(-) create mode 100644 src/app/orchestration/preferences.ts diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index 0e85745..a8a4a97 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -174,7 +174,7 @@ set to Plex Mono explicitly since it can't read the CSS token. --- -## M2 · Editor robustness +## M2 · Editor robustness ✅ (done) **Goal:** the editor becomes trustworthy — draft vs published, schema-aware assistance, and the fit-mode rendering contract. @@ -185,7 +185,9 @@ assistance, and the fit-mode rendering contract. Vega-Lite `"container"`), recursing into layered/concat/child specs (spec §04 Rendering Contract, step 2). - `vega-lite-schema.ts` — provide the Vega-Lite JSON schema for Monaco's - validation/autocomplete (mine vega-editor for sourcing/versioning the schema). + validation/autocomplete. _Delivered early in M1.5 as + `infrastructure/monaco-schema.ts` (bundled schema, offline, `markdownDescription` + hover docs); no further work needed in M2._ **App** @@ -210,6 +212,30 @@ assistance, and the fit-mode rendering contract. - Invalid spec shows inline error; autocomplete suggests Vega-Lite properties. - Each fit mode resizes the chart as specified; choice survives reload. +**Verified:** `typecheck` + `test` (83 passing — `rendering` fit-mode incl. +nested layer/concat/facet specs, `SnippetStore` draft/publish/revert/editorView, +`settings-store` `previewFitMode` round-trip) + `build` (PWA, 41 precache +entries) + `eslint` clean. Implementation notes: editing now writes the +**draft** only (`commitDraft` no longer touches `spec`); `publish`/`revert` live +in `SnippetStore`, with a `bufferEpoch` counter so programmatic buffer reloads +(select/create/revert) refresh Monaco without fighting the cursor mid-typing. +The Draft/Published view is a store-level `editorView`; the published view is +read-only and the preview renders whichever version is shown (`selectShownText`). +The editor (§03E) and preview (§04) share one render error via a small +`PreviewStore`. `previewFitMode` was pulled into `AppStore` + the settings +adapter, hydrated/persisted by a new `orchestration/preferences.ts` mirroring the +theme slice. Publish/Revert **success toasts** stay deferred to M6 (TODO +breadcrumbs at the call sites), matching the existing delete-toast convention. + +Fit-mode rendering needed a layout fix: vega-embed brands the embed host with its +own `.vega-embed { display: inline-block }` (injected at runtime, wins the +cascade), which shrink-wrapped the host so `width: "container"` collapsed (Height +survived only via the old `min-height: 100%`). Fix: embed into a static-class +inner host (React never reconciles its className, so Vega's runtime classes +survive) inside a React-owned frame that carries the fit-sizing class via +two-class selectors that out-specify `.vega-embed`. All four fit modes +user-verified in the running app. + --- ## M3 · Datasets @@ -329,8 +355,12 @@ the reference. **Goal:** the workspace feels finished and meets §10. -- **Panes:** drag-resize handles with min widths; per-pane show/hide toggle strip; - widths + visibility persist (§01A, §09D). +- **Panes:** ~~drag-resize handles with min widths; widths persist~~ ✅ **pulled + forward after M2** (coupled to the preview's container sizing — see + [arch 05 §8](architecture/05-rendering-theming-preview.md)). Side panes carry + remembered widths, the editor flexes between them, widths persist to + `astrolabe:ux-prefs`. **Remaining:** per-pane show/hide **toggle strip** + + visibility persist + proportional redistribution on hide (§01A, §09D). - **Routing:** URL hash view-state (`#snippet-`, `#datasets/...`) with Back/Forward; restore on load (§01E). _(see [Architecture 04 · Routing & Events](architecture/04-routing-and-events.md))_ - **Shortcuts:** Cmd/Ctrl+Shift+N / +K / +S / +, / Esc via a single key router diff --git a/docs/architecture/05-rendering-theming-preview.md b/docs/architecture/05-rendering-theming-preview.md index c96f0a0..3e5fd7d 100644 --- a/docs/architecture/05-rendering-theming-preview.md +++ b/docs/architecture/05-rendering-theming-preview.md @@ -379,9 +379,9 @@ _Live Preview_ spec. The only invariant this doc cares about: > embeds that returned spec. **The user's stored spec is never mutated by > rendering.** -The container-relative fit modes (Width/Height/Full) depend on `renderer: 'svg'` -plus `"container"` sizing to follow the pane; when the pane resizes, re-running -`prepareSpecForRender` + re-embedding (a `flush()`) re-fits the chart. +The container-relative fit modes (Width/Height/Full) depend on `"container"` +sizing to follow the pane. Re-fitting on a **pane resize** is _not_ a re-embed: +the existing view is re-measured via a `ResizeObserver`-driven event — see §8. ### Rules @@ -463,14 +463,64 @@ manual retry, no reload. --- +## 8. Container Sizing & Pane Resize (two gotchas that cost real time) + +Vega-Lite's `"container"` sizing is responsible for the Width/Height/Full fit +modes, and it has **two non-obvious failure modes**. Both were rediscovered the +hard way; this section is the shortcut. + +### Gotcha 1 — the embed host shrink-wraps, collapsing `width:"container"` + +`vega-embed` brands the element you embed into with its own +`.vega-embed { display: inline-block }`, injected into `` at runtime so it +**wins the cascade** over a class you put on that same element. `inline-block` +shrink-wraps horizontally, and `"container"` width reads `host.clientWidth` — so +the chart collapses to near-zero width. (Height often survives because a tall box +keeps `clientHeight`, which is why the symptom is "Width broken, Height fine".) +Note also: `vega-embed` only adds its responsive `chart-wrapper` (the element its +`width:100%` rule targets) **when `actions` are enabled** — we pass +`actions: false`, so that path is dead and the host is branded directly. + +**Fix:** embed into a dedicated **inner host** with a _static_ className (React +never re-reconciles it, so Vega's runtime classes survive) nested inside a +**React-owned frame** that carries the fit-mode class. Size the host with +**two-class selectors** (`.fitWidth .host { width: 100% }`) that out-specify +`.vega-embed`. Original mode lets the host stay natural and the pane scrolls. + +### Gotcha 2 — Vega re-measures only on `window:resize` + +The compiled `width`/`height` signals re-evaluate `containerSize()` **only** on +`events: "window:resize"`. Consequences: `view.resize()` re-runs layout with the +**stale** size (it does _not_ re-measure), and a pane drag fires no window resize, +so a responsive chart does **not** follow the pane on its own. + +**Fix:** a `ResizeObserver` on the host → `window.dispatchEvent(new Event('resize'))` +(behind `RenderHandle.resize()`, keeping the Vega knowledge in the renderer). +`ResizeObserver` callbacks are frame-batched, so this tracks a drag without a +debounce. Because only the container-bound dimension carries the resize handler, +Width re-fits width and leaves height natural automatically — no fit-mode +bookkeeping. Gate the observer to responsive modes (Original needs no re-fit). + +### Rules + +- **Do** give `vega-embed` its own inner host element; never put a React-managed, + changing `className` on the element `vega-embed` brands. +- **Do** out-specify `.vega-embed` (two-class selectors) when you must size the host. +- **Do** bridge pane-resize via a synthetic `window:resize`, not `view.resize()`. +- **Don't** assume `actions: false` leaves you the responsive `chart-wrapper` — it doesn't. +- **Don't** re-embed just to re-fit a resize; re-measure the existing view. + +--- + ## Summary -| Concern | Mechanism | Source of truth | -| ------------- | ----------------------------------------------------------------------- | ----------------------------------------------- | -| Embedding | One `renderSpec` over `vega-embed`, `actions: false`, `renderer: 'svg'` | `src/app/services/chart-renderer.ts` | -| View teardown | `view.finalize()` before each re-render and on unmount | the renderer's `RenderHandle` | -| Theming | Vega `Config` per UI theme, applied at embed time | `chartConfigFor()` in `src/core/vega-themes.ts` | -| Field names | `escapeVegaField` on every data-derived `field:` | `src/core/rendering.ts` | -| Debounce | `createDebouncedRenderer`, delay from `renderDebounce` setting | `src/app/services/debounced-renderer.ts` | -| Spec prep | `prepareSpecForRender` (pure, on a copy) | `src/core/rendering.ts` (see _Live Preview_) | -| Errors | One error field, cleared on success, empty = nothing | `PreviewStore.error` | +| Concern | Mechanism | Source of truth | +| ------------- | ------------------------------------------------------------------------------------ | ----------------------------------------------- | +| Embedding | One `renderSpec` over `vega-embed`, `actions: false`, `renderer: 'svg'` | `src/app/services/chart-renderer.ts` | +| View teardown | `view.finalize()` before each re-render and on unmount | the renderer's `RenderHandle` | +| Theming | Vega `Config` per UI theme, applied at embed time | `chartConfigFor()` in `src/core/vega-themes.ts` | +| Field names | `escapeVegaField` on every data-derived `field:` | `src/core/rendering.ts` | +| Debounce | `createDebouncedRenderer`, delay from `renderDebounce` setting | `src/app/services/debounced-renderer.ts` | +| Spec prep | `prepareSpecForRender` (pure, on a copy) | `src/core/rendering.ts` (see _Live Preview_) | +| Errors | One error field, cleared on success, empty = nothing | `PreviewStore.error` | +| Container fit | Inner host + frame (out-specify `.vega-embed`); resize via synthetic `window:resize` | §8 (`LivePreview` + `chart-renderer`) | diff --git a/docs/architecture/08-vega-editor-techniques.md b/docs/architecture/08-vega-editor-techniques.md index e07dd3c..6ef79e6 100644 --- a/docs/architecture/08-vega-editor-techniques.md +++ b/docs/architecture/08-vega-editor-techniques.md @@ -170,10 +170,14 @@ vega/editor's hand-rolled renderer (`src/components/renderer/renderer.tsx`) reve (`config-editor/config-editor-header.tsx:5-37`). For Astrolabe: pass the chosen `theme`/`config` to `vegaEmbed`, and when our theme changes, re-embed with the new config. - **`"width":"container"` / `"height":"container"` is how VL responsiveness works** — it - compiles to a `containerSize` signal (`renderer.tsx:78-90` detects this). Pair it with a - **`ResizeObserver`** on the preview pane → `view.resize().runAsync()`. This is cleaner than - vega/editor's `window.dispatchEvent(new Event('resize'))` hack (`renderer.tsx:101-122`) and is - the mechanism behind our M2 fit-mode contract. + compiles `width`/`height` to signals that re-read `containerSize()` **only on a + `window:resize` event** (`renderer.tsx:78-90` detects container sizing). Two things a + from-scratch impl _will_ get wrong (we did): (1) `view.resize().runAsync()` does **not** + re-measure — it re-runs layout with the stale size; (2) a pane drag fires no window resize, so + nothing re-fits on its own. The fix is exactly vega/editor's + `window.dispatchEvent(new Event('resize'))` (`renderer.tsx:101-122`) — **not a hack, the + actual mechanism** — driven by a `ResizeObserver` on the pane. It also leaves a non-container + dimension natural for free. Full write-up in doc [05](05-rendering-theming-preview.md) §8. - **Reuse the view for cheap changes.** They rebuild the `View` only on spec change; renderer (svg/canvas) and tooltip toggles re-`initialize()` the existing view (`renderer.tsx:367-371`). - **Capture warnings separately from errors** via a buffering logger (see §4's `LocalLogger`). @@ -273,20 +277,20 @@ defaults-spread" discipline is worth keeping. ## Borrow list (where each lands) -| Technique | Lands in | Milestone | -| ---------------------------------------------------------------------------------------- | -------------------------------------- | --------- | -| Bundle VL schema from package `build/`; `setDiagnosticsOptions` | `src/app/infrastructure/` Monaco setup | M2 | -| `markdownDescription` patch + compact formatter | Monaco setup | M2 | -| Explicit Vite worker wiring (`MonacoEnvironment.getWorker`) | Monaco setup | M2 | -| `fileMatch` schema binding (improvement over `$schema`-only) | Monaco setup | M2 | -| jsonc-parser tolerant parse + line/col syntax errors | `src/core/` | M1/M2 | -| ajv wrapper (`strict:false`, draft-06, color-hex, compile-once) → structured diagnostics | `src/core/` | M2 | -| `LocalLogger`-style buffered diagnostics from pure compile | `src/core/` | M2 | -| Fatal-vs-advisory two-tier error model | rendering/store contract | M1/M2 | -| `"container"` sizing + `ResizeObserver` → `view.resize()` | `rendering.ts` + LivePreview | M2 | -| `finalize()`-before-reembed + **render-generation guard** | LivePreview | M1 | -| theme = `vega-themes` config merged into `vegaEmbed` | preview + settings | M5 | -| `json-stringify-pretty-compact` format action | editor | M2 | +| Technique | Lands in | Milestone | +| ----------------------------------------------------------------------------------------- | -------------------------------------- | --------- | +| Bundle VL schema from package `build/`; `setDiagnosticsOptions` | `src/app/infrastructure/` Monaco setup | M2 | +| `markdownDescription` patch + compact formatter | Monaco setup | M2 | +| Explicit Vite worker wiring (`MonacoEnvironment.getWorker`) | Monaco setup | M2 | +| `fileMatch` schema binding (improvement over `$schema`-only) | Monaco setup | M2 | +| jsonc-parser tolerant parse + line/col syntax errors | `src/core/` | M1/M2 | +| ajv wrapper (`strict:false`, draft-06, color-hex, compile-once) → structured diagnostics | `src/core/` | M2 | +| `LocalLogger`-style buffered diagnostics from pure compile | `src/core/` | M2 | +| Fatal-vs-advisory two-tier error model | rendering/store contract | M1/M2 | +| `"container"` sizing + `ResizeObserver` → synthetic `window:resize` (not `view.resize()`) | `chart-renderer` + LivePreview | M2 | +| `finalize()`-before-reembed + **render-generation guard** | LivePreview | M1 | +| theme = `vega-themes` config merged into `vegaEmbed` | preview + settings | M5 | +| `json-stringify-pretty-compact` format action | editor | M2 | ## Where we deliberately do better than the reference diff --git a/src/app/components/LivePreview.module.css b/src/app/components/LivePreview.module.css index f606ac6..7754e60 100644 --- a/src/app/components/LivePreview.module.css +++ b/src/app/components/LivePreview.module.css @@ -1,16 +1,114 @@ .preview { + display: flex; + flex-direction: column; height: 100%; width: 100%; - overflow: auto; background: var(--bg); } -.chart { +.header { + flex: 0 0 auto; display: flex; - align-items: flex-start; - justify-content: center; - min-height: 100%; + align-items: center; + justify-content: flex-end; + gap: var(--space-3); + height: 40px; + padding: 0 var(--space-4); + border-bottom: var(--border-width) solid var(--border); +} + +.body { + flex: 1 1 auto; + min-height: 0; + overflow: auto; padding: var(--space-5); + box-sizing: border-box; +} + +/* Segmented control — the four fit modes (spec §04). */ +.fit { + display: inline-flex; + border: var(--border-width) solid var(--border-strong); +} + +.fitOption { + appearance: none; + border: none; + background: var(--bg); + color: var(--text-secondary); + font: inherit; + font-size: 12px; + line-height: 1; + padding: var(--space-2) var(--space-3); + cursor: pointer; + transition: + background var(--dur-fast) var(--ease), + color var(--dur-fast) var(--ease); +} + +.fitOption + .fitOption { + border-left: var(--border-width) solid var(--border-strong); +} + +.fitOption:hover { + background: var(--layer-01); + color: var(--text); +} + +.fitActive, +.fitActive:hover { + background: var(--accent); + color: var(--accent-contrast); +} + +/* + * Chart sizing. The host (passed to vega-embed) is branded `.vega-embed` + * (display:inline-block) at runtime; that shrink-wraps it, which is why + * width:"container" collapsed before. The frame carries the fit class and the + * two-class selectors below out-specify `.vega-embed` to give the host a + * definite box for the responsive modes. `box-sizing:border-box` keeps the + * chart inside the body padding rather than overflowing it. + */ +.frame { + box-sizing: border-box; +} + +.host { + box-sizing: border-box; +} + +/* Original — natural size; the body scrolls if the chart is larger than the pane. */ +.fitOriginal { + display: inline-block; +} + +/* Width — host spans the pane width; height stays natural. */ +.fitWidth { + display: block; + width: 100%; +} +.fitWidth .host { + width: 100%; +} + +/* Height — host spans the pane height; width stays natural. */ +.fitHeight { + display: block; + height: 100%; +} +.fitHeight .host { + height: 100%; +} + +/* Full — host fills the pane in both dimensions. */ +.fitFull { + display: block; + width: 100%; + height: 100%; +} +.fitFull .host { + width: 100%; + height: 100%; } .error { diff --git a/src/app/components/LivePreview.tsx b/src/app/components/LivePreview.tsx index eab9428..dac1220 100644 --- a/src/app/components/LivePreview.tsx +++ b/src/app/components/LivePreview.tsx @@ -1,40 +1,90 @@ /** * Live Preview — the right pane (spec §04). * - * Renders the active snippet's current buffer as a Vega-Lite chart, debounced so - * typing stays smooth. The pipeline is: buffer text → JSON.parse → - * prepareSpecForRender (copy, pure) → renderSpec (vega-embed). A render- - * generation token guards against a slow render resolving after a newer one. + * Renders the active snippet's currently-shown spec (draft or published, per the + * editor view) as a Vega-Lite chart, debounced so typing stays smooth. The + * pipeline is: shown text → JSON.parse → prepareSpecForRender (copy, pure, fit + * mode applied) → renderSpec (vega-embed). A render-generation token guards + * against a slow render resolving after a newer one. * - * M1 scope: inline-data specs, Original sizing, basic error text. Fit modes (M2) - * and dataset reference resolution (M3) plug into prepareSpecForRender without - * changing this component. + * The pane header carries the Fit control (4 sizing modes, §04). Render errors + * are published to the shared PreviewStore so the editor pane mirrors them + * (§03E); the preview shows the same message in place of the chart. + * + * M2 scope: inline-data specs, all four fit modes. Dataset reference resolution + * (M3) plugs into prepareSpecForRender without changing this component. */ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useRef } from 'react'; import type { VisualizationSpec } from 'vega-embed'; +import type { FitMode } from '@core/rendering'; import { prepareSpecForRender } from '@core/rendering'; import { chartConfigFor } from '@core/vega-themes'; import { renderSpec, type RenderHandle } from '../services/chart-renderer'; import { useAppStore } from '../stores/AppStore'; -import { useSnippetStore } from '../stores/SnippetStore'; +import { usePreviewStore } from '../stores/PreviewStore'; +import { selectShownText, useSnippetStore } from '../stores/SnippetStore'; import styles from './LivePreview.module.css'; /** Render debounce (ms). Becomes the configurable `renderDebounce` setting in M5. */ const RENDER_DEBOUNCE_MS = 300; +/** The four fit modes in display order (spec §04 → Fit / Sizing Modes). */ +const FIT_MODES: ReadonlyArray<{ mode: FitMode; label: string }> = [ + { mode: 'default', label: 'Original' }, + { mode: 'width', label: 'Width' }, + { mode: 'height', label: 'Height' }, + { mode: 'full', label: 'Full' }, +]; + +/** + * Sizing class for the chart frame per fit mode. The frame is React-owned, so + * these classes drive how the host element (which vega-embed brands with its own + * `display:inline-block`) is sized: the responsive modes give the host a + * definite width/height for Vega's `"container"` measurement (`containerSize()` + * reads `host.clientWidth/Height`), while Original lets it shrink to natural size. + */ +const FIT_CLASS: Record = { + default: styles.fitOriginal, + width: styles.fitWidth, + height: styles.fitHeight, + full: styles.fitFull, +}; + +function FitControl() { + const fitMode = useAppStore((s) => s.previewFitMode); + const setFitMode = useAppStore((s) => s.setPreviewFitMode); + return ( +
+ {FIT_MODES.map(({ mode, label }) => ( + + ))} +
+ ); +} + export function LivePreview() { const hostRef = useRef(null); const handleRef = useRef(null); const generationRef = useRef(0); - const draftText = useSnippetStore((s) => s.draftText); + const shownText = useSnippetStore(selectShownText); + const fitMode = useAppStore((s) => s.previewFitMode); const uiTheme = useAppStore((s) => s.uiTheme); - const [error, setError] = useState(null); + const error = usePreviewStore((s) => s.error); + const setError = usePreviewStore((s) => s.setError); useEffect(() => { const node = hostRef.current; if (!node) return; - const text = draftText.trim(); + const text = shownText.trim(); // The debounced body is async; wrap in a void IIFE so the timer callback // returns void (it handles its own errors internally — nothing awaits it). @@ -58,7 +108,7 @@ export function LivePreview() { const mine = ++generationRef.current; try { - const prepared = prepareSpecForRender(parsed, { fitMode: 'default' }); + const prepared = prepareSpecForRender(parsed, { fitMode }); const config = chartConfigFor(uiTheme); handleRef.current?.destroy(); handleRef.current = null; @@ -67,9 +117,9 @@ export function LivePreview() { // TODO: a superseded render's destroy() calls node.replaceChildren(), // which can blank the live chart if two embeds on the same node are // ever in flight at once (heavy spec whose embed outlasts the 300ms - // debounce). The debounce makes this rare in M1; when fit-mode/dataset - // work (M2/M3) lands, serialize renders or finalize the stale view - // without clearing the shared node. + // debounce). The debounce makes this rare; when dataset work (M3) + // lands, serialize renders or finalize the stale view without + // clearing the shared node. handle.destroy(); // a newer render superseded this one return; } @@ -87,21 +137,50 @@ export function LivePreview() { }, RENDER_DEBOUNCE_MS); return () => clearTimeout(timer); - }, [draftText, uiTheme]); + }, [shownText, fitMode, uiTheme, setError]); - // Finalize the live view on unmount. + // 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 + // component's life. Only responsive fit modes depend on container size; + // Original is fixed natural size and the pane just scrolls. ResizeObserver + // callbacks are frame-batched, so this tracks the drag without thrashing. + useEffect(() => { + const node = hostRef.current; + if (!node || typeof ResizeObserver === 'undefined') return; + const ro = new ResizeObserver(() => { + if (useAppStore.getState().previewFitMode === 'default') return; + handleRef.current?.resize(); + }); + ro.observe(node); + return () => ro.disconnect(); + }, []); + + // Finalize the live view on unmount, and clear the shared error so a stale + // message never outlives this pane. useEffect( () => () => { handleRef.current?.destroy(); handleRef.current = null; + usePreviewStore.getState().setError(null); }, [], ); return (
-