From 62d0697f0e879d894d23b1bb6e76c32be4924c0b Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Wed, 10 Jun 2026 17:10:18 +0300 Subject: [PATCH] Chart builder: data-aware defaults, one-click hint fixes, canvas preview, fullscreen modal --- docs/IMPLEMENTATION-PLAN.md | 12 +- docs/architecture/03-modal-system.md | 11 +- .../05-rendering-theming-preview.md | 17 + docs/architecture/06-type-inference.md | 6 +- .../10-interaction-and-feedback.md | 12 + docs/chart-builder-enhancement-scope.md | 353 ++++++++++++++++++ docs/chart-builder-research.md | 9 +- docs/lyra-review.md | 6 + docs/spec/06-chart-builder.md | 6 +- .../components/ChartBuilderModal.module.css | 63 +++- src/app/components/ChartBuilderModal.test.tsx | 96 ++++- src/app/components/ChartBuilderModal.tsx | 139 ++++++- src/app/components/ModalShell.module.css | 8 + src/app/components/ModalShell.tsx | 16 +- src/app/modals/modal-registry.ts | 9 + src/app/services/chart-renderer.ts | 74 +++- src/app/stores/ChartBuilderStore.test.ts | 13 + src/app/stores/ChartBuilderStore.ts | Bin 10536 -> 10859 bytes src/core/chart-builder.test.ts | 151 +++++++- src/core/chart-builder.ts | 205 ++++++++-- 20 files changed, 1133 insertions(+), 73 deletions(-) create mode 100644 docs/chart-builder-enhancement-scope.md diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index 25a4efc..5e4088a 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -289,10 +289,14 @@ the reference. **Goal:** no-JSON chart composition from a dataset → a new snippet. -> **Enhancement backlog** beyond the Tier-B floor (aggregation, binning, stacking, -> temporal granularity, sort/orientation, cardinality-based warnings, Tier C -> intent-first) lives in [`docs/chart-builder-research.md`](chart-builder-research.md) §8 -> — its single home, so these stop living in chat. +> **Enhancement push (post-M4).** The forward plan now lives in +> [`docs/chart-builder-enhancement-scope.md`](chart-builder-enhancement-scope.md) — it merges +> the Tier-B backlog ([`chart-builder-research.md`](chart-builder-research.md) §8) with the +> Lyra interaction review ([`lyra-review.md`](lyra-review.md)) and sets a **Tier-C** target. +> Shipped beyond the Tier-B floor so far: per-channel aggregate/bin/`timeUnit`, sort/stack; +> **actionable hints** (one-click warning fixes); and a builder UX/perf batch (near-fullscreen +> modal, canvas preview + canvas max-dimension guard, data-aware default pre-population). See +> the scope doc §4 for the sequenced plan and current status. **Core** diff --git a/docs/architecture/03-modal-system.md b/docs/architecture/03-modal-system.md index 77446d0..8f3d4a2 100644 --- a/docs/architecture/03-modal-system.md +++ b/docs/architecture/03-modal-system.md @@ -11,7 +11,9 @@ authoritative architecture for adding, opening, closing, and rendering modals. - **At most one modal open at a time** (mandated by the product spec). Opening a modal closes any other; the two never overlap. - **Uniform dismissal**: close button, `Escape`, or backdrop click — never a - click inside the body. + click inside the body. A modal holding in-progress work can opt out of the + **backdrop** click (`dismissOnBackdrop: false`) so a stray click can't discard it + (the Chart Builder does); close button and `Escape` still dismiss. - **Accessible by default**: focus moves into the modal on open and returns to the trigger on close. - **Unsaved-change safety** for editing modals, with an explicit opt-out for @@ -492,6 +494,13 @@ export function useFocusTrap( - Don't dismiss on clicks inside the body, and don't let Escape fire when no modal is open (the handler only exists while a modal renders). +**Sizing & backdrop opt-out.** The shell picks a **size tier** by modal: a small form +(Extract), a large two-pane manager (Datasets), or a near-fullscreen **work surface** +(Chart Builder — a config pane plus a chart that wants room). The two larger tiers have a +definite height so their inner panes scroll **internally** rather than the modal growing +past the viewport. A modal opts a backdrop click out of dismissal with the registry's +`dismissOnBackdrop: false` (above). + --- ## Confirmation & alert dialogs diff --git a/docs/architecture/05-rendering-theming-preview.md b/docs/architecture/05-rendering-theming-preview.md index 80176ef..240e29f 100644 --- a/docs/architecture/05-rendering-theming-preview.md +++ b/docs/architecture/05-rendering-theming-preview.md @@ -101,6 +101,23 @@ async function rerender(node: HTMLElement, spec: TopLevelSpec, config: Config) { - **Do** pass `actions: false`. Astrolabe owns its own export/copy affordances; the library's overlay menu does not belong on the preview. +- **SVG is the default renderer, canvas is an opt-in for many-mark previews.** SVG is + crisp/inspectable/copyable and stays the default for the editor's LivePreview. But an + SVG chart renders one DOM node per mark, so a many-mark chart (e.g. the Chart Builder's + default one-bar-per-row on a 10k-row dataset) costs **seconds** of main-thread + layout/paint per render (measured ~6.5s on 9994 rows; the chart paints _after_ it first + appears, freezing the tab). The **Chart Builder preview** therefore passes + `renderSpec(…, { renderer: 'canvas' })` — canvas is a single node and paints in + milliseconds. The raster trade-off is invisible for an ephemeral preview, and image + export (`view.toImageURL`) is renderer-agnostic. +- **Canvas has a hard max dimension; SVG doesn't.** A canvas larger than the browser's + limit (~32k px/side, less on Safari) fails to allocate and draws _nothing_ — silently. + So for canvas, `renderSpec` first runs a headless (`'none'`) layout probe, reads the + resolved height, and throws `ChartTooLargeError(heightPx, limitPx)` when it exceeds + `MAX_CANVAS_PX ÷ devicePixelRatio`, so the caller can show the real cause. This is a + **render-size** limit (the chart is physically too big), distinct from the readability + cardinality warnings — don't conflate them. Only an _unbounded_ axis overflows: a + `width: 'container'` axis is bounded, so it's the deleted (natural-height) axis to watch. - **Do** call `view.finalize()` on every previous view before rendering a new one, and on component unmount. - **Do** keep exactly one live view per preview node. diff --git a/docs/architecture/06-type-inference.md b/docs/architecture/06-type-inference.md index 43b2495..98f8de1 100644 --- a/docs/architecture/06-type-inference.md +++ b/docs/architecture/06-type-inference.md @@ -301,8 +301,10 @@ trivially testable. The caller passes `null` for URL and non-tabular datasets. ### 3.3 Column stats: cardinality + numeric extent Alongside the display type, each column carries the two data-shape signals the -**Chart Builder** needs for its data-aware Tier-B hints (spec §06; see -`chart-builder.ts` `builderWarnings`): +**Chart Builder** needs for its data-aware Tier-B hints (`chart-builder.ts` +`builderWarnings`) **and** for its default pre-population (`smartDefaultEncodings` +prefers a low-cardinality category over a high-cardinality key, so the builder never +opens on a degenerate chart; spec §06): - **`distinct`** — the count of distinct non-empty values **in the sample**, counted only up to `DISTINCT_CAP` (50). Past the cap the exact number stops diff --git a/docs/architecture/10-interaction-and-feedback.md b/docs/architecture/10-interaction-and-feedback.md index 0c3da18..b1690cf 100644 --- a/docs/architecture/10-interaction-and-feedback.md +++ b/docs/architecture/10-interaction-and-feedback.md @@ -167,6 +167,18 @@ lives in [04 · Routing & Global Events](04-routing-and-events.md). uses `alert`/`status` roles by severity. Don't invent keyboard models; adopt the documented one. +**Resolved — a control that removes its own container.** When activating a control deletes +the element it lives in (e.g. a Chart Builder guidance hint's one-click **fix** button — +the hint re-derives away once applied), focus must not fall to ``. The rule (council: +Carbon _Actionable notification_ + APG _Alert_): **announce the change politely and move +focus to a stable neighbour.** Concretely, the builder writes "Applied: `