Merge pull request 'fix(charts): make the national-average marker visible on both templates' (#99) from fix/chart-marker-contrast into main

Reviewed-on: #99

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<string> {
+  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);

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 {

nextjs-app/components/SatsChart.tsx

@@ -143,17 +143,40 @@ export default function SatsChart({ subjects }: SatsChartProps) {
           <SubjectColumn key={subject.name} subject={subject} />
         ))}
       </div>
+      {/*
+        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.
+      */}
       <div className={styles.legend}>
         <div className={styles.legendItem}>
-          <div className={styles.legendSwatch} style={{ background: 'var(--status-above)' }} />
+          <div className={styles.legendSwatch} style={{ background: 'var(--brand)' }} />
           Expected standard
         </div>
         <div className={styles.legendItem}>
-          <div className={styles.legendSwatch} style={{ background: 'var(--status-above)' }} />
+          <div className={styles.legendSwatch} style={{ background: 'var(--brand-stronger)' }} />
           Exceeding / high score
         </div>
         <div className={styles.legendItem}>
-          <div className={styles.legendSwatch} style={{ background: 'var(--brand)', width: '3px', height: '12px', borderRadius: '2px' }} />
+          <div
+            className={styles.legendSwatch}
+            style={{
+              background: 'var(--bg-card)',
+              boxShadow: '0 0 0 1px var(--text-primary)',
+              width: '3px',
+              height: '12px',
+              borderRadius: '2px',
+            }}
+          />
           National average
         </div>
       </div>

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;
 }