Compare commits

...
Author SHA1 Message Date
TudorandClaude Opus 5 a7829d591a fix(map): the popup never took the dark theme
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 8s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 15s
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
2026-08-27 22:34:18 +01:00
tudor cf9d41b476 Merge pull request 'fix(analytics): the funnel source read a referrer that never changes' (#134) from fix/navigation-source-soft-nav into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 12s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 52s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 1m40s
Reviewed-on: #134
2026-08-27 08:26:45 +00:00
TudorandClaude Opus 5 e820e7fecd fix(analytics): the funnel source read a referrer that never changes
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m5s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m13s
The staging E2E gate has been red since #132 merged (run 1064, and
1066 after it): "a school reached from a location page is attributed
to it, not to direct" expects `place`, receives `direct`.

#132 fixed a real bug — `/schools/` had no case and fell through to
`direct` — but the mechanism underneath it never worked.
getNavigationSource read document.referrer, which the browser writes
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 for the whole
session.

Verified on staging: load /schools/brentwood, click a school, the URL
becomes /school/… and document.referrer is still "".

So `from` reported `direct` for essentially every in-app journey, not
just the ones through the location layer — search, rankings, compare
and detail were all being counted as "typed the URL". The unit suite
passed throughout because every case set document.referrer directly,
which only happens on a full page load.

The fix is a module-level trail written by RouteTrail, a render-nothing
client component in the root layout. Its lifetime is exactly right: 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.

