From 7199c3a90b9691d9b75ef69c0611780143215af2 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sat, 1 Aug 2026 20:52:56 +0100 Subject: [PATCH] docs(perf): implementation plan for detail-page server/client split Nine tasks mapping to the spec's commit sequence, each ending in an independently testable deliverable. Also corrects the spec's section-sharing table: measured similarity shows Admissions (14%) and History (40%) are not shareable between the two views, only Ofsted (80%) and Finances (91%). This costs no bytes -- the win comes from sections being server components, not from sharing them -- and cuts the diff and regression risk. Co-Authored-By: Claude Opus 5 --- .../plans/2026-08-01-server-client-split.md | 950 ++++++++++++++++++ .../2026-07-30-server-client-split-design.md | 30 +- 2 files changed, 975 insertions(+), 5 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-01-server-client-split.md diff --git a/docs/superpowers/plans/2026-08-01-server-client-split.md b/docs/superpowers/plans/2026-08-01-server-client-split.md new file mode 100644 index 0000000..8b42c64 --- /dev/null +++ b/docs/superpowers/plans/2026-08-01-server-client-split.md @@ -0,0 +1,950 @@ +# Detail-page Server/Client Split Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop ~750 lines of static school-detail markup from shipping to the browser as JavaScript, by turning the page's sections into React Server Components behind a small client shell. + +**Architecture:** A server component imported by a client component becomes a client component. So the sections cannot be children of a client `SchoolDetailView`. Instead `app/school/[slug]/page.tsx` (server) composes the sections and passes them *through* a small client shell as `children` — the pattern already used at `app/page.tsx:98-99`. The shell keeps the genuinely interactive chrome (back link, header reveal, hero map, compare CTA, sticky nav, scroll-spy); everything below it becomes server-rendered. + +**Tech Stack:** Next.js 16.1.6 (App Router, Turbopack), React 19, TypeScript, CSS Modules, Jest + React Testing Library, Playwright (e2e). + +**Spec:** `docs/superpowers/specs/2026-07-30-server-client-split-design.md` + +## Global Constraints + +- Branch is `perf/server-client-split`, already created off `origin/main` at `cab7b4f`. Never push to `main`; this lands as a PR (CLAUDE.md SDLC). +- All paths below are relative to the repo root. The Next.js app lives in `nextjs-app/`; run all `npm` commands from there. +- **Do not start a local server to test** — it does not work in this environment (CLAUDE.md). Verification is `npm run typecheck`, `npm test`, `npm run build`. +- **No visual or behavioural change** on any of the four school shapes, except the two documented CSS merges. No redesign, no copy changes. +- Commits 6 and 7 are **pure moves**: JSX is relocated verbatim, changing only the `styles` import and prop plumbing. If you find a bug mid-move, record it in the PR description — do not fix it inline. +- Characterization tests written in Task 2 must pass **unmodified** through Tasks 3-8. The only file permitted to change is `__tests__/support/renderSchoolDetail.tsx`. +- End commit messages with `Co-Authored-By: Claude Opus 5 `. + +--- + +## What is shared and what is not + +Measured similarity between the two views' sections (`difflib.SequenceMatcher` on stripped lines): + +| Section | Primary lines | Secondary lines | Similarity | Decision | +| --- | --- | --- | --- | --- | +| Ofsted | 88 | 107 | 80% | **Shared** — `OfstedSection` | +| Finances | 29 | 35 | 91% | **Shared** — `FinancesSection` | +| History | 121 | 47 | 40% | Separate components | +| Admissions | 89 | 55 | 14% | Separate components | + +The spec's section-sharing table listed Admissions and History as shared; measurement shows they are not. **This costs no bytes** — the client-JS win comes from a section being a *server* component, not from being shared. Sharing is a maintainability bonus, taken only where it is cheap. + +## File Structure + +**Create — `nextjs-app/lib/`** +- `schoolSections.ts` — pure derived flags (`hasInclusionData`, `hasKS2Results`, `suppressKs2Comparison`, …) and `buildNavItems()`. No JSX, no React. Unit-testable in isolation; consumed by `page.tsx` and the sections. + +**Create — `nextjs-app/components/school/`** (mirrors the existing `components/compare/` layout) +- `schoolSections.module.css` — merged stylesheet (Task 5) +- `sectionShared.tsx` — `Section`, `MetricGrid`, `MetricCard`, `StatRow` primitives, all server +- `OfstedSection.tsx`, `FinancesSection.tsx` — shared, server +- `ResultsSection.tsx`, `AdmissionsSection.tsx`, `InclusionSection.tsx`, `HistorySection.tsx`, `SchoolLifeSection.tsx`, `LocalAreaSection.tsx` — primary, server +- `GcseSection.tsx`, `SecondaryAdmissionsSection.tsx`, `SecondaryHistorySection.tsx`, `WellbeingSection.tsx` — secondary, server +- `AdmissionsViewToggle.tsx` — **the only new client component** +- `PrimarySchoolSections.tsx`, `SecondarySchoolSections.tsx` — server composers +- `SchoolDetailShell.tsx` + `SchoolDetailShell.module.css` — the client shell + +**Create — `nextjs-app/__tests__/`** +- `support/schoolFixtures.ts` — four fixtures: primary, secondary, all-through, special +- `support/renderSchoolDetail.tsx` — the swappable render helper +- `components/schoolDetail.characterization.test.tsx` +- `lib/schoolSections.test.ts` + +**Modify** +- `nextjs-app/jest.setup.js` — `IntersectionObserver`, `window.scrollTo` +- `nextjs-app/app/school/[slug]/page.tsx` — fetch national averages, compose sections +- `nextjs-app/package.json` — drop `swr` +- `e2e/tests/journeys.spec.ts` — detail-page assertions + +**Deliberately untouched** +- `components/Navigation.tsx` — genuinely needs client (`usePathname` plus the comparison count). No task. +- `components/Footer.tsx` — already a server component; no `'use client'` directive. No task. +- `app/layout.tsx` — unchanged. The shared ~172 KB baseline is out of scope; see Task 9 Step 4. + +**Delete** +- `nextjs-app/components/SchoolDetailView.tsx` + `.module.css` +- `nextjs-app/components/SecondarySchoolDetailView.tsx` + `.module.css` +- `nextjs-app/hooks/useSchools.ts`, `useFilters.ts`, `useMetrics.ts`, `useSchoolDetails.ts` + +--- + +### Task 1: Test environment and fixtures + +**Files:** +- Modify: `nextjs-app/jest.setup.js` +- Create: `nextjs-app/__tests__/support/schoolFixtures.ts` + +**Interfaces:** +- Consumes: nothing +- Produces: `primaryFixture`, `secondaryFixture`, `allThroughFixture`, `specialFixture` — each an object matching the prop shape of `SchoolDetailView` (`{ schoolInfo, yearlyData, absenceData, ofsted, census, admissions, admissionsHistory, deprivation, finance }`), plus `nationalAveragesFixture` typed `NationalAverages`. + +- [ ] **Step 1: Add the missing jsdom globals** + +jsdom implements neither API; both detail views use `IntersectionObserver` twice and `window.scrollTo` once. Append to `nextjs-app/jest.setup.js`: + +```javascript +// jsdom implements neither of these; the detail views use both. +global.IntersectionObserver = class { + constructor(callback) { this.callback = callback; } + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { return []; } +}; + +window.scrollTo = jest.fn(); +``` + +- [ ] **Step 2: Build the fixtures** + +Create `nextjs-app/__tests__/support/schoolFixtures.ts`. Derive the exact field names from `nextjs-app/lib/types.ts` (`School`, `SchoolResult`, `OfstedInspection`, `SchoolCensus`, `SchoolAdmissions`, `SchoolDeprivation`, `SchoolFinance`, `NationalAverages`) — do not invent fields; TypeScript will reject them. + +The four fixtures must differ in exactly these ways, because these are the branches the characterization tests exercise: + +| Fixture | `schoolInfo.phase` | Results present | Ofsted | Admissions history | Notes | +| --- | --- | --- | --- | --- | --- | +| `primaryFixture` | `'Primary'` | `rwm_expected_pct: 58` | legacy OEIF, `overall_effectiveness: 2` | 3 years with `first_preference_offer_pct` | drives the trend toggle | +| `secondaryFixture` | `'Secondary'` | `attainment_8_score: 46.2`, `progress_8_score: 0.31` | Report Card (`rc_*` fields set) | 1 year only | no trend toggle | +| `allThroughFixture` | `'All-through'` | **both** `rwm_expected_pct` and `attainment_8_score` | legacy OEIF | 2 years | KS2 content gated back on | +| `specialFixture` | `'Primary'`, `school_type: 'Community special school'` | `rwm_expected_pct: 0`, reading/writing/maths all `0` | none (`null`) | none (`null`) | England comparison suppressed | + +Every fixture needs `urn` in the valid 6-digit range, a `school_name`, and a `local_authority`. Give `primaryFixture` at least 3 entries in `yearlyData` so the history table and chart have data. + +- [ ] **Step 3: Verify the fixtures typecheck** + +Run: `cd nextjs-app && npm run typecheck` +Expected: PASS. Any error here means a fixture field does not exist on the real type — fix the fixture, never the type. + +- [ ] **Step 4: Commit** + +```bash +git add nextjs-app/jest.setup.js nextjs-app/__tests__/support/schoolFixtures.ts +git commit -m "$(cat <<'EOF' +test: add jsdom globals and school detail fixtures + +jsdom provides neither IntersectionObserver nor scrollTo; both detail views +use them. Adds four fixtures (primary, secondary, all-through, special) +covering the branches the characterization tests will pin. + +Co-Authored-By: Claude Opus 5 +EOF +)" +``` + +--- + +### Task 2: Characterization tests + +This is the safety net. **It must be green before any production code moves.** + +**Files:** +- Create: `nextjs-app/__tests__/support/renderSchoolDetail.tsx` +- Create: `nextjs-app/__tests__/components/schoolDetail.characterization.test.tsx` + +**Interfaces:** +- Consumes: fixtures from Task 1. +- Produces: `renderSchoolDetail(fixture, opts?)` and `renderSecondarySchoolDetail(fixture, opts?)`. These are the **only** functions the tests call to render. Task 7 rewrites their bodies; their signatures and every assertion stay frozen. + +- [ ] **Step 1: Write the render helper against the CURRENT components** + +Create `nextjs-app/__tests__/support/renderSchoolDetail.tsx`: + +```tsx +/** + * The single seam between the characterization tests and the component tree. + * + * Task 7 rewrites the bodies of these two functions to render the new + * shell + server-sections composition. Nothing else in the test suite may + * change — assertions passing unmodified across that rewrite is the proof + * that behaviour was preserved. + */ +import { render } from '@testing-library/react'; +import { SchoolDetailView } from '@/components/SchoolDetailView'; +import { SecondarySchoolDetailView } from '@/components/SecondarySchoolDetailView'; + +export function renderSchoolDetail(fixture: any) { + return render(); +} + +export function renderSecondarySchoolDetail(fixture: any) { + return render(); +} +``` + +- [ ] **Step 2: Write the characterization tests** + +Create `nextjs-app/__tests__/components/schoolDetail.characterization.test.tsx`. Mock the charts and analytics at the top — Chart.js needs a canvas jsdom does not provide, and `track()` fires on mount: + +```tsx +jest.mock('@/lib/analytics', () => ({ + track: jest.fn(), + getNavigationSource: () => 'direct', +})); +jest.mock('@/components/PerformanceChart', () => ({ + PerformanceChart: () =>
, +})); +jest.mock('@/components/SatsChart', () => ({ + __esModule: true, + default: () =>
, +})); +jest.mock('@/components/AdmissionsTrendChart', () => ({ + __esModule: true, + default: () =>
, +})); +jest.mock('@/components/SchoolHeroMap', () => ({ + SchoolHeroMap: () =>
, +})); +``` + +Then cover, using `renderSchoolDetail` / `renderSecondarySchoolDetail` only: + +1. **Ofsted, legacy OEIF** (`primaryFixture`) — the grade label `Good` renders; the inspected date renders; the `reports.ofsted.gov.uk` link carries the fixture's URN in its `href`. +2. **Ofsted, Report Card** (`secondaryFixture`) — heading reads `Ofsted Report Card`; the November 2025 disclaimer renders; each populated `rc_*` category renders its label and band. +3. **KS2 results + England delta** (`primaryFixture`) — the RWM percentage renders; a delta versus the England average renders. +4. **KS4 results** (`secondaryFixture`) — Attainment 8 and Progress 8 values render; EBacc rows render. +5. **All-through** (`allThroughFixture`) — **both** a KS2 figure and an Attainment 8 figure render in the same document. This is the highest-value assertion in the file; it is the case most likely to break. +6. **Special school** (`specialFixture`) — no England-comparison delta renders, and the special-school note renders. Guards the PR #70 regression. +7. **Admissions toggle** (`primaryFixture`) — both buttons render; the trend view starts `hidden`; after `userEvent.click` on the trend button the trend view is visible and the year view is `hidden`. Assert on the `hidden` attribute, matching how the current code toggles. +8. **Admissions without trend** (`secondaryFixture`, 1 year) — no toggle buttons render. +9. **Nav items** (`primaryFixture`) — the expected section ids are present in the document; and for `specialFixture` (no ofsted, no admissions) those two are absent. + +Use `getByText` / `queryByText` / `getByRole` — never snapshots, and never assert on CSS-module class names, which are hashed and will change. + +- [ ] **Step 3: Run the tests and confirm they PASS** + +Run: `cd nextjs-app && npm test -- schoolDetail.characterization` +Expected: **PASS.** This is the inverse of normal TDD — these tests describe behaviour that already exists, so green means they correctly pin the current behaviour. If one fails, the test is wrong, not the component. Fix the test. + +- [ ] **Step 4: Commit** + +```bash +git add nextjs-app/__tests__/ +git commit -m "$(cat <<'EOF' +test: pin school detail behaviour before refactor + +Characterization tests covering Ofsted (both layouts), KS2/KS4 results, +all-through dual rendering, special-school comparison suppression, and the +admissions toggle. All rendering goes through renderSchoolDetail(), the one +seam that the server/client split is allowed to change. + +Co-Authored-By: Claude Opus 5 +EOF +)" +``` + +--- + +### Task 3: Move national averages to the server + +The prerequisite for everything after it: `primaryAvg` / `secondaryAvg` feed the England deltas across most sections, and while they arrive via `useEffect` those sections cannot be server components. + +**Files:** +- Modify: `nextjs-app/app/school/[slug]/page.tsx` +- Modify: `nextjs-app/components/SchoolDetailView.tsx:173-179` +- Modify: `nextjs-app/components/SecondarySchoolDetailView.tsx:91-93` + +**Interfaces:** +- Consumes: `fetchNationalAverages()` from `@/lib/api` (already exists, `revalidate: 3600`). +- Produces: both view components gain a required prop `nationalAvg: NationalAverages | null`. + +- [ ] **Step 1: Fetch it in parallel in the page** + +In `app/school/[slug]/page.tsx`, replace the lone `fetchSchoolDetails` call in the default export with a parallel pair. Import `fetchNationalAverages` alongside the existing imports. National averages must never break the page, so it degrades to `null`: + +```tsx +let data; +let nationalAvg: NationalAverages | null = null; +try { + [data, nationalAvg] = await Promise.all([ + fetchSchoolDetails(urn), + fetchNationalAverages().catch(() => null), + ]); +} catch (error) { + console.error(`Failed to fetch school ${urn}:`, error); + notFound(); +} +``` + +Leave `generateMetadata`'s own `fetchSchoolDetails(urn)` call alone — Next.js memoises identical `fetch` calls within a render pass, so it does not cost a second request. + +- [ ] **Step 2: Pass it to both views** + +Add `nationalAvg={nationalAvg}` to both the `` and `` elements at the bottom of the page. + +- [ ] **Step 3: Accept the prop and delete the useEffect** + +In `components/SchoolDetailView.tsx`: add `nationalAvg: NationalAverages | null;` to `SchoolDetailViewProps`, add `nationalAvg` to the destructured parameters, and delete lines 172-179 — the `useState` and the `useEffect` that fetches `/api/national-averages`. The two lines that follow it stay exactly as they are: + +```tsx +const primaryAvg = nationalAvg?.primary ?? {}; +const secondaryAvg = nationalAvg?.secondary ?? {}; +``` + +Apply the identical change to `components/SecondarySchoolDetailView.tsx` at lines 91-93. + +- [ ] **Step 4: Update the render helper to supply the prop** + +In `__tests__/support/renderSchoolDetail.tsx`, spread `nationalAveragesFixture` in: + +```tsx +export function renderSchoolDetail(fixture: any) { + return render(); +} +``` + +This is a permitted change — the helper is the designated seam. The test file itself must not change. + +- [ ] **Step 5: Verify** + +Run: `cd nextjs-app && npm run typecheck && npm test -- schoolDetail.characterization` +Expected: both PASS, with the characterization test file unmodified. + +- [ ] **Step 6: Commit** + +```bash +git add nextjs-app/app/school nextjs-app/components/SchoolDetailView.tsx \ + nextjs-app/components/SecondarySchoolDetailView.tsx nextjs-app/__tests__/support/ +git commit -m "$(cat <<'EOF' +refactor(detail): fetch national averages on the server + +Both detail views fetched /api/national-averages in a useEffect, so the +England-comparison deltas popped in after hydration and the sections that +use them could not become server components. The page now fetches it in +parallel with the school details (backend-cached 1h) and passes it down. + +Removes one client round-trip per detail page. + +Co-Authored-By: Claude Opus 5 +EOF +)" +``` + +--- + +### Task 4: Extract the derived-flags module + +Pulls the pure data-shape logic out of the components so `page.tsx` can compute `navItems` on the server without importing a client component. + +**Files:** +- Create: `nextjs-app/lib/schoolSections.ts` +- Create: `nextjs-app/__tests__/lib/schoolSections.test.ts` + +**Interfaces:** +- Consumes: types from `@/lib/types`; `isSpecialSchool` from `@/lib/utils`. +- Produces: + - `type SchoolFlags = { isAllThrough: boolean; isSecondary: boolean; isPrimary: boolean; latestResults: SchoolResult | null; hasGenderSplit: boolean; hasInclusionData: boolean; hasSchoolLife: boolean; hasDeprivation: boolean; hasFinance: boolean; hasLocation: boolean; hasKS2Results: boolean; hasKS4Results: boolean; hasAnyResults: boolean; isSpecial: boolean; ks2Placeholder: boolean; suppressKs2Comparison: boolean; suppressKs4Comparison: boolean }` + - `computeSchoolFlags(input): SchoolFlags` where `input` is `{ schoolInfo, yearlyData, absenceData, census, deprivation, finance }` + - `type NavItem = { id: string; label: string }` + - `buildNavItems(flags: SchoolFlags, opts: { ofsted: OfstedInspection | null; admissions: SchoolAdmissions | null; yearlyDataLength: number }): NavItem[]` + +- [ ] **Step 1: Write the failing tests** + +Create `nextjs-app/__tests__/lib/schoolSections.test.ts` covering the branches that matter: + +```ts +import { computeSchoolFlags, buildNavItems } from '@/lib/schoolSections'; +import { primaryFixture, allThroughFixture, specialFixture } from '../support/schoolFixtures'; + +describe('computeSchoolFlags', () => { + it('treats an all-through school as both primary-capable and secondary', () => { + const f = computeSchoolFlags(allThroughFixture); + expect(f.isAllThrough).toBe(true); + expect(f.isSecondary).toBe(true); + expect(f.hasKS2Results).toBe(true); + expect(f.hasKS4Results).toBe(true); + }); + + it('suppresses the KS2 comparison for a special school', () => { + const f = computeSchoolFlags(specialFixture); + expect(f.isSpecial).toBe(true); + expect(f.suppressKs2Comparison).toBe(true); + }); + + it('treats an all-zero KS2 row as a placeholder', () => { + const f = computeSchoolFlags(specialFixture); + expect(f.ks2Placeholder).toBe(true); + }); + + it('does not suppress comparison for an ordinary primary', () => { + const f = computeSchoolFlags(primaryFixture); + expect(f.suppressKs2Comparison).toBe(false); + }); +}); + +describe('buildNavItems', () => { + it('omits sections with no data', () => { + const flags = computeSchoolFlags(specialFixture); + const ids = buildNavItems(flags, { + ofsted: null, admissions: null, yearlyDataLength: 1, + }).map((n) => n.id); + expect(ids).not.toContain('ofsted'); + expect(ids).not.toContain('admissions'); + }); + + it('labels the results section by phase', () => { + const primary = computeSchoolFlags(primaryFixture); + const items = buildNavItems(primary, { + ofsted: primaryFixture.ofsted, admissions: primaryFixture.admissions, + yearlyDataLength: primaryFixture.yearlyData.length, + }); + expect(items.find((n) => n.id === 'results')?.label).toBe('SATs'); + }); +}); +``` + +- [ ] **Step 2: Run to verify they fail** + +Run: `cd nextjs-app && npm test -- schoolSections` +Expected: FAIL — `Cannot find module '@/lib/schoolSections'`. + +- [ ] **Step 3: Implement the module** + +Create `nextjs-app/lib/schoolSections.ts` by moving the logic verbatim from `components/SchoolDetailView.tsx` lines **211-267**. Lines 205-209 are the `deprivationDesc` helper — leave it where it is; Task 6 moves it into `LocalAreaSection`. Also bring `latestResults` (line 160). Preserve the comments — they explain the special-school and all-through reasoning and are load-bearing for future readers. The `navItems` push order must stay exactly as it is at lines 258-267; it is engagement-ordered from analytics. + +- [ ] **Step 4: Run to verify they pass** + +Run: `cd nextjs-app && npm test -- schoolSections` +Expected: PASS. + +- [ ] **Step 5: Consume it from both views** + +Replace the inline flag computations in `SchoolDetailView.tsx` and `SecondarySchoolDetailView.tsx` with a call to `computeSchoolFlags` / `buildNavItems`, destructuring the flags so the JSX below is untouched. + +- [ ] **Step 6: Verify nothing regressed** + +Run: `cd nextjs-app && npm run typecheck && npm test` +Expected: all PASS, characterization tests unmodified. + +- [ ] **Step 7: Commit** + +```bash +git add nextjs-app/lib/schoolSections.ts nextjs-app/__tests__/lib/schoolSections.test.ts \ + nextjs-app/components/SchoolDetailView.tsx nextjs-app/components/SecondarySchoolDetailView.tsx +git commit -m "$(cat <<'EOF' +refactor(detail): extract derived flags and nav items to lib + +Pure data-shape logic moves out of the client components so page.tsx can +compute the section list on the server. Adds direct unit coverage for the +all-through and special-school branches, which were previously only +reachable through a full component render. + +Co-Authored-By: Claude Opus 5 +EOF +)" +``` + +--- + +### Task 5: Merge the stylesheets + +Isolated so that a visual regression bisects to the CSS merge rather than to a JSX move. Highest-risk task in the plan; no test catches a mistake here. + +**Files:** +- Create: `nextjs-app/components/school/schoolSections.module.css` + +**Interfaces:** +- Consumes: nothing. +- Produces: a stylesheet exporting every class name used by section markup in either view, plus `genderBarSecondary` and `heroStatValueSecondary` variant classes. + +- [ ] **Step 1: Copy the primary stylesheet as the base** + +```bash +mkdir -p nextjs-app/components/school +cp nextjs-app/components/SchoolDetailView.module.css \ + nextjs-app/components/school/schoolSections.module.css +``` + +- [ ] **Step 2: Add the secondary-only classes** + +33 class names exist only in `SecondarySchoolDetailView.module.css`. Append their rules unchanged. Confirm the list: + +```bash +cd nextjs-app && python3 - <<'PY' +import re +c=lambda p: set(re.findall(r'\.([a-zA-Z][\w-]*)', open(p).read())) +a=c('components/SchoolDetailView.module.css'); b=c('components/SecondarySchoolDetailView.module.css') +print('\n'.join(sorted(b-a))) +PY +``` + +- [ ] **Step 3: Union the 14 incidental-drift differences** + +For these classes, add any property present in the secondary rule but missing from the primary rule. All are defensive overflow properties — `min-width: 0`, `overflow-wrap: break-word`, `word-break: break-word`, `max-width: 100%`, `-webkit-overflow-scrolling: touch`. Never remove a property the primary rule already has. + +Affected: `.card`, `.sectionTitle`, `.metricCard`, `.metricValue`, `.metricsGrid`, `.tableWrapper`, `.heroStatCard`, `.heroStatLabel`, `.historyToggle`, `.ofstedReportLink`, `.genderSplitBoys`, `.genderSplitGirls`, `.genderSplitSep`, and the remaining drift class from the audit in Step 5. + +- [ ] **Step 4: Add variant classes for the two real differences** + +Keep the primary look on the base class and add the secondary look as a separate class: + +```css +/* Secondary detail pages render a slimmer gender bar on a translucent track. + Kept as an explicit variant rather than reconciled, so the divergence is + deliberate instead of accidental drift. */ +.genderBarSecondary { + height: 4px; + background: rgba(0, 0, 0, 0.08); + margin-top: 0.45rem; +} + +/* Secondary hero stats sit tighter and omit the Georgia fallback. */ +.heroStatValueSecondary { + gap: 0.4rem; + font-family: var(--font-playfair), "Playfair Display", serif; +} +``` + +- [ ] **Step 5: Audit the merge** + +Every shared class must now be a superset of both originals, except the two variants: + +```bash +cd nextjs-app && python3 - <<'PY' +import re +def rules(p): + out={} + for m in re.finditer(r'\.([a-zA-Z][\w-]*)\s*\{([^{}]*)\}', open(p).read()): + out.setdefault(m.group(1),[]).append({x.strip() for x in + re.sub(r'\s+',' ',m.group(2)).strip().rstrip(';').split(';') if x.strip()}) + return out +m=rules('components/school/schoolSections.module.css') +for src in ['components/SchoolDetailView.module.css','components/SecondarySchoolDetailView.module.css']: + for name, bodies in rules(src).items(): + if name in ('genderBar','heroStatValue'): continue + if name not in m: print(f"MISSING .{name} (from {src})"); continue + missing = bodies[0] - m[name][0] + if missing: print(f"LOST .{name}: {missing} (from {src})") +print("audit complete") +PY +``` +Expected: no `MISSING` or `LOST` lines before `audit complete`. + +- [ ] **Step 6: Commit** + +```bash +git add nextjs-app/components/school/schoolSections.module.css +git commit -m "$(cat <<'EOF' +style(detail): merge the two detail stylesheets + +Unions the 14 incidental-drift differences between the primary and secondary +modules -- defensive overflow properties (min-width, overflow-wrap, +max-width) that were fixed on one page and never back-ported. Keeps the two +genuine visual differences, .genderBar and .heroStatValue, as explicit +variant classes. + +No intended visual change. Isolated so a regression bisects here rather than +to a JSX move. + +Co-Authored-By: Claude Opus 5 +EOF +)" +``` + +--- + +### Task 6: Extract the server sections + +The bulk of the work, and a **pure move**. Every section below becomes a server component: no `'use client'`, no hooks, no event handlers. + +**Files:** +- Create: `nextjs-app/components/school/sectionShared.tsx` +- Create: the 12 section files listed in File Structure +- Create: `nextjs-app/components/school/AdmissionsViewToggle.tsx` + +**Interfaces:** +- Consumes: `SchoolFlags` and `NavItem` from `@/lib/schoolSections`; `schoolSections.module.css`. +- Produces: each section is a named export taking exactly the props its JSX reads. `AdmissionsViewToggle` is the only `'use client'` file created here, with props `{ title: ReactNode; subtitle: ReactNode; trendLabel: string; yearView: ReactNode; trendView: ReactNode }`. + +- [ ] **Step 1: Extract the shared primitives** + +Create `sectionShared.tsx` mirroring `components/compare/sectionShared.tsx`. Export server components `Section` (renders `
`), `SectionTitle`, `MetricGrid`, `MetricCard`. Re-export the stylesheet as `sectionStyles`, matching the compare module's convention. + +- [ ] **Step 2: Move the shared sections** + +These are the only two sections where you are merging two implementations rather than moving one, so the reconciliation rule matters. **Start from the primary version and add only what the secondary version has that the primary lacks.** Diff them first so you are working from the actual delta rather than reading both in full: + +```bash +cd nextjs-app && diff <(sed -n '598,687p' components/SchoolDetailView.tsx | sed 's/^[ \t]*//') \ + <(sed -n '395,502p' components/SecondarySchoolDetailView.tsx | sed 's/^[ \t]*//') +``` + +For every line the secondary has and the primary does not, decide one of two things and write the decision as a code comment: either it is **phase-specific** (gate it on a prop — e.g. `showSixthForm`), or it is **an improvement the primary never received** (keep it unconditionally). Never delete a branch the primary version has. If a difference is neither — a genuine behavioural conflict — stop and report it rather than guessing. + +- `OfstedSection.tsx` — primary `SchoolDetailView.tsx:598-687`, secondary `SecondarySchoolDetailView.tsx:395-502` (80% similar). Props: `{ ofsted, urn, isReportCard, ofstedInspectedDate, oeifAllSameGrade, oeifAreas }`. Note `RC_CATEGORIES` (lines 44-53) includes `rc_sixth_form`, which only ever populates for secondary — it is already null-guarded, so it needs no prop. +- `FinancesSection.tsx` — primary `SchoolDetailView.tsx:1314-1344`, secondary `SecondarySchoolDetailView.tsx:937-973` (91% similar). Same rule; run the equivalent diff. + +- [ ] **Step 3: Move the primary-only sections** + +Verbatim moves, changing only the `styles` import to `schoolSections.module.css` and replacing closed-over variables with props: + +| Component | Source | Props | +| --- | --- | --- | +| `ResultsSection.tsx` | `SchoolDetailView.tsx:688-959` | `latestResults, flags, primaryAvg, secondaryAvg, schoolInfo` | +| `AdmissionsSection.tsx` | `SchoolDetailView.tsx:960-1051` | `admissions, admissionsHistory, admissionsSummary, isAllThrough` | +| `InclusionSection.tsx` | `SchoolDetailView.tsx:1052-1135` | `latestResults, census, hasGenderSplit` | +| `HistorySection.tsx` | `SchoolDetailView.tsx:1136-1256` | `yearlyData, flags` | +| `SchoolLifeSection.tsx` | `SchoolDetailView.tsx:1257-1289` | `absenceData` | +| `LocalAreaSection.tsx` | `SchoolDetailView.tsx:1290-1313` | `deprivation` — bring `deprivationDesc` (lines 205-209) with it | + +- [ ] **Step 4: Move the secondary-only sections** + +| Component | Source | Props | +| --- | --- | --- | +| `GcseSection.tsx` | `SecondarySchoolDetailView.tsx:503-732` | `latestResults, flags, secondaryAvg, schoolInfo` | +| `SecondaryAdmissionsSection.tsx` | `SecondarySchoolDetailView.tsx:733-792` | `admissions` | +| `SecondaryHistorySection.tsx` | `SecondarySchoolDetailView.tsx:793-839` | `yearlyData, flags` | +| `WellbeingSection.tsx` | `SecondarySchoolDetailView.tsx:840-936` | `absenceData, census` — pass `variant="secondary"` to select `genderBarSecondary` | + +- [ ] **Step 5: Build the admissions toggle island** + +The current code renders both views always and toggles the `hidden` attribute, so the island only owns `hidden` and `aria-pressed`. The buttons sit inside `.admissionsHeader` beside the `

