From c11afc273d4ee7857242ad7f881798a476e69627 Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Fri, 5 Jun 2026 23:46:21 +0300 Subject: [PATCH] Add Chart Builder: no-JSON Vega-Lite composer from a dataset (M4) --- .claude/skills/council/SKILL.md | 25 +- docs/IMPLEMENTATION-PLAN.md | 14 +- docs/chart-builder-research.md | 206 +++++++++++ docs/spec/06-chart-builder.md | 23 +- .../components/ChartBuilderModal.module.css | 254 +++++++++++++ src/app/components/ChartBuilderModal.test.tsx | 77 ++++ src/app/components/ChartBuilderModal.tsx | 333 ++++++++++++++++++ src/app/components/DatasetsModal.tsx | 15 +- src/app/components/ModalShell.tsx | 2 +- src/app/modals/modal-registry.ts | 13 + src/app/stores/ChartBuilderStore.test.ts | 104 ++++++ src/app/stores/ChartBuilderStore.ts | 180 ++++++++++ src/app/stores/SnippetStore.ts | 6 +- src/core/chart-builder.test.ts | 294 ++++++++++++++++ src/core/chart-builder.ts | 323 +++++++++++++++++ src/core/snippet.ts | 13 +- 16 files changed, 1856 insertions(+), 26 deletions(-) create mode 100644 docs/chart-builder-research.md create mode 100644 src/app/components/ChartBuilderModal.module.css create mode 100644 src/app/components/ChartBuilderModal.test.tsx create mode 100644 src/app/components/ChartBuilderModal.tsx create mode 100644 src/app/stores/ChartBuilderStore.test.ts create mode 100644 src/app/stores/ChartBuilderStore.ts create mode 100644 src/core/chart-builder.test.ts create mode 100644 src/core/chart-builder.ts diff --git a/.claude/skills/council/SKILL.md b/.claude/skills/council/SKILL.md index 7801dec..e024da3 100644 --- a/.claude/skills/council/SKILL.md +++ b/.claude/skills/council/SKILL.md @@ -43,6 +43,7 @@ four; that wastes tokens and dilutes the answer. Map the decision to its seat(s) | **Forms / validation / destructive-action** flow | **GOV.UK** → Carbon | | General **usability** gut-check on a flow | **NN/g** 10 heuristics | | **Visual** styling (type, spacing, colour, component look) | **Carbon** + our `docs/architecture/09` | +| **Which chart** for the data/intent (chart-type choice) | **FT Visual Vocabulary** + **Datawrapper** | Then: read the cited file(s), extract the **specific** principle, and report it back with a **citation (member + file path)** and a one-line "how it lands in Astrolabe." Don't @@ -53,12 +54,14 @@ paraphrase the whole source — quote the rule that decides the question. All paths are under `/Users/oleh/code/reference/`. Treat clones as **inspiration, not law** — they drift; the published guidance is the truth, the clone is the fast index. -| Member | Path | Authoritative for | How to query | -| ------------------------------ | ---------------------------------- | -------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **IBM Carbon** | `carbon-website/src/pages/` | Notification taxonomy, status levels, empty/loading states, content basics, data-viz styling | grep `.mdx` under `components/notification`, `patterns/{empty-states,loading,status-indicator}-pattern`, `guidelines/content` | -| **GOV.UK Design System** | `govuk-design-system/src/` | Error & validation messages, failure pages, forms, plain-language content, accessibility | `index.md` under `components/{error-message,error-summary,notification-banner}`, `patterns/{problem-with-the-service-pages,service-unavailable-pages,check-answers}`, `accessibility/` | -| **WAI-ARIA APG** | `aria-practices/content/patterns/` | Keyboard interaction, focus management, ARIA roles/states for widgets | `/-pattern.html` — e.g. `dialog-modal`, `alertdialog`, `alert`, `listbox`, `menu-button`, `disclosure`, `switch`, `tabs`, `tooltip`, `windowsplitter` | -| **Nielsen Norman (distilled)** | `principles/nielsen-norman.md` | 10 usability heuristics; response-time / feedback budgets (0.1s / 1s / 10s) | read directly — it is short and curated | +| Member | Path | Authoritative for | How to query | +| ------------------------------ | ---------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **IBM Carbon** | `carbon-website/src/pages/` | Notification taxonomy, status levels, empty/loading states, content basics, data-viz styling | grep `.mdx` under `components/notification`, `patterns/{empty-states,loading,status-indicator}-pattern`, `guidelines/content` | +| **GOV.UK Design System** | `govuk-design-system/src/` | Error & validation messages, failure pages, forms, plain-language content, accessibility | `index.md` under `components/{error-message,error-summary,notification-banner}`, `patterns/{problem-with-the-service-pages,service-unavailable-pages,check-answers}`, `accessibility/` | +| **WAI-ARIA APG** | `aria-practices/content/patterns/` | Keyboard interaction, focus management, ARIA roles/states for widgets | `/-pattern.html` — e.g. `dialog-modal`, `alertdialog`, `alert`, `listbox`, `menu-button`, `disclosure`, `switch`, `tabs`, `tooltip`, `windowsplitter` | +| **Nielsen Norman (distilled)** | `principles/nielsen-norman.md` | 10 usability heuristics; response-time / feedback budgets (0.1s / 1s / 10s) | read directly — it is short and curated | +| **FT Visual Vocabulary** | `chart-doctor/visual-vocabulary/` | Chart choice: data-relationship taxonomy (Magnitude, Correlation, Change-over-Time, Ranking, Distribution, Deviation, Part-to-whole, Spatial, Flow) → chart type | read `README.md` — the taxonomy is prose; each category gives a "use when…" definition + recommended chart types | +| **Datawrapper (distilled)** | `principles/datawrapper.md` | Chart choice in plain language; practical rules of thumb (bar-is-safe-default, line-vs-column, circles hard to compare, size = quantity) | read directly — short and curated; pairs with the FT clone | ## Close the loop @@ -82,6 +85,10 @@ contract, not the external source. have never been applied before. This is the one event that reopens the one-off sweep (see _Scope_); after it, pointwise maintains the new seat like the rest. -Candidate future seats (not yet seated): **FT Visual Vocabulary / Datawrapper** (chart -choice — our domain), **Shopify Polaris** (UX-writing depth), **web.dev** (perceived -performance / PWA / offline UX). +Candidate future seats (not yet seated): **Shopify Polaris** (UX-writing depth), +**web.dev** (perceived performance / PWA / offline UX — seat at M6 per the plan). + +Seated at M4 (chart choice — our domain): **FT Visual Vocabulary** (clone) + +**Datawrapper** (distilled). Backfill is scoped to the Chart Builder itself (new +surface — no pre-existing chart-choice code to reconcile), so the seating debt is +discharged by building the builder against this canon rather than a separate sweep. diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index 9671fb7..9c0c5de 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -48,7 +48,7 @@ doc before implementing. | **M1** | **MVP core loop** | Author a Vega-Lite snippet, see it render live, it persists | §02, §03A–C, §04, §09A | | **M1.5** | Visual design foundation ✅ | Apply the design language: tokens, IBM Plex, restyled M1 surfaces, chart theme | [arch 09](architecture/09-visual-design.md) | | **M2** | Editor robustness | Draft/Published, validation, schema autocomplete, fit modes | §03D–E, §04, §07(editor) | -| **M3** | Datasets | Named reusable data + reference resolution in preview | §05, §03F, §09B | +| **M3** | Datasets ✅ | Named reusable data + reference resolution in preview | §05, §03F, §09B | | **M4** | Chart Builder | No-JSON chart composition from a dataset | §06 | | **M5** | Settings + Import/Export | Preferences + workspace backup/transfer | §07, §08, §09C | | **M6** | Shell polish | Resize/toggle panes, routing, shortcuts, toasts, a11y, offline | §01, §10 | @@ -307,11 +307,15 @@ the reference. - Build a bar chart from a dataset in a few clicks; preview live-updates; Create → new snippet opens and renders. -**Council** — seat **FT Visual Vocabulary** + **Datawrapper** (chart-choice canon) here: -this is where Astrolabe stops being a pass-through JSON editor and starts making +**Council** — **FT Visual Vocabulary** + **Datawrapper** are now **seated** (chart-choice +canon): this is where Astrolabe stops being a pass-through JSON editor and starts making chart-shaped suggestions/defaults, so "_which chart, and why_" becomes a decision the app -owns — the one thing Carbon's data-viz styling doesn't cover. Seating triggers a one-off -backfill of the builder's defaults/affordances (see [`/council`](../.claude/skills/council/SKILL.md)). +owns — the one thing Carbon's data-viz styling doesn't cover. Rather than a styling-only +seating, we ran a full **research-first** pass (FT + Datawrapper + the formal engines +**Draco** and **Voyager**), recorded in [`docs/chart-builder-research.md`](chart-builder-research.md), +and chose the **Tier B "smart + guarded"** design: smart default mark for the data shape, +valid-type-locked field-type menus, Size-channel discipline, and non-blocking guidance. +The convergent rules and citations live in that doc; the spec (§06) was amended to match. --- diff --git a/docs/chart-builder-research.md b/docs/chart-builder-research.md new file mode 100644 index 0000000..58099b0 --- /dev/null +++ b/docs/chart-builder-research.md @@ -0,0 +1,206 @@ +# Chart Builder — Design Research (M4) + +> **Status:** research complete; informs the M4 build (spec §06). +> **Decision:** build **Tier B — "smart + guarded"** (mark-first, still §06-shaped). +> **Why this doc exists:** the Chart Builder is the point where Astrolabe stops being +> a pass-through JSON editor and starts making chart-shaped suggestions/defaults. +> "Which chart, and why" becomes a decision the app owns, so we researched it +> deliberately before building. This is the record of what we studied and what we +> took from each source — the citations behind every default and guardrail in +> `src/core/chart-builder.ts`. + +--- + +## 1. Scope of the builder (the constraint everything maps into) + +Spec §06: compose a Vega-Lite chart from a dataset with **one mark** ∈ +{Bar, Line, Point, Area, Circle}, mapping columns to **four channels** (X, Y, Color, +Size), each carrying a **field type** ∈ {Quantitative, Nominal, Ordinal, Temporal}, +plus optional pixel width/height → a complete spec saved as a snippet that +references the dataset by name. Column types are inferred upstream as +`number | string | date | boolean` (`src/core/type-inference.ts`). + +No transforms (no binning, aggregation, stacking, regression), no second axis, no +geo. That narrow surface is the lens through which every source below was read: +"what does this canon tell us to do **within Bar/Line/Point/Area/Circle and +X/Y/Color/Size?**" + +## 2. The sources + +Two kinds: **formal CS** (how recommendation engines actually rank charts) and +**chart-choice canon** (how practitioners pick). They were chosen for being +**cloneable/grep-able offline** (the council's working model) and authoritative for +"which chart," which our other seats (Carbon/GOV.UK/APG/NN/g) don't cover. + +| Source | What it is | Local path | +| ------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------- | +| **Draco** (uwdata) | Visualization design knowledge as ASP constraints — the formal "what makes a good chart," with hard (validity) + soft (preference) rules and weights, some learned from human perception experiments (Kim 2018, Saket 2018). | `reference/draco` | +| **Voyager** (vega) | UW IDL's recommendation/exploration tool on CompassQL — the _interaction_ model (field shelves, auto-add, type chips) and effectiveness-ranked encoding suggestions. | `reference/voyager` | +| **FT Visual Vocabulary** (Financial Times) | A poster/taxonomy mapping _what you want to show_ (9 data-relationship categories) → chart types. **Seated** in the council. | `reference/chart-doctor/visual-vocabulary/` | +| **Datawrapper** | Practitioner chart-choice in plain language; intent-first ("the chart's main statement becomes a compass"). **Seated** (distilled). | `reference/principles/datawrapper.md` | + +**Theoretical basis, not seated (deliberately):** **Munzner**, _Visualization Analysis +and Design_ (marks & channels; the channel-effectiveness rankings — magnitude: +position → length → angle → area …; identity: spatial region → hue → shape; +expressiveness & effectiveness principles) and **Wilke**, _Fundamentals of Data +Visualization_ (`clauswilke/dataviz`; example directory by intent + "ugly/bad/wrong" +pedagogy). They are the _why_ beneath Draco and Voyager — Draco's soft weights are an +operationalization of exactly these Mackinlay/APT/Munzner effectiveness rankings — but +they restate the same rules the seated sources already give us, so seating them would +add overlap, not coverage. Cited here as grounding; revisit if we ever build the +intent-first "Tier C" front door, where Munzner's typology and Wilke's directory +would earn their place. + +## 3. What we take from each source + +### From Draco — validity guardrails + a preference ranking (the rigorous core) + +Draco models a chart as ASP facts and rejects/ranks them with **hard** (∞ cost) and +**soft** (weighted) constraints (`asp/optimize.lp`). We can't ship an ASP solver in a +browser, but the rules are a lookup table. The portable subset: + +- **Hard validity (block in the UI):** `reference/draco/asp/hard.lp` + - Quantitative on a string/boolean column — illegal (`:6`). Temporal only on a + datetime column (`:7`). + - **Size encoding a Nominal field — illegal** ("size implies order; nominal is + misleading", `:53`). Size cannot encode **negative** values (`:56`). Size only on + point/text marks (`:110`). + - Bar/Area must include a **zero baseline** on the measure axis (`:103-104`). + - Bar needs a categorical axis — both x and y continuous on a bar is malformed + (`:97`); Line/Area need **both** x and y, and not both discrete (`:91,:94`). + - Same field on x and y — illegal (`:122`). >20 categorical colors — illegal (`:172`). +- **Soft preference (the weights, `asp/weights.lp` + `asp/soft.lp`):** + - Channel-by-type appropriateness (lower = better): continuous data is free on x/y, + costs to put on color (10) or size (1); nominal cheapest on y then x then color; + ordered data expensive on size. → **fill X/Y before Color/Size.** + - Mark by data shape: continuous×continuous → **point** (line/area heavily + penalized); continuous×discrete aggregated → **bar**; discrete×discrete → point/rect. + - Prefer time on x (`temporal_y`, `:147`); never type a number as nominal + (`number_nominal`, weight 10); the loudest nudge is an all-discrete chart with no + measure (`only_discrete`, weight 30). + +The hand-tuned `weights.lp` is the portable "common-sense" set; the learned +`weights_learned.lp` corroborates direction, not magnitude. + +### From Voyager — the interaction model + the valid-type table + +- **`getValidTypes` (`src/components/data-pane/field-list.tsx:140-155`) — adopted + almost verbatim:** number→{quantitative, nominal}, integer→{quantitative, nominal}, + datetime→{temporal}, string→{nominal}, boolean→{nominal}. The type toggle shows only + when ≥2 valid types exist. (We extend slightly — see §4 — to also offer Ordinal, + which Voyager deliberately omits, `encoding.ts:131-134`.) +- **Auto-add / "auto" mark (`models/shelf/index.ts:72-81`):** Voyager lets a field be + added with `channel:'?'` and asks CompassQL to place it by `effectiveness`. The small + builder analogue is a **non-empty smart default** (`defaultBuilderConfig`) so the + preview is never blank. +- **Type chips + swap:** per-field type indicator with a click-to-change popover, and a + cheap x↔y swap (Voyager's `SPEC_FIELD_MOVE` is remove-both + re-add). +- **Out of scope (Voyager scope creep we reject):** wildcard shelves, the full Related + Views gallery, faceting (row/column), and embedding CompassQL/`compassql@0.20.2` + itself. We hand-roll a small decision table in `src/core/` instead of pulling the + engine. + +### From FT Visual Vocabulary — the intent→chart taxonomy (and our coverage gaps) + +`reference/chart-doctor/visual-vocabulary/README.md` (taxonomy is prose). Nine +categories; mapped to **our five marks**: + +| FT category | What it shows | Our expression | +| -------------------- | --------------------------- | ----------------------------------------------------------------------------- | +| **Magnitude** | size comparisons | **Bar** (x=N, y=Q; horizontal x=Q, y=N for long labels) — primary | +| **Ranking** | position in an ordered list | **Bar, sorted** by value (the sort _is_ the feature) | +| **Change over Time** | trends | **Line** (x=T, y=Q; color=N for series); Bar/Area alternatives, single series | +| **Correlation** | relationship of 2+ measures | **Point** (x=Q, y=Q); **Circle/bubble** + size=Q for a third measure | +| **Deviation** | +/− from a reference | **Bar** with signed Q (diverging bar only) | +| **Distribution** | spread/frequency | weak: raw **Point** strip, or **Bar** of pre-binned counts (no bin transform) | +| **Part-to-whole** | component shares | **none well** — redirect to Magnitude/Bar; we can't show true proportions | +| **Spatial** | geography | **none** — exclude | +| **Flow** | movement between states | **none** — exclude | + +**Coverage:** strong on Magnitude, Ranking, Change-over-Time, Correlation; partial on +Deviation/Distribution; none on Part-to-whole/Spatial/Flow. Honest gaps, not silent +degradation. + +### From Datawrapper — plain-language rules + intent labels + +`reference/principles/datawrapper.md`. Corroborates the same default-mark-by-intent +table (comparison→Bar, time→Line, correlation→Point/bubble) and supplies friendlier +intent words (Developments over time / Shares / Comparison / Correlation). Bindable +rules: bar is the safe default; bar over column on small screens; line for continuous +time, columns for a few points; circles are hard to compare precisely; size encodes a +quantity; area = single total (warn on multi-series). + +## 4. The convergent rules — what all four agree on (high-confidence) + +These are not a judgment call; the formal engines and the practitioner canon land on +the same place. They are the spec for `src/core/chart-builder.ts`: + +1. **Column type → valid field types** (Voyager `getValidTypes`; Draco `hard.lp:6-7`): + `number`→{Quantitative (default), Ordinal, Nominal}; `date`→{Temporal only}; + `string`→{Nominal (default), Ordinal}; `boolean`→{Nominal}. Never offer Q for + string/boolean, never Temporal for a non-date. (We add Ordinal where it's a defensible + user assertion of order; Voyager omits it for UX simplicity — our deliberate superset.) +2. **Default mark from the (X, Y) shape** (Draco mark-by-shape; Voyager effectiveness; + FT; Datawrapper): temporal × quantitative → **Line**; quantitative × quantitative → + **Point**; (nominal/ordinal) × quantitative → **Bar**; both-discrete → **Point** + (Bar/Line/Area are invalid with no continuous axis); single axis or unknown → Bar. +3. **Channel priority + Size discipline** (Draco `hard.lp:53,56,110` + non-positional + pref): fill X/Y before Color/Size; Color before Size. **Size is only valid for + Quantitative/Ordinal positive measures on Point/Circle marks** — disabled for Nominal, + Temporal, and negative data (not merely discouraged). +4. **Bar/Area zero-baseline; Line exempt** (Draco `hard.lp:103-104`; FT; ONS/Vox sources + FT links). We expose no axis-truncation control, so Vega-Lite's own defaults already + give zero-baseline bars and free-baseline lines — the rule is satisfied by _not adding_ + an override, nothing to emit. +5. **Chart-choice polish** (FT; Datawrapper): sort bars when ranking; horizontal bar for + long category labels; Size encodes a quantity, Color a category; Area is for a single + series (warn against color-splitting into many). + +## 5. The decision: Tier B — "smart + guarded" + +Three tiers were on the table. **Tier B** was chosen (2026-06-05). + +- **Tier A — spec-literal:** Bar default, four channel dropdowns, type override, smart + pre-population. Matches §06 verbatim but uses almost none of the research; stays a + "dumb" composer. +- **Tier B — smart + guarded (chosen):** Tier A **+** default _mark_ from the (X, Y) type + shape (not always Bar) **+** valid-type-only menus **+** inline non-blocking warnings + from the Draco rules **+** swap-X/Y **+** Size disabled for Nominal/Temporal/negative. + Still mark-first and §06-shaped, but genuinely intelligent. Requires a small §06 + amendment (documented in the spec). +- **Tier C — intent-first aid:** Tier B **+** a "what do you want to show?" front door + (FT/Datawrapper intents → recommended mark + channel layout from intent × column + types). Highest "which chart & why" value; biggest UI; clearly extends §06. Deferred — + if revisited, this is where Munzner's typology and Wilke's directory would be seated. + +## 6. How it maps to implementation + +The convergent rules become pure functions in `src/core/chart-builder.ts` +(tested in `chart-builder.test.ts`), consumed by the builder store/modal: + +- `validFieldTypes(columnType)` → the type menu (rule 1); `defaultFieldType` = its head. +- `defaultMark(xType, yType)` → smart default mark (rule 2); used by + `defaultBuilderConfig`. +- `isChannelTypeAllowed(channel, type)` → Size discipline gate (rule 3). +- `builderWarnings(config)` → inline non-blocking hints (rules 3–5: line/area need both + axes, area + many series, two measures better as a scatter, etc.). +- `buildChartSpec` / `buildSnippetSpecText` → assemble the final spec; zero-baseline is + Vega-Lite-default (rule 4), so nothing is emitted for it. + +## 7. Anti-recommendations (what a naive builder would happily produce, and we don't) + +The highest-value guardrails — encodings a naive UI emits that the canon rejects: + +- A categorical column on **Size** (Draco hard `:53`) — blocked, not warned. +- A **truncated-axis bar** — prevented by never exposing an axis override (Draco `:103`). +- A high-cardinality category on **Color** → unreadable legend (soft w=10; >20 hard). +- A **Line between two raw measures** instead of a scatter (Draco soft w=20) — warned. +- An **all-categorical chart with no measure** (Draco soft w=30, the loudest) — warned. +- A **number typed Nominal** (Draco soft w=10) — discouraged via default = Quantitative. + +--- + +_Citations are to files under `/Users/oleh/code/reference/`. The seated chart-choice +canon (FT clone + Datawrapper distill) lives in the council roster +(`.claude/skills/council/SKILL.md`); Draco/Voyager are reference clones, not council +seats — they're engineering sources, not user-facing design authorities._ diff --git a/docs/spec/06-chart-builder.md b/docs/spec/06-chart-builder.md index 0da698f..5fc532d 100644 --- a/docs/spec/06-chart-builder.md +++ b/docs/spec/06-chart-builder.md @@ -2,6 +2,8 @@ The Chart Builder is a visual, no-JSON way to compose a Vega-Lite chart from a selected dataset. The user picks a mark type and maps the dataset's columns to encoding channels; the builder produces a complete Vega-Lite spec and saves it as a new snippet that references the dataset. It is intended for users who want to start a chart quickly without hand-writing JSON in the _Spec Editor & Draft/Published Workflow_. +> **Design level — "smart + guarded" (Tier B).** The builder is mark-first and stays within the inputs below, but it is not a dumb composer: it picks a sensible default mark for the data shape, offers only field types valid for each column, keeps unsuitable channel mappings out of reach, and surfaces non-blocking guidance for encodings that render poorly. These behaviors are derived from cross-source chart-choice research recorded in [`docs/chart-builder-research.md`](../chart-builder-research.md) (the convergence of Draco, Voyager, the FT Visual Vocabulary, and Datawrapper). The richer "intent-first" front door (ask _what do you want to show?_ and recommend a chart) is explicitly out of scope for now and noted there as a future tier. + ## Opening - Launched from a selected dataset in the _Datasets_ manager via that dataset's "build chart" action. @@ -20,7 +22,7 @@ A two-pane modal: ### Mark type - Single selection from an exact set of five mark types: **Bar, Line, Point, Area, Circle**. -- Defaults to **Bar**. +- On open, the mark **defaults to the type that best fits the pre-populated X/Y field-type shape** (Tier B smart default): a temporal axis against a measure → **Line**; two measures → **Point**; a category against a measure → **Bar**; two categories → **Point**; and **Bar** as the fallback when only one axis (or none) is mapped. The user can switch to any of the five afterward. - Exactly one mark type is active at any time; selecting one updates the preview. ### Encoding channels @@ -28,13 +30,26 @@ A two-pane modal: - Exactly four channels are offered, in this order: **X, Y, Color, Size**. - For each channel the user: - Picks a dataset column from a dropdown of the dataset's detected columns (see _Datasets_ for column detection). A "None" option leaves the channel unmapped. Each column option shows a small type indicator alongside the column name. - - Optionally overrides the channel's **field type**, chosen from an exact set: **Quantitative, Nominal, Ordinal, Temporal**. The type override only appears once a column is selected for that channel. -- When a column is chosen, its field type defaults from the dataset's inferred column type (numeric → Quantitative, date → Temporal, otherwise Nominal); the user may change it afterward. + - Optionally overrides the channel's **field type**. The override appears only once a column is selected, and offers only the **types valid for that column** (Tier B valid-type locking) — a string/boolean column never offers Quantitative, and only a date column offers Temporal. Concretely: number → {Quantitative (default), Ordinal, Nominal}; date → {Temporal}; text → {Nominal (default), Ordinal}; boolean → {Nominal}. When a column admits only one valid type, no override control is shown. +- When a column is chosen, its field type defaults from the dataset's inferred column type (numeric → Quantitative, date → Temporal, otherwise Nominal); the user may change it within the valid set above. +- **Size discipline:** the **Size** channel accepts only Quantitative or Ordinal columns — size implies an ordered magnitude, so categorical (Nominal) and Temporal columns are not offered for Size (they remain available on X/Y/Color). A column that can't go on Size is shown disabled there with a brief reason. - Clearing a channel back to "None" leaves it out of the produced spec. +- A **Swap X/Y** control exchanges the X and Y mappings (field and type) in one click, for quickly flipping the axes of the pre-populated default without re-selecting both columns. ### Default pre-population -- On open, the first detected column is assigned to **X** and the second (if any) to **Y**, each with its derived field type. Remaining channels start unmapped. Mark type starts at Bar. +- On open, the first detected column is assigned to **X** and the second (if any) to **Y**, each with its derived field type. Remaining channels start unmapped. The mark starts at the smart default for that X/Y shape (see _Mark type_), not unconditionally Bar. + +### Guidance (non-blocking) + +The builder surfaces short, plain-language hints for configurations that render but read poorly — advisory only, never blocking the **Create Snippet** action (validation below is the sole gate). These follow the chart-choice research ([`docs/chart-builder-research.md`](../chart-builder-research.md)) and include, for example: + +- A **Line** or **Area** mark with only one axis mapped (both axes are needed to draw it). +- A **Bar/Line/Area** whose X and Y are both categories (nothing to measure). +- **Two measures** on a non-scatter mark (a scatter — Point/Circle — usually reads better). +- An **Area** chart split into multiple colour series (per-series change is hard to see). + +A clean configuration shows no hints. ### Dimensions (optional) diff --git a/src/app/components/ChartBuilderModal.module.css b/src/app/components/ChartBuilderModal.module.css new file mode 100644 index 0000000..b752e8e --- /dev/null +++ b/src/app/components/ChartBuilderModal.module.css @@ -0,0 +1,254 @@ +/* Chart Builder — two-pane modal body (spec §06). */ + +.builder { + display: grid; + grid-template-columns: minmax(320px, 360px) 1fr; + min-height: 480px; + min-width: 0; +} + +.muted { + margin: 0; + padding: var(--space-5); + font-size: 14px; + color: var(--text-secondary); +} + +/* ── Left: configuration ─────────────────────────────────────────────── */ + +.configPane { + display: flex; + flex-direction: column; + gap: var(--space-5); + padding: var(--space-5); + border-right: var(--border-width) solid var(--border); + overflow-y: auto; + min-width: 0; +} + +.datasetName { + margin: 0; + font-size: 13px; + color: var(--text-secondary); +} + +.datasetName strong { + color: var(--text); +} + +.field { + display: flex; + flex-direction: column; + gap: var(--space-2); +} + +.fieldLabel { + font-size: 12px; + font-weight: 500; + color: var(--text-secondary); +} + +.channels { + display: flex; + flex-direction: column; + gap: var(--space-3); +} + +.channelsHeader { + display: flex; + align-items: baseline; + justify-content: space-between; +} + +.swap { + border: none; + background: transparent; + color: var(--accent); + font: inherit; + font-size: 12px; + cursor: pointer; + padding: var(--space-1) var(--space-2); + border-radius: var(--radius); +} + +.swap:hover { + background: var(--layer-01); +} + +.swap:focus-visible { + outline: 2px solid var(--focus); + outline-offset: 1px; +} + +.channelRow { + display: grid; + grid-template-columns: 48px 1fr auto; + align-items: center; + gap: var(--space-2); +} + +.channelLabel { + font-size: 12px; + font-weight: 600; + color: var(--text); +} + +.select, +.typeSelect { + padding: var(--space-2) var(--space-3); + border: var(--border-width) solid var(--border-strong); + border-radius: var(--radius); + background: var(--bg); + color: var(--text); + font: inherit; + font-size: 13px; +} + +.typeSelect { + font-size: 12px; +} + +.select:focus-visible, +.typeSelect:focus-visible, +.dimInput:focus-visible { + outline: 2px solid var(--focus); + outline-offset: -1px; +} + +.dimensions { + display: flex; + flex-direction: column; + gap: var(--space-2); +} + +.dimInputs { + display: flex; + gap: var(--space-3); +} + +.dimField { + display: flex; + flex-direction: column; + gap: var(--space-1); + font-size: 12px; + color: var(--text-secondary); +} + +.dimInput { + width: 100px; + padding: var(--space-2) var(--space-3); + border: var(--border-width) solid var(--border-strong); + border-radius: var(--radius); + background: var(--bg); + color: var(--text); + font: inherit; + font-size: 13px; +} + +.warnings { + display: flex; + flex-direction: column; + gap: var(--space-2); + margin: 0; + padding: var(--space-3); + list-style: none; + background: var(--layer-01); + border: var(--border-width) solid var(--border); + border-radius: var(--radius); +} + +.warning { + font-size: 12px; + line-height: 1.4; + color: var(--text-secondary); +} + +.warning::before { + content: '⚠ '; + color: var(--support-warning, var(--text-secondary)); +} + +.actions { + display: flex; + justify-content: flex-end; + gap: var(--space-3); + margin-top: auto; +} + +.action { + height: 36px; + padding: 0 var(--space-5); + border: var(--border-width) solid var(--border-strong); + border-radius: var(--radius); + background: transparent; + color: var(--text); + font: inherit; + font-weight: 500; + cursor: pointer; + transition: background var(--dur-fast) var(--ease); +} + +.action:hover { + background: var(--layer-01); +} + +.action:focus-visible { + outline: 2px solid var(--focus); + outline-offset: 2px; +} + +.primary { + background: var(--accent); + border-color: transparent; + color: var(--accent-contrast); + font-weight: 600; +} + +.primary:hover { + background: var(--accent-hover); +} + +.primary:disabled { + background: var(--layer-02, var(--layer-01)); + color: var(--text-placeholder); + cursor: not-allowed; +} + +/* ── Right: live preview ─────────────────────────────────────────────── */ + +.previewPane { + display: flex; + flex-direction: column; + padding: var(--space-5); + min-width: 0; + background: var(--bg); +} + +.previewHint { + margin: auto; + font-size: 13px; + color: var(--text-secondary); + text-align: center; +} + +.previewFrame { + flex: 1; + min-width: 0; + display: flex; + align-items: center; + justify-content: center; +} + +.previewHost { + width: 100%; +} + +.previewError { + margin: auto 0; + padding: var(--space-3); + font-family: var(--font-mono); + font-size: 12px; + line-height: 1.5; + white-space: pre-wrap; + color: var(--support-error); +} diff --git a/src/app/components/ChartBuilderModal.test.tsx b/src/app/components/ChartBuilderModal.test.tsx new file mode 100644 index 0000000..b8b4b95 --- /dev/null +++ b/src/app/components/ChartBuilderModal.test.tsx @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { createDataset } from '@core/dataset'; +import { useChartBuilderStore } from '../stores/ChartBuilderStore'; +import { useDatasetStore } from '../stores/DatasetStore'; +import { useSnippetStore } from '../stores/SnippetStore'; +import { ChartBuilderModal } from './ChartBuilderModal'; + +// The builder preview embeds a real Vega chart in an effect; stub the renderer so +// this render test stays a pure React/DOM check (the loop we guard against happens +// during commit, long before any chart is drawn). +vi.mock('../services/chart-renderer', () => ({ + renderSpec: () => Promise.resolve({ destroy() {}, resize() {} }), +})); + +// React 19 wants this flag set for act() to drive effects without warnings. +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const T = new Date('2026-06-01T00:00:00Z'); +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + useChartBuilderStore.getState().reset(); + useDatasetStore.getState().reset(); + useSnippetStore.getState().reset(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); +}); + +describe('ChartBuilderModal', () => { + test('renders without an infinite update loop when the config has warnings (regression)', async () => { + // Two numeric columns → default mark Point (clean). Switching to Bar makes it + // "two measures on a non-scatter" → a NON-EMPTY warnings array — the exact + // condition that previously looped because the warnings selector returned a + // fresh array of objects on every render. The fix derives warnings via useMemo + // over the stable `config` reference instead. + const ds = createDataset({ + name: 'Nums', + data: [ + { a: 1, b: 2 }, + { a: 3, b: 4 }, + ], + format: 'json', + source: 'inline', + now: T, + }); + useDatasetStore.getState().add(ds); + useChartBuilderStore.getState().init(ds.id); + useChartBuilderStore.getState().setMark('bar'); + expect(useChartBuilderStore.getState().config.mark).toBe('bar'); + + // If the component looped, this act() would throw "Maximum update depth exceeded". + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + expect(container.textContent).toContain('Building from'); + expect(container.textContent).toContain('scatter'); // the guidance hint rendered + }); + + test('shows the empty state when no dataset is loaded', async () => { + await act(async () => { + root.render(); + await Promise.resolve(); + }); + expect(container.textContent).toContain('No dataset loaded'); + }); +}); diff --git a/src/app/components/ChartBuilderModal.tsx b/src/app/components/ChartBuilderModal.tsx new file mode 100644 index 0000000..f5eda51 --- /dev/null +++ b/src/app/components/ChartBuilderModal.tsx @@ -0,0 +1,333 @@ +/** + * Chart Builder — the modal body (spec §06). + * + * A two-pane composer: left is the configuration (dataset name, mark selector, one + * row per channel, optional dimensions, guidance, Create), right is a live preview + * of the spec the configuration produces. All spec logic and Tier-B defaults/guards + * come from `@core/chart-builder` via `ChartBuilderStore`; this component is the + * view. The preview is builder-local (its own debounced render over the shared + * `chart-renderer` service) rather than a reuse of `LivePreview`, which is bound to + * the snippet editor's stores. + */ + +import { useEffect, useMemo, useRef, useState } from 'react'; +import { useShallow } from 'zustand/react/shallow'; +import type { VisualizationSpec } from 'vega-embed'; +import { + CHANNELS, + MARK_TYPES, + builderWarnings, + defaultFieldType, + isBuilderConfigValid, + isChannelTypeAllowed, + validFieldTypes, + type ChannelName, + type FieldType, + type MarkType, +} from '@core/chart-builder'; +import type { ColumnType } from '@core/type-inference'; +import { DatasetNotFoundError, prepareSpecForRender } from '@core/rendering'; +import { chartConfigFor } from '@core/vega-themes'; +import { renderSpec, type RenderHandle } from '../services/chart-renderer'; +import { closeModal } from '../modals/ModalCoordinator'; +import { useAppStore } from '../stores/AppStore'; +import { useDatasetStore } from '../stores/DatasetStore'; +import { + selectBuilderSpecText, + selectBuilderValid, + useChartBuilderStore, +} from '../stores/ChartBuilderStore'; +import { SegmentedControl, type SegmentedOption } from './SegmentedControl'; +import styles from './ChartBuilderModal.module.css'; + +const RENDER_DEBOUNCE_MS = 300; + +/** Title-case a token for display (e.g. `bar` → `Bar`, `quantitative` → `Quantitative`). */ +function titleCase(s: string): string { + return s.charAt(0).toUpperCase() + s.slice(1); +} + +const MARK_OPTIONS: ReadonlyArray> = MARK_TYPES.map((m) => ({ + value: m, + label: titleCase(m), +})); + +const CHANNEL_LABELS: Record = { + x: 'X', + y: 'Y', + color: 'Color', + size: 'Size', +}; + +/** A compact type indicator for a column option (text · # · date · ✓). */ +function typeBadge(type: ColumnType): string { + switch (type) { + case 'number': + return '#'; + case 'date': + return 'date'; + case 'boolean': + return 'bool'; + default: + return 'text'; + } +} + +/** Whether a column may be placed on a channel at all (Size discipline, §06). */ +function columnAllowedOnChannel(channel: ChannelName, colType: ColumnType): boolean { + return isChannelTypeAllowed(channel, defaultFieldType(colType)); +} + +function ChannelRow({ channel }: { channel: ChannelName }) { + const columns = useChartBuilderStore((s) => s.columns); + const mapping = useChartBuilderStore((s) => s.config.encodings[channel] ?? null); + const setChannelColumn = useChartBuilderStore((s) => s.setChannelColumn); + const setChannelType = useChartBuilderStore((s) => s.setChannelType); + + const colTypeOf = (name: string): ColumnType => + columns.columnTypes.find((c) => c.name === name)?.type ?? 'string'; + + // Type options valid for this column AND allowed on this channel (e.g. Size hides + // Nominal). Shown only when >1 option and a column is selected (spec §06). + const typeOptions: FieldType[] = mapping + ? validFieldTypes(colTypeOf(mapping.field)).filter((t) => isChannelTypeAllowed(channel, t)) + : []; + + return ( +
+ + + + {mapping && typeOptions.length > 1 && ( + + )} +
+ ); +} + +function BuilderPreview() { + const hostRef = useRef(null); + const handleRef = useRef(null); + const generationRef = useRef(0); + const [error, setError] = useState(null); + + const specText = useChartBuilderStore(selectBuilderSpecText); + const valid = useChartBuilderStore(selectBuilderValid); + const uiTheme = useAppStore((s) => s.uiTheme); + const datasets = useDatasetStore(useShallow((s) => s.datasets)); + + useEffect(() => { + const node = hostRef.current; + const timer = setTimeout(() => { + void (async () => { + const mine = ++generationRef.current; + // Below validation there is nothing to draw — clear the chart and show the + // configuration prompt, not an error (spec §06 → Live Preview placeholder). + if (!valid) { + handleRef.current?.destroy(); + handleRef.current = null; + setError(null); + return; + } + if (!node) return; + try { + const parsed: unknown = JSON.parse(specText); + const prepared = prepareSpecForRender(parsed, { fitMode: 'width', datasets }); + handleRef.current?.destroy(); + handleRef.current = null; + const handle = await renderSpec( + node, + prepared as VisualizationSpec, + chartConfigFor(uiTheme), + ); + if (mine !== generationRef.current) { + handle.destroy(); + return; + } + handleRef.current = handle; + setError(null); + } catch (e) { + if (mine !== generationRef.current) return; + if (e instanceof DatasetNotFoundError) { + setError(`Dataset "${e.datasetName}" not found.`); + } else { + setError(`Couldn't render this chart: ${(e as Error).message}`); + } + } + })(); + }, RENDER_DEBOUNCE_MS); + + return () => clearTimeout(timer); + }, [specText, valid, uiTheme, datasets]); + + // Finalize the view on unmount so the Vega view and its listeners don't leak. + useEffect( + () => () => { + handleRef.current?.destroy(); + handleRef.current = null; + }, + [], + ); + + return ( +
+ {!valid && ( +

Map at least one channel to a column to see a chart.

+ )} + + ); +} + +export function ChartBuilderModal() { + const datasetId = useChartBuilderStore((s) => s.datasetId); + const datasetName = useChartBuilderStore((s) => s.config.datasetName); + const mark = useChartBuilderStore((s) => s.config.mark); + const width = useChartBuilderStore((s) => s.config.width); + const height = useChartBuilderStore((s) => s.config.height); + const setMark = useChartBuilderStore((s) => s.setMark); + const swapXY = useChartBuilderStore((s) => s.swapXY); + const setWidth = useChartBuilderStore((s) => s.setWidth); + const setHeight = useChartBuilderStore((s) => s.setHeight); + const runCreate = useChartBuilderStore((s) => s.createSnippet); + // Derive validity + guidance from the stable `config` reference via useMemo, NOT + // from a store selector: `builderWarnings` builds a fresh array of objects each + // call, which no selector-equality (even useShallow, since the element objects + // differ every time) can stabilize — subscribing to it would re-render forever. + const config = useChartBuilderStore((s) => s.config); + const valid = useMemo(() => isBuilderConfigValid(config), [config]); + const warnings = useMemo(() => builderWarnings(config), [config]); + + if (datasetId === null) { + return

No dataset loaded. Open this from a dataset in Datasets.

; + } + + /** Parse a dimension input: blank → undefined, otherwise a non-negative integer. */ + const parseDim = (raw: string): number | undefined => { + if (raw.trim() === '') return undefined; + const n = Number(raw); + return Number.isFinite(n) && n > 0 ? Math.round(n) : undefined; + }; + + return ( +
+
+

+ Building from {datasetName} +

+ +
+ Mark + +
+ +
+
+ Encoding + +
+ {CHANNELS.map((channel) => ( + + ))} +
+ +
+ Dimensions (optional) +
+ + +
+
+ + {warnings.length > 0 && ( +
    + {warnings.map((w) => ( +
  • + {w.message} +
    • + ))} +
+ )} + +
+ + +
+
+ + +
+ ); +} diff --git a/src/app/components/DatasetsModal.tsx b/src/app/components/DatasetsModal.tsx index b4ded64..3aa2672 100644 --- a/src/app/components/DatasetsModal.tsx +++ b/src/app/components/DatasetsModal.tsx @@ -16,7 +16,7 @@ import { useState } from 'react'; import { useShallow } from 'zustand/react/shallow'; import { datasetReference, type DataSource, type Dataset } from '@core/dataset'; import { detectFormat, detectFormatFromUrl, type DataFormat } from '@core/format-detection'; -import { closeModal, resnapshot } from '../modals/ModalCoordinator'; +import { closeModal, openModal, resnapshot } from '../modals/ModalCoordinator'; import { confirm } from '../stores/ConfirmStore'; import { notify } from '../stores/NotificationStore'; import { selectSelectedDataset, useDatasetStore, byModifiedDesc } from '../stores/DatasetStore'; @@ -221,9 +221,16 @@ function DatasetDetail({ - {/* "Build Chart from dataset" (spec §05) lands enabled with the Chart - Builder in M4. Per council (GOV.UK / NN/g), we don't ship a dead - disabled control in the meantime — the action appears when it works. */} + {/* Build Chart (spec §05 → §06) — opens the Chart Builder on this dataset. + Replaces the Datasets modal (one modal at a time, §01C); detail view has + no transient form state, so no discard prompt. */} +