fix(design): resolve the font tokens — staging was rendering entirely in Times #87

Merged
tudor merged 1 commits from fix/font-cascade-and-map-palette into main 2026-08-06 12:52:35 +00:00
Owner

Staging audit of #86 found the whole site rendering in Times. Screenshot-free proof: getComputedStyle(document.body).fontFamily on https://stx.schoolcompare.co.uk returns "Times", and --font-display / --font-ui both compute to the empty string.

Root cause

next/font's variable classes were on <body>, while globals.css declares the font tokens on :root:

:root { --font-display: var(--font-schibsted), "Schibsted Grotesk", ...; }

A custom property's var() references resolve on the element that declares it. With --font-schibsted defined only on <body>, the reference was undefined at :root, so --font-display computed to the guaranteed-invalid value and every font-family referencing it silently fell back.

Nothing threw. The build was green. The only symptom was visual — which is exactly why it reached staging.

Verified the mechanism in a real browser, both ways round, using the same CSS shapes:

variable class on --font-display at :root resulting body font
<body> (shipped) "" Times
<html> (this PR) "Schibsted Grotesk", … Schibsted Grotesk

Two colour escapes from the same audit

  • Form controls rendered pure black. They don't inherit font or colour from their parent, so the omni search input and the map's "Open full map" button fell back to the UA default. Subtle against --text-primary in light mode; invisible on the dark ground. Fixed with a base inherit rule.
  • Leaflet ships its own palette#ddd tile backdrop, #333 attribution text, and a #0078A8 link blue that was the most saturated colour anywhere on the site and belongs to no part of this system. The map chrome is now tokenised. Tiles stay as OSM renders them.

A full palette sweep across home, rankings and a school detail page found no other off-palette colours — everything else resolved to tokens correctly.

Gates added

The existing suite passed while the site was entirely in Times, so:

  • font tokens resolve to a non-empty value, and the applied family is Schibsted rather than a serif fallback
  • no visible element renders in the browser's default black
  • every rendered colour comes from the token palette — the manual audit, turned into a gate

Also made the og:image check environment-relative: metadataBase pins canonical URLs to the production host, so the absolute URL pointed off-environment when the suite ran against staging.

🤖 Generated with Claude Code

Staging audit of #86 found the whole site rendering in **Times**. Screenshot-free proof: `getComputedStyle(document.body).fontFamily` on https://stx.schoolcompare.co.uk returns `"Times"`, and `--font-display` / `--font-ui` both compute to the empty string. ## Root cause next/font's variable classes were on `<body>`, while `globals.css` declares the font tokens on `:root`: ```css :root { --font-display: var(--font-schibsted), "Schibsted Grotesk", ...; } ``` A custom property's `var()` references resolve **on the element that declares it**. With `--font-schibsted` defined only on `<body>`, the reference was undefined at `:root`, so `--font-display` computed to the guaranteed-invalid value and every `font-family` referencing it silently fell back. Nothing threw. The build was green. The only symptom was visual — which is exactly why it reached staging. Verified the mechanism in a real browser, both ways round, using the same CSS shapes: | variable class on | `--font-display` at `:root` | resulting body font | |---|---|---| | `<body>` (shipped) | `""` | `Times` | | `<html>` (this PR) | `"Schibsted Grotesk", …` | `Schibsted Grotesk` | ## Two colour escapes from the same audit - **Form controls rendered pure black.** They don't inherit font or colour from their parent, so the omni search input and the map's "Open full map" button fell back to the UA default. Subtle against `--text-primary` in light mode; invisible on the dark ground. Fixed with a base inherit rule. - **Leaflet ships its own palette** — `#ddd` tile backdrop, `#333` attribution text, and a `#0078A8` link blue that was the most saturated colour anywhere on the site and belongs to no part of this system. The map chrome is now tokenised. Tiles stay as OSM renders them. A full palette sweep across home, rankings and a school detail page found **no other off-palette colours** — everything else resolved to tokens correctly. ## Gates added The existing suite passed while the site was entirely in Times, so: - font tokens resolve to a non-empty value, and the applied family is Schibsted rather than a serif fallback - no visible element renders in the browser's default black - every rendered colour comes from the token palette — the manual audit, turned into a gate Also made the og:image check environment-relative: `metadataBase` pins canonical URLs to the production host, so the absolute URL pointed off-environment when the suite ran against staging. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
tudor added 1 commit 2026-08-06 12:38:45 +00:00
fix(design): resolve the font tokens, and pull form controls and map chrome onto the palette
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
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 50s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m22s
2433101fa0
Staging audit of the Cohort identity found the whole site rendering in Times.

