fix(charts): make the national-average marker visible on both templates #99

Merged
tudor merged 1 commits from fix/chart-marker-contrast into main 2026-08-15 08:56:41 +00:00
Owner

Audit of the school detail page, and the fix. You were right about the national-average marker — it is worse than low contrast.

The marker was invisible, not faint

It was var(--brand) — the identical value to the bar fill it sits on. Pixel-sampled across every bar on both templates, both themes:

marker bar ratio
primary SatsChart .natTick (6 bars) #0F766E #0F766E 1.00:1
secondary .att8VizNatLine #0F766E #0F766E 1.00:1
dark theme, both #5FC7BB #5FC7BB 1.00:1

The failure mode is perverse. Against the empty track it scored 5.47:1 — so the marker was visible only when a school was below the national average, and vanished for every school at or above it. It failed for exactly the schools people are searching for.

No single colour fixes this: the marker's position is data-driven, so it can land on the bar, on the empty track, or across the boundary. It is now a knockout — a light core carrying a dark edge, both from tokens that flip with the theme, so one part or the other always separates:

on the bar on the track
light 5.47 / 9.70:1 15.17:1
dark 7.81 / 10.85:1 13.52:1

The legend described a chart that didn't exist

swatch was the bar it labelled
"Expected standard" #36743F green teal
"Exceeding / high score" #36743F green — identical to above teal
"National average" #0F766E teal the same teal as the bars

Two different series shared one swatch, neither matched its bar, and the only swatch that did match the bars was labelled "National average" — so reading the chart by matching colours told you the teal bars were the benchmark. Each swatch now carries its bar's exact value, and the marker swatch mirrors the knockout.

Two series drawn in one colour

Expected and Exceeding were both var(--brand), distinguished only by which row you were looking at. They are a sequential pair — exceeding is the same cohort at a harder bar — so they now take two steps of one hue, the harder measure sitting further from the ground in each theme.

They measure 1.77:1 (light) / 1.39:1 (dark) against each other, and I'm accepting that rather than papering over it: two fills that must each clear 3:1 against the same white track are geometrically forced close together. The distinction is carried by the row labels (EXPECTED / EXCEEDING) and the printed values — colour is redundant there, not load-bearing.

Why every existing gate passed

The WCAG journey composites backgrounds by walking the ancestor chain. These markers are absolutely positioned over a sibling, so ancestor-walking reports the marker against the card, never against the thing it overlaps. That blind spot is why this shipped in two templates and survived every run.

The new journey asks the stacking order instead, via document.elementsFromPoint.

Writing it surfaced a second trap worth recording. elementsFromPoint takes viewport coordinates and returns an empty stack for anything off-screen — and these markers sit ~1200px down the page. My first version defaulted an empty stack to white, which made teal-on-teal look like teal-on-white and pass in the light theme. It now scrolls each marker into view, and counts any marker whose backdrop it cannot resolve as a failure rather than a pass.

Verified in both directions

  • Fails against current staging: 1.00:1 over rgb(15,118,110) in light, rgb(95,199,187) in dark
  • Passes against this build: 6/6 markers measured, 0 failures, 5.47–15.17:1

tsc clean, 159/159 unit tests, build green, 44 e2e journeys parse.

Audited and found correct

  • Results Over Time already uses --chart-reference (dashed grey) — properly distinct. The two charts on the same page disagreed; only the bar charts were wrong.
  • Bars vs their track: 4.91:1 ✅
  • All text passes AA in both themes.

Not fixed, flagged

  • Gender split: 1.46:1 between the purple and teal segments. Left alone — the percentages are printed beside it, so the graphic isn't the sole carrier, and changing it touches the phase/demographic tokens.
  • The primary trend chart looked sparse for 2022/23–2024/25 despite this school having 100% in 2024/25. My API-shape guess was wrong so I could not confirm it, and I am not claiming it as a bug — worth a separate look.

🤖 Generated with Claude Code

