fix(map): the popup never took the dark theme

leaflet.css paints `background: white; color: #333` on the popup card and its
tip. LeafletMapInner binds themed content into it — the school name and the
headline figure are var(--text-primary) — so in dark mode #E9EEF0 landed on
#FFFFFF at 1.17:1. The two things the popup exists to say were the two least
readable things on the page.

Every other foreground in that popup failed too, from the same cause: the
muted phase line at 2.90:1, the vs-national delta at 1.94:1, the Ofsted badge
at 1.74:1. Moving the surface onto --bg-card fixes all of them at once —
13.52, 5.45, 8.14 and 9.11:1 respectively. In light mode --bg-card is #FFFFFF,
so the popup renders exactly as it did.

globals.css already pulls the rest of Leaflet's chrome onto the tokens, and
says why: "this matters most in dark mode, where Leaflet's white attribution
bar would otherwise sit on a near-black page." The popup was simply missed.

The View Details button needed its own fix. It pairs background:var(--status-
above) with a literal white label, which theming the card does not reach:
--status-above is #36743F in light but #7FCB8A in dark, taking the label from
5.63:1 to 1.94:1. --text-inverse is the token for ink on a saturated fill, and
the popup's own Ofsted badge already uses it.

darkThemeSafety already guards this defect class, but only inside .module.css.
Neither half of this one lives there — the surface is a third party's, the
text is inline in a TSX template — so it scanned clean throughout. Two rules
added for the layer it could not see. Fixing the grouped-selector blind spot
in its rules() helper was needed to write them: taking only a selector's last
line discarded every selector in a grouped rule but the final one, which makes
a safety guard fail open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM

nextjs-app/__tests__/components/RouteTrail.test.tsx

@@ -0,0 +1,38 @@
+/**
+ * The trail has to be written by something, and it has to be written on every
+ * route — not only the ones that happen to track an event.
+ */
+import { render } from '@testing-library/react';
+
+const recordVisitedPath = jest.fn();
+let pathname = '/schools/brentwood';
+
+jest.mock('next/navigation', () => ({ usePathname: () => pathname }));
+jest.mock('@/lib/analytics', () => ({
+  recordVisitedPath: (p: string) => recordVisitedPath(p),
+}));
+
+// eslint-disable-next-line @typescript-eslint/no-var-requires
+const { RouteTrail } = require('@/components/RouteTrail');
+
+describe('RouteTrail', () => {
+  beforeEach(() => recordVisitedPath.mockClear());
+
+  it('records the page it is mounted on', () => {
+    render(<RouteTrail />);
+    expect(recordVisitedPath).toHaveBeenCalledWith('/schools/brentwood');
+  });
+
+  it('records each new route as the user moves through the app', () => {
+    const { rerender } = render(<RouteTrail />);
+    pathname = '/school/115429-brentwood-school';
+    rerender(<RouteTrail />);
+    expect(recordVisitedPath).toHaveBeenLastCalledWith(
+      '/school/115429-brentwood-school');
+  });
+
+  it('renders nothing, so it can sit anywhere in the layout', () => {
+    const { container } = render(<RouteTrail />);
+    expect(container).toBeEmptyDOMElement();
+  });
+});

nextjs-app/__tests__/components/darkThemeSafety.test.ts

