diff --git a/app/e2e/design-system.spec.ts b/app/e2e/design-system.spec.ts index da6c9d4b..961e4bd3 100644 --- a/app/e2e/design-system.spec.ts +++ b/app/e2e/design-system.spec.ts @@ -108,8 +108,14 @@ test.describe("Design system — Deep Ocean palette & accessibility", () => { const chartEl = preview.locator("[data-testid='base-chart']"); await expect(chartEl).toHaveAttribute("role", "img"); - // ECharts AriaComponent auto-generates a descriptive label from chart data - await expect(chartEl).toHaveAttribute("aria-label", /This is a chart/); + // BarChart auto-derives an aria-label that names the chart kind plus + // the underlying data shape — categories × series — so AT users get + // something more useful than the generic "Chart visualization" or + // ECharts' default "This is a chart" template. + await expect(chartEl).toHaveAttribute( + "aria-label", + /Bar chart with \d+ categories and \d+ series/, + ); }); // ── Colorblind mode toggle ──────────────────────────────────────────── diff --git a/app/src/components/table-renderer.tsx b/app/src/components/table-renderer.tsx index 984c3c74..ed44e8e5 100644 --- a/app/src/components/table-renderer.tsx +++ b/app/src/components/table-renderer.tsx @@ -11,6 +11,7 @@ import { resolveThresholdColor, resolveStylingRuleColor, interpolateColor, + contrastTextColor, } from "@neoboard/components"; import type { StylingRule, ColorScaleConfig } from "@neoboard/components"; import type { ColumnDef } from "@tanstack/react-table"; @@ -25,20 +26,6 @@ const AGG_SYMBOLS: Record = { max: "max", }; -/** Return black or white text based on background luminance for readability. */ -function contrastTextColor(hex: string): string { - const c = hex.replace("#", ""); - const r = parseInt(c.substring(0, 2), 16) / 255; - const g = parseInt(c.substring(2, 4), 16) / 255; - const b = parseInt(c.substring(4, 6), 16) / 255; - // Relative luminance (WCAG formula) - const lum = - 0.2126 * (r <= 0.03928 ? r / 12.92 : ((r + 0.055) / 1.055) ** 2.4) + - 0.7152 * (g <= 0.03928 ? g / 12.92 : ((g + 0.055) / 1.055) ** 2.4) + - 0.0722 * (b <= 0.03928 ? b / 12.92 : ((b + 0.055) / 1.055) ** 2.4); - return lum > 0.179 ? "#000000" : "#ffffff"; -} - export interface TableRendererProps { data: unknown; settings?: Record; @@ -299,35 +286,61 @@ export function TableRenderer({ return ; } + // Avoid the pagination "flash" on first render: when pagination is enabled + // we depend on the measured container height to compute the page size. If + // we render before the ResizeObserver fires, DataGrid mounts with the + // default `pageSize=10`, then immediately re-renders with the dynamic size — + // which visibly snaps the row count. Render an empty wrapper on the first + // tick instead so the observer can measure, then commit a single DataGrid. + // Treat 0/negative as "not ready" too — the wrapper can momentarily measure + // to 0 before layout settles, which would otherwise trigger the same snap. + const hasUsableHeight = containerHeight !== undefined && containerHeight > 0; + const awaitingHeight = enablePagination && !hasUsableHeight; + + // Derive a screen-reader description from data shape — the underlying + // has no top-level label and the wrapper is otherwise just a + // scroll container, so AT users hit it with no context. + const columnCount = records.length ? Object.keys(records[0]).length : 0; + const ariaLabel = + (settings.ariaLabel as string | undefined) ?? + `Table with ${records.length} rows and ${columnCount} columns`; + return ( -
- []} - enableSorting={enableSorting} - enableColumnResizing={enableColumnResizing} - enableSelection={settings.enableSelection as boolean | undefined} - enableGlobalFilter={settings.enableGlobalFilter !== false} - enableColumnFilters={settings.enableColumnFilters !== false} - enablePagination={enablePagination} - pageSize={(settings.pageSize as number) ?? 10} - containerHeight={enablePagination ? containerHeight : undefined} - onCellClick={onCellClick} - clickableColumns={clickableColumns} - getRowStyle={getRowStyle} - getCellStyle={getCellStyle} - enableGrouping={enableGrouping} - initialGrouping={initialGrouping} - pagination={(table) => ( -
- -
- +
+ {awaitingHeight ? null : ( + []} + enableSorting={enableSorting} + enableColumnResizing={enableColumnResizing} + enableSelection={settings.enableSelection as boolean | undefined} + enableColumnFilters={settings.enableColumnFilters !== false} + enablePagination={enablePagination} + pageSize={(settings.pageSize as number) ?? 10} + containerHeight={ + enablePagination && hasUsableHeight ? containerHeight : undefined + } + onCellClick={onCellClick} + clickableColumns={clickableColumns} + getRowStyle={getRowStyle} + getCellStyle={getCellStyle} + enableGrouping={enableGrouping} + initialGrouping={initialGrouping} + pagination={(table) => ( +
+ +
+ +
-
- )} - /> -
+ )} + /> + )} + ); } diff --git a/app/src/plugins/bar/transform.ts b/app/src/plugins/bar/transform.ts index 7ba8ed6c..e0b50603 100644 --- a/app/src/plugins/bar/transform.ts +++ b/app/src/plugins/bar/transform.ts @@ -7,6 +7,8 @@ import { resolveLabelKey, resolveValueKeys, normalizeValue, + collectAllKeys, + toSeriesNumber, type ColumnMapping, } from "../transforms/shared-utils"; @@ -14,6 +16,10 @@ import { * Transform to bar chart format: [{ label, series1, series2 }] * When mapping is provided, uses mapped columns; otherwise uses positional defaults. * Always applies normalizeValue to labels for consistent type handling. + * + * Series keys are collected from the *union* of all rows so sparse data + * (where some series only appear in later rows) isn't silently dropped. + * Cell values use `toSeriesNumber` to preserve missing-vs-zero distinction. */ export function transformToBarData( data: unknown, @@ -21,7 +27,7 @@ export function transformToBarData( ): unknown { const records = toRecords(data); if (!records.length) return []; - const keys = Object.keys(records[0]); + const keys = collectAllKeys(records); if (keys.length < 2) return []; const labelKey = resolveLabelKey(keys, mapping); @@ -32,7 +38,7 @@ export function transformToBarData( label: String(normalizeValue(r[labelKey]) ?? ""), }; for (const k of valueKeys) { - point[k] = Number(r[k]) || 0; + point[k] = toSeriesNumber(r[k]); } return point; }); @@ -45,7 +51,7 @@ export function transformToBarData( export function validateBarData(data: unknown): string | null { const records = toRecords(data); if (!records.length) return null; - const cols = Object.keys(records[0]).length; + const cols = collectAllKeys(records).length; if (cols < 2) return `Bar chart requires at least 2 columns: first column for category labels (x-axis) and one or more columns for numeric values (y-axis). Your query returned only ${cols} column(s). Example: \`SELECT category, count FROM ...\``; return null; diff --git a/app/src/plugins/line/transform.ts b/app/src/plugins/line/transform.ts index ae877a38..6c007203 100644 --- a/app/src/plugins/line/transform.ts +++ b/app/src/plugins/line/transform.ts @@ -7,6 +7,8 @@ import { resolveLabelKey, resolveValueKeys, normalizeValue, + collectAllKeys, + toSeriesNumber, type ColumnMapping, } from "../transforms/shared-utils"; @@ -14,6 +16,11 @@ import { * Transform to line chart format: [{ x, series1, series2 }] * When mapping is provided, uses mapped columns; otherwise uses positional defaults. * Always applies normalizeValue to x-axis values for consistent type handling. + * + * Series keys are collected from the *union* of all rows so sparse data + * (where some series only appear in later rows) isn't silently dropped. + * Cell values use `toSeriesNumber` to preserve missing-vs-zero distinction; + * downstream `connectNulls` controls whether gaps are bridged. */ export function transformToLineData( data: unknown, @@ -21,7 +28,7 @@ export function transformToLineData( ): unknown { const records = toRecords(data); if (!records.length) return []; - const keys = Object.keys(records[0]); + const keys = collectAllKeys(records); if (keys.length < 2) return []; const xKey = resolveLabelKey(keys, mapping); @@ -30,7 +37,7 @@ export function transformToLineData( return records.map((r) => { const point: Record = { x: normalizeValue(r[xKey]) }; for (const k of seriesKeys) { - point[k] = Number(r[k]) || 0; + point[k] = toSeriesNumber(r[k]); } return point; }); @@ -43,7 +50,7 @@ export function transformToLineData( export function validateLineData(data: unknown): string | null { const records = toRecords(data); if (!records.length) return null; - const cols = Object.keys(records[0]).length; + const cols = collectAllKeys(records).length; if (cols < 2) return `Line chart requires at least 2 columns: first column for x-axis values (dates, numbers, or labels) and one or more columns for numeric series. Your query returned only ${cols} column(s). Example: \`SELECT date, revenue FROM ...\``; return null; diff --git a/app/src/plugins/transforms/__tests__/bar.test.ts b/app/src/plugins/transforms/__tests__/bar.test.ts index 7fccf8b1..9d432a7a 100644 --- a/app/src/plugins/transforms/__tests__/bar.test.ts +++ b/app/src/plugins/transforms/__tests__/bar.test.ts @@ -37,12 +37,43 @@ describe("transformToBarData", () => { expect(result[0].s2).toBe(2); }); - it("coerces non-numeric values to 0", () => { + it("maps non-numeric values to null (preserved as gap, not 0)", () => { + // Previously coerced to 0 — that hid bad data. Returning null lets + // ECharts render a gap and keeps the missing-vs-zero distinction. const data = [{ cat: "X", value: "not-a-number" }]; - const result = transformToBarData(data) as Array<{ value: number }>; + const result = transformToBarData(data) as Array<{ value: number | null }>; + expect(result[0].value).toBeNull(); + }); + + it("preserves numeric zero (regression: zero must not become null)", () => { + const data = [{ cat: "X", value: 0 }]; + const result = transformToBarData(data) as Array<{ value: number | null }>; expect(result[0].value).toBe(0); }); + it("maps null/undefined cells to null (missing data, not 0)", () => { + const data = [ + { cat: "A", value: null }, + { cat: "B", value: undefined }, + ]; + const result = transformToBarData(data) as Array<{ value: number | null }>; + expect(result[0].value).toBeNull(); + expect(result[1].value).toBeNull(); + }); + + it("unions series keys across rows so sparse series are not dropped", () => { + // s2 is absent from the first row — before the fix, transformToBarData + // would only emit { label, s1 } and silently lose the s2 series. + const data = [ + { cat: "X", s1: 1 }, + { cat: "Y", s1: 2, s2: 9 }, + ]; + const result = transformToBarData(data) as Array>; + expect(result[0].s1).toBe(1); + expect(result[0].s2).toBeNull(); + expect(result[1].s2).toBe(9); + }); + it("respects column mapping", () => { const data = [{ a: 1, b: 2, c: 3 }]; const result = transformToBarData(data, { diff --git a/app/src/plugins/transforms/__tests__/line.test.ts b/app/src/plugins/transforms/__tests__/line.test.ts index 76f32506..497ee203 100644 --- a/app/src/plugins/transforms/__tests__/line.test.ts +++ b/app/src/plugins/transforms/__tests__/line.test.ts @@ -24,12 +24,43 @@ describe("transformToLineData", () => { expect(transformToLineData([{ x: 1 }])).toEqual([]); }); - it("coerces non-numeric series values to 0", () => { + it("maps non-numeric series values to null (preserved as gap, not 0)", () => { + // Previously coerced to 0 — that hid bad data. Returning null lets + // ECharts render a gap and keeps the missing-vs-zero distinction. const data = [{ x: "Jan", y: "bad" }]; - const result = transformToLineData(data) as Array<{ y: number }>; + const result = transformToLineData(data) as Array<{ y: number | null }>; + expect(result[0].y).toBeNull(); + }); + + it("preserves numeric zero (regression: zero must not become null)", () => { + const data = [{ x: "Jan", y: 0 }]; + const result = transformToLineData(data) as Array<{ y: number | null }>; expect(result[0].y).toBe(0); }); + it("maps null/undefined cells to null (missing data, not 0)", () => { + const data = [ + { x: "Jan", y: null }, + { x: "Feb", y: undefined }, + ]; + const result = transformToLineData(data) as Array<{ y: number | null }>; + expect(result[0].y).toBeNull(); + expect(result[1].y).toBeNull(); + }); + + it("unions series keys across rows so sparse series are not dropped", () => { + // y2 only appears in the second row — before the fix, transformToLineData + // would only emit { x, y1 } and silently lose the y2 series. + const data = [ + { x: "Jan", y1: 1 }, + { x: "Feb", y1: 2, y2: 9 }, + ]; + const result = transformToLineData(data) as Array>; + expect(result[0].y1).toBe(1); + expect(result[0].y2).toBeNull(); + expect(result[1].y2).toBe(9); + }); + it("converts Date objects in x-axis", () => { const data = [{ date: new Date("2024-06-01T00:00:00Z"), revenue: 100 }]; const result = transformToLineData(data) as Array<{ x: unknown }>; diff --git a/app/src/plugins/transforms/__tests__/shared.test.ts b/app/src/plugins/transforms/__tests__/shared.test.ts index df10a8fa..20a2d1d0 100644 --- a/app/src/plugins/transforms/__tests__/shared.test.ts +++ b/app/src/plugins/transforms/__tests__/shared.test.ts @@ -1,5 +1,11 @@ import { describe, it, expect } from "vitest"; -import { toRecords, resolveLabelKey, resolveValueKeys } from "../shared-utils"; +import { + toRecords, + resolveLabelKey, + resolveValueKeys, + collectAllKeys, + toSeriesNumber, +} from "../shared-utils"; describe("toRecords", () => { it("returns array data unchanged", () => { @@ -62,3 +68,61 @@ describe("resolveValueKeys", () => { expect(resolveValueKeys(["a", "b"], "a", { yAxis: [] })).toEqual(["b"]); }); }); + +describe("collectAllKeys", () => { + it("returns the union of keys across all rows in first-seen order", () => { + expect( + collectAllKeys([ + { a: 1, b: 2 }, + { b: 3, c: 4 }, + { a: 5, d: 6 }, + ]), + ).toEqual(["a", "b", "c", "d"]); + }); + + it("returns an empty array for an empty record list", () => { + expect(collectAllKeys([])).toEqual([]); + }); + + it("does not duplicate keys that appear in multiple rows", () => { + expect(collectAllKeys([{ a: 1 }, { a: 2 }, { a: 3 }])).toEqual(["a"]); + }); +}); + +describe("toSeriesNumber", () => { + it("preserves finite numbers including zero and negatives", () => { + expect(toSeriesNumber(0)).toBe(0); + expect(toSeriesNumber(42)).toBe(42); + expect(toSeriesNumber(-3.14)).toBe(-3.14); + }); + + it("parses numeric strings", () => { + expect(toSeriesNumber("10")).toBe(10); + expect(toSeriesNumber("0")).toBe(0); + expect(toSeriesNumber("-2.5")).toBe(-2.5); + }); + + it("returns null for null, undefined and empty string (missing data)", () => { + expect(toSeriesNumber(null)).toBeNull(); + expect(toSeriesNumber(undefined)).toBeNull(); + expect(toSeriesNumber("")).toBeNull(); + }); + + it("returns null for whitespace-only strings (Number(' ') === 0 otherwise)", () => { + // Without this, Number(" ") returns 0 and the value silently masquerades + // as a real zero on the chart. + expect(toSeriesNumber(" ")).toBeNull(); + expect(toSeriesNumber("\t")).toBeNull(); + expect(toSeriesNumber("\n")).toBeNull(); + }); + + it("returns null for non-numeric strings instead of silently giving 0", () => { + expect(toSeriesNumber("not-a-number")).toBeNull(); + expect(toSeriesNumber("NaN")).toBeNull(); + }); + + it("returns null for Infinity / NaN", () => { + expect(toSeriesNumber(Number.POSITIVE_INFINITY)).toBeNull(); + expect(toSeriesNumber(Number.NaN)).toBeNull(); + }); +}); diff --git a/app/src/plugins/transforms/shared-utils.ts b/app/src/plugins/transforms/shared-utils.ts index cbc0b317..aace4de9 100644 --- a/app/src/plugins/transforms/shared-utils.ts +++ b/app/src/plugins/transforms/shared-utils.ts @@ -48,3 +48,36 @@ export function resolveValueKeys( } return keys.filter((k) => k !== labelKey); } + +/** + * Collect the union of keys across every record. Using only `Object.keys(records[0])` + * silently drops series that happen to be absent from the first row (sparse data). + */ +export function collectAllKeys(records: Record[]): string[] { + const seen = new Set(); + const ordered: string[] = []; + for (const r of records) { + for (const k of Object.keys(r)) { + if (!seen.has(k)) { + seen.add(k); + ordered.push(k); + } + } + } + return ordered; +} + +/** + * Coerce a raw cell value to a numeric series value, preserving the + * distinction between "missing" (null) and an actual zero. Returns `null` + * for null/undefined inputs and for values that can't be parsed as a finite + * number; ECharts renders nulls as gaps rather than masquerading them as 0. + */ +export function toSeriesNumber(raw: unknown): number | null { + if (raw === null || raw === undefined) return null; + // Whitespace-only strings would otherwise coerce to 0 via Number(" "), + // hiding what is really a missing cell behind a fake zero. + if (typeof raw === "string" && raw.trim() === "") return null; + const n = typeof raw === "number" ? raw : Number(raw); + return Number.isFinite(n) ? n : null; +} diff --git a/component/src/charts/__tests__/bar-chart.test.tsx b/component/src/charts/__tests__/bar-chart.test.tsx index eb24fea6..6c5cc78e 100644 --- a/component/src/charts/__tests__/bar-chart.test.tsx +++ b/component/src/charts/__tests__/bar-chart.test.tsx @@ -233,4 +233,41 @@ describe("BarChart", () => { expect(opts.series[0].data[0]).toBe(0); expect(opts.series[1].data[0]).toBe(0); }); + + it("auto-derives a descriptive aria-label from data shape (single series)", () => { + // Default "Chart visualization" is unhelpful for screen-reader users. + // The container should reflect the actual data — categories × series. + render(); + expect( + screen.getByLabelText(/bar chart with 3 categories and 1 series/i), + ).toBeInTheDocument(); + }); + + it("auto-derived aria-label lists series names for multi-series", () => { + render(); + const el = screen.getByTestId("base-chart"); + const label = el.getAttribute("aria-label") ?? ""; + expect(label).toMatch(/bar chart with 3 categories and 2 series/i); + expect(label).toContain("sales"); + expect(label).toContain("returns"); + }); + + it("explicit ariaDescription prop overrides the auto-derived label", () => { + render( + , + ); + expect( + screen.getByLabelText("Quarterly product revenue"), + ).toBeInTheDocument(); + }); + + it("auto-derived aria-label handles empty data without crashing", () => { + render(); + const el = screen.getByTestId("base-chart"); + // Empty charts should still have a meaningful label + expect(el.getAttribute("aria-label")).toMatch(/bar chart/i); + }); }); diff --git a/component/src/charts/__tests__/contrast-text-color.test.ts b/component/src/charts/__tests__/contrast-text-color.test.ts new file mode 100644 index 00000000..d6a18e48 --- /dev/null +++ b/component/src/charts/__tests__/contrast-text-color.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect } from "vitest"; +import { contrastTextColor } from "../chart-utils"; + +describe("contrastTextColor", () => { + it("returns white text on a dark background", () => { + expect(contrastTextColor("#000000")).toBe("#ffffff"); + expect(contrastTextColor("#222222")).toBe("#ffffff"); + expect(contrastTextColor("#0000ff")).toBe("#ffffff"); + }); + + it("returns black text on a light background", () => { + expect(contrastTextColor("#ffffff")).toBe("#000000"); + expect(contrastTextColor("#eeeeee")).toBe("#000000"); + expect(contrastTextColor("#ffff00")).toBe("#000000"); + }); + + it("accepts shorthand #rgb hex", () => { + expect(contrastTextColor("#000")).toBe("#ffffff"); + expect(contrastTextColor("#fff")).toBe("#000000"); + expect(contrastTextColor("#0f0")).toBe("#000000"); + }); + + it("accepts uppercase hex", () => { + expect(contrastTextColor("#FFFFFF")).toBe("#000000"); + expect(contrastTextColor("#AABBCC")).toBe("#000000"); + }); + + it("parses rgb() and rgba() color strings", () => { + expect(contrastTextColor("rgb(0, 0, 0)")).toBe("#ffffff"); + expect(contrastTextColor("rgb(255, 255, 255)")).toBe("#000000"); + expect(contrastTextColor("rgba(10, 10, 10, 0.5)")).toBe("#ffffff"); + }); + + it("parses percentage components in rgb()", () => { + expect(contrastTextColor("rgb(0%, 0%, 0%)")).toBe("#ffffff"); + expect(contrastTextColor("rgb(100%, 100%, 100%)")).toBe("#000000"); + }); + + it("falls back to black for unparseable inputs instead of producing invisible text", () => { + // Previously these would crash or silently parse to NaN and emit white + // text — invisible on light backgrounds set by the same styling rule. + expect(contrastTextColor("red")).toBe("#000000"); + expect(contrastTextColor("var(--accent)")).toBe("#000000"); + expect(contrastTextColor("hsl(0, 100%, 50%)")).toBe("#000000"); + expect(contrastTextColor("")).toBe("#000000"); + expect(contrastTextColor("garbage")).toBe("#000000"); + }); + + it("rejects malformed hex strings", () => { + expect(contrastTextColor("#12")).toBe("#000000"); + expect(contrastTextColor("#12345")).toBe("#000000"); + expect(contrastTextColor("#1234567")).toBe("#000000"); + expect(contrastTextColor("#zzz")).toBe("#000000"); + }); + + it("rejects malformed rgb() inputs", () => { + expect(contrastTextColor("rgb(0, 0)")).toBe("#000000"); + expect(contrastTextColor("rgb(a, b, c)")).toBe("#000000"); + }); + + it("rejects rgb()/rgba() with wrong arity (extra channels are not silently dropped)", () => { + // Previously rgb(1,2,3,4,5) parsed the first 3 channels and ignored the + // rest, accepting clearly malformed input. + expect(contrastTextColor("rgb(1, 2, 3, 4)")).toBe("#000000"); + expect(contrastTextColor("rgb(1, 2, 3, 4, 5)")).toBe("#000000"); + expect(contrastTextColor("rgba(1, 2, 3)")).toBe("#000000"); + expect(contrastTextColor("rgba(1, 2, 3, 0.5, 9)")).toBe("#000000"); + }); + + it("rejects rgba() with alpha outside [0, 1]", () => { + expect(contrastTextColor("rgba(0, 0, 0, 2)")).toBe("#000000"); + expect(contrastTextColor("rgba(0, 0, 0, -0.1)")).toBe("#000000"); + expect(contrastTextColor("rgba(0, 0, 0, foo)")).toBe("#000000"); + }); + + it("accepts valid rgba() with in-range alpha", () => { + // Sanity: the stricter validator must not regress valid inputs. + expect(contrastTextColor("rgba(0, 0, 0, 0)")).toBe("#ffffff"); + expect(contrastTextColor("rgba(255, 255, 255, 1)")).toBe("#000000"); + }); +}); diff --git a/component/src/charts/__tests__/line-chart.test.tsx b/component/src/charts/__tests__/line-chart.test.tsx index 7d3bc140..aa18d86a 100644 --- a/component/src/charts/__tests__/line-chart.test.tsx +++ b/component/src/charts/__tests__/line-chart.test.tsx @@ -377,4 +377,37 @@ describe("LineChart", () => { expect(optionsCall.series[1].yAxisIndex).toBe(0); expect(Array.isArray(optionsCall.yAxis)).toBe(true); }); + + // --- Accessibility: auto-derived aria description --- + + it("auto-derives a descriptive aria-label from data shape (single series)", () => { + // Default "Chart visualization" is unhelpful for screen-reader users. + // The container should reflect the actual data — points × series. + render(); + expect( + screen.getByLabelText(/line chart with 3 points and 1 series/i), + ).toBeInTheDocument(); + }); + + it("auto-derived aria-label lists series names for multi-series", () => { + render(); + const el = screen.getByTestId("base-chart"); + const label = el.getAttribute("aria-label") ?? ""; + expect(label).toMatch(/line chart with 3 points and 2 series/i); + expect(label).toContain("revenue"); + expect(label).toContain("cost"); + }); + + it("explicit ariaDescription prop overrides the auto-derived label", () => { + render( + , + ); + expect(screen.getByLabelText("Monthly revenue trend")).toBeInTheDocument(); + }); + + it("auto-derived aria-label handles empty data without crashing", () => { + render(); + const el = screen.getByTestId("base-chart"); + expect(el.getAttribute("aria-label")).toMatch(/line chart/i); + }); }); diff --git a/component/src/charts/bar-chart.tsx b/component/src/charts/bar-chart.tsx index c78f4f8b..f812e407 100644 --- a/component/src/charts/bar-chart.tsx +++ b/component/src/charts/bar-chart.tsx @@ -4,6 +4,7 @@ import { BaseChart } from "./base-chart"; import type { BaseChartProps, BarChartDataPoint } from "./types"; import { useContainerSize } from "@/hooks/useContainerSize"; import { + buildAutoAriaDescription, buildEmptyDataOption, getCompactState, resolveShowLegend, @@ -77,6 +78,7 @@ function BarChart({ referenceLines: referenceLinesJson, stylingRules, paramValues, + ariaDescription, ...rest }: BarChartProps) { const { width, height, containerRef } = useContainerSize(); @@ -91,7 +93,18 @@ function BarChart({ const options = useMemo((): EChartsOption => { if (!data.length) return buildEmptyDataOption(); - const seriesKeys = Object.keys(data[0]).filter((k) => k !== "label"); + // Union keys across every row so sparse data (a series missing from the + // first row) doesn't get dropped from the chart. + const seenKeys = new Set(); + const seriesKeys: string[] = []; + for (const row of data) { + for (const k of Object.keys(row)) { + if (k !== "label" && !seenKeys.has(k)) { + seenKeys.add(k); + seriesKeys.push(k); + } + } + } // Pre-compute row totals for percentage normalization const rowTotals = isPercent @@ -215,9 +228,17 @@ function BarChart({ hideLegend, ]); + // Auto-derive a screen-reader description from the data shape so the + // generic "Chart visualization" fallback is only used when the chart is + // truly empty. Callers can still pass an explicit ariaDescription to + // override (e.g., a widget title that already conveys the meaning). + const effectiveAria = + ariaDescription ?? + buildAutoAriaDescription("Bar chart", data, "label", "categories"); + return (
- +
); } diff --git a/component/src/charts/chart-utils.ts b/component/src/charts/chart-utils.ts index 19a621a8..f5cf8578 100644 --- a/component/src/charts/chart-utils.ts +++ b/component/src/charts/chart-utils.ts @@ -74,6 +74,80 @@ export function formatNumber( return `${prefix}${formatted}${suffix}`; } +// --------------------------------------------------------------------------- +// Contrast text color (WCAG luminance) +// --------------------------------------------------------------------------- + +/** + * Pick black or white text for readability against an arbitrary background + * color. Accepts `#rgb`, `#rrggbb`, or `rgb()` / `rgba()` strings. Anything + * unparseable (named colors, CSS variables, gradients, garbage) falls back to + * black — the old call site silently produced invisible white-on-light text + * when fed an `rgb()` value. + */ +export function contrastTextColor(color: string): string { + const rgb = parseColorToRgb(color); + if (!rgb) return "#000000"; + const [r, g, b] = rgb.map((c) => { + const v = c / 255; + return v <= 0.03928 ? v / 12.92 : ((v + 0.055) / 1.055) ** 2.4; + }); + const lum = 0.2126 * r + 0.7152 * g + 0.0722 * b; + return lum > 0.179 ? "#000000" : "#ffffff"; +} + +function parseHexColor(s: string): [number, number, number] | null { + const hex = /^#([0-9a-f]{3}|[0-9a-f]{6})$/i.exec(s); + if (!hex) return null; + const h = hex[1]; + if (h.length === 3) { + return [ + Number.parseInt(h[0] + h[0], 16), + Number.parseInt(h[1] + h[1], 16), + Number.parseInt(h[2] + h[2], 16), + ]; + } + return [ + Number.parseInt(h.slice(0, 2), 16), + Number.parseInt(h.slice(2, 4), 16), + Number.parseInt(h.slice(4, 6), 16), + ]; +} + +function parseRgbChannel(p: string): number | null { + const n = p.endsWith("%") ? (Number(p.slice(0, -1)) / 100) * 255 : Number(p); + if (!Number.isFinite(n)) return null; + return Math.max(0, Math.min(255, n)); +} + +function parseRgbFunctionColor(s: string): [number, number, number] | null { + const rgb = /^(rgb|rgba)\(([^)]+)\)$/i.exec(s); + if (!rgb) return null; + const fn = rgb[1].toLowerCase(); + const parts = rgb[2].split(",").map((p) => p.trim()); + // Strict arity: rgb() needs exactly 3 components, rgba() exactly 4 — + // anything else (e.g. rgb(1,2,3,4,5)) is malformed and should fall back. + const expected = fn === "rgb" ? 3 : 4; + if (parts.length !== expected) return null; + const channels: number[] = []; + for (let i = 0; i < 3; i++) { + const n = parseRgbChannel(parts[i]); + if (n === null) return null; + channels.push(n); + } + if (fn === "rgba") { + const alpha = Number(parts[3]); + if (!Number.isFinite(alpha) || alpha < 0 || alpha > 1) return null; + } + return [channels[0], channels[1], channels[2]]; +} + +function parseColorToRgb(input: string): [number, number, number] | null { + if (typeof input !== "string") return null; + const s = input.trim(); + return parseHexColor(s) ?? parseRgbFunctionColor(s); +} + // --------------------------------------------------------------------------- // HTML escaping for tooltip content (prevents XSS via database values) // --------------------------------------------------------------------------- @@ -335,6 +409,41 @@ export function buildEmptyDataOption(): EChartsOption { }; } +/** + * Auto-derive a screen-reader description from a chart's data shape. + * Used by bar/line/etc. when the caller does not pass an explicit + * ariaDescription — replaces the generic ECharts "This is a chart" + * fallback with something that names the rows, series count and series. + * + * @param chartType Human-readable chart kind, e.g. "Bar chart" + * @param data The row array passed to the chart + * @param labelKey The row key that holds the X / category label + * (excluded from the series-key enumeration) + * @param rowNoun What a row represents — "categories" / "points" / etc. + */ +export function buildAutoAriaDescription( + chartType: string, + data: Record[], + labelKey: string, + rowNoun: string, +): string { + if (!data.length) return `${chartType} with no data`; + const seen = new Set(); + const seriesKeys: string[] = []; + for (const row of data) { + for (const k of Object.keys(row)) { + if (k !== labelKey && !seen.has(k)) { + seen.add(k); + seriesKeys.push(k); + } + } + } + const seriesPart = seriesKeys.length + ? `${seriesKeys.length} series: ${seriesKeys.join(", ")}` + : "0 series"; + return `${chartType} with ${data.length} ${rowNoun} and ${seriesPart}`; +} + /** * Compact/responsive breakpoints used consistently across all ECharts components. * A chart is "compact" when its container is narrower than 300px. diff --git a/component/src/charts/index.ts b/component/src/charts/index.ts index 96ddc071..81334607 100644 --- a/component/src/charts/index.ts +++ b/component/src/charts/index.ts @@ -13,6 +13,7 @@ export { } from "./theme"; export { COLOR_PALETTES, getPaletteColors } from "./palettes"; export type { ColorPalette } from "./palettes"; +export { contrastTextColor } from "./chart-utils"; export type { ColorThreshold } from "./color-threshold"; export { parseColorThresholds, resolveThresholdColor } from "./color-threshold"; export type { diff --git a/component/src/charts/line-chart.tsx b/component/src/charts/line-chart.tsx index 7ee92de5..7fdda5be 100644 --- a/component/src/charts/line-chart.tsx +++ b/component/src/charts/line-chart.tsx @@ -4,6 +4,7 @@ import { BaseChart } from "./base-chart"; import type { BaseChartProps, LineChartDataPoint } from "./types"; import { useContainerSize } from "@/hooks/useContainerSize"; import { + buildAutoAriaDescription, buildEmptyDataOption, getCompactState, resolveShowLegend, @@ -66,6 +67,37 @@ export interface LineChartProps extends Omit { * - Below 300px wide: hides axis labels, tightens grid margins * - Below 200px tall: hides legend */ +/** + * Collect series keys (every non-"x" column) in first-seen order across rows. + * Lifted out of the options builder so the memo body stays under the + * cognitive-complexity budget. + */ +function collectSeriesKeys(data: LineChartDataPoint[]): string[] { + const seen = new Set(); + const keys: string[] = []; + for (const row of data) { + for (const k of Object.keys(row)) { + if (k !== "x" && !seen.has(k)) { + seen.add(k); + keys.push(k); + } + } + } + return keys; +} + +/** Find the most recent numeric value for `key`, scanning from the tail. */ +function findLastNumericValue( + data: LineChartDataPoint[], + key: string, +): number | undefined { + for (let i = data.length - 1; i >= 0; i -= 1) { + const candidate = data[i][key]; + if (typeof candidate === "number") return candidate; + } + return undefined; +} + function LineChart({ data, xAxisLabel, @@ -84,6 +116,7 @@ function LineChart({ paramValues, rightAxisSeries, rightYAxisLabel, + ariaDescription, samplingThreshold = 1000, samplingMethod = "lttb", ...rest @@ -94,18 +127,21 @@ function LineChart({ const options = useMemo((): EChartsOption => { if (!data.length) return buildEmptyDataOption(); - const seriesKeys = Object.keys(data[0]).filter((k) => k !== "x"); + const seriesKeys = collectSeriesKeys(data); const effectiveShowLegend = resolveShowLegend( showLegend, seriesKeys.length, hideLegend, ); - const refLines = parseReferenceLines(referenceLinesJson); - const markLine = buildMarkLineFromRefs(refLines); + const markLine = buildMarkLineFromRefs( + parseReferenceLines(referenceLinesJson), + ); const xValues = data.map((d) => d.x); const useTimeAxis = isTimeSeriesData(xValues); const rightAxisSet = new Set(rightAxisSeries ?? []); const useDualAxis = rightAxisSet.size > 0; + const useSampling = + samplingThreshold > 0 && data.length > samplingThreshold; const leftYAxis = { type: "value" as const, @@ -125,6 +161,37 @@ function LineChart({ splitLine: { show: false }, }; + const buildSeries = (key: string, idx: number) => { + const lastValue = findLastNumericValue(data, key); + const seriesColor = + lastValue !== undefined + ? resolveItemColor(lastValue, stylingRules, paramValues) + : undefined; + return { + name: key, + type: "line" as const, + yAxisIndex: useDualAxis && rightAxisSet.has(key) ? 1 : 0, + data: useTimeAxis + ? data.map((d) => [d.x, d[key]]) + : data.map((d) => d[key] as number), + smooth, + step: stepped ? ("start" as const) : undefined, + connectNulls, + endLabel: endLabel ? { show: true, formatter: "{a}" } : undefined, + lineStyle: { width: lineWidth, color: seriesColor }, + itemStyle: seriesColor ? { color: seriesColor } : undefined, + showSymbol: showPoints, + areaStyle: area ? {} : undefined, + emphasis: seriesKeys.length > 1 ? { focus: "series" as const } : {}, + // LTTB downsampling for large datasets + ...(useSampling + ? { sampling: samplingMethod as "lttb" | "average" | "max" | "min" } + : {}), + // Attach reference lines to the first series only + ...(idx === 0 && markLine ? { markLine } : {}), + }; + }; + return { tooltip: { trigger: "axis", formatter: buildTooltipFormatter() }, legend: effectiveShowLegend ? { bottom: 0 } : undefined, @@ -142,43 +209,7 @@ function LineChart({ axisLabel: { show: !compact }, }, yAxis: useDualAxis ? [leftYAxis, rightYAxis] : leftYAxis, - series: seriesKeys.map((key, idx) => { - let lastValue: number | undefined; - for (let i = data.length - 1; i >= 0; i -= 1) { - const candidate = data[i][key]; - if (typeof candidate === "number") { - lastValue = candidate; - break; - } - } - const seriesColor = - lastValue !== undefined - ? resolveItemColor(lastValue, stylingRules, paramValues) - : undefined; - return { - name: key, - type: "line" as const, - yAxisIndex: useDualAxis && rightAxisSet.has(key) ? 1 : 0, - data: useTimeAxis - ? data.map((d) => [d.x, d[key]]) - : data.map((d) => d[key] as number), - smooth, - step: stepped ? ("start" as const) : undefined, - connectNulls, - endLabel: endLabel ? { show: true, formatter: "{a}" } : undefined, - lineStyle: { width: lineWidth, color: seriesColor }, - itemStyle: seriesColor ? { color: seriesColor } : undefined, - showSymbol: showPoints, - areaStyle: area ? {} : undefined, - emphasis: seriesKeys.length > 1 ? { focus: "series" as const } : {}, - // LTTB downsampling for large datasets - ...(samplingThreshold > 0 && data.length > samplingThreshold - ? { sampling: samplingMethod as "lttb" | "average" | "max" | "min" } - : {}), - // Attach reference lines to the first series only - ...(idx === 0 && markLine ? { markLine } : {}), - }; - }), + series: seriesKeys.map((key, idx) => buildSeries(key, idx)), }; }, [ data, @@ -204,9 +235,17 @@ function LineChart({ samplingMethod, ]); + // Auto-derive a screen-reader description from the data shape so the + // generic "Chart visualization" fallback is only used when the chart is + // truly empty. Callers can still pass an explicit ariaDescription to + // override (e.g., a widget title that already conveys the meaning). + const effectiveAria = + ariaDescription ?? + buildAutoAriaDescription("Line chart", data, "x", "points"); + return (
- +
); } diff --git a/component/src/charts/single-value-chart.tsx b/component/src/charts/single-value-chart.tsx index 01097578..7ead0c2b 100644 --- a/component/src/charts/single-value-chart.tsx +++ b/component/src/charts/single-value-chart.tsx @@ -2,26 +2,13 @@ import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { cn } from "@/lib/utils"; import type { StylingRule } from "./styling-rule"; import { resolveStylingRuleColor } from "./styling-rule"; -import { formatNumber } from "./chart-utils"; +import { formatNumber, contrastTextColor } from "./chart-utils"; import type { NumberFormat } from "./chart-utils"; export type { ColorThreshold } from "./color-threshold"; export type SingleValueFontSize = "sm" | "md" | "lg" | "xl"; export type SingleValueNumberFormat = NumberFormat; -/** Return black or white text based on background luminance for readability. */ -function contrastTextColor(hex: string): string { - const c = hex.replace("#", ""); - const r = parseInt(c.substring(0, 2), 16) / 255; - const g = parseInt(c.substring(2, 4), 16) / 255; - const b = parseInt(c.substring(4, 6), 16) / 255; - const lum = - 0.2126 * (r <= 0.03928 ? r / 12.92 : ((r + 0.055) / 1.055) ** 2.4) + - 0.7152 * (g <= 0.03928 ? g / 12.92 : ((g + 0.055) / 1.055) ** 2.4) + - 0.0722 * (b <= 0.03928 ? b / 12.92 : ((b + 0.055) / 1.055) ** 2.4); - return lum > 0.179 ? "#000000" : "#ffffff"; -} - const FONT_SIZE_CLASS: Record = { sm: "text-xl", md: "text-2xl", diff --git a/component/src/components/composed/__tests__/chart-options-schema.test.ts b/component/src/components/composed/__tests__/chart-options-schema.test.ts index 3cc13f4b..1fa5d0f0 100644 --- a/component/src/components/composed/__tests__/chart-options-schema.test.ts +++ b/component/src/components/composed/__tests__/chart-options-schema.test.ts @@ -56,10 +56,11 @@ describe("getChartOptions", () => { const keys = getChartOptions("table").map((o) => o.key); expect(keys).toContain("enableSorting"); expect(keys).toContain("enableSelection"); - expect(keys).toContain("enableGlobalFilter"); expect(keys).toContain("enableColumnFilters"); expect(keys).toContain("pageSize"); expect(keys).toContain("emptyMessage"); + // enableGlobalFilter was removed — DataGrid never rendered a search input. + expect(keys).not.toContain("enableGlobalFilter"); }); it("groupBy option has type column-multi-select", () => { @@ -175,10 +176,10 @@ describe("getDefaultChartSettings", () => { it("returns correct defaults for table chart", () => { const d = getDefaultChartSettings("table"); expect(d.enableSorting).toBe(true); - expect(d.enableGlobalFilter).toBe(true); expect(d.enableColumnFilters).toBe(true); expect(d.pageSize).toBe(10); expect(d.emptyMessage).toBe("No results"); + expect(d.enableGlobalFilter).toBeUndefined(); }); it("returns empty object for unknown type", () => { diff --git a/component/src/components/composed/chart-options/table.ts b/component/src/components/composed/chart-options/table.ts index 1cf0d73b..5cefe76f 100644 --- a/component/src/components/composed/chart-options/table.ts +++ b/component/src/components/composed/chart-options/table.ts @@ -18,14 +18,9 @@ export const tableOptions: ChartOptionDef[] = [ category: "Features", description: "Allow selecting individual rows by clicking them.", }, - { - key: "enableGlobalFilter", - label: "Global Search", - type: "boolean", - default: true, - category: "Features", - description: "Show a search box that filters all rows across all columns.", - }, + // NOTE: `enableGlobalFilter` was removed — the schema advertised a "Global + // Search" toggle but DataGrid never rendered a search input, so toggling + // it had no observable effect. Per-column filters cover the filtering need. { key: "enableColumnFilters", label: "Column Filters",