fix(audit): chart colours + Charts-tab layout regressions
- Bar chart now uses a single fixed fill (DEFAULT_BAR_FILL = #1d4ed8) instead of cycling Set2 across the X axis. Time-bucket bars carry no per-day semantics; one colour reads correctly. - Donut chart accepts an optional colorMap so the audit page can map success → green, denied → orange, failure → red. New semanticStatusColors export keeps consumers inside the lib's curated palette per ADR-0023. - Chart envelope and audit grid items get min-width: 0 + max-width: 100% on the inner SVG/figure, fixing the body scrollbar + footer gap caused by Plot's fixed-width fallback render on a hidden tab panel.
This commit is contained in:
@@ -5,3 +5,8 @@ export {
|
||||
type StackedBarChartInputs,
|
||||
} from './lib/stacked-bar-chart/stacked-bar-chart';
|
||||
export type { ChartBaseInputs, ColorScheme } from './lib/_internal/chart-types';
|
||||
export {
|
||||
DEFAULT_BAR_FILL,
|
||||
semanticStatusColors,
|
||||
type SemanticStatus,
|
||||
} from './lib/_internal/palette';
|
||||
|
||||
@@ -9,11 +9,25 @@
|
||||
// only carries the chart-specific tweaks (canvas dimensions,
|
||||
// chart-specific SVG element rules).
|
||||
|
||||
// Host elements default to `display: inline` for Angular custom
|
||||
// elements, which would let the inner `<figure>` escape flex/grid
|
||||
// sizing constraints from the parent container and trigger
|
||||
// horizontal overflow when a chart re-renders before the panel is
|
||||
// laid out. Forcing `display: block` makes the host fill its grid
|
||||
// cell and lets `max-width: 100%` actually take effect.
|
||||
lib-bar-chart,
|
||||
lib-donut-chart,
|
||||
lib-stacked-bar-chart {
|
||||
display: block;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.chart-figure {
|
||||
margin: 0;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 0.5rem;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.chart-caption {
|
||||
@@ -41,8 +55,31 @@
|
||||
|
||||
.chart-canvas {
|
||||
width: 100%;
|
||||
// Grid items default to `min-width: auto` which prevents shrinking
|
||||
// below the intrinsic content size — when Plot ships an `<svg>`
|
||||
// with a fixed `width="720"` attribute (because the canvas was
|
||||
// hidden via `[hidden]` on first render and `clientWidth` was 0),
|
||||
// the canvas couldn't fit in a narrower column and overflowed the
|
||||
// panel, dragging the body's vertical scrollbar with it. The pair
|
||||
// `min-width: 0` + `max-width: 100%` on the inner SVG/figure
|
||||
// forces the chart to scale down to the actual column width.
|
||||
min-width: 0;
|
||||
min-height: 16rem;
|
||||
|
||||
// Constrain Plot's rendered output. Plot returns either a bare
|
||||
// `<svg>` or a `<figure>` wrapping a legend + `<svg>`, both with
|
||||
// fixed pixel dimensions from the `width` config. The browser
|
||||
// honours the viewBox aspect ratio when we cap width to 100% and
|
||||
// let height adjust accordingly.
|
||||
figure {
|
||||
max-width: 100%;
|
||||
margin: 0;
|
||||
}
|
||||
svg {
|
||||
max-width: 100%;
|
||||
height: auto;
|
||||
}
|
||||
|
||||
// Plot's default text colour is hardcoded black; override to the
|
||||
// theme-aware grayscale so axis ticks and labels stay readable
|
||||
// in dark mode without re-rendering.
|
||||
|
||||
@@ -19,10 +19,52 @@ import { interpolateCividis, interpolateViridis, schemeSet2 } from 'd3-scale-chr
|
||||
* rule (added in this PR) blocks direct imports of
|
||||
* `d3-scale-chromatic` outside this file so the lib stays the only
|
||||
* source of palettes.
|
||||
*
|
||||
* In addition to the two scale families, the lib exposes two
|
||||
* curated colour constants:
|
||||
*
|
||||
* - {@link DEFAULT_BAR_FILL} — single fixed fill used by
|
||||
* `<lib-bar-chart>` when no per-bar encoding is requested.
|
||||
* Picked to be visually neutral against both light and dark
|
||||
* surfaces and colour-blind-safe (Wong 2011 "blue" deuteranopia-
|
||||
* tested).
|
||||
* - {@link semanticStatusColors} — small map of intent-bearing
|
||||
* colours (`success / warning / error / info / neutral`)
|
||||
* consumers can opt into via a `colorMap` on the donut chart
|
||||
* when the categorical axis carries *meaning* rather than just
|
||||
* distinct labels. Values are tuned for AA contrast on white
|
||||
* surfaces and remain distinguishable under deuteranopia /
|
||||
* protanopia simulation (red and orange differ in lightness, not
|
||||
* just hue).
|
||||
*/
|
||||
|
||||
export type ColorScheme = 'sequential' | 'categorical';
|
||||
|
||||
/**
|
||||
* Default fill colour for bars when no per-bar encoding applies.
|
||||
* `#1d4ed8` ≈ tailwind blue-700 — high contrast against white,
|
||||
* still readable on a dark surface.
|
||||
*/
|
||||
export const DEFAULT_BAR_FILL = '#1d4ed8';
|
||||
|
||||
/**
|
||||
* Intent-bearing colours for the small set of statuses APF charts
|
||||
* deal with. Consumers cannot author their own colours from outside
|
||||
* the lib (per ADR-0023), but they can pick from this curated map
|
||||
* by name — keeps the colour-blind-safety guarantee while letting
|
||||
* the donut chart encode meaning (success = green, denied =
|
||||
* orange).
|
||||
*/
|
||||
export const semanticStatusColors = {
|
||||
success: '#16a34a', // green-600
|
||||
warning: '#ea580c', // orange-600
|
||||
error: '#dc2626', // red-600
|
||||
info: '#2563eb', // blue-600
|
||||
neutral: '#6b7280', // gray-500
|
||||
} as const;
|
||||
|
||||
export type SemanticStatus = keyof typeof semanticStatusColors;
|
||||
|
||||
/**
|
||||
* Build a function that maps a numeric input in [0, 1] to a hex
|
||||
* colour from the sequential ramp. Use this for value-driven fills
|
||||
|
||||
@@ -72,6 +72,29 @@ describe('BarChart', () => {
|
||||
expect(firstTwo[1]?.textContent).toContain('Bar chart showing the count');
|
||||
});
|
||||
|
||||
it('fills every bar with the same single colour (no per-bar palette cycle)', () => {
|
||||
const fixture = render();
|
||||
const root = fixture.nativeElement as HTMLElement;
|
||||
const svg = root.querySelector<SVGSVGElement>('.chart-canvas svg');
|
||||
expect(svg).not.toBeNull();
|
||||
// Plot applies a literal-string `fill` either on the bar-mark
|
||||
// `<g>` group or on each `<rect>`. Collect every `[fill]` attr
|
||||
// present below the mark group and confirm there's only one
|
||||
// distinct value (i.e. we are no longer cycling a categorical
|
||||
// palette across the X axis). Axes / rules carry their own
|
||||
// fills (text, ticks), so we narrow to the bar-mark subtree
|
||||
// via `aria-label`, which Plot sets to "bar" on the bar group.
|
||||
const barGroup = svg?.querySelector('[aria-label="bar"]');
|
||||
expect(barGroup).not.toBeNull();
|
||||
const fills = new Set<string>();
|
||||
const groupFill = barGroup?.getAttribute('fill');
|
||||
if (groupFill) fills.add(groupFill);
|
||||
barGroup
|
||||
?.querySelectorAll<SVGElement>('[fill]')
|
||||
.forEach((el) => fills.add(el.getAttribute('fill') ?? ''));
|
||||
expect(fills.size).toBe(1);
|
||||
});
|
||||
|
||||
it('regenerates the chart when `data` changes', async () => {
|
||||
const fixture = render();
|
||||
const root = fixture.nativeElement as HTMLElement;
|
||||
|
||||
@@ -9,15 +9,9 @@ import {
|
||||
input,
|
||||
} from '@angular/core';
|
||||
import * as Plot from '@observablehq/plot';
|
||||
import {
|
||||
chartId,
|
||||
findChartSvg,
|
||||
injectSvgTitleDesc,
|
||||
prefersReducedMotion,
|
||||
resolveTheme,
|
||||
} from '../_internal/a11y';
|
||||
import { chartId, findChartSvg, injectSvgTitleDesc, prefersReducedMotion } from '../_internal/a11y';
|
||||
import type { ChartBaseInputs } from '../_internal/chart-types';
|
||||
import { paletteFor } from '../_internal/palette';
|
||||
import { DEFAULT_BAR_FILL } from '../_internal/palette';
|
||||
|
||||
/**
|
||||
* `<lib-bar-chart>` — vertical bar chart over a categorical X axis.
|
||||
@@ -26,6 +20,15 @@ import { paletteFor } from '../_internal/palette';
|
||||
* row in `data` becomes one bar with its height encoding `yKey` and
|
||||
* its position encoding `xKey`.
|
||||
*
|
||||
* Every bar is filled with the same colour by default — a single-
|
||||
* series bar chart's X axis is typically a *label* (day, bucket,
|
||||
* name) rather than a category that meaningfully encodes colour, so
|
||||
* rotating through a categorical palette adds visual noise without
|
||||
* carrying signal. Consumers that genuinely want per-bar colouring
|
||||
* (a categorical axis where each bar IS a category with semantic
|
||||
* meaning) should use `<lib-donut-chart>` instead, or pass an
|
||||
* explicit `fillColor` if they really do want a different shade.
|
||||
*
|
||||
* Per the lib's a11y contract:
|
||||
* - outer `<figure>` carries `role="img"` + `aria-labelledby` +
|
||||
* `aria-describedby`,
|
||||
@@ -33,8 +36,8 @@ import { paletteFor } from '../_internal/palette';
|
||||
* them; the helper in `_internal/a11y.ts` does),
|
||||
* - `<details>` disclosure with a `<table>` fallback ships every
|
||||
* data point in a screen-reader-friendly form,
|
||||
* - palette is resolved from `_internal/palette.ts` (Set2
|
||||
* categorical by default, Viridis sequential when requested),
|
||||
* - fill defaults to `DEFAULT_BAR_FILL` from
|
||||
* `_internal/palette.ts` (a colour-blind-safe blue),
|
||||
* - `prefers-reduced-motion` skips transitions.
|
||||
*/
|
||||
|
||||
@@ -46,6 +49,12 @@ export interface BarChartInputs<T> extends ChartBaseInputs<T> {
|
||||
/** Human-readable axis labels (no implicit i18n inside the lib). */
|
||||
readonly xLabel?: string;
|
||||
readonly yLabel?: string;
|
||||
/**
|
||||
* Override the default single fill colour. Caller-owned — keep it
|
||||
* colour-blind-safe and contrasted against the surrounding surface.
|
||||
* Most consumers should leave it unset and accept the lib default.
|
||||
*/
|
||||
readonly fillColor?: string;
|
||||
}
|
||||
|
||||
@Component({
|
||||
@@ -70,7 +79,7 @@ export class BarChart<T extends Record<string, unknown>> {
|
||||
readonly ariaLabel = input.required<string>();
|
||||
readonly xLabel = input<string | undefined>(undefined);
|
||||
readonly yLabel = input<string | undefined>(undefined);
|
||||
readonly colorScheme = input<'sequential' | 'categorical'>('categorical');
|
||||
readonly fillColor = input<string>(DEFAULT_BAR_FILL);
|
||||
|
||||
protected readonly chartId = chartId();
|
||||
protected readonly titleId = `${this.chartId}-title`;
|
||||
@@ -105,7 +114,7 @@ export class BarChart<T extends Record<string, unknown>> {
|
||||
const yKey = this.yKey();
|
||||
const xLabel = this.xLabel() ?? null;
|
||||
const yLabel = this.yLabel() ?? null;
|
||||
const palette = paletteFor(this.colorScheme(), resolveTheme(), Math.max(1, data.length));
|
||||
const fill = this.fillColor();
|
||||
|
||||
const plot = Plot.plot({
|
||||
// Mirrors the host element's available width so the chart is
|
||||
@@ -115,12 +124,13 @@ export class BarChart<T extends Record<string, unknown>> {
|
||||
marginBottom: 40,
|
||||
x: { label: xLabel, type: 'band' },
|
||||
y: { label: yLabel, grid: true, nice: true },
|
||||
color: { type: 'ordinal', range: palette },
|
||||
marks: [
|
||||
Plot.barY(data as Plot.Data, {
|
||||
x: xKey,
|
||||
y: yKey,
|
||||
fill: xKey,
|
||||
// Literal-string fill — Plot interprets non-key strings as
|
||||
// a fixed colour. Same shade on every bar, by design.
|
||||
fill,
|
||||
// Plot's default tooltip would require interaction wiring
|
||||
// we haven't shipped yet — the tabular fallback under
|
||||
// `<details>` is the v1 substitute for hover values.
|
||||
|
||||
@@ -75,4 +75,30 @@ describe('DonutChart', () => {
|
||||
expect(rows[0]?.querySelector('th')?.textContent?.trim()).toBe('success');
|
||||
expect(rows[0]?.querySelector('td')?.textContent?.trim()).toBe('120');
|
||||
});
|
||||
|
||||
it('honours an optional `colorMap` to colour slices by category', () => {
|
||||
TestBed.configureTestingModule({ imports: [DonutChart] });
|
||||
const fixture = TestBed.createComponent<DonutChart<OutcomeRow>>(DonutChart);
|
||||
fixture.componentRef.setInput('data', SAMPLE);
|
||||
fixture.componentRef.setInput('categoryKey', 'outcome');
|
||||
fixture.componentRef.setInput('valueKey', 'count');
|
||||
fixture.componentRef.setInput('caption', 'Outcome breakdown');
|
||||
fixture.componentRef.setInput('description', 'Donut chart of audit-event outcomes.');
|
||||
fixture.componentRef.setInput('ariaLabel', 'Outcome breakdown — donut chart');
|
||||
fixture.componentRef.setInput('colorMap', {
|
||||
success: '#16a34a',
|
||||
failure: '#dc2626',
|
||||
denied: '#ea580c',
|
||||
});
|
||||
fixture.detectChanges();
|
||||
|
||||
const root = fixture.nativeElement as HTMLElement;
|
||||
const slices = root.querySelectorAll<SVGPathElement>('.chart-canvas svg path');
|
||||
// SAMPLE order: success, failure, denied — fills must match the
|
||||
// map exactly so admins read "green = success, orange = denied"
|
||||
// without consulting the legend.
|
||||
expect(slices[0]?.getAttribute('fill')).toBe('#16a34a');
|
||||
expect(slices[1]?.getAttribute('fill')).toBe('#dc2626');
|
||||
expect(slices[2]?.getAttribute('fill')).toBe('#ea580c');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -36,6 +36,16 @@ export interface DonutChartInputs<T> extends ChartBaseInputs<T> {
|
||||
readonly valueKey: keyof T & string;
|
||||
/** Optional centre-of-donut text (e.g. the total). */
|
||||
readonly centerLabel?: string;
|
||||
/**
|
||||
* Optional per-category fill override. When the keys on the X
|
||||
* axis carry intent (e.g. audit outcomes — `success` MUST read
|
||||
* as green, `denied` as orange), pass a map from category value
|
||||
* to colour. Categories not in the map fall back to the
|
||||
* colour-blind-safe categorical palette. Use the
|
||||
* `semanticStatusColors` export from this lib to stay within the
|
||||
* curated colour set (per ADR-0023).
|
||||
*/
|
||||
readonly colorMap?: Readonly<Record<string, string>>;
|
||||
}
|
||||
|
||||
@Component({
|
||||
@@ -56,6 +66,7 @@ export class DonutChart<T extends Record<string, unknown>> {
|
||||
readonly ariaLabel = input.required<string>();
|
||||
readonly centerLabel = input<string | undefined>(undefined);
|
||||
readonly colorScheme = input<'sequential' | 'categorical'>('categorical');
|
||||
readonly colorMap = input<Readonly<Record<string, string>> | undefined>(undefined);
|
||||
|
||||
protected readonly chartId = chartId();
|
||||
protected readonly titleId = `${this.chartId}-title`;
|
||||
@@ -85,6 +96,7 @@ export class DonutChart<T extends Record<string, unknown>> {
|
||||
const categoryKey = this.categoryKey();
|
||||
const valueKey = this.valueKey();
|
||||
const palette = paletteFor(this.colorScheme(), resolveTheme(), Math.max(1, data.length));
|
||||
const colorMap = this.colorMap();
|
||||
|
||||
const width = canvas.clientWidth || 320;
|
||||
const height = 240;
|
||||
@@ -108,7 +120,13 @@ export class DonutChart<T extends Record<string, unknown>> {
|
||||
arcs.forEach((slice, i) => {
|
||||
const path = canvas.ownerDocument.createElementNS(svgNs, 'path');
|
||||
path.setAttribute('d', arcGen(slice) ?? '');
|
||||
path.setAttribute('fill', palette[i % palette.length] ?? '#888');
|
||||
// Semantic override first (when the consumer maps a category
|
||||
// to an intent-bearing colour), then palette cycle as a
|
||||
// fallback for any unmapped category. Final `#888` is a
|
||||
// defensive default that should never actually trigger.
|
||||
const category = String(slice.data[categoryKey]);
|
||||
const fill = colorMap?.[category] ?? palette[i % palette.length] ?? '#888';
|
||||
path.setAttribute('fill', fill);
|
||||
path.setAttribute('stroke', resolveTheme() === 'dark' ? '#111827' : '#ffffff');
|
||||
path.setAttribute('stroke-width', '1.5');
|
||||
// Each slice gets its own <title> child so hovering announces
|
||||
|
||||
Reference in New Issue
Block a user