Audit of the school detail page, and the fix. You were right about the national-average marker — it is worse than low contrast. ## The marker was invisible, not faint It was `var(--brand)` — the **identical value** to the bar fill it sits on. Pixel-sampled across every bar on both templates, both themes: | | marker | bar | ratio | |---|---|---|---| | primary `SatsChart .natTick` (6 bars) | `#0F766E` | `#0F766E` | **1.00:1** | | secondary `.att8VizNatLine` | `#0F766E` | `#0F766E` | **1.00:1** | | dark theme, both | `#5FC7BB` | `#5FC7BB` | **1.00:1** | **The failure mode is perverse.** Against the *empty* track it scored 5.47:1 — so the marker was visible only when a school was **below** the national average, and vanished for every school at or above it. It failed for exactly the schools people are searching for. No single colour fixes this: the marker's position is data-driven, so it can land on the bar, on the empty track, or across the boundary. It is now a **knockout** — a light core carrying a dark edge, both from tokens that flip with the theme, so one part or the other always separates: | | on the bar | on the track | |---|---|---| | light | 5.47 / 9.70:1 | 15.17:1 | | dark | 7.81 / 10.85:1 | 13.52:1 | ## The legend described a chart that didn't exist | swatch | was | the bar it labelled | |---|---|---| | "Expected standard" | `#36743F` green | teal | | "Exceeding / high score" | `#36743F` green — **identical to above** | teal | | "National average" | `#0F766E` teal | **the same teal as the bars** | Two different series shared one swatch, neither matched its bar, and the only swatch that *did* match the bars was labelled "National average" — so reading the chart by matching colours told you the teal bars were the benchmark. Each swatch now carries its bar's exact value, and the marker swatch mirrors the knockout. ## Two series drawn in one colour Expected and Exceeding were both `var(--brand)`, distinguished only by which row you were looking at. They are a *sequential* pair — exceeding is the same cohort at a harder bar — so they now take two steps of one hue, the harder measure sitting further from the ground in each theme. They measure **1.77:1 (light) / 1.39:1 (dark)** against each other, and I'm accepting that rather than papering over it: two fills that must *each* clear 3:1 against the same white track are geometrically forced close together. The distinction is carried by the row labels (`EXPECTED` / `EXCEEDING`) and the printed values — colour is redundant there, not load-bearing. ## Why every existing gate passed The WCAG journey composites backgrounds by walking the **ancestor** chain. These markers are absolutely positioned over a **sibling**, so ancestor-walking reports the marker against the card, never against the thing it overlaps. That blind spot is why this shipped in two templates and survived every run. The new journey asks the **stacking order** instead, via `document.elementsFromPoint`. > Writing it surfaced a second trap worth recording. `elementsFromPoint` takes *viewport* coordinates and returns an empty stack for anything off-screen — and these markers sit ~1200px down the page. My first version defaulted an empty stack to white, which made teal-on-teal look like teal-on-white and **pass in the light theme**. It now scrolls each marker into view, and counts any marker whose backdrop it cannot resolve as a failure rather than a pass. ## Verified in both directions - **Fails** against current staging: `1.00:1 over rgb(15,118,110)` in light, `rgb(95,199,187)` in dark - **Passes** against this build: 6/6 markers measured, 0 failures, 5.47–15.17:1 `tsc` clean, 159/159 unit tests, build green, 44 e2e journeys parse. ## Audited and found correct - **Results Over Time** already uses `--chart-reference` (dashed grey) — properly distinct. The two charts on the same page disagreed; only the bar charts were wrong. - Bars vs their track: 4.91:1 ✅ - All **text** passes AA in both themes. ## Not fixed, flagged - **Gender split: 1.46:1** between the purple and teal segments. Left alone — the percentages are printed beside it, so the graphic isn't the sole carrier, and changing it touches the phase/demographic tokens. - The **primary trend chart looked sparse** for 2022/23–2024/25 despite this school having 100% in 2024/25. My API-shape guess was wrong so I could not confirm it, and I am **not** claiming it as a bug — worth a separate look. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
tudor added 1 commit 2026-08-15 08:49:08 +00:00
fix(charts): make the national-average marker visible on both templates
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m1s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m5s
59ac9c10b9
The marker was var(--brand) — the identical value to the bar fill it sits on.
Measured by pixel-sampling every bar on both templates, in both themes:

  primary   SatsChart .natTick        1.00:1  (6 bars)
  secondary .att8VizNatLine           1.00:1

Not low contrast. The same colour. It scored 5.47:1 only against the empty
track, which means it was visible precisely when a school was BELOW the
national average and vanished for every school at or above it — failing for
exactly the schools people are looking for.

