Compare commits
13
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e820e7fecd | ||
|
|
4fdeb70a93 | ||
|
|
9a1f56c431 | ||
|
|
ade9dbb3ba | ||
|
|
d1a8596208 | ||
|
|
a3c09d9b67 | ||
|
|
a7f4c86464 | ||
|
|
0804566736 | ||
|
|
55363cbd18 | ||
|
|
868eb344f5 | ||
|
|
0b15497c09 | ||
|
|
d55f6cce23 | ||
|
|
d5a6db289d |
No files matched your search
+16
-3
@@ -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 {
|
||||
|
||||
@@ -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]
|
||||
+197
-5
@@ -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).
|
||||
*/
|
||||
@@ -2163,6 +2296,65 @@ test('typing a school name suggests it, and choosing it opens that school', asyn
|
||||
await expect(page).toHaveURL(/\/school\/\d+/);
|
||||
});
|
||||
|
||||
test('the whole dropdown is reachable, not clipped by the hero', async ({ page }) => {
|
||||
/*
|
||||
* The hero panel had overflow: hidden to clip its artwork to the rounded
|
||||
* corners, and it clipped the dropdown too — 320px of list against 145px of
|
||||
* panel below the input, so roughly half was cut off with nothing to say so.
|
||||
*
|
||||
* toBeVisible() does not catch this: it checks the box is non-empty and not
|
||||
* visibility:hidden, and an ancestor's overflow clips neither. The invariant
|
||||
* that does catch it is that the LAST option is the thing actually painted
|
||||
* at its own coordinates — which fails for clipping and for occlusion alike.
|
||||
*/
|
||||
test.skip(!(await autosuggestIsOn(page)),
|
||||
'the school_autosuggest flag is off in this environment');
|
||||
|
||||
const { schools } = await (await page.request.get('/api/schools?page_size=1')).json();
|
||||
test.skip(!schools?.length, 'no schools in this environment');
|
||||
|
||||
await page.goto('/');
|
||||
await page.getByRole('combobox').first().fill(
|
||||
(schools[0].school_name as string).slice(0, 6));
|
||||
|
||||
const options = page.getByRole('option');
|
||||
await expect(options.first()).toBeVisible();
|
||||
const count = await options.count();
|
||||
|
||||
const painted = await options.nth(count - 1).evaluate((el) => {
|
||||
const r = el.getBoundingClientRect();
|
||||
const hit = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);
|
||||
return { inside: el.contains(hit) || el === hit, bottom: Math.round(r.bottom) };
|
||||
});
|
||||
expect(painted.inside,
|
||||
`the last option is not painted at its own coordinates (bottom ${painted.bottom}) `
|
||||
+ '— 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,108 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
|
||||
/**
|
||||
* The hero search and the results filter bar are the same component in two
|
||||
* costumes. `.filterBar` is the card — background, border, shadow, padding —
|
||||
* and `.heroMode` strips all of it so the search sits directly on the hero
|
||||
* panel.
|
||||
*
|
||||
* Both selectors have specificity (0,1,0), so **source order decides**, and
|
||||
* `.heroMode` only wins because it is declared immediately after. Any later
|
||||
* bare `.filterBar` rule — which in practice means one inside a media query —
|
||||
* silently wins instead, and the hero grows a card's padding back.
|
||||
*
|
||||
* That is exactly what happened: `@media (max-width: 768px) { .filterBar {
|
||||
* padding: 0.875rem } }` re-added 14px in hero mode, indenting the search box,
|
||||
* the hint and the location link 14px past the headline above them and costing
|
||||
* the search field 28px of width on a 390px screen. The two rules directly
|
||||
* below it in the same block were correctly written as
|
||||
* `.filterBar:not(.heroMode)`; this one was missed, and nothing caught it
|
||||
* because the result is a plausible-looking layout rather than a broken one.
|
||||
*/
|
||||
|
||||
const CSS = path.join(__dirname, '..', '..', 'components', 'FilterBar.module.css');
|
||||
|
||||
/** Properties `.heroMode` resets. A later bare `.filterBar` rule setting any
|
||||
* of these puts the card back on the hero. */
|
||||
const RESET_BY_HERO_MODE = [
|
||||
'background', 'border', 'border-radius', 'box-shadow', 'padding',
|
||||
];
|
||||
|
||||
/**
|
||||
* Comments are stripped before anything is parsed.
|
||||
*
|
||||
* A `{` or `}` inside a comment would otherwise desynchronise the brace walk
|
||||
* below and the rule regex alike, and the selector text captured for each rule
|
||||
* would carry the preceding comment along with it.
|
||||
*/
|
||||
function withoutComments(css: string): string {
|
||||
return css.replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
}
|
||||
|
||||
/**
|
||||
* The individual selectors in a rule's prelude.
|
||||
*
|
||||
* Split on commas, because a selector list is a list: `.filterBar, .other { }`
|
||||
* applies to `.filterBar` just as surely as `.filterBar { }` does, and an
|
||||
* earlier version of this guard compared the whole prelude against the literal
|
||||
* string '.filterBar' — so writing the regression as a comma list, or across
|
||||
* two lines, would have walked straight past it.
|
||||
*/
|
||||
function selectorsOf(prelude: string): string[] {
|
||||
return prelude.split(',').map((sel) => sel.trim().replace(/\s+/g, ' '))
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
function mediaQueryBodies(css: string): string[] {
|
||||
const bodies: string[] = [];
|
||||
const re = /@media[^{]*\{/g;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = re.exec(css)) !== null) {
|
||||
// Walk braces from the opening one to find this at-rule's whole body.
|
||||
let depth = 1;
|
||||
let i = m.index + m[0].length;
|
||||
const start = i;
|
||||
while (i < css.length && depth > 0) {
|
||||
if (css[i] === '{') depth++;
|
||||
else if (css[i] === '}') depth--;
|
||||
i++;
|
||||
}
|
||||
bodies.push(css.slice(start, i - 1));
|
||||
}
|
||||
return bodies;
|
||||
}
|
||||
|
||||
describe('FilterBar hero-mode scoping', () => {
|
||||
const css = withoutComments(fs.readFileSync(CSS, 'utf8'));
|
||||
|
||||
it('confirms heroMode still resets the card, which is what makes this matter', () => {
|
||||
const hero = css.match(/\.heroMode\s*\{([^}]*)\}/);
|
||||
expect(hero).not.toBeNull();
|
||||
expect(hero![1]).toMatch(/padding:\s*0/);
|
||||
});
|
||||
|
||||
it('never re-applies card styling to the hero from inside a media query', () => {
|
||||
const offenders: string[] = [];
|
||||
|
||||
for (const body of mediaQueryBodies(css)) {
|
||||
for (const rule of body.matchAll(/([^{}]+)\{([^{}]*)\}/g)) {
|
||||
// Only a *bare* .filterBar is dangerous, and it is dangerous wherever
|
||||
// it appears in a selector list. Scoped variants
|
||||
// (`.filterBar:not(.heroMode)`) and descendants are fine.
|
||||
const selectors = selectorsOf(rule[1]);
|
||||
if (!selectors.includes('.filterBar')) continue;
|
||||
|
||||
for (const prop of RESET_BY_HERO_MODE) {
|
||||
if (new RegExp(`(^|[;\\s])${prop}\\s*:`).test(rule[2])) {
|
||||
offenders.push(`${rule[1].trim()} sets ${prop}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Fix by scoping the rule as `.filterBar:not(.heroMode)`, the way the
|
||||
// neighbouring rules in the same block already are.
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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 />
|
||||
|
||||
@@ -413,7 +413,17 @@
|
||||
/* ── Narrow ───────────────────────────────────────────────────────── */
|
||||
|
||||
@media (max-width: 768px) {
|
||||
.filterBar {
|
||||
/*
|
||||
* Scoped, like the two rules below it.
|
||||
*
|
||||
* The results filter bar is a card — background, border, shadow — and needs
|
||||
* inner padding. The hero's search is not a card: .heroMode zeroes the
|
||||
* padding, border and background so the search sits directly on the panel.
|
||||
* Unscoped, this rule put 14px back, which indented the search box, the hint
|
||||
* and the location link 14px past the headline they sit under, and cost the
|
||||
* search field 28px of width on a 390px screen.
|
||||
*/
|
||||
.filterBar:not(.heroMode) {
|
||||
padding: 0.875rem;
|
||||
}
|
||||
|
||||
@@ -457,6 +467,14 @@
|
||||
align-items: flex-start;
|
||||
}
|
||||
|
||||
/* Optical alignment: the button's own 6px of padding is what makes its
|
||||
label start further right than the hint above it, even once both boxes
|
||||
share a left edge. Pulling the padding back off lines the text up while
|
||||
keeping the tap target. */
|
||||
.heroMode .nearMeBtn {
|
||||
margin-left: -0.375rem;
|
||||
}
|
||||
|
||||
.geoError {
|
||||
text-align: left;
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -90,7 +90,18 @@
|
||||
isolation: isolate;
|
||||
background: var(--hero-ground);
|
||||
border-radius: var(--radius-xl);
|
||||
overflow: hidden;
|
||||
/*
|
||||
* Deliberately NOT overflow: hidden.
|
||||
*
|
||||
* It used to be, to clip the artwork and the scrim to the rounded corners —
|
||||
* and it also clipped the search box's suggestion dropdown, which is 320px
|
||||
* tall against 145px of panel below the input. Roughly half the list was cut
|
||||
* off with no indication anything was missing.
|
||||
*
|
||||
* The two things that actually needed clipping round themselves instead, so
|
||||
* the panel can let a dropdown out. Anything absolutely positioned inside
|
||||
* this panel and taller than the space below it depends on this.
|
||||
*/
|
||||
}
|
||||
|
||||
.heroContent {
|
||||
@@ -107,6 +118,10 @@
|
||||
position: absolute;
|
||||
inset: 0;
|
||||
z-index: 0;
|
||||
/* Rounds itself, because the panel no longer clips it. inset: 0 makes this
|
||||
exactly the panel's own corners. */
|
||||
border-radius: inherit;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.heroArt picture,
|
||||
@@ -143,6 +158,9 @@
|
||||
inset: 0;
|
||||
z-index: 1;
|
||||
pointer-events: none;
|
||||
/* Same reason as .heroArt: the panel stopped clipping, so the scrim keeps
|
||||
its own corners rather than squaring off over the panel's. */
|
||||
border-radius: inherit;
|
||||
background: linear-gradient(
|
||||
to right,
|
||||
var(--hero-ground) 0%,
|
||||
@@ -331,6 +349,10 @@
|
||||
position: static;
|
||||
order: -1;
|
||||
height: 13rem;
|
||||
/* Top corners only. Here the artwork is a band flush with the top of the
|
||||
panel, not a layer covering it — inheriting all four would leave it
|
||||
floating with rounded bottom corners against the copy below. */
|
||||
border-radius: var(--radius-xl) var(--radius-xl) 0 0;
|
||||
}
|
||||
/* The band crop puts the schoolhouse at 73% across — reported by
|
||||
scripts/build-hero-images.js, which derives it from the crop box rather
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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
@@ -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;
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
|
||||
Reference in new issue
Block a user