From 59ac9c10b97e0ef1143f57fea06b324e72ac3d4a Mon Sep 17 00:00:00 2001 From: Tudor Date: Sat, 15 Aug 2026 09:48:41 +0100 Subject: [PATCH] fix(charts): make the national-average marker visible on both templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- e2e/tests/journeys.spec.ts | 102 ++++++++++++++++++ nextjs-app/components/SatsChart.module.css | 46 ++++++-- nextjs-app/components/SatsChart.tsx | 29 ++++- .../school/schoolSections.module.css | 15 ++- 4 files changed, 180 insertions(+), 12 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index d0be24a..a1d3392 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -800,6 +800,108 @@ test('sticky section nav jumps to server-rendered sections', async ({ page }) => * These are silent failures — nothing on the page looks wrong — so they need * a gate. */ +/** + * Chart benchmark markers, against whatever they actually overlap. + * + * The national-average marker shipped as var(--brand) on both templates — + * identical to the bar fill it sits on, so it measured 1.00:1 wherever it + * crossed a bar and was visible only for schools BELOW the benchmark. + * + * The WCAG journey below never saw it, and could not have: it composites + * backgrounds by walking the ANCESTOR chain, while these markers are + * absolutely positioned over a SIBLING. Ancestor-walking is structurally blind + * to overlap. This test asks the stacking order instead — + * document.elementsFromPoint returns what is genuinely beneath a point — which + * is the same question a reader's eye asks. + */ +const MARKER_PROBE = `(() => { + const ps = c => { const m=(c||'').match(/[\\d.]+/g); if(!m) return null; + const a=m.map(Number); return {r:a[0],g:a[1],b:a[2],a:m.length>3?a[3]:1}; }; + const ov = (f,b) => ({r:f.r*f.a+b.r*(1-f.a), g:f.g*f.a+b.g*(1-f.a), b:f.b*f.a+b.b*(1-f.a), a:1}); + const L = c => { const f=v=>{v/=255; return v<=0.03928?v/12.92:Math.pow((v+.055)/1.055,2.4);}; + return .2126*f(c.r)+.7152*f(c.g)+.0722*f(c.b); }; + const RT = (a,b) => { const x=L(a),y=L(b); return (Math.max(x,y)+.05)/(Math.min(x,y)+.05); }; + + const markers = [...document.querySelectorAll('[class*="natTick"], [class*="NatLine"]')] + .filter(el => el.getBoundingClientRect().width > 0); + const out = { count: markers.length, failures: [], unmeasured: 0 }; + + for (const el of markers) { + /* + * Scroll it in first. elementsFromPoint takes VIEWPORT coordinates and + * returns an empty stack for anything off-screen — and these markers sit + * well down the page. Without this the backdrop silently defaults to + * white, which makes a teal-on-teal marker look like teal-on-white and + * pass. 'instant' because globals.css sets scroll-behavior: smooth on + * html, and a smooth scroll would still be moving when we measure. + */ + el.scrollIntoView({ block: 'center', behavior: 'instant' }); + const r = el.getBoundingClientRect(); + const cx = r.left + r.width / 2, cy = r.top + r.height / 2; + const own = ps(getComputedStyle(el).backgroundColor); + const edge = ps((getComputedStyle(el).boxShadow.match(/rgba?\\([^)]*\\)/) || [])[0] || ''); + if (!own) continue; + + // What is genuinely underneath, by stacking order rather than by ancestry. + const stack = document.elementsFromPoint(cx, cy); + if (stack.length === 0) { out.unmeasured++; continue; } // never assume white + let backdrop = null; + for (const under of stack) { + if (under === el || el.contains(under)) continue; + const c = ps(getComputedStyle(under).backgroundColor); + if (c && c.a > 0.9) { backdrop = c; break; } + } + if (!backdrop) { out.unmeasured++; continue; } + + // A knockout marker only needs ONE of its two parts to separate. + const best = Math.max(RT(ov(own, backdrop), backdrop), + edge ? RT(ov(edge, backdrop), backdrop) : 0); + if (best < 3) { + out.failures.push((el.className || '?').toString().split(' ')[0] + + ' ' + best.toFixed(2) + ':1 over rgb(' + + [backdrop.r, backdrop.g, backdrop.b].map(Math.round).join(',') + ')'); + } + } + return out; +})()`; + +async function firstSchoolLink(page: Page, query: string): Promise { + await page.goto(`/?search=${encodeURIComponent(query)}`); + const link = schoolLinks(page).first(); + await expect(link).toBeVisible({ timeout: 15_000 }); + return (await link.getAttribute('href'))!; +} + +for (const scheme of ['light', 'dark'] as const) { + test(`chart benchmark markers stay visible in the ${scheme} theme`, async ({ browser }) => { + const context = await browser.newContext({ colorScheme: scheme }); + const page = await context.newPage(); + const failures: string[] = []; + + // One primary (SATs bars) and one secondary (Attainment 8 bar) — the two + // templates carry separate implementations of the same marker. + for (const query of ['primary', 'academy']) { + const href = await firstSchoolLink(page, query); + await page.goto(href); + await expect(page.locator('h1').first()).toBeVisible({ timeout: 15_000 }); + await page.waitForTimeout(900); // bar widths animate in from an effect + + const res = (await page.evaluate(MARKER_PROBE)) as + { count: number; failures: string[]; unmeasured: number }; + test.skip(res.count === 0 && query === 'primary', + 'no benchmark markers rendered — environment has no national averages'); + // A marker we could not resolve a backdrop for is a hole in the check, + // not a pass. Fail loudly rather than quietly measuring nothing. + expect(res.unmeasured, + `${href}: ${res.unmeasured} of ${res.count} markers had no resolvable backdrop`).toBe(0); + failures.push(...res.failures.map(f => `${href} → ${f}`)); + } + + await context.close(); + expect(failures, `benchmark markers under 3:1 in ${scheme}:\n ${failures.join('\n ')}`).toEqual([]); + }); +} + test('the brand asset set is complete and served', async ({ page }) => { const response = await page.goto('/'); expect(response?.ok()).toBe(true); diff --git a/nextjs-app/components/SatsChart.module.css b/nextjs-app/components/SatsChart.module.css index a489727..77fc88e 100644 --- a/nextjs-app/components/SatsChart.module.css +++ b/nextjs-app/components/SatsChart.module.css @@ -48,13 +48,32 @@ Each bar compares against its own benchmark (expected vs higher standard / greater depth), so the marker sits on the individual bar's track rather than as one line spanning both bars. */ +/* + * Knockout, not a colour. + * + * This marker was var(--brand) — the same value as .barExpected and + * .barExceeding, so wherever it crossed a bar it measured 1.00:1 and was not + * rendered distinguishably at all. 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. + * + * No single colour fixes this, because the marker's position is data-driven: + * it can land on the bar, on the empty track, or straddle the boundary. So it + * is drawn as a knockout — a light core carrying a dark edge. On the teal bar + * the core reads; on the pale track the edge reads. Both tokens flip with the + * theme, so the pairing holds in dark mode too. + * + * The edge is box-shadow rather than border so it costs no layout width and + * cannot shift the 50% translate. + */ .natTick { position: absolute; top: -3px; bottom: -3px; - width: 2px; + width: 3px; transform: translateX(-50%); - background: var(--brand); + background: var(--bg-card); + box-shadow: 0 0 0 1px var(--text-primary); border-radius: 2px; z-index: 4; pointer-events: none; @@ -63,13 +82,14 @@ .natTick::before { content: ''; position: absolute; - top: -3px; + top: -4px; left: 50%; transform: translateX(-50%); - width: 5px; - height: 5px; + width: 6px; + height: 6px; border-radius: 50%; - background: var(--brand); + background: var(--bg-card); + box-shadow: 0 0 0 1px var(--text-primary); } .barHeaderRight { @@ -122,12 +142,24 @@ transition: width 0.8s cubic-bezier(0.25, 0.46, 0.45, 0.94); } +/* + * Expected and Exceeding were both var(--brand) — one colour for two series, + * distinguished only by which row you were looking at, while the legend + * claimed two. + * + * They are a sequential pair, not two categories: "exceeding" is a subset of + * the same cohort at a harder bar. So they take two steps of the same hue + * rather than two different hues, with the harder measure the more intense + * step. --brand-stronger is darker than --brand in the light theme and lighter + * in the dark one, which is the right direction in both: further from the + * ground. + */ .barExpected { background: var(--brand); } .barExceeding { - background: var(--brand); + background: var(--brand-stronger); } .barLabel { diff --git a/nextjs-app/components/SatsChart.tsx b/nextjs-app/components/SatsChart.tsx index 6143f04..9e3c2e7 100644 --- a/nextjs-app/components/SatsChart.tsx +++ b/nextjs-app/components/SatsChart.tsx @@ -143,17 +143,40 @@ export default function SatsChart({ subjects }: SatsChartProps) { ))} + {/* + The legend describes what is drawn, which it previously did not. + + Both data swatches were var(--status-above) — green — while the bars + they labelled were var(--brand) teal, and they were identical to each + other, so two different series shared one swatch. Worse, the only + swatch that matched the bar colour was the one labelled "National + average": anyone reading the chart by matching colours would conclude + the teal bars WERE the national average. + + Each swatch now carries the exact value its bar carries, and the + national-average swatch mirrors the knockout marker rather than being + a flat colour, so it is recognisable as the thing on the chart. + */}
-
+
Expected standard
-
+
Exceeding / high score
-
+
National average
diff --git a/nextjs-app/components/school/schoolSections.module.css b/nextjs-app/components/school/schoolSections.module.css index b43c087..6ea4edc 100644 --- a/nextjs-app/components/school/schoolSections.module.css +++ b/nextjs-app/components/school/schoolSections.module.css @@ -1807,12 +1807,23 @@ border-radius: 4px 0 0 4px; transition: width 0.6s ease; } +/* + * Knockout, for the same reason as .natTick in SatsChart.module.css: this was + * var(--brand), identical to .att8VizFill above it, so where the marker + * crossed the school's own bar it measured 1.00:1 — invisible for exactly the + * schools at or above the national average. + * + * A light core with a dark edge reads on the teal fill and on the pale track + * alike, and both tokens flip with the theme. Keep this in step with + * SatsChart's .natTick; they are the same marker on two templates. + */ .att8VizNatLine { position: absolute; top: -4px; bottom: -4px; - width: 2px; - background: var(--brand); + width: 3px; + background: var(--bg-card); + box-shadow: 0 0 0 1px var(--text-primary); border-radius: 2px; z-index: 2; }