fix(audit): chart colours + Charts-tab layout regressions (#175)
CI / check (push) Successful in 6m42s
CI / commits (push) Has been skipped
CI / scan (push) Successful in 3m14s
CI / a11y (push) Successful in 2m38s
CI / perf (push) Successful in 4m33s

## Summary

Follow-up polish on the audit-log Charts tab shipped in #174. Three small but visible regressions surfaced once real data was loaded:

1. **"Events per day" bar chart cycled a categorical palette across the X axis** — every day got a different colour, which read as "categorical meaning per day" when the X axis is just a time bucket. Switched to a single fixed fill (curated blue from the lib palette).
2. **"Outcome breakdown" donut painted slices in Set2 order** — `success` came out teal, `denied` came out orange. With orange = "danger" in most readers' mental model, this is the wrong way around. Added a `colorMap` input on `<lib-donut-chart>` and a curated `semanticStatusColors` export so the audit page can map `success → green`, `denied → orange`, `failure → red`.
3. **Charts tab caused a body scrollbar + empty space below the footer.** Plot renders into a hidden `[hidden]` panel on first switch, so `canvas.clientWidth` is 0 and Plot falls back to its 600/720 default — the resulting fixed-width SVG was wider than the grid column, dragged horizontal overflow up the tree, and (because the grid item defaults to `min-width: auto`) wouldn't shrink. The shell layout broke. Fixed by constraining the chart envelope and the grid items so the SVG can scale down to the actual column width.

## What lands

### Bar chart — single fill, lib-owned default

- `Plot.barY` mark no longer encodes colour from `xKey`. The `fill` parameter is now a literal-string colour, applied uniformly to every bar.
- New `fillColor` input on `<lib-bar-chart>` for the rare case a consumer needs a different shade; defaults to `DEFAULT_BAR_FILL = '#1d4ed8'` (≈ Tailwind blue-700, picked for AA contrast against both light and dark surfaces, colour-blind-safe).
- Removed the now-unused `color: { type: 'ordinal', range: palette }` scale and the `colorScheme` input on the bar chart — the categorical palette only made sense when `fill: xKey` was the default behaviour.

### Donut chart — optional semantic mapping

- New `colorMap?: Readonly<Record<string, string>>` input on `<lib-donut-chart>`. When provided, slice fill resolves to `colorMap[category]` first, falling back to the categorical palette for any unmapped category. Lib still owns the palette per ADR-0023; the map only constrains *which colour the consumer picks for which category*, not what colours exist.
- New `semanticStatusColors` export from `shared-charts` — a curated 5-entry map (`success / warning / error / info / neutral`) tuned for AA contrast and deuteranopia/protanopia distinguishability. Consumers stay inside the lib's palette without authoring their own hex codes.
- The audit page now ships `outcomeColorMap = { success: green, failure: red, denied: orange }` and passes it to the donut. The donut's `description` (screen-reader fallback) and the legend chip semantics now line up.

### Chart envelope — overflow containment

`libs/shared/charts/src/lib/_internal/chart-envelope.scss`:

- Force `display: block` + `min-width: 0` on every chart host element (`lib-bar-chart`, `lib-donut-chart`, `lib-stacked-bar-chart`). Angular custom-element hosts default to `display: inline`, which lets the inner `<figure>` escape parent sizing constraints — the root cause of the body-scrollbar symptom.
- `.chart-canvas` gets `min-width: 0` and constrains every inner `figure { max-width: 100% }` + `svg { max-width: 100%; height: auto }`. The SVG keeps its viewBox aspect ratio while scaling down to the actual column width.

`apps/portal-admin/src/app/pages/audit/audit.scss`:

- `.chart-tile { min-width: 0; }` — grid items default to `min-width: auto`, which prevents shrinking below the intrinsic content width. Without it, Plot's fixed-width SVG could still push the grid column past `1fr` even with the lib-side fixes.

## Notes for the reviewer

- **Why a hardcoded hex (`#1d4ed8`) for `DEFAULT_BAR_FILL` rather than a brand token?** The chart lib is intentionally decoupled from `libs/shared/tokens` — it has no SCSS/CSS-vars dependency and Plot writes the fill as an SVG attribute, not a CSS value, so a CSS variable wouldn't apply anyway. The hex stays inside the lib, marked as the canonical default, with the docstring promising AA contrast + colour-blind safety. If the brand wants to take it over later, swap the constant in one place.
- **`semanticStatusColors` is a curated map, not an open extension point.** Five entries (`success / warning / error / info / neutral`) covers the audit module and any future status-bearing donut (user list, integrations health, etc.). Adding a sixth entry needs a small PR + an a11y-contrast check, which is the right friction.
- **The `aria-label="bar"` selector in the new bar-chart spec.** Plot tags its bar-mark `<g>` with `aria-label="bar"` (and similarly `"rule"` for `Plot.ruleY`). Selecting on it scopes the fill-uniqueness check to actual bars, not axes / ticks / labels. If Plot changes that label upstream the spec breaks loudly — preferable to a fragile geometric selector.
- **Lazy fetch policy and the empty-state edge case** from #174 are unchanged. The donut still receives `colorMap` even when `data` is empty; the lib's `arcs.forEach` short-circuits on zero arcs so no colour lookup happens.
- **`audit.scss` is now 7.5 KB**, still over the 6 KB component-style warning (untouched from #174) and well under the 8 KB error. The new `.chart-tile { min-width: 0 }` rule is two lines.

## Test plan

- [x] `pnpm nx test shared-charts` — **15 specs pass** (was 14: +1 donut `colorMap` test, +1 bar single-fill test, -1 obsolete bar palette assumption).
- [x] `pnpm nx test portal-admin` — **62 specs pass** (unchanged from #174 — semantic mapping is a template-only change, behavioural tests still hold).
- [x] `pnpm nx run-many -t lint test build -p portal-shell,portal-admin,portal-bff,shared-charts` — clean. Same three pre-existing lint warnings, no new ones; bundle sizes unchanged within ±0.1 KB.
- [ ] **Manual smoke** — `pnpm nx serve portal-admin` + `pnpm nx serve portal-bff`, sign in with `Portal.Admin`, navigate to `/admin/audit`:
  - Switch to **Charts** tab — daily-volume bars all render the same blue.
  - Donut centre matches `s.total`, the green slice is `success`, the orange slice is `denied`, the red slice is `failure`. Hover each slice — `<title>` shows `<category>: <count>` (untouched a11y contract from the lib).
  - Body scrollbar stays absent; footer stays anchored at the viewport bottom with no white-space gap.
  - Resize the window through the `(max-width: 800px)` breakpoint — grid collapses to one column, charts re-flow without horizontal overflow.
  - Dark mode toggle — colours stay readable, no contrast regressions.

## What's next

Nothing in this chantier. The audit dashboard is now visually coherent and layout-stable. Two background items to track separately when the CMS / user-list pages land their own dashboards:

- Tab-state URL persistence (carried forward from #174's "what's next").
- Promote the WAI-ARIA tab pattern out of `audit.html` into `libs/shared/ui/tabs/` once a second consumer appears.

---------

Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr>
Reviewed-on: #175
This commit was merged in pull request #175.
This commit is contained in:
2026-05-17 01:17:33 +02:00
parent 9f5106b805
commit 9e7eb4af15
10 changed files with 201 additions and 17 deletions
@@ -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