From 4e0d8bcf877999b9a4423a836c60643c4becc471 Mon Sep 17 00:00:00 2001 From: Tudor Date: Mon, 21 Sep 2026 22:42:59 +0100 Subject: [PATCH] feat(web): render similar schools on both detail templates Inside SchoolDetailShell rather than after it, because the sticky nav's scroll-spy finds sections with getElementById and can only reach one that lives in the shell. Last in the order, and last in the nav, because the two must agree or the nav links to an anchor that was never rendered. hasSimilarSchools is optional on NavItemsInput, matching hasLocation beside it: absent has to mean "no section", and making it required would have churned ten unrelated call sites for no added safety. Co-Authored-By: Claude Opus 5 --- .../__tests__/lib/schoolSections.test.ts | 36 +++++++++++++++++++ .../app/(frontend)/school/[slug]/page.tsx | 6 ++++ .../school/PrimarySchoolSections.tsx | 15 +++++++- .../school/SecondarySchoolSections.tsx | 16 +++++++-- nextjs-app/lib/schoolSections.ts | 18 ++++++++-- 5 files changed, 86 insertions(+), 5 deletions(-) diff --git a/nextjs-app/__tests__/lib/schoolSections.test.ts b/nextjs-app/__tests__/lib/schoolSections.test.ts index acaec4e..fdd807a 100644 --- a/nextjs-app/__tests__/lib/schoolSections.test.ts +++ b/nextjs-app/__tests__/lib/schoolSections.test.ts @@ -172,3 +172,39 @@ describe('buildSecondaryNavItems', () => { expect(ids).not.toContain('history'); }); }); + +describe('the similar-schools nav item', () => { + const navInput = { + ofsted: null, admissions: null, admissionDistance: null, + hasLocation: true, yearlyDataLength: 1, + }; + + it('appears on both templates when the section renders', () => { + const primary = computeSchoolFlags(primaryFixture); + const secondary = computeSecondaryFlags(secondaryFixture); + const input = { ...navInput, hasSimilarSchools: true }; + + expect(buildNavItems(primary, input).map((i) => i.id)).toContain('similar'); + expect(buildSecondaryNavItems(secondary, input).map((i) => i.id)).toContain('similar'); + }); + + it('is absent when the section does not render', () => { + const primary = computeSchoolFlags(primaryFixture); + const secondary = computeSecondaryFlags(secondaryFixture); + const input = { ...navInput, hasSimilarSchools: false }; + + expect(buildNavItems(primary, input).map((i) => i.id)).not.toContain('similar'); + expect(buildSecondaryNavItems(secondary, input).map((i) => i.id)).not.toContain('similar'); + }); + + it('is absent when nothing says either way', () => { + const primary = computeSchoolFlags(primaryFixture); + expect(buildNavItems(primary, navInput).map((i) => i.id)).not.toContain('similar'); + }); + + it('comes last, because the section renders last', () => { + const primary = computeSchoolFlags(primaryFixture); + const ids = buildNavItems(primary, { ...navInput, hasSimilarSchools: true }).map((i) => i.id); + expect(ids[ids.length - 1]).toBe('similar'); + }); +}); diff --git a/nextjs-app/app/(frontend)/school/[slug]/page.tsx b/nextjs-app/app/(frontend)/school/[slug]/page.tsx index fe0f4af..23b98e8 100644 --- a/nextjs-app/app/(frontend)/school/[slug]/page.tsx +++ b/nextjs-app/app/(frontend)/school/[slug]/page.tsx @@ -8,6 +8,7 @@ import { APIFetchError, fetchSchoolDetails, fetchSchools, fetchNationalAverages import { notFound, redirect } from 'next/navigation'; import { SchoolDetailShell } from '@/components/school/SchoolDetailShell'; import { NearbyPlaces } from '@/components/school/NearbyPlaces'; +import { shouldRenderSimilar } from '@/components/school/SimilarSchoolsSection'; import { schoolBreadcrumbJsonLd, type SchoolPlace } from '@/lib/jsonld'; import { PrimarySchoolSections } from '@/components/school/PrimarySchoolSections'; import { SecondarySchoolSections } from '@/components/school/SecondarySchoolSections'; @@ -155,6 +156,8 @@ export default async function SchoolPage({ params }: SchoolPageProps) { // nothing rather than throwing, which is how this shipped without a // lockstep deploy of the two images. const places: SchoolPlace[] = data.places ?? []; + // Absent on an older API build, exactly like `places` above. + const similarSchools = data.similar_schools ?? []; // Redirect bare URN to canonical slug URL const canonicalSlug = schoolUrl(urn, school_info.school_name).replace('/school/', ''); @@ -186,6 +189,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { admissions: admissions ?? null, admissionDistance: admission_distance ?? null, hasLocation: school_info.latitude != null && school_info.longitude != null, + hasSimilarSchools: shouldRenderSimilar(similarSchools), yearlyDataLength: yearly_data.length, }; const primaryNavItems = buildNavItems(primaryFlags, navInput); @@ -262,6 +266,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { finance={finance ?? null} nationalAvg={nationalAvg} destinations={destinations ?? null} + similarSchools={similarSchools} flags={secondaryFlags} /> @@ -284,6 +289,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { deprivation={deprivation ?? null} finance={finance ?? null} nationalAvg={nationalAvg} + similarSchools={similarSchools} flags={primaryFlags} /> diff --git a/nextjs-app/components/school/PrimarySchoolSections.tsx b/nextjs-app/components/school/PrimarySchoolSections.tsx index 1eeabf5..4898c78 100644 --- a/nextjs-app/components/school/PrimarySchoolSections.tsx +++ b/nextjs-app/components/school/PrimarySchoolSections.tsx @@ -14,6 +14,7 @@ import type { School, SchoolResult, AbsenceData, OfstedInspection, SchoolCensus, SchoolAdmissions, SchoolAdmissionDistance, SchoolDeprivation, SchoolFinance, NationalAverages, + SimilarSchool, } from '@/lib/types'; import { ofstedLegacyAreas } from '@/lib/utils'; import type { SchoolFlags } from '@/lib/schoolSections'; @@ -26,6 +27,7 @@ import { HistorySection } from './HistorySection'; import { SchoolLifeSection } from './SchoolLifeSection'; import { LocalAreaSection } from './LocalAreaSection'; import { FinancesSection } from './FinancesSection'; +import { SimilarSchoolsSection } from './SimilarSchoolsSection'; export interface PrimarySchoolSectionsProps { schoolInfo: School; @@ -39,13 +41,15 @@ export interface PrimarySchoolSectionsProps { deprivation: SchoolDeprivation | null; finance: SchoolFinance | null; nationalAvg: NationalAverages | null; + /** Nearby schools of a comparable intake. Absent on an older API build. */ + similarSchools?: SimilarSchool[]; flags: SchoolFlags; } export function PrimarySchoolSections({ schoolInfo, yearlyData, absenceData, ofsted, census, admissions, admissionsHistory, admissionDistance, - deprivation, finance, nationalAvg, flags, + deprivation, finance, nationalAvg, similarSchools, flags, }: PrimarySchoolSectionsProps) { const primaryAvg = nationalAvg?.primary ?? {}; const secondaryAvg = nationalAvg?.secondary ?? {}; @@ -146,6 +150,15 @@ export function PrimarySchoolSections({ )} {flags.hasFinance && finance && } + + {/* Last: it is where the reader goes next, not part of this school. */} + ); } diff --git a/nextjs-app/components/school/SecondarySchoolSections.tsx b/nextjs-app/components/school/SecondarySchoolSections.tsx index 68f8ba9..6cc2a25 100644 --- a/nextjs-app/components/school/SecondarySchoolSections.tsx +++ b/nextjs-app/components/school/SecondarySchoolSections.tsx @@ -14,7 +14,7 @@ import type { School, SchoolResult, AbsenceData, OfstedInspection, SchoolCensus, SchoolAdmissions, SchoolAdmissionDistance, SchoolDeprivation, SchoolFinance, NationalAverages, - SchoolDestinations, + SchoolDestinations, SimilarSchool, } from '@/lib/types'; import { ofstedLegacyAreas } from '@/lib/utils'; import type { SecondaryFlags } from '@/lib/schoolSections'; @@ -27,6 +27,7 @@ import { DistanceSection } from './DistanceSection'; import { SecondaryHistorySection } from './SecondaryHistorySection'; import { WellbeingSection } from './WellbeingSection'; import { FinancesSection } from './FinancesSection'; +import { SimilarSchoolsSection } from './SimilarSchoolsSection'; import styles from './schoolSections.module.css'; export interface SecondarySchoolSectionsProps { @@ -47,13 +48,15 @@ export interface SecondarySchoolSectionsProps { finance: SchoolFinance | null; nationalAvg: NationalAverages | null; destinations: SchoolDestinations | null; + /** Nearby schools of a comparable intake. Absent on an older API build. */ + similarSchools?: SimilarSchool[]; flags: SecondaryFlags; } export function SecondarySchoolSections({ schoolInfo, yearlyData, ofsted, census, admissions, admissionsHistory, admissionDistance, - deprivation, finance, nationalAvg, destinations, flags, + deprivation, finance, nationalAvg, destinations, similarSchools, flags, }: SecondarySchoolSectionsProps) { const secondaryAvg = nationalAvg?.secondary ?? {}; @@ -141,6 +144,15 @@ export function SecondarySchoolSections({ {flags.hasFinance && finance && ( )} + + {/* Last: it is where the reader goes next, not part of this school. */} + ); } diff --git a/nextjs-app/lib/schoolSections.ts b/nextjs-app/lib/schoolSections.ts index eeec922..fedc0be 100644 --- a/nextjs-app/lib/schoolSections.ts +++ b/nextjs-app/lib/schoolSections.ts @@ -128,6 +128,10 @@ export interface NavItemsInput { * measure a postcode, so the nav must gate on them too or it will link to an * anchor that was never rendered. */ hasLocation?: boolean; + /** Whether the similar-schools section will render. Optional for the same + * reason hasLocation is: the nav must never link to an anchor that was not + * rendered, and absent has to mean "no section". */ + hasSimilarSchools?: boolean; yearlyDataLength: number; } @@ -142,7 +146,10 @@ export interface NavItemsInput { */ export function buildNavItems( flags: SchoolFlags, - { ofsted, admissions, admissionDistance, hasLocation, yearlyDataLength }: NavItemsInput, + { + ofsted, admissions, admissionDistance, hasLocation, + hasSimilarSchools, yearlyDataLength, + }: NavItemsInput, ): NavItem[] { const navItems: NavItem[] = []; if (ofsted) navItems.push({ id: 'ofsted', label: 'Ofsted' }); @@ -161,6 +168,8 @@ export function buildNavItems( if (flags.hasSchoolLife) navItems.push({ id: 'school-life', label: 'School Life' }); if (flags.hasDeprivation) navItems.push({ id: 'local-area', label: 'Local Area' }); if (flags.hasFinance) navItems.push({ id: 'finances', label: 'Finances' }); + // Last, because the section renders last — the scroll-spy reads this order. + if (hasSimilarSchools) navItems.push({ id: 'similar', label: 'Similar schools' }); return navItems; } @@ -239,7 +248,10 @@ export function computeSecondaryFlags({ */ export function buildSecondaryNavItems( flags: SecondaryFlags, - { ofsted, admissions, admissionDistance, hasLocation, yearlyDataLength }: NavItemsInput, + { + ofsted, admissions, admissionDistance, hasLocation, + hasSimilarSchools, yearlyDataLength, + }: NavItemsInput, ): NavItem[] { const navItems: NavItem[] = []; if (ofsted) navItems.push({ id: 'ofsted', label: 'Ofsted' }); @@ -257,5 +269,7 @@ export function buildSecondaryNavItems( 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' }); + // Last, because the section renders last — the scroll-spy reads this order. + if (hasSimilarSchools) navItems.push({ id: 'similar', label: 'Similar schools' }); return navItems; }