From ed66fe9c0590c919fc62566f32359e8a1c2cda48 Mon Sep 17 00:00:00 2001 From: Oleh Omelchenko Date: Fri, 12 Jun 2026 14:45:31 +0300 Subject: [PATCH] Chart builder: SelectControl pickers, channel chooser, per-type aggregates --- docs/architecture/03-modal-system.md | 7 + .../05-rendering-theming-preview.md | 4 + .../10-interaction-and-feedback.md | 28 ++ docs/chart-builder-enhancement-scope.md | 119 ++++- docs/spec/06-chart-builder.md | 19 +- docs/ux-second-pass.md | 28 +- .../components/ChartBuilderModal.module.css | 153 ++++-- src/app/components/ChartBuilderModal.test.tsx | 98 +++- src/app/components/ChartBuilderModal.tsx | 470 +++++++++++------- src/app/components/SelectControl.module.css | 123 +++++ src/app/components/SelectControl.tsx | 243 +++++++++ src/app/stores/ChartBuilderStore.test.ts | 32 ++ src/app/stores/ChartBuilderStore.ts | 35 +- src/core/chart-builder.test.ts | 87 +++- src/core/chart-builder.ts | 62 ++- 15 files changed, 1232 insertions(+), 276 deletions(-) create mode 100644 src/app/components/SelectControl.module.css create mode 100644 src/app/components/SelectControl.tsx diff --git a/docs/architecture/03-modal-system.md b/docs/architecture/03-modal-system.md index 8f3d4a2..cfdbf0f 100644 --- a/docs/architecture/03-modal-system.md +++ b/docs/architecture/03-modal-system.md @@ -421,6 +421,13 @@ open, wraps `Tab`/`Shift+Tab` within the modal, and restores focus on close. The optional `initialSelector` picks _which_ child takes focus (e.g. Cancel for a destructive confirm); it falls back to the first focusable child. +The trap wraps Tab **only within the shell element**: content portaled to +`` (a `SelectControl` panel or other disclosure popover opened from inside +a modal) is outside both the trap's DOM subtree and its keydown listener. Such +popovers must therefore handle Tab themselves — close and refocus their trigger +(the native-select convention) — so focus can't strand outside the dialog while +it is open. + ```ts // src/app/hooks/useFocusTrap.ts import { useRef, useEffect } from 'react'; diff --git a/docs/architecture/05-rendering-theming-preview.md b/docs/architecture/05-rendering-theming-preview.md index 2979ca5..757e41b 100644 --- a/docs/architecture/05-rendering-theming-preview.md +++ b/docs/architecture/05-rendering-theming-preview.md @@ -471,6 +471,10 @@ the existing view is re-measured via a `ResizeObserver`-driven event — see §8 - **Don't** put reference resolution or fit-mode logic in the renderer — it is pure core logic and must be unit-testable without a DOM. - **Don't** mutate the input spec anywhere in the pipeline. +- **Fit modes overwrite the spec's own sizing** (Width replaces `width` _and + deletes_ `height`, etc.), so a surface that lets the user set an explicit + width/height must pass `fitMode: 'default'` while either is set and reserve the + container fit for auto sizing — the Chart Builder preview does exactly this. --- diff --git a/docs/architecture/10-interaction-and-feedback.md b/docs/architecture/10-interaction-and-feedback.md index e77e49d..f390ca3 100644 --- a/docs/architecture/10-interaction-and-feedback.md +++ b/docs/architecture/10-interaction-and-feedback.md @@ -418,6 +418,34 @@ _(Consulted via /council → NN/g #4 consistency, #6 recognition-over-recall, #8 WAI-ARIA APG disclosure + menu-and-menubar; Carbon popover/overflow-menu/text-toolbar. This bullet is the contract.)_ +**Resolved — value pickers are the SelectControl disclosure, not native `` in the builder are + now **`SelectControl`** — a reusable value-picker disclosure (the SortControl primitive + generalized; replaces native selects app-wide where the control is part of a designed + surface). Field chips open an explicit **channel chooser** (occupied channels say what + they'd replace); an **armed** channel short-circuits it and is now visible (accent ring + + status line + Esc disarms). "or constant" → **"Use a constant"** ghost button. Chart + properties (Title · Subtitle · W · H) moved to a **strip under the preview**; Sort/Stack + stay by the encodings. Resolutions recorded in `architecture/10` §5; `ux-second-pass.md` + cleared (drag stays deferred). + - **Visual-pass fixes:** pill ✕ flush right (pills hug content), Swap X/Y beside the Axes + heading, field shelf scrolls (280px viewport, sticky group heads), per-channel transforms + inline beside the pill, preview honors explicit width/height (fit-mode only while auto). + - **Verified:** `typecheck` + `test` (774, +10) + `eslint` + `build`. **Owed:** a fresh + visual look at the reworked surfaces (user-driven). + +- **2026-06-12 (scope)** — **3D added to Phase 3: builder entry points & discoverability** + (council-reviewed: NN/g #6 recognition + #7 expert accelerators, Carbon empty-states). + Today the builder's only door is Datasets → row "Build Chart" while the primary "Create + New Snippet" lands in blank Monaco — the data model's shape, not the user's intent. + Decided: fork the library's creation surface (primary **Build Chart**, ghost **New JSON + snippet**), give the builder an internal dataset picker + a no-datasets empty state, add + an onboarding tile. 3D lands **with or just before 3A** — the Build-Chart button opens + onto the intent front door, making 3A the app's guided creation flow. Detail in §3 · 3D. + +- **2026-06-11 (scope)** — **3C added to Phase 3: open-in-builder (strict spec hydration).** + A snippet-list button, enabled exactly when the spec round-trips losslessly through the + builder dialect (checked by re-assemble + deep-compare, not feature enumeration). Strict + only — no lossy/residue modes (§5's round-trip trap stays closed). Detail in §3 · 3C. + - **2026-06-11 (Phase 2)** — **2A + 2B shipped: the field-first interaction substrate.** Up next: **3A intent-first front door**. - **2A · Value-or-field channels (the Property model).** `ChannelMapping` gained a constant `value` arm; the assembler emits `{ value }` and the measure/stack/area-split/prune logic @@ -304,6 +341,12 @@ revision (field shelf alongside the channel rows)._ ### Phase 3 — Tier C (the intent-first front door) — _the defining feature of this push_ +The grand idea, stated once: **the builder is the app's rapid, intuitive on-ramp; Monaco +is the expert surface; the JSON spec stays the document.** Phase 3 is where that becomes +true — 3A gives the builder an intent-first opening screen, 3D makes that screen the +app's guided creation flow (not a feature hidden in a modal), and 3B/3C seed and re-enter +it. + **3A · Intent-first front door** — _the B→C step_ Source: research §5/§8 Tier C (FT Visual Vocabulary + Datawrapper intent taxonomy). A _"what do you want to show?"_ entry mapping **intent × column types → recommended mark + @@ -331,7 +374,72 @@ this as the deferred tier, so this is the planned amendment, not drift._ Source: Lyra §3.7. A small set of **curated starter snippets**, one per covered FT intent (Magnitude/Bar, Change-over-time/Line, Correlation/Point, Distribution/histogram, Part-to-whole/stacked). Improves first-run, doubles as living documentation of what the app -does well. Natural home: the snippet library. _Spec impact: §02 (library seed content)._ +does well. Natural home: the snippet library. **Author them in the builder dialect so they +hydrate via 3C.** _Spec impact: §02 (library seed content)._ + +**3C · Open in builder (strict spec hydration)** — _decided 2026-06-11; pairs with 3B_ +Reverse the assembler: a pure `parseChartSpec(spec) → BuilderConfig | null` so an existing +snippet can re-enter the builder populated. **Strict policy only** — a snippet-list button +("Open in builder") enabled exactly when the spec is **losslessly** representable in the +builder's dialect; anything richer stays Monaco-only. Losslessness is checked not by +enumerating unsupported features but by **re-assembling the parsed config and +deep-comparing against the original** (ignoring key order, `$schema`, and the injected +`tooltip: true`) — exact, ~20 lines, and it stays correct automatically as the dialect +grows (every Phase-4 addition widens what hydrates for free). No lossy or +residue-preserving mode: the moment the builder can overwrite a richer spec, the GUI starts +competing with the JSON as the document (the Lyra round-trip trap — §5). + +Value: converts the builder from **create-only to create-and-revise** for its own output, +and makes the 3B starters openable as builder seeds. Known fiddly bits (all mechanical): +inverse of `escapeVegaField`, filter-value un-coercion back to the `BuilderFilter` string +shape, `sort: "-y"` → `{sort: 'descending'}`. Round-trip property tests +(`parse(build(config))` ≈ `config`) are the natural core coverage. Per-row check in the +library is cheap but memoize it. **Decisions at build time:** save semantics on an opened +snippet (update-in-place via a builder "edit" mode vs. always-create-new — today the +builder only creates) and the disabled-button affordance (disabled-with-reason vs. hidden — +park in `ux-second-pass.md` if non-obvious). _Spec impact: §02 (library row action) + §06 +(hydration + the strict gate)._ + +**3D · Entry points & discoverability** — _decided 2026-06-12, council-reviewed; lands with or just before 3A_ +Today the builder has **one entry, two levels deep**: Datasets modal → per-dataset "Build +Chart" (`DatasetsModal.tsx`) — the data model's shape ("charts come from datasets"), not +the user's intent ("I want to make a chart"). Meanwhile the library's pinned primary +action, "Create New Snippet", drops straight into blank Monaco — the **expert** path is +the default, and the audience the builder serves may never find it. Council ruling +(recorded here; contract updates at build time): + +- **NN/g #6 (recognition over recall):** the build-a-chart action must be visible where + the intent forms — the library's creation surface, not recalled as a dataset-row action. +- **NN/g #7 (flexibility/efficiency):** the guided path is what novices see; raw JSON is + the expert accelerator — visible, one click, never hidden. Builder-forward, Monaco + intact. +- **Carbon empty-states:** one primary action per surface ("pick the most important"; a + tertiary button for the secondary CTA); a no-data state explains what the space will + hold, offers one primary next step, and never dead-ends. + +The decided shape: + +1. **Fork the library's creation surface** — primary **Build Chart** (opens the builder), + ghost/tertiary **New JSON snippet** (today's instant create, unchanged). Two plain + buttons with clear hierarchy — no split/menu-button widget (two static choices don't + earn an ARIA menu). +2. **Dataset picker inside the builder** (header select, default = most recently used), so + the builder opens without a preselected dataset. Build-time detail: switching datasets + mid-build re-derives smart defaults vs. prunes incompatible encodings — decide then. + Useful beyond 3D (switch data without leaving the builder). +3. **No-datasets empty state** in the builder, per the Carbon no-data pattern: what the + builder does + one primary action ("Add a dataset" → Datasets modal). No dead end. +4. **Onboarding tile** — a data-first path ("import your data → build a chart") beside the + existing examples gallery. +5. **Keep the dataset-row "Build Chart"** — contextual shortcut, pre-picks the dataset; it + just stops being the only door. + +**Composition with 3A (the point of the sequencing):** the Build-Chart button lands on the +intent front door — "what do you want to show, with which data?" _is_ the builder's +opening screen. 3D without 3A opens onto the mark-first builder (fine, interim); 3A +without 3D is a front door nobody finds. Discoverability is a prerequisite for 3A's value, +so: **3D with or just before 3A.** _Spec impact: §02 (library creation actions + +onboarding) + §06 (dataset picker, empty state)._ ### Builder UX & perf — in-flight fixes (2026-06-10, from dogfooding the Superstore dataset) @@ -418,8 +526,10 @@ Phase 1 1A actionable hints ✓ done 1E expr-validate ✓ done (syntax + unknown-field; completion popup deferred) Phase 2 2A value-or-field channels (Property model) ✓ done 2B field shelf + in-place type cycling ✓ done (field-first + on-chart shelves) -Phase 3 3A intent-first front door (Tier C) ← next; built on 2B; the defining feature +Phase 3 3D entry points & discoverability ← next, with/just before 3A (its prerequisite) + 3A intent-first front door (Tier C) ← built on 2B; the defining feature 3B starter examples + 3C open in builder (strict hydration) ← added 2026-06-11; pairs with 3B Phase 4 (gated) theta/facets/styling-overrides/undo/lookup — decide after Phase 3 Also shipped (builder UX/perf, from dogfooding): near-fullscreen modal, internal-scroll @@ -431,7 +541,10 @@ Rationale for the order: Phase 1 is the cheapest large quality jump and needs no interaction model, so it ships value while the bigger design settles. Phase 2 is pure substrate — low _user-visible_ payoff alone, but Phase 3 is much cleaner on top of it than bolted onto the channel-first UI. Phase 3 delivers the brief's headline ("recommendations -on the fly"). Phase 4 is deliberately deferred to protect the "moderately capable" ceiling. +on the fly") — 3D first, because a front door nobody finds delivers nothing: the entry +points make the builder the app's guided creation flow, then 3A gives that flow its +intent-first opening. Phase 4 is deliberately deferred to protect the "moderately capable" +ceiling. --- diff --git a/docs/spec/06-chart-builder.md b/docs/spec/06-chart-builder.md index 624e83d..fc491fa 100644 --- a/docs/spec/06-chart-builder.md +++ b/docs/spec/06-chart-builder.md @@ -14,7 +14,7 @@ The Chart Builder is a visual, no-JSON way to compose a Vega-Lite chart from a s A two-pane modal: -- **Left — configuration:** dataset name, a **Data** section (row filters, calculated fields, and a collapsible row preview — see _Data_ below), mark type selector, a **field shelf** (the dataset's columns, field-first), a **Marks** card (the Colour and Size encodings), optional width/height inputs, and a "Create Snippet" action. +- **Left — configuration:** dataset name, a **Data** section (row filters, calculated fields, and a collapsible row preview — see _Data_ below), mark type selector, a **field shelf** (the dataset's columns, field-first), a **Marks** card (the Colour and Size encodings), and a "Create Snippet" action. Chart-level properties (title/subtitle, width/height) sit on the **preview side**, under the chart (see _Chart properties_). - **Right — live preview:** the **Columns** (X) and **Rows** (Y) shelves stacked above a rendered chart that updates as the configuration changes, with a placeholder/error area. Position is a property of the chart, so its controls sit on the chart (Tableau's Columns/Rows metaphor). ## Data (preview, filters, calculated fields) @@ -62,9 +62,9 @@ The section is ordered **input → shaping** so the distinction reads at a glanc Exactly four channels are offered: **X, Y** (the positional axes, on the on-chart Columns/Rows shelves) and **Colour, Size** (the Marks card). Assignment is **field-first**. - **The field shelf** lists the dataset's detected columns (see _Datasets_), each with a small type glyph, plus any calculated fields and a field-less **"Count of records"** measure (Vega-Lite `count`). Past a threshold of columns the shelf groups into **Dimensions** (categories/dates) and **Measures** (numerics); a small dataset stays a single flat list. A column already mapped somewhere is dimmed (it may still be placed on more than one channel). -- **Assigning a field:** clicking a shelf field places it on the **armed** channel, else the first empty channel that accepts it (X→Y→Colour→Size). A channel slot is armed by clicking it (then a clicked field fills it); the armed slot shows a "pick a field" prompt. A field that no channel can take is a no-op. -- **A mapped channel is a pill:** a leading type chip, the field (or "Count") label, and a remove (×). The type chip **is the field-type control** — activating it cycles the field's type within the set **valid for that column** and channel (Tier B valid-type locking): number → {Quantitative (default), Ordinal, Nominal}; date → {Temporal}; text → {Nominal (default), Ordinal}; boolean → {Nominal}. When only one type applies (e.g. a date), the chip is inert. A fresh mapping defaults its type from the inferred column type (numeric → Quantitative, date → Temporal, otherwise Nominal). -- **Constant values (the Property model):** the **Colour** and **Size** channels may instead hold a **fixed constant** — a literal colour or size applied to every mark, emitted as Vega-Lite `{ value }` rather than a field binding. An empty Colour/Size slot offers an "or constant" affordance; the bound constant shows a colour picker (Colour) or a number (Size). X and Y stay field-only (a constant position is not useful). Switching a channel between a field and a constant is reversible, and the prior field type is preserved across the toggle. +- **Assigning a field:** clicking a shelf field opens an explicit **channel chooser** listing the channels that accept it (an occupied channel is labelled with what it would replace); picking one places the field there. With a channel **armed**, the chooser is skipped — the clicked field fills the armed channel directly. A channel slot is armed by clicking it; arming is visible at the shelf (an accent ring plus a status line naming the target — "Assigning to X — choose a field below. Esc cancels"), and **Esc** disarms without closing the builder. A field that no channel can take offers no choices. +- **A mapped channel is a pill:** a leading type chip, the field (or "Count") label, and a remove (×). The type chip **is the field-type control** — activating it opens a **direct pick** of the types **valid for that column** and channel (Tier B valid-type locking): number → {Quantitative (default), Ordinal, Nominal}; date → {Temporal}; text → {Nominal (default), Ordinal}; boolean → {Nominal}. When only one type applies (e.g. a date), the chip is inert. A fresh mapping defaults its type from the inferred column type (numeric → Quantitative, date → Temporal, otherwise Nominal). +- **Constant values (the Property model):** the **Colour** and **Size** channels may instead hold a **fixed constant** — a literal colour or size applied to every mark, emitted as Vega-Lite `{ value }` rather than a field binding. An empty Colour/Size slot offers a **"Use a constant"** ghost button; the bound constant shows a colour picker (Colour) or a number (Size). X and Y stay field-only (a constant position is not useful). Switching a channel between a field and a constant is reversible, and the prior field type is preserved across the toggle. - **Size discipline:** the **Size** channel accepts only columns whose natural type is a magnitude (numeric) — size implies an ordered magnitude, so categories and dates are not placed on Size by assignment (they remain available on X/Y/Colour). A constant size is always allowed. - **Clearing** a channel (the pill's ×) leaves it out of the produced spec. - A **Swap X/Y** control, by the on-chart shelves, exchanges the X and Y bindings (field/constant and type) in one click — for quickly flipping the axes of the pre-populated default. @@ -77,7 +77,7 @@ Each on-chart shelf shows, beside its axis slot, a **non-interactive placeholder Once a column is mapped, the channel offers the transforms that apply to its field type — and only those: -- **Aggregate** (a measure / Quantitative field): one of `Sum`, `Mean`, `Median`, `Min`, `Max`, or `None`. (The field-less `Count` measure is chosen via the "Count of records" column option above.) +- **Aggregate** (any field — the menu narrows by field type): a Quantitative field offers `Sum`, `Mean`, `Median`, `Min`, `Max`, `Count distinct`, or `None`; a Temporal or Ordinal field offers `Min`, `Max`, `Count distinct` (an ordering but no arithmetic); a Nominal field offers `Count distinct` alone. `Count distinct` counts a field's unique values, so the channel reads as a **quantitative measure** whatever the field's own type (e.g. "unique customers per region" on a Color or Y channel); the field's asserted type is preserved and restored when the aggregate is removed. (The field-less `Count` measure is chosen via the "Count of records" column option above.) - **Bin** (a Quantitative field): bins the values into ranges — e.g. a Quantitative X binned with a Count Y is a histogram. Binning and aggregating the same field are mutually exclusive (setting one clears the other). - **Granularity** (a Temporal field): a Vega-Lite `timeUnit` — Year, Year-Quarter, Year-Month, Year-Month-Day, Quarter, Month, Week, Day of month, Day of week, Hour — or `None` (raw timestamps). Defaults to **None** (no silent change to what the raw data shows). @@ -106,10 +106,13 @@ The builder surfaces short, plain-language hints for configurations that render A clean configuration shows no hints. -### Dimensions (optional) +### Chart properties (optional) +A slim strip pinned **under the live preview** — these describe the chart itself, so they live on the chart side rather than in the configuration pane: + +- **Title** and **Subtitle** text inputs, written into the spec's top-level `title` (a bare string for a lone title; the `{ text, subtitle }` object form when both are set). A subtitle is emitted only alongside a title — Vega-Lite has no standalone subtitle — so the Subtitle input is disabled until a title exists. A non-empty title is also preferred verbatim as the created snippet's name. - Optional numeric **Width** and **Height** inputs in pixels. -- When left empty, the chart uses default/responsive sizing (consistent with _Live Preview_); when provided, the values are written into the spec. +- When left empty, sizing is default/responsive (consistent with _Live Preview_); when provided, the values are written into the spec **and the builder preview renders at that explicit size** (the preview's fit-to-pane sizing applies only while sizing is auto). ## Live Preview @@ -127,7 +130,7 @@ A clean configuration shows no hints. Selecting "Create Snippet" produces the final artifact: -- Builds a complete Vega-Lite spec containing: the schema reference, a named data reference to the dataset, any top-level `transform` (calculated fields first, then row filters — see _Data_), the chosen mark (with tooltips enabled), the bound encodings (a field encoding carries its field and field type plus any aggregate / bin / `timeUnit` transform; a constant encoding is a `{ value }`), chart-level sort and stacking where set, and any explicit width/height. +- Builds a complete Vega-Lite spec containing: the schema reference, a named data reference to the dataset, any top-level `transform` (calculated fields first, then row filters — see _Data_), the chosen mark (with tooltips enabled), the bound encodings (a field encoding carries its field and field type plus any aggregate / bin / `timeUnit` transform; a constant encoding is a `{ value }`), chart-level sort and stacking where set, any title/subtitle, and any explicit width/height. - Channels left unmapped are omitted; if no encodings exist the spec omits the encoding block entirely (prevented by validation here). - Creates a new snippet from that spec with an auto-generated descriptive name, adds it to the snippet library, and records that it was built from the dataset. - Links the snippet to the dataset by recording the dataset reference, so the bidirectional snippet↔dataset relationship is established (see _Datasets_). diff --git a/docs/ux-second-pass.md b/docs/ux-second-pass.md index 2cdd794..3b761f5 100644 --- a/docs/ux-second-pass.md +++ b/docs/ux-second-pass.md @@ -8,23 +8,15 @@ record the resolution into the contract (`docs/architecture/09`+`10` and the rel ## Open -### Chart Builder — encoding type control: cycle vs. explicit pick +_(none — the 2026-06-12 batch resolved all parked items: type-cycle chip → direct-pick +SelectControl; field-assignment flow → explicit channel chooser + visible armed state, drag +still deferred; "or constant" → "Use a constant" ghost button; chart-level controls → +properties strip under the preview. Resolutions recorded in `architecture/10` §5 and +`spec/06`.)_ -- **Where:** `src/app/components/ChartBuilderModal.tsx` → `ChannelPill` (the `pillType` chip). -- **What:** a mapped channel's field type is shown as a **cycling chip** — activating it steps - N → O → Q → T within the set valid for the column/channel. It is compact and matches the - approved field-first pill design, but a _cycling_ button gives a screen-reader / keyboard - user no way to jump straight to a type; they step through the cycle. The pre-2B control was - an explicit `N | O | Q | T` segmented radio (direct pick, APG radio pattern). -- **Decision to make:** keep the cycle, restore the explicit segmented control, or a hybrid - (chip for a quick mouse cycle + an accessible radio/menu for direct selection). -- **Lens:** WAI-ARIA APG (radio vs. toggle button), IBM Carbon, the project's keyboard-first - accessibility bar. +## Deferred (not design debts, revisit on demand) -### Chart Builder — empty Colour/Size "or constant" affordance - -- **Where:** `ChannelPill` / `ChannelSlot` — the constant-value entry on Colour/Size (2A). -- **What:** an empty Colour/Size slot offers a small underlined "or constant" text button to - switch the channel from a field binding to a fixed `{ value }`. Confirm the copy, emphasis, - and discoverability read well alongside the field-assign affordance. -- **Lens:** Carbon / GOV.UK content + control emphasis. +- **Drag-and-drop field assignment** — chips are click/keyboard-first by design; drag would + be a progressive enhancement on top of the chooser model, not a replacement. Revisit if + real usage asks for it (likely alongside Phase 4 faceting, where drag-to-shelf reads most + naturally). diff --git a/src/app/components/ChartBuilderModal.module.css b/src/app/components/ChartBuilderModal.module.css index 7eddaf4..dce290c 100644 --- a/src/app/components/ChartBuilderModal.module.css +++ b/src/app/components/ChartBuilderModal.module.css @@ -103,18 +103,6 @@ gap: var(--space-2); } -.filterField { - flex: 1; - min-width: 0; - 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; -} - .filterPredicate { display: flex; align-items: center; @@ -173,7 +161,6 @@ font-family: var(--font-mono); } -.filterField:focus-visible, .calcName:focus-visible, .valueInput:focus-visible, .exprInput:focus-visible { @@ -428,16 +415,6 @@ color: var(--text-placeholder); } -.mini { - padding: var(--space-1) var(--space-2); - border: var(--border-width) solid var(--border-strong); - border-radius: var(--radius); - background: var(--bg); - color: var(--text); - font: inherit; - font-size: 12px; -} - .toggle { display: inline-flex; align-items: center; @@ -451,31 +428,11 @@ gap: var(--space-5); } -.mini: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); @@ -713,6 +670,44 @@ color: var(--text-secondary); } +/* Scroll viewport around the field lists: a wide dataset (Superstore: 25 columns) + must not push the Marks card and chart controls below the fold. Group headings + pin to the top while their list scrolls under them. */ +.shelfScroll { + display: flex; + flex-direction: column; + gap: var(--space-2); + max-height: 280px; + overflow-y: auto; +} + +/* An armed channel makes the shelf the live target — say so where the next click + happens (NN/g #1): an accent ring on the list plus the status line above it. */ +.shelfArmed { + outline: 2px solid var(--accent); + outline-offset: 2px; + border-radius: var(--radius); +} + +.armedHint { + margin: 0; + font-size: 12px; + color: var(--text-secondary); +} + +.armedHint strong { + color: var(--accent); +} + +.shelfScroll .shelfGroupHead { + position: sticky; + top: 0; + z-index: 1; + margin-top: 0; + padding: var(--space-2) 0; + background: var(--layer-01); +} + .shelfList { display: flex; flex-direction: column; @@ -785,11 +780,14 @@ gap: var(--space-2); } -/* Pill — a bound channel (field/count or constant). */ +/* Pill — a bound channel (field/count or constant). The per-channel transform + controls (Aggregate/Bin/Granularity) flow inline beside the pill, wrapping only + when the row truly runs out of room (a narrow window; the Marks card). */ .pillWrap { display: flex; - flex-direction: column; - gap: var(--space-2); + flex-wrap: wrap; + align-items: center; + gap: var(--space-2) var(--space-3); min-width: 0; } @@ -827,6 +825,10 @@ opacity: 0.85; } +.pillType[aria-expanded='true'] { + background: color-mix(in srgb, #000 30%, var(--accent)); +} + .pillType:focus-visible { outline: 2px solid var(--accent-contrast); outline-offset: -2px; @@ -841,6 +843,8 @@ } .pillName { + /* Grow into any pill slack so the remove ✕ stays at the pill's right edge. */ + flex: 1; min-width: 0; padding: var(--space-2) var(--space-3); overflow: hidden; @@ -937,19 +941,22 @@ outline-offset: 1px; } +/* Ghost button (Carbon: links navigate, buttons act — this one acts). */ .slotConst { - padding: var(--space-1); + padding: var(--space-1) var(--space-2); border: none; + border-radius: var(--radius); background: none; color: var(--accent); font: inherit; font-size: 11px; white-space: nowrap; - text-decoration: underline; cursor: pointer; + transition: background var(--dur-fast) var(--ease); } .slotConst:hover { + background: var(--layer-02); color: var(--accent-hover); } @@ -966,10 +973,12 @@ padding: var(--space-5) var(--space-5) 0; } +/* Swap sits beside the heading — an action flushed to the far edge of a very wide + pane reads as unrelated to the shelves it acts on. */ .shelvesHead { display: flex; align-items: baseline; - justify-content: space-between; + gap: var(--space-4); } .shelfStrip { @@ -1000,11 +1009,53 @@ padding: var(--space-2) var(--space-3); } -.shelfSlots .slot, -.shelfSlots .pillWrap { +/* Empty slots keep a minimum presence; a filled pill hugs its content so the + remove ✕ sits at its right edge rather than adrift in dead pill background. */ +.shelfSlots .slot { min-width: 190px; } +/* Chart properties — title/subtitle/size as a slim strip pinned under the preview + (chart-level properties live on the chart side; council 2026-06-12). */ +.chartProps { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: var(--space-4); + padding: var(--space-3) var(--space-5); + border-top: var(--border-width) solid var(--border); +} + +.propField { + display: inline-flex; + align-items: center; + gap: var(--space-2); + font-size: 12px; + color: var(--text-secondary); + white-space: nowrap; +} + +.propInput { + width: 200px; + padding: var(--space-1) var(--space-2); + border: var(--border-width) solid var(--border-strong); + border-radius: var(--radius); + background: var(--bg); + color: var(--text); + font: inherit; + font-size: 12px; +} + +.propInput:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +.propInput:focus-visible { + outline: 2px solid var(--focus); + outline-offset: -1px; +} + /* Reserved faceting placeholder (Phase 4) — non-interactive, signals where it'll live. */ .facetSlot { display: flex; diff --git a/src/app/components/ChartBuilderModal.test.tsx b/src/app/components/ChartBuilderModal.test.tsx index b6cda59..716de13 100644 --- a/src/app/components/ChartBuilderModal.test.tsx +++ b/src/app/components/ChartBuilderModal.test.tsx @@ -194,7 +194,7 @@ describe('ChartBuilderModal', () => { await vi.advanceTimersByTimeAsync(400); // drive the debounced preview render }); - expect(container.querySelector('select[aria-label="Filter operator"]')).toBeTruthy(); + expect(container.querySelector('button[aria-label^="Filter operator"]')).toBeTruthy(); const calls = vi.mocked(renderSpec).mock.calls; const lastSpec = calls[calls.length - 1][1] as { transform?: unknown }; expect(lastSpec.transform).toEqual([{ filter: { field: 'revenue', gt: 60 } }]); @@ -297,7 +297,7 @@ describe('ChartBuilderModal', () => { expect(refLink()!.getAttribute('href')).toContain('vega.github.io'); }); - test('clicking a field in the shelf assigns it to a channel as a pill (field-first, 2B)', async () => { + test('clicking a field opens the channel chooser; picking a channel assigns it (field-first, 2B)', async () => { const ds = createDataset({ name: 'Shop', data: [{ region: 'E', sales: 5 }], @@ -309,7 +309,6 @@ describe('ChartBuilderModal', () => { const id = useDatasetStore.getState().datasets[0].id; const store = useChartBuilderStore.getState(); store.init(id); - // Clear the smart-default axes so the click lands on the first empty channel (X). store.setChannelColumn('x', null); store.setChannelColumn('y', null); @@ -328,12 +327,100 @@ describe('ChartBuilderModal', () => { await Promise.resolve(); }); + // Unarmed, the click opens an explicit channel chooser (portaled to ) + // rather than silently filling the first empty seat (council 2026-06-12). + expect(useChartBuilderStore.getState().config.encodings.x).toBeNull(); + const option = Array.from(document.body.querySelectorAll('button')).find((b) => + b.textContent?.includes('Columns (X)'), + ); + expect(option).toBeDefined(); + + await act(async () => { + option!.click(); + await Promise.resolve(); + }); + expect(useChartBuilderStore.getState().config.encodings.x).toEqual({ field: 'region', type: 'nominal', }); }); + test('an armed channel short-circuits the chooser: the field assigns directly (2B)', async () => { + const ds = createDataset({ + name: 'Shop', + data: [{ region: 'E', sales: 5 }], + format: 'json', + source: 'inline', + now: T, + }); + useDatasetStore.getState().add(ds); + const id = useDatasetStore.getState().datasets[0].id; + const store = useChartBuilderStore.getState(); + store.init(id); + store.setChannelColumn('x', null); + store.setChannelColumn('y', null); + store.focusChannel('y'); + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + // The armed state announces itself at the shelf. + expect(container.textContent).toContain('Assigning to Y'); + + const fieldButton = Array.from(container.querySelectorAll('button')).find((b) => + b.textContent?.includes('region'), + ); + await act(async () => { + fieldButton!.click(); + await Promise.resolve(); + }); + + expect(useChartBuilderStore.getState().config.encodings.y).toEqual({ + field: 'region', + type: 'nominal', + }); + expect(useChartBuilderStore.getState().activeChannel).toBeNull(); + }); + + test('opening a SelectControl lands focus on the selected option, not the first (regression)', async () => { + const ds = createDataset({ + name: 'Sales', + data: [{ day: '2026-01-01', v: 1 }], + format: 'json', + source: 'inline', + now: T, + }); + useDatasetStore.getState().add(ds); + const id = useDatasetStore.getState().datasets[0].id; + const store = useChartBuilderStore.getState(); + store.init(id); + store.setChannelColumn('x', 'day'); // temporal → the pill offers Granularity + store.setChannelTimeUnit('x', 'month'); // "Month" sits mid-list, after "None (raw)" + + await act(async () => { + root.render(); + await Promise.resolve(); + }); + + const trigger = container.querySelector( + 'button[aria-label^="Granularity for"]', + ); + expect(trigger).toBeTruthy(); + await act(async () => { + trigger!.click(); + await Promise.resolve(); + }); + + // A selector list ('[aria-current="true"], button') would return the first + // button in document order — the "None (raw)" option — instead of the selection. + const focused = document.activeElement as HTMLElement; + expect(focused.getAttribute('aria-current')).toBe('true'); + expect(focused.textContent).toContain('Month'); + }); + test('Colour can be switched to a constant value (the Property model, 2A/2B)', async () => { const ds = createDataset({ name: 'Shop', @@ -351,9 +438,10 @@ describe('ChartBuilderModal', () => { await Promise.resolve(); }); - // The empty Colour slot offers an "or constant" affordance (Colour is first in Marks). + // The empty Colour slot offers a "Use a constant" ghost button (Colour is + // first in Marks). const constButton = Array.from(container.querySelectorAll('button')).find( - (b) => b.textContent === 'or constant', + (b) => b.textContent === 'Use a constant', ); expect(constButton).toBeDefined(); diff --git a/src/app/components/ChartBuilderModal.tsx b/src/app/components/ChartBuilderModal.tsx index 9df9c54..3c131c2 100644 --- a/src/app/components/ChartBuilderModal.tsx +++ b/src/app/components/ChartBuilderModal.tsx @@ -1,15 +1,19 @@ /** * Chart Builder — the modal body (spec §06). * - * A two-pane composer: left is the configuration (dataset name, mark selector, one - * block per channel, chart-level sort/stacking, 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. Each channel is a small block: - * a column dropdown (with a field-less "Count of records" option), a fixed - * `N | O | Q | T` field-type segmented control (the column's invalid types are - * disabled), and the transforms that apply to its type (aggregate + bin for a - * measure, granularity for a temporal field). The preview is builder-local (its own + * A two-pane composer. Left: the Data section (filters, calculated fields, row + * preview), the mark selector, the field shelf (columns as type-glyphed chips), the + * Marks card (Colour/Size — field or constant), chart-level sort/stacking, guidance, + * and Create. Right: the on-chart Columns/Rows shelves, the live preview, and the + * chart-properties strip (title/subtitle/size). All spec logic and defaults/guards + * come from `@core/chart-builder` via `ChartBuilderStore`; this component is the view. + * + * Assignment is field-first: a chip click opens an explicit channel chooser, or + * assigns directly when a channel is armed (the slot's "Pick a field…" state, made + * visible at the shelf). A bound channel renders as a pill — a field-type chip that + * opens a direct type pick, the field name, a remove ✕ — with its per-type + * transforms (aggregate / bin / granularity) inline beside it. Pickers use + * `SelectControl`, not native selects. 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. */ @@ -29,12 +33,14 @@ import { filterOpArity, isBuilderConfigValid, isChannelTypeAllowed, + isColumnAllowedOnChannel, isValueMapping, supportsAggregate, supportsBin, supportsSort, supportsStack, supportsTimeUnit, + validAggregateOps, validFieldTypes, validFilterOps, type BuilderCalculate, @@ -65,6 +71,7 @@ import { useChartBuilderStore, } from '../stores/ChartBuilderStore'; import { SegmentedControl, type SegmentedOption } from './SegmentedControl'; +import { SelectControl } from './SelectControl'; import { Icon } from './Icon'; import styles from './ChartBuilderModal.module.css'; @@ -124,8 +131,16 @@ const TYPE_ABBR: Record = { temporal: 'T', }; -/** Non-count aggregate operators offered for a quantitative field. */ -const FIELD_AGGREGATES: readonly AggregateOp[] = ['sum', 'mean', 'median', 'min', 'max']; +/** Readable labels for the non-count aggregate operators (`validAggregateOps` + * supplies the per-type menu — e.g. a Nominal field offers only Count distinct). */ +const AGGREGATE_LABELS: Record, string> = { + sum: 'Sum', + mean: 'Mean', + median: 'Median', + min: 'Min', + max: 'Max', + distinct: 'Count distinct', +}; /** Friendly labels for each temporal granularity. */ const TIME_UNIT_LABELS: Record = { @@ -286,14 +301,9 @@ function ChannelPill({ const count = isCount(mapping); const typeOptions = count ? [] : channelTypeOptions(channel, mapping, columns); - const canCycle = typeOptions.length > 1; + const canPick = typeOptions.length > 1; const currentType: FieldType = count ? 'quantitative' : mapping.type; const label = count ? 'Count' : (mapping.field ?? ''); - const cycleType = () => { - if (!canCycle) return; - const i = typeOptions.indexOf(mapping.type); - setChannelType(channel, typeOptions[(i + 1) % typeOptions.length]); - }; const hasTransforms = !count && @@ -304,18 +314,20 @@ function ChannelPill({ return (
- {/* TODO(ux-second-pass): the type chip cycles N→O→Q→T — no direct pick for - keyboard/SR users. Cycle vs. explicit radio is parked for a batched council - review (docs/ux-second-pass.md). */} - + {/* The type chip opens a direct pick of the valid types (council 2026-06-12: + a cycling button gave keyboard/SR users no way to jump to a type). */} + ({ value: t, label: titleCase(t) }))} + value={currentType} + onSelect={(t) => setChannelType(channel, t)} + triggerClassName={styles.pillType} + triggerContent={TYPE_ABBR[currentType]} + triggerTitle={titleCase(currentType)} + disabled={!canPick} + /> {label} @@ -327,26 +339,25 @@ function ChannelPill({ {hasTransforms && (
{supportsAggregate(mapping.type) && ( - + onSelect={(v) => setChannelAggregate(channel, v || undefined)} + /> +
)} {supportsBin(mapping.type) && ( @@ -361,23 +372,22 @@ function ChannelPill({ )} {supportsTimeUnit(mapping.type) && ( - + onSelect={(v) => setChannelTimeUnit(channel, v || undefined)} + /> +
)}
)} @@ -421,36 +431,75 @@ function ChannelSlot({ channel, hint }: { channel: ChannelName; hint?: string }) {active ? 'Pick a field…' : (hint ?? 'Add a field')} {channelAcceptsValue(channel) && ( + // A ghost button, not a link-styled affordance: it acts (binds a constant), + // and Carbon draws the line at links navigate / buttons act (council + // 2026-06-12). Verb-first label per the content rules. )} ); } +/** What a channel currently shows, for the assignment chooser's "replaces …" hint. */ +function occupantLabel(mapping: ChannelMapping | null | undefined): string | undefined { + if (!mapping) return undefined; + if (mapping.value !== undefined) return 'replaces the constant'; + if (mapping.aggregate === 'count' && !mapping.field) return 'replaces Count'; + return mapping.field ? `replaces ${mapping.field}` : undefined; +} + +/** Channel names as the assignment chooser shows them (the on-chart shelf words). */ +const ASSIGN_LABELS: Record = { + x: 'Columns (X)', + y: 'Rows (Y)', + color: 'Color', + size: 'Size', +}; + /** * The field shelf (spec §06 → Encoding, field-first): the dataset's columns (plus any * calculated fields and a field-less "Count of records") as clickable chips with a type - * glyph. Clicking a field assigns it to the armed channel, else the first empty channel - * that accepts it (`assignField`). Past `FIELD_SHELF_SPLIT_MIN` columns it groups into - * Dimensions (categories/dates) and Measures (numerics); a small dataset stays flat. - * Already-mapped fields are dimmed (a field may still be placed on several channels). + * glyph. Clicking a chip opens an explicit channel chooser (the channels that accept the + * column; an occupied one says what it would replace); with a channel armed, the click + * assigns straight there instead and Esc disarms. Past `FIELD_SHELF_SPLIT_MIN` columns it + * groups into Dimensions (categories/dates) and Measures (numerics); a small dataset stays + * flat. Already-mapped fields are dimmed (a field may still be placed on several channels). */ function FieldShelf() { const baseColumns = useChartBuilderStore((s) => s.columns); const calculates = useChartBuilderStore((s) => s.config.calculates); const encodings = useChartBuilderStore((s) => s.config.encodings); const assignField = useChartBuilderStore((s) => s.assignField); + const activeChannel = useChartBuilderStore((s) => s.activeChannel); + const focusChannel = useChartBuilderStore((s) => s.focusChannel); const columns = useMemo( () => effectiveColumns(baseColumns, calculates), [baseColumns, calculates], ); + // Esc disarms the armed channel (captured so the modal itself doesn't close). + // TODO: with a channel armed AND a SelectControl popover open, one Esc both closes + // the popover and disarms — both are capture-phase document listeners, so + // stopPropagation can't serialize them. Rare combination; needs a shared + // escape-layer stack if it ever matters. + useEffect(() => { + if (!activeChannel) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + e.stopPropagation(); + focusChannel(null); + } + }; + document.addEventListener('keydown', onKey, true); + return () => document.removeEventListener('keydown', onKey, true); + }, [activeChannel, focusChannel]); + const assigned = useMemo(() => { const set = new Set(); for (const ch of CHANNELS) { @@ -463,33 +512,59 @@ function FieldShelf() { const colTypeOf = (name: string): ColumnType => columns.columnTypes.find((c) => c.name === name)?.type ?? 'string'; - const fieldButton = (name: string) => ( - - ); + // A field chip: with a channel armed, clicking assigns straight there (the fast + // path); unarmed, it opens an explicit channel chooser instead of silently + // grabbing the first empty seat (council 2026-06-12 — NN/g #3, user choice). + const fieldChip = ( + name: string, + display: string, + domId: string, + colType: ColumnType, + glyph: string, + ) => { + const choices = CHANNELS.filter((ch) => isColumnAllowedOnChannel(ch, colType)); + return ( + ({ + value: ch, + label: ASSIGN_LABELS[ch], + detail: occupantLabel(encodings[ch]), + }))} + onSelect={(ch) => assignField(name, ch)} + beforeOpen={() => { + if (activeChannel) { + assignField(name); + return false; + } + return true; + }} + triggerClassName={`${styles.shelfField} ${assigned.has(name) ? styles.shelfFieldUsed : ''}`} + triggerContent={ + <> + + {display} + + } + /> + ); + }; - const countButton = ( - - ); + const fieldButton = (name: string) => + fieldChip( + name, + name, + `f${columns.columns.indexOf(name)}`, + colTypeOf(name), + TYPE_ABBR[defaultFieldType(colTypeOf(name))], + ); + + const countButton = fieldChip(COUNT_FIELD, 'Count of records', 'count', 'number', '∑'); const dimensions = columns.columns.filter((n) => colTypeOf(n) !== 'number'); const measures = columns.columns.filter((n) => colTypeOf(n) === 'number'); @@ -499,22 +574,33 @@ function FieldShelf() { return (
Fields - {split ? ( - <> - Dimensions -
{dimensions.map(fieldButton)}
- Measures + {/* Arming a channel must be visible at the place the next click happens + (NN/g #1): the shelf gains an accent ring and a status line naming the + target. The hint is a polite status so AT hears the mode change too. */} + {activeChannel && ( +

+ Assigning to {CHANNEL_LABELS[activeChannel]} — choose a field below. Esc + cancels. +

+ )} +
+ {split ? ( + <> + Dimensions +
{dimensions.map(fieldButton)}
+ Measures +
+ {measures.map(fieldButton)} + {countButton} +
+ + ) : (
- {measures.map(fieldButton)} + {columns.columns.map(fieldButton)} {countButton}
- - ) : ( -
- {columns.columns.map(fieldButton)} - {countButton} -
- )} + )} +
); } @@ -687,19 +773,15 @@ function FilterRow({ filter, columns }: { filter: BuilderFilter; columns: Builde onChange={(e) => updateFilter(filter.id, { expr: e.target.value })} /> ) : ( - + ({ value: name, label: name }))} + value={filter.field} + onSelect={(name) => setFilterField(filter.id, name)} + triggerContent={filter.field ? undefined : 'Choose a field…'} + /> )} + {open && + createPortal( +
+

{heading ?? label}

+
+ {options.map((o) => { + const selected = value !== undefined && o.value === value; + return ( + + ); + })} +
+
, + document.body, + )} + + ); +} diff --git a/src/app/stores/ChartBuilderStore.test.ts b/src/app/stores/ChartBuilderStore.test.ts index 621d01f..4a84d69 100644 --- a/src/app/stores/ChartBuilderStore.test.ts +++ b/src/app/stores/ChartBuilderStore.test.ts @@ -104,6 +104,38 @@ describe('transforms — aggregate / bin / timeUnit / count', () => { expect(cb().config.encodings.x).toEqual({ field: 'price', type: 'nominal' }); }); + test('assignField with an explicit channel places the field there, replacing any occupant', () => { + const id = seedDataset('S', [{ region: 'N', revenue: 5 }]); + cb().init(id); + cb().setChannelColumn('x', 'revenue'); + cb().assignField('region', 'x'); // explicit target from the chip's channel chooser + expect(cb().config.encodings.x).toEqual({ field: 'region', type: 'nominal' }); + expect(cb().activeChannel).toBeNull(); + }); + + test('setTitle / setSubtitle set and clear the chart title fields', () => { + const id = seedDataset('S', [{ v: 1 }]); + cb().init(id); + cb().setTitle('My chart'); + cb().setSubtitle('A closer look'); + expect(cb().config.title).toBe('My chart'); + expect(cb().config.subtitle).toBe('A closer look'); + cb().setTitle(''); + expect(cb().config.title).toBeUndefined(); + }); + + test('retyping drops a now-invalid aggregate but keeps a distinct (valid for any type)', () => { + const id = seedDataset('S', [{ price: 5 }]); + cb().init(id); + cb().setChannelColumn('x', 'price'); + cb().setChannelAggregate('x', 'sum'); + cb().setChannelType('x', 'nominal'); // sum can't reduce a nominal field + expect(cb().config.encodings.x?.aggregate).toBeUndefined(); + cb().setChannelAggregate('x', 'distinct'); + cb().setChannelType('x', 'ordinal'); // distinct counts unique values of anything + expect(cb().config.encodings.x?.aggregate).toBe('distinct'); + }); + test('setChannelTimeUnit sets and clears granularity', () => { const id = seedDataset('S', [{ day: '2026-01-01', v: 1 }]); cb().init(id); diff --git a/src/app/stores/ChartBuilderStore.ts b/src/app/stores/ChartBuilderStore.ts index f9bd977..1c80dbe 100644 --- a/src/app/stores/ChartBuilderStore.ts +++ b/src/app/stores/ChartBuilderStore.ts @@ -27,9 +27,9 @@ import { isChannelTypeAllowed, isColumnAllowedOnChannel, pruneEncodings, - supportsAggregate, supportsBin, supportsTimeUnit, + validAggregateOps, validFieldTypes, validFilterOps, type AggregateOp, @@ -106,9 +106,11 @@ export interface ChartBuilderState { /** * Place a clicked shelf field on a channel: the armed channel if it accepts the * column, else the first empty channel that does (X→Y→Color→Size). A no-op when no - * channel can take it. Clears the armed slot. + * channel can take it. Clears the armed slot. An explicit `channel` (the field + * chip's channel chooser) overrides both — it places the column there even when + * the channel is occupied (the chooser labels that as a replacement). */ - assignField: (columnName: string) => void; + assignField: (columnName: string, channel?: ChannelName) => void; /** * Set/replace a channel's binding with a **constant value** (the Property model, * spec §06) — a fixed colour/size emitted as Vega-Lite `{ value }`. Only Color/Size @@ -151,6 +153,10 @@ export interface ChartBuilderState { removeCalculate: (id: string) => void; setWidth: (width: number | undefined) => void; setHeight: (height: number | undefined) => void; + /** Set/clear the chart title (whitespace-only clears). */ + setTitle: (raw: string) => void; + /** Set/clear the subtitle; emitted into the spec only alongside a title. */ + setSubtitle: (raw: string) => void; /** Build the spec, create + activate a linked snippet, toast, and close. */ createSnippet: (now?: Date) => boolean; reset: () => void; @@ -259,9 +265,9 @@ export const useChartBuilderStore = create((set, get) => ({ focusChannel: (channel) => set({ activeChannel: channel }), - assignField: (columnName) => + assignField: (columnName, channel) => set((s) => { - const target = chooseTargetChannel(s, columnName); + const target = channel ?? chooseTargetChannel(s, columnName); if (!target) return { activeChannel: null }; // nothing can take it → just disarm const mapping = mappingForColumn(s, target, columnName); return { @@ -288,10 +294,18 @@ export const useChartBuilderStore = create((set, get) => ({ set((s) => { const current = s.config.encodings[channel]; if (!current) return s; // no field on this channel → nothing to retype - // Drop transforms that no longer apply to the new type (e.g. an aggregate or - // bin when leaving Quantitative, a granularity when leaving Temporal). + // Drop transforms that no longer apply to the new type (e.g. a sum or bin when + // leaving Quantitative, a granularity when leaving Temporal). A `distinct` + // survives any retype (it applies to every field type); `count` is field-less + // and never reaches here (a count pill offers no type cycle). const next: ChannelMapping = { ...current, type }; - if (!supportsAggregate(type)) delete next.aggregate; + if ( + next.aggregate && + next.aggregate !== 'count' && + !validAggregateOps(type).includes(next.aggregate) + ) { + delete next.aggregate; + } if (!supportsBin(type)) delete next.bin; if (!supportsTimeUnit(type)) delete next.timeUnit; return updateEncoding(s, channel, next); @@ -451,6 +465,11 @@ export const useChartBuilderStore = create((set, get) => ({ setWidth: (width) => set((s) => ({ config: { ...s.config, width } })), setHeight: (height) => set((s) => ({ config: { ...s.config, height } })), + // The raw input value is kept (mid-edit spaces and all) so typing feels normal; + // the assembler trims and skips a blank/whitespace-only title at emission. + setTitle: (raw) => set((s) => ({ config: { ...s.config, title: raw || undefined } })), + setSubtitle: (raw) => set((s) => ({ config: { ...s.config, subtitle: raw || undefined } })), + createSnippet: (now) => { const { config } = get(); if (!isBuilderConfigValid(config)) return false; // guarded by a disabled action too diff --git a/src/core/chart-builder.test.ts b/src/core/chart-builder.test.ts index 7dc458e..776ed8a 100644 --- a/src/core/chart-builder.test.ts +++ b/src/core/chart-builder.test.ts @@ -6,6 +6,7 @@ import { isChannelTypeAllowed, isColumnAllowedOnChannel, supportsAggregate, + validAggregateOps, supportsBin, supportsTimeUnit, supportsSort, @@ -840,12 +841,96 @@ describe('transforms — aggregate / bin / timeUnit', () => { it('exposes the transform-applicability predicates by field type', () => { expect(supportsAggregate('quantitative')).toBe(true); - expect(supportsAggregate('nominal')).toBe(false); + // Every type now takes at least one aggregate (`distinct` applies to anything). + expect(supportsAggregate('nominal')).toBe(true); expect(supportsBin('quantitative')).toBe(true); expect(supportsBin('temporal')).toBe(false); expect(supportsTimeUnit('temporal')).toBe(true); expect(supportsTimeUnit('quantitative')).toBe(false); }); + + it('offers per-type aggregate menus: arithmetic needs numbers, min/max an ordering, distinct anything', () => { + expect(validAggregateOps('quantitative')).toEqual([ + 'sum', + 'mean', + 'median', + 'min', + 'max', + 'distinct', + ]); + expect(validAggregateOps('temporal')).toEqual(['min', 'max', 'distinct']); + expect(validAggregateOps('ordinal')).toEqual(['min', 'max', 'distinct']); + expect(validAggregateOps('nominal')).toEqual(['distinct']); + }); + + it('emits a distinct-count of a categorical field as a quantitative measure', () => { + const spec = buildChartSpec({ + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'region', type: 'nominal' }, + y: { field: 'customer', type: 'nominal', aggregate: 'distinct' }, + }, + }); + const enc = spec.encoding as Record>; + // The carried (nominal) type is for round-tripping; the emitted type is the + // effective one — a count of unique values reads as a quantitative measure. + expect(enc.y).toEqual({ field: 'customer', type: 'quantitative', aggregate: 'distinct' }); + }); + + it('keeps the field type on order-preserving aggregates (a temporal min is still temporal)', () => { + const spec = buildChartSpec({ + datasetName: 'D', + mark: 'point', + encodings: { + x: { field: 'region', type: 'nominal' }, + y: { field: 'orderDate', type: 'temporal', aggregate: 'min' }, + }, + }); + const enc = spec.encoding as Record>; + expect(enc.y).toEqual({ field: 'orderDate', type: 'temporal', aggregate: 'min' }); + }); + + it('emits title as a bare string, the object form with a subtitle, nothing without a title', () => { + const base: BuilderConfig = { + datasetName: 'D', + mark: 'bar', + encodings: { x: { field: 'region', type: 'nominal' } }, + }; + expect(buildChartSpec({ ...base, title: 'Sales by region' }).title).toBe('Sales by region'); + expect(buildChartSpec({ ...base, title: 'Sales', subtitle: 'FY26' }).title).toEqual({ + text: 'Sales', + subtitle: 'FY26', + }); + // A subtitle alone is not emitted (VL has no standalone subtitle), nor is a + // whitespace-only title. + expect(buildChartSpec({ ...base, subtitle: 'orphan' }).title).toBeUndefined(); + expect(buildChartSpec({ ...base, title: ' ' }).title).toBeUndefined(); + }); + + it('prefers a user-written title as the generated snippet name', () => { + expect( + generateChartName({ + datasetName: 'D', + mark: 'bar', + title: 'Quarterly revenue', + encodings: { x: { field: 'region', type: 'nominal' } }, + }), + ).toBe('Quarterly revenue'); + }); + + it('names a distinct-count chart with a "unique" phrase', () => { + expect( + generateChartName({ + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'region', type: 'nominal' }, + y: { field: 'customer', type: 'nominal', aggregate: 'distinct' }, + }, + }), + ).toBe('Bar chart of unique customer by region'); + }); }); describe('sort (ranking)', () => { diff --git a/src/core/chart-builder.ts b/src/core/chart-builder.ts index 725c80e..4e9dd58 100644 --- a/src/core/chart-builder.ts +++ b/src/core/chart-builder.ts @@ -44,9 +44,12 @@ export type ChannelName = (typeof CHANNELS)[number]; /** * Aggregation operators a channel may apply (Vega-Lite `aggregate`). `count` is * special — it is **field-less** (counts records), so a `count` mapping carries no - * `field`. The rest reduce a quantitative `field`. + * `field`. `distinct` counts a field's unique values, so it applies to **any** field + * type and reads as a quantitative measure. The arithmetic ops (sum/mean/median) + * reduce a quantitative field; min/max also order a temporal or ordinal one. See + * `validAggregateOps` for the per-type menu. */ -export const AGGREGATE_OPS = ['count', 'sum', 'mean', 'median', 'min', 'max'] as const; +export const AGGREGATE_OPS = ['count', 'distinct', 'sum', 'mean', 'median', 'min', 'max'] as const; export type AggregateOp = (typeof AGGREGATE_OPS)[number]; /** @@ -193,6 +196,10 @@ export interface BuilderConfig { mark: MarkType; /** Per-channel mapping; `null` (or absent) means the channel is unmapped. */ encodings: Partial>; + /** Optional chart title (Vega-Lite top-level `title`). */ + title?: string; + /** Optional subtitle; emitted only alongside a title (VL nests it under `title`). */ + subtitle?: string; /** Optional explicit chart width in pixels. */ width?: number; /** Optional explicit chart height in pixels. */ @@ -276,9 +283,29 @@ export function isColumnAllowedOnChannel(channel: ChannelName, columnType: Colum return isChannelTypeAllowed(channel, defaultFieldType(columnType)); } -/** Whether a non-count aggregate (sum/mean/…) can apply to this field type. */ +/** + * The non-count aggregates that legitimately apply to a field of this type — the + * channel's Aggregate menu. Arithmetic reduction (sum/mean/median) needs numbers; + * min/max need an ordering (numbers, dates, asserted-ordinal values); `distinct` + * (count of unique values) applies to anything — the natural measure to wring out + * of a category ("how many unique customers"), which is why the menu is per-type + * rather than quantitative-only. + */ +export function validAggregateOps(type: FieldType): Exclude[] { + switch (type) { + case 'quantitative': + return ['sum', 'mean', 'median', 'min', 'max', 'distinct']; + case 'temporal': + case 'ordinal': + return ['min', 'max', 'distinct']; + case 'nominal': + return ['distinct']; + } +} + +/** Whether any non-count aggregate (sum/…/distinct) can apply to this field type. */ export function supportsAggregate(type: FieldType): boolean { - return type === 'quantitative'; + return validAggregateOps(type).length > 0; } /** Whether binning into ranges can apply to this field type. */ @@ -498,9 +525,13 @@ function mappedChannels(config: BuilderConfig): Array<[ChannelName, ChannelMappi }); } -/** The effective field type a mapping encodes (a count is quantitative). */ +/** The effective field type a mapping encodes: a count or a distinct-count reads as + * a quantitative measure whatever the underlying field; other aggregates (sum, a + * temporal min/…) keep the field's own type. */ function effectiveType(mapping: ChannelMapping): FieldType { - return mapping.aggregate === 'count' ? 'quantitative' : mapping.type; + return mapping.aggregate === 'count' || mapping.aggregate === 'distinct' + ? 'quantitative' + : mapping.type; } /** True when a mapping reads as a continuous measure (count/aggregate or continuous type). */ @@ -1034,7 +1065,9 @@ function encodingObject(mapping: ChannelMapping): Record { // Escape `.`/`[`/`]` so a column literally named e.g. `user.age` is read as that // field, not a nested-property accessor (docs/architecture/05 §4). if (mapping.field !== undefined) enc.field = escapeVegaField(mapping.field); - enc.type = mapping.type; + // The emitted type is the *effective* one: a distinct-count of any field is a + // quantitative measure (the carried field type is preserved for a later un-aggregate). + enc.type = effectiveType(mapping); if (mapping.aggregate) enc.aggregate = mapping.aggregate; if (mapping.bin) enc.bin = true; if (mapping.timeUnit) enc.timeUnit = mapping.timeUnit; @@ -1047,7 +1080,7 @@ function encodingObject(mapping: ChannelMapping): Record { * any top-level `transform` (calculated fields then row filters), the mark with * tooltips enabled, every mapped encoding (field, type, and any aggregate/bin/ * timeUnit transform), chart-level sort (rank a categorical axis by its measure) - * and stack (part-to-whole), and any explicit width/height. Unmapped + * and stack (part-to-whole), any title/subtitle, and any explicit width/height. Unmapped * channels are omitted; if nothing is mapped the `encoding` block is omitted * entirely (validation prevents saving that, but the live preview may render a bare * mark while the user is still configuring). @@ -1063,6 +1096,15 @@ export function buildChartSpec(config: BuilderConfig): ChartSpec { const transform = buildTransforms(config); if (transform.length > 0) spec.transform = transform; + // Title/subtitle: a bare string for a lone title, the object form when a + // subtitle rides along. A subtitle without a title is not emitted (VL has no + // standalone subtitle; the UI disables the input until a title exists). + const title = config.title?.trim(); + if (title) { + const subtitle = config.subtitle?.trim(); + spec.title = subtitle ? { text: title, subtitle } : title; + } + spec.mark = { type: config.mark, tooltip: true }; const encoding: Record> = {}; @@ -1109,6 +1151,7 @@ function describeMapping(mapping: ChannelMapping): string { if (mapping.value !== undefined) return 'a constant'; if (mapping.aggregate === 'count') return 'count'; const field = mapping.field ?? ''; + if (mapping.aggregate === 'distinct') return `unique ${field}`; if (mapping.aggregate) return `${mapping.aggregate} of ${field}`; return field; } @@ -1121,6 +1164,9 @@ function describeMapping(mapping: ChannelMapping): string { * no timestamp — so the name describes the chart, not when it was made. */ export function generateChartName(config: BuilderConfig): string { + // A user-written chart title is the best possible name — prefer it verbatim. + const title = config.title?.trim(); + if (title) return title; const mark = markLabel(config.mark); const x = config.encodings.x; const y = config.encodings.y;