diff --git a/CHANGELOG.md b/CHANGELOG.md index ebed59d0..47f51567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this ## [Unreleased] +### Fixed + +- **library:** rejected updates preserve the last valid chart/table; animated maps, tile maps and tables resume resizing after entrance completion. Sankey updates no longer replay entrances, and disabling tooltips no longer disables callbacks. +- **inspection:** layered charts retain each leaf's tooltip formatting, observation identity and crosshair evidence even when series names repeat or paint order changes. Reader pins reconcile filter visibility without requiring data replacement; unsupported layered pinning clears with an announcement. +- **keyboard:** maps, tile maps, bar lists and Sankey share keyboard inspection/activation; graph Tab no longer traps focus. Tables preserve logical focus, cancel pending search on Escape, distinguish typed/ambiguous row keys and preserve native mobile disclosure controls. +- **data:** missing, blank, boolean and nonfinite measurements no longer fabricate zero-valued marks, coordinates or differences. Sankey rejects undrawable flows with diagnostics. Line/area gaps remain intact during interrupted animation. +- **export:** specialized SVG/raster exports render settled geometry without transient hover effects; maps preserve the current camera and clipping. Font embedding respects inline typography and inherited span fonts. Table CSV includes every filtered/sorted row across pagination. +- **typography:** bar lists and Sankey use shared chrome wrapping and measured font styles, keeping headings inside narrow containers. + ### Changed - **graph!:** `highlight()` now layers over the sticky category filter instead of replacing it. In v8.0 the two shared one slot and the last writer won, so hovering a custom legend row threw away the filter the reader had just clicked. The effective emphasis is now the intersection of filter and highlight, except that a highlight disjoint from the filter previews on its own and a highlight matching no nodes is a no-op layer. Two consequences: `clearHighlight()` releases only the transient highlight and returns to the filtered view (clearing the filter is `setActiveCategories([])`), and `onHighlightChange` reports the effective set, so it can fire with an array where it previously fired `null`. Legend `active` flags now track the filter alone. See [migration guide](docs/migrating-v8.md#24-graph-highlight-layers-over-the-category-filter-81). diff --git a/docs/architecture.md b/docs/architecture.md index b337fca6..b07ea35d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -196,11 +196,17 @@ Visual features are computed per-column based on the column config: heatmap buil ## Rendering strategy -The vanilla adapter renders charts by building a fresh SVG element from the `ChartLayout`. It doesn't diff or patch. On every update or resize, it tears down the old SVG and creates a new one. +The vanilla adapter compiles a candidate layout before committing an update. Rejected chart, map, tile map, bar list, Sankey and table specifications leave the last valid view and its interactions usable. Cartesian charts reconcile compatible SVG scenes by stable mark keys; incompatible scenes and specialized renderers rebuild from their compiled layouts. This keeps the engine independent of DOM lifetime. + +Line and area geometry can morph when point identity and path topology permit it. Paths with missing-observation breaks use an opacity crossfade of the canonical paths, including interrupted updates, so motion never connects absent evidence. Entrance completion releases the resize gate and replays any deferred resize. Scatter charts past a point-count threshold (or mounted with the `renderer: 'canvas'` option) render their dots on a `` instead (see [canvas mark mode](spec-reference.md#canvas-mark-mode)). The canvas is a sibling inserted *before* the SVG and painting the background, gridlines and marks; the SVG keeps axes, trendline, annotations and chrome. The engine only records the decision as `ChartLayout.markRenderMode` -- the renderer keys strictly on its own `canvasMarks` option, so any caller that omits it (SSR, exports) gets a complete SVG regardless of what the layout says. -For tables, it creates HTML elements (table, thead, tbody, etc.) with interactivity wired up: sort headers, search input, pagination controls, keyboard navigation. +For tables, it creates HTML elements (table, thead, tbody, etc.) with interactivity wired up: sort headers, search input, pagination controls, keyboard navigation. Logical row/column focus survives rebuilds; typed row keys prevent collisions between `1` and `"1"`. Duplicate or missing authored keys receive revision-local identifiers and a warning. DOM row IDs are opaque implementation details. + +Maps, tile maps, bar lists and Sankey diagrams share mark keyboard navigation. Arrows inspect marks, Enter/Space invoke existing activation callbacks, Escape dismisses inspection and Tab follows the page's native focus order. Focus restores by mark identity after a rebuild, with a chart-root fallback for removed or ambiguous marks. Tooltip visibility is independent from callback wiring. Tables leave nested links and disclosure controls in charge of their own keys. + +Static chart exports render the terminal layout rather than serializing hover or in-flight geometry. Map exports retain the current camera, authored focus and zoom viewport clipping. Font embedding resolves inline styles before presentation attributes and inherits font declarations for text spans, including detached export scenes. Table CSV exports use all filtered/sorted rows, independently of the displayed page. Both adapters use `ResizeObserver` to detect container size changes and trigger recompilation at the new dimensions. The responsive system uses breakpoints to adjust layout strategy (label density, legend position, annotation placement). @@ -218,9 +224,8 @@ sequenceDiagram React->>Vanilla: chart.update(spec) Vanilla->>Engine: compileChart(spec, options) Engine-->>Vanilla: ChartLayout - Vanilla->>DOM: Remove old SVG - Vanilla->>DOM: Create new SVG from layout - Vanilla->>DOM: Wire tooltips + keyboard nav + Vanilla->>DOM: Reconcile compatible scene or replace specialized view + Vanilla->>DOM: Restore valid focus and wire interactions ``` ## Theme resolution @@ -231,7 +236,7 @@ Themes flow through three stages: 2. **Deep merge** (`resolveTheme()`): Merge user config onto `DEFAULT_THEME`. Produces a `ResolvedTheme` with every field set. 3. **Dark mode adaptation** (`adaptTheme()`): If dark mode is active, transform the resolved theme. Swap background/text, adjust palette brightness, lighten gridlines. -The default theme uses Inter as the primary font, editorial typography hierarchy (22px title, 15px subtitle, 13px body, 11px axis ticks), and a categorical palette tuned for distinguishability. +The default theme uses Inter as the primary font and a categorical palette tuned for distinguishability. Theme tokens remain the source of truth for typography; the opt-in `clarity` preset provides the newer editorial hierarchy without changing v8 defaults. Themes can be applied per-component (via prop) or globally (via `VizThemeProvider` in React, or the `theme` option in vanilla). diff --git a/docs/codebase-map.md b/docs/codebase-map.md index 9f64226c..9ae3f511 100644 --- a/docs/codebase-map.md +++ b/docs/codebase-map.md @@ -91,6 +91,10 @@ | SVG DOM helpers (`createSVGElement`, `setAttrs`, `applyTextStyle`) | `packages/vanilla/src/renderers/svg-dom.ts` (note: `computeXAxisExtent` was removed; use `layout.chrome.bottomAnchorY` instead) | | Gradient utilities (`LinearGradient` resolution) | `packages/vanilla/src/gradient-utils.ts` | | Resize observer wiring | `packages/vanilla/src/resize-observer.ts` | +| Specialized mark keyboard inspection and focus restoration | `packages/vanilla/src/interactions/mark-navigation.ts` → `wireMarkNavigation`, `captureMarkFocus`, `restoreMarkFocus`; used by map/tilemap/barlist/Sankey mounts. | +| Numeric observation validity | `packages/engine/src/data/finite-number.ts` → `finiteNumber`; shared by measured mark channels, scales, geographic coordinates, aggregates, bins, density and window comparisons. | +| Library lifecycle/interaction QA | `packages/vanilla/src/__tests__/lifecycle-contracts.test.ts`; manual Ladle fixture `examples/src/testing/library-contracts.stories.tsx`; browser checks `e2e/invariants/library-interactions.spec.ts`. | +| Zoom viewport clipping for live maps and exports | `packages/vanilla/src/map-camera.ts` → `ensureMapClip`; reused by `map-zoom.ts` and terminal map export. | | Animation lifecycle / cleanup | `packages/vanilla/src/animation.ts`. `computeAnimationDuration(svg)` is the shared total-entrance-time calc (used by cleanup timing and GIF capture). | | Data-update transition driver (rAF-based mark/axis tweening) | `packages/vanilla/src/transition/` (driver.ts, gate.ts, svg-tweens.ts, canvas-tweens.ts, chrome-tweens.ts, ...; `transition.ts` is the re-export barrel) | | Export (SVG/PNG/JPG/CSV) | `packages/vanilla/src/export.ts` → `exportSVG`, `exportPNG`, `exportJPG`, `exportCSV`, plus `rasterizeSVGToCanvas`/`getSVGDimensions`/`embedFonts` helpers reused by GIF export. Wired into `chart.export(format)` in `mount.ts`. | diff --git a/docs/spec-reference.md b/docs/spec-reference.md index c135ed86..9e3a7ee6 100644 --- a/docs/spec-reference.md +++ b/docs/spec-reference.md @@ -1892,7 +1892,15 @@ The reference [Studio workbench](workbench.md) consumes these additive interface - **Motion:** `animation: 'fluid' | 'subtle' | 'legacy'`; explicit phases override their preset. A phase can use `spring: { stiffness: 320, damping: 36, mass: 1 }` or the existing duration/easing configuration. Combining a spring with duration/easing in the same phase is an error. Quantitative springs are bounded, with a finite terminal state. Omitted/false animation remains disabled. - **Responsiveness:** `responsive: { preserveEnd: true, labelSummary: true }` retains the final x-axis boundary and moves overflowing endpoint identities to a wrapped list. Explicit axis values and author choices remain authoritative. `chromeLayout: 'grow'` gives that content space. Tables may opt into `mobileLayout: 'priority'`; `cards` remains the default, with `scroll` also available. - **Field metadata:** chart, table and map documents accept `fields`, keyed by field name. Each entry may declare `type` (`text`, `number`, `date`), `role: 'identity'`, `label`, `unit`, `measure`, `provenance`, `representation` (`number`, `percent`, `proportion`), `interval: { lower: 'lowerField', upper: 'upperField' }`, or `statusField`. Supplied intervals are never synthesized. Percent means 0–100 and proportion means 0–1. -- **Reader identity:** a unique authored `encoding.key` supports observation persistence. Missing/duplicate keys clear observation pins on data replacement. Aggregated observations are groups, not raw rows, and the reference reader clears them on data/grouping revision. The `createReader` controller is independent from editor selection and uses `ChartInstance.sampleLayout()` for supported moving geometry. +- **Reader identity:** a unique authored `encoding.key` supports observation persistence. Missing/duplicate keys clear observation pins on data replacement. Aggregated observations are groups, not raw rows, and the reference reader clears them on data/grouping revision. The `createReader` controller is independent from editor selection and uses `ChartInstance.sampleLayout()` for supported moving geometry. Call `reader.update(spec)` after `chart.update(spec)`; it reconciles visibility against the compiled destination marks even when the source data array is unchanged. An explicit `visibleKeys` override remains supported. Layered charts retain per-layer hover/crosshair inspection, but reader pinning across layers is not yet supported: a `LayerSpec` clears pins with an announcement rather than guessing cross-layer identity. - **Design review:** `diagnoseChart(spec, layout)` returns structured checks with code, severity, checkability, path, viewport, explanation and origin. `CompileOptions.onDiagnostic` receives these checks. `recommendCompositions(data, question, roles)` requires explicit field roles and returns explained alternatives. `applySpecPatch` rejects stale snapshot preconditions; hosts should preview patches and retain undo. The reference editor uses the full authored document for patches even while previewing a filtered scene. Host-neutral identity, comparison and explicit dataset-binding helpers are also exported from the engine. They provide foundations for future linked views, without implicitly declaring compatible measures or raw-row/aggregate relationships. See the [delivery record](design-evolution.md) for the advanced capabilities and release gates still pending. + +### Numeric observations and interaction contracts + +Measured channels accept finite numbers and nonblank numeric strings, including zero. Null, blank text, booleans, `NaN` and infinities are absent measurements, not zeroes. This applies to chart quantities, map coordinates/colors, size/domain extraction, aggregates, bins, density and window differences/percentage changes. Supplied missing line/area observations remain gaps during rendering, updates and exports. Sankey requires positive finite flow values; omitted flows produce a warning and an all-invalid dataset produces an empty finite layout. Existing cumulative-sum, text-size fallback and absent-series stack conventions are unchanged. + +Maps, tile maps, bar lists and Sankey support arrows for inspection and Enter/Space for their existing click callbacks, including when `tooltip: false`. Escape dismisses inspection; Tab exits naturally. A keyboard-focused map mark uses arrows for mark navigation; the map root retains its separate zoom/pan shortcuts. Tables restore keyed cell/header focus across updates and keep native mobile detail disclosures operable. + +Table `rowKey` values must be unique to support identity across revisions. Numeric `1` and string `"1"` are distinct. Missing/duplicate keys emit `TABLE_ROW_KEY_AMBIGUOUS` and use revision-local identities. Treat compiled row IDs as opaque. CSV export includes all filtered and sorted rows, not just the current pagination page. diff --git a/e2e/invariants/library-interactions.spec.ts b/e2e/invariants/library-interactions.spec.ts new file mode 100644 index 00000000..0c1fbae2 --- /dev/null +++ b/e2e/invariants/library-interactions.spec.ts @@ -0,0 +1,113 @@ +import { expect, test } from '@playwright/test'; + +const fixture = '/?story=testing--library-contracts--lifecycle&mode=preview'; +const renderers = [ + ['Bar list', '.oc-barlist-row'], ['Map', '.oc-map-feature'], ['Tile map', '.oc-tilemap-tile'], ['Sankey', '.oc-sankey-node'], +] as const; + +for (const [name, selector] of renderers) { + test(`${name}: keyboard evidence, native Tab exit, resize, rejected edit and settled export`, async ({ page }) => { + const errors: string[] = []; + page.on('pageerror', (error) => errors.push(error.message)); + await page.goto(fixture); + await page.getByRole('combobox', { name: 'Renderer', exact: true }).selectOption(name); + const host = page.getByTestId('library-host'); + const first = host.locator(selector).first(); + await expect(first).toBeVisible(); + await expect(host.locator('svg').first()).not.toHaveClass(/oc-animate/); + await first.focus(); + await page.keyboard.press('ArrowRight'); + const focused = host.locator('[data-navigation-key]:focus'); + await expect(focused).toHaveCount(1); + const key = await focused.getAttribute('data-navigation-key'); + await page.keyboard.press('Enter'); + await expect(page.getByTestId('selected-evidence')).not.toHaveText('No selection'); + if (name !== 'Map' || await focused.getAttribute('data-nodata') !== 'true') { + await expect(host.locator('.oc-tooltip')).toBeVisible(); + } + await page.keyboard.press('Escape'); + await expect(host.locator('.oc-tooltip')).toBeHidden(); + await page.keyboard.press('Tab'); + if (name === 'Map') { + // Zoom controls are legitimate extra stops, but map marks must not trap Tab. + await expect(focused).toHaveCount(0); + } else { + await expect(focused).toHaveCount(0); + // Attribution links remain ordinary page navigation stops. + if (await host.locator('a:focus').count()) await page.keyboard.press('Tab'); + await expect(host.locator('[data-navigation-key]:focus')).toHaveCount(0); + } + await first.focus(); + await page.keyboard.press('ArrowRight'); + await page.setViewportSize({ width: 390, height: 844 }); + await expect.poll(() => host.locator('[data-navigation-key]:focus').getAttribute('data-navigation-key')).toBe(key); + await expect.poll(async () => Number((await host.locator('svg').first().getAttribute('viewBox'))!.split(' ')[2])).toBeLessThan(391); + await page.getByRole('button', { name: 'Reject invalid update', exact: true }).click(); + await expect(page.getByRole('status')).toHaveText('Invalid data rejected; previous view preserved'); + await expect(first).toBeVisible(); + await page.getByRole('button', { name: 'Update', exact: true }).click(); + await expect(host).toContainText(`${name}: revision 1`); + const textFits = await host.evaluate((element) => { + const bounds = element.getBoundingClientRect(); + return [...element.querySelectorAll('.oc-title,.oc-subtitle')].every((text) => { + const box = text.getBoundingClientRect(); + return box.left >= bounds.left - 1 && box.right <= bounds.right + 1; + }); + }); + expect(textFits).toBe(true); + await expect(host.locator('svg').first()).not.toHaveClass(/oc-animate/); + const downloadPromise = page.waitForEvent('download'); + await page.getByRole('button', { name: 'Export', exact: true }).click(); + const download = await downloadPromise; + expect(download.suggestedFilename()).toMatch(/\.svg$/); + expect(await download.failure()).toBeNull(); + expect(errors).toEqual([]); + }); +} + +test('table: Escape wins over pending search, sort retains focus and priority cells stay navigable', async ({ page }) => { + await page.goto(fixture); + await page.getByRole('combobox', { name: 'Renderer', exact: true }).selectOption('Table'); + const host = page.getByTestId('library-host'); + const search = host.locator('input'); + await search.fill('California'); + await search.press('Escape'); + await expect(search).toHaveValue(''); + await expect(host.locator('tbody')).toBeFocused(); + // Wait past the debounce and prove the stale query never returns. + await expect(host.locator('tbody tr')).toHaveCount(10); + await page.waitForTimeout(250); + await expect(host.locator('tbody tr')).toHaveCount(10); + const header = host.locator('th[data-column="state"]'); + await header.focus(); + await page.keyboard.press('Enter'); + await expect(header).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(host.locator('.oc-table-cell-focus')).toHaveText('California'); + await page.keyboard.press('Enter'); + await expect(page.getByTestId('selected-evidence')).toContainText('California'); + await page.setViewportSize({ width: 390, height: 844 }); + await expect(host.locator('.oc-table--priority')).toBeVisible(); + await expect(host.locator('tbody')).toBeFocused(); + await page.keyboard.press('End'); + await expect(host.locator('.oc-table-cell-focus')).toBeVisible(); + await expect(host.locator('.oc-table-cell-focus')).toContainText('More'); + const summary = host.locator('summary').first(); + await summary.focus(); + await page.keyboard.press('Enter'); + await expect(host.locator('details').first()).toHaveAttribute('open', ''); + await page.keyboard.press('Space'); + await expect(host.locator('details').first()).not.toHaveAttribute('open'); +}); + +test('graph: the canvas releases both Tab directions', async ({ page }) => { + await page.goto('/?story=graphs--graphs&mode=preview'); + const canvas = page.locator('.oc-graph-canvas').first(); + await expect(canvas).toBeVisible(); + await canvas.focus(); + await page.keyboard.press('Tab'); + await expect(canvas).not.toBeFocused(); + await canvas.focus(); + await page.keyboard.press('Shift+Tab'); + await expect(canvas).not.toBeFocused(); +}); diff --git a/e2e/visual/__screenshots__/stories.spec.ts-snapshots/sankey-narrow-long-labels-visual-darwin.png b/e2e/visual/__screenshots__/stories.spec.ts-snapshots/sankey-narrow-long-labels-visual-darwin.png index 0502365f..c243f7cc 100644 Binary files a/e2e/visual/__screenshots__/stories.spec.ts-snapshots/sankey-narrow-long-labels-visual-darwin.png and b/e2e/visual/__screenshots__/stories.spec.ts-snapshots/sankey-narrow-long-labels-visual-darwin.png differ diff --git a/examples/src/testing/library-contracts.css b/examples/src/testing/library-contracts.css new file mode 100644 index 00000000..2ac918c6 --- /dev/null +++ b/examples/src/testing/library-contracts.css @@ -0,0 +1,37 @@ +.library-contracts { + padding: 24px; + color: #18181b; + background: #fff; +} +.library-contracts--dark { + color: #fafafa; + background: #09090b; +} +.library-contracts-controls { + display: flex; + flex-wrap: wrap; + gap: 8px; +} +.library-contracts button, +.library-contracts select { + min-height: 44px; + padding: 8px 12px; + font: inherit; +} +.library-contracts-host { + width: 100%; + max-width: 900px; + min-height: 420px; + margin: 16px 0; +} +.library-contracts-host--narrow { + max-width: 390px; +} +.library-contracts pre { + white-space: pre-wrap; + overflow-wrap: anywhere; +} + +.library-contracts-canvas { + min-height: 420px; +} diff --git a/examples/src/testing/library-contracts.stories.tsx b/examples/src/testing/library-contracts.stories.tsx new file mode 100644 index 00000000..fa406ef4 --- /dev/null +++ b/examples/src/testing/library-contracts.stories.tsx @@ -0,0 +1,216 @@ +/** Interactive QA fixture for the existing renderers' lifecycle and input contracts. */ +import type { + BarListSpec, + ChartSpec, + GeoMapSpec, + SankeySpec, + TableSpec, + TileMapSpec, +} from '@opendata-ai/openchart-core'; +import { + createBarList, + createChart, + createGeoMap, + createSankey, + createTable, + createTileMap, +} from '@opendata-ai/openchart-vanilla'; +import { useEffect, useRef, useState } from 'react'; +import usStatesTopo from 'us-atlas/states-albers-10m.json'; +import { energyFlow } from '../data/energy-flow'; +import { stateEconomies } from '../data/state-economies'; +import { usStateUnemployment } from '../data/us-state-unemployment'; +import './library-contracts.css'; + +export default { title: 'Testing / Library Contracts' }; + +type Kind = 'Bar' | 'Bar list' | 'Map' | 'Tile map' | 'Sankey' | 'Table'; +interface Controls { + update(): void; + reject(): void; + resize(): void; + export(): void; + destroy(): void; +} +const kinds: Kind[] = ['Bar', 'Bar list', 'Map', 'Tile map', 'Sankey', 'Table']; + +export function Lifecycle() { + const host = useRef(null); + const controls = useRef(null); + const [kind, setKind] = useState('Tile map'); + const [narrow, setNarrow] = useState(false); + const [status, setStatus] = useState('Ready'); + const [evidence, setEvidence] = useState('No selection'); + const [dark, setDark] = useState(false); + + useEffect(() => { + if (!host.current) return; + const options = { darkMode: dark ? ('force' as const) : ('off' as const) }; + const clicked = (data: unknown) => setEvidence(JSON.stringify(data)); + const chrome = { + title: `${kind}: interaction and resize check`, + subtitle: 'Use Tab, arrows, Enter and Escape. Updates retain the last valid chart.', + }; + const common = { animation: true as const, chrome }; + const economies = stateEconomies.data.map((row) => ({ ...row })); + const bar: ChartSpec = { + ...common, + mark: 'bar', + data: economies, + encoding: { + x: { field: 'gdp', type: 'quantitative' }, + y: { field: 'state', type: 'nominal' }, + key: { field: 'state', type: 'nominal' }, + }, + }; + const barlist: BarListSpec = { + ...common, + type: 'barlist', + data: economies, + encoding: { + label: { field: 'state', type: 'nominal' }, + value: { field: 'gdp', type: 'quantitative' }, + }, + }; + const map: GeoMapSpec = { + ...common, + type: 'map', + geo: { features: usStatesTopo, idField: 'name', zoom: true }, + data: economies, + encoding: { + key: { field: 'state', type: 'nominal' }, + color: { field: 'gdp', type: 'quantitative' }, + }, + }; + const tilemap: TileMapSpec = { + ...common, + type: 'tilemap', + data: { ...usStateUnemployment.data }, + }; + const sankey: SankeySpec = { + ...common, + type: 'sankey', + data: energyFlow.data.map((row) => ({ ...row })), + encoding: { + source: { field: 'source', type: 'nominal' }, + target: { field: 'target', type: 'nominal' }, + value: { field: 'value', type: 'quantitative' }, + }, + }; + const table: TableSpec = { + ...common, + type: 'table', + data: economies, + rowKey: 'state', + search: true, + mobileLayout: 'priority', + columns: [ + { key: 'state', label: 'State', sortable: true, priority: 1 }, + { key: 'gdp', label: 'GDP ($tn)', sortable: true, priority: 1 }, + { key: 'pop', label: 'Population (m)', priority: 2 }, + ], + }; + const spec = { + Bar: bar, + 'Bar list': barlist, + Map: map, + 'Tile map': tilemap, + Sankey: sankey, + Table: table, + }[kind]; + const instance = + kind === 'Bar' + ? createChart(host.current, bar, { ...options, onMarkClick: clicked }) + : kind === 'Bar list' + ? createBarList(host.current, barlist, { ...options, onRowClick: clicked }) + : kind === 'Map' + ? createGeoMap(host.current, map, { ...options, onMarkClick: clicked }) + : kind === 'Tile map' + ? createTileMap(host.current, tilemap, { ...options, onTileClick: clicked }) + : kind === 'Sankey' + ? createSankey(host.current, sankey, { + ...options, + onNodeClick: clicked, + onLinkClick: clicked, + }) + : createTable(host.current, table, { ...options, onRowClick: clicked }); + // The selected factory/spec pair is fixed for this fixture's mounted lifetime. + const update = (next: typeof spec) => instance.update(next as never); + let revision = 0; + controls.current = { + update: () => { + revision += 1; + update({ ...spec, chrome: { ...chrome, title: `${kind}: revision ${revision}` } }); + setStatus(`Updated revision ${revision}`); + }, + reject: () => { + try { + update({ ...spec, data: null } as unknown as typeof spec); + setStatus('Unexpectedly accepted invalid data'); + } catch { + setStatus('Invalid data rejected; previous view preserved'); + } + }, + resize: () => instance.resize(), + export: () => { + const text = + kind === 'Table' + ? (instance as ReturnType).export('csv') + : ((instance as ReturnType).export('svg') as string); + const blob = new Blob([text], { type: kind === 'Table' ? 'text/csv' : 'image/svg+xml' }); + const url = URL.createObjectURL(blob); + const link = document.createElement('a'); + link.href = url; + link.download = `openchart-${kind.toLowerCase().replaceAll(' ', '-')}.${kind === 'Table' ? 'csv' : 'svg'}`; + link.click(); + setTimeout(() => URL.revokeObjectURL(url), 1000); + setStatus('Exported settled view'); + }, + destroy: () => instance.destroy(), + }; + setStatus('Ready'); + setEvidence('No selection'); + return () => { + instance.destroy(); + controls.current = null; + }; + }, [kind, dark]); + + return ( +
+

Library interaction checks

