From 94151c58ea38a9256d66d15161293486a500c7a2 Mon Sep 17 00:00:00 2001 From: Tudor Date: Thu, 20 Aug 2026 14:54:02 +0100 Subject: [PATCH] fix(admissions): move the cut-off detail into its own section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admissions card measured 1503px on a live school page — half the height of every section put together, and nearly three times the next largest — with its default view rendering as four tiles adrift in about 1080px of blank card. The cause was a layout trick meeting content it was never sized for. The admissions views are stacked in one grid cell so switching them never shifts layout, and the hidden ones keep their box: only visibility is dropped. That works while the views are comparable. The distance view added in #102 carries a chart, a table and a map, came to 1402px against the tile grid's 316px, and pinned every other view to its height — including the one that renders by default, which nobody had clicked. Rather than only unpinning it, the detail moves out. Every other topic on the page is a section with a nav entry, and "how close did we need to live, and would we have got in?" is a topic, not a variant reading of the intake figures. The headline number stays on the Admissions tile where the intake story is; the record behind it now lives in a Distance section directly below. admissions 1503px -> 554px distance new -> 743px (median section on the page is ~528px) Three further changes, each of which also makes the content better rather than only shorter: * The map renders on request. Before a postcode is entered it is a circle drawn round a school, and it costs a Leaflet bundle and 240px to say so; a successful check opens it automatically, which is the point at which it starts answering something. Map height 320px -> 240px. * The chart appears only at the four published years that let the summary state a direction. Below that we already refuse to call the series a trend, and a line through three points asserts one regardless of what the sentence beneath it admits. The table carries those years anyway, with the reasons a line cannot show. * Three caveat paragraphs become one. They said walking-route twice and made the same point about priorities in two voices. It now sits in CutoffDistanceDetail rather than inside the check, so it still renders for a school with coordinates missing, where there is a table but no map and no check. The new e2e guard asserts no section exceeds 2.5x the median section height. Measuring the card's internals cannot catch this: the tile grid is flex: 1, so it absorbs the stretch and every box still looks full. The first version of this test targeted an arbitrary primary, passed against the live bug, and proved nothing; pointed at a school that actually holds cut-off history it fails on staging with "#admissions is 1459px against a 526px median". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WDvkyqqHABm4bmth2kjAxE --- e2e/tests/journeys.spec.ts | 84 +++++++++++------ .../components/CutoffMapPanel.test.tsx | 31 +++++-- .../components/lastDistanceOffered.test.tsx | 61 ++++++++++--- nextjs-app/app/school/[slug]/page.tsx | 2 + .../components/school/AdmissionsSection.tsx | 38 ++------ .../school/CutoffDistanceDetail.tsx | 43 +++++---- .../components/school/CutoffMapPanel.tsx | 91 ++++++++++++------- .../components/school/DistanceSection.tsx | 50 ++++++++++ .../school/PrimarySchoolSections.tsx | 11 ++- .../school/SecondaryAdmissionsSection.tsx | 13 +-- .../school/SecondarySchoolSections.tsx | 9 +- .../components/school/lastDistanceOffered.ts | 57 +++++++----- .../school/schoolSections.module.css | 76 ++++++++++++---- nextjs-app/lib/schoolSections.ts | 22 ++++- 14 files changed, 403 insertions(+), 185 deletions(-) create mode 100644 nextjs-app/components/school/DistanceSection.tsx diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 1d36ffb..165ad0d 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1324,16 +1324,13 @@ test('a school with several published years gets the Distance view', async ({ pa await page.goto(`/school/${found!.urn}`); await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); - const isSecondary = /secondary/i.test(found!.phase ?? ''); - if (!isSecondary) { - // Primary pages carry a segmented control; the detail sits behind it. - const distanceTab = page.getByRole('button', { name: 'Distance' }); - await expect(distanceTab).toBeVisible(); - await distanceTab.click(); - await expect(distanceTab).toHaveAttribute('aria-pressed', 'true'); - } + // Its own section on both templates, not a third tab inside Admissions — + // stacking a view that tall in the admissions viewport sized the whole card + // to it and left the default view mostly blank. + await expect(page.locator('#distance')).toBeVisible(); + await expect(page.getByRole('button', { name: 'Distance' })).toHaveCount(0); - // Every published year must appear as a row, whichever template rendered it. + // Every published year must appear as a row. for (const h of found!.history as { year: number }[]) { await expect(page.getByRole('rowheader', { name: String(h.year) })).toBeVisible(); } @@ -1345,10 +1342,7 @@ test('the year table never leaves a gap unexplained', async ({ page }) => { test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); - if (!/secondary/i.test(found!.phase ?? '')) { - await page.getByRole('button', { name: 'Distance' }).click(); - } + await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); const statuses = await page.locator('[class*="cutoffPill"]').allTextContents(); expect(statuses.length).toBeGreaterThan(0); @@ -1364,10 +1358,7 @@ test('the postcode check answers with a distance and per-year verdicts', async ( test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); - if (!/secondary/i.test(found!.phase ?? '')) { - await page.getByRole('button', { name: 'Distance' }).click(); - } + await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); const input = page.getByLabel('Your postcode'); await expect(input).toBeVisible(); @@ -1391,13 +1382,14 @@ test('the postcode check states its limits before it is used', async ({ page }) test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); - if (!/secondary/i.test(found!.phase ?? '')) { - await page.getByRole('button', { name: 'Distance' }).click(); - } + await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); - await expect(page.getByText(/An indication only/)).toBeVisible(); - await expect(page.getByText(/not a catchment boundary/)).toBeVisible(); + const caveat = page.getByText(/Distance is the last criterion applied/); + await expect(caveat).toBeVisible(); + await expect(caveat).toContainText(/not a catchment boundary/); + await expect(caveat).toContainText(/walking route/); + // One caveat for the section, not the three paragraphs it replaced. + await expect(caveat).toHaveCount(1); }); test('the distance section never scrolls the page sideways', async ({ page }) => { @@ -1406,10 +1398,7 @@ test('the distance section never scrolls the page sideways', async ({ page }) => await page.setViewportSize({ width: 390, height: 844 }); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); - if (!/secondary/i.test(found!.phase ?? '')) { - await page.getByRole('button', { name: 'Distance' }).click(); - } + await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); // The year table is deliberately wider than a phone; its own wrapper has to // absorb that, or the whole page slides under the reader's thumb. @@ -1417,3 +1406,44 @@ test('the distance section never scrolls the page sideways', async ({ page }) => document.documentElement.scrollWidth - document.documentElement.clientWidth); expect(overflow, 'page must not scroll horizontally').toBeLessThanOrEqual(1); }); + +test('no single section dominates the height of a school page', async ({ page }) => { + /* + * The admissions views are stacked in one grid cell so switching them never + * shifts layout, which means the card is sized by its TALLEST view while the + * hidden ones keep their box. A distance view carrying a chart, a table and a + * map was added there and measured 1402px against the tile grid's 316px; the + * DEFAULT view rendered as four tiles adrift in ~1080px of blank card, and + * Admissions alone came to half the height of every section on the page + * (1503px against 526px for the next largest). + * + * Measuring the card's internals cannot catch it: the tile grid is + * `flex: 1`, so it absorbs the stretch and every box still looks full. What + * a reader actually sees is one section wildly out of proportion with its + * neighbours, so that is what this asserts. + */ + const found = await schoolWithCutoff(page, 2); + test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); + + await page.goto(`/school/${found!.urn}`); + await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); + await page.waitForTimeout(1200); // charts settle, and they carry real height + + const sections = await page.evaluate(() => + [...document.querySelectorAll('section[id]')] + .map((s) => ({ id: s.id, h: Math.round(s.getBoundingClientRect().height) })) + .filter((s) => s.h > 0)); + + expect(sections.length).toBeGreaterThanOrEqual(3); + const heights = sections.map((s) => s.h).sort((a, b) => a - b); + const median = heights[Math.floor(heights.length / 2)]; + const worst = sections.reduce((a, b) => (a.h > b.h ? a : b)); + + // Generous: a rich section may fairly run to twice a plain one. Nearly three + // times over is the shape of a layout fault, not of denser content. + expect( + worst.h / median, + `#${worst.id} is ${worst.h}px against a ${median}px median: ` + + sections.map((s) => `${s.id}=${s.h}`).join(', '), + ).toBeLessThan(2.5); +}); diff --git a/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx b/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx index ad8c253..aabb4ca 100644 --- a/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx +++ b/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx @@ -132,13 +132,30 @@ describe('CutoffMapPanel', () => { expect(screen.getByRole('alert')).toHaveTextContent(/Something went wrong/); }); - it('always states the limits of the check', async () => { + it('does not carry the caveat itself', () => { + // It moved to CutoffDistanceDetail so it renders once per section, and so + // it still appears for a school with no coordinates (no map, no check). + // Asserting its absence here is what stops the old three-paragraph stutter + // creeping back. renderPanel([d(2024, 800)]); - // Present before any postcode is entered, not revealed with the answer. - expect(screen.getByText(/An indication only/)).toBeInTheDocument(); - // The two caveats divide the work: the map says the rings are not a - // boundary, the check says the measurement may not be a straight line. - expect(screen.getByText(/not a catchment boundary/)).toBeInTheDocument(); - expect(screen.getByText(/walking route rather than a straight line/)).toBeInTheDocument(); + expect(screen.queryByText(/Distance is the last criterion/)).not.toBeInTheDocument(); + }); + + it('keeps the map behind a request until there is a reason to show it', async () => { + renderPanel([d(2024, 800)]); + expect(screen.queryByTestId('cutoff-map')).not.toBeInTheDocument(); + + // A successful check is that reason: the rings only answer a question once + // there is a home to sit beside them. + mockGeocode.mockResolvedValue(northOf(200)); + await check('SE23 3NA'); + await screen.findByRole('status'); + expect(screen.getByTestId('cutoff-map')).toBeInTheDocument(); + }); + + it('can also show the map without a postcode, on request', () => { + renderPanel([d(2024, 800)]); + fireEvent.click(screen.getByRole('button', { name: /Show these distances on a map/ })); + expect(screen.getByTestId('cutoff-map')).toBeInTheDocument(); }); }); diff --git a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx index 375883f..c3c13bd 100644 --- a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx +++ b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx @@ -96,27 +96,33 @@ const history = (pts: [number, number][]): SchoolAdmissionDistance[] => year, distance_m, route_count: 1, la_name: 'Camden', distance_unit_raw: 'miles', })); -describe('primary Distance view', () => { - it('appears once there are two or more published years', () => { - renderSchoolDetail({ +describe('primary Distance section', () => { + it('appears as its own section once there are two or more published years', () => { + // Not a third tab inside Admissions: stacking a 1402px view in the + // admissions viewport sized the whole card to it and left the default view + // as four tiles in ~1080px of blank card. + const { container } = renderSchoolDetail({ ...primaryFixture, admissionDistance: cutoff({ distance_m: 700, year: 2024 }), admissionDistanceHistory: history([[2021, 1000], [2022, 900], [2023, 800], [2024, 700]]), }); - expect(screen.getByRole('button', { name: 'Distance' })).toBeInTheDocument(); + expect(container.querySelector('#distance')).toBeInTheDocument(); + expect(screen.getByText('How far the last place went')).toBeInTheDocument(); + // And it did not come back as a tab. + expect(screen.queryByRole('button', { name: 'Distance' })).not.toBeInTheDocument(); }); - it('stays hidden on a single published year', () => { + it('stays away on a single published year', () => { // One point is a fact, not a history; a chart of it invites a trend reading // that is not there. - renderSchoolDetail({ + const { container } = renderSchoolDetail({ ...primaryFixture, admissionDistance: cutoff({ distance_m: 700, year: 2024 }), admissionDistanceHistory: history([[2024, 700]]), }); - expect(screen.queryByRole('button', { name: 'Distance' })).not.toBeInTheDocument(); + expect(container.querySelector('#distance')).not.toBeInTheDocument(); }); it('lists every year in the span, including the ones with no figure', () => { @@ -126,10 +132,6 @@ describe('primary Distance view', () => { admissionDistanceHistory: history([[2021, 1000], [2024, 700]]), }); - // The view is server-rendered but starts hidden behind its tab, and role - // queries skip hidden subtrees — so open it the way a reader would. - fireEvent.click(screen.getByRole('button', { name: 'Distance' })); - // 2022 and 2023 were never published but must still appear as rows, or the // gap in the chart has nothing explaining it. for (const year of ['2021', '2022', '2023', '2024']) { @@ -156,6 +158,34 @@ describe('primary Distance view', () => { expect(screen.queryByText(/all offered/i)).not.toBeInTheDocument(); }); + it('draws the chart only once it will also state a direction', () => { + const four = renderSchoolDetail({ + ...primaryFixture, + admissionDistance: cutoff({ distance_m: 700, year: 2024 }), + admissionDistanceHistory: history([[2021, 1000], [2022, 900], [2023, 800], [2024, 700]]), + }); + expect(screen.getByText(/Last distance offered, by year/)).toBeInTheDocument(); + four.unmount(); + + renderSchoolDetail({ + ...primaryFixture, + admissionDistance: cutoff({ distance_m: 700, year: 2024 }), + admissionDistanceHistory: history([[2023, 800], [2024, 700]]), + }); + expect(screen.queryByText(/Last distance offered, by year/)).not.toBeInTheDocument(); + }); + + it('states the caveat exactly once, even though the check also renders', () => { + renderSchoolDetail({ + ...primaryFixture, + schoolInfo: { ...primaryFixture.schoolInfo, latitude: 51.5, longitude: -0.12 }, + admissionDistance: cutoff({ distance_m: 700, year: 2024 }), + admissionDistanceHistory: history([[2021, 1000], [2024, 700]]), + }); + + expect(screen.getAllByText(/Distance is the last criterion/)).toHaveLength(1); + }); + it('withholds a trend reading while the record is thin', () => { renderSchoolDetail({ ...primaryFixture, @@ -168,16 +198,19 @@ describe('primary Distance view', () => { }); }); -describe('secondary Distance detail', () => { - it('renders inline, with no extra tab, once there are two published years', () => { +describe('secondary Distance section', () => { + it('renders as its own section once there are two published years', () => { renderSecondarySchoolDetail({ ...secondaryFixture, admissionDistance: cutoff({ distance_m: 3472.96 }), admissionDistanceHistory: history([[2023, 3800], [2024, 3472.96]]), }); - expect(screen.getByText(/Last distance offered, by year/)).toBeInTheDocument(); expect(screen.getByRole('rowheader', { name: '2023' })).toBeInTheDocument(); + // Two points is below the threshold that lets us state a direction, so no + // chart is drawn — a line through three points asserts a trend the + // summary underneath would refuse to. + expect(screen.queryByText(/Last distance offered, by year/)).not.toBeInTheDocument(); }); it('explains a selective school by how it admits rather than as missing data', () => { diff --git a/nextjs-app/app/school/[slug]/page.tsx b/nextjs-app/app/school/[slug]/page.tsx index b29ac71..547e70a 100644 --- a/nextjs-app/app/school/[slug]/page.tsx +++ b/nextjs-app/app/school/[slug]/page.tsx @@ -177,6 +177,8 @@ export default async function SchoolPage({ params }: SchoolPageProps) { ofsted: ofsted ?? null, admissions: admissions ?? null, admissionDistance: admission_distance ?? null, + admissionDistanceHistory: admission_distance_history ?? [], + admissionsHistory: admissions_history ?? [], yearlyDataLength: yearly_data.length, }; const primaryNavItems = buildNavItems(primaryFlags, navInput); diff --git a/nextjs-app/components/school/AdmissionsSection.tsx b/nextjs-app/components/school/AdmissionsSection.tsx index 9cb9d24..6b423b7 100644 --- a/nextjs-app/components/school/AdmissionsSection.tsx +++ b/nextjs-app/components/school/AdmissionsSection.tsx @@ -9,28 +9,29 @@ * AdmissionsViewToggle client island, which receives both views as * server-rendered children. When there is only one year of offer data no * toggle renders at all, so such pages ship zero admissions JavaScript. + * + * The two views are stacked in one grid cell so switching never shifts layout, + * which means the card is sized by the TALLER of them. Keep any view added + * here close to the tile grid in height: a cut-off-distance view carrying a + * chart, a table and a map was tried, measured 1402px against the tiles' 316px, + * and left the default view as four tiles adrift in blank card. It now lives in + * DistanceSection. */ import type { ReactNode } from 'react'; -import type { School, SchoolAdmissions, SchoolAdmissionDistance } from '@/lib/types'; +import type { SchoolAdmissions, SchoolAdmissionDistance } from '@/lib/types'; import { formatAcademicYear, formatPercentage } from '@/lib/utils'; import { summariseAdmissions } from '@/lib/compareLogic'; import { Section, sectionStyles as styles } from './sectionShared'; import { AdmissionsViewToggle, type AdmissionsView } from './AdmissionsViewToggle'; import { AdmissionsTrendChart } from './charts'; -import { CutoffDistanceDetail } from './CutoffDistanceDetail'; -import { - describeCutoff, cutoffYearRows, - CUTOFF_NOTE, CUTOFF_MEASUREMENT_NOTE, -} from './lastDistanceOffered'; +import { describeCutoff, CUTOFF_NOTE, CUTOFF_MEASUREMENT_NOTE } from './lastDistanceOffered'; export function AdmissionsSection({ admissions, admissionsHistory, admissionDistance, - admissionDistanceHistory, - schoolInfo, isAllThrough, }: { /* Nullable: the two data sources are independent. A minority of schools have @@ -40,18 +41,9 @@ export function AdmissionsSection({ admissions: SchoolAdmissions | null; admissionsHistory: SchoolAdmissions[]; admissionDistance: SchoolAdmissionDistance | null; - admissionDistanceHistory: SchoolAdmissionDistance[]; - schoolInfo: School; isAllThrough: boolean; }) { const cutoff = describeCutoff(admissionDistance); - // Every year in the span, including the ones with nothing in them — the - // classification of a missing year is what the Distance view is mostly for. - const cutoffRows = cutoffYearRows(admissionDistanceHistory, admissionsHistory); - const publishedYears = cutoffRows.filter((r) => r.status === 'published').length; - // One published year is a fact, not a history: the tile already says it, and - // a chart of a single point invites a trend reading that is not there. - const showDistanceView = publishedYears >= 2; // Trend toggle only appears with ≥2 years carrying an offer rate. const admissionsOfferYears = admissionsHistory.filter((h) => h.first_preference_offer_pct != null).length; @@ -182,10 +174,6 @@ export function AdmissionsSection({ ); - const distanceView = showDistanceView && ( - - ); - const views: AdmissionsView[] = [ { id: 'year', label: 'This year', content: yearView, className: styles.admissionsViewYear }, ]; @@ -197,14 +185,6 @@ export function AdmissionsSection({ className: styles.admissionsViewTrend, }); } - if (showDistanceView) { - views.push({ - id: 'distance', - label: 'Distance', - content: distanceView, - className: styles.admissionsViewTrend, - }); - } return (
diff --git a/nextjs-app/components/school/CutoffDistanceDetail.tsx b/nextjs-app/components/school/CutoffDistanceDetail.tsx index 88093d0..f44514e 100644 --- a/nextjs-app/components/school/CutoffDistanceDetail.tsx +++ b/nextjs-app/components/school/CutoffDistanceDetail.tsx @@ -21,38 +21,44 @@ import { CutoffTrendChart } from './charts'; import { CutoffYearTable } from './CutoffYearTable'; import { CutoffMapPanel } from './CutoffMapPanel'; import { - cutoffTrendSummary, cutoffCoverageNote, - CUTOFF_NOTE, CUTOFF_MEASUREMENT_NOTE, + cutoffTrendSummary, cutoffCoverageNote, CUTOFF_CHECK_CAVEAT, type CutoffYearRow, } from './lastDistanceOffered'; export function CutoffDistanceDetail({ rows, schoolInfo, - /** Repeat the "not a catchment" caveat here. On the primary page this view is - * behind a tab, so a reader can reach it without ever seeing the tile that - * carries the caveat; on the secondary page the caveat is already directly - * above, and repeating it verbatim reads as a stutter. */ - showCaveat = true, }: { rows: CutoffYearRow[]; schoolInfo: School; - showCaveat?: boolean; }) { const publishedYears = rows.filter((r) => r.status === 'published').length; if (publishedYears < 2) return null; const trendSummary = cutoffTrendSummary(rows); const coverageNote = cutoffCoverageNote(rows); + /* + * The chart appears at the same four points that let cutoffTrendSummary + * state a direction. Below that we already refuse to call the series a + * trend, and drawing a trend line under that refusal contradicts it — three + * points joined by a line say "look, it is falling" whatever the sentence + * beneath admits. The table carries every one of those years anyway, with + * the reasons a line cannot show, so nothing is lost by leaving it out. + */ + const showChart = publishedYears >= 4; return ( <> -
Last distance offered, by year
- -

- The line breaks where no figure was published — it is never drawn across - a gap. The table below says what happened in each of those years. -

+ {showChart && ( + <> +
Last distance offered, by year
+ +

+ Gaps are years with no published figure — the line is never drawn + across one. The table says what happened in each. +

+ + )} {trendSummary &&

{trendSummary}

} @@ -60,11 +66,10 @@ export function CutoffDistanceDetail({ - {showCaveat && ( -

- {CUTOFF_NOTE} {CUTOFF_MEASUREMENT_NOTE} -

- )} + {/* The single caveat for the whole section. It lives here rather than + inside the check, so it still renders for a school with no + coordinates — where there is a table but no map and no check. */} +

{CUTOFF_CHECK_CAVEAT}

); } diff --git a/nextjs-app/components/school/CutoffMapPanel.tsx b/nextjs-app/components/school/CutoffMapPanel.tsx index d03c295..71928f5 100644 --- a/nextjs-app/components/school/CutoffMapPanel.tsx +++ b/nextjs-app/components/school/CutoffMapPanel.tsx @@ -4,9 +4,14 @@ * CutoffMapPanel — "Where the last place went". * * The rings and the postcode check live in one component because they are one - * question asked twice: the map answers "how far is that?" and the form answers - * "how far are we?". Entering a postcode drops a pin on the same rings rather - * than producing a separate verdict somewhere else on the page. + * question asked twice: the form answers "how far are we?" and the map shows + * that answer against the cut-offs. Entering a postcode drops a pin on the same + * rings rather than producing a separate verdict elsewhere on the page. + * + * The map is not rendered until asked for. Before a postcode is entered it is a + * circle drawn round a school, and it costs a Leaflet bundle and 240px of + * section height to say that; a successful check opens it automatically, + * because that is the point at which it starts answering something. * * The postcode never leaves the browser except to postcodes.io for a lat/long, * and nothing is stored — this is a client-side measurement, not a lookup @@ -19,7 +24,7 @@ import type { School } from '@/lib/types'; import { geocodePostcode, calculateDistance } from '@/lib/api'; import { isValidPostcode } from '@/lib/utils'; import { - compareToCutoffs, CUTOFF_CHECK_CAVEAT, CUTOFF_MAP_CAVEAT, + compareToCutoffs, type CutoffYearRow, type CutoffCheckResult, type CutoffVerdict, } from './lastDistanceOffered'; import type { CutoffRing } from '../LeafletCutoffMapInner'; @@ -66,6 +71,12 @@ export function CutoffMapPanel({ const [result, setResult] = useState(null); const [error, setError] = useState(null); const [busy, setBusy] = useState(false); + /* The map is not rendered until it is asked for. It costs a Leaflet bundle + and 240px of section height, and before a postcode is entered it is a + circle drawn round a school — it only starts answering a question once + there is a home to put beside the rings. A successful check therefore + opens it automatically. */ + const [mapOpen, setMapOpen] = useState(false); const lat = schoolInfo.latitude; const lng = schoolInfo.longitude; @@ -107,6 +118,7 @@ export function CutoffMapPanel({ return; } setHome({ lat: point.latitude, lng: point.longitude }); + setMapOpen(true); // calculateDistance returns kilometres; everything here is metres. const metres = calculateDistance(point.latitude, point.longitude, lat, lng) * 1000; setResult(compareToCutoffs(metres, rows)); @@ -123,38 +135,14 @@ export function CutoffMapPanel({ return (
-

Where the last place went

- -
- -
- -
    -
  • -
    • - {rings.length > 1 && ( -
  • -
    • - )} - {home && ( -
  • -
    • - )} -
- -

{CUTOFF_MAP_CAVEAT}

- + {/* No "Where the last place went" heading above this any more. It + introduced a map that now sits behind a button, so it named nothing, + and it echoed the section title directly above it. */}
-

How far away are you?

+

How far away are you?

- We measure a straight line from your postcode to the school and compare - it with each year’s cut-off. Your postcode is not stored. + Straight-line distance from your postcode, compared with each year’s + cut-off. Not stored.

@@ -198,7 +186,40 @@ export function CutoffMapPanel({
)} -

{CUTOFF_CHECK_CAVEAT}

+ {mapOpen ? ( +
+
+ +
+
    +
  • +
    • + {rings.length > 1 && ( +
  • +
    • + )} + {home && ( +
  • +
    • + )} +
+
+ ) : ( + + )} +
); diff --git a/nextjs-app/components/school/DistanceSection.tsx b/nextjs-app/components/school/DistanceSection.tsx new file mode 100644 index 0000000..f9dbd8b --- /dev/null +++ b/nextjs-app/components/school/DistanceSection.tsx @@ -0,0 +1,50 @@ +/** + * DistanceSection — "How far the last place went". + * + * Its own section rather than a third tab inside Admissions. + * + * It began as a tab, which broke the admissions card in a way worth recording: + * the two admissions views are stacked in one grid cell so switching them never + * shifts layout, which means the card is sized by its TALLEST view. A view + * carrying a chart, a table and a map came to 1402px against the tile grid's + * 316px, so the default view — the one nobody had clicked — rendered as four + * tiles adrift in ~1080px of blank card, and Admissions alone accounted for + * half the height of every section on the page. + * + * The structural point stands on its own though: every other topic here is a + * section with a nav entry, and "how close did we need to live, and would we + * have got in?" is a topic, not a variant reading of the intake figures. The + * headline number stays on the Admissions tile where the intake story is; the + * record behind it lives here. + * + * Server component; the map and postcode form carry their own client boundary. + */ + +import type { School, SchoolAdmissions, SchoolAdmissionDistance } from '@/lib/types'; +import { Section, sectionStyles as styles } from './sectionShared'; +import { CutoffDistanceDetail } from './CutoffDistanceDetail'; +import { cutoffYearRows, hasCutoffDetail } from './lastDistanceOffered'; + +export function DistanceSection({ + admissionDistanceHistory, + admissionsHistory, + schoolInfo, +}: { + admissionDistanceHistory: SchoolAdmissionDistance[]; + admissionsHistory: SchoolAdmissions[]; + schoolInfo: School; +}) { + const rows = cutoffYearRows(admissionDistanceHistory, admissionsHistory); + if (!hasCutoffDetail(rows)) return null; + + return ( +
+

How far the last place went

+

+ The distance of the furthest home offered a place each year, and how your + own address compares. +

+ +
+ ); +} diff --git a/nextjs-app/components/school/PrimarySchoolSections.tsx b/nextjs-app/components/school/PrimarySchoolSections.tsx index 8afd49f..d3707b5 100644 --- a/nextjs-app/components/school/PrimarySchoolSections.tsx +++ b/nextjs-app/components/school/PrimarySchoolSections.tsx @@ -20,6 +20,7 @@ import type { SchoolFlags } from '@/lib/schoolSections'; import { OfstedSection } from './OfstedSection'; import { ResultsSection } from './ResultsSection'; import { AdmissionsSection } from './AdmissionsSection'; +import { DistanceSection } from './DistanceSection'; import { InclusionSection } from './InclusionSection'; import { HistorySection } from './HistorySection'; import { SchoolLifeSection } from './SchoolLifeSection'; @@ -103,12 +104,18 @@ export function PrimarySchoolSections({ admissions={admissions} admissionsHistory={admissionsHistory} admissionDistance={admissionDistance} - admissionDistanceHistory={admissionDistanceHistory} - schoolInfo={schoolInfo} isAllThrough={flags.isAllThrough} /> )} + {/* Its own section, directly after Admissions: it answers the question + the tile above raises. */} + + {flags.hasInclusionData && ( { const policy = schoolInfo.admissions_policy?.toLowerCase() ?? ''; @@ -116,11 +112,6 @@ export function SecondaryAdmissionsSection({

)} - {/* Inline rather than behind a tab: this template is one flat panel by - design and has no segmented control to hang a third view on. The - caveat is suppressed because it is already directly above. */} - - {hasSixthForm && (
This school has a sixth form (Post-16 provision). Post-16 destination data coming soon. diff --git a/nextjs-app/components/school/SecondarySchoolSections.tsx b/nextjs-app/components/school/SecondarySchoolSections.tsx index aa9b37e..8d49217 100644 --- a/nextjs-app/components/school/SecondarySchoolSections.tsx +++ b/nextjs-app/components/school/SecondarySchoolSections.tsx @@ -20,6 +20,7 @@ import type { SecondaryFlags } from '@/lib/schoolSections'; import { OfstedSection } from './OfstedSection'; import { GcseSection } from './GcseSection'; import { SecondaryAdmissionsSection } from './SecondaryAdmissionsSection'; +import { DistanceSection } from './DistanceSection'; import { SecondaryHistorySection } from './SecondaryHistorySection'; import { WellbeingSection } from './WellbeingSection'; import { FinancesSection } from './FinancesSection'; @@ -91,13 +92,19 @@ export function SecondarySchoolSections({ )} + {/* See PrimarySchoolSections — same placement, same reason. */} + + {yearlyData.length > 1 && ( r.status === 'published').length >= 2; +} diff --git a/nextjs-app/components/school/schoolSections.module.css b/nextjs-app/components/school/schoolSections.module.css index 066ec49..ab7bf35 100644 --- a/nextjs-app/components/school/schoolSections.module.css +++ b/nextjs-app/components/school/schoolSections.module.css @@ -1627,8 +1627,23 @@ color: var(--text-secondary); } -/* Stack both views in one grid cell so the card sizes to the taller view — - toggling modes never shifts layout. */ +/* + * Stack both views in one grid cell so the card sizes to the taller view — + * toggling modes never shifts layout. + * + * This only works while the views are of COMPARABLE height, and it fails + * silently rather than loudly when they are not. The hidden view keeps its + * box (display stays block below; only visibility is dropped), so the grid + * row is sized by the tallest view and every other view is padded out to + * match it. A cut-off-distance view carrying a chart, a table and a map was + * added here and came to 1402px against the tile grid's 316px, which left + * the DEFAULT view — the one nobody had clicked — as four tiles floating in + * ~1080px of blank card. + * + * Anything substantially taller than the tile grid belongs in its own + * section, not in this viewport. That is where the distance detail now + * lives; see DistanceSection. + */ .admissionsViewport { display: grid; } @@ -2191,17 +2206,11 @@ border-top: 1px solid var(--border); } -.cutoffMapTitle { - margin: 0 0 0.85rem; - font-family: var(--font-display); - font-size: 1.1rem; - font-weight: 700; - letter-spacing: -0.01em; - color: var(--text-primary); -} .cutoffMapFigure { - height: 320px; + /* Enough to read a set of concentric rings and no more — this is a + diagram of a number, not a map anyone navigates by. */ + height: 240px; border-radius: var(--radius-md); overflow: hidden; border: 1px solid var(--border); @@ -2267,16 +2276,18 @@ /* ── Postcode check ── */ +/* The check now leads the panel, so it carries no divider of its own — the + rule above it belonged to a map that used to sit there. */ .cutoffCheck { - margin-top: 1.5rem; - padding-top: 1.25rem; - border-top: 1px solid var(--border); + margin-top: 0; } .cutoffCheckTitle { margin: 0 0 0.25rem; font-family: var(--font-display); - font-size: 1rem; + /* Now the panel's only heading (an h3), so it carries the weight the + retired .cutoffMapTitle used to. */ + font-size: 1.1rem; font-weight: 700; color: var(--text-primary); } @@ -2441,6 +2452,39 @@ @media (max-width: 640px) { .cutoffMapFigure { - height: 260px; + height: 200px; } } + +/* ── Map, revealed on request ── */ + +.cutoffMapToggle { + display: inline-flex; + align-items: center; + gap: 0.4rem; + margin-top: 1rem; + padding: 0.5rem 0.9rem; + font: inherit; + font-size: 0.875rem; + font-weight: 600; + color: var(--brand-strong); + background: none; + border: 1px solid var(--border); + border-radius: 999px; + cursor: pointer; + transition: border-color var(--transition), background var(--transition); +} + +.cutoffMapToggle:hover { + border-color: var(--brand); + background: var(--brand-bg); +} + +.cutoffMapToggle:focus-visible { + outline: 2px solid var(--brand); + outline-offset: 2px; +} + +.cutoffMapReveal { + margin-top: 1rem; +} diff --git a/nextjs-app/lib/schoolSections.ts b/nextjs-app/lib/schoolSections.ts index 4a4dbc5..100cdf6 100644 --- a/nextjs-app/lib/schoolSections.ts +++ b/nextjs-app/lib/schoolSections.ts @@ -12,6 +12,7 @@ import type { OfstedInspection, SchoolAdmissions, SchoolAdmissionDistance, SchoolDeprivation, SchoolFinance, } from './types'; import { isSpecialSchool } from './utils'; +import { cutoffYearRows, hasCutoffDetail } from '@/components/school/lastDistanceOffered'; export interface SchoolFlagsInput { schoolInfo: School; @@ -122,6 +123,11 @@ export interface NavItemsInput { /** A published cut-off distance renders the Admissions section on its own, * so it has to open the nav entry too. */ admissionDistance: SchoolAdmissionDistance | null; + /** Drives the separate Distance section. Gated through hasCutoffDetail, the + * same predicate the section itself uses, so the nav can never link to an + * anchor that was not rendered. */ + admissionDistanceHistory?: SchoolAdmissionDistance[]; + admissionsHistory?: SchoolAdmissions[]; yearlyDataLength: number; } @@ -136,7 +142,10 @@ export interface NavItemsInput { */ export function buildNavItems( flags: SchoolFlags, - { ofsted, admissions, admissionDistance, yearlyDataLength }: NavItemsInput, + { + ofsted, admissions, admissionDistance, + admissionDistanceHistory, admissionsHistory, yearlyDataLength, + }: NavItemsInput, ): NavItem[] { const navItems: NavItem[] = []; if (ofsted) navItems.push({ id: 'ofsted', label: 'Ofsted' }); @@ -147,6 +156,9 @@ export function buildNavItems( }); } if (admissions || admissionDistance) navItems.push({ id: 'admissions', label: 'Admissions' }); + if (hasCutoffDetail(cutoffYearRows(admissionDistanceHistory, admissionsHistory))) { + navItems.push({ id: 'distance', label: 'Distance' }); + } if (flags.hasInclusionData) navItems.push({ id: 'inclusion', label: 'Pupils' }); if (yearlyDataLength > 0) navItems.push({ id: 'history', label: 'History' }); if (flags.hasSchoolLife) navItems.push({ id: 'school-life', label: 'School Life' }); @@ -216,12 +228,18 @@ export function computeSecondaryFlags({ */ export function buildSecondaryNavItems( flags: SecondaryFlags, - { ofsted, admissions, admissionDistance, yearlyDataLength }: NavItemsInput, + { + ofsted, admissions, admissionDistance, + admissionDistanceHistory, admissionsHistory, yearlyDataLength, + }: NavItemsInput, ): NavItem[] { const navItems: NavItem[] = []; if (ofsted) navItems.push({ id: 'ofsted', label: 'Ofsted' }); if (flags.hasResults) navItems.push({ id: 'gcse', label: 'GCSEs' }); if (admissions || admissionDistance) navItems.push({ id: 'admissions', label: 'Admissions' }); + if (hasCutoffDetail(cutoffYearRows(admissionDistanceHistory, admissionsHistory))) { + navItems.push({ id: 'distance', label: 'Distance' }); + } if (yearlyDataLength > 1) navItems.push({ id: 'history', label: 'History' }); if (flags.hasWellbeing) navItems.push({ id: 'wellbeing', label: 'Wellbeing' }); if (flags.hasFinance) navItems.push({ id: 'finances', label: 'Finances' });