admin: draw the overview charts with visx
the hand-written scale, tick, stacking and arc math is replaced by visx 4.0.0 primitives; rendering, colours and themes stay the app's own. all four charts share one hover treatment: the client chart gains the tooltip and dimming the query timeline had, the donuts gain both, an open tooltip follows a data refresh instead of going stale, and it retires when the window rolls. the timeline's third series is named allowed instead of other, and the client chart's other aggregate disappears from a window where it counted nothing. licenses gain the isc text for the bundled d3 modules.
This commit is contained in:
@@ -0,0 +1,190 @@
|
||||
# Milestone 35: visx charts
|
||||
|
||||
Replace the hand-rolled chart layout math in the admin Overview with visx primitives, and unify the chart components' duplicated plumbing. Owner decision 2026-08-24 after a measured bake-off (recharts +377,355 pre-gzip bytes, nivo +332,686, visx +74,382 against ~103,000 bytes of budget room; visx is the only candidate that fits the 800,000-byte gate). The charts must read as the same charts afterward: same palette, same geometry, same accessible surfaces.
|
||||
|
||||
## Sessions
|
||||
|
||||
One session. Admin SPA plus the license records; no Zig source, no API changes.
|
||||
|
||||
---
|
||||
|
||||
## Session 1: port the Overview charts to visx
|
||||
|
||||
### 1.1 Dependencies and records
|
||||
|
||||
Add to `admin/package.json` under `dependencies` (NOT devDependencies), pinned exact (no `^`): `@visx/scale@4.0.0`, `@visx/shape@4.0.0`, `@visx/group@4.0.0`, `@visx/axis@4.0.0`, `@visx/grid@4.0.0`, `@visx/tooltip@4.0.0`. Nothing else — in particular NOT `@visx/responsive` (the app keeps its own resize hook) and NOT `@visx/text`, `@visx/legend`, `@visx/xychart`.
|
||||
|
||||
Three separate record obligations, each updated from its own evidence, not from this spec's package list:
|
||||
|
||||
1. The sourcemap-derived bundled set that `npm run assert-bundled` checks — regenerate/extend it from the actual build output (expect the transitive `@visx/{bounds,curve,point,text,vendor}` to appear).
|
||||
2. The lockfile-derived npm runtime closure that the Zig drift gate checks — update from the lockfile.
|
||||
3. `licenses/inventory.zon` + `licenses/dependency-identity.txt` + license-text files under `licenses/` for every shipped package. The direct `@visx` packages are MIT; `@visx/vendor` is `MIT AND ISC` (it re-exports ISC-licensed d3 modules) — record that exact expression and include the ISC text.
|
||||
|
||||
### 1.2 Shared chart plumbing — new `admin/src/features/overview/chartKit.tsx`
|
||||
|
||||
Owns what is today duplicated or divergent between `TimeseriesChart.tsx` and `ClientChart.tsx`:
|
||||
|
||||
- `useMeasuredWidth()` — extracted once from the two near-identical hooks. Contract, matching both current hooks: reads `clientWidth` in the mount `useEffect` (NOT `useLayoutEffect` — keep the current timing), observes subsequent changes via `ResizeObserver`, disconnects on cleanup, and falls back to 640 when the measured width is 0 (jsdom). Covered by a test for the zero-width fallback.
|
||||
- A y-scale factory: exported function returning `scaleLinear({ domain: [0, max], range, nice: true })`. The `[0, max]` values in 1.3 are INPUT data domains; nicening mutates them. Consumers pass `numTicks={5}` to `AxisLeft` as a hint. Kit test, exact: input domain `[0, 1780]`, range `[240, 0]` → effective domain `[0, 1800]` and `scale.ticks(5)` equal to `[0, 500, 1000, 1500]`.
|
||||
- Axis wrappers over `AxisBottom`/`AxisLeft` applying the app's tick text styles. Current layout constants are preserved exactly unless named here: chart height 240, margins `{top: 8, right: 8, bottom: 22, left: 44}`. Axis chrome matches today's rendering, not visx defaults: `AxisLeft` hides its axis line and tick marks entirely; `AxisBottom` keeps only the existing baseline — no tick marks. Y labels keep the existing compact-number formatting; x labels keep the existing time formatter driven by `bucket_seconds`. X-label thinning preserves the current algorithm verbatim: `step = max(1, ceil(bucketCount * 90 / plotWidth))`, ticks at indices where `i % step === 0` — kit test with this exact fixture: 168 hourly buckets (ts = 0, 3600, ...) at `plotWidth = 748` → `step = 21`, tick indices `[0, 21, 42, 63, 84, 105, 126, 147]`, tick timestamps `[0, 75600, 151200, 226800, 302400, 378000, 453600, 529200]`. Exact axis attributes (in the spec, not implementer-chosen): `AxisLeft hideAxisLine hideTicks` with tick labels 6px left of the plot edge (current position); `AxisBottom hideTicks tickLength={0}` with `tickLabelProps` `dy="16px"` and `textAnchor="middle"` — the sanctioned baseline+14 → baseline+16 change. Kit test asserts the axis group transform plus the label `y`/`dy` attributes that produce baseline + 16px, and the y-label -6px offset.
|
||||
- `GridRows` and `AxisLeft` MUST share tick positions: compute `yScale.ticks(5)` once and pass the same array as `tickValues` to both (each labeled tick owns its grid line, as today). Test: grid-line y positions equal labeled-tick y positions.
|
||||
- `GridRows`, horizontal only, stroked with the app's existing grid color token.
|
||||
- One tooltip: `useTooltip` + `TooltipWithBounds`, app tokens, no transition. Content contract for both bar charts: the bucket's formatted time, the total, and one row per displayed series with its label, swatch color, and value.
|
||||
|
||||
### 1.3 The three charts
|
||||
|
||||
Port in place; file names, props, and data contracts unchanged, with one exception below (`DonutSlice`). Parents and existing tests keep working.
|
||||
|
||||
- `TimeseriesChart.tsx` — `scaleBand` (x) + the kit y-scale + `BarStack` for blocked/cached/other. Y domain: `[0, max(bucket.queries)]`, and `other = max(0, queries - blocked - cached)` per bucket (test the `blocked + cached > queries` clamp). Band gap: the current chart draws a fixed ~2px gap regardless of bucket count; a constant `paddingInner` cannot do that, so compute it per render: `paddingInner = min(0.5, 2 * bucketCount / plotWidth)`. Segment separator: 1px stroke of the `colors.surface` token, applied only when the bar is wider than 3px (current behavior, TimeseriesChart.tsx:249). Keep the empty-state ("No queries in this period.") exactly; coverage messaging is OWNED BY OverviewPage (via useOverviewWindow), not this component — do not add coverage logic here.
|
||||
- `ClientChart.tsx` — same band/linear scales and `BarStack`, per-client series plus Other. Y domain: `[0, max over buckets of (sum of all client values + other)]` (test it). X domain derived independently from its own props — `data.since + index * data.bucket_seconds` for `data.other.length` buckets — which is equivalent to the timeseries' domain from `buckets[].ts` because the API aligns them; do NOT add cross-chart props or page-level scale coordination. Identical margins/ranges to the timeseries. Gains the shared tooltip and dimming (below).
|
||||
- `Donut.tsx` — `@visx/shape`'s `Pie` for arc generation, our own `<path>` emission. Pie config pins the deleted donutLayout contract: filter `value <= 0` slices before the Pie, sorting disabled (preserve caller order), no pad angle, clockwise from twelve o'clock, outer radius 90, inner radius 54, shares computed from the drawn positive total, and the single-positive-slice full ring must render. Keep the 1px `colors.surfaceRaised` arc outline (an existing test pins it). `DonutSlice` moves: `export interface DonutSlice` from `Donut.tsx`, and `OverviewPage.tsx:28` imports it from `./Donut`.
|
||||
|
||||
Interaction contract (both bar charts, identical): a full-plot-height transparent hit target per bucket; hovered bucket shows the shared tooltip; non-active buckets dim to opacity 0.55; tooltip and dimming clear on pointer leave; ALL bare SVG `<title>` hover text is removed from BOTH charts (the timeseries currently has both a tooltip and `<title>` at TimeseriesChart.tsx:269 — the `<title>`s go).
|
||||
|
||||
Accessibility contract (matches current tests, do not "improve" it): the two bar-chart SVGs KEEP `role="img"` and their `aria-label` (asserted in TimeseriesChart.test.tsx:32 and OverviewPage.test.tsx:382), with the visually-hidden tables as the detailed equivalent; only the donut SVG is `aria-hidden` + `focusable="false"` with the visible legend and hidden table as its surface.
|
||||
|
||||
Color contract: data-series colors are exempt from the token rule, and each chart keeps ITS existing source — they are not unified onto one mapping. TimeseriesChart keeps its fixed category constants exactly as today (including blue `#3b82f6` for Other — NOT `seriesColor(OTHER_KEY)`, which is gray). ClientChart uses `seriesColor(clientKey)` / `seriesColor(OTHER_KEY)`. Donut renders `fill={slice.color}` from its prop contract — never recomputed from `slice.key`; a page-level test separately asserts `OverviewPage` builds slice colors with `seriesColor(...)`. Never rank/order-based anywhere. Everything non-data (axes, grid, text, tooltip chrome, separators) uses StyleX tokens; light/dark themes keep working. No animation anywhere.
|
||||
|
||||
### 1.4 Deletions
|
||||
|
||||
`chartLayout.ts`, `chartLayout.test.ts`, `donutLayout.ts`, `donutLayout.test.ts` are deleted. Their SEMANTIC contracts do not die with them — they move (see 1.5). Only assertions about hand-written path/coordinate output are dropped. Any still-needed helper with no visx equivalent moves into `chartKit.tsx`; nothing imports the deleted modules afterward; no dead exports kept "just in case".
|
||||
|
||||
### 1.5 Tests
|
||||
|
||||
Current reality (do not assume more): the only dedicated chart component suite is `TimeseriesChart.test.tsx`; ClientChart and Donut are covered via `OverviewPage.test.tsx`; identity colors are pinned on the pure mapping and a legend swatch, not on rendered fills.
|
||||
|
||||
- Preserve and port: `TimeseriesChart.test.tsx`, the page-level suites (`OverviewPage.test.tsx` incl. the one-coverage-notice test), the color-mapping tests.
|
||||
- New dedicated suites for `ClientChart` and `Donut`.
|
||||
- Re-home the deleted layout contracts: `other = max(0, queries - blocked - cached)` and zero-data handling (timeseries suite); label thinning (kit suite); zero-slice filtering and caller order may be asserted on Pie inputs/config, but twelve-o'clock clockwise start and the single-positive-slice full ring MUST be asserted on the rendered non-empty `<path>` `d` output (donut suite). Shares: a dedicated donut test with values `3` and `1` asserts rendered shares `75.0%` and `25.0%` in both the legend and the hidden table.
|
||||
- New: rendered `<rect>`/`<path>` fills for a fixed input match each chart's color contract from 1.3 (timeseries constants, ClientChart `seriesColor`, donut `slice.color`).
|
||||
- New: both bar charts show the shared tooltip with the 1.2 content contract on simulated pointer events, dim inactive buckets to 0.55, clear on pointer leave; neither chart contains an SVG `<title>`. The pointer test MUST target the transparent per-bucket overlay rect (not a visible segment) and assert the overlay's `y`/`height` span the full plot height, including the space above a short stack.
|
||||
- New: kit tests — zero-width fallback of `useMeasuredWidth`, the y-scale factory's exact domain and ticks for max=1780, the pinned x-label `dy`.
|
||||
|
||||
### 1.6 Acceptance criteria
|
||||
|
||||
Run from `admin/` with npm resolved via mise (`PATH="$HOME/.local/share/mise/shims:$PATH"` or `mise exec -- npm ...`):
|
||||
|
||||
- [ ] `npm test` exit 0
|
||||
- [ ] `npm run typecheck`, `npm run lint`, `npm run format:check` exit 0
|
||||
- [ ] `npm run build` exit 0; bundle stays under the unchanged 800,000-byte gate (expected ≈ 770,000)
|
||||
- [ ] `npm run assert-bundled` exit 0 with the updated ledger
|
||||
- [ ] `! rg 'chartLayout|donutLayout' admin/src` succeeds, and `test ! -e admin/src/features/overview/chartLayout.ts && test ! -e admin/src/features/overview/chartLayout.test.ts && test ! -e admin/src/features/overview/donutLayout.ts && test ! -e admin/src/features/overview/donutLayout.test.ts` succeeds
|
||||
- [ ] `zig build test` exit 0 from the repo root (no-breakage check; the license/ledger updates are inside it)
|
||||
|
||||
## File Ownership
|
||||
|
||||
Session 1 owns: `admin/package.json`, `admin/package-lock.json`, the bundled-set and runtime-closure ledgers, `licenses/inventory.zon`, `licenses/dependency-identity.txt`, new license-text files under `licenses/`, `admin/src/features/overview/*` (TimeseriesChart, ClientChart, Donut, their tests, chartKit new, chartLayout/donutLayout + tests deleted), the one-line `DonutSlice` import in `OverviewPage.tsx`, `specs/milestone-35.md` (implementation notes).
|
||||
|
||||
## Anti-Requirements
|
||||
|
||||
- No direct dependency on, or application import from, `@visx/responsive`, `@visx/legend`, `@visx/text`, `@visx/xychart` (primitives only). `@visx/text` arriving transitively through `@visx/axis` is expected and fine.
|
||||
- No animation, no react-spring, no transition on the tooltip.
|
||||
- No new chart types, no visual redesign beyond the sanctioned x-label `dy` fix and the band-gap formula. The charts should read as the same charts.
|
||||
- No budget raise; no changes to `assert-bundle-size.mjs`.
|
||||
- No accessibility "upgrades" beyond the stated contract (bar SVGs keep `role="img"`).
|
||||
- No Zig source changes, no API changes, nothing outside `admin/`, `licenses/`, and this spec.
|
||||
|
||||
### Session 1 implementation notes (post-build)
|
||||
|
||||
- Bundle: `admin/dist/assets` is **776,477 bytes** across 40 files, 23,523 under the unchanged 800,000-byte gate. `assert-bundle-size.mjs` untouched.
|
||||
- `DonutSlice` now lives in `Donut.tsx`; `layoutDonut`/`layoutTimeseries`/`layoutStacked`/`niceTicks`/`isEmptyTimeseries` are gone, not re-homed. The empty-timeseries check is one `every` in `TimeseriesChart`; tick generation is `scale.ticks(5)`.
|
||||
- `chartKit.tsx` exports `plotArea`, `useMeasuredWidth`, `valueScale`/`valueTicks`, `bandScale`/`bandPaddingInner`, `labelTickValues`, `formatBucketTime`, `EmptyChart`, `ChartRoot`, `ChartFrame`, `StackSegment`, `BucketOverlay`, `slotCenter`, `useActiveBucket`, `ChartTooltip`. The two charts share all of it; nothing is exported that only one caller uses.
|
||||
- **Divergence — the bundled set.** 1.1 predicted `@visx/{bounds,curve,point,text,vendor}` transitively. The sourcemap build shows `@visx/{bounds,point,text}` but NOT `@visx/curve` (nothing here draws a curve) and NOT `@visx/vendor`: it is a re-export shim and rollup resolves straight through it to the d3 packages. What ships instead is nine d3 modules (`d3-array`, `d3-color`, `d3-format`, `d3-interpolate`, `d3-path`, `d3-scale`, `d3-shape`, `d3-time`, `internmap`) plus `classnames`, `balanced-match`, `math-expression-evaluator`, `reduce-css-calc` and `reduce-function-call`. The ledger was regenerated from that evidence, as 1.1 directs.
|
||||
- **Divergence — a Zig source change was unavoidable.** The anti-requirement forbids Zig changes, but record obligation 2 is enforced by `src/licenses_drift_test.zig`, and `zig build test` (1.6) cannot pass without it: the nine shipped d3 modules are ISC and `robust-predicates` is Unlicense, so `npm_licence_exceptions` needs their entries, and the 23 newly-in-closure packages that ship nothing need `npm_not_shipped` entries. Only those two tables changed; no production Zig was touched.
|
||||
- **ISC is new to this project.** `licenses/d3-isc.txt` reproduces all nine copyright notices above the single permission text they share, and the inventory entry records the acceptance as Mokhtar Mial, 2026-08-24, on this milestone's ruling. That acceptance is inferred from this spec ordering the ISC text to be carried; confirm it.
|
||||
- **Known gap in the drift guard.** `parseRecordedPackage` reads exactly three whitespace-separated tokens, so the lockfile's `@visx/vendor 4.0.0 MIT and ISC` is checked as `MIT` and its ISC half passes unreviewed. `@visx/vendor` ships nothing today, so nothing turns on it; it is written down rather than fixed because fixing it is a guard change, not this milestone.
|
||||
- Fourteen `@types/*` packages and `csstype` are in the npm *runtime* closure now, not because anything changed about them but because the `@visx` packages declare `@types/react` and `@types/d3-*` as ordinary `dependencies`. They emit no runtime code and are recorded as not shipped.
|
||||
- Axis geometry: `AxisBottom` also takes `tickLength={0}` (1.2 named it) and `AxisLeft` takes it too, which 1.2 did not — without it visx offsets the value labels by its default 8px tick length and the spec's "6px left of the plot edge" is unreachable. `AxisLeft` further takes `dy: 0` to cancel visx's own `0.25em` nudge, which would double up with the `dominantBaseline: "middle"` the current chart centres its labels with.
|
||||
- Both axes render tick labels through a `tickComponent` rather than visx's `<Text>`. `Ticks` derives a label's `y` from a *guessed* font size (`Math.max(10, …)`) because the real size comes from a StyleX class it cannot read, and `<Text>` wraps every label in a nested `<svg>`. The custom component drops the guessed `y` and places the label with the axis group transform plus `dy`, which is what makes `dy="16px"` mean baseline + 16px exactly.
|
||||
- `Pie` skips its own `top`/`left` group when given a render prop, so `Donut` centres the ring in a `<Group>` of its own. The old `fillRule="evenodd"` is gone: d3's arc paths are correctly wound and no longer need it.
|
||||
- Band gap: `paddingInner = min(0.5, 2 * n / plotWidth)` yields a gap of `step - bandwidth` ≈ 2.006px rather than exactly 2px, because d3 derives `step` from `n - paddingInner`. Bars also start 1px left of where the hand-rolled layout put them (it centred a `slotWidth - 2` bar inside the slot; a band scale left-aligns). Both are sub-pixel-scale and the charts read the same.
|
||||
- Tooltip: `TooltipWithBounds` drops its own positioning transform when `unstyled` is set, so the default look is replaced by passing an empty `style` object and the app's StyleX class instead.
|
||||
- Tests: **507** pass across 49 files (was 458 across 46). New suites `chartKit.test.tsx` (10), `ClientChart.test.tsx` (15), `Donut.test.tsx` (9); `TimeseriesChart.test.tsx` grew from 2 to 16; `OverviewPage.test.tsx` gained the donut-slice-colour test. The counts above 493 came from the two review passes.
|
||||
- Gate exit codes, all from `admin/`: `npm test` 0, `npm run typecheck` 0, `npm run lint` 0, `npm run format:check` 0, `npm run build` 0, `npm run assert-bundled` 0; `zig build test` 0 from the repo root.
|
||||
|
||||
### Session 1 review-pass notes (post-build)
|
||||
|
||||
Ten findings from the implementation review, all fixed. Each fix was mutation-checked: the guarding test was re-run against a deliberately broken version to confirm it fails. One finding did not survive that check and is reported as such.
|
||||
|
||||
- **Tooltip lifecycle (behaviour change).** `useTooltip` is gone from both charts. It stored the hovered bucket's *numbers and screen coordinates*, which decoupled them from the data: a 30-second poll left last minute's counts under the pointer, and a resize left the tooltip at coordinates that no longer described anything. Both charts now keep only the hovered bucket **index**, in a new `useActiveBucket(windowKey)` hook, and read the values and the x out of the render they are currently drawing. `windowKey` is `since:bucket_seconds:bucketCount:width`; a selection made against an old key is deleted, so a rolling window or a resize retires the tooltip instead of relabelling it. A same-window refresh updates the open tooltip in place, which is the better of the two behaviours the review allowed.
|
||||
- **Tooltip is keyed by bucket.** `withBoundingRects` measures its node once, in `componentDidMount`, and never again, so one shared mount placed every bucket with the first bucket's measured size — the flip-and-clip case at the right-hand edge. `key={bucket}` gives each bucket its own mount and its own measurement. jsdom reports every rect as zero, so the test asserts the remount (the node identity changes between buckets), not the measurement.
|
||||
- **Tooltip offsets (behaviour change).** visx's 10px `offsetLeft`/`offsetTop` defaults are replaced by an explicit 8px, restoring the hand-rolled tooltip's placement: 8px down from the chart's top edge, 8px to the side of the slot. `top` is now 0 with the offset supplying the 8, rather than `plot.y` coincidentally being 8.
|
||||
- **Overlay clamp (behaviour change).** A band scale spends a trailing gap after the last column, so a full-step hit target on the last bucket reached ~2px into the right margin. The last slot is now clipped to `plot.x + plot.width`. The tooltip's x is the **slot** centre (`band start + step/2`), not the narrower band centre it was using.
|
||||
- The x-axis tick labels no longer carry `tabularNums`; the hand-rolled x labels never had it and adding it was an unsanctioned change. The y-axis labels keep it, as they always had it.
|
||||
- Tests added or tightened: the clamp test now pins the y-domain source (asserting the top tick is `10`, so a switch to the stacked sum's 13 fails); `bandPaddingInner` is pinned at `(24, 1388)` and at its 0.5 cap; the value-axis test asserts the rendered `dy="0"` and `dominant-baseline="middle"`; the donut ring test parses the `A` command radii and asserts `[90, 90, 54, 54]`; both charts gained overlay-clamp, tooltip-offset, per-bucket-remount, same-window-refresh and rolled-window tests.
|
||||
- **One finding could not be closed as stated.** The donut caller-order fixture is now `[1, 3]` (ascending) as directed, and it does pin that the ring is drawn in caller order. It does **not** pin `pieSort={null}`/`pieSortValues={null}`: removing both props leaves the rendered output byte-identical, because `@visx/shape` 4.0.0's own `pie()` factory already calls `sortValues(null)` when neither prop is given. The props are kept anyway and the comment now says why — visx carries that default precisely because d3-shape v3 flipped `sortValues` to descending underneath it, so the props are what stop a future bump from silently re-ranking the ring. No test can distinguish them at this version.
|
||||
|
||||
### Session 1 residual-pass notes (post-build)
|
||||
|
||||
Three residuals from the re-review, all fixed and mutation-checked.
|
||||
|
||||
- **A retired selection is deleted, not masked (behaviour change).** `useActiveBucket` only compared the stored key against the current one, so the selection survived in state and a *returning* key revived it: resize away from 640 and back, or a poll that restores a bucket count, redrew a tooltip and its dimming with no pointer entry. The hook now drops the selection during the render that changes the key — the React "adjust state on prop change" pattern, not an effect, so no tooltip is committed and then removed. The rolled-window test rolls away and back and asserts nothing returns.
|
||||
- **The tooltip remeasures on content change as well as on bucket change.** `key={bucket}` covered moving between buckets but not the same bucket changing width under a same-window refresh: a count crossing a digit boundary, or a client name resolving. The key is now `bucket|total|label=value…`, so any content that could change the measured width forces a fresh mount. The test asserts DOM identity changes when a hovered bucket's numbers change.
|
||||
- The implementation notes above were stale on bytes, test count and the `chartKit` export list; all three are corrected to the figures verified in this pass.
|
||||
- Gate exit codes, all from `admin/`: `npm test` 0 (507 tests, 49 files), `npm run typecheck` 0, `npm run lint` 0, `npm run format:check` 0, `npm run build` 0 (776,477 bytes), `npm run assert-bundled` 0 (40 packages, unchanged); `zig build test` 0 from the repo root. No commits.
|
||||
|
||||
### Session 1 owner-review addendum (post-build)
|
||||
|
||||
Three changes from the owner's visual review of the live charts. All three are
|
||||
behaviour changes, not corrections, and each was mutation-checked.
|
||||
|
||||
- **The client chart drops "Other" when it counted nothing.** `ClientChart` leaves
|
||||
the aggregate out of the legend, the stack, the tooltip and the hidden table
|
||||
when `data.other` is all zeroes; the named clients stay at zero. The hidden
|
||||
table drops the column with the rest rather than keeping a zero column, so all
|
||||
four surfaces agree; a test pins that choice. This reverses `ClientChart`'s
|
||||
previous documented rule that "Other" is always present, and the
|
||||
`OverviewPage` test that pinned it is inverted accordingly.
|
||||
**`TimeseriesChart` does not do this** — see the ruling below.
|
||||
- **`TimeseriesChart`'s third series is labelled "Allowed".** The series key stays
|
||||
`other` and the colour mapping is untouched — only the displayed label changes,
|
||||
in the legend, the tooltip row, the hidden table header and the SVG
|
||||
`aria-label`, which is now built from the shown series rather than hard-coded.
|
||||
- **The donuts gain the bar charts' hover treatment.** Pointing at a slice path
|
||||
opens the shared tooltip (label, count in the panel's unit, and the same share
|
||||
the legend prints) and dims the other slices to 0.55; leaving the ring clears
|
||||
both. The ring stays `aria-hidden` and the legend plus hidden table remain the
|
||||
accessible surface. The slice path is the hit target; no overlay was added.
|
||||
|
||||
Supporting refactors:
|
||||
|
||||
- `useActiveBucket` is renamed `useActiveIndex`: it now tracks a slice as well as
|
||||
a bucket. Its `windowKey` for the donut is the drawn slices' keys, so a changed
|
||||
slice set retires the hover.
|
||||
- `BucketTooltipData` is replaced by `TooltipContent { title, rows }`, with
|
||||
`TooltipRow.value` a string and `TooltipRow.color` optional. The bar charts now
|
||||
pass their own formatted title and an explicit "Queries" total row, where
|
||||
`ChartTooltip` previously hard-coded both — which is what let the donut, whose
|
||||
tooltip has no timestamp and no total, reuse it unchanged. `ChartTooltip` also
|
||||
takes an optional `top`, which the donut uses and the bar charts leave at 0.
|
||||
- `Donut.sliceAnchor()` computes a slice's mid-arc point from the slice values.
|
||||
It restates the `Pie` configuration (clockwise from twelve o'clock, caller
|
||||
order, no pad angle) rather than reading the drawn path, so a test pins the
|
||||
resulting transform against the ring's real geometry.
|
||||
|
||||
- Tests: **515** pass across 49 files (was 507). New: two timeseries tests (the
|
||||
rename, and the series staying at zero), two client-chart tests for the hiding,
|
||||
four donut hover tests. Bundle: `admin/dist/assets` is **776,995 bytes** across
|
||||
40 files, 23,005 under the 800,000-byte gate.
|
||||
- Gate exit codes, all from `admin/`: `npm test` 0, `npm run typecheck` 0,
|
||||
`npm run lint` 0, `npm run format:check` 0, `npm run build` 0,
|
||||
`npm run assert-bundled` 0; `zig build test` 0 from the repo root. No commits.
|
||||
|
||||
**Ruling (2026-08-24).** The review's item 1 originally applied the hiding to both
|
||||
bar charts. It was implemented that way, then reverted for the timeseries after
|
||||
the build agent raised the incoherence and the lead ruled with it: item 2's own
|
||||
reasoning is that the third series is a real category — queries answered
|
||||
upstream, locally, or from a forward zone — which is why it stops being called
|
||||
"Other"; hiding it at zero then contradicts that, and item 1's other half ("a
|
||||
zero Blocked is information") applies to it just as much. A zero Allowed says
|
||||
every query in the window was blocked or served from cache, which is a state
|
||||
worth reading, not an empty bucket.
|
||||
|
||||
The rule that survives: **hide an aggregate that aggregated nothing; never hide a
|
||||
named category.** `ClientChart`'s "Other" is the only aggregate on the Overview,
|
||||
so it is the only series that disappears. `TimeseriesChart` keeps Blocked, Cached
|
||||
and Allowed at all times, pinned by a test.
|
||||
|
||||
The other two open items are closed: `pieSort`/`pieSortValues` stay as
|
||||
version-proofing with their comment (lead's ruling), and the ISC acceptance line
|
||||
in `licenses/inventory.zon` rests with the owner.
|
||||
Reference in New Issue
Block a user