` while the viewport is a sibling below, so the component must span both to preserve the DOM exactly. + +Create `nextjs-app/components/school/AdmissionsViewToggle.tsx`: + +```tsx +'use client'; + +import { useState, type ReactNode } from 'react'; +import styles from './schoolSections.module.css'; + +/** + * The only interactive part of the admissions section. Both views are always + * in the DOM (matching the previous behaviour) and this toggles `hidden`, so + * the server-rendered markup passed in as yearView/trendView never ships as + * client JavaScript. + */ +export function AdmissionsViewToggle({ + title, subtitle, trendLabel, yearView, trendView, +}: { + title: ReactNode; + subtitle: ReactNode; + trendLabel: string; + yearView: ReactNode; + trendView: ReactNode; +}) { + const [view, setView] = useState<'year' | 'trend'>('year'); + + return ( + <> +
+

{title}

+
+ + +
+
+ {subtitle} +
+ + +
+ + ); +} +``` + +In `AdmissionsSection.tsx`, when `showAdmissionsTrend` is false, render the header and year view directly with **no** client component — those schools then ship zero admissions JavaScript. + +- [ ] **Step 6: Verify it compiles** + +Run: `cd nextjs-app && npm run typecheck` +Expected: PASS. The old views still exist and still render; nothing is wired up yet. + +- [ ] **Step 7: Commit** + +```bash +git add nextjs-app/components/school/ +git commit -m "$(cat <<'EOF' +refactor(detail): extract sections as server components + +Moves the section markup out of the two client views into +components/school/, mirroring the components/compare/ layout. Sections are +server components; the only new client file is AdmissionsViewToggle, which +owns the hidden/aria-pressed state and receives both views as server-rendered +children. + +Ofsted and Finances are shared between phases (80% and 91% similar); +Admissions and History are not (14% and 40%) and stay separate. + +Pure move -- no logic or markup changes. Not yet wired up. + +Co-Authored-By: Claude Opus 5 +EOF +)" +``` + +--- + +### Task 7: Add the shell and wire it up + +Where the bytes actually move. + +**Files:** +- Create: `nextjs-app/components/school/SchoolDetailShell.tsx` + `.module.css` +- Create: `nextjs-app/components/school/PrimarySchoolSections.tsx`, `SecondarySchoolSections.tsx` +- Modify: `nextjs-app/app/school/[slug]/page.tsx` +- Modify: `nextjs-app/__tests__/support/renderSchoolDetail.tsx` +- Delete: `SchoolDetailView.tsx` + `.module.css`, `SecondarySchoolDetailView.tsx` + `.module.css` + +**Interfaces:** +- Consumes: everything from Tasks 4-6. +- Produces: `SchoolDetailShell` — `'use client'`, props `{ schoolInfo: School; navItems: NavItem[]; flags: SchoolFlags; children: ReactNode }`. + +- [ ] **Step 1: Build the shell** + +Create `SchoolDetailShell.tsx` from `SchoolDetailView.tsx` lines 1-595 — the back link, header, details reveal, hero map, compare CTA, sticky nav, jump sheet — plus the hooks at lines 79-104 and the effects at 120-203 and 270-300. Keep `'use client'`. It renders `{children}` where the sections used to begin (after the nav closes, at what is currently line 595). + +`navItems` becomes a prop instead of being computed inline. The scroll-spy at line 283 uses `document.getElementById(id)` and needs no change — it finds server-rendered sections exactly as before. + +For the shell's stylesheet, do not eyeball which rules are shell-only — determine it mechanically. Every class the shell's JSX references goes into `SchoolDetailShell.module.css`; classes referenced by both the shell and a section stay duplicated in both files (CSS Modules hash them independently, so duplication is correct, not a smell). List the shell's classes with: + +```bash +cd nextjs-app && grep -o 'styles\.[a-zA-Z][A-Za-z0-9_]*' components/school/SchoolDetailShell.tsx \ + | sort -u | sed 's/styles\.//' +``` + +Copy each of those rules — including every media-query and `:hover`/`:focus` block that mentions them — from `SchoolDetailView.module.css` before deleting it in Step 6. + +- [ ] **Step 2: Build the two composers** + +Server components — no `'use client'`. Each returns the section sequence in the current order, gated by the same flags: + +```tsx +export function PrimarySchoolSections({ data, flags, nationalAvg }: PrimarySectionsProps) { + const { ofsted, admissions, /* … */ } = data; + return ( + <> + {ofsted && } + {flags.hasAnyResults && flags.latestResults && } + {admissions && } + {flags.hasInclusionData && } + {data.yearlyData.length > 0 && } + {flags.hasSchoolLife && } + {flags.hasDeprivation && } + {flags.hasFinance && } + + ); +} +``` + +The conditions must match `buildNavItems` exactly, or the nav will link to sections that do not exist. + +`SecondarySchoolSections` follows the same shape with `GcseSection`, `SecondaryAdmissionsSection`, `SecondaryHistorySection`, `WellbeingSection`, `FinancesSection`. + +- [ ] **Step 3: Rewire the page** + +In `app/school/[slug]/page.tsx`, replace the `isSecondary ? : ` block. Compute flags and nav items on the server, then wrap: + +```tsx +const flags = computeSchoolFlags({ schoolInfo, yearlyData, absenceData, census, deprivation, finance }); +const navItems = buildNavItems(flags, { ofsted, admissions, yearlyDataLength: yearlyData.length }); + +return ( + <> +