No single colour fixes this, because the marker's position is data-driven: it
can land on the bar, on the empty track, or across the boundary. It is now a
knockout — a light core carrying a dark edge, both from tokens that flip with
the theme, so one part or the other always separates:

              on the bar        on the track
  light       5.47 / 9.70:1     15.17:1
  dark        7.81 / 10.85:1    13.52:1

THE LEGEND DESCRIBED A CHART THAT DID NOT EXIST

Both data swatches were var(--status-above) green while their bars were
var(--brand) teal, and the two were identical to each other — one swatch for
two series. Worse, the only swatch matching the bar colour was the one
labelled "National average", so reading the chart by matching colours told you
the teal bars were the benchmark. Each swatch now carries its bar's exact
value, and the marker swatch mirrors the knockout.

TWO SERIES, ONE COLOUR

Expected and Exceeding were both var(--brand), distinguished only by row.
They are a sequential pair — exceeding is the same cohort at a harder bar — so
they take two steps of one hue, the harder measure being the step further from
the ground in each theme.

They measure 1.77:1 (light) and 1.39:1 (dark) against each other, and that is
accepted rather than overlooked: two fills that must EACH clear 3:1 against
the same white track are geometrically forced close together. The distinction
is carried by the row labels and printed values; colour is redundant here, not
load-bearing.

THE TEST THAT SHOULD HAVE CAUGHT THIS

The WCAG journey composites backgrounds by walking the ancestor chain, but
these markers are absolutely positioned over a sibling — ancestor-walking is
structurally blind to overlap, which is why this shipped in two templates and
passed every gate. The new journey asks the stacking order instead, via
document.elementsFromPoint.

Writing it surfaced a second trap worth recording: elementsFromPoint takes
viewport coordinates and returns an empty stack off-screen, and these markers
sit ~1200px down. The first version defaulted an empty stack to white, which
made teal-on-teal look like teal-on-white and PASS in the light theme. It now
scrolls each marker into view and counts any marker it cannot resolve a
backdrop for as a failure rather than a pass.

Verified both directions: the new journey fails against current staging with
"1.00:1 over rgb(15,118,110)" in both themes, and passes against this build
with 6/6 markers measured at 5.47–15.17:1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

🤖 AI Code Review (Claude Code)

This PR fixes a real accessibility bug where the national-average benchmark markers on both chart templates (SatsChart and the Attainment-8 viz) rendered in the same color as the bars behind them, making them invisible whenever a school met or exceeded the national average; it replaces the flat-color marker with a theme-aware knockout style, fixes duplicate/mismatched legend swatch colors, and adds a solid Playwright regression test that measures actual on-screen stacking order and contrast rather than DOM ancestry. The change is well-reasoned, uses existing design tokens (--brand-stronger, --bg-card, --text-primary) that are already defined elsewhere in the codebase, and does not touch CI/CD or backend code.

🟡 Minor

  • e2e/tests/journeys.spec.ts: The dynamic test.skip(res.count === 0 && query === 'primary', ...) only guards the 'primary' query. If the 'academy' query ever lands on a page with zero rendered national-average markers (e.g. a school lacking secondaryAvg data), res.count and res.unmeasured are both 0, so both assertions trivially pass and that iteration silently exercises nothing — no skip message is emitted and the loop looks like it verified the marker when it verified nothing.
## 🤖 AI Code Review (Claude Code) This PR fixes a real accessibility bug where the national-average benchmark markers on both chart templates (SatsChart and the Attainment-8 viz) rendered in the same color as the bars behind them, making them invisible whenever a school met or exceeded the national average; it replaces the flat-color marker with a theme-aware knockout style, fixes duplicate/mismatched legend swatch colors, and adds a solid Playwright regression test that measures actual on-screen stacking order and contrast rather than DOM ancestry. The change is well-reasoned, uses existing design tokens (--brand-stronger, --bg-card, --text-primary) that are already defined elsewhere in the codebase, and does not touch CI/CD or backend code. ### 🟡 Minor - **e2e/tests/journeys.spec.ts**: The dynamic test.skip(res.count === 0 && query === 'primary', ...) only guards the 'primary' query. If the 'academy' query ever lands on a page with zero rendered national-average markers (e.g. a school lacking secondaryAvg data), res.count and res.unmeasured are both 0, so both assertions trivially pass and that iteration silently exercises nothing — no skip message is emitted and the loop looks like it verified the marker when it verified nothing.
tudor merged commit 38acc76555 into main 2026-08-15 08:56:41 +00:00
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: tudor/school_compare#99