Compare commits

..
Author SHA1 Message Date
TudorandClaude Opus 5 2433101fa0 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
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>
2026-08-06 13:38:17 +01:00
3 changed files with 171 additions and 3 deletions
+116 -1
View File
@@ -642,7 +642,10 @@ test('the brand asset set is complete and served', async ({ page }) => {
await expect(ogImage).toHaveCount(1); await expect(ogImage).toHaveCount(1);
const ogUrl = await ogImage.getAttribute('content'); const ogUrl = await ogImage.getAttribute('content');
expect(ogUrl).toBeTruthy(); expect(ogUrl).toBeTruthy();
const og = await page.request.get(ogUrl!); // metadataBase pins canonical URLs to the production host, which is correct
// for prod but means the absolute URL points off-environment on staging.
// Fetch the path against whichever environment we're actually testing.
const og = await page.request.get(new URL(ogUrl!).pathname + new URL(ogUrl!).search);
expect(og.ok()).toBe(true); expect(og.ok()).toBe(true);
expect(og.headers()['content-type']).toContain('image/png'); expect(og.headers()['content-type']).toContain('image/png');
@@ -702,3 +705,115 @@ test('the dark theme actually repaints the page', async ({ browser }) => {
expect(dark.bg).not.toBe(light.bg); expect(dark.bg).not.toBe(light.bg);
expect(dark.fg).not.toBe(light.fg); expect(dark.fg).not.toBe(light.fg);
}); });
/**
* Typography and palette integrity.
*
* The identity PR shipped with every font-family silently falling back to
* Times: the font variables landed on <body> while the tokens referencing
* them were declared on :root, so --font-display computed to the
* guaranteed-invalid value. Nothing threw, no test failed, and the build was
* green — the only symptom was visual. These assertions make that class of
* failure loud.
*/
test('the brand typefaces actually load and apply', async ({ page }) => {
await page.goto('/');
await expect(page.locator('h1').first()).toBeVisible({ timeout: 15_000 });
const fonts = await page.evaluate(() => {
const root = getComputedStyle(document.documentElement);
return {
body: getComputedStyle(document.body).fontFamily,
heading: getComputedStyle(document.querySelector('h1')!).fontFamily,
displayToken: root.getPropertyValue('--font-display').trim(),
uiToken: root.getPropertyValue('--font-ui').trim(),
proseToken: root.getPropertyValue('--font-prose').trim(),
};
});
// An empty token means the var() chain broke, which is the exact failure
// mode above — the computed font-family would look plausible either way.
expect(fonts.displayToken, '--font-display resolved').not.toBe('');
expect(fonts.uiToken, '--font-ui resolved').not.toBe('');
expect(fonts.proseToken, '--font-prose resolved').not.toBe('');
expect(fonts.body).toContain('Schibsted');
expect(fonts.heading).toContain('Schibsted');
// Times/serif anywhere in the interface means we fell back.
expect(fonts.body).not.toMatch(/Times|serif$/);
});
test('no visible text falls back to the browser default black', async ({ page }) => {
// Form controls don't inherit colour from their parent, so a missing
// declaration renders pure black — subtle in light mode, invisible in dark.
await page.goto('/');
await expect(page.locator('h1').first()).toBeVisible({ timeout: 15_000 });
const blacks = await page.evaluate(() =>
[...document.querySelectorAll('body *')]
.filter((el) => {
const r = el.getBoundingClientRect();
if (r.width < 2 || r.height < 2) return false;
if (el.closest('.leaflet-container')) return false;
return getComputedStyle(el).color === 'rgb(0, 0, 0)';
})
.map((el) => el.tagName.toLowerCase() + '.' + (el.getAttribute('class') || '').split(' ')[0])
.slice(0, 10)
);
expect(blacks, `elements rendering pure black: ${blacks.join(', ')}`).toEqual([]);
});
test('rendered colours all come from the token palette', async ({ page }) => {
// Turns the manual design audit into a gate: anything painted with a colour
// the token layer doesn't define has escaped the system, and will not
// follow the dark theme.
await page.goto('/rankings');
await expect(page.locator('table, [class*="rankings"]').first()).toBeVisible({ timeout: 15_000 });
const strays = await page.evaluate(() => {
const root = getComputedStyle(document.documentElement);
const palette = new Set<string>();
for (const sheet of document.styleSheets) {
let rules: CSSRuleList;
try { rules = sheet.cssRules; } catch { continue; }
for (const rule of rules) {
const r = rule as CSSStyleRule;
if (r.selectorText !== ':root' || !r.style) continue;
for (const prop of r.style) {
if (!prop.startsWith('--')) continue;
const v = root.getPropertyValue(prop).trim();
if (v) palette.add(v.toLowerCase());
}
}
}
const norm = (c: string) => {
const d = document.createElement('div');
d.style.color = c;
document.body.appendChild(d);
const v = getComputedStyle(d).color;
d.remove();
return v;
};
const allowed = new Set([...palette].filter((v) => /^#|^rgb/.test(v)).map(norm));
const found: string[] = [];
for (const el of document.querySelectorAll('body *')) {
const box = el.getBoundingClientRect();
if (box.width < 2 || box.height < 2) continue;
if (el.closest('.leaflet-container')) continue; // OSM tiles are imagery
const s = getComputedStyle(el);
const checks: Array<[string, string]> = [['color', s.color]];
if (s.backgroundColor !== 'rgba(0, 0, 0, 0)') checks.push(['background', s.backgroundColor]);
for (const [prop, value] of checks) {
if (!value || value.startsWith('rgba(') || allowed.has(value)) continue;
const cls = (el.getAttribute('class') || '(none)').split(' ')[0];
const entry = `${value} as ${prop} on ${cls}`;
if (!found.includes(entry)) found.push(entry);
}
}
return found.slice(0, 12);
});
expect(strays, `off-palette colours: ${strays.join('; ')}`).toEqual([]);
});
+46
View File
@@ -264,6 +264,18 @@ body {
-webkit-tap-highlight-color: transparent; -webkit-tap-highlight-color: transparent;
} }
/* Form controls don't inherit font or colour from their parent — the UA
supplies its own. Without this they render in the system font at pure
black, which is nearly invisible against --text-primary in light mode and
completely invisible on the dark ground. */
input,
select,
textarea,
button {
font-family: inherit;
color: inherit;
}
/* Every digit that could line up in a column does. This is a data product; /* Every digit that could line up in a column does. This is a data product;
proportional numerals in a results table read as amateur. */ proportional numerals in a results table read as amateur. */
table, table,
@@ -401,6 +413,40 @@ table,
font-size: 0.8125rem; font-size: 0.8125rem;
} }
/*
* Leaflet ships its own palette — a #ddd tile backdrop, #333 attribution text
* and a #0078A8 link blue that is the most saturated colour anywhere on the
* site and belongs to no part of this system. Pull the map chrome onto the
* tokens. This matters most in dark mode, where Leaflet's white attribution
* bar would otherwise sit on a near-black page.
*
* The tiles themselves stay as OSM renders them; only the chrome is ours.
*/
.leaflet-container {
background: var(--bg-secondary);
font-family: var(--font-ui);
}
.leaflet-control-attribution {
background: rgba(var(--text-inverse-rgb), 0.82);
color: var(--text-muted);
}
.leaflet-control-attribution a {
color: var(--brand);
}
.leaflet-bar a {
background: var(--bg-card);
color: var(--text-primary);
border-bottom-color: var(--border);
}
.leaflet-bar a:hover {
background: var(--bg-secondary);
color: var(--text-primary);
}
/* Main content column */ /* Main content column */
.main { .main {
max-width: 1400px; max-width: 1400px;
+9 -2
View File
@@ -80,7 +80,14 @@ export default function RootLayout({
children: React.ReactNode; children: React.ReactNode;
}>) { }>) {
return ( return (
<html lang="en"> // The font variable classes must sit on <html>, not <body>. globals.css
// declares --font-display/--font-ui on :root as var(--font-schibsted),
// and a custom property's var() references resolve on the element that
// declares it. With the classes on <body>, --font-schibsted was undefined
// at :root, so --font-display computed to the guaranteed-invalid value and
// every font-family that referenced it silently fell back — the whole site
// rendered in Times.
<html lang="en" className={`${schibsted.variable} ${literata.variable}`}>
<head> <head>
<link rel="preconnect" href="https://analytics.schoolcompare.co.uk" /> <link rel="preconnect" href="https://analytics.schoolcompare.co.uk" />
<link rel="preconnect" href="https://api.postcodes.io" /> <link rel="preconnect" href="https://api.postcodes.io" />
@@ -102,7 +109,7 @@ export default function RootLayout({
strategy="afterInteractive" strategy="afterInteractive"
/> />
</head> </head>
<body className={`${schibsted.variable} ${literata.variable}`}> <body>
<ComparisonProvider> <ComparisonProvider>
<a href="#main-content" className="skip-link">Skip to main content</a> <a href="#main-content" className="skip-link">Skip to main content</a>
<Navigation /> <Navigation />