Two follow-ups from the staging E2E run and a re-audit of the live site.
The failing test was wrong; the site was right
Staging now serves "Schibsted Grotesk", "Schibsted Grotesk Fallback", …, -apple-system, sans-serif — the font fix worked.
My assertion was /Times|serif$/, and serif$ matches the tail of sans-serif. A healthy font stack could never have passed it. Any stack ends in a generic family, so that pattern was guaranteed to fail on a correct page.
Now it checks the first family in the stack — the only one actually requested — and matches Times as a whole word:
expect(fonts.body).toBe('Schibsted Grotesk');expect(fonts.heading).toBe('Schibsted Grotesk');expect(fonts.prose).toBe('Literata');// never covered before
expect(fonts.bodyStack).not.toMatch(/\bTimes\b/);
The Leaflet overrides never took effect
Re-auditing the live page after #87 deployed, the map attribution was still #0078A8. leaflet.css is imported from a client component, so its chunk loads after globals.css — and at equal specificity (both a bare .leaflet-container) the later sheet wins.
Measured on the live page:
stylesheet
order
selector
background
globals
0
.leaflet-container
var(--bg-secondary)
leaflet.css
3
.leaflet-container
rgb(221, 221, 221) ← wins
Prefixing the overrides with html takes them to 0,1,1, which beats a bare class regardless of load order. Injecting that on the live page turned the background to rgb(233, 232, 226) and the attribution link to rgb(88, 74, 155).
The palette gate should have caught this. It skipped everything inside .leaflet-container to avoid flagging OSM tile imagery — which also skipped Leaflet's own chrome. Narrowed the exemption to .leaflet-tile-pane, so map controls are now covered.
Tidy
Dropped the duplicated family name from the font tokens. next/font already expands the variable to the family plus its metric-matched fallback, so the stack named Schibsted Grotesk twice. It read like a safety net but wasn't one: an unresolvable var() with no fallback invalidates the whole declaration, so the literal after it never gets a turn.
Verified against live staging
body and headings resolve to Schibsted Grotesk, prose to Literata
the search input is rgb(22, 32, 42) (--text-primary), not pure black — the #87 fix confirmed
the higher-specificity Leaflet rules win when injected into the live page
Two follow-ups from the staging E2E run and a re-audit of the live site.
## The failing test was wrong; the site was right
Staging now serves `"Schibsted Grotesk", "Schibsted Grotesk Fallback", …, -apple-system, sans-serif` — the font fix worked.
My assertion was `/Times|serif$/`, and **`serif$` matches the tail of `sans-serif`**. A healthy font stack could never have passed it. Any stack ends in a generic family, so that pattern was guaranteed to fail on a correct page.
Now it checks the **first** family in the stack — the only one actually requested — and matches `Times` as a whole word:
```ts
expect(fonts.body).toBe('Schibsted Grotesk');
expect(fonts.heading).toBe('Schibsted Grotesk');
expect(fonts.prose).toBe('Literata'); // never covered before
expect(fonts.bodyStack).not.toMatch(/\bTimes\b/);
```
## The Leaflet overrides never took effect
Re-auditing the live page after #87 deployed, the map attribution was still `#0078A8`. `leaflet.css` is imported from a client component, so its chunk loads **after** globals.css — and at equal specificity (both a bare `.leaflet-container`) the later sheet wins.
Measured on the live page:
| stylesheet | order | selector | background |
|---|---|---|---|
| globals | 0 | `.leaflet-container` | `var(--bg-secondary)` |
| leaflet.css | 3 | `.leaflet-container` | `rgb(221, 221, 221)` ← wins |
Prefixing the overrides with `html` takes them to 0,1,1, which beats a bare class regardless of load order. Injecting that on the live page turned the background to `rgb(233, 232, 226)` and the attribution link to `rgb(88, 74, 155)`.
**The palette gate should have caught this.** It skipped everything inside `.leaflet-container` to avoid flagging OSM tile imagery — which also skipped Leaflet's own chrome. Narrowed the exemption to `.leaflet-tile-pane`, so map controls are now covered.
## Tidy
Dropped the duplicated family name from the font tokens. next/font already expands the variable to the family plus its metric-matched fallback, so the stack named Schibsted Grotesk twice. It read like a safety net but wasn't one: an unresolvable `var()` with no fallback invalidates the whole declaration, so the literal after it never gets a turn.
## Verified against live staging
- body and headings resolve to `Schibsted Grotesk`, prose to `Literata`
- the search input is `rgb(22, 32, 42)` (`--text-primary`), not pure black — the #87 fix confirmed
- the higher-specificity Leaflet rules win when injected into the live page
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Two follow-ups from watching the staging E2E run and re-auditing the live site.
The typeface gate failed, but the site was right. Staging now serves
"Schibsted Grotesk", "Schibsted Grotesk Fallback", …, -apple-system, sans-serif
— exactly what it should. My assertion was /Times|serif$/, and `serif$` matches
the tail of `sans-serif`, so a healthy stack could never pass. It now checks
the FIRST family in the stack (the only one actually requested) and matches
Times as a whole word. Also asserts running prose resolves to Literata, which
the earlier version never covered.
The Leaflet overrides from the previous fix never took effect. leaflet.css is
imported from a client component, so its chunk loads after globals.css; at
equal specificity (both bare `.leaflet-container`) the later sheet wins.
Verified on the live page: my rule sat at stylesheet index 0, Leaflet's at
index 3, and the attribution link was still #0078A8. Prefixing the overrides
with `html` takes them to 0,1,1, which beats a bare class regardless of load
order — injecting that on the live page turned the background to
rgb(233,232,226) and the link to rgb(88,74,155).
The palette gate would have caught this, except it skipped everything inside
.leaflet-container to avoid flagging OSM tile imagery — which also skipped
Leaflet's own chrome. Narrowed the exemption to .leaflet-tile-pane so the map
controls are now covered.
Also dropped the duplicated family name from the font tokens: next/font
already expands the variable to the family plus its metric-matched fallback,
so the stack was naming Schibsted Grotesk twice. It looked like a safety net
but wasn't — an unresolvable var() with no fallback invalidates the whole
declaration, so the literal after it never gets a turn.
Verified against live staging: body and headings resolve to Schibsted Grotesk,
prose to Literata, the search input is --text-primary rather than pure black,
and the higher-specificity Leaflet rules win when injected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit fixes two staging-observed regressions: an overly strict/buggy font e2e assertion (the old /serif$/ regex matched the tail of a healthy 'sans-serif' fallback, and the test checked the whole comma-separated stack instead of the first requested family) and Leaflet chrome CSS overrides losing to leaflet.css due to equal specificity and later load order. Both fixes are backed by correct, verifiable reasoning: the CSS var()-invalidation semantics for the removed literal font fallbacks are accurate, the html prefix genuinely raises specificity above Leaflet's default rules, and the conditional prose-font assertion does execute on the tested homepage since EditorialSection (with its .editorialText p) renders there by default. No correctness, security, data-loss, or CI/deploy issues were found.
✅ No issues found.
## 🤖 AI Code Review (Claude Code)
This commit fixes two staging-observed regressions: an overly strict/buggy font e2e assertion (the old /serif$/ regex matched the tail of a healthy 'sans-serif' fallback, and the test checked the whole comma-separated stack instead of the first requested family) and Leaflet chrome CSS overrides losing to leaflet.css due to equal specificity and later load order. Both fixes are backed by correct, verifiable reasoning: the CSS var()-invalidation semantics for the removed literal font fallbacks are accurate, the `html` prefix genuinely raises specificity above Leaflet's default rules, and the conditional prose-font assertion does execute on the tested homepage since EditorialSection (with its `.editorialText p`) renders there by default. No correctness, security, data-loss, or CI/deploy issues were found.
✅ No issues found.
Swept home, search results, rankings, admissions, compare (primary + secondary) and school detail pages for primary, secondary, special and no-data schools, in both themes, measuring computed styles rather than reading CSS.
Contrast was the big finding. The status hues were specced against --bg-primary, but they are used as chip text on their own tint, which sits on cards and secondary surfaces — not the page ground. Measured in the wild: 3.85–4.44:1, under AA, on Ofsted badges, delta chips, report-card chips and metric values. That is most of the product's actual signal. Darkened the light hues and lifted dark teal so each clears AA on its own tint, i.e. the worst case rather than the easy one.
The footer inverted in dark mode. It was painting itself with a text token and lettering itself with a background token (background: var(--text-primary); color: var(--bg-secondary)) — correct in one theme, a light slab at the bottom of a dark page in the other, with amber headings at 1.98:1. Added --surface-sunken and its on-* companions, which stay dark in both themes.
A methodology note worth recording: an earlier measurement pass reported seven dark-mode failures that do not exist. Several components carry transition: color, and I was reading mid-transition values. The new AA gate waits for transitions to settle. Most of what a naive contrast audit flags here is its own artifact.
No off-palette colours or wrong fonts anywhere outside Leaflet, which this branch already fixes.
### Update — full staging audit added to this branch
Swept home, search results, rankings, admissions, compare (primary + secondary) and school detail pages for **primary, secondary, special and no-data** schools, in **both themes**, measuring computed styles rather than reading CSS.
**Contrast was the big finding.** The status hues were specced against `--bg-primary`, but they are used as chip text on their own tint, which sits on cards and secondary surfaces — not the page ground. Measured in the wild: **3.85–4.44:1, under AA**, on Ofsted badges, delta chips, report-card chips and metric values. That is most of the product's actual signal. Darkened the light hues and lifted dark teal so each clears AA on its own tint, i.e. the worst case rather than the easy one.
**The footer inverted in dark mode.** It was painting itself with a text token and lettering itself with a background token (`background: var(--text-primary); color: var(--bg-secondary)`) — correct in one theme, a light slab at the bottom of a dark page in the other, with amber headings at 1.98:1. Added `--surface-sunken` and its `on-*` companions, which stay dark in both themes.
**A methodology note worth recording:** an earlier measurement pass reported seven dark-mode failures that do not exist. Several components carry `transition: color`, and I was reading mid-transition values. The new AA gate waits for transitions to settle. Most of what a naive contrast audit flags here is its own artifact.
No off-palette colours or wrong fonts anywhere outside Leaflet, which this branch already fixes.
Full audit across home, search, rankings, admissions, compare, and primary /
secondary / special / no-data school pages, in both themes, measuring computed
styles rather than reading CSS.
Contrast: the status hues were specced against --bg-primary, but they are used
as chip text on their own tint, which sits on cards and secondary surfaces
rather than the page ground. Measured in the wild they were 3.85–4.44:1 —
under AA — on Ofsted badges, delta chips, report-card chips and metric values,
i.e. most of the product's actual signal. Darkened the light hues
(#0e6e66 → #0b625a, #9a5b00 → #7f4a00) and lifted the dark teal
(#3fb3a4 → #4fc0b0) so each clears AA on its own tint, which is the worst case
rather than the easy one. Chart and series ramps follow.
The footer was painting itself with a text token and lettering itself with a
background token: `background: var(--text-primary); color: var(--bg-secondary)`.
That reads correctly in one theme and inverts in the other — in dark mode it
became a light slab at the bottom of a dark page, with amber section headings
at 1.98:1. Added --surface-sunken and its on-* companions, which stay dark in
BOTH themes (deliberately not --surface-inverse, whose whole job is to flip),
and moved the footer onto them. Section headings were also using a status hue
purely as decoration; they are now a muted on-surface token.
The "Open full map" pill was a fixed white background with a themed text
colour, so in dark mode it rendered light violet on white at 1.84:1. Both
sides are token-driven now.
Tabular numerals are now the default for .main rather than per-component
opt-in, with prose opting back out — a handful of figures (miniNatPill,
compareRowVal, factVal) had been missed by the class-name-based pass.
Added a two-theme AA gate over home, rankings and admissions. It waits for
`transition: color` to settle first: an earlier measurement pass read
mid-transition values and reported seven failures that did not exist at rest.
Worth stating plainly — most of what a naive audit flags here is its own
artifact, and the check has to account for that to be worth having.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This PR recalibrates the status/chart/series colour tokens for AA contrast on tinted chips (not just page backgrounds), adds a dedicated dark-band 'sunken surface' token set for the footer instead of reusing the theme-flipping inverse surface, hardens the Leaflet CSS override specificity, and adds a new automated WCAG AA contrast e2e test plus refined font-stack assertions. Overall it's a solid, well-reasoned accessibility fix; the main issues are two overlooked spots the change itself should have caught.
🟡 Minor
nextjs-app/components/school/SchoolDetailShell.module.css: .headerHasMap .actions .btnAdd background was changed from a hardcoded translucent white (deliberately theme-independent, per the removed comment 'Sits on the map hero, which renders light in both themes') to var(--bg-card), which flips dark in dark mode even though the map hero underneath is always light OSM tile imagery. Worse, the sibling :hover rule still hardcodes background: #fff while color stays var(--brand-strong), which in dark mode is a light lavender (#b0a3ee) meant for dark surfaces — hovering the button in dark mode yields ~2.25:1 contrast on white, failing AA (needs 4.5:1/3:1), directly undermining this PR's own contrast goal. Same duplicated bug in nextjs-app/components/school/schoolSections.module.css (lines ~77-86).
e2e/tests/journeys.spec.ts: The new WCAG AA contrast test's CONTRAST_PROBE excludes the entire .leaflet-container (if (el.closest('.leaflet-container')) continue;), while the two pre-existing tests just above it were narrowed in this same diff from excluding .leaflet-container to only .leaflet-tile-pane specifically so they'd cover the newly palette-styled Leaflet chrome (html .leaflet-control-attribution, .leaflet-bar a, etc.) added in globals.css. The inconsistency means the new AA test never actually contrast-checks the Leaflet controls this PR just restyled.
## 🤖 AI Code Review (Claude Code)
This PR recalibrates the status/chart/series colour tokens for AA contrast on tinted chips (not just page backgrounds), adds a dedicated dark-band 'sunken surface' token set for the footer instead of reusing the theme-flipping inverse surface, hardens the Leaflet CSS override specificity, and adds a new automated WCAG AA contrast e2e test plus refined font-stack assertions. Overall it's a solid, well-reasoned accessibility fix; the main issues are two overlooked spots the change itself should have caught.
### 🟡 Minor
- **nextjs-app/components/school/SchoolDetailShell.module.css**: `.headerHasMap .actions .btnAdd` background was changed from a hardcoded translucent white (deliberately theme-independent, per the removed comment 'Sits on the map hero, which renders light in both themes') to `var(--bg-card)`, which flips dark in dark mode even though the map hero underneath is always light OSM tile imagery. Worse, the sibling `:hover` rule still hardcodes `background: #fff` while `color` stays `var(--brand-strong)`, which in dark mode is a light lavender (#b0a3ee) meant for dark surfaces — hovering the button in dark mode yields ~2.25:1 contrast on white, failing AA (needs 4.5:1/3:1), directly undermining this PR's own contrast goal. Same duplicated bug in nextjs-app/components/school/schoolSections.module.css (lines ~77-86).
- **e2e/tests/journeys.spec.ts**: The new WCAG AA contrast test's CONTRAST_PROBE excludes the entire `.leaflet-container` (`if (el.closest('.leaflet-container')) continue;`), while the two pre-existing tests just above it were narrowed in this same diff from excluding `.leaflet-container` to only `.leaflet-tile-pane` specifically so they'd cover the newly palette-styled Leaflet chrome (`html .leaflet-control-attribution`, `.leaflet-bar a`, etc.) added in globals.css. The inconsistency means the new AA test never actually contrast-checks the Leaflet controls this PR just restyled.
tudor
merged commit f31f6f8404 into main2026-08-06 14:47:06 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Two follow-ups from the staging E2E run and a re-audit of the live site.
The failing test was wrong; the site was right
Staging now serves
"Schibsted Grotesk", "Schibsted Grotesk Fallback", …, -apple-system, sans-serif— the font fix worked.My assertion was
/Times|serif$/, andserif$matches the tail ofsans-serif. A healthy font stack could never have passed it. Any stack ends in a generic family, so that pattern was guaranteed to fail on a correct page.Now it checks the first family in the stack — the only one actually requested — and matches
Timesas a whole word:The Leaflet overrides never took effect
Re-auditing the live page after #87 deployed, the map attribution was still
#0078A8.leaflet.cssis imported from a client component, so its chunk loads after globals.css — and at equal specificity (both a bare.leaflet-container) the later sheet wins.Measured on the live page:
.leaflet-containervar(--bg-secondary).leaflet-containerrgb(221, 221, 221)← winsPrefixing the overrides with
htmltakes them to 0,1,1, which beats a bare class regardless of load order. Injecting that on the live page turned the background torgb(233, 232, 226)and the attribution link torgb(88, 74, 155).The palette gate should have caught this. It skipped everything inside
.leaflet-containerto avoid flagging OSM tile imagery — which also skipped Leaflet's own chrome. Narrowed the exemption to.leaflet-tile-pane, so map controls are now covered.Tidy
Dropped the duplicated family name from the font tokens. next/font already expands the variable to the family plus its metric-matched fallback, so the stack named Schibsted Grotesk twice. It read like a safety net but wasn't one: an unresolvable
var()with no fallback invalidates the whole declaration, so the literal after it never gets a turn.Verified against live staging
Schibsted Grotesk, prose toLiteratargb(22, 32, 42)(--text-primary), not pure black — the #87 fix confirmed🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This commit fixes two staging-observed regressions: an overly strict/buggy font e2e assertion (the old /serif$/ regex matched the tail of a healthy 'sans-serif' fallback, and the test checked the whole comma-separated stack instead of the first requested family) and Leaflet chrome CSS overrides losing to leaflet.css due to equal specificity and later load order. Both fixes are backed by correct, verifiable reasoning: the CSS var()-invalidation semantics for the removed literal font fallbacks are accurate, the
htmlprefix genuinely raises specificity above Leaflet's default rules, and the conditional prose-font assertion does execute on the tested homepage since EditorialSection (with its.editorialText p) renders there by default. No correctness, security, data-loss, or CI/deploy issues were found.✅ No issues found.
Update — full staging audit added to this branch
Swept home, search results, rankings, admissions, compare (primary + secondary) and school detail pages for primary, secondary, special and no-data schools, in both themes, measuring computed styles rather than reading CSS.
Contrast was the big finding. The status hues were specced against
--bg-primary, but they are used as chip text on their own tint, which sits on cards and secondary surfaces — not the page ground. Measured in the wild: 3.85–4.44:1, under AA, on Ofsted badges, delta chips, report-card chips and metric values. That is most of the product's actual signal. Darkened the light hues and lifted dark teal so each clears AA on its own tint, i.e. the worst case rather than the easy one.The footer inverted in dark mode. It was painting itself with a text token and lettering itself with a background token (
background: var(--text-primary); color: var(--bg-secondary)) — correct in one theme, a light slab at the bottom of a dark page in the other, with amber headings at 1.98:1. Added--surface-sunkenand itson-*companions, which stay dark in both themes.A methodology note worth recording: an earlier measurement pass reported seven dark-mode failures that do not exist. Several components carry
transition: color, and I was reading mid-transition values. The new AA gate waits for transitions to settle. Most of what a naive contrast audit flags here is its own artifact.No off-palette colours or wrong fonts anywhere outside Leaflet, which this branch already fixes.
🤖 AI Code Review (Claude Code)
This PR recalibrates the status/chart/series colour tokens for AA contrast on tinted chips (not just page backgrounds), adds a dedicated dark-band 'sunken surface' token set for the footer instead of reusing the theme-flipping inverse surface, hardens the Leaflet CSS override specificity, and adds a new automated WCAG AA contrast e2e test plus refined font-stack assertions. Overall it's a solid, well-reasoned accessibility fix; the main issues are two overlooked spots the change itself should have caught.
🟡 Minor
.headerHasMap .actions .btnAddbackground was changed from a hardcoded translucent white (deliberately theme-independent, per the removed comment 'Sits on the map hero, which renders light in both themes') tovar(--bg-card), which flips dark in dark mode even though the map hero underneath is always light OSM tile imagery. Worse, the sibling:hoverrule still hardcodesbackground: #fffwhilecolorstaysvar(--brand-strong), which in dark mode is a light lavender (#b0a3ee) meant for dark surfaces — hovering the button in dark mode yields ~2.25:1 contrast on white, failing AA (needs 4.5:1/3:1), directly undermining this PR's own contrast goal. Same duplicated bug in nextjs-app/components/school/schoolSections.module.css (lines ~77-86)..leaflet-container(if (el.closest('.leaflet-container')) continue;), while the two pre-existing tests just above it were narrowed in this same diff from excluding.leaflet-containerto only.leaflet-tile-panespecifically so they'd cover the newly palette-styled Leaflet chrome (html .leaflet-control-attribution,.leaflet-bar a, etc.) added in globals.css. The inconsistency means the new AA test never actually contrast-checks the Leaflet controls this PR just restyled.