+
+ + + + + + +
+ {status} + +
+
+
+ + 
{evidence}
+
+ ); +} diff --git a/llms-full.txt b/llms-full.txt index 854055a6..f9906548 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -2817,11 +2817,19 @@ The reference [Studio workbench](workbench.md) consumes these additive interface - **Motion:** `animation: 'fluid' | 'subtle' | 'legacy'`; explicit phases override their preset. A phase can use `spring: { stiffness: 320, damping: 36, mass: 1 }` or the existing duration/easing configuration. Combining a spring with duration/easing in the same phase is an error. Quantitative springs are bounded, with a finite terminal state. Omitted/false animation remains disabled. - **Responsiveness:** `responsive: { preserveEnd: true, labelSummary: true }` retains the final x-axis boundary and moves overflowing endpoint identities to a wrapped list. Explicit axis values and author choices remain authoritative. `chromeLayout: 'grow'` gives that content space. Tables may opt into `mobileLayout: 'priority'`; `cards` remains the default, with `scroll` also available. - **Field metadata:** chart, table and map documents accept `fields`, keyed by field name. Each entry may declare `type` (`text`, `number`, `date`), `role: 'identity'`, `label`, `unit`, `measure`, `provenance`, `representation` (`number`, `percent`, `proportion`), `interval: { lower: 'lowerField', upper: 'upperField' }`, or `statusField`. Supplied intervals are never synthesized. Percent means 0–100 and proportion means 0–1. -- **Reader identity:** a unique authored `encoding.key` supports observation persistence. Missing/duplicate keys clear observation pins on data replacement. Aggregated observations are groups, not raw rows, and the reference reader clears them on data/grouping revision. The `createReader` controller is independent from editor selection and uses `ChartInstance.sampleLayout()` for supported moving geometry. +- **Reader identity:** a unique authored `encoding.key` supports observation persistence. Missing/duplicate keys clear observation pins on data replacement. Aggregated observations are groups, not raw rows, and the reference reader clears them on data/grouping revision. The `createReader` controller is independent from editor selection and uses `ChartInstance.sampleLayout()` for supported moving geometry. Call `reader.update(spec)` after `chart.update(spec)`; it reconciles visibility against the compiled destination marks even when the source data array is unchanged. An explicit `visibleKeys` override remains supported. Layered charts retain per-layer hover/crosshair inspection, but reader pinning across layers is not yet supported: a `LayerSpec` clears pins with an announcement rather than guessing cross-layer identity. - **Design review:** `diagnoseChart(spec, layout)` returns structured checks with code, severity, checkability, path, viewport, explanation and origin. `CompileOptions.onDiagnostic` receives these checks. `recommendCompositions(data, question, roles)` requires explicit field roles and returns explained alternatives. `applySpecPatch` rejects stale snapshot preconditions; hosts should preview patches and retain undo. The reference editor uses the full authored document for patches even while previewing a filtered scene. Host-neutral identity, comparison and explicit dataset-binding helpers are also exported from the engine. They provide foundations for future linked views, without implicitly declaring compatible measures or raw-row/aggregate relationships. See the [delivery record](design-evolution.md) for the advanced capabilities and release gates still pending. +### Numeric observations and interaction contracts + +Measured channels accept finite numbers and nonblank numeric strings, including zero. Null, blank text, booleans, `NaN` and infinities are absent measurements, not zeroes. This applies to chart quantities, map coordinates/colors, size/domain extraction, aggregates, bins, density and window differences/percentage changes. Supplied missing line/area observations remain gaps during rendering, updates and exports. Sankey requires positive finite flow values; omitted flows produce a warning and an all-invalid dataset produces an empty finite layout. Existing cumulative-sum, text-size fallback and absent-series stack conventions are unchanged. + +Maps, tile maps, bar lists and Sankey support arrows for inspection and Enter/Space for their existing click callbacks, including when `tooltip: false`. Escape dismisses inspection; Tab exits naturally. A keyboard-focused map mark uses arrows for mark navigation; the map root retains its separate zoom/pan shortcuts. Tables restore keyed cell/header focus across updates and keep native mobile detail disclosures operable. + +Table `rowKey` values must be unique to support identity across revisions. Numeric `1` and string `"1"` are distinct. Missing/duplicate keys emit `TABLE_ROW_KEY_AMBIGUOUS` and use revision-local identities. Treat compiled row IDs as opaque. CSV export includes all filtered and sorted rows, not just the current pagination page. + --- diff --git a/packages/engine/src/__tests__/layer-identity.test.ts b/packages/engine/src/__tests__/layer-identity.test.ts new file mode 100644 index 00000000..060676cc --- /dev/null +++ b/packages/engine/src/__tests__/layer-identity.test.ts @@ -0,0 +1,106 @@ +import type { ChartSpec, LayerSpec } from '@opendata-ai/openchart-core'; +import { describe, expect, it } from 'vitest'; +import { compileLayer } from '../compile'; + +const dimensions = { width: 640, height: 420 }; + +function pointLayer(name: string, value: number, format: string, zIndex: number): ChartSpec { + return { + mark: 'point', + data: [{ id: 'same-key', name, x: 1, value }], + zIndex, + encoding: { + key: { field: 'id', type: 'nominal' }, + x: { field: 'x', type: 'quantitative' }, + y: { field: 'value', type: 'quantitative' }, + tooltip: [{ field: 'value', type: 'quantitative', title: name, format }], + }, + }; +} + +describe('layered observation identity', () => { + it.each([ + 'shared', + 'independent', + ] as const)('keeps each leaf tooltip and format attached to the painted mark with %s scales', (scale) => { + const layout = compileLayer( + { + layer: [pointLayer('Revenue', 1250, '$,.0f', 1), pointLayer('Share', 0.25, '.0%', 0)], + resolve: { scale: { y: scale } }, + }, + dimensions, + ); + const rows = layout.marks.map((mark, index) => ({ + name: 'data' in mark && !Array.isArray(mark.data) ? mark.data.name : undefined, + content: layout.tooltipDescriptors.get(`${mark.type}-${index}`), + })); + expect(rows).toEqual([ + { + name: 'Share', + content: expect.objectContaining({ fields: [{ label: 'Share', value: '25%' }] }), + }, + { + name: 'Revenue', + content: expect.objectContaining({ fields: [{ label: 'Revenue', value: '$1,250' }] }), + }, + ]); + expect(layout.tooltipDescriptors.size).toBe(2); + }); + + it('keeps leaf keys distinct and stable across nested-layer paint order and viewport changes', () => { + const first = pointLayer('First', 10, '.0f', 1); + const second = pointLayer('Second', 20, '.0f', 0); + const spec: LayerSpec = { layer: [{ layer: [first] }, second] }; + const before = compileLayer(spec, dimensions); + const after = compileLayer( + { ...spec, layer: [{ layer: [{ ...first, zIndex: -1 }] }, second] }, + { width: 320, height: 420 }, + ); + const keysByName = (layout: typeof before) => + new Map( + layout.marks.map((mark) => [ + 'data' in mark && !Array.isArray(mark.data) ? mark.data.name : undefined, + mark.key, + ]), + ); + expect(new Set(before.marks.map((mark) => mark.key)).size).toBe(2); + expect(keysByName(after)).toEqual(keysByName(before)); + expect(after.tooltipDescriptors.get('point-0')?.fields[0].label).toBe('First'); + expect(after.tooltipDescriptors.get('point-1')?.fields[0].label).toBe('Second'); + const single = compileLayer({ layer: [{ layer: [first] }] }, dimensions); + expect(single.marks[0].key).toBe(keysByName(before).get('First')); + }); + + it.each([ + 'line', + 'area', + ] as const)('preserves distinct %s leaf identities and point formats when series names repeat', (mark) => { + const leaf = (name: string, value: number, format: string): ChartSpec => ({ + mark, + data: [ + { id: 'a', name, series: 'Shared', x: 1, value }, + { id: 'b', name, series: 'Shared', x: 2, value: value * 2 }, + ], + encoding: { + key: { field: 'id', type: 'nominal' }, + x: { field: 'x', type: 'ordinal' }, + y: { field: 'value', type: 'quantitative', stack: null }, + color: { field: 'series', type: 'nominal' }, + tooltip: [{ field: 'value', type: 'quantitative', title: name, format }], + }, + }); + const layout = compileLayer( + { layer: [leaf('Money', 10, '$.0f'), leaf('Rate', 0.1, '.0%')] }, + dimensions, + ); + expect(new Set(layout.marks.map((item) => item.key)).size).toBe(layout.marks.length); + const series = layout.marks.filter((item) => item.type === mark); + expect(series).toHaveLength(2); + expect(series.map((item) => item.seriesKey)).toEqual(['Shared', 'Shared']); + expect(series.map((item) => item.dataPoints?.[0].tooltip?.fields)).toEqual([ + [{ label: 'Money', value: '$10' }], + [{ label: 'Rate', value: '10%' }], + ]); + expect(series[0].pointKeys?.[0]).not.toBe(series[1].pointKeys?.[0]); + }); +}); diff --git a/packages/engine/src/__tests__/numeric-observations.test.ts b/packages/engine/src/__tests__/numeric-observations.test.ts new file mode 100644 index 00000000..e63c8959 --- /dev/null +++ b/packages/engine/src/__tests__/numeric-observations.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest'; +import { compileBarList } from '../barlist/compile-barlist'; +import { compileChart } from '../compile'; + +const options = { width: 640, height: 400, onWarn: () => {} }; +const missingValues = [null, undefined, '', ' ', false, true, NaN, Infinity, -Infinity, 'bad']; +const rows = [ + { label: 'zero', value: 0, end: 10 }, + { label: 'measured', value: ' 20 ', end: 30 }, + ...missingValues.map((value, index) => ({ label: `missing-${index}`, value, end: 10 })), +]; + +describe('measured numeric data across chart families', () => { + it.each([ + 'circle', + 'arc', + 'range', + 'beeswarm', + 'waffle', + 'parliament', + ])('%s does not fabricate a zero observation from missing data', (mark) => { + const encoding = ['arc', 'waffle', 'parliament'].includes(mark) + ? { + theta: { field: 'value', type: 'quantitative' }, + color: { field: 'label', type: 'nominal' }, + } + : { + x: { field: 'value', type: 'quantitative' }, + y: { field: 'label', type: 'nominal' }, + ...(mark === 'range' ? { x2: { field: 'end' } } : {}), + }; + const layout = compileChart({ mark, data: rows, encoding }, options); + const dataMarks = layout.marks.filter( + (item) => !item.aria.decorative && !Array.isArray(item.data), + ); + expect(dataMarks.length).toBeGreaterThan(0); + expect(new Set(dataMarks.map((item) => (item.data as Record).label))).toEqual( + new Set(['arc', 'waffle', 'parliament'].includes(mark) ? ['measured'] : ['zero', 'measured']), + ); + expect(JSON.stringify(dataMarks)).not.toMatch(/NaN|Infinity/); + }); + + it('requires both measured interval endpoints', () => { + const layout = compileChart( + { + mark: 'range', + data: rows.map((row) => ({ ...row, value: row.end, end: row.value })), + encoding: { + x: { field: 'value', type: 'quantitative' }, + x2: { field: 'end' }, + y: { field: 'label', type: 'nominal' }, + }, + }, + options, + ); + expect( + new Set(layout.marks.map((mark) => (mark.data as Record).label)), + ).toEqual(new Set(['zero', 'measured'])); + }); + + it('keeps calendar holes empty while showing measured zero days', () => { + const layout = compileChart( + { + mark: 'calendar', + data: rows.map((row, index) => ({ + ...row, + date: `2026-01-${String(index + 1).padStart(2, '0')}`, + })), + encoding: { + x: { field: 'date', type: 'temporal' }, + color: { field: 'value', type: 'quantitative' }, + }, + }, + options, + ); + const cells = layout.marks.filter((mark) => mark.type === 'rect' && !mark.aria.decorative); + expect(cells).toHaveLength(2); + expect(cells.map((mark) => (mark.data as Record).value)).toEqual([0, ' 20 ']); + }); + + it('keeps barlist blanks out of ranking and proportions', () => { + const layout = compileBarList( + { + type: 'barlist', + data: rows, + encoding: { + label: { field: 'label', type: 'nominal' }, + value: { field: 'value', type: 'quantitative' }, + }, + }, + options, + ); + expect(layout.rows.map((row) => row.label.text)).toEqual(['measured', 'zero']); + expect(JSON.stringify(layout.rows)).not.toMatch(/NaN|Infinity/); + }); + + it('counts only measured observations in histogram bins', () => { + const layout = compileChart( + { + mark: 'bar', + data: rows, + encoding: { + x: { field: 'value', type: 'quantitative', bin: true }, + y: { aggregate: 'count' }, + }, + }, + options, + ); + const bars = layout.marks.filter((mark) => mark.type === 'rect'); + expect(bars).toHaveLength(2); + expect(bars.map((bar) => bar.aria.label)).toEqual( + expect.arrayContaining([expect.stringMatching(/: 1$/), expect.stringMatching(/: 1$/)]), + ); + }); + + it('requires measured bounds and counts for authored histogram bins', () => { + const data = [ + { start: 0, end: 10, count: 2 }, + ...missingValues.flatMap((value) => [ + { start: value, end: 10, count: 2 }, + { start: 0, end: value, count: 2 }, + { start: 0, end: 10, count: value }, + ]), + ]; + const layout = compileChart( + { + mark: 'bar', + data, + encoding: { + x: { field: 'start', type: 'quantitative' }, + x2: { field: 'end' }, + y: { field: 'count', type: 'quantitative' }, + }, + }, + options, + ); + const bars = layout.marks.filter((mark) => mark.type === 'rect'); + expect(bars).toHaveLength(1); + expect(bars[0].data).toEqual({ start: 0, end: 10, count: 2 }); + }); +}); + +it('leaves heatmap cells empty when their quantitative color is unmeasured', () => { + const layout = compileChart( + { + mark: 'rect', + data: rows, + encoding: { + x: { field: 'label', type: 'nominal' }, + y: { field: 'end', type: 'nominal' }, + color: { field: 'value', type: 'quantitative' }, + }, + }, + options, + ); + expect(layout.marks.map((mark) => (mark.data as Record).value)).toEqual([ + 0, + ' 20 ', + ]); +}); diff --git a/packages/engine/src/barlist/compile-barlist.ts b/packages/engine/src/barlist/compile-barlist.ts index 194b3e69..9c36e887 100644 --- a/packages/engine/src/barlist/compile-barlist.ts +++ b/packages/engine/src/barlist/compile-barlist.ts @@ -31,6 +31,7 @@ import { import { emitSpecWarnings, expandSpecSugar } from '../compile/spec-sugar'; import { resolveAnimation } from '../compiler/animation'; import { compile as compileSpec } from '../compiler/index'; +import { finiteNumber } from '../data/finite-number'; import { resolveFieldFormatter } from '../format/field-format'; import { resolveChromeLayout } from '../layout/shared'; import type { NormalizedBarListSpec } from './types'; @@ -151,10 +152,7 @@ export function compileBarList(spec: unknown, options: CompileOptions): BarListL // Filter valid rows, sort descending, cap to the lesser of maxItems and available height const validRows = barlistSpec.data - .filter((row) => { - const val = row[valueField]; - return val !== null && val !== undefined && !Number.isNaN(Number(val)); - }) + .filter((row) => finiteNumber(row[valueField]) !== undefined) .sort((a, b) => Number(b[valueField]) - Number(a[valueField])) .slice(0, Math.min(barlistSpec.maxItems, maxFittingRows)); diff --git a/packages/engine/src/charts/bar/__tests__/compute.test.ts b/packages/engine/src/charts/bar/__tests__/compute.test.ts index 62c4e4bb..192a60dc 100644 --- a/packages/engine/src/charts/bar/__tests__/compute.test.ts +++ b/packages/engine/src/charts/bar/__tests__/compute.test.ts @@ -994,3 +994,43 @@ describe('markDef overrides', () => { } }); }); + +describe('bar numeric observations', () => { + it('skips missing and nonfinite lengths while preserving zero and signed numeric strings', () => { + const spec = makeSimpleBarSpec(); + spec.data = [ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'bad', + 0, + ' 20 ', + -10, + ].map((value, index) => ({ category: String(index), value: value })); + const scales = computeScales(spec, chartArea, spec.data); + const marks = computeBarMarks(spec, scales, chartArea, fullStrategy, CONTAINER_WIDTH); + expect(marks).toHaveLength(3); + expect(marks.map((mark) => mark.data.value)).toEqual([0, ' 20 ', -10]); + for (const mark of marks) { + expect([mark.x, mark.y, mark.width, mark.height].every(Number.isFinite)).toBe(true); + } + }); + + it('does not pull a nonzero domain down to missing values', () => { + const spec = makeSimpleBarSpec(); + const channel = spec.encoding.x!; + channel.scale = { zero: false, nice: false }; + spec.data = [20, 30, '', ' ', false, true].map((value, index) => ({ + category: String(index), + value: value, + })); + const scales = computeScales(spec, chartArea, spec.data); + expect(scales.x!.scale.domain()).toEqual([20, 30]); + }); +}); diff --git a/packages/engine/src/charts/bar/compute.ts b/packages/engine/src/charts/bar/compute.ts index 2562012c..f86a82bd 100644 --- a/packages/engine/src/charts/bar/compute.ts +++ b/packages/engine/src/charts/bar/compute.ts @@ -22,6 +22,7 @@ import { isGradientDef, NARROW_VIEWPORT_MAX } from '@opendata-ai/openchart-core' import type { ScaleBand, ScaleLinear } from 'd3-scale'; import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { isConditionalValueDef, resolveConditionalValue } from '../../transforms/conditional'; import { formatLabelValue } from '../_shared/format-label-value'; @@ -123,10 +124,7 @@ export function computeBarMarks( } // Missing observations have no measurable length; never render them as zero bars. - const measuredData = spec.data.filter((row) => { - const value = row[xChannel.field]; - return value != null && value !== '' && Number.isFinite(Number(value)); - }); + const measuredData = spec.data.filter((row) => finiteNumber(row[xChannel.field]) !== undefined); const bandwidth = yScale.bandwidth(); // Baseline = pixel x where the bar's left edge anchors. When the x-domain diff --git a/packages/engine/src/charts/beeswarm/compute.ts b/packages/engine/src/charts/beeswarm/compute.ts index fa83d53c..7b11d10f 100644 --- a/packages/engine/src/charts/beeswarm/compute.ts +++ b/packages/engine/src/charts/beeswarm/compute.ts @@ -24,6 +24,7 @@ import type { ScaleBand, ScaleLinear } from 'd3-scale'; import { buildSizeScale, SIZE_SCALE_DEFAULTS } from '../../compile/size-scale'; import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, getSequentialColor, resolveKnockoutColor } from '../utils'; import { dodgeOffsets } from './dodge'; @@ -108,13 +109,8 @@ export function computeBeeswarmMarks( const dots: ResolvedDot[] = []; for (const row of spec.data) { - const raw = row[valueChannel.field]; - // Drop null/undefined explicitly: the validator tolerates them (it only - // rejects non-numeric or non-finite present values), but Number(null) is 0, - // which would silently place a phantom dot at value 0 instead of skipping. - if (raw == null) continue; - const value = Number(raw); - if (!Number.isFinite(value)) continue; + const value = finiteNumber(row[valueChannel.field]); + if (value === undefined) continue; const position = positionScale(value); if (!Number.isFinite(position)) continue; @@ -129,12 +125,12 @@ export function computeBeeswarmMarks( } let radius = DEFAULT_DOT_RADIUS; - if (sizeScale && sizeField) { - const sizeVal = Number(row[sizeField]); - if (Number.isFinite(sizeVal)) { - radius = sizeScale(sizeVal); - } + if (sizeField) { + const sizeValue = finiteNumber(row[sizeField]); + if (sizeValue === undefined) continue; + if (sizeScale) radius = sizeScale(sizeValue); } + if (!Number.isFinite(radius) || radius < 0) continue; dots.push({ row: row as Record, value, position, radius, lane, laneCenter }); } @@ -155,10 +151,9 @@ export function computeBeeswarmMarks( let color: string | GradientDef; if (isSequentialColor && colorField) { - const colorVal = Number(dot.row[colorField]); - color = Number.isFinite(colorVal) - ? getSequentialColor(scales, colorVal) - : getColor(scales, '__default__'); + const value = finiteNumber(dot.row[colorField]); + color = + value !== undefined ? getSequentialColor(scales, value) : getColor(scales, '__default__'); } else if (colorField) { color = getColor(scales, String(dot.row[colorField] ?? '__default__')); } else { diff --git a/packages/engine/src/charts/binned-bar/compute.ts b/packages/engine/src/charts/binned-bar/compute.ts index 06a942d1..edc41fad 100644 --- a/packages/engine/src/charts/binned-bar/compute.ts +++ b/packages/engine/src/charts/binned-bar/compute.ts @@ -26,9 +26,9 @@ import type { } from '@opendata-ai/openchart-core'; import { formatNumber } from '@opendata-ai/openchart-core'; import type { ScaleLinear } from 'd3-scale'; - import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, groupByField, stackSeamStroke } from '../utils'; @@ -123,9 +123,9 @@ function binGeometry( x2Field: string, xScale: ScaleLinear, ): { x: number; width: number } | undefined { - const start = Number(row[xField]); - const end = Number(row[x2Field]); - if (!Number.isFinite(start) || !Number.isFinite(end)) return undefined; + const start = finiteNumber(row[xField]); + const end = finiteNumber(row[x2Field]); + if (start === undefined || end === undefined) return undefined; const px0 = xScale(start); const px1 = xScale(end); @@ -181,8 +181,8 @@ function overlappingBins( for (const row of rows) { const geom = binGeometry(row, xField, x2Field, xScale); if (!geom) continue; - const value = Number(row[yField]); - if (!Number.isFinite(value)) continue; + const value = finiteNumber(row[yField]); + if (value === undefined) continue; // A count is never negative, but an authored bar + x2 spec can carry a // negative y. Anchor on the baseline in both directions rather than @@ -235,9 +235,9 @@ function stackedBins( for (const row of rows) { const geom = binGeometry(row, xField, x2Field, xScale); if (!geom) continue; - const value = Number(row[yField]); + const value = finiteNumber(row[yField]); // Stacking a negative segment has no coherent meaning for a count. - if (!Number.isFinite(value) || value <= 0) continue; + if (value === undefined || value <= 0) continue; const binKey = serializeKeyValue(row[xField]); const used = offsets.get(binKey) ?? 0; diff --git a/packages/engine/src/charts/calendar/compute.ts b/packages/engine/src/charts/calendar/compute.ts index 80c9024d..aa3b6ce4 100644 --- a/packages/engine/src/charts/calendar/compute.ts +++ b/packages/engine/src/charts/calendar/compute.ts @@ -36,8 +36,8 @@ import type { } from '@opendata-ai/openchart-core'; import { defaultNumberFormatter, formatDate } from '@opendata-ai/openchart-core'; import { type CountableTimeInterval, utcDay, utcMonday, utcSunday } from 'd3-time'; - import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getSequentialColor } from '../utils'; @@ -159,8 +159,8 @@ export function computeCalendarMarks( const day = parseUtcDay(row[xEnc.field]); if (!day) continue; years.add(day.getUTCFullYear()); - const value = Number(row[colorEnc.field]); - if (!Number.isFinite(value)) continue; + const value = finiteNumber(row[colorEnc.field]); + if (value === undefined) continue; dayData.set(isoDayKey(day), { row: row as Record, value }); } if (years.size === 0) return []; diff --git a/packages/engine/src/charts/column/__tests__/compute.test.ts b/packages/engine/src/charts/column/__tests__/compute.test.ts index 847ae581..4e13555e 100644 --- a/packages/engine/src/charts/column/__tests__/compute.test.ts +++ b/packages/engine/src/charts/column/__tests__/compute.test.ts @@ -569,3 +569,43 @@ describe('stack modes', () => { } }); }); + +describe('column numeric observations', () => { + it('skips missing and nonfinite lengths while preserving zero and signed numeric strings', () => { + const spec = makeSimpleColumnSpec(); + spec.data = [ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'bad', + 0, + ' 20 ', + -10, + ].map((value, index) => ({ month: String(index), sales: value })); + const scales = computeScales(spec, chartArea, spec.data); + const marks = computeColumnMarks(spec, scales, chartArea, fullStrategy); + expect(marks).toHaveLength(3); + expect(marks.map((mark) => mark.data.sales)).toEqual([0, ' 20 ', -10]); + for (const mark of marks) { + expect([mark.x, mark.y, mark.width, mark.height].every(Number.isFinite)).toBe(true); + } + }); + + it('does not pull a nonzero domain down to missing values', () => { + const spec = makeSimpleColumnSpec(); + const channel = spec.encoding.y!; + channel.scale = { zero: false, nice: false }; + spec.data = [20, 30, '', ' ', false, true].map((value, index) => ({ + month: String(index), + sales: value, + })); + const scales = computeScales(spec, chartArea, spec.data); + expect(scales.y!.scale.domain()).toEqual([20, 30]); + }); +}); diff --git a/packages/engine/src/charts/column/compute.ts b/packages/engine/src/charts/column/compute.ts index 523c98f5..136ee1b7 100644 --- a/packages/engine/src/charts/column/compute.ts +++ b/packages/engine/src/charts/column/compute.ts @@ -25,6 +25,7 @@ import { isGradientDef } from '@opendata-ai/openchart-core'; import type { ScaleBand, ScaleLinear } from 'd3-scale'; import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { isConditionalValueDef, resolveConditionalValue } from '../../transforms/conditional'; import { formatLabelValue } from '../_shared/format-label-value'; @@ -92,10 +93,7 @@ export function computeColumnMarks( } // Missing observations have no measurable length; never render them as zero bars. - const measuredData = spec.data.filter((row) => { - const value = row[yChannel.field]; - return value != null && value !== '' && Number.isFinite(Number(value)); - }); + const measuredData = spec.data.filter((row) => finiteNumber(row[yChannel.field]) !== undefined); const bandwidth = xScale.bandwidth(); // Baseline = pixel y where the column's bottom edge anchors. When the diff --git a/packages/engine/src/charts/dot/compute.ts b/packages/engine/src/charts/dot/compute.ts index b7dce82a..5ece3b24 100644 --- a/packages/engine/src/charts/dot/compute.ts +++ b/packages/engine/src/charts/dot/compute.ts @@ -20,9 +20,9 @@ import type { ResolvedTheme, } from '@opendata-ai/openchart-core'; import type { ScaleBand, ScaleLinear } from 'd3-scale'; - import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, getSequentialColor, groupByField, resolveKnockoutColor } from '../utils'; @@ -154,8 +154,8 @@ function computeDumbbellMarks( // Collect all x-values for this category to find the range const xValues: number[] = []; for (const row of rows) { - const value = Number(row[valueField] ?? 0); - if (Number.isFinite(value)) xValues.push(value); + const value = finiteNumber(row[valueField]); + if (value !== undefined) xValues.push(value); } if (xValues.length === 0) continue; @@ -186,8 +186,8 @@ function computeDumbbellMarks( // Individual dots for each series value for (const row of rows) { - const value = Number(row[valueField] ?? 0); - if (!Number.isFinite(value)) continue; + const value = finiteNumber(row[valueField]); + if (value === undefined) continue; const cx = xScale(value); const colorCategory = String(row[colorField] ?? ''); @@ -235,8 +235,8 @@ function computeLollipopMarks( for (const row of data) { const category = String(row[categoryField] ?? ''); - const value = Number(row[valueField] ?? 0); - if (!Number.isFinite(value)) continue; + const value = finiteNumber(row[valueField]); + if (value === undefined) continue; const bandY = yScale(category); if (bandY === undefined) continue; diff --git a/packages/engine/src/charts/line/__tests__/compute.test.ts b/packages/engine/src/charts/line/__tests__/compute.test.ts index fdc8e077..82130a50 100644 --- a/packages/engine/src/charts/line/__tests__/compute.test.ts +++ b/packages/engine/src/charts/line/__tests__/compute.test.ts @@ -1187,3 +1187,81 @@ describe('sequential color encoding', () => { } }); }); + +describe('line and area numeric observations', () => { + it.each([ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'bad', + ])('does not turn %s into a measured line or area point', (value) => { + const spec = makeSingleSeriesSpec(); + spec.encoding.x = { field: 'x', type: 'quantitative' }; + spec.data = [ + { x: 0, value: 0 }, + { x: 1, value }, + { x: 2, value: ' 20 ' }, + ]; + const scales = computeScales(spec, chartArea, spec.data); + const marks = computeLineMarks(spec, scales, chartArea, fullStrategy); + const line = marks.find((mark) => mark.type === 'line')!; + expect(line.data).toHaveLength(2); + expect(line.path.match(/M/g)).toHaveLength(2); + const areas = computeAreaMarks( + { ...spec, markType: 'area', markDef: { type: 'area' } }, + scales, + chartArea, + fullStrategy, + ); + expect(areas[0].data).toHaveLength(2); + expect(areas[0].path.match(/M/g)).toHaveLength(2); + expect(areas[0].path).not.toMatch(/NaN|Infinity/); + }); + + it('leaves a shared gap in a stack when an authored boundary observation is missing', () => { + const spec = makeMultiSeriesSpec(); + spec.markType = 'area'; + spec.markDef = { type: 'area' }; + spec.encoding.x = { field: 'x', type: 'quantitative' }; + spec.data = ['US', 'UK'].flatMap((country) => + [0, 1, 2].map((x) => ({ + country, + x, + value: x === 1 && country === 'US' ? ' ' : country === 'US' ? 10 : 20, + })), + ); + const scales = computeScales(spec, chartArea, spec.data); + const marks = computeAreaMarks(spec, scales, chartArea, fullStrategy); + expect(marks).toHaveLength(2); + for (const mark of marks) { + expect(mark.data.map((row) => row.x)).toEqual([0, 2]); + expect(mark.dataPoints?.map((point) => point.datum.x)).toEqual([0, 2]); + expect(mark.pointKeys).toHaveLength(2); + expect(mark.path.match(/M/g)).toHaveLength(2); + expect(mark.path).not.toMatch(/NaN|Infinity/); + } + }); + + it('does not fill an interval down to zero when its authored lower bound is missing', () => { + const spec = makeSingleSeriesSpec(); + spec.markType = 'area'; + spec.markDef = { type: 'area' }; + spec.encoding.x = { field: 'x', type: 'quantitative' }; + spec.encoding.y2 = { field: 'lower', type: 'quantitative' }; + spec.data = [ + { x: 0, value: 20, lower: 10 }, + { x: 1, value: 20, lower: ' ' }, + { x: 2, value: 20, lower: 10 }, + ]; + const scales = computeScales(spec, chartArea, spec.data); + const marks = computeAreaMarks(spec, scales, chartArea, fullStrategy); + expect(marks[0].data.map((row) => row.x)).toEqual([0, 2]); + expect(marks[0].path.match(/M/g)).toHaveLength(2); + }); +}); diff --git a/packages/engine/src/charts/line/area.ts b/packages/engine/src/charts/line/area.ts index 3ac344c9..91a780b7 100644 --- a/packages/engine/src/charts/line/area.ts +++ b/packages/engine/src/charts/line/area.ts @@ -29,9 +29,9 @@ import { stackOffsetSilhouette, stackOrderNone, } from 'd3-shape'; - import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, resolveSeriesStroke, scaleValue, sortByField } from '../utils'; import { LINE_STROKE_WIDTH } from './compute'; @@ -151,6 +151,7 @@ function computeSingleArea( // Compute points, filtering out null values const validPoints: { x: number; yTop: number; yBottom: number; row: DataRow }[] = []; + const segments: (typeof validPoints)[] = [[]]; // Check for y2 channel (band between y and y2) const y2Channel = (encoding as Encoding & { y2?: { field: string; type: string } }).y2; @@ -159,19 +160,17 @@ function computeSingleArea( const xVal = scaleValue(scales.x.scale, scales.x.type, row[xChannel.field]); const yVal = scaleValue(scales.y.scale, scales.y.type, row[yChannel.field]); - if (xVal === null || yVal === null) continue; - - const yBottomVal = - y2Channel && row[y2Channel.field] != null - ? scaleValue(scales.y.scale, scales.y.type, row[y2Channel.field]) - : null; + const yBottomVal = y2Channel + ? scaleValue(scales.y.scale, scales.y.type, row[y2Channel.field]) + : baselineY; + if (xVal === null || yVal === null || yBottomVal === null) { + if (segments[segments.length - 1].length > 0) segments.push([]); + continue; + } - validPoints.push({ - x: xVal, - yTop: yVal, - yBottom: yBottomVal ?? baselineY, - row, - }); + const point = { x: xVal, yTop: yVal, yBottom: yBottomVal, row }; + validPoints.push(point); + segments[segments.length - 1].push(point); } if (validPoints.length === 0) continue; @@ -184,14 +183,14 @@ function computeSingleArea( .y1((d) => d.yTop) .curve(curve); - const pathStr = areaGenerator(validPoints) ?? ''; + const pathStr = segments.map((segment) => areaGenerator(segment) ?? '').join(' '); // Top-line path for stroking only the data line (not the baseline) const topLineGenerator = line<{ x: number; yTop: number }>() .x((d) => d.x) .y((d) => d.yTop) .curve(curve); - const topPathStr = topLineGenerator(validPoints) ?? ''; + const topPathStr = segments.map((segment) => topLineGenerator(segment) ?? '').join(' '); const topPoints = validPoints.map((p) => ({ x: p.x, y: p.yTop })); const bottomPoints = validPoints.map((p) => ({ x: p.x, y: p.yBottom })); @@ -318,6 +317,9 @@ function computeStackedArea( const keys = Array.from(seriesKeys); const xValues = Array.from(xValueSet); + // An explicitly missing observation makes the cumulative boundary unknown. + // Keep a shared gap at that x rather than inventing a zero-width segment. + const missingBoundaries = new Set(); // Build a pivot table: one row per x value, one column per series const pivotData: Record[] = xValues.map((xVal) => { const pivot: Record = { __x__: xVal }; @@ -329,7 +331,9 @@ function computeStackedArea( if (xRows) { for (const row of xRows) { const series = String(row[colorField]); - pivot[series] = row[yChannel.field] ?? 0; + const value = finiteNumber(row[yChannel.field]); + if (value === undefined) missingBoundaries.add(xVal); + pivot[series] = value ?? 0; } } return pivot; @@ -358,17 +362,35 @@ function computeStackedArea( const seriesKey = layer.key; const color = getColor(scales, seriesKey); - const validPoints: { x: number; yTop: number; yBottom: number }[] = []; + const validPoints: { + x: number; + yTop: number; + yBottom: number; + xKey: string; + datum: DataRow; + }[] = []; + const segments: (typeof validPoints)[] = [[]]; for (const d of layer) { const xVal = scaleValue(scales.x.scale, scales.x.type, d.data.__x__); - if (xVal === null) continue; - + const xKey = String(d.data.__x__); const yTop = yScale(d[1] as number); const yBottom = yScale(d[0] as number); + if ( + xVal === null || + missingBoundaries.has(xKey) || + !Number.isFinite(yTop) || + !Number.isFinite(yBottom) + ) { + if (segments[segments.length - 1].length > 0) segments.push([]); + continue; + } - validPoints.push({ x: xVal, yTop, yBottom }); + const datum = rowsByXSeries.get(`${xKey}::${seriesKey}`) ?? d.data; + const point = { x: xVal, yTop, yBottom, xKey, datum }; + validPoints.push(point); + segments[segments.length - 1].push(point); } if (validPoints.length === 0) continue; @@ -380,13 +402,13 @@ function computeStackedArea( .y1((p) => p.yTop) .curve(stackCurve); - const pathStr = areaGenerator(validPoints) ?? ''; + const pathStr = segments.map((segment) => areaGenerator(segment) ?? '').join(' '); const topLineGenerator = line<{ x: number; yTop: number }>() .x((p) => p.x) .y((p) => p.yTop) .curve(stackCurve); - const topPathStr = topLineGenerator(validPoints) ?? ''; + const topPathStr = segments.map((segment) => topLineGenerator(segment) ?? '').join(' '); const topPoints = validPoints.map((p) => ({ x: p.x, y: p.yTop })); const bottomPoints = validPoints.map((p) => ({ x: p.x, y: p.yBottom })); @@ -413,7 +435,7 @@ function computeStackedArea( fillOpacity = spec.markDef.fillOpacity ?? STACKED_FILL_OPACITY; } - const rawPointKeys = validPoints.map((_p, idx) => serializeKeyValue(layer[idx]?.data.__x__)); + const rawPointKeys = validPoints.map((point) => serializeKeyValue(point.xKey)); marks.push({ type: 'area', @@ -431,17 +453,8 @@ function computeStackedArea( seriesKey, stacked: true, stackNormalized: stackProp === 'normalize', - data: layer.map((d) => { - const xStr = String(d.data.__x__); - return (rowsByXSeries.get(`${xStr}::${seriesKey}`) ?? d.data) as Record; - }), - dataPoints: validPoints.map((p, idx) => { - const xStr = String(layer[idx]?.data.__x__); - const datum = (rowsByXSeries.get(`${xStr}::${seriesKey}`) ?? - layer[idx]?.data ?? - {}) as Record; - return { x: p.x, y: p.yTop, datum }; - }), + data: validPoints.map((point) => point.datum), + dataPoints: validPoints.map((point) => ({ x: point.x, y: point.yTop, datum: point.datum })), aria, }); } diff --git a/packages/engine/src/charts/line/compute.ts b/packages/engine/src/charts/line/compute.ts index 31698ef9..8b39c600 100644 --- a/packages/engine/src/charts/line/compute.ts +++ b/packages/engine/src/charts/line/compute.ts @@ -20,9 +20,9 @@ import type { } from '@opendata-ai/openchart-core'; import { getRepresentativeColor } from '@opendata-ai/openchart-core'; import { line } from 'd3-shape'; - import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, @@ -298,8 +298,8 @@ export function computeLineMarks( // Sequential color: each point gets colored by its data value let pointColor = color; if (isSequentialColor) { - const val = Number(p.row[sequentialColorField!]); - pointColor = Number.isFinite(val) ? getSequentialColor(scales, val) : color; + const value = finiteNumber(p.row[sequentialColorField!]); + pointColor = value !== undefined ? getSequentialColor(scales, value) : color; } const hollow = isEndpoints && visible; const pointColorStr = getRepresentativeColor(pointColor); @@ -375,7 +375,10 @@ export function computeLineMarks( /** Get the midpoint numeric value of a field across rows (for sequential line stroke). */ function _getMidValue(rows: DataRow[], field: string): number { - const values = rows.map((r) => Number(r[field])).filter(Number.isFinite); + const values = rows.flatMap((row) => { + const value = finiteNumber(row[field]); + return value === undefined ? [] : [value]; + }); if (values.length === 0) return 0; const sorted = values.sort((a, b) => a - b); return sorted[Math.floor(sorted.length / 2)]; diff --git a/packages/engine/src/charts/parliament/compute.ts b/packages/engine/src/charts/parliament/compute.ts index 6ce3e622..ba6c0850 100644 --- a/packages/engine/src/charts/parliament/compute.ts +++ b/packages/engine/src/charts/parliament/compute.ts @@ -33,9 +33,9 @@ import type { TextMarkLayout, } from '@opendata-ai/openchart-core'; import { formatPercent } from '@opendata-ai/openchart-core'; - import { serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, stackSeamStroke } from '../utils'; @@ -195,8 +195,8 @@ export function computeParliamentMarks( const partyRows = new Map(); for (const row of spec.data) { const party = String(row[colorField] ?? ''); - const val = Number(row[valueChannel.field] ?? 0); - if (!Number.isFinite(val) || val < 0) continue; + const val = finiteNumber(row[valueChannel.field]); + if (val === undefined || val < 0) continue; partyTotals.set(party, (partyTotals.get(party) ?? 0) + Math.round(val)); if (!partyRows.has(party)) partyRows.set(party, row); } diff --git a/packages/engine/src/charts/pie/compute.ts b/packages/engine/src/charts/pie/compute.ts index 89c16ae4..b7f0b4e9 100644 --- a/packages/engine/src/charts/pie/compute.ts +++ b/packages/engine/src/charts/pie/compute.ts @@ -25,9 +25,9 @@ import { } from '@opendata-ai/openchart-core'; import type { PieArcDatum } from 'd3-shape'; import { arc as d3Arc, pie as d3Pie } from 'd3-shape'; - import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { resolveConditionalValue } from '../../transforms/conditional'; import { resolveKnockoutColor } from '../utils'; @@ -210,8 +210,8 @@ export function computePieMarks( for (const row of spec.data) { const cat = String(row[categoryField] ?? ''); - const val = Number(row[valueChannel.field] ?? 0); - if (!Number.isFinite(val) || val < 0) continue; + const val = finiteNumber(row[valueChannel.field]); + if (val === undefined || val < 0) continue; categoryTotals.set(cat, (categoryTotals.get(cat) ?? 0) + val); if (!categoryRows.has(cat)) { @@ -233,8 +233,8 @@ export function computePieMarks( // Each data row is a slice. Use a label field if present, or index. for (let i = 0; i < spec.data.length; i++) { const row = spec.data[i]; - const val = Number(row[valueChannel.field] ?? 0); - if (!Number.isFinite(val) || val < 0) continue; + const val = finiteNumber(row[valueChannel.field]); + if (val === undefined || val < 0) continue; // Try common label fields const label = String(row.label ?? row.name ?? row.category ?? `Slice ${i + 1}`); diff --git a/packages/engine/src/charts/range/compute.ts b/packages/engine/src/charts/range/compute.ts index 104e0e1d..2a2288d5 100644 --- a/packages/engine/src/charts/range/compute.ts +++ b/packages/engine/src/charts/range/compute.ts @@ -27,10 +27,10 @@ import type { } from '@opendata-ai/openchart-core'; import { getRepresentativeColor } from '@opendata-ai/openchart-core'; import type { ScaleBand, ScaleLinear } from 'd3-scale'; - import { computeArrowheadPoints } from '../../annotations/geometry'; import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, stackSeamStroke } from '../utils'; @@ -131,9 +131,9 @@ export function computeRangeMarks( const startRaw = row[startChannel.field]; const endRaw = row[endChannel.field]; if (startRaw == null || endRaw == null) continue; - const startVal = Number(startRaw); - const endVal = Number(endRaw); - if (!Number.isFinite(startVal) || !Number.isFinite(endVal)) continue; + const startVal = finiteNumber(startRaw); + const endVal = finiteNumber(endRaw); + if (startVal === undefined || endVal === undefined) continue; const center = bandPos + bandwidth / 2; const pStart = valueScale(startVal); diff --git a/packages/engine/src/charts/rect/index.ts b/packages/engine/src/charts/rect/index.ts index fae17cbb..d68dcf18 100644 --- a/packages/engine/src/charts/rect/index.ts +++ b/packages/engine/src/charts/rect/index.ts @@ -18,9 +18,9 @@ import type { Encoding, Mark, MarkAria, Rect, RectMark } from '@opendata-ai/openchart-core'; import { formatNumber } from '@opendata-ai/openchart-core'; import type { ScaleBand } from 'd3-scale'; - import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import type { ChartRenderer } from '../registry'; import { getColor, getSequentialColor, scaleValue } from '../utils'; @@ -75,8 +75,8 @@ export function computeRectMarks( // case); a nominal one picks a categorical swatch. let fill: RectMark['fill']; if (colorEnc && isSequential) { - const value = Number(row[colorEnc.field]); - if (!Number.isFinite(value)) continue; + const value = finiteNumber(row[colorEnc.field]); + if (value === undefined) continue; fill = getSequentialColor(scales, value); } else if (colorEnc) { fill = getColor(scales, String(row[colorEnc.field] ?? '__default__')); diff --git a/packages/engine/src/charts/scatter/__tests__/compute.test.ts b/packages/engine/src/charts/scatter/__tests__/compute.test.ts index 4ced3f6d..c1f0f9ae 100644 --- a/packages/engine/src/charts/scatter/__tests__/compute.test.ts +++ b/packages/engine/src/charts/scatter/__tests__/compute.test.ts @@ -665,3 +665,23 @@ describe('trendline opt-out', () => { expect((line as { strokeWidth: number }).strokeWidth).toBe(3); }); }); + +describe('scatter numeric observations', () => { + it('requires measured x, y and encoded size without inventing zero positions or bubbles', () => { + const spec = makeBasicScatterSpec(); + spec.encoding.size = { field: 'size', type: 'quantitative' }; + const invalid = [null, undefined, '', ' ', false, true, NaN, Infinity, -Infinity, 'bad']; + spec.data = [ + ...invalid.map((x) => ({ x, y: 10, size: 10 })), + ...invalid.map((y) => ({ x: 10, y, size: 10 })), + ...invalid.map((size) => ({ x: 10, y: 10, size })), + { x: 0, y: 0, size: 0 }, + { x: ' 20 ', y: ' 20 ', size: ' 20 ' }, + ]; + const scales = computeScales(spec, chartArea, spec.data); + const marks = computeScatterMarks(spec, scales, chartArea, fullStrategy); + expect(marks).toHaveLength(2); + expect(marks.map((mark) => mark.data.x)).toEqual([0, ' 20 ']); + for (const mark of marks) expect([mark.cx, mark.cy, mark.r].every(Number.isFinite)).toBe(true); + }); +}); diff --git a/packages/engine/src/charts/scatter/compute.ts b/packages/engine/src/charts/scatter/compute.ts index 1fec2ac9..c52421ba 100644 --- a/packages/engine/src/charts/scatter/compute.ts +++ b/packages/engine/src/charts/scatter/compute.ts @@ -21,6 +21,7 @@ import type { ScaleBand, ScaleLinear, ScalePoint, ScaleTime } from 'd3-scale'; import { buildSizeScale, SIZE_SCALE_DEFAULTS } from '../../compile/size-scale'; import { dedupeKeys, serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor, getSequentialColor, resolveKnockoutColor } from '../utils'; @@ -51,6 +52,7 @@ function resolvePosition( | ScaleBand | ScalePoint, ): number | undefined { + if (value == null) return undefined; switch (channelType) { case 'nominal': case 'ordinal': { @@ -69,9 +71,10 @@ function resolvePosition( return Number.isNaN(px) ? undefined : px; } default: { - const num = Number(value); - if (!Number.isFinite(num)) return undefined; - return (scale as ScaleLinear)(num); + const number = finiteNumber(value); + if (number === undefined) return undefined; + const position = (scale as ScaleLinear)(number); + return Number.isFinite(position) ? position : undefined; } } } @@ -168,10 +171,9 @@ export function computeScatterMarks( const category = colorField && !isSequentialColor ? String(row[colorField] ?? '') : undefined; let color: string | GradientDef; if (isSequentialColor && colorField) { - const val = Number(row[colorField]); - color = Number.isFinite(val) - ? getSequentialColor(scales, val) - : getColor(scales, '__default__'); + const value = finiteNumber(row[colorField]); + color = + value !== undefined ? getSequentialColor(scales, value) : getColor(scales, '__default__'); } else if (colorField) { color = getColor(scales, category ?? '__default__'); } else { @@ -180,12 +182,12 @@ export function computeScatterMarks( } let radius = constSize != null ? constSize : DEFAULT_POINT_RADIUS; - if (sizeScale && sizeField) { - const sizeVal = Number(row[sizeField]); - if (Number.isFinite(sizeVal)) { - radius = sizeScale(sizeVal); - } + if (sizeField) { + const sizeValue = finiteNumber(row[sizeField]); + if (sizeValue === undefined) continue; + if (sizeScale) radius = sizeScale(sizeValue); } + if (!Number.isFinite(radius) || radius < 0) continue; const labelParts = [`${xChannel.field}=${rawX}`, `${yChannel.field}=${rawY}`]; if (category) labelParts.push(`${colorField}=${category}`); diff --git a/packages/engine/src/charts/utils.ts b/packages/engine/src/charts/utils.ts index d6ff8d36..309cacb9 100644 --- a/packages/engine/src/charts/utils.ts +++ b/packages/engine/src/charts/utils.ts @@ -15,6 +15,7 @@ import { type ResolvedTheme, } from '@opendata-ai/openchart-core'; import type { ScaleBand, ScaleLinear, ScalePoint, ScaleTime } from 'd3-scale'; +import { finiteNumber } from '../data/finite-number'; import type { D3Scale, ResolvedScales } from '../layout/scales'; // --------------------------------------------------------------------------- @@ -172,9 +173,10 @@ export function scaleValue(scale: D3Scale, scaleType: string, value: unknown): n return result ?? null; } - const num = typeof value === 'number' ? value : Number(value); - if (!Number.isFinite(num)) return null; - return (scale as ScaleLinear)(num); + const number = finiteNumber(value); + if (number === undefined) return null; + const position = (scale as ScaleLinear)(number); + return Number.isFinite(position) ? position : null; } // --------------------------------------------------------------------------- diff --git a/packages/engine/src/charts/waffle/compute.ts b/packages/engine/src/charts/waffle/compute.ts index 6b138268..465e6af2 100644 --- a/packages/engine/src/charts/waffle/compute.ts +++ b/packages/engine/src/charts/waffle/compute.ts @@ -29,9 +29,9 @@ import type { RectMark, } from '@opendata-ai/openchart-core'; import { formatPercent } from '@opendata-ai/openchart-core'; - import { serializeKeyValue } from '../../compiler/keys'; import type { NormalizedChartSpec } from '../../compiler/types'; +import { finiteNumber } from '../../data/finite-number'; import type { ResolvedScales } from '../../layout/scales'; import { getColor } from '../utils'; @@ -118,8 +118,8 @@ export function computeWaffleMarks( const categoryRows = new Map(); for (const row of spec.data) { const cat = String(row[colorField] ?? ''); - const val = Number(row[valueChannel.field] ?? 0); - if (!Number.isFinite(val) || val < 0) continue; + const val = finiteNumber(row[valueChannel.field]); + if (val === undefined || val < 0) continue; categoryTotals.set(cat, (categoryTotals.get(cat) ?? 0) + val); if (!categoryRows.has(cat)) { diff --git a/packages/engine/src/compile/__tests__/size-scale.test.ts b/packages/engine/src/compile/__tests__/size-scale.test.ts index a2ad8c1b..8fb70943 100644 --- a/packages/engine/src/compile/__tests__/size-scale.test.ts +++ b/packages/engine/src/compile/__tests__/size-scale.test.ts @@ -106,3 +106,22 @@ describe('buildSizeScale', () => { expect(resolved?.domain).toEqual([0, 100]); }); }); + +it('fits magnitude only to measurements, excluding whitespace and booleans', () => { + const data = [ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'bad', + ' 20 ', + 30, + ].map((v) => ({ v })); + expect(buildSizeScale(size(), data, SQRT)?.domain).toEqual([20, 30]); + expect(buildSizeScale(size(), [...data, { v: 0 }], SQRT)?.domain).toEqual([0, 30]); +}); diff --git a/packages/engine/src/compile/layer.ts b/packages/engine/src/compile/layer.ts index b5832c29..d4ba9abe 100644 --- a/packages/engine/src/compile/layer.ts +++ b/packages/engine/src/compile/layer.ts @@ -26,6 +26,33 @@ import { createMeasureFn } from '../layout/plan'; type ChartCompiler = (spec: unknown, options: CompileOptions) => ChartLayout; +/** + * Paint order controls interaction IDs; authored leaf order controls identity. + * Copy each leaf's descriptors instead of rebuilding them with the primary + * encoding, which would replace its formats and transformed observations. + */ +function appendLayerMarks( + layout: ChartLayout, + leafIndex: number, + marks: Mark[], + tooltips: ChartLayout['tooltipDescriptors'], + sourceMarks = layout.marks, +): void { + const scope = `layer:${leafIndex}:`; + for (const [index, mark] of sourceMarks.entries()) { + const content = layout.tooltipDescriptors.get(`${mark.type}-${index}`); + if (content) tooltips.set(`${mark.type}-${marks.length}`, content); + const key = `${scope}${mark.type}:${mark.key !== undefined ? `key:${mark.key}` : `index:${index}`}`; + marks.push({ + ...mark, + key, + ...((mark.type === 'line' || mark.type === 'area') && mark.pointKeys + ? { pointKeys: mark.pointKeys.map((pointKey) => `${scope}${pointKey}`) } + : {}), + }); + } +} + /** * Compile a LayerSpec into a single ChartLayout. * @@ -46,7 +73,11 @@ export function compileLayer( if (leaves.length === 1) { const singleSpec = buildPrimarySpec(leaves, spec); - return compileChart(singleSpec, options); + const layout = compileChart(singleSpec, options); + const marks: Mark[] = []; + const tooltipDescriptors: ChartLayout['tooltipDescriptors'] = new Map(); + appendLayerMarks(layout, 0, marks, tooltipDescriptors); + return { ...layout, marks, tooltipDescriptors }; } // Past this point every compile is one piece of a multi-leaf layer, not a @@ -62,6 +93,7 @@ export function compileLayer( const primaryLayout = compileChart(primarySpec, layerOptions); const allMarks: Mark[] = []; + const tooltipDescriptors: ChartLayout['tooltipDescriptors'] = new Map(); const seenLabels = new Set(); const pLegend = primaryLayout.legend; const mergedLegendEntries = 'entries' in pLegend ? [...pLegend.entries] : []; @@ -71,6 +103,7 @@ export function compileLayer( const indexedLeaves = leaves.map((leaf, i) => ({ leaf, + leafIndex: i, zIndex: (leaf as ChartSpec).zIndex ?? i, })); indexedLeaves.sort((a, b) => a.zIndex - b.zIndex); @@ -87,7 +120,7 @@ export function compileLayer( // they annotate. Union the quantitative domains across every leaf and pin them. const sharedDomains = computeSharedDomains(leaves); - for (const { leaf } of indexedLeaves) { + for (const { leaf, leafIndex } of indexedLeaves) { const themedLeaf = { ...withSharedDomains(leaf as ChartSpec, sharedDomains), theme: (leaf as ChartSpec).theme ?? spec.theme, @@ -95,7 +128,7 @@ export function compileLayer( }; const leafLayout = compileChart(themedLeaf as unknown, leafOptions); - allMarks.push(...leafLayout.marks); + appendLayerMarks(leafLayout, leafIndex, allMarks, tooltipDescriptors); const leafLeg = leafLayout.legend; if ('entries' in leafLeg) { @@ -116,6 +149,7 @@ export function compileLayer( return { ...primaryLayout, marks: allMarks, + tooltipDescriptors, legend: mergedLegend, // `legends` is what the renderer iterates, so the merged color legend has to // land in it too. Spreading `primaryLayout` alone would leave legends[0] as @@ -390,22 +424,17 @@ function compileLayerIndependent( } } - const l0Count = layout0.marks.length; - const mergedTooltips = new Map(layout0.tooltipDescriptors); - for (const [key, value] of layout1.tooltipDescriptors) { - const match = /^(rect|point|arc)-(\d+)$/.exec(key); - if (match) { - const offsetKey = `${match[1]}-${Number(match[2]) + l0Count}`; - mergedTooltips.set(offsetKey, value); - } else { - mergedTooltips.set(key, value); - } - } - const z0 = leaf0.zIndex ?? 0; const z1 = leaf1.zIndex ?? 1; - const marks = - z0 <= z1 ? [...adjustedMarks0, ...taggedMarks1] : [...taggedMarks1, ...adjustedMarks0]; + const marks: Mark[] = []; + const mergedTooltips: ChartLayout['tooltipDescriptors'] = new Map(); + const layers = [ + { layout: layout0, leafIndex: 0, sourceMarks: adjustedMarks0, zIndex: z0 }, + { layout: layout1, leafIndex: 1, sourceMarks: taggedMarks1, zIndex: z1 }, + ].sort((a, b) => a.zIndex - b.zIndex); + for (const layer of layers) { + appendLayerMarks(layer.layout, layer.leafIndex, marks, mergedTooltips, layer.sourceMarks); + } const mergedLegend = { ...layout0.legend, diff --git a/packages/engine/src/compile/size-scale.ts b/packages/engine/src/compile/size-scale.ts index 095d5cf5..5865b371 100644 --- a/packages/engine/src/compile/size-scale.ts +++ b/packages/engine/src/compile/size-scale.ts @@ -20,6 +20,7 @@ import type { DataRow, EncodingChannel } from '@opendata-ai/openchart-core'; import { max, min } from 'd3-array'; import { scaleLinear, scaleSqrt } from 'd3-scale'; +import { finiteNumber } from '../data/finite-number'; /** How a size value maps onto its visual magnitude. */ export type SizeCurve = 'sqrt' | 'linear'; @@ -105,14 +106,8 @@ export function buildSizeScale( const values: number[] = []; for (const row of data) { - const raw = row[field]; - // Reject holes before coercing: `Number(null)` and `Number('')` are both a - // finite 0, so a null would enter the extent as a real zero and drag the - // domain floor down -- rescaling every genuine mark, and drawing a - // minimum-size mark for a datum that has no size at all. - if (raw == null || raw === '') continue; - const v = Number(raw); - if (Number.isFinite(v)) values.push(v); + const value = finiteNumber(row[field]); + if (value !== undefined) values.push(value); } const explicitDomain = sizeEncoding.scale?.domain as [number, number] | undefined; diff --git a/packages/engine/src/data/finite-number.ts b/packages/engine/src/data/finite-number.ts new file mode 100644 index 00000000..a9fa1cf7 --- /dev/null +++ b/packages/engine/src/data/finite-number.ts @@ -0,0 +1,7 @@ +/** Read a measured number without treating missing or nonnumeric data as zero. */ +export function finiteNumber(value: unknown): number | undefined { + if (typeof value !== 'number' && typeof value !== 'string') return undefined; + if (typeof value === 'string' && value.trim() === '') return undefined; + const number = Number(value); + return Number.isFinite(number) ? number : undefined; +} diff --git a/packages/engine/src/geo/__tests__/compile-geo-map.test.ts b/packages/engine/src/geo/__tests__/compile-geo-map.test.ts index 75ece09b..47a79e13 100644 --- a/packages/engine/src/geo/__tests__/compile-geo-map.test.ts +++ b/packages/engine/src/geo/__tests__/compile-geo-map.test.ts @@ -1528,3 +1528,90 @@ describe('compileGeoMap', () => { }); }); }); + +describe('map numeric observations', () => { + it.each([ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'Infinity', + 'bad', + ])('renders %s as missing color data without changing the measured scale', (value) => { + const layout = compileGeoMap( + { + type: 'map', + geo: { features: MINI_TOPO, projection: 'mercator' }, + data: [ + { fips: '06', value }, + { fips: '48', value: 0 }, + { fips: '36', value: ' 20 ' }, + ], + encoding: { + key: { field: 'fips', type: 'nominal' }, + color: { field: 'value', type: 'quantitative' }, + }, + }, + DEFAULT_OPTIONS, + ); + const missing = layout.features.find((feature) => feature.id === '06')!; + expect(missing.noData).toBe(true); + expect(missing.aria.label).toBe('California: no data'); + expect(layout.tooltipDescriptors.get('06')?.fields).toContainEqual({ + label: 'value', + value: 'No data', + }); + expect(layout.features.find((feature) => feature.id === '48')?.aria.label).toBe('Texas: 0'); + expect(layout.features.find((feature) => feature.id === '36')?.aria.label).toBe('New York: 20'); + }); + + it('omits points with missing coordinates or encoded sizes, retaining real zero coordinates', () => { + const invalid = [null, undefined, '', ' ', false, true, NaN, Infinity, -Infinity, 'bad']; + const layout = compileGeoMap( + { + type: 'map', + geo: { features: MINI_TOPO, projection: 'equalEarth' }, + data: [], + encoding: { key: { field: 'id', type: 'nominal' } }, + points: { + data: [ + ...invalid.map((value, index) => ({ + name: `lon-${index}`, + lon: value, + lat: 0, + size: 10, + })), + ...invalid.map((value, index) => ({ + name: `lat-${index}`, + lon: 0, + lat: value, + size: 10, + })), + ...invalid.map((value, index) => ({ + name: `size-${index}`, + lon: 0, + lat: 0, + size: value, + })), + { name: 'origin', lon: 0, lat: 0, size: 0 }, + { name: 'numeric strings', lon: ' 10 ', lat: ' 20 ', size: ' 20 ' }, + ], + key: { field: 'name', type: 'nominal' }, + longitude: { field: 'lon', type: 'quantitative' }, + latitude: { field: 'lat', type: 'quantitative' }, + size: { field: 'size', type: 'quantitative' }, + }, + }, + DEFAULT_OPTIONS, + ); + expect(layout.pointMarks.map((mark) => mark.key).sort()).toEqual(['numeric strings', 'origin']); + for (const mark of layout.pointMarks) { + expect([mark.cx, mark.cy, mark.r].every(Number.isFinite)).toBe(true); + } + }); +}); diff --git a/packages/engine/src/geo/compile-geo-map.ts b/packages/engine/src/geo/compile-geo-map.ts index d6da567d..5900c952 100644 --- a/packages/engine/src/geo/compile-geo-map.ts +++ b/packages/engine/src/geo/compile-geo-map.ts @@ -41,6 +41,7 @@ import { buildSizeScale, SIZE_SCALE_DEFAULTS } from '../compile/size-scale'; import { emitSpecWarnings, expandSpecSugar } from '../compile/spec-sugar'; import { resolveAnimation } from '../compiler/animation'; import { compile as compileSpec } from '../compiler/index'; +import { finiteNumber } from '../data/finite-number'; import { resolveChromeLayout } from '../layout/shared'; import { type ClassScale, @@ -321,11 +322,8 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay if (colorEncoding && isQuantitative) { const colorValues: number[] = []; for (const row of mapSpec.data) { - const v = row[colorEncoding.field]; - if (v != null) { - const n = Number(v); - if (!Number.isNaN(n)) colorValues.push(n); - } + const value = finiteNumber(row[colorEncoding.field]); + if (value !== undefined) colorValues.push(value); } // Maps class their values rather than ramping them continuously, so the // legend infra draws swatches. `quantize` (round breaks) is the default; @@ -381,11 +379,10 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay const pointContinuousContent = showLegend && pointColor?.type === 'quantitative' ? computeContinuousLegendContentForChannel( - mapSpec - .points!.data.map((row) => row[pointColor.field]) - .filter((value) => value != null) - .map(Number) - .filter(Number.isFinite), + mapSpec.points!.data.flatMap((row) => { + const value = finiteNumber(row[pointColor.field]); + return value === undefined ? [] : [value]; + }), { ...pointColor, scale: { ...pointColor.scale, type: 'quantile' } }, theme, fullArea.width, @@ -496,6 +493,10 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay // 13. Build color scale + feature marks const formatter = buildD3Formatter(mapSpec.encoding?.color?.format ?? mapSpec.valueFormat) ?? formatNumber; + const formatQuantitative = (value: unknown): string => { + const number = finiteNumber(value); + return number === undefined ? 'No data' : formatter(number); + }; // The theme's own neutral ramp, so a warm or cool theme gets a warm or cool // "no data" gray instead of an uninvited zinc. const neutralFill = theme.colors.neutral[100]; @@ -707,19 +708,16 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay if (pts.color.type === 'quantitative') { const colorValues: number[] = []; for (const row of pts.data) { - const raw = row[pts.color.field]; - if (raw != null) { - const v = Number(raw); - if (Number.isFinite(v)) colorValues.push(v); - } + const value = finiteNumber(row[pts.color.field]); + if (value !== undefined) colorValues.push(value); } if (colorValues.length > 0) { const palette = pts.color.scale?.scheme ?? 'blue'; const paletteStops = [...(SEQUENTIAL_PALETTES[palette] ?? SEQUENTIAL_PALETTES.blue)]; const qScale = scaleQuantile().domain(colorValues).range(paletteStops); pointColorScale = (val: unknown) => { - const v = Number(val); - return Number.isFinite(v) ? qScale(v) : CATEGORICAL_PALETTE[0]; + const value = finiteNumber(val); + return value !== undefined ? qScale(value) : neutralFill; }; } } else { @@ -767,19 +765,26 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay for (let i = 0; i < pts.data.length; i++) { const row = pts.data[i]; - const lon = Number(row[lonField]); - const lat = Number(row[latField]); - if (!Number.isFinite(lon) || !Number.isFinite(lat)) continue; + const lon = finiteNumber(row[lonField]); + const lat = finiteNumber(row[latField]); + if (lon === undefined || lat === undefined) continue; + const sizeValue = pts.size ? finiteNumber(row[pts.size.field]) : undefined; + if (pts.size && sizeValue === undefined) continue; const projected = projection([lon, lat]); - if (!projected) { + if (!projected || !projected.every(Number.isFinite)) { droppedPoints.push([lon, lat]); continue; } - const r = sizeScale ? sizeScale.scale(Number(row[sizeScale.field])) : 5; + const r = sizeScale && sizeValue !== undefined ? sizeScale.scale(sizeValue) : 5; + if (!Number.isFinite(r) || r < 0) continue; const fill = - pointColorScale && pts.color ? pointColorScale(row[pts.color.field]) : defaultFill; + pointColorScale && pts.color + ? pointColorScale(row[pts.color.field]) + : pts.color?.type === 'quantitative' + ? neutralFill + : defaultFill; const key = keyChannel ? String(row[keyChannel.field] ?? i) : String(i); // Knockout ring in the canvas color: overlapping bubbles stay countable. const stroke = resolveKnockoutColor(theme); @@ -996,17 +1001,17 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay if (tooltipChannels) { for (const ch of tooltipChannels) { const val = mark.data[ch.field]; - if (val != null) { + if (val != null || ch.type === 'quantitative') { fields.push({ label: ch.title ?? ch.field, - value: ch.type === 'quantitative' ? formatter(Number(val)) : String(val), + value: ch.type === 'quantitative' ? formatQuantitative(val) : String(val), }); } } - } else if (colorEncoding && isQuantitative && mark.data[colorEncoding.field] != null) { + } else if (colorEncoding && isQuantitative) { fields.push({ label: colorEncoding.title ?? colorEncoding.field, - value: formatter(Number(mark.data[colorEncoding.field])), + value: formatQuantitative(mark.data[colorEncoding.field]), }); } else if (colorEncoding && !isQuantitative && mark.data[colorEncoding.field] != null) { fields.push({ @@ -1030,10 +1035,10 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay if (ptTooltipChannels) { for (const ch of ptTooltipChannels) { const val = pm.data[ch.field]; - if (val != null) { + if (val != null || ch.type === 'quantitative') { fields.push({ label: ch.title ?? ch.field, - value: ch.type === 'quantitative' ? formatter(Number(val)) : String(val), + value: ch.type === 'quantitative' ? formatQuantitative(val) : String(val), }); } } @@ -1047,16 +1052,15 @@ export function compileGeoMap(spec: unknown, options: CompileOptions): GeoMapLay } if (pts.color) { const val = pm.data[pts.color.field]; - if (val != null) + if (val != null || pts.color.type === 'quantitative') fields.push({ label: pts.color.title ?? pts.color.field, - value: pts.color.type === 'quantitative' ? formatter(Number(val)) : String(val), + value: pts.color.type === 'quantitative' ? formatQuantitative(val) : String(val), }); } if (pts.size) { const val = pm.data[pts.size.field]; - if (val != null) - fields.push({ label: pts.size.title ?? pts.size.field, value: formatter(Number(val)) }); + fields.push({ label: pts.size.title ?? pts.size.field, value: formatQuantitative(val) }); } } if (fields.length > 0) { @@ -1260,25 +1264,16 @@ function buildQuantitativeMarks(opts: QuantitativeOptions): { const featureId = feat.id as string | number; const dataRow = joined.get(featureId) ?? null; - const hasData = dataRow !== null; let fill = neutralFill; let binIndex: number | undefined; // A value of exactly 0 is data. Only a missing row, a null cell or a // non-numeric cell counts as a hole. - let hasValue = false; - if (hasData) { - const raw = dataRow[colorField]; - if (raw != null) { - const v = Number(raw); - if (!Number.isNaN(v)) { - hasValue = true; - if (classScale) { - fill = classScale.color(v); - binIndex = classScale.binIndex(v); - } - } - } + const value = finiteNumber(dataRow?.[colorField]); + const hasValue = value !== undefined; + if (value !== undefined && classScale) { + fill = classScale.color(value); + binIndex = classScale.binIndex(value); } const props = feat.properties as Record | null; @@ -1307,7 +1302,7 @@ function buildQuantitativeMarks(opts: QuantitativeOptions): { aria: { role: 'img', label: name - ? `${name}: ${hasData && dataRow[colorField] != null && !Number.isNaN(Number(dataRow[colorField])) ? formatter(Number(dataRow[colorField])) : 'no data'}` + ? `${name}: ${value !== undefined ? formatter(value) : 'no data'}` : String(featureId), }, animationIndex: animIndex++, diff --git a/packages/engine/src/layout/scales.ts b/packages/engine/src/layout/scales.ts index 3b2b0099..89efd61c 100644 --- a/packages/engine/src/layout/scales.ts +++ b/packages/engine/src/layout/scales.ts @@ -47,6 +47,7 @@ import { import { isBinnedBarEncoding } from '../charts/post-process'; import type { NormalizedChartSpec } from '../compiler/types'; +import { finiteNumber } from '../data/finite-number'; import { DEFAULT_BIN_COUNT, sampleRampColors } from '../legend/continuous'; import { fieldIterable } from './shared'; @@ -114,23 +115,12 @@ export interface ResolvedScales { // Helpers // --------------------------------------------------------------------------- -/** - * Extract and parse a field to numbers in a single pass (fused fieldValues + - * parseNumbers). Semantics preserved exactly: - * - `v != null` filter first (drops null/undefined, keeps everything else - * including empty string). - * - Non-number values go through `Number(v)`, so `''` becomes `0` and is - * KEPT (0 is finite). Do not "fix" this -- it's existing behavior. - * - `Number.isFinite` filters out NaN/Infinity after coercion. - * - Element order preserved (data order). - */ +/** Extract measured numbers in data order, without coercing holes to zero. */ function numericFieldValues(data: DataRow[], field: string): number[] { const result: number[] = []; - for (const d of data) { - const v = d[field]; - if (v == null) continue; - const n = typeof v === 'number' ? v : Number(v); - if (Number.isFinite(n)) result.push(n); + for (const row of data) { + const value = finiteNumber(row[field]); + if (value !== undefined) result.push(value); } return result; } @@ -937,8 +927,8 @@ export function computeScales( const sums = new Map(); for (const row of data) { const cat = String(row[yField] ?? ''); - const val = Number(row[xField] ?? 0); - if (Number.isFinite(val) && val > 0) { + const val = finiteNumber(row[xField]); + if (val !== undefined && val > 0) { sums.set(cat, (sums.get(cat) ?? 0) + val); } } @@ -960,8 +950,8 @@ export function computeScales( const sums = new Map(); for (const row of data) { const cat = String(row[yField] ?? ''); - const val = Number(row[xField] ?? 0); - if (Number.isFinite(val) && val > 0) { + const val = finiteNumber(row[xField]); + if (val !== undefined && val > 0) { sums.set(cat, (sums.get(cat) ?? 0) + val); } } @@ -1071,8 +1061,8 @@ export function computeScales( const sums = new Map(); for (const row of data) { const cat = String(row[xField] ?? ''); - const val = Number(row[yField] ?? 0); - if (Number.isFinite(val) && val > 0) { + const val = finiteNumber(row[yField]); + if (val !== undefined && val > 0) { sums.set(cat, (sums.get(cat) ?? 0) + val); } } @@ -1094,8 +1084,8 @@ export function computeScales( const sums = new Map(); for (const row of data) { const cat = String(row[xField] ?? ''); - const val = Number(row[yField] ?? 0); - if (Number.isFinite(val) && val > 0) { + const val = finiteNumber(row[yField]); + if (val !== undefined && val > 0) { sums.set(cat, (sums.get(cat) ?? 0) + val); } } diff --git a/packages/engine/src/sankey/__tests__/compile-sankey.test.ts b/packages/engine/src/sankey/__tests__/compile-sankey.test.ts index 1d9b9151..423ade1a 100644 --- a/packages/engine/src/sankey/__tests__/compile-sankey.test.ts +++ b/packages/engine/src/sankey/__tests__/compile-sankey.test.ts @@ -704,3 +704,69 @@ describe('sankey other bucketing', () => { expect(after.nodes.map((n) => n.nodeId).sort()).toEqual(['Big', 'Sink', 'Small1']); }); }); + +describe('Sankey finite flow geometry', () => { + it.each([ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'Infinity', + 'bad', + -10, + 0, + '0', + ])('returns an empty finite figure with an actionable warning for %s flow', (amount) => { + const warnings: string[] = []; + const layout = compileSankey( + { ...basicSpec, data: [{ from: 'A', to: 'B', amount }] }, + { + ...defaultOptions, + onWarn: (message) => warnings.push(message), + }, + ); + expect(layout.nodes).toEqual([]); + expect(layout.links).toEqual([]); + expect(Object.values(layout.area).every(Number.isFinite)).toBe(true); + expect(layout.a11y.keyboardNavigable).toBe(false); + expect(warnings.join(' ')).toContain('positive finite'); + expect(warnings.join(' ')).toContain('amount'); + }); + + it('retains measured flows and original rows while excluding invalid flow nodes and legends', () => { + const validRows = [ + { from: 'A', to: 'B', amount: ' 10 ', group: 'Measured' }, + { from: 'B', to: 'C', amount: 10, group: 'Measured' }, + ]; + const invalidRows = [null, '', false, Infinity, -1, 0].map((amount, index) => ({ + from: `invalid-${index}`, + to: 'C', + amount, + group: 'Missing', + })); + const layout = compileSankey( + { + ...basicSpec, + data: [...validRows, ...invalidRows], + encoding: { ...basicSpec.encoding, color: { field: 'group', type: 'nominal' } }, + }, + { ...defaultOptions, onWarn: () => {} }, + ); + expect(layout.nodes.map((node) => node.nodeId).sort()).toEqual(['A', 'B', 'C']); + expect(layout.links).toHaveLength(2); + for (const link of layout.links) { + expect(link.path).not.toMatch(/NaN|Infinity/); + expect(link.value).toBe(10); + } + for (const node of layout.nodes) { + expect([node.x, node.y, node.width, node.height].every(Number.isFinite)).toBe(true); + } + expect(JSON.stringify(layout.legend)).not.toContain('Missing'); + expect(validRows[0].amount).toBe(' 10 '); + }); +}); diff --git a/packages/engine/src/sankey/__tests__/layout.test.ts b/packages/engine/src/sankey/__tests__/layout.test.ts index dc2bdd51..bf191813 100644 --- a/packages/engine/src/sankey/__tests__/layout.test.ts +++ b/packages/engine/src/sankey/__tests__/layout.test.ts @@ -163,3 +163,14 @@ describe('computeSankeyLayout', () => { } }); }); + +it('returns no geometry for nonpositive or unmeasured direct layout flows', () => { + const data = [null, undefined, '', ' ', false, true, NaN, Infinity, -1, 0].map((value) => ({ + source: 'A', + target: 'B', + value, + })); + expect( + computeSankeyLayout(data, 'source', 'target', 'value', area, 12, 16, 'justify', 6), + ).toEqual({ nodes: [], links: [] }); +}); diff --git a/packages/engine/src/sankey/compile-sankey.ts b/packages/engine/src/sankey/compile-sankey.ts index 0781ceed..123d2b36 100644 --- a/packages/engine/src/sankey/compile-sankey.ts +++ b/packages/engine/src/sankey/compile-sankey.ts @@ -38,6 +38,7 @@ import { import { emitSpecWarnings, expandSpecSugar } from '../compile/spec-sugar'; import { resolveAnimation } from '../compiler/animation'; import { compile as compileSpec } from '../compiler/index'; +import { finiteNumber } from '../data/finite-number'; import { resolveFieldFormatter } from '../format/field-format'; import { resolveChromeLayout } from '../layout/shared'; import { ENTRY_GAP, measureLegendWrap, SWATCH_GAP, SWATCH_SIZE } from '../legend/wrap'; @@ -205,10 +206,11 @@ function rewriteRowsForOther( const newTgt = merged.get(tgt) ?? tgt; if (newSrc === newTgt) continue; const key = `${newSrc}\u0000${newTgt}`; - const value = Number(row[valueField]) || 0; + const value = finiteNumber(row[valueField]); + if (value === undefined || value <= 0) continue; const existing = index.get(key); if (existing) { - existing[valueField] = (Number(existing[valueField]) || 0) + value; + existing[valueField] = (finiteNumber(existing[valueField]) ?? 0) + value; continue; } const next = { ...row, [sourceField]: newSrc, [targetField]: newTgt, [valueField]: value }; @@ -405,15 +407,44 @@ export function compileSankey(spec: unknown, options: CompileOptions): SankeyLay const targetField = sankeySpec.encoding.target.field; const valueField = sankeySpec.encoding.value.field; const colorField = sankeySpec.encoding.color?.field; + let missingFlowCount = 0; + let negativeFlowCount = 0; + let zeroFlowCount = 0; + const measuredData = sankeySpec.data.filter((row) => { + const value = finiteNumber(row[valueField]); + if (value === undefined) missingFlowCount++; + else if (value < 0) negativeFlowCount++; + else if (value === 0) zeroFlowCount++; + else return true; + return false; + }); + const omittedCount = missingFlowCount + negativeFlowCount + zeroFlowCount; + if (omittedCount > 0) { + const reasons = [ + missingFlowCount > 0 ? `${missingFlowCount} missing, nonnumeric, or nonfinite` : '', + negativeFlowCount > 0 ? `${negativeFlowCount} negative` : '', + zeroFlowCount > 0 ? `${zeroFlowCount} zero-width` : '', + ].filter(Boolean); + emitSpecWarnings( + [ + `Sankey omitted ${omittedCount} flow rows from "${valueField}" (${reasons.join(', ')}). ` + + 'Sankey geometry requires positive finite values. Supply measured positive amounts; zero flows have no drawable width.', + ], + options.onWarn, + ); + } + if (measuredData.length === 0) { + return emptyLayout(fullArea, chrome, theme, { ...options, height: grownHeight }, watermark); + } const flowFmt = resolveFieldFormatter({ surfaceFormat: resolvedValueFormat, - values: sankeySpec.data.map((r) => r[valueField]), + values: measuredData.map((r) => r[valueField]), }); // 5b. Pre-compute legend to reserve vertical space // We need the color map first, so build a temporary one from raw data const tempNodeIds = new Set(); - for (const row of sankeySpec.data) { + for (const row of measuredData) { tempNodeIds.add(String(row[sourceField])); tempNodeIds.add(String(row[targetField])); } @@ -421,14 +452,14 @@ export function compileSankey(spec: unknown, options: CompileOptions): SankeyLay [...tempNodeIds].map((id) => ({ id })), theme.colors.categorical, colorField, - sankeySpec.data, + measuredData, sourceField, targetField, ); const legend = buildSankeyLegend( tempColorMap, colorField, - sankeySpec.data, + measuredData, sourceField, targetField, theme, @@ -455,7 +486,7 @@ export function compileSankey(spec: unknown, options: CompileOptions): SankeyLay const valueFontWeight = theme.fonts.weights.normal; const nodeLabelAlign = sankeySpec.nodeLabelAlign ?? 'auto'; - let workingData = sankeySpec.data; + let workingData = measuredData; const runLayout = (rect: Rect) => computeSankeyLayout( workingData, diff --git a/packages/engine/src/sankey/layout.ts b/packages/engine/src/sankey/layout.ts index f5c42628..b1944902 100644 --- a/packages/engine/src/sankey/layout.ts +++ b/packages/engine/src/sankey/layout.ts @@ -9,6 +9,7 @@ import type { Rect, SankeyNodeAlign } from '@opendata-ai/openchart-core'; import type { SankeyExtraProperties, SankeyGraph, SankeyLink, SankeyNode } from 'd3-sankey'; import { sankey, sankeyCenter, sankeyJustify, sankeyLeft, sankeyRight } from 'd3-sankey'; +import { finiteNumber } from '../data/finite-number'; // --------------------------------------------------------------------------- // Types @@ -75,30 +76,28 @@ export function computeSankeyLayout( iterations: number, nodeSort?: string[], ): SankeyLayoutResult { - // Extract unique node IDs from source and target columns - const nodeSet = new Set(); - for (const row of data) { - nodeSet.add(String(row[sourceField])); - nodeSet.add(String(row[targetField])); - } - - // Build node and link arrays (cloned so d3-sankey mutations don't affect input) - const nodes: Array<{ id: string; label: string }> = [...nodeSet].map((id) => ({ - id, - label: id, - })); - + // Zero has no drawable flow area. Missing or nonpositive flows must not + // create isolated nodes that make d3-sankey divide by zero. const links: Array<{ source: string; target: string; value: number; data: Record; - }> = data.map((row) => ({ - source: String(row[sourceField]), - target: String(row[targetField]), - value: Number(row[valueField]) || 0, - data: { ...row }, - })); + }> = []; + const nodeSet = new Set(); + for (const row of data) { + const value = finiteNumber(row[valueField]); + if (value === undefined || value <= 0) continue; + const source = String(row[sourceField]); + const target = String(row[targetField]); + nodeSet.add(source); + nodeSet.add(target); + links.push({ source, target, value, data: { ...row } }); + } + if (links.length === 0) return { nodes: [], links: [] }; + + // Clone nodes and rows: d3-sankey mutates the graph in place. + const nodes = [...nodeSet].map((id) => ({ id, label: id })); // Configure and run d3-sankey const alignFn = ALIGN_MAP[nodeAlign] ?? sankeyJustify; diff --git a/packages/engine/src/tables/__tests__/compile-table.test.ts b/packages/engine/src/tables/__tests__/compile-table.test.ts index 2d8e7828..662e92fc 100644 --- a/packages/engine/src/tables/__tests__/compile-table.test.ts +++ b/packages/engine/src/tables/__tests__/compile-table.test.ts @@ -82,10 +82,10 @@ describe('compileTable', () => { const spec = { ...baseSpec, rowKey: 'name' }; const layout = compileTable(spec, baseOptions); - expect(layout.rows[0].id).toBe('Alice'); - expect(layout.rows[1].id).toBe('Bob'); - expect(layout.rows[2].id).toBe('Charlie'); - expect(layout.rows[3].id).toBe('Diana'); + expect(layout.rows[0].id).toBe('string:Alice'); + expect(layout.rows[1].id).toBe('string:Bob'); + expect(layout.rows[2].id).toBe('string:Charlie'); + expect(layout.rows[3].id).toBe('string:Diana'); }); it('applies tabular-nums font variant on number cells', () => { diff --git a/packages/engine/src/tables/compile-table.ts b/packages/engine/src/tables/compile-table.ts index 3414616e..d6b255ba 100644 --- a/packages/engine/src/tables/compile-table.ts +++ b/packages/engine/src/tables/compile-table.ts @@ -25,6 +25,7 @@ import { computeChrome, estimateTextWidth } from '@opendata-ai/openchart-core'; import { emitSpecWarnings } from '../compile/spec-sugar'; import { resolveAnimation } from '../compiler/animation'; import type { NormalizedTableSpec } from '../compiler/types'; +import { resolveObservationIdentities } from '../design/identity'; import { computeBarCell, computeColumnMax, computeColumnMin } from './bar-column'; import { computeCategoryColors } from './category-colors'; import { formatCell } from './format-cells'; @@ -581,10 +582,19 @@ export function compileTableLayout( } } + const identities = spec.rowKey ? resolveObservationIdentities(data, spec.rowKey) : null; + if (identities?.some((identity) => !identity.persistent)) + emitSpecWarnings( + [ + 'TABLE_ROW_KEY_AMBIGUOUS: rowKey contains duplicate or missing values; affected rows use revision-local identities.', + ], + options.onWarn, + ); + // 8. Build rows from visible data const rows: TableRow[] = currentData.map((row, i) => { const origIdx = originalIndices[i]; - const rowId = spec.rowKey ? String(row[spec.rowKey] ?? origIdx) : String(origIdx); + const rowId = identities?.[origIdx].observation ?? String(origIdx); const cells: TableCell[] = spec.columns.map((col, c) => { const resolved = resolvedColumns[c]; diff --git a/packages/engine/src/tilemap/__tests__/compile-tilemap.test.ts b/packages/engine/src/tilemap/__tests__/compile-tilemap.test.ts index 041779fa..1b47adc3 100644 --- a/packages/engine/src/tilemap/__tests__/compile-tilemap.test.ts +++ b/packages/engine/src/tilemap/__tests__/compile-tilemap.test.ts @@ -660,3 +660,45 @@ describe('compileTileMap', () => { }); }); }); + +describe('tilemap numeric observations', () => { + it.each([ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'Infinity', + 'bad', + ])('keeps %s missing while retaining numeric zero and numeric strings', (value) => { + const layout = compileTileMap( + { + type: 'tilemap', + data: [ + { state: 'CA', value }, + { state: 'TX', value: 0 }, + { state: 'NY', value: ' 20 ' }, + ], + encoding: { state: { field: 'state' }, value: { field: 'value' } }, + }, + defaultOptions, + ); + expect(layout.tiles.find((tile) => tile.stateCode === 'CA')).toMatchObject({ + value: null, + hasData: false, + }); + expect(layout.tiles.find((tile) => tile.stateCode === 'TX')).toMatchObject({ + value: 0, + hasData: true, + }); + expect(layout.tiles.find((tile) => tile.stateCode === 'NY')).toMatchObject({ + value: 20, + hasData: true, + }); + expect(layout.tiles.every((tile) => Number.isFinite(tile.fillOpacity))).toBe(true); + }); +}); diff --git a/packages/engine/src/tilemap/compile-tilemap.ts b/packages/engine/src/tilemap/compile-tilemap.ts index 3aacb129..54e1fa7d 100644 --- a/packages/engine/src/tilemap/compile-tilemap.ts +++ b/packages/engine/src/tilemap/compile-tilemap.ts @@ -48,6 +48,7 @@ import { scaleLinear } from 'd3-scale'; import { emitSpecWarnings, expandSpecSugar } from '../compile/spec-sugar'; import { resolveAnimation } from '../compiler/animation'; import { compile as compileSpec } from '../compiler/index'; +import { finiteNumber } from '../data/finite-number'; import { resolveFieldFormatter } from '../format/field-format'; import { resolveChromeLayout } from '../layout/shared'; import { computeTilePositions, STATE_CODE_SET, STATE_NAMES, US_STATE_TILES } from './layout'; @@ -269,12 +270,9 @@ function compileQuantitative( const stateValueMap = new Map(); for (const row of tilemapSpec.data) { const stateCode = String(row[stateField]); - const raw = row[valueField]; - if (STATE_CODE_SET.has(stateCode) && raw !== null && raw !== undefined) { - const value = Number(raw); - if (!Number.isNaN(value)) { - stateValueMap.set(stateCode, value); - } + const value = finiteNumber(row[valueField]); + if (STATE_CODE_SET.has(stateCode) && value !== undefined) { + stateValueMap.set(stateCode, value); } } diff --git a/packages/engine/src/transforms/__tests__/aggregate.test.ts b/packages/engine/src/transforms/__tests__/aggregate.test.ts index a9ec5f92..27727919 100644 --- a/packages/engine/src/transforms/__tests__/aggregate.test.ts +++ b/packages/engine/src/transforms/__tests__/aggregate.test.ts @@ -157,3 +157,37 @@ describe('runAggregate', () => { expect(result).toHaveLength(0); }); }); + +it('ignores nonmeasurements in numeric summaries without changing row counts', () => { + const data = [ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'bad', + 0, + ' 20 ', + -5, + ].map((value) => ({ value })); + expect( + runAggregate(data, { + groupby: [], + aggregate: [ + { op: 'sum', field: 'value', as: 'sum' }, + { op: 'mean', field: 'value', as: 'mean' }, + { op: 'count', field: 'value', as: 'count' }, + ], + }), + ).toEqual([{ sum: 15, mean: 5, count: 13 }]); + expect( + runAggregate(data.slice(0, 10), { + groupby: [], + aggregate: [{ op: 'sum', field: 'value', as: 'sum' }], + }), + ).toEqual([{ sum: null }]); +}); diff --git a/packages/engine/src/transforms/__tests__/bin.test.ts b/packages/engine/src/transforms/__tests__/bin.test.ts index 4c61a5d5..fc93ecd5 100644 --- a/packages/engine/src/transforms/__tests__/bin.test.ts +++ b/packages/engine/src/transforms/__tests__/bin.test.ts @@ -201,3 +201,19 @@ describe('runBin', () => { }); }); }); + +it.each(['linear', 'log'] as const)('keeps nonmeasurements unbinned on a %s scale', (scaleType) => { + const data = [null, undefined, '', ' ', false, true, NaN, Infinity, -Infinity, 'bad'].map( + (value) => ({ value }), + ); + const result = runBin([...data, { value: ' 20 ' }], { + bin: { scaleType }, + field: 'value', + as: ['start', 'end'], + }); + expect(result.slice(0, data.length).every((row) => row.start === null && row.end === null)).toBe( + true, + ); + expect(Number.isFinite(result.at(-1)?.start)).toBe(true); + expect(Number.isFinite(result.at(-1)?.end)).toBe(true); +}); diff --git a/packages/engine/src/transforms/__tests__/density.test.ts b/packages/engine/src/transforms/__tests__/density.test.ts index 9a9fef51..8eae9799 100644 --- a/packages/engine/src/transforms/__tests__/density.test.ts +++ b/packages/engine/src/transforms/__tests__/density.test.ts @@ -108,7 +108,7 @@ describe('runDensity', () => { it('ignores rows with a missing value rather than counting them as zero', () => { const withNulls: DataRow[] = [ ...sample(100, 500, 20, 23), - ...Array.from({ length: 50 }, () => ({ v: null })), + ...[null, undefined, '', ' ', false, true, NaN, Infinity, -Infinity].map((v) => ({ v })), ]; const clean = runDensity(sample(100, 500, 20, 23), { density: 'v', steps: 40 }); const dirty = runDensity(withNulls, { density: 'v', steps: 40 }); diff --git a/packages/engine/src/transforms/__tests__/window.test.ts b/packages/engine/src/transforms/__tests__/window.test.ts index 364f5eca..70b8c5e4 100644 --- a/packages/engine/src/transforms/__tests__/window.test.ts +++ b/packages/engine/src/transforms/__tests__/window.test.ts @@ -284,3 +284,52 @@ describe('window transform', () => { expect(result.map((r) => r.delta)).toEqual([10, 30]); }); }); + +describe('window comparisons require measured values', () => { + it.each([ + null, + undefined, + '', + ' ', + false, + true, + NaN, + Infinity, + -Infinity, + 'bad', + ])('returns no comparison across a %s observation', (value) => { + const result = runTransforms( + [10, value, ' 20 ', 0, 10].map((value, index) => ({ value, index })), + [ + { + window: [ + { op: 'diff', field: 'value', as: 'delta' }, + { op: 'pct_change', field: 'value', as: 'change' }, + ], + sort: [{ field: 'index' }], + }, + ], + ); + expect(result.map((row) => row.delta)).toEqual([null, null, null, -20, 10]); + expect(result.map((row) => row.change)).toEqual([null, null, null, -1, null]); + }); + + it('returns no comparison when the numeric result cannot be represented finitely', () => { + const result = runTransforms( + [{ value: -Number.MAX_VALUE }, { value: Number.MAX_VALUE }], + [ + { + window: [ + { op: 'diff', field: 'value', as: 'delta' }, + { op: 'pct_change', field: 'value', as: 'change' }, + ], + sort: [], + }, + ], + ); + expect(result.map((row) => [row.delta, row.change])).toEqual([ + [null, null], + [null, null], + ]); + }); +}); diff --git a/packages/engine/src/transforms/aggregate.ts b/packages/engine/src/transforms/aggregate.ts index 81b98962..1409ee08 100644 --- a/packages/engine/src/transforms/aggregate.ts +++ b/packages/engine/src/transforms/aggregate.ts @@ -7,6 +7,7 @@ */ import type { AggregateOp, AggregateTransform, DataRow } from '@opendata-ai/openchart-core'; +import { finiteNumber } from '../data/finite-number'; /** * Compute a single aggregate operation over an array of numeric values. @@ -74,12 +75,8 @@ export function aggregateRows(op: AggregateOp, field: string, rows: DataRow[]): if (op === 'count') return rows.length; if (op === 'distinct') return new Set(rows.map((row) => row[field])).size; const values = rows.flatMap((row) => { - const raw = row[field]; - if (raw == null || typeof raw === 'boolean' || (typeof raw === 'string' && !raw.trim())) - return []; - if (typeof raw !== 'number' && typeof raw !== 'string') return []; - const value = Number(raw); - return Number.isFinite(value) ? [value] : []; + const value = finiteNumber(row[field]); + return value === undefined ? [] : [value]; }); return values.length ? computeAggregate(op, values) : null; } diff --git a/packages/engine/src/transforms/bin.ts b/packages/engine/src/transforms/bin.ts index 2bb5a1a0..a10556b4 100644 --- a/packages/engine/src/transforms/bin.ts +++ b/packages/engine/src/transforms/bin.ts @@ -5,6 +5,7 @@ */ import type { BinParams, BinTransform, DataRow } from '@opendata-ai/openchart-core'; +import { finiteNumber } from '../data/finite-number'; /** * Compute a nice step size for binning. @@ -85,8 +86,8 @@ export function runBin(data: DataRow[], transform: BinTransform): DataRow[] { let min = Infinity; let max = -Infinity; for (const row of data) { - const v = Number(row[field]); - if (Number.isFinite(v)) { + const v = finiteNumber(row[field]); + if (v !== undefined) { // For log scale, skip non-positive values if (params.scaleType === 'log' && v <= 0) continue; if (v < min) min = v; @@ -102,10 +103,10 @@ export function runBin(data: DataRow[], transform: BinTransform): DataRow[] { if (params.scaleType === 'log') { const edges = computeLogBinEdges(extent, maxbins); return data.map((row) => { - const v = Number(row[field]); + const v = finiteNumber(row[field]); const newRow = { ...row }; - if (!Number.isFinite(v) || v <= 0) { + if (v === undefined || v <= 0) { newRow[startAs] = null; if (endAs) newRow[endAs] = null; } else { @@ -131,10 +132,10 @@ export function runBin(data: DataRow[], transform: BinTransform): DataRow[] { const step = params.step ?? computeStep(extent, maxbins, nice); return data.map((row) => { - const v = Number(row[field]); + const v = finiteNumber(row[field]); const newRow = { ...row }; - if (!Number.isFinite(v)) { + if (v === undefined) { newRow[startAs] = null; if (endAs) newRow[endAs] = null; } else { diff --git a/packages/engine/src/transforms/density.ts b/packages/engine/src/transforms/density.ts index a321552e..06a113f6 100644 --- a/packages/engine/src/transforms/density.ts +++ b/packages/engine/src/transforms/density.ts @@ -8,6 +8,7 @@ import type { DataRow, DensityTransform } from '@opendata-ai/openchart-core'; import { deviation, quantile } from 'd3-array'; +import { finiteNumber } from '../data/finite-number'; /** Default number of points the curve is evaluated at. */ const DEFAULT_STEPS = 200; @@ -114,10 +115,8 @@ export function runDensity(data: DataRow[], transform: DensityTransform): DataRo // so a row with no value would otherwise plant a kernel at zero. const samples: number[] = []; for (const r of rows) { - const raw = r[field]; - if (raw == null || raw === '') continue; - const v = Number(raw); - if (Number.isFinite(v)) samples.push(v); + const value = finiteNumber(r[field]); + if (value !== undefined) samples.push(value); } if (samples.length === 0) continue; const bandwidth = diff --git a/packages/engine/src/transforms/window.ts b/packages/engine/src/transforms/window.ts index b448e375..7c1ac558 100644 --- a/packages/engine/src/transforms/window.ts +++ b/packages/engine/src/transforms/window.ts @@ -7,6 +7,7 @@ */ import type { DataRow, WindowTransform } from '@opendata-ai/openchart-core'; +import { finiteNumber } from '../data/finite-number'; /** * Build a composite group key from a row's groupby field values. @@ -125,20 +126,22 @@ export function runWindow(data: DataRow[], transform: WindowTransform): DataRow[ case 'diff': { const lagIdx = i - offset; if (lagIdx >= 0) { - const current = Number(entry.row[def.field]); - const lagged = Number(sorted[lagIdx].row[def.field]); + const current = finiteNumber(entry.row[def.field]); + const lagged = finiteNumber(sorted[lagIdx].row[def.field]); computed = - Number.isFinite(current) && Number.isFinite(lagged) ? current - lagged : null; + current !== undefined && lagged !== undefined + ? (finiteNumber(current - lagged) ?? null) + : null; } break; } case 'pct_change': { const lagIdx = i - offset; if (lagIdx >= 0) { - const current = Number(entry.row[def.field]); - const lagged = Number(sorted[lagIdx].row[def.field]); - if (Number.isFinite(current) && Number.isFinite(lagged) && lagged !== 0) { - computed = (current - lagged) / lagged; + const current = finiteNumber(entry.row[def.field]); + const lagged = finiteNumber(sorted[lagIdx].row[def.field]); + if (current !== undefined && lagged !== undefined && lagged !== 0) { + computed = finiteNumber((current - lagged) / lagged) ?? null; } } break; diff --git a/packages/vanilla/src/__tests__/export.test.ts b/packages/vanilla/src/__tests__/export.test.ts index dd03d9ba..76cfd902 100644 --- a/packages/vanilla/src/__tests__/export.test.ts +++ b/packages/vanilla/src/__tests__/export.test.ts @@ -9,7 +9,7 @@ import type { ChartSpec } from '@opendata-ai/openchart-core'; import type { CompileOptions } from '@opendata-ai/openchart-engine'; import { compileChart } from '@opendata-ai/openchart-engine'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { createContainer } from '../__test-fixtures__/dom'; import { barSpec, lineSpec } from '../__test-fixtures__/specs'; import { @@ -21,6 +21,7 @@ import { getSVGBackgroundColor, } from '../export'; import { createChart } from '../mount'; +import { createSankey } from '../sankey-mount'; import { type CanvasStub, stubCanvas2D } from '../scatter-canvas/__tests__/canvas-stub'; import { renderChartSVG } from '../svg-renderer'; @@ -38,6 +39,7 @@ function renderToSVG(spec = lineSpec) { afterEach(() => { document.body.innerHTML = ''; + vi.unstubAllGlobals(); }); // --------------------------------------------------------------------------- @@ -92,6 +94,110 @@ describe('exportSVG', () => { // --------------------------------------------------------------------------- describe('exportSVGWithFonts', () => { + function installFontFixtures(weights: Array = [400, 500, 600, 700]) { + const stylesheet = document.createElement('style'); + stylesheet.textContent = weights + .map( + (weight) => + `@font-face { font-family: 'ExportFont'; font-weight: ${weight}; src: url('https://fonts.example/face-${weight}.woff2') format('woff2'); }`, + ) + .join('\n'); + document.body.appendChild(stylesheet); + // Only the network boundary is stubbed; font discovery, rendering and + // serialization run through the production export pipeline. + const fetchFont = vi.fn(async (_url: string) => new Response(new Uint8Array([1, 2, 3]))); + vi.stubGlobal('fetch', fetchFont); + return fetchFont; + } + + it('embeds inline-styled fonts from a terminal Sankey with attribution disabled', async () => { + const fetchFont = installFontFixtures(); + const container = createContainer(390, 420); + const sankey = createSankey( + container, + { + type: 'sankey', + data: [{ from: 'Supply', to: 'Demand', value: 12 }], + encoding: { + source: { field: 'from', type: 'nominal' }, + target: { field: 'to', type: 'nominal' }, + value: { field: 'value', type: 'quantitative' }, + }, + chrome: { title: 'Where the energy goes' }, + }, + { watermark: false, theme: { fonts: { family: 'ExportFont, sans-serif' } } }, + ); + expect(container.querySelector('text[font-family]')).toBeNull(); + const exported = await sankey.export('svg-with-fonts'); + expect(fetchFont).toHaveBeenCalled(); + expect(exported).toContain("font-family: 'ExportFont'"); + expect(exported).toContain('src: url(data:font/woff2;base64,AQID)'); + // Embedding the detached export must not mutate the interactive SVG. + expect(container.querySelector('svg style')).toBeNull(); + sankey.destroy(); + }); + + it('embeds inherited span fonts and gives inline styles precedence over attributes', async () => { + const fetchFont = installFontFixtures(); + const svg = renderToSVG(); + const title = svg.querySelector('.oc-title')!; + title.setAttribute('font-family', 'UnusedFont'); + title.setAttribute('font-weight', '900'); + title.style.fontFamily = 'ExportFont, sans-serif'; + title.style.fontWeight = 'normal'; + const span = document.createElementNS('http://www.w3.org/2000/svg', 'tspan'); + span.style.fontWeight = 'bold'; + span.textContent = ' highlighted'; + title.appendChild(span); + + const exported = await exportSVGWithFonts(svg); + expect(fetchFont.mock.calls.map(([url]) => url).sort()).toEqual([ + 'https://fonts.example/face-400.woff2', + 'https://fonts.example/face-700.woff2', + ]); + expect(exported).toContain("font-family: 'ExportFont'; font-weight: 400"); + expect(exported).toContain("font-family: 'ExportFont'; font-weight: 700"); + }); + + it.each([ + ['normal', '400'], + ['bold', '700'], + ])('matches a %s font-face declaration to its numeric text weight', async (keyword, weight) => { + const fetchFont = installFontFixtures([keyword]); + const svg = renderToSVG(); + const title = svg.querySelector('.oc-title')!; + title.style.fontFamily = 'ExportFont'; + title.style.fontWeight = weight; + const exported = await exportSVGWithFonts(svg); + expect(fetchFont).toHaveBeenCalledWith(`https://fonts.example/face-${keyword}.woff2`); + expect(exported).toContain(`font-family: 'ExportFont'; font-weight: ${weight}`); + }); + + it('resets an initial text weight instead of inheriting the ancestor weight', async () => { + const fetchFont = installFontFixtures(); + const svg = renderToSVG(); + svg.style.fontWeight = '700'; + const title = svg.querySelector('.oc-title')!; + title.style.fontFamily = 'ExportFont'; + title.style.fontWeight = 'initial'; + const exported = await exportSVGWithFonts(svg); + expect(fetchFont.mock.calls.map(([url]) => url)).toEqual([ + 'https://fonts.example/face-400.woff2', + ]); + expect(exported).toContain("font-family: 'ExportFont'; font-weight: 400"); + }); + + it('does not inherit an authored font family through an initial declaration', async () => { + const fetchFont = installFontFixtures(); + const svg = renderToSVG(); + svg.style.fontFamily = 'ExportFont'; + const title = svg.querySelector('.oc-title')!; + title.style.fontFamily = 'initial'; + const exported = await exportSVGWithFonts(svg); + expect(fetchFont).not.toHaveBeenCalled(); + expect(exported).not.toContain('@font-face'); + }); + it('returns a promise', () => { const svg = renderToSVG(); const result = exportSVGWithFonts(svg); diff --git a/packages/vanilla/src/__tests__/layer-interaction.test.ts b/packages/vanilla/src/__tests__/layer-interaction.test.ts new file mode 100644 index 00000000..92b74ab2 --- /dev/null +++ b/packages/vanilla/src/__tests__/layer-interaction.test.ts @@ -0,0 +1,126 @@ +import type { ChartSpec, LayerSpec } from '@opendata-ai/openchart-core'; +import { afterEach, describe, expect, it } from 'vitest'; +import { createContainer, createMouseEvent } from '../__test-fixtures__/dom'; +import { createChart } from '../mount'; + +afterEach(() => { + document.body.innerHTML = ''; +}); + +function leaf( + mark: ChartSpec['mark'], + name: string, + value: number, + format: string, + zIndex: number, +): ChartSpec { + return { + mark, + zIndex, + animation: false, + data: [{ id: 'same-key', series: 'Shared', name, x: 1, value }], + encoding: { + key: { field: 'id', type: 'nominal' }, + x: { field: 'x', type: mark === 'point' ? 'quantitative' : 'ordinal' }, + y: { field: 'value', type: 'quantitative', stack: null }, + color: { field: 'series', type: 'nominal' }, + tooltip: [{ field: 'value', type: 'quantitative', title: name, format }], + }, + }; +} + +describe('inspection of real layered charts', () => { + it.each([ + 'shared', + 'independent', + ] as const)('shows exact leaf formatting on hover and keyboard with %s scales', (scale) => { + const container = createContainer(); + const spec: LayerSpec = { + layer: [leaf('point', 'Revenue', 1250, '$,.0f', 1), leaf('point', 'Share', 0.25, '.0%', 0)], + resolve: { scale: { y: scale } }, + }; + const clicked: unknown[] = []; + const chart = createChart(container, spec, { + onMarkClick: (event) => clicked.push(event.datum.name), + }); + try { + const marks = [...container.querySelectorAll('.oc-marks [data-mark-id]')]; + expect(marks).toHaveLength(2); + for (const [index, expected] of ['25%', '$1,250'].entries()) { + marks[index].dispatchEvent(createMouseEvent('mouseenter')); + expect(container.querySelector('[role="tooltip"]')?.textContent).toContain(expected); + marks[index].dispatchEvent(createMouseEvent('click')); + marks[index].dispatchEvent(createMouseEvent('mouseleave')); + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(container.querySelector('[role="tooltip"]')?.textContent).toContain(expected); + } + expect(clicked).toEqual(['Share', 'Revenue']); + } finally { + chart.destroy(); + } + }); + + it.each([ + 'line', + 'area', + ] as const)('dispatches each %s leaf with the same series name to its own observation', (mark) => { + const container = createContainer(); + const clicked: unknown[] = []; + const chart = createChart( + container, + { + layer: [{ layer: [leaf(mark, 'First', 10, '.0f', 1)] }, leaf(mark, 'Second', 20, '.0f', 0)], + }, + { onMarkClick: (event) => clicked.push({ name: event.datum.name, series: event.series }) }, + ); + try { + const marks = [...container.querySelectorAll(`.oc-mark-${mark}`)]; + expect(marks).toHaveLength(2); + expect(new Set(marks.map((element) => element.getAttribute('data-mark-id'))).size).toBe(2); + for (const element of marks) element.dispatchEvent(createMouseEvent('click')); + expect(clicked).toEqual([ + { name: 'Second', series: 'Shared' }, + { name: 'First', series: 'Shared' }, + ]); + } finally { + chart.destroy(); + } + }); + + it.each([ + 'line', + 'area', + ] as const)('inspects both %s layers with repeated series names through the shared crosshair', (mark) => { + const container = createContainer(); + const chart = createChart(container, { + layer: [leaf(mark, 'Revenue', 1250, '$,.0f', 1), leaf(mark, 'Share', 0.25, '.0%', 0)], + }); + try { + const tooltip = container.querySelector('[role="tooltip"]')!; + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(tooltip.textContent).toContain('25%'); + expect(tooltip.textContent).toContain('$1,250'); + expect(tooltip.textContent).toContain('Shared · Share'); + expect(tooltip.textContent).toContain('Shared · Revenue'); + expect(container.querySelectorAll('[data-snap-dots] circle')).toHaveLength(2); + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true })); + expect(tooltip.querySelectorAll('.oc-tooltip-row--emphasis')).toHaveLength(1); + expect(tooltip.querySelector('.oc-tooltip-row--emphasis')?.textContent).toContain('25%'); + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true })); + expect(tooltip.querySelectorAll('.oc-tooltip-row--emphasis')).toHaveLength(1); + expect(tooltip.querySelector('.oc-tooltip-row--emphasis')?.textContent).toContain('$1,250'); + const line = chart.layout.marks.find( + (item) => item.type === mark && item.data[0].name === 'Share', + ); + if (!line || (line.type !== 'line' && line.type !== 'area') || !line.dataPoints?.length) + throw new Error('Missing share series'); + const point = line.dataPoints[0]; + container + .querySelector('[data-voronoi-overlay]')! + .dispatchEvent(createMouseEvent('mousemove', point.x, point.y)); + expect(tooltip.querySelector('.oc-tooltip-row--emphasis')?.textContent).toContain('25%'); + } finally { + chart.destroy(); + } + }); +}); diff --git a/packages/vanilla/src/__tests__/lifecycle-contracts.test.ts b/packages/vanilla/src/__tests__/lifecycle-contracts.test.ts new file mode 100644 index 00000000..98352746 --- /dev/null +++ b/packages/vanilla/src/__tests__/lifecycle-contracts.test.ts @@ -0,0 +1,394 @@ +import type { + BarListSpec, + ChartSpec, + GeoMapSpec, + SankeySpec, + TableSpec, + TileMapSpec, +} from '@opendata-ai/openchart-core'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createBarList } from '../barlist-mount'; +import { createGeoMap } from '../map-mount'; +import { createChart } from '../mount'; +import { createSankey } from '../sankey-mount'; +import { createTable } from '../table-mount'; +import { createTileMap } from '../tilemap-mount'; + +const rows = [ + { name: 'Alpha', value: 10 }, + { name: 'Beta', value: 20 }, +]; +const chart: ChartSpec = { + mark: 'bar', + data: rows, + encoding: { x: { field: 'value', type: 'quantitative' }, y: { field: 'name', type: 'nominal' } }, +}; +const barlist: BarListSpec = { + type: 'barlist', + data: rows, + encoding: { + label: { field: 'name', type: 'nominal' }, + value: { field: 'value', type: 'quantitative' }, + }, +}; +const map: GeoMapSpec = { + type: 'map', + geo: { + projection: 'mercator', + features: { + type: 'Topology', + objects: { + regions: { + type: 'GeometryCollection', + geometries: [ + { type: 'Polygon', id: 'Alpha', properties: { name: 'Alpha' }, arcs: [[0]] }, + { type: 'Polygon', id: 'Beta', properties: { name: 'Beta' }, arcs: [[1]] }, + ], + }, + }, + arcs: [ + [ + [0, 0], + [0, 1], + [1, 1], + [1, 0], + [0, 0], + ], + [ + [2, 0], + [2, 1], + [3, 1], + [3, 0], + [2, 0], + ], + ], + }, + }, + data: rows, + encoding: { + key: { field: 'name', type: 'nominal' }, + color: { field: 'value', type: 'quantitative' }, + }, +}; +const tilemap: TileMapSpec = { + type: 'tilemap', + data: [ + { state: 'CA', value: 10 }, + { state: 'TX', value: 20 }, + ], + encoding: { + state: { field: 'state', type: 'nominal' }, + value: { field: 'value', type: 'quantitative' }, + }, +}; +const sankey: SankeySpec = { + type: 'sankey', + data: [ + { from: 'Alpha', to: 'Beta', value: 10 }, + { from: 'Beta', to: 'Gamma', value: 10 }, + ], + encoding: { + source: { field: 'from', type: 'nominal' }, + target: { field: 'to', type: 'nominal' }, + value: { field: 'value', type: 'quantitative' }, + }, +}; +const table: TableSpec = { + type: 'table', + data: rows, + rowKey: 'name', + columns: [ + { key: 'name', label: 'Name', sortable: true }, + { key: 'value', label: 'Value' }, + ], + search: true, +}; + +type Spec = ChartSpec | BarListSpec | GeoMapSpec | TileMapSpec | SankeySpec; +function fixture( + name: string, + create: ( + element: HTMLElement, + spec: S, + options: { responsive: false }, + ) => { update(spec: S): void; resize(): void; export(format: 'svg'): unknown; destroy(): void }, + spec: S, + selector: string, +) { + return { + name, + selector, + mount: (element: HTMLElement, animation: boolean) => { + const instance = create(element, { ...spec, animation }, { responsive: false }); + return { + ...instance, + invalid: () => instance.update({ ...spec, data: null } as unknown as S), + update: () => + instance.update({ ...spec, animation, chrome: { title: 'Updated evidence' } }), + svg: () => instance.export('svg') as string, + }; + }, + }; +} +const fixtures = [ + fixture('bar', createChart, chart, '.oc-mark-rect'), + fixture('bar list', createBarList, barlist, '.oc-barlist-row'), + fixture('map', createGeoMap, map, '.oc-map-feature'), + fixture('tile map', createTileMap, tilemap, '.oc-tilemap-tile'), + fixture('Sankey', createSankey, sankey, '.oc-sankey-node'), +]; + +function container() { + let width = 600; + const element = document.createElement('div'); + element.getBoundingClientRect = () => ({ + width, + height: 400, + x: 0, + y: 0, + left: 0, + top: 0, + right: width, + bottom: 400, + toJSON: () => ({}), + }); + document.body.append(element); + return { + element, + width: (value: number) => { + width = value; + }, + }; +} +afterEach(() => { + document.body.replaceChildren(); + vi.useRealTimers(); +}); + +describe.each(fixtures)('$name lifecycle contract', ({ mount, selector, name }) => { + it('keeps the last valid view and export usable after a rejected update', () => { + const host = container(); + const instance = mount(host.element, false); + const svg = host.element.querySelector('svg'); + const evidence = host.element.querySelectorAll(selector).length; + expect(() => instance.invalid()).toThrow(); + expect(host.element.querySelector('svg')).toBe(svg); + expect(instance.svg()).toContain(' { + vi.useFakeTimers(); + const host = container(); + const instance = mount(host.element, true); + host.width(390); + instance.resize(); + vi.advanceTimersByTime(5000); + expect(host.element.querySelector('svg')?.getAttribute('viewBox')?.split(' ')[2]).toBe('390'); + host.width(768); + instance.resize(); + expect(host.element.querySelector('svg')?.getAttribute('viewBox')?.split(' ')[2]).toBe('768'); + instance.update(); + expect(host.element.querySelector('svg')?.classList.contains('oc-animate')).toBe(false); + instance.destroy(); + }); + + if (name !== 'bar') { + it('supports inspection, activation and native Tab exit, preserving focus on resize', () => { + const host = container(); + const instance = mount(host.element, false); + const first = host.element.querySelector(selector)!; + first.focus(); + first.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true, cancelable: true }), + ); + const selectedKey = document.activeElement?.getAttribute('data-navigation-key'); + expect(selectedKey).toBeTruthy(); + expect(document.activeElement).not.toBe(first); + const tab = new KeyboardEvent('keydown', { key: 'Tab', bubbles: true, cancelable: true }); + document.activeElement?.dispatchEvent(tab); + expect(tab.defaultPrevented).toBe(false); + host.width(390); + instance.resize(); + expect(document.activeElement?.getAttribute('data-navigation-key')).toBe(selectedKey); + expect(host.element.querySelectorAll('[tabindex="0"][data-navigation-key]')).toHaveLength(1); + document.activeElement?.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), + ); + expect(host.element.querySelector('.oc-tooltip')?.style.display).toBe('none'); + instance.destroy(); + }); + } +}); + +it('Sankey callbacks work without tooltips and teardown clears active evidence', () => { + const host = container(); + const clicks: unknown[] = []; + const hovers: unknown[] = []; + const instance = createSankey(host.element, sankey, { + responsive: false, + tooltip: false, + onNodeClick: (node) => clicks.push(node), + onNodeHover: (node) => hovers.push(node), + }); + const node = host.element.querySelector('.oc-sankey-node')!; + node.focus(); + node.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + expect(clicks).toHaveLength(1); + expect(hovers.at(-1)).toEqual(clicks[0]); + instance.destroy(); + expect(hovers.at(-1)).toBeNull(); +}); + +it('exports a settled Sankey without transient hover dimming or changing the live view', () => { + const host = container(); + const instance = createSankey( + host.element, + { ...sankey, animation: true }, + { responsive: false }, + ); + const node = host.element.querySelector('.oc-sankey-node')!; + node.dispatchEvent(new MouseEvent('mouseenter')); + const before = host.element.querySelector('svg')!.outerHTML; + const exported = instance.export('svg') as string; + const parsed = new DOMParser().parseFromString(exported, 'image/svg+xml'); + expect(parsed.querySelector('.oc-animate')).toBeNull(); + const fills = [...parsed.querySelectorAll('.oc-sankey-link path')].map((el) => + Number(el.getAttribute('fill-opacity')), + ); + expect(fills).toEqual(instance.layout.links.map((link) => link.fillOpacity)); + expect(host.element.querySelector('svg')!.outerHTML).toBe(before); + instance.destroy(); +}); + +it('table Escape cancels queued typing and focuses the current body', () => { + vi.useFakeTimers(); + const host = container(); + const instance = createTable(host.element, table, { responsive: false }); + const input = host.element.querySelector('input')!; + input.focus(); + input.value = 'Alpha'; + input.dispatchEvent(new Event('input', { bubbles: true })); + input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); + vi.advanceTimersByTime(500); + expect(instance.getState().search).toBe(''); + expect(host.element.querySelectorAll('tbody tr')).toHaveLength(2); + expect(document.activeElement).toBe(host.element.querySelector('tbody')); + instance.destroy(); +}); + +it('table preserves the logical cell after reordering and rejects invalid updates atomically', () => { + const host = container(); + const instance = createTable(host.element, table, { responsive: false }); + host.element.querySelector('tbody')!.focus(); + instance.update({ ...table, data: [...rows].reverse() }); + expect(host.element.querySelector('.oc-table-cell-focus')?.textContent).toBe('Alpha'); + const view = host.element.querySelector('table'); + expect(() => instance.update({ ...table, data: null } as unknown as TableSpec)).toThrow(); + expect(host.element.querySelector('table')).toBe(view); + instance.resize(); + expect(host.element.querySelector('.oc-table-cell-focus')?.textContent).toBe('Alpha'); + instance.destroy(); +}); + +it('table animation does not permanently block responsive layout', () => { + vi.useFakeTimers(); + const host = container(); + const instance = createTable(host.element, { ...table, animation: true }, { responsive: false }); + host.width(390); + instance.resize(); + vi.advanceTimersByTime(5000); + expect(host.element.querySelector('.oc-table--cards')).not.toBeNull(); + host.width(768); + instance.resize(); + expect(host.element.querySelector('.oc-table--cards')).toBeNull(); + instance.destroy(); +}); + +it('table callbacks distinguish typed, duplicate and missing keys after sorting', () => { + const host = container(); + const data = [ + { id: 1, name: 'Numeric' }, + { id: '1', name: 'String' }, + { id: 'same', name: 'First duplicate' }, + { id: 'same', name: 'Second duplicate' }, + { name: 'Missing' }, + ]; + const selected: unknown[] = []; + const instance = createTable( + host.element, + { ...table, rowKey: 'id', data, columns: [{ key: 'name', label: 'Name', sortable: true }] }, + { responsive: false, onRowClick: (row) => selected.push(row) }, + ); + instance.setState({ sort: { column: 'name', direction: 'asc' } }); + const renderedRows = [...host.element.querySelectorAll('tbody tr')]; + expect(new Set(renderedRows.map((row) => row.getAttribute('data-row-id'))).size).toBe( + data.length, + ); + for (const row of renderedRows) row.dispatchEvent(new MouseEvent('click', { bubbles: true })); + expect(selected).toEqual([...data].sort((a, b) => a.name.localeCompare(b.name))); + instance.destroy(); +}); + +it('table CSV includes every filtered row even when the displayed table is paginated', () => { + const host = container(); + const instance = createTable( + host.element, + { ...table, pagination: { pageSize: 1 } }, + { responsive: false }, + ); + expect(host.element.querySelectorAll('tbody tr')).toHaveLength(1); + instance.setState({ page: 1, sort: { column: 'name', direction: 'desc' } }); + expect(instance.export('csv').split('\n')).toEqual(['Name,Value', 'Beta,20', 'Alpha,10']); + instance.destroy(); +}); + +it.each([ + 'line', + 'area', +] as const)('%s gaps survive intermediate motion, interruption and terminal export', (mark) => { + const host = container(); + const spec: ChartSpec = { + mark, + animation: { preset: 'fluid', enter: false }, + data: [ + { x: 0, y: 1 }, + { x: 1, y: 2 }, + { x: 2, y: null }, + { x: 3, y: 3 }, + { x: 4, y: 4 }, + ], + encoding: { x: { field: 'x', type: 'ordinal' }, y: { field: 'y', type: 'quantitative' } }, + }; + const instance = createChart(host.element, spec, { responsive: false }); + const next = { + ...spec, + data: spec.data.map((row) => ({ ...row, y: typeof row.y === 'number' ? row.y * 2 : row.y })), + }; + const motion = instance.beginManualUpdate(next)!; + expect(motion).not.toBeNull(); + motion.step(motion.totalMs / 2); + const selector = `.oc-mark-${mark}:not(.oc-ghost) path`; + const expected = instance.layout.marks.find((item) => item.type === mark); + if (!expected || (expected.type !== 'line' && expected.type !== 'area')) + throw new Error('Missing path mark'); + expect(expected.path.match(/M/g)).toHaveLength(2); + expect(host.element.querySelector(selector)?.getAttribute('d')).toBe(expected.path); + const interrupted = instance.beginManualUpdate(spec)!; + interrupted.step(interrupted.totalMs / 2); + const target = instance.layout.marks.find((item) => item.type === mark); + if (!target || (target.type !== 'line' && target.type !== 'area')) + throw new Error('Missing target'); + expect(host.element.querySelector(selector)?.getAttribute('d')).toBe(target.path); + interrupted.step(interrupted.totalMs); + expect(host.element.querySelector(selector)?.getAttribute('d')).toBe(target.path); + const exported = new DOMParser().parseFromString(instance.export('svg'), 'image/svg+xml'); + expect(exported.querySelector(selector)?.getAttribute('d')).toBe(target.path); + instance.destroy(); +}); diff --git a/packages/vanilla/src/__tests__/map-zoom.test.ts b/packages/vanilla/src/__tests__/map-zoom.test.ts index 33ebb674..5e6cf09c 100644 --- a/packages/vanilla/src/__tests__/map-zoom.test.ts +++ b/packages/vanilla/src/__tests__/map-zoom.test.ts @@ -540,6 +540,15 @@ describe('GeoMap reader zoom (geo.zoom)', () => { const ref = group().getAttribute('clip-path')!; expect(ref).toMatch(/^url\(#oc-map-zoom-clip-\d+\)$/); expect(container.querySelector(ref.slice(4, -1))).not.toBeNull(); + const exported = new DOMParser().parseFromString(inst.export('svg') as string, 'image/svg+xml'); + expect(exported.querySelector('.oc-map-group')?.getAttribute('clip-path')).toBe(ref); + const exportedClip = exported.querySelector(ref.slice(4, -1)); + expect(exportedClip?.querySelector('rect')?.getAttribute('width')).toBe( + String(inst.layout.area.width), + ); + expect(exportedClip?.querySelector('rect')?.getAttribute('height')).toBe( + String(inst.layout.area.height), + ); button(container, 'reset').click(); expect(group().hasAttribute('clip-path')).toBe(false); inst.destroy(); diff --git a/packages/vanilla/src/__tests__/reader.test.ts b/packages/vanilla/src/__tests__/reader.test.ts new file mode 100644 index 00000000..09322cfb --- /dev/null +++ b/packages/vanilla/src/__tests__/reader.test.ts @@ -0,0 +1,204 @@ +import type { ChartSpec, LayerSpec, ReaderSelection } from '@opendata-ai/openchart-core'; +import { afterEach, describe, expect, it } from 'vitest'; +import { createContainer } from '../__test-fixtures__/dom'; +import { createChart } from '../mount'; +import { createReader } from '../reader'; + +const spec: ChartSpec = { + mark: 'bar', + animation: false, + data: [ + { id: 'a', name: 'Alpha', value: 0 }, + { id: 'b', name: 'Beta', value: 80 }, + ], + encoding: { + key: { field: 'id', type: 'nominal' }, + x: { field: 'name', type: 'nominal', sort: null }, + y: { field: 'value', type: 'quantitative', scale: { domain: [0, 100] } }, + }, +}; +const cleanup: Array<() => void> = []; +afterEach(() => { + for (const destroy of cleanup.splice(0).reverse()) destroy(); + document.body.innerHTML = ''; +}); + +function mount(initial: ChartSpec | LayerSpec = spec) { + const container = createContainer(); + const chart = createChart(container, initial); + cleanup.push(() => chart.destroy()); + const changes: ReaderSelection[] = []; + const reader = createReader(container, chart, initial, (selection) => changes.push(selection)); + cleanup.push(() => reader.destroy()); + const update = (next: ChartSpec | LayerSpec, visibleKeys?: ReadonlySet) => { + chart.update(next); + reader.update(next, visibleKeys); + }; + return { container, chart, reader, changes, update }; +} + +describe('reader identity and visibility on compiled charts', () => { + it('retains a zero-value pin and marks it filtered when a transform hides its row', () => { + const { reader, update } = mount(); + reader.pin(spec.data[0]); + expect(reader.selection.pinned[0]).toMatchObject({ observation: 'string:a', state: 'visible' }); + + update({ ...spec, transform: [{ filter: { field: 'id', equal: 'b' } }] }); + expect(reader.selection.pinned[0]).toMatchObject({ + observation: 'string:a', + datum: spec.data[0], + state: 'filtered', + }); + + update(spec); + expect(reader.selection.pinned[0]).toMatchObject({ observation: 'string:a', state: 'visible' }); + }); + + it('resolves transformed copies through unique typed source keys', () => { + const mixed: ChartSpec = { + ...spec, + data: [ + { id: 1, name: 'Numeric', value: 10 }, + { id: '1', name: 'Text', value: 20 }, + ], + }; + const { reader, update } = mount(mixed); + reader.pin(mixed.data[0]); + reader.pin(mixed.data[1]); + update({ + ...mixed, + transform: [ + { calculate: { op: '*', field: 'value', value: 2 }, as: 'doubled' }, + { filter: { field: 'name', equal: 'Text' } }, + ], + }); + expect( + reader.selection.pinned.map(({ observation, state }) => ({ observation, state })), + ).toEqual([ + { observation: 'number:1', state: 'filtered' }, + { observation: 'string:1', state: 'visible' }, + ]); + }); + + it('preserves duplicate-key row identity within a revision and clears it on replacement', () => { + const duplicate: ChartSpec = { + ...spec, + data: spec.data.map((row) => ({ ...row, id: 'same' })), + }; + const { reader, update } = mount(duplicate); + reader.pin(duplicate.data[1]); + update({ ...duplicate, transform: [{ filter: { field: 'name', equal: 'Alpha' } }] }); + expect(reader.selection.pinned[0]).toMatchObject({ + datum: duplicate.data[1], + persistent: false, + state: 'filtered', + }); + update(duplicate); + expect(reader.selection.pinned[0]).toMatchObject({ + datum: duplicate.data[1], + state: 'visible', + }); + update({ ...duplicate, data: duplicate.data.map((row) => ({ ...row })) }); + expect(reader.selection.pinned).toEqual([]); + expect(reader.selection.announcement).toContain('unique key'); + }); + + it('does not bind an ambiguous copied row to the first duplicate observation', () => { + const duplicate: ChartSpec = { + ...spec, + data: [spec.data[1], { ...spec.data[1] }], + }; + const { reader } = mount(duplicate); + reader.pin({ ...spec.data[1] }); + expect(reader.selection.pinned).toEqual([]); + }); + + it('keeps distinct identical rows available to keyboard inspection within their revision', () => { + const duplicate: ChartSpec = { ...spec, data: [{ ...spec.data[1] }, { ...spec.data[1] }] }; + const { container, reader } = mount(duplicate); + for (let i = 0; i < 2; i++) { + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + } + expect(reader.selection.pinned.map(({ observation }) => observation)).toEqual([ + 'revision-row:0', + 'revision-row:1', + ]); + }); + + it('refreshes series identity without losing a pinned row when the color field changes', () => { + const grouped: ChartSpec = { + ...spec, + data: spec.data.map((row) => ({ ...row, group: 'Before', alternate: 'After' })), + encoding: { ...spec.encoding, color: { field: 'group', type: 'nominal' } }, + }; + const { reader, update } = mount(grouped); + reader.pin(grouped.data[1]); + expect(reader.selection.pinned[0].series).toBe('string:Before'); + update({ + ...grouped, + encoding: { ...grouped.encoding, color: { field: 'alternate', type: 'nominal' } }, + }); + expect(reader.selection.pinned[0]).toMatchObject({ + observation: 'string:b', + series: 'string:After', + state: 'visible', + }); + }); + + it('respects an explicit host visibility set and otherwise restores compiled visibility', () => { + const { reader, update } = mount(); + reader.pin(spec.data[1]); + update(spec, new Set(['string:a'])); + expect(reader.selection.pinned[0].state).toBe('filtered'); + update(spec); + expect(reader.selection.pinned[0].state).toBe('visible'); + }); + + it('uses destination visibility while a removed mark is still transitioning out', () => { + const animated: ChartSpec = { + ...spec, + animation: { preset: 'fluid', enter: false }, + encoding: { + ...spec.encoding, + y: { field: 'value', type: 'quantitative', scale: { domain: [0, 100], clip: true } }, + }, + }; + const { chart, reader } = mount(animated); + reader.pin(animated.data[1]); + const filtered: ChartSpec = { + ...animated, + encoding: { + ...animated.encoding, + y: { field: 'value', type: 'quantitative', scale: { domain: [0, 50], clip: true } }, + }, + }; + const transition = chart.beginManualUpdate(filtered); + expect(transition).not.toBeNull(); + transition!.step(50); + reader.update(filtered); + expect(reader.selection.pinned[0]).toMatchObject({ + observation: 'string:b', + state: 'filtered', + }); + transition!.cancel(); + }); + + it('handles layers explicitly without choosing an arbitrary child dataset', () => { + const layered: LayerSpec = { layer: [spec, { ...spec, mark: 'point' }] }; + const { container, reader, changes, update } = mount(layered); + expect(changes.at(-1)?.announcement).toMatch(/layered charts/i); + reader.pin(spec.data[1]); + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + container.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + expect(reader.selection.pinned).toEqual([]); + expect(reader.selection.preview).toBeNull(); + + update(spec); + reader.pin(spec.data[1]); + expect(reader.selection.pinned[0].observation).toBe('string:b'); + update(layered); + expect(reader.selection.pinned).toEqual([]); + expect(reader.selection.announcement).toMatch(/layered charts/i); + }); +}); diff --git a/packages/vanilla/src/__tests__/table-keyboard.test.ts b/packages/vanilla/src/__tests__/table-keyboard.test.ts index f6edabbe..4cb71069 100644 --- a/packages/vanilla/src/__tests__/table-keyboard.test.ts +++ b/packages/vanilla/src/__tests__/table-keyboard.test.ts @@ -147,7 +147,7 @@ describe('tbody arrow key navigation', () => { it('focus highlights first cell and sets aria-activedescendant', () => { const tbody = focusTbody(wrapper); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-0'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-0$/); const cell = wrapper.querySelector('.oc-table-cell-focus'); expect(cell).not.toBeNull(); expect(cell?.textContent).toBe('R0C0'); @@ -156,25 +156,25 @@ describe('tbody arrow key navigation', () => { it('ArrowDown moves focus down one row', () => { const tbody = focusTbody(wrapper); keydown(tbody, 'ArrowDown'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-1-0'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-1-0$/); }); it('ArrowDown does not go past last row', () => { const tbody = focusTbody(wrapper); for (let i = 0; i < 10; i++) keydown(tbody, 'ArrowDown'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-4-0'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-4-0$/); }); it('ArrowRight moves focus right one column', () => { const tbody = focusTbody(wrapper); keydown(tbody, 'ArrowRight'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-1'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-1$/); }); it('ArrowRight does not go past last column', () => { const tbody = focusTbody(wrapper); for (let i = 0; i < 10; i++) keydown(tbody, 'ArrowRight'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-2'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-2$/); }); it('ArrowLeft moves focus left', () => { @@ -182,13 +182,13 @@ describe('tbody arrow key navigation', () => { keydown(tbody, 'ArrowRight'); keydown(tbody, 'ArrowRight'); keydown(tbody, 'ArrowLeft'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-1'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-1$/); }); it('ArrowLeft does not go past first column', () => { const tbody = focusTbody(wrapper); keydown(tbody, 'ArrowLeft'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-0'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-0$/); }); it('Home moves to first column in current row', () => { @@ -196,13 +196,13 @@ describe('tbody arrow key navigation', () => { keydown(tbody, 'ArrowRight'); keydown(tbody, 'ArrowRight'); keydown(tbody, 'Home'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-0'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-0$/); }); it('End moves to last column in current row', () => { const tbody = focusTbody(wrapper); keydown(tbody, 'End'); - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-2'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-2$/); }); it('navigation clears previous focus highlight', () => { @@ -274,7 +274,7 @@ describe('header keyboard navigation', () => { keydown(headers[1] as HTMLElement, 'ArrowDown'); const tbody = wrapper.querySelector('tbody')!; // The tbody should have activedescendant set to first row, column 1 - expect(tbody.getAttribute('aria-activedescendant')).toBe('oc-cell-0-1'); + expect(tbody.getAttribute('aria-activedescendant')).toMatch(/-cell-0-1$/); }); }); diff --git a/packages/vanilla/src/__tests__/transition.test.ts b/packages/vanilla/src/__tests__/transition.test.ts index 57b7ad8b..3b8a87df 100644 --- a/packages/vanilla/src/__tests__/transition.test.ts +++ b/packages/vanilla/src/__tests__/transition.test.ts @@ -1604,6 +1604,40 @@ const DATA_D = [ ]; describe('interruption retargeting', () => { + it('a rejected edit cannot leak interrupted area geometry into a later update', () => { + const specA = noEnter(areaSpec(LINE_DATA_A)); + const specB = noEnter(areaSpec(LINE_DATA_B)); + const container = createContainer(); + const chart = createChart(container, specA); + const areaPath = () => + container.querySelector('.oc-mark-area:not(.oc-ghost) path')?.getAttribute('d'); + + try { + chart.update(specB); + pumpRaf(0); + pumpRaf(150); + const interruptedPath = areaPath(); + expect(() => chart.update({ ...specB, data: null } as unknown as ChartSpec)).toThrow(); + expect(areaPath()).toBe(interruptedPath); + + // The rejected edit leaves the original animation free to settle. + pumpRaf(2000); + const settledPath = areaPath(); + expect(settledPath).not.toBe(interruptedPath); + + chart.update(specA); + // A new transition must start at the last settled scene, without + // resurfacing a ghost captured by the rejected edit. + expect(areaPath()).toBe(settledPath); + expect(container.querySelector('.oc-mark-area.oc-ghost')).toBeNull(); + runToCompletion(); + const { svg } = compileAndRender(specA); + expect(areaPath()).toBe(svg.querySelector('.oc-mark-area path')?.getAttribute('d')); + } finally { + chart.destroy(); + } + }); + it('A -> B -> interrupt at ~50% -> C -> complete -> equals fresh render of C', () => { const specA = noEnter(columnSpec(DATA_A)); const specB = noEnter(columnSpec(DATA_B)); diff --git a/packages/vanilla/src/animation.ts b/packages/vanilla/src/animation.ts index 2d82d210..8b3b5d94 100644 --- a/packages/vanilla/src/animation.ts +++ b/packages/vanilla/src/animation.ts @@ -77,7 +77,10 @@ export function setupAnimationCleanup( * Same timeout-based approach as chart animations: compute total time from * CSS custom properties and row count, then remove oc-animate after completion. */ -export function setupTableAnimationCleanup(wrapper: HTMLElement): () => void { +export function setupTableAnimationCleanup( + wrapper: HTMLElement, + onComplete?: () => void, +): () => void { const style = wrapper.style; const duration = parseFloat(style.getPropertyValue('--oc-animation-duration')) || 500; const stagger = parseFloat(style.getPropertyValue('--oc-animation-stagger')) || 0; @@ -90,6 +93,7 @@ export function setupTableAnimationCleanup(wrapper: HTMLElement): () => void { const timer = setTimeout(() => { wrapper.classList.remove('oc-animate'); + onComplete?.(); }, totalTime); return () => { diff --git a/packages/vanilla/src/barlist-mount.ts b/packages/vanilla/src/barlist-mount.ts index e04a10f1..42da96fe 100644 --- a/packages/vanilla/src/barlist-mount.ts +++ b/packages/vanilla/src/barlist-mount.ts @@ -24,6 +24,12 @@ import { type JPGExportOptions, type SVGExportOptions, } from './export'; +import { + captureMarkFocus, + type NavigableMark, + restoreMarkFocus, + wireMarkNavigation, +} from './interactions/mark-navigation'; import { createMeasureText, resolveFontFamily, scheduleFontReload } from './measure-text'; import { observeResize } from './resize-observer'; import { resolveDarkMode } from './resolve-dark-mode'; @@ -90,10 +96,10 @@ export function createBarList( // { ...spec.theme, ...options.theme }, so options.theme wins over the // spec-level theme; fall back to the container's computed font. Measuring a // different font than gets rendered desyncs layout metrics and the reload watcher. - function resolveEffectiveFont(): string { + function resolveEffectiveFont(candidate = currentSpec): string { return ( options?.theme?.fonts?.family ?? - currentSpec.theme?.fonts?.family ?? + candidate.theme?.fonts?.family ?? resolveFontFamily(container) ); } @@ -109,7 +115,7 @@ export function createBarList( }; } - function compile(): BarListLayout { + function compile(candidate = currentSpec, measure = measureText): BarListLayout { const { width, height } = getContainerDimensions(); const darkMode = resolveDarkMode(options?.darkMode); @@ -119,10 +125,10 @@ export function createBarList( theme: options?.theme, darkMode, watermark: options?.watermark, - measureText, + measureText: measure, }; - const layout = compileBarList(currentSpec, compileOpts); + const layout = compileBarList(candidate, compileOpts); // Auto-size height to content so few-row lists don't leave empty space. if (layout.rows.length > 0) { @@ -141,6 +147,8 @@ export function createBarList( function wireTooltipAndInteraction(svg: SVGSVGElement, layout: BarListLayout): () => void { const cleanups: Array<() => void> = []; + const navigation: NavigableMark[] = []; + let activeLeave: (() => void) | null = null; const rowElements = svg.querySelectorAll('.oc-barlist-row'); for (const el of rowElements) { @@ -151,6 +159,8 @@ export function createBarList( const row = layout.rows[Number(indexStr)]; const handleMouseEnter = (e: Event) => { + activeLeave?.(); + activeLeave = handleMouseLeave; const mouseEvent = e as MouseEvent; if (content && tooltipManager && options?.tooltip !== false) { const svgRect = svg.getBoundingClientRect(); @@ -178,6 +188,8 @@ export function createBarList( }; const handleMouseLeave = () => { + if (activeLeave !== handleMouseLeave) return; + activeLeave = null; tooltipManager?.hide(); options?.onRowHover?.(null); }; @@ -192,6 +204,13 @@ export function createBarList( } }; + navigation.push({ + element: el, + key: row?.label.text ?? indexStr, + enter: handleMouseEnter, + leave: handleMouseLeave, + activate: handleClick, + }); el.addEventListener('mouseenter', handleMouseEnter); el.addEventListener('mousemove', handleMouseMove); el.addEventListener('mouseleave', handleMouseLeave); @@ -205,12 +224,16 @@ export function createBarList( }); } + cleanups.push(wireMarkNavigation(svg, navigation)); return () => { + activeLeave?.(); for (const cleanup of cleanups) cleanup(); }; } function render(animate = false): void { + const focusedMark = captureMarkFocus(svgElement); + tooltipManager?.hide(); if (animationCleanup) { animationCleanup(); animationCleanup = null; @@ -236,7 +259,7 @@ export function createBarList( } } - if (currentLayout.animation?.enter) { + if (animate && currentLayout.animation?.enter) { animationCleanup = setupAnimationCleanup(newSvg, () => { // Null the handle BEFORE replaying, or the deferred resize hits the // entrance-in-flight gate and re-defers forever (mount.ts pattern). @@ -258,18 +281,21 @@ export function createBarList( fontsReloadPending = false; container.dataset.ocFontsState = 'ready'; } + if (svgElement) restoreMarkFocus(svgElement, focusedMark); } function update(newSpec: BarListSpec): void { + if (destroyed) return; + // Compile before adopting an edit: a rejected spec must leave the last + // valid view, its interactions, and future resize/export calls intact. + const nextFont = resolveEffectiveFont(newSpec); + const nextMeasure = nextFont === fontFamily ? measureText : createMeasureText(nextFont); + const nextLayout = compile(newSpec, nextMeasure); currentSpec = newSpec; - // A new spec can change theme.fonts.family; rebuild the measurer so layout - // measures the font compile will actually render with. - const nextFont = resolveEffectiveFont(); - if (nextFont !== fontFamily) { - fontFamily = nextFont; - measureText = createMeasureText(fontFamily); - } - currentLayout = compile(); + fontFamily = nextFont; + measureText = nextMeasure; + + currentLayout = nextLayout; render(); } @@ -288,15 +314,19 @@ export function createBarList( options_?: JPGExportOptions | SVGExportOptions, ): string | Promise | Promise { if (!svgElement) return ''; + // Export settled geometry, independent of transient hover and animation. + const terminalSvg = renderBarListSVG(currentLayout, { animate: false }); + const resolvedOptions = { theme: currentLayout.theme, ...options_ }; + switch (format) { case 'svg': - return exportSVG(svgElement); + return exportSVG(terminalSvg); case 'svg-with-fonts': - return exportSVGWithFonts(svgElement); + return exportSVGWithFonts(terminalSvg, resolvedOptions); case 'png': - return exportPNG(svgElement, options_ as JPGExportOptions); + return exportPNG(terminalSvg, resolvedOptions); case 'jpg': - return exportJPG(svgElement, options_ as JPGExportOptions); + return exportJPG(terminalSvg, resolvedOptions); default: return ''; } diff --git a/packages/vanilla/src/barlist-renderer.ts b/packages/vanilla/src/barlist-renderer.ts index a214c063..a6fd3aea 100644 --- a/packages/vanilla/src/barlist-renderer.ts +++ b/packages/vanilla/src/barlist-renderer.ts @@ -6,8 +6,8 @@ */ import type { BarListLayout, BarListRowMark, ResolvedAnimation } from '@opendata-ai/openchart-core'; -import { textAscent } from '@opendata-ai/openchart-core'; import { stampAnimationVars } from './animation-vars'; +import { renderChromeElement } from './renderers/chrome'; const SVG_NS = 'http://www.w3.org/2000/svg'; const XLINK_NS = 'http://www.w3.org/1999/xlink'; @@ -43,20 +43,13 @@ function renderChrome(parent: SVGElement, layout: BarListLayout): void { if (!el) continue; const isBottom = key === 'source' || key === 'byline' || key === 'footer'; - const text = createSVGElement('text'); - // el.y is the TOP of the text box; convert to the alphabetic baseline - // (WebKit mishandles dominant-baseline:hanging, see renderers/chrome.ts). - setAttrs(text, { - x: el.x, - y: (isBottom ? bottomOffset + el.y : el.y) + textAscent(el.style.fontSize), - }); - text.setAttribute('class', `oc-${key}`); - text.setAttribute('font-family', el.style.fontFamily); - text.setAttribute('font-size', String(el.style.fontSize)); - text.setAttribute('font-weight', String(el.style.fontWeight)); - (text as SVGElement & ElementCSSInlineStyle).style.setProperty('fill', el.style.fill); - text.textContent = el.text; - g.appendChild(text); + renderChromeElement( + g, + isBottom ? { ...el, y: bottomOffset + el.y } : el, + `oc-${key}`, + key, + layout.measureText, + ); } parent.appendChild(g); diff --git a/packages/vanilla/src/export.ts b/packages/vanilla/src/export.ts index 6d2747ec..1b4fe40b 100644 --- a/packages/vanilla/src/export.ts +++ b/packages/vanilla/src/export.ts @@ -89,15 +89,40 @@ export function ensureSVGDimensions(svgString: string, width: number, height: nu // --------------------------------------------------------------------------- /** - * Collect unique font-family + font-weight combos from all elements. + * Resolve an authored font declaration, including SVG inheritance. Inline + * styles win over presentation attributes, as they do in the live renderer. + * This also works on detached terminal scenes used for exports. */ +function inheritedFontProperty(element: SVGElement, root: SVGElement, property: string): string { + let current: SVGElement | null = element; + while (current) { + const value = current.style.getPropertyValue(property) || current.getAttribute(property); + const keyword = value?.trim().toLowerCase(); + // Unlike inheritance, initial resets to normal weight or the browser's + // default family, which has no authored font face to embed. + if (keyword === 'initial') return property === 'font-weight' ? '400' : ''; + if (value && keyword !== 'inherit' && keyword !== 'unset') return value; + if (current === root) break; + current = current.parentElement as SVGElement | null; + } + return ''; +} + +function normalizeFontWeight(weight: string): string { + const value = weight.trim(); + return value === 'normal' ? '400' : value === 'bold' ? '700' : value; +} + +/** Collect font families and weights, including styled spans within labels. */ function collectUsedFonts(svgElement: SVGElement): Map> { const fonts = new Map>(); - const textElements = svgElement.querySelectorAll('text'); + const textElements = svgElement.querySelectorAll('text, tspan'); for (const el of textElements) { - const family = el.getAttribute('font-family'); - const weight = el.getAttribute('font-weight') || '400'; + const family = inheritedFontProperty(el, svgElement, 'font-family'); + const weight = normalizeFontWeight( + inheritedFontProperty(el, svgElement, 'font-weight') || '400', + ); if (family) { // Take the first font in the stack (e.g., "Inter, sans-serif" → "Inter") const primary = family.split(',')[0].trim().replace(/["']/g, ''); @@ -138,7 +163,7 @@ function findFontFaceRules( if (!(rule instanceof CSSFontFaceRule)) continue; const familyRaw = rule.style.getPropertyValue('font-family').replace(/["']/g, '').trim(); - const weight = rule.style.getPropertyValue('font-weight') || '400'; + const weight = normalizeFontWeight(rule.style.getPropertyValue('font-weight') || '400'); const style = rule.style.getPropertyValue('font-style') || 'normal'; const src = rule.style.getPropertyValue('src'); diff --git a/packages/vanilla/src/graph-mount.ts b/packages/vanilla/src/graph-mount.ts index 9c1b32e9..70538846 100644 --- a/packages/vanilla/src/graph-mount.ts +++ b/packages/vanilla/src/graph-mount.ts @@ -1397,6 +1397,8 @@ export function createGraph( getAdjacency: () => adjacencyMap, onSelect(nodeId) { selectedNodeIds = new Set([nodeId]); + interactionManager?.setSelection([nodeId]); + armFocus(performance.now()); needsRender = true; scheduleRender(); options?.onNodeClick?.(nodeDataById(nodeId)); @@ -1404,6 +1406,8 @@ export function createGraph( }, onDeselect() { selectedNodeIds.clear(); + interactionManager?.setSelection([]); + armFocus(performance.now()); needsRender = true; scheduleRender(); options?.onSelectionChange?.([]); diff --git a/packages/vanilla/src/graph/__tests__/keyboard.test.ts b/packages/vanilla/src/graph/__tests__/keyboard.test.ts index 5ea18b72..3995c860 100644 --- a/packages/vanilla/src/graph/__tests__/keyboard.test.ts +++ b/packages/vanilla/src/graph/__tests__/keyboard.test.ts @@ -121,15 +121,21 @@ describe('attachGraphKeyboardNav', () => { }); // ----------------------------------------------------------------------- - // Tab focus + // Focus entry // ----------------------------------------------------------------------- - describe('Tab', () => { + describe('focus entry and exit', () => { + it.each([false, true])('never intercepts Tab (shift=%s)', (shiftKey) => { + const event = new KeyboardEvent('keydown', { key: 'Tab', shiftKey, cancelable: true }); + canvas.dispatchEvent(event); + expect(event.defaultPrevented).toBe(false); + expect(callbacks.onSelect).not.toHaveBeenCalled(); + }); it('focuses the first node when none selected', () => { - // Tab should set internal focus to first node (center). It does not + // Focus should set internal focus to first node (center). It does not // call onSelect on its own -- the focus is internal. Arrow keys after // Tab will use the focused node for navigation. - keydown(canvas, 'Tab'); + canvas.dispatchEvent(new FocusEvent('focus')); // Verify focus by pressing ArrowRight, which should navigate from // center to right and call onSelect('right'). @@ -147,7 +153,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); // Now focused on 'left'. ArrowRight from left should navigate to center. keydown(c, 'ArrowRight'); expect(cbs.onSelect).toHaveBeenCalledWith('center'); @@ -165,7 +171,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); // No error, no callbacks fired expect(cbs.onSelect).not.toHaveBeenCalled(); @@ -178,9 +184,9 @@ describe('attachGraphKeyboardNav', () => { // ----------------------------------------------------------------------- describe('ArrowKey navigation', () => { - // First Tab to set focus to center, then test arrow keys + // First focus to set the starting node to center, then test arrow keys function focusCenter(): void { - keydown(canvas, 'Tab'); + canvas.dispatchEvent(new FocusEvent('focus')); } it('ArrowRight navigates to the right neighbor', () => { @@ -207,10 +213,9 @@ describe('attachGraphKeyboardNav', () => { expect(callbacks.onSelect).toHaveBeenCalledWith('down'); }); - it('does nothing when no node is focused', () => { - // No Tab pressed, so no focus. Arrow keys should be no-ops. + it('navigates on the first arrow without requiring a Tab keydown', () => { keydown(canvas, 'ArrowRight'); - expect(callbacks.onSelect).not.toHaveBeenCalled(); + expect(callbacks.onSelect).toHaveBeenCalledWith('right'); }); it('does nothing when there are no neighbors', () => { @@ -228,7 +233,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowRight'); expect(cbs.onSelect).not.toHaveBeenCalled(); @@ -250,7 +255,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowRight'); expect(cbs.onSelect).not.toHaveBeenCalled(); @@ -264,7 +269,7 @@ describe('attachGraphKeyboardNav', () => { describe('Enter', () => { it('selects the focused node when not already selected', () => { - keydown(canvas, 'Tab'); // focus center + canvas.dispatchEvent(new FocusEvent('focus')); // focus center keydown(canvas, 'Enter'); expect(callbacks.onSelect).toHaveBeenCalledWith('center'); }); @@ -279,7 +284,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); // focus goes to selected node 'center' + c.dispatchEvent(new FocusEvent('focus')); // focus goes to selected node 'center' keydown(c, 'Enter'); expect(cbs.onDeselect).toHaveBeenCalled(); expect(cbs.onSelect).not.toHaveBeenCalled(); @@ -287,9 +292,9 @@ describe('attachGraphKeyboardNav', () => { cl(); }); - it('does nothing when no node is focused', () => { + it('selects the first node on Enter without a prior Tab keydown', () => { keydown(canvas, 'Enter'); - expect(callbacks.onSelect).not.toHaveBeenCalled(); + expect(callbacks.onSelect).toHaveBeenCalledWith('center'); expect(callbacks.onDeselect).not.toHaveBeenCalled(); }); }); @@ -300,14 +305,14 @@ describe('attachGraphKeyboardNav', () => { describe('Escape', () => { it('clears focus and calls onDeselect', () => { - keydown(canvas, 'Tab'); // focus center + canvas.dispatchEvent(new FocusEvent('focus')); // focus center keydown(canvas, 'Escape'); expect(callbacks.onDeselect).toHaveBeenCalled(); - // Focus is cleared, so arrow keys should do nothing + // Navigation can resume immediately after dismissal. keydown(canvas, 'ArrowRight'); - expect(callbacks.onSelect).not.toHaveBeenCalled(); + expect(callbacks.onSelect).toHaveBeenCalledWith('right'); }); }); @@ -440,7 +445,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowRight'); expect(cbs.onSelect).toHaveBeenCalledWith('up-right'); @@ -468,7 +473,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowLeft'); expect(cbs.onSelect).toHaveBeenCalledWith('far-left'); @@ -496,7 +501,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowDown'); expect(cbs.onSelect).toHaveBeenCalledWith('below'); @@ -524,7 +529,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowUp'); expect(cbs.onSelect).toHaveBeenCalledWith('above'); @@ -553,7 +558,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowRight'); // far-right has higher score (275 > 100) due to larger dx expect(cbs.onSelect).toHaveBeenCalledWith('far-right'); @@ -583,7 +588,7 @@ describe('attachGraphKeyboardNav', () => { }); const cl = attachGraphKeyboardNav(options); - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); keydown(c, 'ArrowRight'); // aligned (80) beats offset (0) because the perpendicular penalty kicks in expect(cbs.onSelect).toHaveBeenCalledWith('aligned'); @@ -597,17 +602,17 @@ describe('attachGraphKeyboardNav', () => { // ----------------------------------------------------------------------- describe('edge cases', () => { - it('Tab keeps focus if the focused node still exists', () => { + it('Focus entry keeps focus if the focused node still exists', () => { // Tab once to focus center - keydown(canvas, 'Tab'); + canvas.dispatchEvent(new FocusEvent('focus')); // Tab again -- focusedNodeId is still valid, should keep it - keydown(canvas, 'Tab'); + canvas.dispatchEvent(new FocusEvent('focus')); // Arrow right from center should still go to right keydown(canvas, 'ArrowRight'); expect(callbacks.onSelect).toHaveBeenCalledWith('right'); }); - it('Tab refocuses to first node if focused node was removed', () => { + it('Focus entry refocuses to first node if focused node was removed', () => { const mutableNodes = [...makeCrossNodes()]; const { options, @@ -619,7 +624,7 @@ describe('attachGraphKeyboardNav', () => { const cl = attachGraphKeyboardNav(options); // Tab to focus center (first node) - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); // Navigate right to focus on 'right' keydown(c, 'ArrowRight'); expect(cbs.onSelect).toHaveBeenCalledWith('right'); @@ -630,7 +635,7 @@ describe('attachGraphKeyboardNav', () => { // Tab again -- focused node 'right' no longer exists via findNodeById, // so Tab falls through to refocus on nodes[0] which is 'center' - keydown(c, 'Tab'); + c.dispatchEvent(new FocusEvent('focus')); // Enter to confirm we're focused on center keydown(c, 'Enter'); @@ -641,7 +646,7 @@ describe('attachGraphKeyboardNav', () => { it('sequential navigation updates focus correctly', () => { // Tab to center, then right, then the focus should be on 'right' - keydown(canvas, 'Tab'); + canvas.dispatchEvent(new FocusEvent('focus')); keydown(canvas, 'ArrowRight'); expect(callbacks.onSelect).toHaveBeenCalledWith('right'); diff --git a/packages/vanilla/src/graph/keyboard.ts b/packages/vanilla/src/graph/keyboard.ts index 3eaff468..6e372c83 100644 --- a/packages/vanilla/src/graph/keyboard.ts +++ b/packages/vanilla/src/graph/keyboard.ts @@ -56,6 +56,16 @@ export function attachGraphKeyboardNav(options: KeyboardNavOptions): () => void return getNodes().find((n) => n.id === id); } + function ensureFocus(): void { + if (focusedNodeId && findNodeById(focusedNodeId)) return; + focusedNodeId = getSelectedIds().find((id) => findNodeById(id)) ?? getNodes()[0]?.id ?? null; + } + + function onFocus(): void { + focusedNodeId = getSelectedIds().find((id) => findNodeById(id)) ?? focusedNodeId; + ensureFocus(); + } + /** * Given a set of neighbor node ids, pick the one that best matches * the arrow key direction relative to the current focused node. @@ -103,23 +113,11 @@ export function attachGraphKeyboardNav(options: KeyboardNavOptions): () => void } function onKeyDown(e: KeyboardEvent): void { + // Tab belongs to the page. Node navigation starts when the canvas receives + // focus, and recovers if the previously focused node was removed. + if (e.key === 'Tab') return; + ensureFocus(); switch (e.key) { - case 'Tab': { - // Focus first/selected node - const selected = getSelectedIds(); - const nodes = getNodes(); - if (nodes.length === 0) return; - - if (selected.length > 0) { - focusedNodeId = selected[0]; - } else if (!focusedNodeId || !findNodeById(focusedNodeId)) { - focusedNodeId = nodes[0].id; - } - - e.preventDefault(); - break; - } - case 'ArrowUp': case 'ArrowDown': case 'ArrowLeft': @@ -200,9 +198,11 @@ export function attachGraphKeyboardNav(options: KeyboardNavOptions): () => void } canvas.addEventListener('keydown', onKeyDown); + canvas.addEventListener('focus', onFocus); // Return cleanup function return () => { canvas.removeEventListener('keydown', onKeyDown); + canvas.removeEventListener('focus', onFocus); }; } diff --git a/packages/vanilla/src/interactions/chart-events.ts b/packages/vanilla/src/interactions/chart-events.ts index e83a628c..b807495c 100644 --- a/packages/vanilla/src/interactions/chart-events.ts +++ b/packages/vanilla/src/interactions/chart-events.ts @@ -13,13 +13,13 @@ function buildMarkDataMap( const mark = layout.marks[i]; switch (mark.type) { case 'line': - map.set(`line-${mark.seriesKey ?? i}`, { + map.set(`line-${i}`, { datum: mark.data[0] ?? {}, series: mark.seriesKey, }); break; case 'area': - map.set(`area-${mark.seriesKey ?? i}`, { + map.set(`area-${i}`, { datum: mark.data[0] ?? {}, series: mark.seriesKey, }); diff --git a/packages/vanilla/src/interactions/crosshair.ts b/packages/vanilla/src/interactions/crosshair.ts index a256ce00..ca876122 100644 --- a/packages/vanilla/src/interactions/crosshair.ts +++ b/packages/vanilla/src/interactions/crosshair.ts @@ -11,7 +11,9 @@ interface SeriesPoint { } interface SeriesGroup { + key: string; seriesKey: string; + markId: string; color: string; pointsByX: Map; } @@ -56,7 +58,7 @@ function snapKey(x: number): number { } function collectSeriesGroups(layout: ChartLayout): SeriesGroup[] { - // Dedupe by seriesKey: area charts emit BOTH an AreaMark and a derived + // Dedupe within each leaf: area charts emit BOTH an AreaMark and a derived // LineMark per series. Without dedupe, single-series area charts show // "line-0" / "area-1" in the tooltip instead of the actual data fields. // Prefer the line mark (same logic as endpoint-labels). @@ -65,7 +67,12 @@ function collectSeriesGroups(layout: ChartLayout): SeriesGroup[] { for (let i = 0; i < layout.marks.length; i++) { const mark = layout.marks[i]; if ((mark.type === 'line' || mark.type === 'area') && mark.dataPoints?.length) { - const key = mark.seriesKey ?? '__default__'; + // compileLayer namespaces mark keys by original leaf index, independently + // of paint order. Equal series names in separate leaves are distinct + // evidence; a band's derived line still shares its leaf's series group. + const scope = mark.key?.match(/^layer:\d+:/)?.[0] ?? ''; + const seriesKey = mark.seriesKey ?? '__default__'; + const key = `${scope}${seriesKey}`; const existingType = markTypeByKey.get(key); if (existingType === 'line' && mark.type === 'area') continue; const color = mark.type === 'line' ? mark.stroke : getRepresentativeColor(mark.fill); @@ -73,7 +80,7 @@ function collectSeriesGroups(layout: ChartLayout): SeriesGroup[] { for (const dp of mark.dataPoints) { pointsByX.set(snapKey(dp.x), { ...dp }); } - byKey.set(key, { seriesKey: key, color, pointsByX }); + byKey.set(key, { key, seriesKey, markId: `${mark.type}-${i}`, color, pointsByX }); markTypeByKey.set(key, mark.type); } } @@ -115,6 +122,9 @@ function buildSliceTooltip( const fields: TooltipField[] = []; const isMulti = hits.length > 1; + const seriesCounts = new Map(); + for (const { group } of hits) + seriesCounts.set(group.seriesKey, (seriesCounts.get(group.seriesKey) ?? 0) + 1); // The stack total is precomputed per x by the engine and carried on every // layer's row; take the first one and append it once, below the series rows. let total: TooltipField | undefined; @@ -137,10 +147,15 @@ function buildSliceTooltip( null; if (!yField) continue; fields.push({ - label: group.seriesKey, + label: + group.seriesKey === '__default__' + ? yField.label + : (seriesCounts.get(group.seriesKey) ?? 0) > 1 + ? `${group.seriesKey} · ${yField.label}` + : group.seriesKey, value: yField.value, color: group.color, - ...(emphasisKey === group.seriesKey ? { emphasis: true } : {}), + ...(emphasisKey === group.key ? { emphasis: true } : {}), }); } else { for (const f of tip.fields) { @@ -246,8 +261,10 @@ export function wireVoronoiTooltipEvents( } currentEmphasis = emphasisKey; - if (emphasisKey) emphasis?.setSeries(emphasisKey); - else emphasis?.clear(); + const emphasizedGroup = groups.find((group) => group.key === emphasisKey); + if (emphasizedGroup) { + emphasis?.setMark(svg.querySelector(`[data-mark-id="${emphasizedGroup.markId}"]`)); + } else emphasis?.clear(); const tooltip = buildSliceTooltip(hits, emphasisKey); if (!tooltip) return false; @@ -264,7 +281,7 @@ export function wireVoronoiTooltipEvents( function keysAt(snappedX: number): string[] { const keys: string[] = []; for (const group of groups) { - if (group.pointsByX.has(snappedX)) keys.push(group.seriesKey); + if (group.pointsByX.has(snappedX)) keys.push(group.key); } return keys; } @@ -284,7 +301,7 @@ export function wireVoronoiTooltipEvents( const dy = Math.abs(point.y - svgY); if (dy < nearestDy) { nearestDy = dy; - nearestKey = group.seriesKey; + nearestKey = group.key; } } const emphasisKey = nearestDy <= EMPHASIS_PROXIMITY_PX ? nearestKey : null; diff --git a/packages/vanilla/src/interactions/mark-navigation.ts b/packages/vanilla/src/interactions/mark-navigation.ts new file mode 100644 index 00000000..5f382a25 --- /dev/null +++ b/packages/vanilla/src/interactions/mark-navigation.ts @@ -0,0 +1,94 @@ +/** Keyboard access to the same evidence and actions as pointer inspection. */ +export interface NavigableMark { + element: Element; + key: string; + enter: (event: MouseEvent) => void; + leave: () => void; + activate: () => void; +} + +/** Capture logical focus before replacing a specialized renderer's SVG. */ +export function captureMarkFocus(svg: SVGSVGElement | null): string | null { + const active = document.activeElement; + if (!svg || !active || !svg.contains(active)) return null; + return active.getAttribute('data-navigation-key') ?? ''; +} + +/** Restore a surviving mark, or the chart itself when that mark was removed. */ +export function restoreMarkFocus(svg: SVGSVGElement, key: string | null): void { + if (key === null) return; + const matches = [...svg.querySelectorAll('[data-navigation-key]')].filter( + (element) => element.getAttribute('data-navigation-key') === key, + ); + const target = matches.length === 1 ? matches[0] : svg; + if (target === svg && !svg.hasAttribute('tabindex')) svg.setAttribute('tabindex', '-1'); + target.focus({ preventScroll: true }); +} + +/** + * One tab stop per chart; arrows traverse marks, Enter/Space activate, Escape + * dismisses inspection. Tab always retains its native page-navigation behavior. + */ +export function wireMarkNavigation(svg: SVGSVGElement, marks: NavigableMark[]): () => void { + const cleanups: Array<() => void> = []; + // An image is atomic to assistive technology; a group exposes its focusable marks. + if (svg.getAttribute('role') === 'img') svg.setAttribute('role', 'group'); + marks.forEach((mark, index) => { + const element = mark.element as SVGElement; + element.setAttribute('tabindex', index === 0 ? '0' : '-1'); + element.setAttribute('data-navigation-key', mark.key); + const focus = () => { + for (const other of marks) other.element.setAttribute('tabindex', '-1'); + element.setAttribute('tabindex', '0'); + const bounds = element.getBoundingClientRect(); + mark.enter( + new MouseEvent('focus', { + clientX: bounds.left + bounds.width / 2, + clientY: bounds.top + bounds.height / 2, + }), + ); + }; + const keydown = (event: KeyboardEvent) => { + let next: number | undefined; + switch (event.key) { + case 'ArrowRight': + case 'ArrowDown': + next = Math.min(index + 1, marks.length - 1); + break; + case 'ArrowLeft': + case 'ArrowUp': + next = Math.max(index - 1, 0); + break; + case 'Home': + next = 0; + break; + case 'End': + next = marks.length - 1; + break; + case 'Enter': + case ' ': + mark.activate(); + break; + case 'Escape': + mark.leave(); + break; + default: + return; + } + event.preventDefault(); + event.stopPropagation(); + if (next !== undefined) (marks[next].element as SVGElement).focus({ preventScroll: true }); + }; + element.addEventListener('focus', focus); + element.addEventListener('blur', mark.leave); + element.addEventListener('keydown', keydown); + cleanups.push(() => { + element.removeEventListener('focus', focus); + element.removeEventListener('blur', mark.leave); + element.removeEventListener('keydown', keydown); + }); + }); + return () => { + for (const cleanup of cleanups) cleanup(); + }; +} diff --git a/packages/vanilla/src/map-camera.ts b/packages/vanilla/src/map-camera.ts index 9603b2dd..62ebcc09 100644 --- a/packages/vanilla/src/map-camera.ts +++ b/packages/vanilla/src/map-camera.ts @@ -188,3 +188,20 @@ export function applyMapCamera(svg: SVGElement, camera: Camera, layout: GeoMapLa } } } + +/** Define the map viewport clip for both live zoom and terminal exports. */ +export function ensureMapClip(svg: SVGSVGElement, layout: GeoMapLayout, clipId: string): void { + if (svg.querySelector(`#${clipId}`)) return; + let defs = svg.querySelector('defs'); + if (!defs) { + defs = document.createElementNS('http://www.w3.org/2000/svg', 'defs'); + svg.insertBefore(defs, svg.firstChild); + } + const clip = document.createElementNS('http://www.w3.org/2000/svg', 'clipPath'); + clip.setAttribute('id', clipId); + const rect = document.createElementNS('http://www.w3.org/2000/svg', 'rect'); + rect.setAttribute('width', String(layout.area.width)); + rect.setAttribute('height', String(layout.area.height)); + clip.appendChild(rect); + defs.appendChild(clip); +} diff --git a/packages/vanilla/src/map-mount.ts b/packages/vanilla/src/map-mount.ts index 26be6983..f1594a42 100644 --- a/packages/vanilla/src/map-mount.ts +++ b/packages/vanilla/src/map-mount.ts @@ -23,10 +23,17 @@ import { type JPGExportOptions, type SVGExportOptions, } from './export'; +import { + captureMarkFocus, + type NavigableMark, + restoreMarkFocus, + wireMarkNavigation, +} from './interactions/mark-navigation'; import { applyMapCamera, cameraForTarget, clampMapCamera, + ensureMapClip, FOCUS_DIM_OPACITY, focusTargetForFeatures, type GeoMapCameraOptions, @@ -221,10 +228,10 @@ export function createGeoMap( container.classList.add('oc-map-root'); // Resolve the effective font the same way compile() will. - function resolveEffectiveFont(): string { + function resolveEffectiveFont(candidate = currentSpec): string { return ( options?.theme?.fonts?.family ?? - currentSpec.theme?.fonts?.family ?? + candidate.theme?.fonts?.family ?? resolveFontFamily(container) ); } @@ -256,7 +263,7 @@ export function createGeoMap( return { width, height: Math.round(width * 0.625) }; } - function compile(): GeoMapLayout { + function compile(candidate = currentSpec, measure = measureText): GeoMapLayout { const { width, height } = getContainerDimensions(); const darkMode = resolveDarkMode(options?.darkMode); @@ -266,10 +273,10 @@ export function createGeoMap( theme: options?.theme, darkMode, watermark: options?.watermark, - measureText, + measureText: measure, }; - return compileGeoMap(currentSpec, compileOpts); + return compileGeoMap(candidate, compileOpts); } // --------------------------------------------------------------------------- @@ -278,6 +285,8 @@ export function createGeoMap( function wireTooltipAndInteraction(svg: SVGSVGElement, layout: GeoMapLayout): () => void { const cleanups: Array<() => void> = []; + const navigation: NavigableMark[] = []; + let activeLeave: (() => void) | null = null; const featuresById = new Map(); for (const f of layout.features) { @@ -294,9 +303,12 @@ export function createGeoMap( const feature = featuresById.get(featureId); const handleMouseEnter = (e: Event) => { + activeLeave?.(); + activeLeave = handleMouseLeave; const mouseEvent = e as MouseEvent; (el as SVGElement & ElementCSSInlineStyle).style.setProperty('cursor', 'pointer'); - raiseFeature(el, svg); + // Reparenting a focused SVG node drops native keyboard focus. + if (document.activeElement !== el) raiseFeature(el, svg); el.classList.add('oc-map-feature--hover'); if (content && tooltipManager && options?.tooltip !== false) { const svgRect = svg.getBoundingClientRect(); @@ -325,6 +337,8 @@ export function createGeoMap( }; const handleMouseLeave = () => { + if (activeLeave !== handleMouseLeave) return; + activeLeave = null; (el as SVGElement & ElementCSSInlineStyle).style.removeProperty('cursor'); el.classList.remove('oc-map-feature--hover'); restoreFeatureOrder(el); @@ -343,6 +357,13 @@ export function createGeoMap( } }; + navigation.push({ + element: el, + key: `feature:${featureId}`, + enter: handleMouseEnter, + leave: handleMouseLeave, + activate: handleClick, + }); el.addEventListener('mouseenter', handleMouseEnter); el.addEventListener('mousemove', handleMouseMove); el.addEventListener('mouseleave', handleMouseLeave); @@ -388,6 +409,8 @@ export function createGeoMap( const pointMark = pointMarksByKey.get(pointKey); const handlePointMouseEnter = (e: Event) => { + activeLeave?.(); + activeLeave = handlePointMouseLeave; const mouseEvent = e as MouseEvent; (el as SVGElement & ElementCSSInlineStyle).style.setProperty('cursor', 'pointer'); (el as SVGElement & ElementCSSInlineStyle).style.setProperty('filter', 'brightness(1.15)'); @@ -417,6 +440,8 @@ export function createGeoMap( }; const handlePointMouseLeave = () => { + if (activeLeave !== handlePointMouseLeave) return; + activeLeave = null; (el as SVGElement & ElementCSSInlineStyle).style.removeProperty('cursor'); (el as SVGElement & ElementCSSInlineStyle).style.removeProperty('filter'); tooltipManager?.hide(); @@ -433,6 +458,13 @@ export function createGeoMap( } }; + navigation.push({ + element: el, + key: `point:${pointKey}`, + enter: handlePointMouseEnter, + leave: handlePointMouseLeave, + activate: handlePointClick, + }); el.addEventListener('mouseenter', handlePointMouseEnter); el.addEventListener('mousemove', handlePointMouseMove); el.addEventListener('mouseleave', handlePointMouseLeave); @@ -446,7 +478,9 @@ export function createGeoMap( }); } + cleanups.push(wireMarkNavigation(svg, navigation)); return () => { + activeLeave?.(); for (const cleanup of cleanups) { cleanup(); } @@ -586,9 +620,9 @@ export function createGeoMap( liveEl.textContent = `Zoomed to ${names.join(', ')}`; } - function applyFocusDim(ids: Array | null): void { - if (!svgElement) return; - const features = svgElement.querySelectorAll('.oc-map-feature'); + function applyFocusDim(ids: Array | null, target = svgElement): void { + if (!target) return; + const features = target.querySelectorAll('.oc-map-feature'); if (!ids || ids.length === 0) { for (const el of features) { (el as SVGElement & ElementCSSInlineStyle).style.removeProperty('opacity'); @@ -610,6 +644,8 @@ export function createGeoMap( // --------------------------------------------------------------------------- function render(animate = false): void { + const focusedMark = captureMarkFocus(svgElement); + tooltipManager?.hide(); // Cancel running animations if (animationCleanup) { animationCleanup(); @@ -650,6 +686,7 @@ export function createGeoMap( const isAnimating = animate && !!currentLayout.animation?.enter; if (isAnimating) { animationCleanup = setupAnimationCleanup(newSvg, () => { + animationCleanup = null; if (pendingResize && !destroyed) { pendingResize = false; resize(); @@ -691,15 +728,19 @@ export function createGeoMap( isFirstRender = false; syncZoomController(oldSvgHadFocus); + if (svgElement) restoreMarkFocus(svgElement, focusedMark); } function update(newSpec: GeoMapSpec): void { + if (destroyed) return; + // Compile before adopting an edit: a rejected spec must leave the last + // valid view, its interactions, and future resize/export calls intact. + const nextFont = resolveEffectiveFont(newSpec); + const nextMeasure = nextFont === fontFamily ? measureText : createMeasureText(nextFont); + const nextLayout = compile(newSpec, nextMeasure); currentSpec = newSpec; - const nextFont = resolveEffectiveFont(); - if (nextFont !== fontFamily) { - fontFamily = nextFont; - measureText = createMeasureText(fontFamily); - } + fontFamily = nextFont; + measureText = nextMeasure; // Capture paint + point geometry optimistically; gate the tween on the // *new* layout's animation config after compile @@ -714,7 +755,7 @@ export function createGeoMap( const prevFocusSig = focusSignature(currentLayout.focus); const prevMapSize = currentLayout.mapSize; - currentLayout = compile(); + currentLayout = nextLayout; if (!currentFocusIds) rescaleFreeCamera(prevMapSize); render(); @@ -791,15 +832,27 @@ export function createGeoMap( ): string | Promise | Promise { if (!svgElement) return ''; + // Export settled geometry, independent of transient hover and animation. + const terminalSvg = renderMapSVG(currentLayout, { animate: false }); + applyMapCamera(terminalSvg, currentCamera, currentLayout); + const clip = svgElement.querySelector('.oc-map-group')?.getAttribute('clip-path'); + if (clip) { + const clipId = clip.slice(5, -1); + ensureMapClip(terminalSvg, currentLayout, clipId); + terminalSvg.querySelector('.oc-map-group')?.setAttribute('clip-path', clip); + } + applyFocusDim(currentFocusIds, terminalSvg); + const resolvedOptions = { theme: currentLayout.theme, ...options_ }; + switch (format) { case 'svg': - return exportSVG(svgElement); + return exportSVG(terminalSvg); case 'svg-with-fonts': - return exportSVGWithFonts(svgElement); + return exportSVGWithFonts(terminalSvg, resolvedOptions); case 'png': - return exportPNG(svgElement, options_ as JPGExportOptions); + return exportPNG(terminalSvg, resolvedOptions); case 'jpg': - return exportJPG(svgElement, options_ as JPGExportOptions); + return exportJPG(terminalSvg, resolvedOptions); default: return ''; } diff --git a/packages/vanilla/src/map-zoom.ts b/packages/vanilla/src/map-zoom.ts index 51d3f253..5369b989 100644 --- a/packages/vanilla/src/map-zoom.ts +++ b/packages/vanilla/src/map-zoom.ts @@ -24,6 +24,7 @@ import type { GeoMapLayout } from '@opendata-ai/openchart-core'; import { clampMapCamera, + ensureMapClip, panCamera, type ResolvedMapZoom, wheelZoomFactor, @@ -66,7 +67,6 @@ const DRAG_THRESHOLD = 3; const HINT_MS = 1500; const EPS = 1e-3; -const SVG_NS = 'http://www.w3.org/2000/svg'; let clipIdCounter = 0; const ICONS = { @@ -539,22 +539,6 @@ export function createMapZoomController(host: MapZoomHost): MapZoomController { overlay.style.height = `${(area.height / height) * 100}%`; } - function ensureClip(svg: SVGSVGElement, layout: GeoMapLayout): void { - if (svg.querySelector(`#${clipId}`)) return; - let defs = svg.querySelector('defs'); - if (!defs) { - defs = document.createElementNS(SVG_NS, 'defs'); - svg.insertBefore(defs, svg.firstChild); - } - const clip = document.createElementNS(SVG_NS, 'clipPath'); - clip.setAttribute('id', clipId); - const rect = document.createElementNS(SVG_NS, 'rect'); - rect.setAttribute('width', String(layout.area.width)); - rect.setAttribute('height', String(layout.area.height)); - clip.appendChild(rect); - defs.appendChild(clip); - } - // ------------------------------------------------------------------------- // Lifecycle // ------------------------------------------------------------------------- @@ -569,7 +553,7 @@ export function createMapZoomController(host: MapZoomHost): MapZoomController { const layout = host.getLayout(); svg.setAttribute('tabindex', '0'); svg.setAttribute('aria-keyshortcuts', '+ - 0 ArrowUp ArrowDown ArrowLeft ArrowRight'); - ensureClip(svg, layout); + ensureMapClip(svg, layout, clipId); ensureOverlay(cfg.controls); placeOverlay(layout); if (restoreFocus) svg.focus({ preventScroll: true }); diff --git a/packages/vanilla/src/mount.ts b/packages/vanilla/src/mount.ts index 536a354c..e50d5372 100644 --- a/packages/vanilla/src/mount.ts +++ b/packages/vanilla/src/mount.ts @@ -81,12 +81,7 @@ import { renderChartSVG } from './svg-renderer'; import { createTextEditOverlay, createTextEditOverlayAtPosition } from './text-edit-overlay'; import { stampThemeProperties } from './theme-tokens'; import { createTooltipManager, type TooltipManager } from './tooltip'; -import { - canTransition, - type GeometrySnapshot, - runTransition, - type TransitionHandle, -} from './transition'; +import { canTransition, runTransition, type TransitionHandle } from './transition'; import { createYouDrawIt, type YouDrawItController } from './you-draw-it'; // --------------------------------------------------------------------------- @@ -300,7 +295,6 @@ export function createChart( // Data-update transition state let transitionHandle: TransitionHandle | null = null; - let transitionSnapshot: GeometrySnapshot | null = null; // Set when webfonts have loaded and a recompile is owed to reflect final font // metrics. The next render() that actually recompiles flips @@ -342,10 +336,10 @@ export function createChart( // spec-level theme; fall back to the container's computed font. Measuring // against a different font than compile renders (e.g. a spec that sets // theme.fonts.family) desyncs layout metrics and the font-reload watcher. - function resolveEffectiveFont(): string { + function resolveEffectiveFont(candidate = currentSpec): string { return ( options?.theme?.fonts?.family ?? - currentSpec.theme?.fonts?.family ?? + candidate.theme?.fonts?.family ?? resolveFontFamily(container) ); } @@ -357,7 +351,13 @@ export function createChart( // Compilation // --------------------------------------------------------------------------- - function compileAt(width: number, height: number): ChartLayout { + function compileAt( + width: number, + height: number, + candidate: typeof currentSpec, + measure: typeof measureText, + runtime: boolean, + ): ChartLayout { const darkMode = resolveDarkMode(options?.darkMode); const compileOpts: CompileOptions = { @@ -367,13 +367,12 @@ export function createChart( darkMode, renderer: options?.renderer, watermark: options?.watermark, - measureText, + measureText: measure, }; - if (isLayerSpec(currentSpec)) { - return compileLayer(withRuntimeHidden(currentSpec as LayerSpec) as LayerSpec, compileOpts); - } - return compileChart(withRuntimeHidden(currentSpec) as ChartSpec | GraphSpec, compileOpts); + const effective = runtime ? withRuntimeHidden(candidate) : candidate; + if (isLayerSpec(effective)) return compileLayer(effective, compileOpts); + return compileChart(effective as ChartSpec | GraphSpec, compileOpts); } /** @@ -420,18 +419,18 @@ export function createChart( ); } - function compile(): ChartLayout { - const { width, height } = getContainerDimensions(); + function compile(candidate = currentSpec, measure = measureText, runtime = true): ChartLayout { + const { width, height } = getContainerDimensions(candidate); // Auto-height growth applies to ChartSpec mounts only. GraphSpec, // LayerSpec, and sparkline mounts keep the fixed-fallback behavior. const growsWithChrome = isAutoHeight === true && - !isLayerSpec(currentSpec) && - !isGraphSpec(currentSpec as unknown as Record) && - (currentSpec as ChartSpec).display !== 'sparkline'; + !isLayerSpec(candidate) && + !isGraphSpec(candidate as unknown as Record) && + (candidate as ChartSpec).display !== 'sparkline'; if (!growsWithChrome) { - return compileAt(width, height); + return compileAt(width, height, candidate, measure, runtime); } // Bounded convergence loop, max 3 compiles. Chrome height depends on @@ -441,13 +440,13 @@ export function createChart( // un-strip chrome, changing the overheads once more. (Height classes are // not the issue — they only strip below 200/350px, and growing from 400 // moves away from those thresholds.) Pass 3 is accepted unconditionally. - let layout = compileAt(width, height); + let layout = compileAt(width, height, candidate, measure, runtime); const overheads = verticalOverheads(layout, width); if (overheads > 0) { - layout = compileAt(width, height + overheads); + layout = compileAt(width, height + overheads, candidate, measure, runtime); const overheads2 = verticalOverheads(layout, width); if (overheads2 !== overheads) { - layout = compileAt(width, height + overheads2); + layout = compileAt(width, height + overheads2, candidate, measure, runtime); } } return layout; @@ -545,10 +544,9 @@ export function createChart( // Container dimensions // --------------------------------------------------------------------------- - function getContainerDimensions(): { width: number; height: number } { + function getContainerDimensions(candidate = currentSpec): { width: number; height: number } { const rect = container.getBoundingClientRect(); - const isSparkline = - 'display' in currentSpec && (currentSpec as ChartSpec).display === 'sparkline'; + const isSparkline = 'display' in candidate && (candidate as ChartSpec).display === 'sparkline'; if (isSparkline) { let width = rect.width; let height = rect.height; @@ -874,13 +872,15 @@ export function createChart( // Render cycle // --------------------------------------------------------------------------- - function render(retainScene = false): void { + function render(retainScene = false, prepared?: ChartLayout): void { const retainedSVG = retainScene ? svgElement : null; if (isDragging) { pendingRender = true; return; } + const nextLayout = prepared ?? compile(); + // Cancel any in-progress data-update transition if (transitionHandle) { transitionHandle.cancel(); @@ -941,7 +941,7 @@ export function createChart( srTableWrapper = null; } - currentLayout = compile(); + currentLayout = nextLayout; // Recorded for the ResizeObserver's self-induced-resize guard. lastRenderedWidth = currentLayout.dimensions.width; lastRenderedSvgHeight = currentLayout.dimensions.height; @@ -1297,21 +1297,15 @@ export function createChart( const prevLayout = currentLayout; const entranceWasRunning = cleanupAnimations != null; - // Snapshot in-flight transition geometry before render() cancels it. - // This enables retargeting: the next transition starts from the - // interrupted position instead of snapping to the previous final state. - if (transitionHandle?.running) { - transitionSnapshot = transitionHandle.snapshot(); - } - + const nextFont = resolveEffectiveFont(newSpec); + const nextMeasure = nextFont === fontFamily ? measureText : createMeasureText(nextFont); + const nextLayout = compile(newSpec, nextMeasure, layoutOnly); + // Capture only after compilation succeeds, before render() cancels the + // current motion. A rejected edit must not leave geometry for a later update. + const snapshot = transitionHandle?.running ? transitionHandle.snapshot() : undefined; currentSpec = newSpec; - // A new spec can change theme.fonts.family; rebuild the measurer so layout - // measures the font compile will actually render with. - const nextFont = resolveEffectiveFont(); - if (nextFont !== fontFamily) { - fontFamily = nextFont; - measureText = createMeasureText(fontFamily); - } + fontFamily = nextFont; + measureText = nextMeasure; if (!layoutOnly) { runtimeHiddenSeries.clear(); runtimeShownSeries.clear(); @@ -1329,7 +1323,7 @@ export function createChart( if (updateOpts && 'selectedElement' in updateOpts) { selectedElement = updateOpts.selectedElement ?? null; } - render(canTransitionSpecShape(prevSpec, newSpec)); + render(canTransitionSpecShape(prevSpec, newSpec), nextLayout); // After render, check if we can run a smooth transition instead of the // instant swap that render() already performed. @@ -1344,16 +1338,12 @@ export function createChart( entranceInFlight: entranceWasRunning, }) ) { - // Consume snapshot (if any) for retargeting, then clear - const snapshot = transitionSnapshot; - transitionSnapshot = null; - const handle = runTransition({ svg: svgElement as SVGSVGElement, prevLayout, nextLayout: currentLayout, animation: currentLayout.animation!, - fromSnapshot: snapshot ?? undefined, + fromSnapshot: snapshot, manual, // render() above already rebuilt the layer for the NEXT layout, so this // is the destination-mode layer the tween writes into. @@ -1367,8 +1357,6 @@ export function createChart( transitionHandle = handle; return handle; } - // No transition started; clear any stale snapshot - transitionSnapshot = null; return null; } @@ -1496,8 +1484,6 @@ export function createChart( transitionHandle.cancel(); transitionHandle = null; } - transitionSnapshot = null; - if (cleanupAnimations) { cleanupAnimations(); cleanupAnimations = null; diff --git a/packages/vanilla/src/reader.ts b/packages/vanilla/src/reader.ts index 68cdc7b4..5ea9b122 100644 --- a/packages/vanilla/src/reader.ts +++ b/packages/vanilla/src/reader.ts @@ -1,10 +1,13 @@ import type { + ChartLayout, ChartSpec, DataRow, + LayerSpec, ReaderObservation, ReaderSelection, } from '@opendata-ai/openchart-core'; import { + observationKey, reconcileReaderSelection, resolveObservationIdentities, } from '@opendata-ai/openchart-engine'; @@ -12,31 +15,46 @@ import type { ChartInstance } from './mount'; export interface ReaderController { readonly selection: ReaderSelection; - update(spec: ChartSpec, visibleKeys?: ReadonlySet): void; + /** Call after chart.update(). Without an override, visibility follows compiled marks. */ + update(spec: ChartSpec | LayerSpec, visibleKeys?: ReadonlySet): void; pin(datum: DataRow): void; clear(): void; destroy(): void; } -/** Persistent reader interaction, separate from editor ElementRef selection. */ +const LAYER_UNSUPPORTED = + 'Reader inspection is unavailable for layered charts: observations cannot be bound to a single dataset.'; +const seriesField = (spec: ChartSpec) => + spec.encoding.color && 'field' in spec.encoding.color ? spec.encoding.color.field : undefined; + +/** + * Persistent reader interaction, separate from editor ElementRef selection. + * Layered specs clear/disable inspection and announce the unsupported binding; + * updating back to a single chart resumes inspection without choosing a leaf. + */ export function createReader( container: HTMLElement, instance: ChartInstance, - initial: ChartSpec, + initial: ChartSpec | LayerSpec, onChange: (selection: ReaderSelection) => void, ): ReaderController { - let spec = initial; - let state: ReaderSelection = { preview: null, pinned: [], announcement: '' }; + let spec = 'layer' in initial ? null : initial; + let state: ReaderSelection = { + preview: null, + pinned: [], + announcement: spec ? '' : LAYER_UNSUPPORTED, + }; let keyboardIndex = -1; let settleFrame: number | null = null; - let identities = resolveObservationIdentities( - spec.data, - spec.encoding.key?.field, - spec.encoding.color && 'field' in spec.encoding.color ? spec.encoding.color.field : undefined, - ); + let visibilityOverride: ReadonlySet | undefined; + let identities = spec + ? resolveObservationIdentities(spec.data, spec.encoding.key?.field, seriesField(spec)) + : []; const emit = () => onChange({ ...state, pinned: [...state.pinned] }); - const visibleData = (): Array<{ datum: DataRow; key?: string }> => { - const rows = instance.sampleLayout().marks.flatMap((mark) => { + const visibleData = ( + layout: ChartLayout = instance.sampleLayout(), + ): Array<{ datum: DataRow; key?: string }> => { + const rows = layout.marks.flatMap((mark) => { if ('opacity' in mark && mark.opacity === 0) return []; if (mark.type === 'line' || mark.type === 'area') return (mark.dataPoints ?? []).map((point, index) => ({ @@ -46,14 +64,49 @@ export function createReader( if ('data' in mark) return [{ datum: mark.data as DataRow, key: mark.key }]; return []; }); - return [...new Map(rows.map((row) => [JSON.stringify(row.datum), row])).values()]; + // A line and its point overlay share rows. Distinct duplicate rows must not + // collapse merely because their serialized values happen to be identical. + return [...new Map(rows.map((row) => [row.datum, row])).values()]; + }; + const compiledVisibleKeys = (observations: ReaderObservation[]): ReadonlySet => { + const keys = new Set(); + if (!spec || !observations.length) return keys; + // Use the destination layout, not interpolated geometry or exit ghosts. + // Zero-length marks still count; absence retains the pin as filtered. + const rows = visibleData(instance.layout); + const references = new Set(rows.map(({ datum }) => datum)); + const signatures = new Set(rows.map(({ datum }) => JSON.stringify(datum))); + const keyField = spec.encoding.key?.field; + const visibleSourceKeys = new Set( + keyField ? rows.map(({ datum }) => observationKey(datum[keyField])) : [], + ); + for (const observation of observations) { + if (observation.kind === 'aggregate') { + if (signatures.has(JSON.stringify(observation.datum))) keys.add(observation.observation); + continue; + } + const index = identities.findIndex((id) => id.observation === observation.observation); + if (index < 0) continue; + const row = spec.data[index]; + const signature = JSON.stringify(row); + const visible = observation.persistent + ? visibleSourceKeys.has(observation.observation) + : references.has(row) || + (signatures.has(signature) && + spec.data.filter((source) => JSON.stringify(source) === signature).length === 1); + if (visible) keys.add(observation.observation); + } + return keys; }; const resolve = (datum: DataRow): ReaderObservation | null => { + if (!spec) return null; const keyField = spec.encoding.key?.field; let index = spec.data.indexOf(datum); + if (index >= 0 && spec.data.lastIndexOf(datum) !== index) return null; if (index < 0 && keyField) index = spec.data.findIndex( - (row, i) => identities[i].persistent && row[keyField] === datum[keyField], + (_row, i) => + identities[i].persistent && identities[i].observation === observationKey(datum[keyField]), ); // Compilers may normalize a row. Within one revision, a unique exact row // match is safe; an ambiguous match must never pick the first observation. @@ -61,13 +114,14 @@ export function createReader( const matches = spec.data.flatMap((row, i) => JSON.stringify(row) === JSON.stringify(datum) ? [i] : [], ); + if (matches.length > 1) return null; if (matches.length === 1) index = matches[0]; } if (index >= 0) return { ...identities[index], datum: spec.data[index], state: 'visible' }; // Aggregated/bin observations describe visible groups, not a raw source row. // Their identity includes the transformation and grouping mark; replacement // clears them until an explicit host mapping supplies revision persistence. - const group = visibleData().find( + const group = visibleData(instance.layout).find( (row) => row.datum === datum || JSON.stringify(row.datum) === JSON.stringify(datum), ); return group @@ -83,6 +137,11 @@ export function createReader( const pin = (datum: DataRow) => { const observation = resolve(datum); if (!observation) return; + observation.state = (visibilityOverride ?? compiledVisibleKeys([observation])).has( + observation.observation, + ) + ? 'visible' + : 'filtered'; state = { ...state, preview: observation, @@ -95,10 +154,15 @@ export function createReader( emit(); }; const clear = () => { - state = { preview: null, pinned: [], announcement: 'Selection dismissed.' }; + state = { + preview: null, + pinned: [], + announcement: spec ? 'Selection dismissed.' : LAYER_UNSUPPORTED, + }; emit(); }; function hit(event: MouseEvent | PointerEvent): DataRow | null { + if (!spec) return null; const svg = container.querySelector('svg'); if (!svg) return null; const bounds = svg.getBoundingClientRect(); @@ -165,6 +229,7 @@ export function createReader( emit(); }; const keydown = (event: KeyboardEvent) => { + if (!spec) return; if ( (event.target as HTMLElement).closest( 'input, textarea, select, button, [contenteditable=true]', @@ -197,6 +262,7 @@ export function createReader( container.addEventListener('pointerleave', leave); container.addEventListener('click', click); container.addEventListener('keydown', keydown, true); + if (!spec) emit(); return { get selection() { return state; @@ -204,9 +270,23 @@ export function createReader( pin, clear, update(next, visibleKeys) { + if (settleFrame !== null) { + cancelAnimationFrame(settleFrame); + settleFrame = null; + } + visibilityOverride = visibleKeys; + if ('layer' in next) { + spec = null; + identities = []; + keyboardIndex = -1; + state = { preview: null, pinned: [], announcement: LAYER_UNSUPPORTED }; + emit(); + return; + } const meaningChanged = + !spec || JSON.stringify([next.mark, next.encoding, next.transform]) !== - JSON.stringify([spec.mark, spec.encoding, spec.transform]); + JSON.stringify([spec.mark, spec.encoding, spec.transform]); if ( meaningChanged && (state.pinned.some((item) => item.kind === 'aggregate') || @@ -221,27 +301,38 @@ export function createReader( }; } const replaced = - next.data !== spec.data || next.encoding.key?.field !== spec.encoding.key?.field; + !spec || next.data !== spec.data || next.encoding.key?.field !== spec.encoding.key?.field; + const resuming = !spec; spec = next; identities = resolveObservationIdentities( spec.data, spec.encoding.key?.field, - spec.encoding.color && 'field' in spec.encoding.color - ? spec.encoding.color.field - : undefined, + seriesField(spec), ); - if (replaced || visibleKeys) + const effectiveVisibility = visibleKeys ?? compiledVisibleKeys(state.pinned); + if (replaced) state = reconcileReaderSelection( state, spec.data, spec.encoding.key?.field, - spec.encoding.color && 'field' in spec.encoding.color - ? spec.encoding.color.field - : undefined, - visibleKeys, + seriesField(spec), + effectiveVisibility, ); + else + state = { + ...state, + preview: null, + pinned: state.pinned.map((item) => ({ + ...item, + ...(item.kind !== 'aggregate' + ? identities.find((id) => id.observation === item.observation) + : {}), + state: effectiveVisibility.has(item.observation) ? 'visible' : 'filtered', + })), + }; + if (resuming) state = { ...state, announcement: '' }; + if (replaced || meaningChanged) keyboardIndex = -1; emit(); - if (settleFrame !== null) cancelAnimationFrame(settleFrame); const settle = () => { settleFrame = null; if (!state.pinned.some((item) => item.previous)) return; diff --git a/packages/vanilla/src/renderers/marks.ts b/packages/vanilla/src/renderers/marks.ts index fa806549..91d041d7 100644 --- a/packages/vanilla/src/renderers/marks.ts +++ b/packages/vanilla/src/renderers/marks.ts @@ -133,7 +133,7 @@ export function registerMarkRenderer( function renderLineMark(mark: LineMark, index: number): SVGElement { const g = createSVGElement('g'); - g.setAttribute('data-mark-id', `line-${mark.seriesKey ?? index}`); + g.setAttribute('data-mark-id', `line-${index}`); g.setAttribute('class', 'oc-mark oc-mark-line'); stampAnimationAttrs(g, mark, index); @@ -200,7 +200,7 @@ function renderLineMark(mark: LineMark, index: number): SVGElement { function renderAreaMark(mark: AreaMark, index: number): SVGElement { const g = createSVGElement('g'); - g.setAttribute('data-mark-id', `area-${mark.seriesKey ?? index}`); + g.setAttribute('data-mark-id', `area-${index}`); g.setAttribute('class', 'oc-mark oc-mark-area'); stampAnimationAttrs(g, mark, index); diff --git a/packages/vanilla/src/sankey-mount.ts b/packages/vanilla/src/sankey-mount.ts index 6bf58b4a..8c570dec 100644 --- a/packages/vanilla/src/sankey-mount.ts +++ b/packages/vanilla/src/sankey-mount.ts @@ -24,6 +24,12 @@ import { type JPGExportOptions, type SVGExportOptions, } from './export'; +import { + captureMarkFocus, + type NavigableMark, + restoreMarkFocus, + wireMarkNavigation, +} from './interactions/mark-navigation'; import { createMeasureText, resolveFontFamily, scheduleFontReload } from './measure-text'; import { observeResize } from './resize-observer'; import { resolveDarkMode } from './resolve-dark-mode'; @@ -63,7 +69,7 @@ export interface SankeyInstance { /** Export the sankey diagram. */ export( format: 'svg' | 'svg-with-fonts' | 'png' | 'jpg', - options?: JPGExportOptions, + options?: JPGExportOptions | SVGExportOptions, ): string | Promise | Promise; /** Remove all DOM elements and disconnect observers. */ destroy(): void; @@ -140,10 +146,10 @@ export function createSankey( // { ...spec.theme, ...options.theme }, so options.theme wins over the // spec-level theme; fall back to the container's computed font. Measuring a // different font than gets rendered desyncs layout metrics and the reload watcher. - function resolveEffectiveFont(): string { + function resolveEffectiveFont(candidate = currentSpec): string { return ( options?.theme?.fonts?.family ?? - currentSpec.theme?.fonts?.family ?? + candidate.theme?.fonts?.family ?? resolveFontFamily(container) ); } @@ -163,7 +169,7 @@ export function createSankey( }; } - function compile(): SankeyLayout { + function compile(candidate = currentSpec, measure = measureText): SankeyLayout { const { width, height } = getContainerDimensions(); const darkMode = resolveDarkMode(options?.darkMode); @@ -173,10 +179,10 @@ export function createSankey( theme: options?.theme, darkMode, watermark: options?.watermark, - measureText, + measureText: measure, }; - return compileSankey(currentSpec, compileOpts); + return compileSankey(candidate, compileOpts); } // --------------------------------------------------------------------------- @@ -185,6 +191,8 @@ export function createSankey( function wireTooltipAndInteraction(svg: SVGSVGElement, layout: SankeyLayout): () => void { const cleanups: Array<() => void> = []; + const navigation: NavigableMark[] = []; + let activeLeave: (() => void) | null = null; const nodesById = new Map(); for (const n of layout.nodes) { @@ -202,6 +210,8 @@ export function createSankey( const nodeData = nodeId ? (nodesById.get(nodeId)?.data ?? {}) : {}; const handleMouseEnter = (e: Event) => { + activeLeave?.(); + activeLeave = handleMouseLeave; const mouseEvent = e as MouseEvent; if (content && tooltipManager) { const svgRect = svg.getBoundingClientRect(); @@ -224,6 +234,8 @@ export function createSankey( }; const handleMouseLeave = () => { + if (activeLeave !== handleMouseLeave) return; + activeLeave = null; tooltipManager?.hide(); options?.onNodeHover?.(null); resetLinkOpacity(svg, layout); @@ -233,6 +245,13 @@ export function createSankey( options?.onNodeClick?.(nodeData); }; + navigation.push({ + element: el, + key: nodeId ? `node:${nodeId}` : markId, + enter: handleMouseEnter, + leave: handleMouseLeave, + activate: handleClick, + }); el.addEventListener('mouseenter', handleMouseEnter); el.addEventListener('mousemove', handleMouseMove); el.addEventListener('mouseleave', handleMouseLeave); @@ -248,16 +267,18 @@ export function createSankey( // Wire tooltip on link elements const linkElements = svg.querySelectorAll('.oc-sankey-link'); - for (const el of linkElements) { + for (const [index, el] of linkElements.entries()) { const markId = el.getAttribute('data-mark-id'); if (!markId) continue; const content = layout.tooltipDescriptors.get(markId); const sourceId = el.getAttribute('data-source'); const targetId = el.getAttribute('data-target'); - const linkData = findLinkData(layout, sourceId, targetId); + const linkData = layout.links[index]?.data ?? {}; const handleMouseEnter = (e: Event) => { + activeLeave?.(); + activeLeave = handleMouseLeave; const mouseEvent = e as MouseEvent; if (content && tooltipManager) { const svgRect = svg.getBoundingClientRect(); @@ -286,6 +307,8 @@ export function createSankey( }; const handleMouseLeave = () => { + if (activeLeave !== handleMouseLeave) return; + activeLeave = null; tooltipManager?.hide(); options?.onLinkHover?.(null); resetLinkOpacity(svg, layout); @@ -295,6 +318,13 @@ export function createSankey( options?.onLinkClick?.(linkData); }; + navigation.push({ + element: el, + key: JSON.stringify(['link', sourceId, targetId]), + enter: handleMouseEnter, + leave: handleMouseLeave, + activate: handleClick, + }); el.addEventListener('mouseenter', handleMouseEnter); el.addEventListener('mousemove', handleMouseMove); el.addEventListener('mouseleave', handleMouseLeave); @@ -308,26 +338,15 @@ export function createSankey( }); } + cleanups.push(wireMarkNavigation(svg, navigation)); return () => { + activeLeave?.(); for (const cleanup of cleanups) { cleanup(); } }; } - /** - * Find the data record for a link by source/target ids. - */ - function findLinkData( - layout: SankeyLayout, - sourceId: string | null, - targetId: string | null, - ): Record { - if (!sourceId || !targetId) return {}; - const link = layout.links.find((l) => l.sourceId === sourceId && l.targetId === targetId); - return link?.data ?? {}; - } - /** * Trace the path through a set of seed nodes: everything upstream of the * `up` seeds and everything downstream of the `down` seeds, links included. @@ -409,13 +428,10 @@ export function createSankey( */ function resetLinkOpacity(svg: SVGSVGElement, layout: SankeyLayout): void { const linkElements = svg.querySelectorAll('.oc-sankey-link'); - for (const el of linkElements) { + for (const [index, el] of linkElements.entries()) { const path = el.querySelector('path'); if (!path) continue; - // Look up the original opacity via data attributes rather than positional index - const source = el.getAttribute('data-source'); - const target = el.getAttribute('data-target'); - const link = layout.links.find((l) => l.sourceId === source && l.targetId === target); + const link = layout.links[index]; path.setAttribute('fill-opacity', String(link?.fillOpacity ?? 0.5)); } @@ -429,8 +445,11 @@ export function createSankey( // Render // --------------------------------------------------------------------------- - function render(): void { + function render(prepared?: SankeyLayout): void { if (destroyed) return; + const nextLayout = prepared ?? compile(); + const focusedMark = captureMarkFocus(svgElement); + tooltipManager?.hide(); // Cancel in-progress animations before tearing down if (animationCleanup) { @@ -452,8 +471,7 @@ export function createSankey( svgElement.parentNode.removeChild(svgElement); } - // Compile - currentLayout = compile(); + currentLayout = nextLayout; // Determine if we should animate const shouldAnimate = isFirstRender && !!currentLayout.animation?.enter; @@ -477,8 +495,8 @@ export function createSankey( if (!tooltipManager) { tooltipManager = createTooltipManager(container); } - cleanupTooltipEvents = wireTooltipAndInteraction(svgElement, currentLayout); } + cleanupTooltipEvents = wireTooltipAndInteraction(svgElement, currentLayout); // Animation cleanup on first render if (shouldAnimate && svgElement) { @@ -501,6 +519,7 @@ export function createSankey( fontsReloadPending = false; container.dataset.ocFontsState = 'ready'; } + if (svgElement) restoreMarkFocus(svgElement, focusedMark); } // --------------------------------------------------------------------------- @@ -509,16 +528,16 @@ export function createSankey( function update(newSpec: SankeySpec): void { if (destroyed) return; + // Compile before adopting an edit: a rejected spec must leave the last + // valid view, its interactions, and future resize/export calls intact. + const nextFont = resolveEffectiveFont(newSpec); + const nextMeasure = nextFont === fontFamily ? measureText : createMeasureText(nextFont); + const nextLayout = compile(newSpec, nextMeasure); currentSpec = newSpec; - // A new spec can change theme.fonts.family; rebuild the measurer so layout - // measures the font compile will actually render with. - const nextFont = resolveEffectiveFont(); - if (nextFont !== fontFamily) { - fontFamily = nextFont; - measureText = createMeasureText(fontFamily); - } - isFirstRender = true; // Allow animation on update - render(); + fontFamily = nextFont; + measureText = nextMeasure; + + render(nextLayout); } function resize(): void { @@ -534,21 +553,25 @@ export function createSankey( function doExport( format: 'svg' | 'svg-with-fonts' | 'png' | 'jpg', - exportOptions?: JPGExportOptions, + exportOptions?: JPGExportOptions | SVGExportOptions, ): string | Promise | Promise { if (!svgElement) { throw new Error('Sankey is not rendered yet'); } + // Export settled geometry, independent of transient hover and animation. + const terminalSvg = renderSankeySVG(currentLayout); + const resolvedOptions = { theme: currentLayout.theme, ...exportOptions }; + switch (format) { case 'svg': - return exportSVG(svgElement); + return exportSVG(terminalSvg); case 'svg-with-fonts': - return exportSVGWithFonts(svgElement, exportOptions as SVGExportOptions); + return exportSVGWithFonts(terminalSvg, resolvedOptions); case 'png': - return exportPNG(svgElement, exportOptions); + return exportPNG(terminalSvg, resolvedOptions); case 'jpg': - return exportJPG(svgElement, exportOptions); + return exportJPG(terminalSvg, resolvedOptions); default: throw new Error(`Unsupported export format: ${format}`); } @@ -623,8 +646,8 @@ export function createSankey( // Wire tooltip + interaction events if (options?.tooltip !== false && svgElement) { tooltipManager = createTooltipManager(container); - cleanupTooltipEvents = wireTooltipAndInteraction(svgElement, currentLayout); } + cleanupTooltipEvents = wireTooltipAndInteraction(svgElement, currentLayout); // Animation cleanup if (shouldAnimate && svgElement) { diff --git a/packages/vanilla/src/sankey-renderer.ts b/packages/vanilla/src/sankey-renderer.ts index e58c7bca..9d7cc434 100644 --- a/packages/vanilla/src/sankey-renderer.ts +++ b/packages/vanilla/src/sankey-renderer.ts @@ -11,21 +11,20 @@ import type { LegendLayout, MeasureTextFn, ResolvedAnimation, - ResolvedChromeElement, SankeyLayout, SankeyLinkMark, SankeyNodeMark, - TextStyle, } from '@opendata-ai/openchart-core'; import { BRAND_FONT_SIZE, BRAND_MIN_WIDTH, estimateTextWidth, - textAscent, wrapText, } from '@opendata-ai/openchart-core'; import { clampStaggerDelay } from '@opendata-ai/openchart-engine'; import { stampAnimationVars } from './animation-vars'; +import { renderChromeElement } from './renderers/chrome'; +import { applyTextStyle } from './renderers/svg-dom'; const SVG_NS = 'http://www.w3.org/2000/svg'; const XLINK_NS = 'http://www.w3.org/1999/xlink'; @@ -45,24 +44,6 @@ function setAttrs(el: SVGElement, attrs: Record): void } } -function applyTextStyle(el: SVGElement, style: TextStyle): void { - setAttrs(el, { - 'font-family': style.fontFamily, - 'font-size': style.fontSize, - 'font-weight': style.fontWeight, - }); - (el as SVGElement & ElementCSSInlineStyle).style.setProperty('fill', style.fill); - if (style.textAnchor) { - el.setAttribute('text-anchor', style.textAnchor); - } - if (style.dominantBaseline) { - el.setAttribute('dominant-baseline', style.dominantBaseline); - } - if (style.fontVariant) { - el.setAttribute('font-variant', style.fontVariant); - } -} - /** * Stamp animation index attributes on a mark element when animation is enabled. */ @@ -82,44 +63,6 @@ function stampAnimationAttrs( // Chrome rendering // --------------------------------------------------------------------------- -function renderChromeElement( - parent: SVGElement, - element: ResolvedChromeElement, - className: string, - chromeKey: string, - measureText?: MeasureTextFn, -): void { - const text = createSVGElement('text'); - // element.y is the TOP of the text box; convert to the alphabetic baseline - // (see renderers/chrome.ts — WebKit mishandles dominant-baseline:hanging). - setAttrs(text, { x: element.x, y: element.y + textAscent(element.style.fontSize) }); - applyTextStyle(text, element.style); - text.setAttribute('class', className); - text.setAttribute('data-chrome-key', chromeKey); - - const lines = wrapText( - element.text, - element.style.fontSize, - element.style.fontWeight, - element.maxWidth, - measureText, - ); - - if (lines.length === 1) { - text.textContent = element.text; - } else { - const lineHeight = element.style.fontSize * (element.style.lineHeight ?? 1.3); - for (let i = 0; i < lines.length; i++) { - const tspan = createSVGElement('tspan'); - setAttrs(tspan, { x: element.x, dy: i === 0 ? 0 : lineHeight }); - tspan.textContent = lines[i]; - text.appendChild(tspan); - } - } - - parent.appendChild(text); -} - function renderChrome(parent: SVGElement, layout: SankeyLayout): void { const g = createSVGElement('g'); g.setAttribute('class', 'oc-chrome'); diff --git a/packages/vanilla/src/table-keyboard.ts b/packages/vanilla/src/table-keyboard.ts index aacc3a33..7488b875 100644 --- a/packages/vanilla/src/table-keyboard.ts +++ b/packages/vanilla/src/table-keyboard.ts @@ -13,8 +13,12 @@ export interface KeyboardNavOptions { /** The wrapper element containing the whole table UI. */ wrapper: HTMLElement; + /** Logical cell to restore after a resize, sort, or data update. */ + initialCell?: { rowId: string; columnKey: string }; /** Callback to trigger sort on a column. */ onSort: (columnKey: string) => void; + /** Activate the focused row through the host's existing row-click callback. */ + onRowActivate?: (rowId: string) => void; /** Callback to clear search and return focus to the table body. */ onClearSearch: () => void; /** Callback to announce text to screen readers via the live region. */ @@ -30,6 +34,8 @@ interface CellPosition { // Main // --------------------------------------------------------------------------- +let navigationId = 0; + /** * Attach keyboard navigation to a rendered table. * @@ -39,6 +45,7 @@ export function attachKeyboardNav(options: KeyboardNavOptions): () => void { const { wrapper, onSort, onClearSearch, onAnnounce } = options; let focusedCell: CellPosition = { row: -1, col: 0 }; + const idPrefix = `oc-table-${++navigationId}`; const table = wrapper.querySelector('table'); if (!table) return () => {}; @@ -59,11 +66,11 @@ export function attachKeyboardNav(options: KeyboardNavOptions): () => void { if (!thead) return []; const headerRow = thead.querySelector('tr'); if (!headerRow) return []; - return Array.from(headerRow.querySelectorAll('th')); + return Array.from(headerRow.querySelectorAll('th')).filter((cell) => !cell.hidden); } function getCellsInRow(tr: HTMLTableRowElement): HTMLTableCellElement[] { - return Array.from(tr.querySelectorAll('td')); + return Array.from(tr.querySelectorAll('td')).filter((cell) => !cell.hidden); } function getColCount(): number { @@ -99,7 +106,7 @@ export function attachKeyboardNav(options: KeyboardNavOptions): () => void { const cell = cells[col]; if (!cell) return; - const cellId = `oc-cell-${row}-${col}`; + const cellId = `${idPrefix}-cell-${row}-${col}`; cell.id = cellId; cell.classList.add('oc-table-cell-focus'); cell.setAttribute('data-row', String(row)); @@ -117,13 +124,21 @@ export function attachKeyboardNav(options: KeyboardNavOptions): () => void { function handleTbodyFocus(): void { // When tbody receives focus, highlight the first cell (or restore last) if (focusedCell.row < 0) { - setFocusedCell(0, 0); + const row = getRows().findIndex( + (item) => item.getAttribute('data-row-id') === options.initialCell?.rowId, + ); + const col = getHeaderCells().findIndex( + (item) => item.getAttribute('data-column') === options.initialCell?.columnKey, + ); + setFocusedCell(Math.max(0, row), Math.max(0, col)); } else { setFocusedCell(focusedCell.row, focusedCell.col); } } function handleTbodyKeydown(e: KeyboardEvent): void { + // Native links and priority-row disclosures own their keyboard behavior. + if (e.target !== tbody) return; const rows = getRows(); if (rows.length === 0) return; @@ -131,6 +146,15 @@ export function attachKeyboardNav(options: KeyboardNavOptions): () => void { const { row, col } = focusedCell; switch (e.key) { + case 'Enter': + case ' ': { + const rowId = rows[Math.max(row, 0)]?.getAttribute('data-row-id'); + if (rowId && options.onRowActivate) { + e.preventDefault(); + options.onRowActivate(rowId); + } + break; + } case 'ArrowDown': e.preventDefault(); if (row < rows.length - 1) { @@ -225,8 +249,8 @@ export function attachKeyboardNav(options: KeyboardNavOptions): () => void { if (e.key === 'Escape') { e.preventDefault(); onClearSearch(); - // Return focus to tbody - if (tbody) { + // The host may replace the body; focus the old one only if it survived. + if (tbody?.isConnected) { tbody.focus(); onAnnounce('Search cleared'); } diff --git a/packages/vanilla/src/table-mount.ts b/packages/vanilla/src/table-mount.ts index 46af84d9..1cf91f5c 100644 --- a/packages/vanilla/src/table-mount.ts +++ b/packages/vanilla/src/table-mount.ts @@ -120,6 +120,7 @@ export function createTable( let disconnectResize: (() => void) | null = null; let cleanupKeyboard: (() => void) | null = null; let cleanupAnimations: (() => void) | null = null; + let pendingResize = false; let isFirstRender = true; let destroyed = false; // Whether the user has touched sorting. Once they have, a null sort means @@ -173,7 +174,7 @@ export function createTable( } } - function compile(): TableLayout { + function compile(candidate = currentSpec): TableLayout { const state = getState(); const darkMode = resolveDarkMode(options?.darkMode); const { width } = getContainerDimensions(); @@ -193,7 +194,7 @@ export function createTable( page: state.page, }; - return compileTable(currentSpec, compileOpts); + return compileTable(candidate, compileOpts); } /** The sort once the user has touched it, otherwise defer to the spec. */ @@ -261,91 +262,138 @@ export function createTable( wrapperElement.classList.toggle('oc-table--regular', !condensed); } - function render(): void { + function render(candidate = currentSpec): void { if (destroyed) return; - try { - // Cancel any in-flight animations before re-rendering - if (cleanupAnimations) { - cleanupAnimations(); - cleanupAnimations = null; - } - - // Clean up previous keyboard nav - if (cleanupKeyboard) { - cleanupKeyboard(); - cleanupKeyboard = null; - } + // Keep the previous view usable if validation or compilation rejects an edit. + const nextLayout = compile(candidate); + const active = document.activeElement as HTMLElement | null; + const bodyFocused = active === wrapperElement?.querySelector('tbody'); + const focusedCell = wrapperElement?.querySelector('.oc-table-cell-focus'); + const initialCell = + bodyFocused && focusedCell + ? { + rowId: focusedCell.closest('tr')?.getAttribute('data-row-id') ?? '', + columnKey: focusedCell.getAttribute('data-column') ?? '', + } + : undefined; + const headerKey = wrapperElement?.contains(active) + ? active?.closest('th')?.getAttribute('data-column') + : null; + const pageAction = wrapperElement?.contains(active) + ? active?.getAttribute('data-page-action') + : null; + currentSpec = candidate; + // Cancel any in-flight animations before re-rendering + if (cleanupAnimations) { + cleanupAnimations(); + cleanupAnimations = null; + } - // Clean up previous render - if (wrapperElement?.parentNode) { - wrapperElement.parentNode.removeChild(wrapperElement); - wrapperElement = null; - } + // Clean up previous keyboard nav + if (cleanupKeyboard) { + cleanupKeyboard(); + cleanupKeyboard = null; + } - currentLayout = compile(); - const shouldAnimate = isFirstRender && !!currentLayout.animation?.enter; - wrapperElement = renderTable(currentLayout, container, { animate: shouldAnimate }); + // Clean up previous render + if (wrapperElement?.parentNode) { + wrapperElement.parentNode.removeChild(wrapperElement); + wrapperElement = null; + } - // Set up animation cleanup on first animated render - if (shouldAnimate && wrapperElement) { - cleanupAnimations = setupTableAnimationCleanup(wrapperElement); - } - if (isFirstRender) { - isFirstRender = false; - } + currentLayout = nextLayout; + const shouldAnimate = isFirstRender && !!currentLayout.animation?.enter; + wrapperElement = renderTable(currentLayout, container, { animate: shouldAnimate }); - // Apply dark mode class - const isDark = resolveDarkMode(options?.darkMode); - if (isDark) { - container.classList.add('oc-dark'); - } else { - container.classList.remove('oc-dark'); - } + // Set up animation cleanup on first animated render + if (shouldAnimate && wrapperElement) { + cleanupAnimations = setupTableAnimationCleanup(wrapperElement, () => { + cleanupAnimations = null; + if (pendingResize && !destroyed) { + pendingResize = false; + resize(); + } + }); + } + if (isFirstRender) { + isFirstRender = false; + } - // Apply responsive breakpoint - applyBreakpointClass(); + // Apply dark mode class + const isDark = resolveDarkMode(options?.darkMode); + if (isDark) { + container.classList.add('oc-dark'); + } else { + container.classList.remove('oc-dark'); + } - // Add clickable class if onRowClick is provided - if (options?.onRowClick) { - wrapperElement.classList.add('oc-table--clickable'); - } + // Apply responsive breakpoint + applyBreakpointClass(); - // Wire up event handlers - wireEvents(); - - // Wire up keyboard navigation - if (wrapperElement) { - cleanupKeyboard = attachKeyboardNav({ - wrapper: wrapperElement, - onSort: (columnKey: string) => { - const state = getState(); - const newSort = cycleSort(state.sort, columnKey); - updateState({ sort: newSort, page: 0 }); - - // Announce sort change - if (newSort) { - const dir = newSort.direction === 'asc' ? 'ascending' : 'descending'; - announce(`Sorted by ${columnKey} ${dir}`); - } else { - announce('Sort cleared'); - } + // Add clickable class if onRowClick is provided + if (options?.onRowClick) { + wrapperElement.classList.add('oc-table--clickable'); + } - if (!isControlled) { - rerender(); - } - }, - onClearSearch: () => { - updateState({ search: '', page: 0 }); - if (!isControlled) { - rerender(); + // Wire up event handlers + wireEvents(); + + // Wire up keyboard navigation + if (wrapperElement) { + cleanupKeyboard = attachKeyboardNav({ + wrapper: wrapperElement, + initialCell, + onRowActivate: options?.onRowClick + ? (rowId) => { + const row = currentLayout.rows.find((item) => item.id === rowId); + if (row) options.onRowClick?.(row.data); } - }, - onAnnounce: announce, - }); - } - } catch (err) { - console.error('[viz] Table render failed:', err); + : undefined, + onSort: (columnKey: string) => { + const state = getState(); + const newSort = cycleSort(state.sort, columnKey); + updateState({ sort: newSort, page: 0 }); + + // Announce sort change + if (newSort) { + const dir = newSort.direction === 'asc' ? 'ascending' : 'descending'; + announce(`Sorted by ${columnKey} ${dir}`); + } else { + announce('Sort cleared'); + } + + if (!isControlled) { + rerender(); + } + }, + onClearSearch: () => { + if (searchDebounceTimer !== null) clearTimeout(searchDebounceTimer); + searchDebounceTimer = null; + updateState({ search: '', page: 0 }); + if (!isControlled) { + rerender(); + } + wrapperElement?.querySelector('tbody')?.focus(); + announce('Search cleared'); + }, + onAnnounce: announce, + }); + } + if (bodyFocused) wrapperElement?.querySelector('tbody')?.focus(); + else if (headerKey) { + [...(wrapperElement?.querySelectorAll('th[data-column]') ?? [])] + .find((element) => element.dataset.column === headerKey) + ?.focus(); + } else if (pageAction) { + const buttons = [ + ...(wrapperElement?.querySelectorAll('[data-page-action]') ?? []), + ]; + ( + buttons.find((button) => button.dataset.pageAction === pageAction && !button.disabled) ?? + buttons.find((button) => !button.disabled) ?? + wrapperElement?.querySelector('tbody') + )?.focus(); } } @@ -457,7 +505,7 @@ export function createTable( /** * Re-render the table, preserving search input focus across the DOM rebuild. */ - function rerender(): void { + function rerender(candidate = currentSpec): void { if (destroyed) return; // Capture current search input state before re-render @@ -469,7 +517,7 @@ export function createTable( const selectionStart = searchInput?.selectionStart ?? 0; const selectionEnd = searchInput?.selectionEnd ?? 0; - render(); + render(candidate); // Restore search focus after re-render if (hadFocus) { @@ -486,13 +534,15 @@ export function createTable( function update(newSpec: TableSpec): void { if (destroyed) return; - currentSpec = newSpec; - render(); + rerender(newSpec); } function resize(): void { if (destroyed) return; - if (cleanupAnimations) return; // Skip resize during entrance animation + if (cleanupAnimations) { + pendingResize = true; + return; + } rerender(); } @@ -507,15 +557,18 @@ export function createTable( const darkMode = resolveDarkMode(options?.darkMode); const { width } = getContainerDimensions(); - const fullLayout = compileTable(currentSpec, { - width, - height: 600, - theme: options?.theme, - darkMode, - sort: resolveEffectiveSort(state), - search: state.search || undefined, - // No page/pageSize: get all rows - }); + const fullLayout = compileTable( + { ...currentSpec, pagination: false }, + { + width, + height: 600, + theme: options?.theme, + darkMode, + sort: resolveEffectiveSort(state), + search: state.search || undefined, + // No page/pageSize: get all rows + }, + ); const headers = fullLayout.columns.map((c) => c.label); const csvRows = [headers.map(csvEscape).join(',')]; diff --git a/packages/vanilla/src/table-renderer.ts b/packages/vanilla/src/table-renderer.ts index d6227682..9ef29ad4 100644 --- a/packages/vanilla/src/table-renderer.ts +++ b/packages/vanilla/src/table-renderer.ts @@ -151,6 +151,7 @@ function renderTbody(rows: TableRow[], columns: ResolvedColumn[]): HTMLTableSect const td = renderCell(cell); td.setAttribute('role', 'gridcell'); + td.setAttribute('data-column', columns[c].key); td.style.textAlign = columns[c].align; // Cards mode (below 400px) hides the header and prints these instead; // the CSS reads them through `content: attr(data-label)`. @@ -184,6 +185,7 @@ function renderTfoot( const cell = totalRow.cells[c]; const td = cell ? renderCell(cell) : document.createElement('td'); td.setAttribute('role', 'gridcell'); + td.setAttribute('data-column', columns[c].key); td.style.textAlign = columns[c].align; td.setAttribute('data-label', columns[c].label); td.setAttribute('data-priority', String(columns[c].priority)); diff --git a/packages/vanilla/src/tilemap-mount.ts b/packages/vanilla/src/tilemap-mount.ts index 18037f3f..e605e3ba 100644 --- a/packages/vanilla/src/tilemap-mount.ts +++ b/packages/vanilla/src/tilemap-mount.ts @@ -24,6 +24,12 @@ import { type JPGExportOptions, type SVGExportOptions, } from './export'; +import { + captureMarkFocus, + type NavigableMark, + restoreMarkFocus, + wireMarkNavigation, +} from './interactions/mark-navigation'; import { createMeasureText, resolveFontFamily, scheduleFontReload } from './measure-text'; import { observeResize } from './resize-observer'; import { resolveDarkMode } from './resolve-dark-mode'; @@ -155,10 +161,10 @@ export function createTileMap( // { ...spec.theme, ...options.theme }, so options.theme wins over the // spec-level theme; fall back to the container's computed font. Measuring a // different font than gets rendered desyncs layout metrics and the reload watcher. - function resolveEffectiveFont(): string { + function resolveEffectiveFont(candidate = currentSpec): string { return ( options?.theme?.fonts?.family ?? - currentSpec.theme?.fonts?.family ?? + candidate.theme?.fonts?.family ?? resolveFontFamily(container) ); } @@ -204,7 +210,7 @@ export function createTileMap( return { width, height }; } - function compile(): TileMapLayout { + function compile(candidate = currentSpec, measure = measureText): TileMapLayout { const { width, height } = getContainerDimensions(); const darkMode = resolveDarkMode(options?.darkMode); @@ -214,10 +220,10 @@ export function createTileMap( theme: options?.theme, darkMode, watermark: options?.watermark, - measureText, + measureText: measure, }; - return compileTileMap(currentSpec, compileOpts); + return compileTileMap(candidate, compileOpts); } // --------------------------------------------------------------------------- @@ -226,6 +232,8 @@ export function createTileMap( function wireTooltipAndInteraction(svg: SVGSVGElement, layout: TileMapLayout): () => void { const cleanups: Array<() => void> = []; + const navigation: NavigableMark[] = []; + let activeLeave: (() => void) | null = null; const tilesByStateCode = new Map(); for (const t of layout.tiles) { @@ -244,6 +252,8 @@ export function createTileMap( const tile = tilesByStateCode.get(stateCode); const handleMouseEnter = (e: Event) => { + activeLeave?.(); + activeLeave = handleMouseLeave; const mouseEvent = e as MouseEvent; tilesGroup?.classList.add('oc-hover-active'); el.classList.add('oc-tilemap-tile--hover'); @@ -274,6 +284,8 @@ export function createTileMap( }; const handleMouseLeave = () => { + if (activeLeave !== handleMouseLeave) return; + activeLeave = null; tilesGroup?.classList.remove('oc-hover-active'); el.classList.remove('oc-tilemap-tile--hover'); tooltipManager?.hide(); @@ -291,6 +303,13 @@ export function createTileMap( } }; + navigation.push({ + element: el, + key: stateCode, + enter: handleMouseEnter, + leave: handleMouseLeave, + activate: handleClick, + }); el.addEventListener('mouseenter', handleMouseEnter); el.addEventListener('mousemove', handleMouseMove); el.addEventListener('mouseleave', handleMouseLeave); @@ -304,7 +323,9 @@ export function createTileMap( }); } + cleanups.push(wireMarkNavigation(svg, navigation)); return () => { + activeLeave?.(); for (const cleanup of cleanups) { cleanup(); } @@ -316,6 +337,8 @@ export function createTileMap( // --------------------------------------------------------------------------- function render(animate = false): void { + const focusedMark = captureMarkFocus(svgElement); + tooltipManager?.hide(); // Cancel running animations if (animationCleanup) { animationCleanup(); @@ -349,6 +372,7 @@ export function createTileMap( // Setup animation cleanup only when actually animating if (animate && currentLayout.animation?.enter) { animationCleanup = setupAnimationCleanup(newSvg, () => { + animationCleanup = null; // On animation complete, check if resize was pending if (pendingResize && !destroyed) { pendingResize = false; @@ -367,18 +391,21 @@ export function createTileMap( fontsReloadPending = false; container.dataset.ocFontsState = 'ready'; } + if (svgElement) restoreMarkFocus(svgElement, focusedMark); } function update(newSpec: TileMapSpec): void { + if (destroyed) return; + // Compile before adopting an edit: a rejected spec must leave the last + // valid view, its interactions, and future resize/export calls intact. + const nextFont = resolveEffectiveFont(newSpec); + const nextMeasure = nextFont === fontFamily ? measureText : createMeasureText(nextFont); + const nextLayout = compile(newSpec, nextMeasure); currentSpec = newSpec; - // A new spec can change theme.fonts.family; rebuild the measurer so layout - // measures the font compile will actually render with. - const nextFont = resolveEffectiveFont(); - if (nextFont !== fontFamily) { - fontFamily = nextFont; - measureText = createMeasureText(fontFamily); - } - currentLayout = compile(); + fontFamily = nextFont; + measureText = nextMeasure; + + currentLayout = nextLayout; render(); } @@ -405,15 +432,19 @@ export function createTileMap( ): string | Promise | Promise { if (!svgElement) return ''; + // Export settled geometry, independent of transient hover and animation. + const terminalSvg = renderTileMapSVG(currentLayout, { animate: false }); + const resolvedOptions = { theme: currentLayout.theme, ...options_ }; + switch (format) { case 'svg': - return exportSVG(svgElement); + return exportSVG(terminalSvg); case 'svg-with-fonts': - return exportSVGWithFonts(svgElement); + return exportSVGWithFonts(terminalSvg, resolvedOptions); case 'png': - return exportPNG(svgElement, options_ as JPGExportOptions); + return exportPNG(terminalSvg, resolvedOptions); case 'jpg': - return exportJPG(svgElement, options_ as JPGExportOptions); + return exportJPG(terminalSvg, resolvedOptions); default: return ''; } diff --git a/packages/vanilla/src/transition/apply.ts b/packages/vanilla/src/transition/apply.ts index 50553ba0..73a80b38 100644 --- a/packages/vanilla/src/transition/apply.ts +++ b/packages/vanilla/src/transition/apply.ts @@ -106,11 +106,14 @@ export function applyTweenState( break; } case 'line': { + // Enter/exit are opacity-only: retain gaps and frozen interrupted paths. + if (tw.kind !== 'update') break; tw.ptsBuffer = interpolatePoints(tw.fromPts, tw.toPts, eased, tw.ptsBuffer); applyLinePath(tw.el, tw.ptsBuffer, tw.interpolate, tw.pathEl); break; } case 'area': { + if (tw.kind !== 'update') break; tw.topBuffer = interpolatePoints(tw.fromTop, tw.toTop, eased, tw.topBuffer); tw.bottomBuffer = interpolatePoints(tw.fromBottom, tw.toBottom, eased, tw.bottomBuffer); applyAreaPaths( @@ -231,9 +234,11 @@ export function snapTweenToFinal(tw: Tween): void { } break; case 'line': + if (tw.kind !== 'update') break; applyLinePath(tw.el, tw.toPts, tw.interpolate, tw.pathEl); break; case 'area': + if (tw.kind !== 'update') break; applyAreaPaths( tw.el, tw.toTop, diff --git a/packages/vanilla/src/transition/driver.ts b/packages/vanilla/src/transition/driver.ts index 3fdae9d0..c00a60da 100644 --- a/packages/vanilla/src/transition/driver.ts +++ b/packages/vanilla/src/transition/driver.ts @@ -428,17 +428,26 @@ export function runTransition(args: { case 'line': { // Freeze the current interpolated path string const pts = interpolatePoints(tw.fromPts, tw.toPts, easedUpdate); - const d = buildLinePath(pts, tw.interpolate); + const d = + tw.kind === 'update' + ? buildLinePath(pts, tw.interpolate) + : (tw.pathEl?.getAttribute('d') ?? ''); snap.set(key, { type: 'line', d, points: pts, pointKeys: tw.pointKeys }); break; } case 'area': { const top = interpolatePoints(tw.fromTop, tw.toTop, easedUpdate); const bottom = interpolatePoints(tw.fromBottom, tw.toBottom, easedUpdate); - const d = buildAreaPath(top, bottom, tw.interpolate); + const d = + tw.kind === 'update' + ? buildAreaPath(top, bottom, tw.interpolate) + : (tw.fillPathEl?.getAttribute('d') ?? ''); let topD: string | undefined; if (tw.hasStroke) { - topD = buildLinePath(top, tw.interpolate); + topD = + tw.kind === 'update' + ? buildLinePath(top, tw.interpolate) + : (tw.topPathEl?.getAttribute('d') ?? ''); } snap.set(key, { type: 'area', diff --git a/packages/vanilla/src/transition/svg-tweens.ts b/packages/vanilla/src/transition/svg-tweens.ts index bccb9df5..7c4a1b26 100644 --- a/packages/vanilla/src/transition/svg-tweens.ts +++ b/packages/vanilla/src/transition/svg-tweens.ts @@ -279,7 +279,11 @@ export function buildLineTweens( const prevKeySet = new Set(prevPKs); const hasSurvivors = nextPKs.some((k) => prevKeySet.has(k)); - if (!hasSurvivors) { + // Flat point arrays cannot represent missing-data breaks. Crossfade the + // canonical paths rather than inventing a segment across absent evidence. + const hasGaps = + (prev.path?.match(/M/g)?.length ?? 0) > 1 || (next.path?.match(/M/g)?.length ?? 0) > 1; + if (!hasSurvivors || hasGaps) { // Crossfade: fade out old (ghost), fade in new const ghost = renderSingleMark(prev, 0); if (ghost) { @@ -483,7 +487,11 @@ export function buildAreaTweens( const prevKeySet = new Set(prevPKs); const hasSurvivors = nextPKs.some((k) => prevKeySet.has(k)); - if (!hasSurvivors) { + // Flat point arrays cannot represent missing-data breaks. Crossfade the + // canonical paths rather than inventing a segment across absent evidence. + const hasGaps = + (prev.path?.match(/M/g)?.length ?? 0) > 1 || (next.path?.match(/M/g)?.length ?? 0) > 1; + if (!hasSurvivors || hasGaps) { // Crossfade const ghost = renderSingleMark(prev, 0); if (ghost) { diff --git a/packages/vanilla/src/you-draw-it.ts b/packages/vanilla/src/you-draw-it.ts index 20d7952e..c8808970 100644 --- a/packages/vanilla/src/you-draw-it.ts +++ b/packages/vanilla/src/you-draw-it.ts @@ -110,16 +110,14 @@ export function createYouDrawIt(options: YouDrawItOptions): YouDrawItController /** Locate the target line's in the freshly rendered SVG. */ function findTargetLinePath(svg: SVGSVGElement, seriesKey?: string): SVGPathElement | null { - // Line marks render as with a - // single child (see renderers/marks.ts). Prefer the seriesKey match; - // fall back to the first line mark for single-series charts (no color enc). - if (seriesKey) { - const escaped = - typeof CSS !== 'undefined' && typeof CSS.escape === 'function' - ? CSS.escape(seriesKey) - : seriesKey.replace(/"/g, '\\"'); - const byKey = svg.querySelector(`[data-mark-id="line-${escaped}"] path`); - if (byKey) return byKey; + // Interaction IDs identify individual marks, not series. Match the authored + // series directly so quotes or repeated names in other mark types are safe. + if (seriesKey !== undefined) { + const group = [...svg.querySelectorAll('.oc-mark-line')].find( + (element) => element.getAttribute('data-series') === seriesKey, + ); + const path = group?.querySelector('path'); + if (path) return path; } return svg.querySelector('.oc-mark-line path'); }