Reading it skips entries equal to the current path rather than taking
the second-to-last. That makes the answer independent of whether the
layout effect or the page effect ran first — React orders those by
tree position, which is not a contract worth resting a measurement on
— and it gives the right answer both when the user returns to a page
they came from and on a hard load of a school page.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM
2026-08-27 09:22:37 +01:00
tudor 4fdeb70a93 Merge pull request 'feat(places): say what each school is, not only how it scored' (#133) from feat/place-school-attributes into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 19s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 50s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 1m42s
Reviewed-on: #133
2026-08-27 07:59:10 +00:00
TudorandClaude Opus 5 9a1f56c431 feat(places): say what each school is, not only how it scored
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m4s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 2m44s
The location tables carried one column: a percentage. A parent
shortlisting from a town page is asking a different question first —
does it take my child's age, is it a faith school, does it have a
nursery — and the page could not answer any of it.

Primary tables gain Ages, Religious character, Nursery and
Constituency; secondary tables the same minus Nursery, which is a
question about a different intake. An all-through school renders in
both groups, so its nursery shows under primary alone.

The measure moves to the second column rather than the last. Six
columns overflow a phone and .tableWrap turns that into a horizontal
swipe; with the measure last, the one number the page exists for is
the one scrolled off the screen.

Cell rules are the ones the school page already uses, so the two
surfaces cannot disagree about the same school: "Does not apply",
"None" and "Not applicable" all read as no religious character, and
the en-dash age normalisation moves into formatAgeSpan, which
formatAgeRange now delegates to.

Backend: nursery_provision and parliamentary_constituency were not in
the place response. Both are optional GIAS mart columns that
data_loader degrades to NULL, and the `in rows.columns` guard keeps a
mart the pipeline has not rebuilt working.

Also fixes a live bug on the same line: SCHOOL_COLUMNS already ends
with latitude and longitude, and the endpoint concatenated them again,
so pandas dropped one of every duplicated pair and warned "columns are
not unique" on each request. Ordered de-duplication removes the
warning and the silent drop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM
2026-08-27 08:49:14 +01:00
tudor ade9dbb3ba Merge pull request 'feat(analytics): measure the location layer, and stop calling it direct' (#132) from feat/place-analytics into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 51s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 1m44s
2026-08-27 07:29:48 +00:00
TudorandClaude Opus 5 d1a8596208 feat(analytics): measure the location layer, and stop calling it direct
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 40s
The location pages were only half-tracked. Umami counts a pageview for
each of the ~3,900 URLs automatically, but nothing else: components/
places contained no track() call, and place_viewed was not even a
declared event name.

The part that mattered was worse than a gap. getNavigationSource mapped
a same-origin referrer to a funnel source and had no case for /schools/,
so every school view arriving through the location layer fell through to
'direct' — the bucket you read as "typed the URL, no referrer". W2's
whole purpose is funnelling search traffic onto school pages, so the one
measurement that says whether it worked was reporting the wrong answer,
and reporting it confidently. Verified live against staging: expected
"place", received "direct".

/schools/ is checked before /school/. They differ by one letter and mean
different things — the location layer versus a single school — and a
prefix test in the wrong order silently merges them.

place_viewed carries kind, slug, phase and school_count. kind is the
reason it exists: whether to keep investing in these pages turns on
which sort earns engagement, and a pageview cannot say, because all four
families share the /schools/ prefix and only the registry knows which is
which. It is a client component because PlaceView is a server component;
one line in PlaceView covers all four families, since they all render
through it.

Both E2E journeys were verified failing against staging first — one
because place_viewed does not exist there, the other on the exact
"place" vs "direct" mismatch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
2026-08-27 08:23:21 +01:00
tudor a3c09d9b67 Merge pull request 'fix(suggest): the dropdown reopened on top of the search results' (#131) from fix/suggest-reopens-over-results into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 50s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 1m37s
Reviewed-on: #131
2026-08-26 21:07:55 +00:00
tudor a7f4c86464 Merge pull request 'fix(search): the mobile hero search was indented by a card's padding' (#130) from fix/mobile-hero-search into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 12s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 50s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 5m50s
Reviewed-on: #130
2026-08-26 20:51:20 +00:00
TudorandClaude Opus 5 55363cbd18 fix(suggest): the dropdown reopened on top of the search results
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m2s
PR Checks / Backend Smoke (pull_request) Successful in 8s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 55s
Three staging-gate failures, two of them one real bug.

After a search, the results-page bar still holds the term in its input,
so on every render the query was >= 2 characters and the suggestion list
opened again — on top of the very results the search had just produced.
Playwright reported it as "<li role=option ...> intercepts pointer
events" while trying to click the first result; a reader would simply
have found their first result unclickable. Both the school-detail and
hero-map journeys failed on it, and neither is about autosuggest.

Suggestions now answer typing, not the mere presence of a value:
`hasTyped` gates the hook, is set on change, and is cleared when a
search is submitted or a suggestion is chosen. A pre-filled input makes
no request and shows no list.

Third failure was my test, not the product. An unphased place page
renders one table per phase, and an all-through school legitimately
appears in both — so the page's school links were never one alphabetical
run. The assertion collected them all together and only passed because
no town it picked had held an all-through school. When the data gave
Abbots Langley one, Breakspeare School appeared in the primary table and
again in the secondary, and the test failed on correct behaviour. It now
checks each table separately, and passes against the data that broke it.

Guards: a jest test that a pre-filled input neither fetches nor opens
(verified by reverting — it is the only one that fails), and an E2E
journey that submits a search and then requires the first result to be
clickable, which is the reader-facing version of the same thing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
2026-08-26 21:38:16 +01:00
tudor 868eb344f5 Merge pull request 'fix(map): the hero map's fade to the header was hardcoded white' (#129) from fix/dark-map-fade into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 51s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 0s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 5m49s
Reviewed-on: #129
2026-08-26 20:32:45 +00:00
TudorandClaude Opus 5 3236efa846 fix(map): the hero map's fade to the header was hardcoded white
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m7s
PR Checks / Backend Smoke (pull_request) Successful in 8s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 49s
The fade between the map band and the school header ramped through
rgba(255,255,255,...) and landed on var(--bg-card). In the light theme
that is white into white and invisible, as designed. In the dark theme
it climbed to 95% WHITE and then met a near-black card, putting a bright
band across the full width exactly where the map should dissolve into
the title.

Fading to the colour the gradient lands on is the whole trick, and it
only works if that colour is a token — so --bg-card-rgb now exists in
both theme blocks, matching the --hero-ground-rgb precedent.

Two more defects in the same file, same cause, found while in there:

The controls floating over the map paired a hardcoded white background
with color: var(--text-primary), which resolves to #E9EEF0 in dark —
near-white text on a near-white button. These deliberately do NOT follow
the theme, because the map tiles are light in both, so the ink is now
literal too and says why. A themed token is the wrong tool for a surface
that never changes.

The loading skeleton swept 50% white across var(--bg-secondary), which
is a bright flash every 1.4s on a dark page. It now sweeps toward the
card colour, a shade lighter than the ground in both themes.

The guard is a stylesheet test rather than a render test, because the
bug is invisible in the theme it was written for. Verified by reverting
each fix in turn: it names .fade and .openHint exactly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
2026-08-26 21:20:57 +01:00
21 changed files with 1145 additions and 29 deletions

No files matched your search

+16 -3
View File
@@ -1337,9 +1337,22 @@ async def get_place(request: Request, kind: str, slug: str,
for m in ("rwm_expected_pct", "attainment_8_score")
}
cols = [c for c in SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
"rwm_expected_pct", "attainment_8_score",
"total_pupils"]
# dict.fromkeys, not a list: SCHOOL_COLUMNS already ends with latitude and
# longitude, so concatenating them again selected each twice and pandas
# dropped one of every duplicated pair with a "columns are not unique"
# warning. Ordered de-duplication keeps the column order and the warning
# cannot come back.
#
# nursery_provision and parliamentary_constituency are not in
# SCHOOL_COLUMNS and the place table shows both. The `in rows.columns`
# guard is what keeps a mart the pipeline has not rebuilt working: those
# two are the optional GIAS columns data_loader degrades to NULL.
cols = [c for c in dict.fromkeys(
SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
"nursery_provision",
"parliamentary_constituency",
"rwm_expected_pct", "attainment_8_score",
"total_pupils"])
if c in rows.columns]
return {
+46
View File
@@ -137,3 +137,49 @@ def test_an_authority_without_a_page_is_named_but_carries_no_slug(straddling_cli
by_name = {a["name"]: a for a in body["place"]["authorities"]}
assert by_name["Essex"]["slug"] == "essex"
assert by_name["Isles Of Scilly"]["slug"] is None
def _attributed_df() -> pd.DataFrame:
"""The same town, with the four attributes the place table now shows."""
df = _schools_df()
df["age_range"] = "4-11"
df["religious_denomination"] = "Church of England"
df["nursery_provision"] = True
df["parliamentary_constituency"] = "Brentwood and Ongar"
return df
@pytest.fixture()
def attributed_client(monkeypatch):
from backend import app as app_module
monkeypatch.setattr(app_module, "load_school_data", _attributed_df)
monkeypatch.setattr(app_module, "load_latest_school_data", _attributed_df)
monkeypatch.setattr(app_module, "_place_registry", None)
return TestClient(app_module.app, raise_server_exceptions=False)
def test_place_detail_carries_the_attributes_the_table_shows(attributed_client):
"""age_range and religious_denomination ride in on SCHOOL_COLUMNS.
nursery_provision and parliamentary_constituency do not, and the place
table needs all four — a column the response cannot fill is a column of
dashes on ~3,900 pages.
"""
body = attributed_client.get("/api/places/town/brentwood").json()
school = body["schools"][0]
assert school["age_range"] == "4-11"
assert school["religious_denomination"] == "Church of England"
assert school["nursery_provision"] is True
assert school["parliamentary_constituency"] == "Brentwood and Ongar"
def test_place_detail_survives_a_mart_without_the_optional_columns(client):
"""The base fixture has neither column, as an unrebuilt mart does not.
data_loader degrades those to NULL rather than failing the load, so the
endpoint must not assume they are present.
"""
res = client.get("/api/places/town/brentwood")
assert res.status_code == 200
assert "nursery_provision" not in res.json()["schools"][0]
+162 -5
View File
@@ -1978,6 +1978,59 @@ test('a place page links its phase variants, and they resolve', async ({ page })
await expect(page.locator('h1')).toContainText(new RegExp(`${phase} schools in`, 'i'));
});
/*
* The table shipped with one column of scores. A parent shortlisting from a
* town page needs to know whether a school takes their child's age, whether
* it is a faith school, and — for a primary — whether it has a nursery,
* before a percentage means anything.
*
* These assert the column headings rather than the values: nursery_provision
* and parliamentary_constituency are optional mart columns, and on an
* environment whose pipeline has not rebuilt them the API degrades them to
* absent. A value assertion would then fail for a data reason, not a code one.
*/
async function phasedPlace(page: Page, phase: 'primary' | 'secondary') {
const place = await firstPlaceOfKind(page, 'town');
const detail = await (await page.request.get(`/api/places/town/${place.slug}`)).json();
test.skip(!(detail.place.phases ?? []).includes(phase),
`no ${phase} page clears the threshold here`);
return place;
}
test('a primary place page names each school as well as scoring it', async ({ page }) => {
const place = await phasedPlace(page, 'primary');
await page.goto(`/schools/${place.slug}/primary`);
for (const heading of ['Ages', 'Religious character', 'Nursery', 'Constituency']) {
await expect(page.getByRole('columnheader', { name: heading, exact: true }))
.toBeVisible();
}
// age_range rides in on SCHOOL_COLUMNS and predates the optional columns,
// so it is the one attribute safe to assert a value for anywhere.
await expect(page.locator('table tbody td').filter({ hasText: /^\d+–\d+$/ }).first())
.toBeVisible();
});
test('a secondary place page does not ask about nurseries', async ({ page }) => {
const place = await phasedPlace(page, 'secondary');
await page.goto(`/schools/${place.slug}/secondary`);
await expect(page.getByRole('columnheader', { name: 'Ages', exact: true }))
.toBeVisible();
await expect(page.getByRole('columnheader', { name: 'Nursery', exact: true }))
.toHaveCount(0);
});
test('the measure stays beside the school name, not behind a swipe', async ({ page }) => {
// Six columns overflow a phone; .tableWrap turns that into a horizontal
// scroll. With the measure last, the number the page exists for is the one
// off the screen.
const place = await phasedPlace(page, 'primary');
await page.setViewportSize({ width: 390, height: 844 });
await page.goto(`/schools/${place.slug}/primary`);
const second = page.locator('table thead th').nth(1);
await expect(second).toContainText(/reading, writing/i);
await expect(second).toBeInViewport();
});
test('phase variants are submitted in the places sitemap', async ({ page }) => {
const xml = await (await page.request.get('/sitemaps/places-1.xml')).text();
expect(xml).toMatch(/\/schools\/[a-z0-9-]+\/primary</);
@@ -2094,12 +2147,32 @@ test('a place page lists its schools alphabetically', async ({ page }) => {
expect(town).toBeTruthy();
await page.goto(`/schools/${town.slug}`);
const names = await page.locator('a[href^="/school/"]').allTextContents();
expect(names.length).toBeGreaterThan(1);
const sorted = [...names].sort((a, b) =>
a.toLowerCase().localeCompare(b.toLowerCase()));
expect(names).toEqual(sorted);
/*
* Per table, not per page.
*
* An unphased place page renders one table per phase, and an all-through
* school legitimately appears in both — so the page's school links are not
* one alphabetical run and never were. This assertion used to collect them
* all together and only passed because no town it picked happened to hold an
* all-through school; when the data gave Abbots Langley one, Breakspeare
* School showed up in the primary table and again in the secondary, and the
* test failed on correct behaviour.
*/
const tables = page.locator('table');
const tableCount = await tables.count();
expect(tableCount).toBeGreaterThan(0);
let checked = 0;
for (let i = 0; i < tableCount; i++) {
const names = await tables.nth(i).locator('a[href^="/school/"]').allTextContents();
if (names.length < 2) continue; // a one-row table says nothing about order
const sorted = [...names].sort((a, b) =>
a.toLowerCase().localeCompare(b.toLowerCase()));
expect(names, `table ${i + 1} is not alphabetical`).toEqual(sorted);
checked++;
}
expect(checked, 'no table had enough rows to check the ordering').toBeGreaterThan(0);
});
test('the rankings page still orders by score, not name', async ({ page }) => {
@@ -2112,6 +2185,66 @@ test('the rankings page still orders by score, not name', async ({ page }) => {
expect(scores).toEqual([...scores].sort((a: number, b: number) => b - a));
});
/*
* Analytics on the location layer.
*
* Umami counts a pageview for every one of these URLs already. What it cannot
* say is which *kind* of location page earns engagement, because all four
* families share the /schools/ prefix — and that is the question that decides
* whether to keep investing in them.
*/
/** Capture Umami events, with the real script blocked so it cannot clobber
* the stub. Must be called before the first navigation. */
async function captureEvents(page: Page) {
const events: Array<{ name: string; data: Record<string, unknown> }> = [];
await page.route('**/analytics.schoolcompare.co.uk/**', (route) => route.abort());
await page.exposeFunction('__capture',
(name: string, data: Record<string, unknown>) => { events.push({ name, data }); });
await page.addInitScript(() => {
(window as unknown as { umami: unknown }).umami = {
track: (name: string, data: unknown) =>
(window as unknown as { __capture: (n: string, d: unknown) => void })
.__capture(name, data),
};
});
return events;
}
test('a location page reports which kind of place it is', async ({ page }) => {
const events = await captureEvents(page);
const place = await firstPlaceOfKind(page, 'authority');
await page.goto(`/schools/authority/${place.slug}`);
await expect.poll(() => events.find((e) => e.name === 'place_viewed'),
{ timeout: 10_000 }).toBeTruthy();
const event = events.find((e) => e.name === 'place_viewed')!;
expect(event.data.kind).toBe('authority');
expect(event.data.slug).toBe(place.slug);
expect(event.data.phase).toBe('all');
});
test('a school reached from a location page is attributed to it, not to direct', async ({ page }) => {
/*
* The defect this was written for. getNavigationSource had no case for
* /schools/, so every school view that came through the location layer was
* filed as 'direct' — the bucket you read as "typed the URL". The one
* measurement that says whether ~3,900 SEO pages work was reporting the
* wrong answer, confidently.
*/
const events = await captureEvents(page);
const place = await firstPlaceOfKind(page, 'town');
await page.goto(`/schools/${place.slug}`);
await page.locator('a[href^="/school/"]').first().click();
await page.waitForURL(/\/school\//);
await expect.poll(() => events.find((e) => e.name === 'school_viewed'),
{ timeout: 10_000 }).toBeTruthy();
expect(events.find((e) => e.name === 'school_viewed')!.data.from).toBe('place');
});
/*
* School autosuggest (spec 2026-08-26).
*/
@@ -2198,6 +2331,30 @@ test('the whole dropdown is reachable, not clipped by the hero', async ({ page }
+ '— an ancestor is clipping or covering the dropdown').toBeTruthy();
});
test('the dropdown does not survive into the results it produced', async ({ page }) => {
/*
* The bug that took the staging gate down, and it was not a test problem:
* after a search the results-page bar still holds the term, so the dropdown
* reopened on top of the results and swallowed the click on the first one.
* Playwright reported it as "<li role=option> intercepts pointer events"; a
* reader would simply have found their first result unclickable.
*/
test.skip(!(await autosuggestIsOn(page)),
'the school_autosuggest flag is off in this environment');
await page.goto('/');
await page.getByRole('combobox').first().fill('school');
await expect(page.getByRole('option').first()).toBeVisible();
await page.getByRole('button', { name: /Search/i }).first().click();
await page.waitForURL(/search=school/);
await expect(page.getByRole('listbox')).toHaveCount(0);
// And the results underneath are actually reachable, which is the point.
await page.locator('a[href^="/school/"]').first().click({ timeout: 15_000 });
await expect(page).toHaveURL(/\/school\//);
});
test('with autosuggest off, the search box is a plain input', async ({ page }) => {
test.skip(await autosuggestIsOn(page),
'the school_autosuggest flag is on in this environment');
@@ -3,10 +3,11 @@ import userEvent from '@testing-library/user-event';
import { FilterBar } from '@/components/FilterBar';
const push = jest.fn();
let searchParams = new URLSearchParams();
jest.mock('next/navigation', () => ({
useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }),
usePathname: () => '/',
useSearchParams: () => new URLSearchParams(),
useSearchParams: () => searchParams,
}));
const FILTERS = {
@@ -24,6 +25,7 @@ beforeEach(() => {
phase: 'Primary', school_type: 'Community school' }] }),
})) as unknown as typeof fetch;
push.mockClear();
searchParams = new URLSearchParams();
});
afterEach(() => { global.fetch = realFetch; });
@@ -74,3 +76,35 @@ describe('FilterBar autosuggest', () => {
expect.stringContaining('search=brecknock')));
});
});
describe('FilterBar autosuggest does not reopen over results', () => {
it('stays shut when the input arrives pre-filled from the URL', async () => {
/*
* The results-page bar renders with the search term already in the input.
* Opening on that would drop the dropdown on top of the results the search
* just produced — which is exactly what happened: the first result became
* unclickable, because the list sat over it and swallowed the pointer.
*
* Suggestions answer typing, not the presence of a value.
*/
searchParams = new URLSearchParams('search=brecknock');
render(<FilterBar filters={FILTERS} autosuggest />);
expect(screen.getByRole('combobox')).toHaveValue('brecknock');
await new Promise((r) => setTimeout(r, 300)); // past the 200ms debounce
expect(global.fetch).not.toHaveBeenCalled();
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
});
it('closes the dropdown when the search is submitted', async () => {
render(<FilterBar filters={FILTERS} autosuggest />);
const input = screen.getByRole('combobox');
await userEvent.type(input, 'brecknock');
expect(await screen.findByRole('listbox')).toBeInTheDocument();
await userEvent.type(input, '{Enter}');
await waitFor(() =>
expect(screen.queryByRole('listbox')).not.toBeInTheDocument());
});
});
@@ -346,3 +346,134 @@ describe('PlaceView unlinkable authorities', () => {
.toContain('Isles Of Scilly');
});
});
describe('PlaceView school attributes', () => {
/*
* The table shipped with one column of scores, which answers "how did they
* do" and nothing about whether the school is one a family could use. Age
* range, faith, nursery and constituency are the four facts a parent
* filters on before they look at a number at all.
*/
const withAttributes: PlaceDetail = {
place: { kind: 'town', slug: 'chelmsford', name: 'Chelmsford', count: 3,
parent_authority: 'Essex', phases: ['primary', 'secondary'] },
schools: [
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
rwm_expected_pct: 82, attainment_8_score: null,
age_range: '4-11', religious_denomination: 'Church of England',
nursery_provision: true,
parliamentary_constituency: 'Chelmsford' } as never,
{ urn: 2, school_name: 'Beta High', phase: 'Secondary',
rwm_expected_pct: null, attainment_8_score: 47,
age_range: '11-16', religious_denomination: 'Does not apply',
nursery_provision: false,
parliamentary_constituency: 'Witham' } as never,
],
averages: { rwm_expected_pct: 63, attainment_8_score: 45 },
};
function headings(container: HTMLElement, table = 0): string[] {
return Array.from(container.querySelectorAll('table')[table]
.querySelectorAll('thead th')).map((th) => th.textContent ?? '');
}
it('heads a primary table with all four attributes', () => {
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
expect(headings(container)).toEqual([
'School', 'Reading, writing & maths',
'Ages', 'Religious character', 'Nursery', 'Constituency',
]);
});
it('omits nursery from a secondary table, where it does not apply', () => {
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
expect(headings(container, 1)).toEqual([
'School', 'Attainment 8', 'Ages', 'Religious character', 'Constituency',
]);
});
it('keeps the measure beside the school name, where a phone can see it', () => {
// Six columns overflow a phone and .tableWrap turns that into a swipe.
// With the measure last, the one number the page exists for is the one
// scrolled off the screen.
const { container } = render(<PlaceView detail={withAttributes}
phase="primary" englandAverage={61} neighbours={[]} />);
expect(headings(container)[1]).toBe('Reading, writing & maths');
});
it('shows the age range without repeating the column heading', () => {
render(<PlaceView detail={withAttributes} englandAverage={61}
neighbours={[]} />);
expect(screen.getByText('4–11')).toBeInTheDocument();
expect(screen.queryByText('Ages 4–11')).not.toBeInTheDocument();
});
it('names the faith of a faith school', () => {
render(<PlaceView detail={withAttributes} englandAverage={61}
neighbours={[]} />);
expect(screen.getByText('Church of England')).toBeInTheDocument();
});
it('reads "Does not apply" as no religious character, not as a value', () => {
// GIAS spells the absence of a faith as "Does not apply", which is a
// database answer rather than an English one. The school page already
// suppresses it; the two must not disagree about the same school.
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
const secondary = container.querySelectorAll('table')[1]
.querySelectorAll('tbody td');
expect(secondary[3].textContent).toBe('—');
expect(screen.queryByText(/Does not apply/)).not.toBeInTheDocument();
});
it('marks a nursery as such and a school without one as not', () => {
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
const cells = container.querySelectorAll('table')[0]
.querySelectorAll('tbody td');
expect(cells[4].textContent).toBe('Yes');
});
it('names the constituency of each school', () => {
render(<PlaceView detail={withAttributes} englandAverage={61}
neighbours={[]} />);
expect(screen.getByText('Chelmsford', { selector: 'td' })).toBeInTheDocument();
expect(screen.getByText('Witham', { selector: 'td' })).toBeInTheDocument();
});
it('dashes an attribute the data does not carry', () => {
// nursery_provision and parliamentary_constituency are absent from marts
// the pipeline has not rebuilt, and the API degrades them to null rather
// than failing. A row must survive that.
const bare: PlaceDetail = {
...withAttributes,
schools: [{ urn: 3, school_name: 'Gamma Primary', phase: 'Primary',
rwm_expected_pct: 70 } as never],
};
const { container } = render(<PlaceView detail={bare} phase="primary"
englandAverage={61} neighbours={[]} />);
const cells = Array.from(container.querySelectorAll('tbody td'))
.map((td) => td.textContent);
expect(cells.slice(2)).toEqual(['—', '—', '—', '—']);
});
it('gives an all-through school its nursery under primary only', () => {
// All-through schools render in both groups. Nursery belongs to the
// primary reading of the same school, not the secondary one.
const allThrough: PlaceDetail = {
...withAttributes,
schools: [{ urn: 4, school_name: 'Delta Academy', phase: 'All-through',
rwm_expected_pct: 66, attainment_8_score: 51,
age_range: '4-18', religious_denomination: 'None',
nursery_provision: true,
parliamentary_constituency: 'Chelmsford' } as never],
};
const { container } = render(<PlaceView detail={allThrough}
englandAverage={61} neighbours={[]} />);
const tables = container.querySelectorAll('table');
expect(tables[0].textContent).toContain('Yes');
expect(tables[1].textContent).not.toContain('Yes');
});
});
@@ -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();
});
});
@@ -0,0 +1,45 @@
import { render } from '@testing-library/react';
import { TrackPlaceView } from '@/components/places/TrackPlaceView';
const trackMock = jest.fn();
jest.mock('@/lib/analytics', () => ({
track: (...args: unknown[]) => trackMock(...args),
getNavigationSource: () => 'search',
}));
describe('TrackPlaceView', () => {
beforeEach(() => trackMock.mockClear());
it('reports which kind of location page was viewed', () => {
/*
* `kind` is the reason this event exists. Whether to keep investing in the
* location layer turns on which *sort* of page earns engagement — towns,
* authorities or postcode districts — and a bare pageview cannot say,
* because all four families share the /schools/ prefix.
*/
render(<TrackPlaceView kind="authority" slug="kent" count={412} />);
expect(trackMock).toHaveBeenCalledWith('place_viewed', {
kind: 'authority', slug: 'kent', phase: 'all',
school_count: 412, from: 'search',
});
});
it('names the phase when the page is a phase variant', () => {
render(<TrackPlaceView kind="town" slug="brentwood" count={29} phase="primary" />);
expect(trackMock).toHaveBeenCalledWith('place_viewed',
expect.objectContaining({ phase: 'primary' }));
});
it('fires once, not once per render', () => {
const { rerender } = render(
<TrackPlaceView kind="town" slug="brentwood" count={29} />);
rerender(<TrackPlaceView kind="town" slug="brentwood" count={29} />);
expect(trackMock).toHaveBeenCalledTimes(1);
});
it('renders nothing', () => {
const { container } = render(
<TrackPlaceView kind="town" slug="brentwood" count={29} />);
expect(container).toBeEmptyDOMElement();
});
});
@@ -0,0 +1,164 @@
import fs from 'fs';
import path from 'path';
/**
* Guards against light-theme-only CSS.
*
* The site themes entirely through tokens redefined under
* `@media (prefers-color-scheme: dark)`. A hardcoded colour therefore does not
* fail loudly — it renders perfectly in the theme it was written for and
* quietly wrongly in the other, which nobody sees unless they happen to be in
* dark mode when they look.
*
* Both rules below are drawn from real defects in SchoolHeroMap.module.css,
* found by eye rather than by any test:
*
* - the map's fade to the header ramped through hardcoded white and landed on
* `var(--bg-card)`. Invisible in light; a bright band across the full width
* of a near-black card in dark.
* - the controls floating over the map paired a hardcoded white background
* with `color: var(--text-primary)`, which resolves to #E9EEF0 in dark —
* near-white text on a near-white button.
*/
const COMPONENTS = path.join(__dirname, '..', '..', 'components');
function stylesheets(dir: string): string[] {
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) return stylesheets(full);
return entry.name.endsWith('.module.css') ? [full] : [];
});
}
/** Innermost `selector { body }` pairs. Nested at-rules never match as rules,
* 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 }> {
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],
}));
}
const HARDCODED_WHITE_BG = /background[^;]*(?:255,\s*255,\s*255|#fff\b|#ffffff\b)/i;
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);
});
it('never pairs a hardcoded white background with a themed text colour', () => {
const offenders = files.flatMap((file) =>
rules(fs.readFileSync(file, 'utf8'))
.filter((r) => HARDCODED_WHITE_BG.test(r.body) && THEMED_COLOR.test(r.body))
.map((r) => `${path.relative(COMPONENTS, file)} ${r.selector}`));
// Either the surface follows the theme and so should the text, or it does
// not and the text must be literal too. Mixing them is how near-white text
// ends up on a near-white button.
expect(offenders).toEqual([]);
});
it('never fades to a themed colour through a hardcoded one', () => {
const offenders = files.flatMap((file) =>
rules(fs.readFileSync(file, 'utf8'))
.filter((r) => /linear-gradient/.test(r.body)
&& /var\(--bg-(card|primary|secondary)\)/.test(r.body)
&& /255,\s*255,\s*255|#fff\b/i.test(r.body))
.map((r) => `${path.relative(COMPONENTS, file)} ${r.selector}`));
// A gradient that lands on a token has to be made of that token, or the
// ramp and its destination disagree in one theme. Use the matching
// `--*-rgb` token for the transparent stops.
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([]);
});
});
+158
View File
@@ -0,0 +1,158 @@
import { getNavigationSource } from '@/lib/analytics';
/** jsdom's document.referrer is read-only; redefining it is the way in. */
function referrer(url: string) {
Object.defineProperty(document, 'referrer', { value: url, configurable: true });
}
const ORIGIN = 'http://localhost';
describe('getNavigationSource', () => {
afterEach(() => referrer(''));
it('attributes a visit from a location page to the place layer', () => {
/*
* The one this was added for.
*
* W2 published ~3,900 location pages whose entire purpose is to funnel
* search traffic onto school pages. Before this case existed they fell
* through to 'direct' — so the location layer's contribution was not
* merely missing from the funnel, it was being counted in the bucket you
* read as "typed the URL". The measurement that decides whether W2 worked
* was confidently reporting the wrong answer.
*/
referrer(`${ORIGIN}/schools/barnet`);
expect(getNavigationSource()).toBe('place');
});
it.each([
['/schools/authority/kent', 'authority'],
['/schools/near/sw11', 'outcode'],
['/schools/brentwood/primary', 'phase variant'],
])('covers %s (%s)', (path) => {
referrer(`${ORIGIN}${path}`);
expect(getNavigationSource()).toBe('place');
});
it('still calls a school page "detail", one character away', () => {
// /school/ and /schools/ differ by one letter and mean different things.
// A prefix test written in the wrong order silently merges them.
referrer(`${ORIGIN}/school/100010-brecknock-primary-school`);
expect(getNavigationSource()).toBe('detail');
});
it.each([
['/', 'search'],
['/rankings', 'rankings'],
['/compare?urns=1,2', 'compare'],
])('leaves %s attributed as %s', (path, expected) => {
referrer(`${ORIGIN}${path}`);
expect(getNavigationSource()).toBe(expected);
});
it('treats an external referrer as direct', () => {
// Umami records the real referrer on the pageview; this field is only
// about internal navigation.
referrer('https://www.google.com/search?q=schools+in+barnet');
expect(getNavigationSource()).toBe('direct');
});
it('treats no referrer as direct', () => {
referrer('');
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');
});
});
+26
View File
@@ -13,6 +13,8 @@ import {
metricKind,
shortName,
computeYBounds,
formatAgeRange,
formatAgeSpan,
} from '@/lib/utils';
describe('formatPercentage', () => {
@@ -320,3 +322,27 @@ describe('shortName', () => {
expect(shortName('A'.repeat(30), 10)).toBe('AAAAAAAAA…');
});
});
describe('formatAgeSpan', () => {
it('normalises a hyphenated range to an en dash, without a label', () => {
// The place table carries "Ages" in the column heading, so repeating it
// in every cell is noise. formatAgeRange keeps the label for the contexts
// that have no heading to hang it on.
expect(formatAgeSpan('4-11')).toBe('4–11');
});
it('leaves a range it does not recognise alone rather than mangling it', () => {
expect(formatAgeSpan('3-19 (SEN)')).toBe('3-19 (SEN)');
});
it('returns an empty string for a missing range', () => {
expect(formatAgeSpan(null)).toBe('');
expect(formatAgeSpan(undefined)).toBe('');
});
});
describe('formatAgeRange', () => {
it('keeps its label, so the two helpers stay distinguishable', () => {
expect(formatAgeRange('4-11')).toBe('Ages 4–11');
});
});
+33
View File
@@ -28,6 +28,9 @@
--bg-primary: #FAFAF8; /* Warm White */
--bg-secondary: #F5EFE6; /* Sand — hero panels, sunken rows */
--bg-card: #FFFFFF;
/* For gradients that have to fade to the card colour. A hardcoded white
ramp reads as a bright band against a dark card. */
--bg-card-rgb: 255, 255, 255;
--surface-inverse: #0F766E;
/* ── Ink ────────────────────────────────────────────────────────── */
@@ -234,6 +237,7 @@
--bg-primary: #111A20;
--bg-secondary: #16222A;
--bg-card: #18242C;
--bg-card-rgb: 24, 36, 44;
--surface-inverse: #E9EEF0;
--text-primary: #E9EEF0;
@@ -584,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;
+5
View File
@@ -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 />
+19 -2
View File
@@ -69,14 +69,28 @@ export function FilterBar({
const [omniValue, setOmniValue] = useState(initialOmniValue);
const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`;
/*
* Suggestions answer typing, not the mere presence of a value.
*
* Without this the results-page bar reopened the dropdown over the results:
* after a search the input still holds the term, so on every render the
* query was >= 2 characters and the list opened again — on top of the very
* results the search had just produced, swallowing the click on the first
* one. The E2E gate caught it as "<li role=option> intercepts pointer
* events", but a reader would just have found the page unclickable.
*/
const [hasTyped, setHasTyped] = useState(false);
// Suppressed once the value parses as a postcode: the box takes a school
// name OR a postcode, and suggesting schools during postcode entry fights
// the user rather than helping them.
const suggestEnabled = autosuggest && !isValidPostcode(omniValue);
const suggestEnabled = autosuggest && hasTyped && !isValidPostcode(omniValue);
const { suggestions, open, activeIndex, setActiveIndex, close } =
useSchoolSuggest(omniValue, suggestEnabled);
const pickSuggestion = (s: Suggestion) => {
setHasTyped(false);
close();
track('search_submitted', {
query: s.school_name.toLowerCase(),
@@ -169,6 +183,9 @@ export function FilterBar({
const handleSearchSubmit = (e: React.FormEvent) => {
e.preventDefault();
// The search has been made; the suggestions that led to it are spent.
setHasTyped(false);
close();
if (!omniValue.trim()) {
updateURL({ search: "", postcode: "", radius: "" });
return;
@@ -271,7 +288,7 @@ export function FilterBar({
ref={inputRef}
type="search"
value={omniValue}
onChange={(e) => setOmniValue(e.target.value)}
onChange={(e) => { setOmniValue(e.target.value); setHasTyped(true); }}
onKeyDown={handleOmniKeyDown}
onBlur={close}
placeholder="School name or postcode"
+1 -1
View File
@@ -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);
+27
View File
@@ -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;
}
+31 -7
View File
@@ -47,7 +47,13 @@
width: 100%;
height: 100%;
background:
linear-gradient(100deg, rgba(255, 255, 255, 0) 40%, rgba(255, 255, 255, .5) 50%, rgba(255, 255, 255, 0) 60%) var(--bg-secondary);
/* Sweeps toward the card colour, which is a shade lighter than this
ground in both themes. Hardcoded white was a bright flash across a
dark page every 1.4s while the tiles loaded. */
linear-gradient(100deg,
rgba(var(--bg-card-rgb), 0) 40%,
rgba(var(--bg-card-rgb), .5) 50%,
rgba(var(--bg-card-rgb), 0) 60%) var(--bg-secondary);
background-size: 200% 100%;
animation: shimmer 1.4s infinite;
}
@@ -76,6 +82,15 @@
justify-content: center;
}
/*
* Controls that float ON the map.
*
* The map tiles are light in both themes, so these deliberately do NOT follow
* the theme — they follow the map. The literal ink below is the point: paired
* with a hardcoded white background, `color: var(--text-primary)` resolved to
* #E9EEF0 in the dark theme and put near-white text on a near-white button.
* A themed token is the wrong tool for a surface that never changes.
*/
.openHint {
display: inline-flex;
align-items: center;
@@ -85,7 +100,8 @@
border-radius: 999px;
font-size: 13px;
font-weight: 600;
color: var(--text-primary);
/* See "Controls that float ON the map" above. */
color: #1C2731;
background: rgba(255, 255, 255, .85);
-webkit-backdrop-filter: blur(6px);
backdrop-filter: blur(6px);
@@ -113,11 +129,18 @@
on top of the blend. */
z-index: 450;
pointer-events: none;
/* The card colour, not white.
This ramp was hardcoded white and ended at var(--bg-card). In the light
theme that is white into white and invisible, as intended. In the dark
theme it climbed to 95% WHITE and then met a near-black card — a bright
band across the full width, right where the map is supposed to dissolve
into the header. Fading to the same colour the gradient lands on is the
whole trick, and it only works if that colour is a token. */
background: linear-gradient(to bottom,
rgba(255, 255, 255, 0) 0%,
rgba(255, 255, 255, .35) 35%,
rgba(255, 255, 255, .75) 62%,
rgba(255, 255, 255, .95) 82%,
rgba(var(--bg-card-rgb), 0) 0%,
rgba(var(--bg-card-rgb), .35) 35%,
rgba(var(--bg-card-rgb), .75) 62%,
rgba(var(--bg-card-rgb), .95) 82%,
var(--bg-card) 100%);
}
@@ -134,7 +157,8 @@
border: none;
border-radius: 8px;
background: rgba(255, 255, 255, .92);
color: var(--text-primary);
/* See "Controls that float ON the map" above. */
color: #1C2731;
cursor: pointer;
box-shadow: 0 2px 10px rgba(var(--shadow-rgb), .2);
}
@@ -164,6 +164,34 @@
white-space: nowrap;
}
/*
* Attribute columns. Muted, because they qualify the row rather than compete
* with the measure for it, and hugging their content so the school name keeps
* the spare width — the same width:1% trick as .num, which is what stops six
* columns from splitting evenly and squeezing the names into two lines each.
*
* .attr never wraps: "4–11" and "Yes" broken across lines read as two values.
* .attrWide may — "Church of England" and some constituency names are long
* enough that forcing one line would push the measure off a phone screen.
*/
.table th.attr,
.table td.attr,
.table th.attrWide,
.table td.attrWide {
color: var(--text-secondary);
width: 1%;
}
.table th.attr,
.table td.attr {
white-space: nowrap;
}
.table th.attrWide,
.table td.attrWide {
min-width: 8rem;
}
/* The measure is spelled out; the tooltip carries the definition. */
.metricHead {
text-decoration: none;
+50 -1
View File
@@ -13,8 +13,9 @@ import Link from 'next/link';
import type { PlaceDetail, PlaceSummary } from '@/lib/places';
import { placeUrl, authoritySlug } from '@/lib/places';
import type { School } from '@/lib/types';
import { schoolUrl } from '@/lib/utils';
import { schoolUrl, formatAgeSpan } from '@/lib/utils';
import { absoluteUrl } from '@/lib/site';
import { TrackPlaceView } from './TrackPlaceView';
import styles from './PlaceView.module.css';
interface Props {
@@ -63,8 +64,31 @@ function isPhase(school: School, phase: PhaseKey): boolean {
: p.includes('primary') || p.includes('middle');
}
/*
* GIAS spells the absence of a faith as "Does not apply", and sometimes
* "None" or "Not applicable" — database answers, not English ones. The school
* page and the comparison already suppress all three; this is the same rule,
* so the two surfaces cannot disagree about the same school.
*/
const NO_FAITH = /^(none|does not apply|not applicable)$/i;
/** An attribute the data does not carry. Distinct from the measure's "Not
* published": four of those per row would drown the row it qualifies. */
const NO_VALUE = '—';
function faithOf(school: School): string {
const denom = school.religious_denomination ?? '';
return denom && !NO_FAITH.test(denom) ? denom : NO_VALUE;
}
function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey }) {
const metric = METRICS[phase];
/*
* Nursery is a primary question. An all-through school renders in both
* groups, and its nursery belongs to the primary reading of it — under
* "Secondary schools" the column would be a fact about a different intake.
*/
const showNursery = phase === 'primary';
return (
<div className={styles.tableWrap}>
<table className={styles.table}>
@@ -78,6 +102,13 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
{metric.heading}
</abbr>
</th>
{/* The measure sits second, not last. Six columns overflow a
phone and .tableWrap turns that into a swipe; last would put
the one number the page exists for off the screen. */}
<th scope="col" className={styles.attr}>Ages</th>
<th scope="col" className={styles.attrWide}>Religious character</th>
{showNursery && <th scope="col" className={styles.attr}>Nursery</th>}
<th scope="col" className={styles.attrWide}>Constituency</th>
</tr>
</thead>
<tbody>
@@ -95,6 +126,19 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
? <span className={styles.noData}>Not published</span>
: `${Math.round(Number(value))}${metric.unit}`}
</td>
<td className={styles.attr}>{formatAgeSpan(s.age_range) || NO_VALUE}</td>
<td className={styles.attrWide}>{faithOf(s)}</td>
{showNursery && (
<td className={styles.attr}>
{/* Undefined is a mart the pipeline has not rebuilt, and
false is a school without one. Neither is a "Yes", and
neither is worth two different words. */}
{s.nursery_provision ? 'Yes' : NO_VALUE}
</td>
)}
<td className={styles.attrWide}>
{s.parliamentary_constituency || NO_VALUE}
</td>
</tr>
);
})}
@@ -165,6 +209,11 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
return (
<div className={styles.container}>
{/* One line, and all four place families are measured, because they all
render through this component. */}
<TrackPlaceView kind={place.kind} slug={place.slug}
count={place.count} phase={phase} />
<script
type="application/ld+json"
dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLd) }}
@@ -0,0 +1,47 @@
'use client';
/**
* Fires `place_viewed` once per location page.
*
* A separate client component because PlaceView is a server component and
* cannot call into the browser. It renders nothing — its whole job is the
* effect, which keeps the page itself server-rendered.
*
* Umami already counts a pageview for every one of these URLs, so this is not
* about traffic. It is about `kind`: whether to keep investing in the location
* layer turns on which *sort* of page earns engagement — towns, authorities,
* London localities or postcode districts — and a pageview cannot say, because
* all four families share the /schools/ prefix and only the registry knows
* which is which.
*/
import { useEffect } from 'react';
import { track, getNavigationSource } from '@/lib/analytics';
interface Props {
kind: string;
slug: string;
count: number;
phase?: 'primary' | 'secondary';
}
export function TrackPlaceView({ kind, slug, count, phase }: Props) {
useEffect(() => {
track('place_viewed', {
kind,
slug,
// "all" rather than omitting it, so the unphased page is a value in the
// same field rather than a gap that has to be interpreted.
phase: phase ?? 'all',
school_count: count,
// Internal navigation only. An arrival from Google reads as 'direct'
// here; Umami's own pageview referrer is where external attribution
// lives, and these pages exist to be arrived at externally.
from: getNavigationSource(),
});
// Once per place, not once per render.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [kind, slug, phase]);
return null;
}
+71 -7
View File
@@ -19,6 +19,7 @@ export type EventName =
| 'empty_results'
// Engagement
| 'school_viewed'
| 'place_viewed'
| 'section_nav_used'
| 'chart_metric_changed'
| 'metric_compared_in_rankings'
@@ -56,17 +57,80 @@ export function track(name: EventName, data?: Payload): void {
* Categorise where the user navigated from, for funnel attribution
* (mostly used on school_viewed). Only checks same-origin referrers.
*/
export function getNavigationSource(): 'search' | 'rankings' | 'compare' | 'detail' | 'direct' {
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';
if (p.startsWith('/school/')) return 'detail';
return 'direct';
return classifyPath(ref.pathname);
} catch {
return 'direct';
}
+12 -2
View File
@@ -82,11 +82,21 @@ export function shortName(name: string, maxLength = 32): string {
* Display-only — leaves the raw `age_range` field (used for sixth-form
* detection) untouched. Falls back to the raw value if it's not a plain range.
*/
export function formatAgeRange(ageRange: string | null | undefined): string {
export function formatAgeSpan(ageRange: string | null | undefined): string {
if (!ageRange) return '';
const match = ageRange.match(/^\s*(\d+)\s*[-–]\s*(\d+)\s*$/);
if (!match) return ageRange;
return `Ages ${match[1]}–${match[2]}`;
return `${match[1]}–${match[2]}`;
}
/**
* The same span, labelled — for the places that show it with no column
* heading to carry the word "Ages". Delegates so the en-dash normalisation
* lives in one place.
*/
export function formatAgeRange(ageRange: string | null | undefined): string {
const span = formatAgeSpan(ageRange);
return /^\d+–\d+$/.test(span) ? `Ages ${span}` : span;
}
// ============================================================================