diff --git a/nextjs-app/__tests__/components/PlaceView.test.tsx b/nextjs-app/__tests__/components/PlaceView.test.tsx index 6540db9..931f2db 100644 --- a/nextjs-app/__tests__/components/PlaceView.test.tsx +++ b/nextjs-app/__tests__/components/PlaceView.test.tsx @@ -7,9 +7,9 @@ const detail: PlaceDetail = { parent_authority: 'Essex', phases: ['primary'] }, schools: [ { urn: 1, school_name: 'Alpha Primary', rwm_expected_pct: 82, - ofsted_grade: 1 } as never, + ofsted_grade: 1, phase: 'Primary' } as never, { urn: 2, school_name: 'Beta Primary', rwm_expected_pct: 44, - ofsted_grade: 3 } as never, + ofsted_grade: 3, phase: 'Primary' } as never, ], averages: { rwm_expected_pct: 63, attainment_8_score: null }, }; @@ -112,3 +112,59 @@ describe('PlaceView phase variants', () => { .not.toBeInTheDocument(); }); }); + +describe('PlaceView presentation', () => { + // /schools/brentwood shipped with 8 of 27 rows blank: an unphased page shows + // one primary-only measure for a list that also holds secondaries. + const mixed: PlaceDetail = { + place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 4, + parent_authority: 'Essex', phases: ['primary', 'secondary'] }, + schools: [ + { urn: 1, school_name: 'Alpha Primary', phase: 'Primary', + rwm_expected_pct: 82, attainment_8_score: null } as never, + { urn: 2, school_name: 'Beta High', phase: 'Secondary', + rwm_expected_pct: null, attainment_8_score: 47 } as never, + ], + averages: { rwm_expected_pct: 63, attainment_8_score: 45 }, + }; + + it('gives each phase its own table rather than one column of blanks', () => { + render(); + expect(screen.getByRole('heading', { name: /^Primary schools/ })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: /^Secondary schools/ })).toBeInTheDocument(); + expect(screen.getByText('82%')).toBeInTheDocument(); + expect(screen.getByText('47')).toBeInTheDocument(); + }); + + it('names the measure in plain words, not jargon', () => { + // The first cut said "RWM expected", which appears nowhere else on the site. + render(); + expect(screen.getByText('Reading, writing & maths')).toBeInTheDocument(); + expect(screen.getByText('Attainment 8')).toBeInTheDocument(); + expect(screen.queryByText(/RWM expected/i)).not.toBeInTheDocument(); + }); + + it('says a missing result is unpublished rather than showing a bare dash', () => { + const noResult: PlaceDetail = { + ...mixed, + schools: [{ urn: 3, school_name: 'New Primary', phase: 'Primary', + rwm_expected_pct: null, attainment_8_score: null } as never], + }; + render(); + expect(screen.getByText('Not published')).toBeInTheDocument(); + }); + + it('styles school links to the site convention rather than browser default', () => { + const { container } = render(); + const link = container.querySelector('a[href^="/school/"]'); + expect(link?.className).toBeTruthy(); + }); + + it('a phased page shows one table and no phase headings', () => { + render(); + expect(screen.queryByRole('heading', { name: /^Secondary schools/ })) + .not.toBeInTheDocument(); + }); +}); diff --git a/nextjs-app/components/places/PlaceView.module.css b/nextjs-app/components/places/PlaceView.module.css index 5aec3a3..3f69233 100644 --- a/nextjs-app/components/places/PlaceView.module.css +++ b/nextjs-app/components/places/PlaceView.module.css @@ -1,4 +1,8 @@ -/* Tokens only — see globals.css. Matches RankingsView's conventions. */ +/* Tokens only — see globals.css. Follows RankingsView's conventions, and in + particular its link treatment: table links take --text-primary with no + underline and a brand-coloured hover, not the browser default. The first + cut used bare with no class at all, which rendered as default blue + underlined links and read as unstyled beside the rest of the site. */ .container { width: 100%; min-width: 0; @@ -24,6 +28,44 @@ line-height: 1.6; } +/* Links in running copy: brand colour, underline on hover only. */ +.inlineLink { + color: var(--brand); + text-decoration: none; + transition: color 0.2s ease; +} + +.inlineLink:hover { + color: var(--brand-strong); + text-decoration: underline; +} + +/* Phase variants are separate indexable pages, so the bare place page has to + link them — a sitemap entry alone leaves them with no internal path in. */ +.phaseLinks { + display: flex; + flex-wrap: wrap; + gap: 0.5rem 0.75rem; + margin: 0 0 1.25rem; +} + +.phaseLink { + display: inline-block; + padding: 0.4rem 0.875rem; + border: 1px solid var(--border-strong); + border-radius: 999px; + font-size: 0.875rem; + font-weight: 500; + color: var(--text-primary); + text-decoration: none; + transition: border-color 0.2s ease, color 0.2s ease; +} + +.phaseLink:hover { + border-color: var(--brand); + color: var(--brand-strong); +} + /* The one number a list cannot give you, so it gets its own band. */ .compare { background: var(--bg-secondary); @@ -46,6 +88,30 @@ color: var(--text-secondary); } +.group { + margin-bottom: 2rem; +} + +.groupHeading { + display: flex; + align-items: baseline; + gap: 0.625rem; + font-size: 1.25rem; + font-weight: 600; + color: var(--text-primary); + font-family: var(--font-display); + margin: 0 0 0.75rem; +} + +.groupCount { + font-size: 0.8125rem; + font-weight: 500; + color: var(--text-secondary); + background: var(--bg-secondary); + border-radius: 999px; + padding: 0.125rem 0.5rem; +} + /* Wide content scrolls in its own container so the page body never does. */ .tableWrap { overflow-x: auto; @@ -72,8 +138,6 @@ color: var(--text-secondary); font-weight: 600; font-size: 0.8125rem; - text-transform: uppercase; - letter-spacing: 0.04em; } .table tbody tr:last-child td { @@ -86,6 +150,30 @@ font-variant-numeric: tabular-nums; } +/* The measure is spelled out; the tooltip carries the definition. */ +.metricHead { + text-decoration: none; + cursor: help; + border-bottom: 1px dotted var(--border-strong); +} + +/* Table links: site convention is body colour, brand on hover. */ +.schoolLink { + color: var(--text-primary); + text-decoration: none; + transition: color 0.2s ease; +} + +.schoolLink:hover { + color: var(--brand-strong); +} + +/* "Not published" is a fact about the school, not an error. */ +.noData { + color: var(--text-muted); + font-size: 0.8125rem; +} + .neighbours { margin-top: 2rem; } @@ -106,13 +194,3 @@ padding: 0; margin: 0; } - -/* Phase variants are separate indexable pages, so the bare place page has to - link them — a sitemap entry alone leaves them with no internal path in. */ -.phaseLinks { - display: flex; - flex-wrap: wrap; - gap: 0.5rem 1rem; - margin: 0 0 1.25rem; - font-size: 0.9375rem; -} diff --git a/nextjs-app/components/places/PlaceView.tsx b/nextjs-app/components/places/PlaceView.tsx index db37692..5c4b463 100644 --- a/nextjs-app/components/places/PlaceView.tsx +++ b/nextjs-app/components/places/PlaceView.tsx @@ -12,6 +12,7 @@ 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 { absoluteUrl } from '@/lib/site'; import styles from './PlaceView.module.css'; @@ -30,19 +31,101 @@ const OFSTED_LABELS: Array<[number, string]> = [ [3, 'Requires improvement'], [4, 'Inadequate'], ]; +/** + * Column headings, taken from the site's own metric dictionary rather than + * invented here — see METRIC_DEFINITIONS in backend/schemas.py, surfaced at + * /api/metrics. The first cut said "RWM expected", which is jargon that + * appears nowhere else on the site. + */ +const METRICS = { + primary: { + key: 'rwm_expected_pct' as const, + heading: 'Reading, writing & maths', + hint: '% meeting the expected standard in reading, writing and maths', + unit: '%', + }, + secondary: { + key: 'attainment_8_score' as const, + heading: 'Attainment 8', + hint: "Average grade across a pupil's best 8 GCSEs, including English and maths", + unit: '', + }, +}; + +type PhaseKey = keyof typeof METRICS; + +/** All-through schools sit in both phases, matching the search filters. */ +function isPhase(school: School, phase: PhaseKey): boolean { + const p = (school.phase ?? '').toLowerCase(); + if (p === 'all-through') return true; + return phase === 'secondary' + ? p.includes('secondary') || p === '16 plus' + : p.includes('primary') || p.includes('middle'); +} + +function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey }) { + const metric = METRICS[phase]; + return ( +
+ + + + + + + + + {schools.map((s) => { + const value = s[metric.key]; + return ( + + + + + ); + })} + +
School + + {metric.heading} + +
+ + {s.school_name} + + + {value == null + ? Not published + : `${Math.round(Number(value))}${metric.unit}`} +
+
+ ); +} + export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) { const { place, schools, averages } = detail; - const metric = phase === 'secondary' ? 'attainment_8_score' : 'rwm_expected_pct'; - const local = averages[metric]; + const local = averages[METRICS[phase ?? 'primary'].key]; const phaseWord = phase === 'secondary' ? 'Secondary schools' : phase === 'primary' ? 'Primary schools' : 'Schools'; const graded = OFSTED_LABELS .map(([grade, label]) => [label, schools.filter((s) => s.ofsted_grade === grade).length] as const) .filter(([, n]) => n > 0); - // ItemList tells Google this page is a ranked set rather than prose; - // BreadcrumbList puts the place in a hierarchy. Capped at 20 because that - // is what the page shows above the fold and what the markup should mirror. + /* + * An unphased page holds both primaries and secondaries, and they are + * scored on different measures — a percentage and a 0-90 score. Showing one + * column for both left 30% of rows blank on /schools/brentwood and put two + * incomparable scales in one column when it did not. + * + * So the phases get a table each. A blank cell inside one now means the + * school genuinely has no published result, which is worth saying. + */ + const groups: Array<[PhaseKey, School[]]> = phase + ? [[phase, schools]] + : (['primary', 'secondary'] as PhaseKey[]) + .map((p) => [p, schools.filter((s) => isPhase(s, p))] as [PhaseKey, School[]]) + .filter(([, list]) => list.length > 0); + const jsonLd = { '@context': 'https://schema.org', '@graph': [ @@ -60,8 +143,7 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) { '@type': 'BreadcrumbList', itemListElement: [ - { '@type': 'ListItem', position: 1, name: 'Schools', - item: absoluteUrl('/') }, + { '@type': 'ListItem', position: 1, name: 'Schools', item: absoluteUrl('/') }, { '@type': 'ListItem', position: 2, name: place.name }, ], }, @@ -74,6 +156,7 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) type="application/ld+json" dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLd) }} /> +

{phaseWord} in {place.name}

@@ -81,7 +164,8 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) {place.parent_authority && ( <> {' · '} - + {place.parent_authority} @@ -92,7 +176,7 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) {!phase && (place.phases ?? []).length > 0 && (