perf(detail): render school detail sections on the server #84
@@ -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 <noreply@anthropic.com>`.
|
||||
|
||||
---
|
||||
|
||||
## 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 <noreply@anthropic.com>
|
||||
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(<SchoolDetailView {...fixture} />);
|
||||
}
|
||||
|
||||
export function renderSecondarySchoolDetail(fixture: any) {
|
||||
return render(<SecondarySchoolDetailView {...fixture} />);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **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: () => <div data-testid="performance-chart" />,
|
||||
}));
|
||||
jest.mock('@/components/SatsChart', () => ({
|
||||
__esModule: true,
|
||||
default: () => <div data-testid="sats-chart" />,
|
||||
}));
|
||||
jest.mock('@/components/AdmissionsTrendChart', () => ({
|
||||
__esModule: true,
|
||||
default: () => <div data-testid="admissions-trend-chart" />,
|
||||
}));
|
||||
jest.mock('@/components/SchoolHeroMap', () => ({
|
||||
SchoolHeroMap: () => <div data-testid="hero-map" />,
|
||||
}));
|
||||
```
|
||||
|
||||
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 <noreply@anthropic.com>
|
||||
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 `<SecondarySchoolDetailView>` and `<SchoolDetailView>` 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(<SchoolDetailView {...fixture} nationalAvg={nationalAveragesFixture} />);
|
||||
}
|
||||
```
|
||||
|
||||
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 <noreply@anthropic.com>
|
||||
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 <noreply@anthropic.com>
|
||||
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 <noreply@anthropic.com>
|
||||
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 `<section id className={styles.card}>`), `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 `<h2>` 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 (
|
||||
<>
|
||||
<div className={styles.admissionsHeader}>
|
||||
<h2 className={styles.sectionTitle}>{title}</h2>
|
||||
<div className={styles.admissionsSeg} role="group" aria-label="Admissions view">
|
||||
<button type="button" aria-pressed={view === 'year'} onClick={() => setView('year')}>
|
||||
This year
|
||||
</button>
|
||||
<button type="button" aria-pressed={view === 'trend'} onClick={() => setView('trend')}>
|
||||
{trendLabel}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
{subtitle}
|
||||
<div className={styles.admissionsViewport}>
|
||||
<div className={styles.admissionsViewYear} hidden={view !== 'year'}>{yearView}</div>
|
||||
<div className={styles.admissionsViewTrend} hidden={view !== 'trend'}>{trendView}</div>
|
||||
</div>
|
||||
</>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
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 <noreply@anthropic.com>
|
||||
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 && <OfstedSection ofsted={ofsted} urn={data.schoolInfo.urn} /* … */ />}
|
||||
{flags.hasAnyResults && flags.latestResults && <ResultsSection /* … */ />}
|
||||
{admissions && <AdmissionsSection /* … */ />}
|
||||
{flags.hasInclusionData && <InclusionSection /* … */ />}
|
||||
{data.yearlyData.length > 0 && <HistorySection /* … */ />}
|
||||
{flags.hasSchoolLife && <SchoolLifeSection /* … */ />}
|
||||
{flags.hasDeprivation && <LocalAreaSection /* … */ />}
|
||||
{flags.hasFinance && <FinancesSection /* … */ />}
|
||||
</>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
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 ? <SecondarySchoolDetailView/> : <SchoolDetailView/>` 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 (
|
||||
<>
|
||||
<script type="application/ld+json" dangerouslySetInnerHTML={{ __html: JSON.stringify(structuredData) }} />
|
||||
<SchoolDetailShell schoolInfo={school_info} navItems={navItems} flags={flags}>
|
||||
{isSecondary
|
||||
? <SecondarySchoolSections data={data} flags={flags} nationalAvg={nationalAvg} />
|
||||
: <PrimarySchoolSections data={data} flags={flags} nationalAvg={nationalAvg} />}
|
||||
</SchoolDetailShell>
|
||||
</>
|
||||
);
|
||||
```
|
||||
|
||||
Keep the existing `isSecondary` / `isAllThrough` derivation exactly as it is — all-through schools must still route to the primary composer, which renders both KS2 and KS4 content.
|
||||
|
||||
- [ ] **Step 4: Swap the render helper — the moment of truth**
|
||||
|
||||
Rewrite **only** `__tests__/support/renderSchoolDetail.tsx`:
|
||||
|
||||
```tsx
|
||||
export function renderSchoolDetail(fixture: any) {
|
||||
const flags = computeSchoolFlags(fixture);
|
||||
const navItems = buildNavItems(flags, {
|
||||
ofsted: fixture.ofsted, admissions: fixture.admissions,
|
||||
yearlyDataLength: fixture.yearlyData.length,
|
||||
});
|
||||
return render(
|
||||
<SchoolDetailShell schoolInfo={fixture.schoolInfo} navItems={navItems} flags={flags}>
|
||||
<PrimarySchoolSections data={fixture} flags={flags} nationalAvg={nationalAveragesFixture} />
|
||||
</SchoolDetailShell>,
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run the characterization tests**
|
||||
|
||||
Run: `cd nextjs-app && npm test -- schoolDetail.characterization`
|
||||
Expected: **PASS, with `schoolDetail.characterization.test.tsx` byte-identical to Task 2.**
|
||||
|
||||
This is the proof the refactor preserved behaviour. If a test fails, the refactor is wrong — fix the components, never the test. Confirm the file is untouched:
|
||||
|
||||
```bash
|
||||
git diff --exit-code nextjs-app/__tests__/components/schoolDetail.characterization.test.tsx
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Delete the old views**
|
||||
|
||||
```bash
|
||||
git rm nextjs-app/components/SchoolDetailView.tsx nextjs-app/components/SchoolDetailView.module.css \
|
||||
nextjs-app/components/SecondarySchoolDetailView.tsx nextjs-app/components/SecondarySchoolDetailView.module.css
|
||||
```
|
||||
|
||||
Confirm nothing still imports them:
|
||||
```bash
|
||||
cd nextjs-app && grep -rn "SchoolDetailView\|SecondarySchoolDetailView" app components lib __tests__ | grep -v node_modules
|
||||
```
|
||||
Expected: no output.
|
||||
|
||||
- [ ] **Step 7: Full verification**
|
||||
|
||||
Run: `cd nextjs-app && npm run typecheck && npm test && npm run build`
|
||||
Expected: all three PASS.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add -A nextjs-app/
|
||||
git commit -m "$(cat <<'EOF'
|
||||
refactor(detail): render sections on the server behind a client shell
|
||||
|
||||
page.tsx now composes the sections and passes them through
|
||||
SchoolDetailShell as children, so ~750 lines of static markup stop shipping
|
||||
as client JavaScript. The shell keeps what is genuinely interactive: back
|
||||
link, header reveal, hero map, compare CTA, sticky nav and scroll-spy.
|
||||
|
||||
The scroll-spy already located sections via document.getElementById, so it
|
||||
works unchanged against server-rendered children.
|
||||
|
||||
Characterization tests from the earlier commit pass unmodified.
|
||||
|
||||
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 8: Delete the unused SWR hooks
|
||||
|
||||
**Files:**
|
||||
- Delete: `nextjs-app/hooks/useSchools.ts`, `useFilters.ts`, `useMetrics.ts`, `useSchoolDetails.ts`
|
||||
- Modify: `nextjs-app/package.json`
|
||||
|
||||
- [ ] **Step 1: Confirm nothing imports them**
|
||||
|
||||
```bash
|
||||
cd nextjs-app && grep -rn "useSchools\|useFilters\|useMetrics\|useSchoolDetails" app components lib __tests__ | grep -v node_modules
|
||||
```
|
||||
Expected: no output. If anything appears, stop and report it — do not delete.
|
||||
|
||||
- [ ] **Step 2: Delete and drop the dependency**
|
||||
|
||||
```bash
|
||||
git rm nextjs-app/hooks/useSchools.ts nextjs-app/hooks/useFilters.ts \
|
||||
nextjs-app/hooks/useMetrics.ts nextjs-app/hooks/useSchoolDetails.ts
|
||||
cd nextjs-app && npm uninstall swr
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Verify**
|
||||
|
||||
Run: `cd nextjs-app && npm run typecheck && npm test && npm run build`
|
||||
Expected: all PASS.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add -A nextjs-app/
|
||||
git commit -m "$(cat <<'EOF'
|
||||
chore: drop unused SWR hooks and dependency
|
||||
|
||||
useSchools, useFilters, useMetrics and useSchoolDetails were imported by
|
||||
nothing and were the only consumers of swr. All data fetching goes through
|
||||
lib/api.ts on the server.
|
||||
|
||||
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 9: E2E coverage and measurement
|
||||
|
||||
CLAUDE.md requires e2e updates in the same PR as user-facing changes; these journeys gate whether staging is fit for human testing.
|
||||
|
||||
**Files:**
|
||||
- Modify: `e2e/tests/journeys.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Read the existing journeys**
|
||||
|
||||
Run: `cat e2e/tests/journeys.spec.ts` and match its existing helpers, selectors and naming. Note the staging-data constraint: year filters must use the **latest** year, not the oldest — staging carries only partial history.
|
||||
|
||||
- [ ] **Step 2: Add detail-page assertions**
|
||||
|
||||
Add a journey that visits a school detail page and asserts: the Ofsted section renders; the results section renders a figure; the sticky section nav renders and a jump link scrolls to its section; and — the regression this whole refactor risks — the admissions year/trend toggle switches views. Assert on user-visible text and roles, never CSS-module class names.
|
||||
|
||||
- [ ] **Step 3: Verify the spec parses**
|
||||
|
||||
Run: `cd e2e && npx playwright test --list`
|
||||
Expected: the new tests appear. Do not run them locally — they target staging, which is deployed from `main`.
|
||||
|
||||
- [ ] **Step 4: Measure the result**
|
||||
|
||||
```bash
|
||||
cd nextjs-app && rm -rf .next && npx next build >/dev/null 2>&1 && python3 - <<'PY'
|
||||
import re,os,gzip,glob
|
||||
for html in sorted(glob.glob('.next/server/app/**/*.html', recursive=True)):
|
||||
src=set(re.findall(r'/_next/(static/[^"\']+\.js)', open(html).read()))
|
||||
tot=sum(len(gzip.compress(open(os.path.join('.next',s),'rb').read()))
|
||||
for s in src if os.path.exists(os.path.join('.next',s)))
|
||||
print(f"{tot/1024:8.1f} KB gz {len(src):3d} scripts {html}")
|
||||
PY
|
||||
```
|
||||
|
||||
Record the figures for the PR description. Compare against the `cab7b4f` baseline: 172 KB gz shared baseline, `/admissions` 177 KB gz. **Report the route-specific figure and the shared baseline separately** — this change does not target the baseline, and an unchanged baseline is not a failure.
|
||||
|
||||
- [ ] **Step 5: Commit and open the PR**
|
||||
|
||||
```bash
|
||||
git add e2e/tests/journeys.spec.ts
|
||||
git commit -m "$(cat <<'EOF'
|
||||
test(e2e): cover school detail sections and admissions toggle
|
||||
|
||||
The server/client split moves the detail sections out of the client bundle;
|
||||
these journeys gate that the rendered page still behaves the same on staging.
|
||||
|
||||
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||||
EOF
|
||||
)"
|
||||
git push -u origin perf/server-client-split
|
||||
```
|
||||
|
||||
Open the PR against `main` per the Gitea flow in `docs/DEPLOY.md`. The description must include: the before/after byte table from Step 4, the note that the shared baseline is deliberately unchanged, the two CSS variants introduced in Task 5, and any bugs found-but-not-fixed during the pure-move tasks.
|
||||
|
||||
Do **not** trigger the production promotion workflow — that is the human's call.
|
||||
|
||||
---
|
||||
|
||||
## Verification Checklist
|
||||
|
||||
Before claiming the work complete, confirm each of these with actual command output:
|
||||
|
||||
- [ ] `cd nextjs-app && npm run typecheck` — passes
|
||||
- [ ] `cd nextjs-app && npm test` — passes
|
||||
- [ ] `cd nextjs-app && npm run build` — passes
|
||||
- [ ] `git diff cab7b4f -- nextjs-app/__tests__/components/schoolDetail.characterization.test.tsx` — **empty** after Task 2's commit
|
||||
- [ ] `grep -rn "SchoolDetailView" nextjs-app/app nextjs-app/components` — no output
|
||||
- [ ] `grep -rn "swr" nextjs-app/package.json` — no output
|
||||
- [ ] CSS audit script from Task 5 Step 5 — no `MISSING` or `LOST` lines
|
||||
- [ ] Byte measurement recorded in the PR description
|
||||
@@ -100,12 +100,32 @@ shell but to stop the other ~750 lines being attached to it.
|
||||
|
||||
### Section sharing
|
||||
|
||||
| Shared by both views | Primary only | Secondary only |
|
||||
| --- | --- | --- |
|
||||
| Ofsted, Admissions, History, Finances, and the shell | Results (KS2), Inclusion, School Life, Local Area | Wellbeing |
|
||||
Sharing is decided by measured similarity, not assumption. Comparing the two
|
||||
views' section bodies line-by-line (`difflib.SequenceMatcher` on stripped lines):
|
||||
|
||||
All-through schools render both the KS2 and KS4 section sets. The shared library
|
||||
makes this natural rather than duplicated.
|
||||
| Section | Primary | Secondary | Similarity | Decision |
|
||||
| --- | --- | --- | --- | --- |
|
||||
| Finances | 29 lines | 35 | 91% | Shared |
|
||||
| Ofsted | 88 | 107 | 80% | Shared |
|
||||
| History | 121 | 47 | 40% | Separate |
|
||||
| Admissions | 89 | 55 | 14% | Separate |
|
||||
|
||||
Admissions and History were assumed shareable when this design was first drafted;
|
||||
measurement during planning showed they are not — primary's admissions carries
|
||||
the year/trend toggle and chart, secondary's is a much simpler panel.
|
||||
|
||||
**This costs nothing in bytes.** The client-JS win comes from a section being a
|
||||
*server* component, not from being shared between views. Sharing is a
|
||||
maintainability benefit, taken only where it is cheap. Forcing the two
|
||||
low-similarity sections together would inflate the diff and the regression risk
|
||||
for no performance gain.
|
||||
|
||||
| Shared | Primary only | Secondary only |
|
||||
| --- | --- | --- |
|
||||
| Ofsted, Finances, `sectionShared` primitives, and the shell | Results (KS2), Admissions, History, Inclusion, School Life, Local Area | GCSEs, Admissions, History, Wellbeing |
|
||||
|
||||
All-through schools render both the KS2 and KS4 section sets via the primary
|
||||
composer.
|
||||
|
||||
### CSS strategy
|
||||
|
||||
|
||||
Reference in New Issue
Block a user