perf(detail): render school detail sections on the server #84
@@ -107,6 +107,41 @@ shell but to stop the other ~750 lines being attached to it.
|
||||
All-through schools render both the KS2 and KS4 section sets. The shared library
|
||||
makes this natural rather than duplicated.
|
||||
|
||||
### CSS strategy
|
||||
|
||||
Shared section components need one stylesheet, but the two existing CSS modules
|
||||
disagree. Measured on `cab7b4f`: `SchoolDetailView.module.css` defines 142
|
||||
classes, `SecondarySchoolDetailView.module.css` 112, with 79 names in common — of
|
||||
which 22 have different rules, and 16 of those are used inside section markup by
|
||||
both views (`.card`, `.sectionTitle`, `.metricCard`, `.metricValue`,
|
||||
`.metricsGrid`, `.tableWrapper`, the `heroStat*` set, the `genderSplit*` set,
|
||||
`.historyToggle`, `.ofstedReportLink`).
|
||||
|
||||
The 16 conflicts split into two kinds:
|
||||
|
||||
**Incidental drift (14 classes).** One view carries defensive overflow properties
|
||||
the other lacks — `min-width: 0`, `overflow-wrap: break-word`,
|
||||
`word-break: break-word`, `max-width: 100%`, `-webkit-overflow-scrolling: touch`.
|
||||
These are independent bug-fixes that were never back-ported, not design
|
||||
decisions. **Resolution: take the union.** No visible change, and it fixes latent
|
||||
long-school-name overflow on whichever page currently lacks the property.
|
||||
|
||||
**Real visual differences (2 classes).**
|
||||
|
||||
| Class | Primary | Secondary |
|
||||
| --- | --- | --- |
|
||||
| `.genderBar` | `height: 6px`, `background: var(--border-color, #e5dfd5)`, `width: 100%` | `height: 4px`, `background: rgba(0, 0, 0, 0.08)`, `margin-top: 0.45rem` |
|
||||
| `.heroStatValue` | `gap: 0.5rem`, `justify-content: flex-start`, Playfair fallback includes Georgia | `gap: 0.4rem`, Playfair fallback omits Georgia |
|
||||
|
||||
**Resolution: keep both looks as distinct classes** and have the owning section
|
||||
take a `variant: 'primary' | 'secondary'` prop selecting between them. Zero
|
||||
intended visual change, one shared stylesheet, and the divergence becomes
|
||||
explicit rather than accidental.
|
||||
|
||||
The merged stylesheet is `components/school/schoolSections.module.css`. Shell-only
|
||||
rules stay behind in the per-view modules. Classes outside the 79 shared names
|
||||
move across unchanged.
|
||||
|
||||
### Prerequisite: national averages move server-side
|
||||
|
||||
`SchoolDetailView.tsx:173-179` fetches `/api/national-averages` in a `useEffect`,
|
||||
@@ -174,13 +209,16 @@ bisectable:
|
||||
1. `test:` jest.setup additions (IntersectionObserver, scrollTo), chart mocks, fixtures
|
||||
2. `test:` characterization tests against the current components — all green before anything moves
|
||||
3. `refactor:` server-side `nationalAverages` in `page.tsx`; drop both `useEffect` fetches
|
||||
4. `refactor:` extract `components/school/sectionShared.tsx` and the server sections (pure moves, no logic edits)
|
||||
5. `refactor:` add `SchoolDetailShell` and `AdmissionsViewToggle`; rewire `page.tsx`; delete the two old views
|
||||
6. `chore:` delete the four unused SWR hooks; drop `swr` from `package.json`
|
||||
7. `test:` extend `e2e/journeys.spec.ts` with detail-page assertions
|
||||
4. `refactor:` extract the shared derived-flags/`navItems` module
|
||||
5. `style:` merge the two CSS modules into `components/school/schoolSections.module.css` per the CSS strategy above
|
||||
6. `refactor:` extract `components/school/sectionShared.tsx` and the server sections (pure moves, no logic edits)
|
||||
7. `refactor:` add `SchoolDetailShell` and `AdmissionsViewToggle`; rewire `page.tsx`; delete the two old views
|
||||
8. `chore:` delete the four unused SWR hooks; drop `swr` from `package.json`
|
||||
9. `test:` extend `e2e/journeys.spec.ts` with detail-page assertions
|
||||
|
||||
Commits 4 and 5 move the bytes; commits 1 and 2 are the safety net that makes
|
||||
them safe. Commit 6 lands late so it cannot confuse a bisect.
|
||||
Commits 6 and 7 move the bytes; commits 1 and 2 are the safety net that makes
|
||||
them safe. Commit 5 is isolated so a visual regression bisects to the CSS merge
|
||||
rather than to a JSX move. Commit 8 lands late so it cannot confuse a bisect.
|
||||
|
||||
## Verification
|
||||
|
||||
@@ -220,7 +258,12 @@ recorded in the PR, but no new CI gate is added in this change.
|
||||
|
||||
**DOM drift.** Moving ~750 lines of JSX risks subtle markup changes that CSS
|
||||
modules depend on. Commits 4 and 5 are strict moves and the section CSS moves
|
||||
with them; any styling change is out of scope.
|
||||
with them; any styling change beyond the CSS-merge rules above is out of scope.
|
||||
|
||||
**CSS merge.** Merging two large stylesheets is the highest-risk part of the
|
||||
work. The union rule is mechanical, but a missed conflict shows up as a visual
|
||||
regression that no unit test catches. The merge gets its own commit, and the
|
||||
16 known conflicts are enumerated above so they can be verified individually.
|
||||
|
||||
**All-through schools.** The trickiest case: `isAllThrough` gates KS2 content
|
||||
back on for a secondary-phase school (`app/school/[slug]/page.tsx:150-154`). It
|
||||
|
||||
Reference in New Issue
Block a user