Root cause: next/font's variable classes were on <body>, while globals.css
declares --font-display/--font-ui/--font-prose on :root as
`var(--font-schibsted), ...`. A custom property's var() references resolve on
the element that declares it, so at :root --font-schibsted was undefined,
--font-display computed to the guaranteed-invalid value, and every
font-family referencing it fell back. Nothing threw and the build was green —
the only symptom was visual.

Verified the mechanism in a browser both ways round: class on <body> gives an
empty token and a Times body font; class on <html> resolves to Schibsted
Grotesk. The classes now sit on <html>.

Two colour escapes from the same audit:

* Form controls don't inherit font or colour from their parent, so the omni
  search input and the map's "Open full map" button rendered in the system
  font at pure black. Nearly invisible against --text-primary in light mode
  and completely invisible on the dark ground. Added a base inherit rule.
* Leaflet ships its own palette — a #ddd tile backdrop, #333 attribution text
  and a #0078A8 link blue that was the most saturated colour anywhere on the
  site. The map chrome now uses tokens; the tiles stay as OSM renders them.

Three e2e gates added, because the existing suite passed while the site was
entirely in Times:

* the font tokens resolve to a non-empty value and the applied family is
  Schibsted, not a serif fallback
* no visible element renders in the browser's default black
* every rendered colour comes from the token palette — the manual audit,
  turned into a gate

Also made the og:image check environment-relative: metadataBase pins canonical
URLs to the production host, so the absolute URL pointed off-environment when
the suite ran against staging.

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

🤖 AI Code Review (Claude Code)

This PR fixes a real bug where next/font CSS variable classes were applied to instead of <html>/:root, causing every font-family token to resolve to nothing and the whole site to fall back to Times; it also adds palette-aware styling for native form controls and Leaflet map chrome, plus new Playwright tests that assert fonts/colors actually resolve from the token system, and fixes an existing OG-image test to work across staging/production hosts. The change is frontend/test-only, well-reasoned, and includes strong regression coverage for the class of bug it fixes; no severe production, security, or data-integrity issues were found.

🟡 Minor

  • nextjs-app/app/globals.css: The new input/select/textarea/button rule sets color: inherit but not a matching background-color. If native controls still render with the browser's default white background, inherited dark-theme text (light foreground token) could become low-contrast or invisible against that background — the same failure mode this PR is otherwise trying to eliminate. Not verifiable from the diff alone since background styling may already exist elsewhere in the file.
  • nextjs-app/app/globals.css: .leaflet-control-attribution uses rgba(var(--text-inverse-rgb), 0.82), which depends on a --text-inverse-rgb custom property (raw R,G,B components, not a full color) that isn't defined in this diff. If it isn't defined elsewhere, the rgba() fails to parse and the attribution bar background silently falls back to Leaflet's own default instead of the token palette.
  • e2e/tests/journeys.spec.ts: In the 'rendered colours all come from the token palette' test, the allow-list only keeps :root custom properties matching /^#|^rgb/; any token defined with a named CSS color (e.g. white) or hsl()/hsla() would be silently excluded from the allow-list, causing legitimate on-token elements to be flagged as off-palette and the test to fail spuriously.
## 🤖 AI Code Review (Claude Code) This PR fixes a real bug where next/font CSS variable classes were applied to <body> instead of <html>/:root, causing every font-family token to resolve to nothing and the whole site to fall back to Times; it also adds palette-aware styling for native form controls and Leaflet map chrome, plus new Playwright tests that assert fonts/colors actually resolve from the token system, and fixes an existing OG-image test to work across staging/production hosts. The change is frontend/test-only, well-reasoned, and includes strong regression coverage for the class of bug it fixes; no severe production, security, or data-integrity issues were found. ### 🟡 Minor - **nextjs-app/app/globals.css**: The new input/select/textarea/button rule sets `color: inherit` but not a matching `background-color`. If native controls still render with the browser's default white background, inherited dark-theme text (light foreground token) could become low-contrast or invisible against that background — the same failure mode this PR is otherwise trying to eliminate. Not verifiable from the diff alone since background styling may already exist elsewhere in the file. - **nextjs-app/app/globals.css**: .leaflet-control-attribution uses `rgba(var(--text-inverse-rgb), 0.82)`, which depends on a `--text-inverse-rgb` custom property (raw R,G,B components, not a full color) that isn't defined in this diff. If it isn't defined elsewhere, the rgba() fails to parse and the attribution bar background silently falls back to Leaflet's own default instead of the token palette. - **e2e/tests/journeys.spec.ts**: In the 'rendered colours all come from the token palette' test, the allow-list only keeps :root custom properties matching /^#|^rgb/; any token defined with a named CSS color (e.g. `white`) or `hsl()`/`hsla()` would be silently excluded from the allow-list, causing legitimate on-token elements to be flagged as off-palette and the test to fail spuriously.
tudor merged commit 91b49f1a59 into main 2026-08-06 12:52:35 +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#87