Compare commits
9
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d1a8596208 | ||
|
|
a3c09d9b67 | ||
|
|
a7f4c86464 | ||
|
|
0804566736 | ||
|
|
55363cbd18 | ||
|
|
868eb344f5 | ||
|
|
0b15497c09 | ||
|
|
d55f6cce23 | ||
|
|
3236efa846 |
No files matched your search
+109
-5
@@ -2094,12 +2094,32 @@ test('a place page lists its schools alphabetically', async ({ page }) => {
|
|||||||
expect(town).toBeTruthy();
|
expect(town).toBeTruthy();
|
||||||
|
|
||||||
await page.goto(`/schools/${town.slug}`);
|
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()));
|
* Per table, not per page.
|
||||||
expect(names).toEqual(sorted);
|
*
|
||||||
|
* 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 }) => {
|
test('the rankings page still orders by score, not name', async ({ page }) => {
|
||||||
@@ -2112,6 +2132,66 @@ test('the rankings page still orders by score, not name', async ({ page }) => {
|
|||||||
expect(scores).toEqual([...scores].sort((a: number, b: number) => b - a));
|
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).
|
* School autosuggest (spec 2026-08-26).
|
||||||
*/
|
*/
|
||||||
@@ -2198,6 +2278,30 @@ test('the whole dropdown is reachable, not clipped by the hero', async ({ page }
|
|||||||
+ '— an ancestor is clipping or covering the dropdown').toBeTruthy();
|
+ '— 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('with autosuggest off, the search box is a plain input', async ({ page }) => {
|
||||||
test.skip(await autosuggestIsOn(page),
|
test.skip(await autosuggestIsOn(page),
|
||||||
'the school_autosuggest flag is on in this environment');
|
'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';
|
import { FilterBar } from '@/components/FilterBar';
|
||||||
|
|
||||||
const push = jest.fn();
|
const push = jest.fn();
|
||||||
|
let searchParams = new URLSearchParams();
|
||||||
jest.mock('next/navigation', () => ({
|
jest.mock('next/navigation', () => ({
|
||||||
useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }),
|
useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }),
|
||||||
usePathname: () => '/',
|
usePathname: () => '/',
|
||||||
useSearchParams: () => new URLSearchParams(),
|
useSearchParams: () => searchParams,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const FILTERS = {
|
const FILTERS = {
|
||||||
@@ -24,6 +25,7 @@ beforeEach(() => {
|
|||||||
phase: 'Primary', school_type: 'Community school' }] }),
|
phase: 'Primary', school_type: 'Community school' }] }),
|
||||||
})) as unknown as typeof fetch;
|
})) as unknown as typeof fetch;
|
||||||
push.mockClear();
|
push.mockClear();
|
||||||
|
searchParams = new URLSearchParams();
|
||||||
});
|
});
|
||||||
afterEach(() => { global.fetch = realFetch; });
|
afterEach(() => { global.fetch = realFetch; });
|
||||||
|
|
||||||
@@ -74,3 +76,35 @@ describe('FilterBar autosuggest', () => {
|
|||||||
expect.stringContaining('search=brecknock')));
|
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());
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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,78 @@
|
|||||||
|
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. */
|
||||||
|
function rules(css: string): Array<{ selector: string; body: string }> {
|
||||||
|
return Array.from(css.matchAll(/([^{}]+)\{([^{}]*)\}/g), (m) => ({
|
||||||
|
selector: m[1].trim().split('\n').pop()!.trim(),
|
||||||
|
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);
|
||||||
|
|
||||||
|
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([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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,64 @@
|
|||||||
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -28,6 +28,9 @@
|
|||||||
--bg-primary: #FAFAF8; /* Warm White */
|
--bg-primary: #FAFAF8; /* Warm White */
|
||||||
--bg-secondary: #F5EFE6; /* Sand — hero panels, sunken rows */
|
--bg-secondary: #F5EFE6; /* Sand — hero panels, sunken rows */
|
||||||
--bg-card: #FFFFFF;
|
--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;
|
--surface-inverse: #0F766E;
|
||||||
|
|
||||||
/* ── Ink ────────────────────────────────────────────────────────── */
|
/* ── Ink ────────────────────────────────────────────────────────── */
|
||||||
@@ -234,6 +237,7 @@
|
|||||||
--bg-primary: #111A20;
|
--bg-primary: #111A20;
|
||||||
--bg-secondary: #16222A;
|
--bg-secondary: #16222A;
|
||||||
--bg-card: #18242C;
|
--bg-card: #18242C;
|
||||||
|
--bg-card-rgb: 24, 36, 44;
|
||||||
--surface-inverse: #E9EEF0;
|
--surface-inverse: #E9EEF0;
|
||||||
|
|
||||||
--text-primary: #E9EEF0;
|
--text-primary: #E9EEF0;
|
||||||
|
|||||||
@@ -413,7 +413,17 @@
|
|||||||
/* ── Narrow ───────────────────────────────────────────────────────── */
|
/* ── Narrow ───────────────────────────────────────────────────────── */
|
||||||
|
|
||||||
@media (max-width: 768px) {
|
@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;
|
padding: 0.875rem;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -457,6 +467,14 @@
|
|||||||
align-items: flex-start;
|
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 {
|
.geoError {
|
||||||
text-align: left;
|
text-align: left;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -69,14 +69,28 @@ export function FilterBar({
|
|||||||
const [omniValue, setOmniValue] = useState(initialOmniValue);
|
const [omniValue, setOmniValue] = useState(initialOmniValue);
|
||||||
|
|
||||||
const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`;
|
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
|
// Suppressed once the value parses as a postcode: the box takes a school
|
||||||
// name OR a postcode, and suggesting schools during postcode entry fights
|
// name OR a postcode, and suggesting schools during postcode entry fights
|
||||||
// the user rather than helping them.
|
// the user rather than helping them.
|
||||||
const suggestEnabled = autosuggest && !isValidPostcode(omniValue);
|
const suggestEnabled = autosuggest && hasTyped && !isValidPostcode(omniValue);
|
||||||
const { suggestions, open, activeIndex, setActiveIndex, close } =
|
const { suggestions, open, activeIndex, setActiveIndex, close } =
|
||||||
useSchoolSuggest(omniValue, suggestEnabled);
|
useSchoolSuggest(omniValue, suggestEnabled);
|
||||||
|
|
||||||
const pickSuggestion = (s: Suggestion) => {
|
const pickSuggestion = (s: Suggestion) => {
|
||||||
|
setHasTyped(false);
|
||||||
close();
|
close();
|
||||||
track('search_submitted', {
|
track('search_submitted', {
|
||||||
query: s.school_name.toLowerCase(),
|
query: s.school_name.toLowerCase(),
|
||||||
@@ -169,6 +183,9 @@ export function FilterBar({
|
|||||||
|
|
||||||
const handleSearchSubmit = (e: React.FormEvent) => {
|
const handleSearchSubmit = (e: React.FormEvent) => {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
|
// The search has been made; the suggestions that led to it are spent.
|
||||||
|
setHasTyped(false);
|
||||||
|
close();
|
||||||
if (!omniValue.trim()) {
|
if (!omniValue.trim()) {
|
||||||
updateURL({ search: "", postcode: "", radius: "" });
|
updateURL({ search: "", postcode: "", radius: "" });
|
||||||
return;
|
return;
|
||||||
@@ -271,7 +288,7 @@ export function FilterBar({
|
|||||||
ref={inputRef}
|
ref={inputRef}
|
||||||
type="search"
|
type="search"
|
||||||
value={omniValue}
|
value={omniValue}
|
||||||
onChange={(e) => setOmniValue(e.target.value)}
|
onChange={(e) => { setOmniValue(e.target.value); setHasTyped(true); }}
|
||||||
onKeyDown={handleOmniKeyDown}
|
onKeyDown={handleOmniKeyDown}
|
||||||
onBlur={close}
|
onBlur={close}
|
||||||
placeholder="School name or postcode"
|
placeholder="School name or postcode"
|
||||||
|
|||||||
@@ -47,7 +47,13 @@
|
|||||||
width: 100%;
|
width: 100%;
|
||||||
height: 100%;
|
height: 100%;
|
||||||
background:
|
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%;
|
background-size: 200% 100%;
|
||||||
animation: shimmer 1.4s infinite;
|
animation: shimmer 1.4s infinite;
|
||||||
}
|
}
|
||||||
@@ -76,6 +82,15 @@
|
|||||||
justify-content: center;
|
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 {
|
.openHint {
|
||||||
display: inline-flex;
|
display: inline-flex;
|
||||||
align-items: center;
|
align-items: center;
|
||||||
@@ -85,7 +100,8 @@
|
|||||||
border-radius: 999px;
|
border-radius: 999px;
|
||||||
font-size: 13px;
|
font-size: 13px;
|
||||||
font-weight: 600;
|
font-weight: 600;
|
||||||
color: var(--text-primary);
|
/* See "Controls that float ON the map" above. */
|
||||||
|
color: #1C2731;
|
||||||
background: rgba(255, 255, 255, .85);
|
background: rgba(255, 255, 255, .85);
|
||||||
-webkit-backdrop-filter: blur(6px);
|
-webkit-backdrop-filter: blur(6px);
|
||||||
backdrop-filter: blur(6px);
|
backdrop-filter: blur(6px);
|
||||||
@@ -113,11 +129,18 @@
|
|||||||
on top of the blend. */
|
on top of the blend. */
|
||||||
z-index: 450;
|
z-index: 450;
|
||||||
pointer-events: none;
|
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,
|
background: linear-gradient(to bottom,
|
||||||
rgba(255, 255, 255, 0) 0%,
|
rgba(var(--bg-card-rgb), 0) 0%,
|
||||||
rgba(255, 255, 255, .35) 35%,
|
rgba(var(--bg-card-rgb), .35) 35%,
|
||||||
rgba(255, 255, 255, .75) 62%,
|
rgba(var(--bg-card-rgb), .75) 62%,
|
||||||
rgba(255, 255, 255, .95) 82%,
|
rgba(var(--bg-card-rgb), .95) 82%,
|
||||||
var(--bg-card) 100%);
|
var(--bg-card) 100%);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -134,7 +157,8 @@
|
|||||||
border: none;
|
border: none;
|
||||||
border-radius: 8px;
|
border-radius: 8px;
|
||||||
background: rgba(255, 255, 255, .92);
|
background: rgba(255, 255, 255, .92);
|
||||||
color: var(--text-primary);
|
/* See "Controls that float ON the map" above. */
|
||||||
|
color: #1C2731;
|
||||||
cursor: pointer;
|
cursor: pointer;
|
||||||
box-shadow: 0 2px 10px rgba(var(--shadow-rgb), .2);
|
box-shadow: 0 2px 10px rgba(var(--shadow-rgb), .2);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import { placeUrl, authoritySlug } from '@/lib/places';
|
|||||||
import type { School } from '@/lib/types';
|
import type { School } from '@/lib/types';
|
||||||
import { schoolUrl } from '@/lib/utils';
|
import { schoolUrl } from '@/lib/utils';
|
||||||
import { absoluteUrl } from '@/lib/site';
|
import { absoluteUrl } from '@/lib/site';
|
||||||
|
import { TrackPlaceView } from './TrackPlaceView';
|
||||||
import styles from './PlaceView.module.css';
|
import styles from './PlaceView.module.css';
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
@@ -165,6 +166,11 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div className={styles.container}>
|
<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
|
<script
|
||||||
type="application/ld+json"
|
type="application/ld+json"
|
||||||
dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLd) }}
|
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'
|
| 'empty_results'
|
||||||
// Engagement
|
// Engagement
|
||||||
| 'school_viewed'
|
| 'school_viewed'
|
||||||
|
| 'place_viewed'
|
||||||
| 'section_nav_used'
|
| 'section_nav_used'
|
||||||
| 'chart_metric_changed'
|
| 'chart_metric_changed'
|
||||||
| 'metric_compared_in_rankings'
|
| 'metric_compared_in_rankings'
|
||||||
@@ -56,7 +57,10 @@ export function track(name: EventName, data?: Payload): void {
|
|||||||
* Categorise where the user navigated from, for funnel attribution
|
* Categorise where the user navigated from, for funnel attribution
|
||||||
* (mostly used on school_viewed). Only checks same-origin referrers.
|
* (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';
|
||||||
|
|
||||||
|
export function getNavigationSource(): NavigationSource {
|
||||||
if (typeof window === 'undefined' || !document.referrer) return 'direct';
|
if (typeof window === 'undefined' || !document.referrer) return 'direct';
|
||||||
try {
|
try {
|
||||||
const ref = new URL(document.referrer);
|
const ref = new URL(document.referrer);
|
||||||
@@ -65,6 +69,11 @@ export function getNavigationSource(): 'search' | 'rankings' | 'compare' | 'deta
|
|||||||
if (p === '/' || p === '') return 'search';
|
if (p === '/' || p === '') return 'search';
|
||||||
if (p.startsWith('/rankings')) return 'rankings';
|
if (p.startsWith('/rankings')) return 'rankings';
|
||||||
if (p.startsWith('/compare')) return 'compare';
|
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';
|
if (p.startsWith('/school/')) return 'detail';
|
||||||
return 'direct';
|
return 'direct';
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
Reference in new issue
Block a user