@@ -32,10 +32,17 @@ function stylesheets(dir: string): string[] {
 }
 
 /** Innermost `selector { body }` pairs. Nested at-rules never match as rules,
- *  because their body contains braces. */
+ *  because their body contains braces.
+ *
+ *  Comments are stripped before matching rather than after, so that the whole
+ *  selector survives. Taking only its last line — which is what stripping a
+ *  leading comment used to require — silently discarded every selector in a
+ *  grouped rule but the final one, and a safety guard that cannot see half its
+ *  input fails open. */
 function rules(css: string): Array<{ selector: string; body: string }> {
-  return Array.from(css.matchAll(/([^{}]+)\{([^{}]*)\}/g), (m) => ({
-    selector: m[1].trim().split('\n').pop()!.trim(),
+  const bare = css.replace(/\/\*[\s\S]*?\*\//g, '');
+  return Array.from(bare.matchAll(/([^{}]+)\{([^{}]*)\}/g), (m) => ({
+    selector: m[1].trim().replace(/\s*\n\s*/g, ' '),
     body: m[2],
   }));
 }
@@ -45,6 +52,15 @@ const THEMED_COLOR = /(?:^|[^-])color:\s*var\(--/;
 
 const files = stylesheets(COMPONENTS);
 
+/** Component sources, for the third-party-surface rule below. */
+function sources(dir: string): string[] {
+  return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) return sources(full);
+    return entry.name.endsWith('.tsx') ? [full] : [];
+  });
+}
+
 describe('dark-theme safety', () => {
   it('finds stylesheets to check', () => {
     expect(files.length).toBeGreaterThan(0);
@@ -76,3 +92,73 @@ describe('dark-theme safety', () => {
     expect(offenders).toEqual([]);
   });
 });
+
+/**
+ * The same defect one stylesheet further out.
+ *
+ * The rules above scan our own CSS modules. They cannot see a surface painted
+ * by a third-party sheet: leaflet.css hardcodes `background: white` on
+ * `.leaflet-popup-content-wrapper` and `.leaflet-popup-tip`, and
+ * LeafletMapInner builds its popup as an HTML string with inline
+ * `color: var(--text-primary)`. Neither half lives in a .module.css, so the
+ * module scan passed while dark mode rendered #E9EEF0 on #FFFFFF — 1.17:1,
+ * with the school name and the headline figure effectively invisible.
+ *
+ * globals.css already pulls the rest of Leaflet's chrome onto the tokens (the
+ * attribution bar, the zoom controls) for exactly this reason. The popup was
+ * simply missed.
+ */
+describe('third-party surfaces under themed text', () => {
+  const GLOBALS = path.join(__dirname, '..', '..', 'app', 'globals.css');
+
+  /** Leaflet surfaces our own code writes token-coloured text onto. */
+  const LEAFLET_POPUP_SURFACES = [
+    '.leaflet-popup-content-wrapper',
+    '.leaflet-popup-tip',
+  ];
+
+  it('still finds a component painting themed text into a Leaflet popup', () => {
+    // Guards the rule below against passing vacuously if the popups are ever
+    // rewritten as React components rather than HTML strings.
+    const themed = sources(COMPONENTS).filter((file) => {
+      const src = fs.readFileSync(file, 'utf8');
+      return /bindPopup\(/.test(src) && /color:var\(--|color: var\(--/.test(src);
+    });
+
+    expect(themed.length).toBeGreaterThan(0);
+  });
+
+  it('themes the Leaflet popup surface, because the text on it is themed', () => {
+    const globals = rules(fs.readFileSync(GLOBALS, 'utf8'));
+
+    const unthemed = LEAFLET_POPUP_SURFACES.filter((surface) => {
+      const rule = globals.find((r) => r.selector.includes(surface));
+      return !rule || !/background[^;]*var\(--/.test(rule.body);
+    });
+
+    // Leaflet's white is not a colour this site owns. Either the surface
+    // follows the theme or the text on it must be literal — and the text is
+    // already themed.
+    expect(unthemed).toEqual([]);
+  });
+
+  it('never puts a literal white label on a themed fill', () => {
+    /*
+     * The mirror image of the module-CSS rule above, and the half of the popup
+     * that theming the card does not reach. "View Details" is
+     * `background:var(--status-above);color:white`; --status-above is #36743F
+     * in light but #7FCB8A in dark, so the label went from 5.63:1 to 1.94:1.
+     *
+     * --text-inverse is the token for ink on a saturated fill — #FFFFFF in
+     * light, #111A20 in dark — and the popup's Ofsted badge already uses it.
+     */
+    const offenders = sources(COMPONENTS).flatMap((file) => {
+      const src = fs.readFileSync(file, 'utf8');
+      return Array.from(
+        src.matchAll(/background:\s*var\(--[^;"']*;[^"']*?color:\s*(white|#fff\b|#ffffff\b)/gi),
+        () => path.relative(COMPONENTS, file));
+    });
+
+    expect(offenders).toEqual([]);
+  });
+});

nextjs-app/__tests__/lib/analytics.test.ts

@@ -62,3 +62,97 @@ describe('getNavigationSource', () => {
     expect(getNavigationSource()).toBe('direct');
   });
 });
+
+/*
+ * The defect the existing suite could not see.
+ *
+ * Every test above sets document.referrer, which the browser writes only when
+ * a *document* loads. Every internal navigation in this app is an App Router
+ * soft navigation — history.pushState, no new document — so document.referrer
+ * keeps naming whatever opened the tab for the whole session. Verified on
+ * staging: /schools/brentwood → click a school → URL changes to /school/…
+ * and document.referrer is still "".
+ *
+ * So `from` reported 'direct' for essentially every in-app journey, and the
+ * suite passed because it only ever exercised the full-page-load path.
+ */
+function freshAnalytics() {
+  let mod!: typeof import('@/lib/analytics');
+  jest.isolateModules(() => {
+    mod = require('@/lib/analytics');
+  });
+  return mod;
+}
+
+function at(path: string) {
+  window.history.pushState({}, '', path);
+}
+
+describe('getNavigationSource across a soft navigation', () => {
+  afterEach(() => {
+    referrer('');
+    at('/');
+  });
+
+  it('attributes a school view to the place page the user actually came from', () => {
+    const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
+    at('/schools/brentwood');
+    recordVisitedPath('/schools/brentwood');
+
+    at('/school/115429-brentwood-school');
+    recordVisitedPath('/school/115429-brentwood-school');
+
+    expect(source()).toBe('place');
+  });
+
+  it('does not depend on whether the new path was recorded first', () => {
+    // The trail is written by a layout-level effect and read by a page-level
+    // one. React orders those by tree position, which is not a contract worth
+    // resting a measurement on, so the answer must be the same either way.
+    const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
+    recordVisitedPath('/rankings');
+    at('/school/115429-brentwood-school');
+
+    expect(source()).toBe('rankings');
+  });
+
+  it('names the previous page, not the current one, when both are schools', () => {
+    const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
+    at('/school/100010-brecknock-primary-school');
+    recordVisitedPath('/school/100010-brecknock-primary-school');
+
+    at('/school/115429-brentwood-school');
+    recordVisitedPath('/school/115429-brentwood-school');
+
+    expect(source()).toBe('detail');
+  });
+
+  it('looks past a return visit to the page the user came back from', () => {
+    const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
+    for (const p of ['/schools/brentwood', '/school/115429-brentwood-school',
+                     '/schools/brentwood']) {
+      at(p);
+      recordVisitedPath(p);
+    }
+    expect(source()).toBe('detail');
+  });
+
+  it('falls back to the referrer on a real document load, where it is true', () => {
+    // A fresh module is a fresh document: nothing has been recorded, and
+    // document.referrer is meaningful again.
+    const { getNavigationSource: source } = freshAnalytics();
+    at('/school/115429-brentwood-school');
+    referrer(`${ORIGIN}/schools/barnet`);
+
+    expect(source()).toBe('place');
+  });
+
+  it('still reads an arrival from outside as direct', () => {
+    const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
+    at('/schools/brentwood');
+    recordVisitedPath('/schools/brentwood');
+    referrer('https://www.google.com/search?q=schools+in+brentwood');
+
+    expect(source()).toBe('direct');
+  });
+});

nextjs-app/app/globals.css

@@ -588,6 +588,35 @@ html .leaflet-bar a:hover {
     color: var(--text-primary);
 }
 
+/*
+ * The popup, which leaflet.css paints `background: white; color: #333` on both
+ * the card and its tip. The content LeafletMapInner binds into it is themed —
+ * the school name and the headline figure are `var(--text-primary)` — so in
+ * dark mode that was #E9EEF0 on #FFFFFF, a contrast ratio of 1.17:1. The name
+ * and the number were the two least readable things on the page.
+ *
+ * Moving the surface onto --bg-card fixes every foreground at once rather than
+ * one at a time: the muted phase line goes 2.90:1 -> 5.45:1, the vs-national
+ * delta 1.94:1 -> 8.14:1, the Ofsted badge 1.74:1 -> 9.11:1. In light mode
+ * --bg-card is #FFFFFF, so the popup looks as it always did.
+ */
+html .leaflet-popup-content-wrapper,
+html .leaflet-popup-tip {
+    background: var(--bg-card);
+    color: var(--text-primary);
+}
+
+/* Leaflet's own selector is `.leaflet-container a.leaflet-popup-close-button`
+   at 0,2,1 — an `html` prefix alone would lose to it. */
+html .leaflet-container a.leaflet-popup-close-button {
+    color: var(--text-muted);
+}
+
+html .leaflet-container a.leaflet-popup-close-button:hover,
+html .leaflet-container a.leaflet-popup-close-button:focus {
+    color: var(--text-primary);
+}
+
 /* Main content column */
 .main {
     max-width: 1400px;

nextjs-app/app/layout.tsx

@@ -4,6 +4,7 @@ import Script from 'next/script';
 import { Navigation } from '@/components/Navigation';
 import { Footer } from '@/components/Footer';
 import { ComparisonToast } from '@/components/ComparisonToast';
+import { RouteTrail } from '@/components/RouteTrail';
 import { ComparisonProvider } from '@/context/ComparisonProvider';
 import { SITE_URL } from '@/lib/site';
 import './globals.css';
@@ -114,6 +115,10 @@ export default function RootLayout({
         />
       </head>
       <body>
+        {/* Records every route so funnel attribution has a previous page to
+            name. document.referrer cannot: a soft navigation creates no
+            document, so the browser never updates it. */}
+        <RouteTrail />
         <ComparisonProvider>
           <a href="#main-content" className="skip-link">Skip to main content</a>
           <Navigation />

nextjs-app/components/LeafletMapInner.tsx

@@ -184,7 +184,7 @@ export default function LeafletMapInner({ schools, center, zoom, referencePoint,
     ${phaseLabel}${school.local_authority ? ` · ${escapeHtml(school.local_authority)}` : ''}${distanceStr}
   </div>
   ${metricHtml}
-  <a href="${slug}" style="display:block;text-align:center;padding:6px;background:var(--status-above);color:white;border-radius:5px;text-decoration:none;font-size:12px;font-weight:600;margin-top:8px">View Details →</a>
+  <a href="${slug}" style="display:block;text-align:center;padding:6px;background:var(--status-above);color:var(--text-inverse);border-radius:5px;text-decoration:none;font-size:12px;font-weight:600;margin-top:8px">View Details →</a>
 </div>`;
 
         marker.bindPopup(popupContent);

nextjs-app/components/RouteTrail.tsx

@@ -0,0 +1,27 @@
+/**
+ * Writes the in-app navigation trail that funnel attribution reads.
+ *
+ * Renders nothing. It exists because document.referrer cannot answer "which
+ * page did they come from" in an App Router app: a soft navigation creates no
+ * document, so the browser never updates it. See the trail comment in
+ * lib/analytics.ts.
+ *
+ * Mounted once in the root layout, so every route is recorded — including the
+ * ones that fire no event of their own, which are still somebody else's
+ * previous page.
+ */
+'use client';
+
+import { useEffect } from 'react';
+import { usePathname } from 'next/navigation';
+import { recordVisitedPath } from '@/lib/analytics';
+
+export function RouteTrail() {
+  const pathname = usePathname();
+
+  useEffect(() => {
+    recordVisitedPath(pathname);
+  }, [pathname]);
+
+  return null;
+}

nextjs-app/lib/analytics.ts

@@ -60,22 +60,77 @@ export function track(name: EventName, data?: Payload): void {
 export type NavigationSource =
   'search' | 'rankings' | 'compare' | 'detail' | 'place' | 'direct';
 
+/*
+ * The in-app trail.
+ *
+ * document.referrer is written by the browser only when a *document* loads.
+ * Every internal navigation here is an App Router soft navigation —
+ * history.pushState, no new document — so document.referrer goes on naming
+ * whatever opened the tab (usually nothing, or a search engine) for the whole
+ * session. Reading it to answer "which page did they come from" therefore
+ * returned 'direct' for essentially every in-app journey, including the one
+ * the location layer exists to produce.
+ *
+ * Verified on staging: /schools/brentwood, click a school, the URL becomes
+ * /school/… and document.referrer is still "".
+ *
+ * A module-level trail is the counterpart with exactly the right lifetime. It
+ * survives soft navigation, and it dies on a real document load — which is
+ * precisely when document.referrer becomes meaningful again, so the two cover
+ * each other with no overlap.
+ */
+const TRAIL_LIMIT = 4;
+const trail: string[] = [];
+
+/** Record a path the user is now on. Called by RouteTrail on every route. */
+export function recordVisitedPath(path: string): void {
+  if (trail[trail.length - 1] === path) return;
+  trail.push(path);
+  if (trail.length > TRAIL_LIMIT) trail.shift();
+}
+
+/**
+ * The most recent path that is not the one being viewed.
+ *
+ * Skipping the current path rather than taking trail[length - 2] is what
+ * makes the answer independent of ordering: the trail is written by a
+ * layout-level effect and read by a page-level one, and React orders those by
+ * tree position — not a contract worth resting a measurement on. It also
+ * gives the right answer when the user goes back to a page they came from.
+ */
+function previousInAppPath(): string | null {
+  if (typeof window === 'undefined') return null;
+  const current = window.location.pathname;
+  for (let i = trail.length - 1; i >= 0; i -= 1) {
+    if (trail[i] !== current) return trail[i];
+  }
+  return null;
+}
+
+function classifyPath(p: string): NavigationSource {
+  if (p === '/' || p === '') return 'search';
+  if (p.startsWith('/rankings')) return 'rankings';
+  if (p.startsWith('/compare')) return 'compare';
+  // `/schools/` before `/school/`: they differ by one letter and mean
+  // different things — the location layer versus a single school. Checked
+  // first so the narrower-looking prefix cannot shadow it if either string
+  // is ever edited.
+  if (p.startsWith('/schools/')) return 'place';
+  if (p.startsWith('/school/')) return 'detail';
+  return 'direct';
+}
+
 export function getNavigationSource(): NavigationSource {
+  const internal = previousInAppPath();
+  if (internal) return classifyPath(internal);
+
+  // No trail means this is the first page of the document, so the referrer is
+  // the only witness — and an honest one.
   if (typeof window === 'undefined' || !document.referrer) return 'direct';
   try {
     const ref = new URL(document.referrer);
     if (ref.origin !== window.location.origin) return 'direct';
-    const p = ref.pathname;
-    if (p === '/' || p === '') return 'search';
-    if (p.startsWith('/rankings')) return 'rankings';
-    if (p.startsWith('/compare')) return 'compare';
-    // `/schools/` before `/school/`: they differ by one letter and mean
-    // different things — the location layer versus a single school. Checked
-    // first so the narrower-looking prefix cannot shadow it if either string
-    // is ever edited.
-    if (p.startsWith('/schools/')) return 'place';
-    if (p.startsWith('/school/')) return 'detail';
-    return 'direct';
+    return classifyPath(ref.pathname);
   } catch {
     return 'direct';
   }