diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 9dabf5d..b7ab141 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -561,6 +561,10 @@ test('a desktop postcode search opens on the map with the list beside it', async const pane = page.locator('[class*="mapListPane"]'); const card = pane.locator('[data-urn]').first(); await expect(card).toBeVisible({ timeout: 15_000 }); + // The list view's own row, not a cut-down card: it carries the same View + // link and Compare button. + await expect(card.getByRole('link', { name: 'View', exact: true })).toBeVisible(); + await expect(card.getByRole('button', { name: /Compar/ })).toBeVisible(); await expect(page.locator('.sc-pin').first()).toBeVisible({ timeout: 15_000 }); // The split runs to the bottom of the screen rather than stopping short. @@ -574,6 +578,17 @@ test('a desktop postcode search opens on the map with the list beside it', async await card.click({ position: { x: 6, y: 6 } }); await expect(page.locator('.sc-pin--selected')).toHaveCount(1); await expect(page.locator('.sc-popup')).toContainText(name); + + // And from the keyboard: each row has a "Show … on the map" button that + // appears on focus. + const second = pane.locator('[data-urn]').nth(1); + const secondName = (await second.locator('a').first().innerText()).trim(); + const show = second.getByRole('button', { name: `Show ${secondName} on the map` }); + await show.focus(); + await expect(show).toBeVisible(); + await page.keyboard.press('Enter'); + await expect(show).toHaveAttribute('aria-pressed', 'true'); + await expect(page.locator('.sc-popup')).toContainText(secondName); }); // 402 is the iPhone 17, where the toolbar overflowed (see below). @@ -629,7 +644,8 @@ for (const width of [360, 390, 402, 430]) { const small = await page.evaluate(() => { const toolbar = document.querySelector('[class*="resultsToolbar"]'); const fabEl = document.querySelector('[class*="viewFab"]'); - return [...(toolbar?.querySelectorAll('a, button, input, select') ?? []), fabEl] + const closeEl = document.querySelector('[class*="closeSheetBtn"]'); + return [...(toolbar?.querySelectorAll('a, button, input, select') ?? []), fabEl, closeEl] .filter((el): el is HTMLElement => !!el && !!(el as HTMLElement).offsetParent) .map((el) => ({ t: el.innerText?.trim().slice(0, 24) || el.getAttribute('aria-label'), w: el.getBoundingClientRect().width, h: el.getBoundingClientRect().height })) diff --git a/nextjs-app/__tests__/components/ResultsMapView.test.tsx b/nextjs-app/__tests__/components/ResultsMapView.test.tsx index 6045375..332dbea 100644 --- a/nextjs-app/__tests__/components/ResultsMapView.test.tsx +++ b/nextjs-app/__tests__/components/ResultsMapView.test.tsx @@ -104,11 +104,11 @@ it('selects the pin from the card, and the card from the pin', async () => { fireEvent.click(within(card(1)).getByText(/pupils/)); expect(screen.getByTestId('map')).toHaveAttribute('data-selected', '1'); - expect(card(1).className).toMatch(/compactItemSelected/); + expect(card(1).className).toMatch(/mapRowSelected/); fireEvent.click(screen.getByRole('button', { name: 'pin' })); - expect(card(2).className).toMatch(/compactItemSelected/); - expect(card(1).className).not.toMatch(/compactItemSelected/); + expect(card(2).className).toMatch(/mapRowSelected/); + expect(card(1).className).not.toMatch(/mapRowSelected/); }); it('clicking a card\'s link or button does not also select it', async () => { @@ -121,8 +121,9 @@ it('clicking a card\'s link or button does not also select it', async () => { it('shows the England comparison for mainstream schools only, and never a placeholder 0%', async () => { const { container } = await renderMap(); const card = (urn: number) => container.querySelector(`[data-urn="${urn}"]`) as HTMLElement; - expect(card(2)).toHaveTextContent('52% RWM -10 pts · 269 pupils'); - expect(card(3)).toHaveTextContent('62 pupils'); + expect(card(2)).toHaveTextContent('52%Reading, Writing & Maths-10 pts vs national'); + expect(card(2)).toHaveTextContent('269pupils'); + expect(card(3)).toHaveTextContent('62pupils'); expect(card(3)).not.toHaveTextContent(/%|pts/); }); @@ -152,3 +153,22 @@ it('builds no list cards on a phone, where the pane is hidden', async () => { // The count stays: it is the pane's heading, shown above the map. expect(screen.getByRole('heading', { name: /3 schools within/ })).toBeInTheDocument(); }); + +it('draws the list view\'s own row beside the map, with the same content', async () => { + const { container } = await renderMap(); + const beside = container.querySelector('[data-urn="2"] > [class~="row"]')!.textContent; + + fireEvent.click(screen.getByRole('button', { name: 'List' })); + const row = screen.getByRole('link', { name: 'Southmead Primary School' }).closest('[class~="row"]')!; + expect(row.parentElement?.className).toMatch(/schoolList/); + expect(row.textContent).toBe(beside); +}); + +it('lets a keyboard pick a pin from the list, with a real button', async () => { + await renderMap(); + const show = screen.getByRole('button', { name: 'Show Southmead Primary School on the map' }); + expect(show).toHaveAttribute('aria-pressed', 'false'); + fireEvent.click(show); + expect(screen.getByTestId('map')).toHaveAttribute('data-selected', '2'); + expect(show).toHaveAttribute('aria-pressed', 'true'); +}); diff --git a/nextjs-app/__tests__/components/rowContainerGuard.test.ts b/nextjs-app/__tests__/components/rowContainerGuard.test.ts new file mode 100644 index 0000000..b0937f3 --- /dev/null +++ b/nextjs-app/__tests__/components/rowContainerGuard.test.ts @@ -0,0 +1,41 @@ +import fs from 'fs'; +import path from 'path'; + +/* + * SchoolRow and SecondarySchoolRow switch to their narrow layout with a + * container query on a `results` container, not a media query, because the + * same row fills the phone list and the narrow list beside the desktop map. + * Outside a `results` container the query never matches and the row keeps its + * wide layout on a phone, a silent regression rather than an error. + * + * HomeView provides the container on every list it renders the rows into. + * Anything else that starts rendering them must do the same; this fails so + * that the person adding it reads this first. + */ + +const ROOT = path.join(__dirname, '..', '..'); +const DIRS = ['app', 'components', 'lib']; +const ROW_IMPORT = /from\s+['"][^'"]*\/(SchoolRow|SecondarySchoolRow)['"]/; + +function sources(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return entry.name === 'node_modules' ? [] : sources(full); + return /\.tsx?$/.test(entry.name) ? [full] : []; + }); +} + +it('renders the results rows only where a `results` container is provided', () => { + const importers = DIRS.flatMap((d) => sources(path.join(ROOT, d))) + .filter((file) => ROW_IMPORT.test(fs.readFileSync(file, 'utf8'))) + .map((file) => path.relative(ROOT, file)); + expect(importers).toEqual(['components/HomeView.tsx']); +}); + +it('gives each of HomeView\'s row lists the `results` container', () => { + const css = fs.readFileSync(path.join(ROOT, 'components', 'HomeView.module.css'), 'utf8'); + for (const list of ['.schoolList', '.compactList', '.bottomSheet']) { + const rule = new RegExp(`\\${list}\\s*\\{[^}]*container:\\s*results\\s*/\\s*inline-size`); + expect({ list, provided: rule.test(css) }).toEqual({ list, provided: true }); + } +}); diff --git a/nextjs-app/components/HomeView.module.css b/nextjs-app/components/HomeView.module.css index 862abfe..7ea8c08 100644 --- a/nextjs-app/components/HomeView.module.css +++ b/nextjs-app/components/HomeView.module.css @@ -591,7 +591,7 @@ .mapViewContainer { display: grid; - grid-template-columns: minmax(340px, 420px) minmax(0, 1fr); + grid-template-columns: minmax(360px, 460px) minmax(0, 1fr); height: calc(100dvh - var(--map-top) - var(--map-bottom)); min-height: 480px; background: var(--bg-card); @@ -631,117 +631,53 @@ overflow-y: auto; padding: 0.125rem 1rem 1rem; scrollbar-width: thin; + /* The rows lay themselves out by this list's width (SchoolRow.module.css), + which here is always narrow, whatever the screen. */ + container: results / inline-size; } -/* Compact School Item: the list pane's card, and the phone's bottom sheet. */ -.compactItem { - display: flex; - flex-direction: column; - gap: 0.4375rem; - padding: 0.75rem 0.875rem; - background: var(--bg-card); - border: 1px solid var(--border); - border-radius: 10px; +/* A row in the list beside the map: clicking it picks its pin. */ +.mapRow { + position: relative; cursor: pointer; - transition: border-color var(--transition), box-shadow var(--transition); + border-radius: 10px; } -.compactItem:hover { - border-color: var(--border-strong); +.mapRowSelected > :last-child { + outline: 2px solid var(--brand); + outline-offset: 1px; } -.compactItemSelected, -.compactItemSelected:hover { - border-color: var(--brand); - box-shadow: 0 0 0 2px rgba(var(--brand-rgb), 0.28); +/* Visually hidden until focused, then a pill over the row's top edge. */ +.showOnMap { + position: absolute; + width: 1px; + height: 1px; + overflow: hidden; + clip-path: inset(50%); + white-space: nowrap; } -.compactItemHeader { - display: flex; - justify-content: space-between; - align-items: flex-start; - gap: 0.625rem; -} - -.compactItemName { - font-family: var(--font-display); - font-size: 0.9375rem; - font-weight: 700; - line-height: 1.3; - color: var(--text-primary); - text-decoration: none; -} - -.compactItemName:hover { - color: var(--brand-strong); - text-decoration: underline; -} - -.distanceBadge { - flex-shrink: 0; - padding: 0.125rem 0.375rem; - font-size: 0.75rem; - font-weight: 700; +.showOnMap:focus-visible { + top: -0.5rem; + right: 0.75rem; + z-index: 1; + width: auto; + height: auto; + padding: 0.375rem 0.75rem; + overflow: visible; + clip-path: none; background: var(--brand); color: var(--brand-on); - border-radius: 4px; - white-space: nowrap; + border: 0; + border-radius: 999px; + font-family: var(--font-ui); + font-size: var(--step--1); + font-weight: 700; + outline: 2px solid var(--text-primary); + outline-offset: 2px; } -.compactItemTags { - display: flex; - flex-wrap: wrap; - gap: 0.375rem; -} - -.compactBadge, -.compactTag { - padding: 0.125rem 0.4375rem; - border-radius: 4px; - font-size: 0.6875rem; - font-weight: 600; - white-space: nowrap; -} - -.compactTag { - background: var(--bg-secondary); - color: var(--text-secondary); - font-weight: 500; -} - -.ofsted1, -.ofsted2 { background: var(--status-above-bg); color: var(--status-above); } -.ofsted3 { background: var(--status-below-bg); color: var(--status-below); } -.ofsted4 { background: var(--status-below); color: var(--text-inverse); } -.ofstedRc { background: var(--phase-secondary-text); color: var(--text-inverse); } -.ofstedInspected { background: var(--phase-primary-bg); color: var(--phase-primary-text); } -.ofstedPending { background: var(--border); color: var(--text-muted); } - -.compactItemFooter { - display: flex; - justify-content: space-between; - align-items: center; - gap: 0.75rem; -} - -.compactStat { - font-size: 0.8125rem; - color: var(--text-secondary); -} - -.compactStat strong { - font-size: 0.9375rem; - color: var(--text-primary); -} - -.deltaUp { color: var(--status-above); font-weight: 600; } -.deltaDown { color: var(--status-below); font-weight: 600; } - -.compactItemActions { - display: flex; - gap: 0.5rem; - flex-shrink: 0; -} .sectionHeader { @@ -779,6 +715,8 @@ flex-direction: column; gap: 0.5rem; margin-bottom: 1.25rem; + /* The rows lay themselves out by this list's width (SchoolRow.module.css). */ + container: results / inline-size; } /* Staggered fade-in for rows */ @@ -881,33 +819,37 @@ animation: slideUpSheet 0.3s cubic-bezier(0.16, 1, 0.3, 1) forwards; } - .bottomSheet .compactItem { - border: none; - box-shadow: none; - background: transparent; - padding: 1rem; - cursor: default; - } + /* A 30px circle, drawn by ::before, inside a 44px target (MOBILE.md). */ .closeSheetBtn { position: absolute; - top: -12px; - right: -12px; - width: 30px; - height: 30px; - background: var(--bg-card); - border: 1px solid var(--border); - border-radius: 50%; + top: -19px; + right: -15px; + width: 44px; + height: 44px; + padding: 0; + background: none; + border: 0; display: flex; align-items: center; justify-content: center; font-size: 1.25rem; color: var(--text-secondary); cursor: pointer; - box-shadow: 0 2px 8px rgba(var(--shadow-rgb), 0.1); z-index: 10; } + .closeSheetBtn::before { + content: ''; + position: absolute; + inset: 7px; + z-index: -1; + background: var(--bg-card); + border: 1px solid var(--border); + border-radius: 50%; + box-shadow: 0 2px 8px rgba(var(--shadow-rgb), 0.1); + } + @keyframes slideUpSheet { from { transform: translateY(120%); @@ -935,6 +877,13 @@ display: none; } + /* The sheet holds one results row, which is the card itself. It is not + inside a `results` container, so give it one: a phone-width sheet takes + the row's narrow layout. */ + .bottomSheet { + container: results / inline-size; + } + .mapListPane .resultsHeader { padding: 0.625rem 0.875rem; } diff --git a/nextjs-app/components/HomeView.tsx b/nextjs-app/components/HomeView.tsx index 8d6918a..ceb5ce8 100644 --- a/nextjs-app/components/HomeView.tsx +++ b/nextjs-app/components/HomeView.tsx @@ -16,7 +16,6 @@ import { HeroIllustration } from './Illustration'; import { useComparisonContext } from '@/context/ComparisonContext'; import { fetchSchools, fetchLAaverages, fetchNationalAverages } from '@/lib/api'; import type { SchoolsResponse, Filters, School } from '@/lib/types'; -import { schoolUrl, buildOfstedListBadge, isSpecialSchool, listRwmValue } from '@/lib/utils'; import { track } from '@/lib/analytics'; import styles from './HomeView.module.css'; @@ -558,6 +557,32 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed const isMapView = initialSchools.schools.length > 0 && resultsView === 'map' && isLocationSearch; + // One school as a results row: the list view, the list beside the map and + // the phone's bottom sheet all draw the same thing. + const renderRow = (school: School) => ( + school.attainment_8_score != null ? ( + + ) : ( + + ) + ); + // The count and the sort. Above the list in list view; at the top of the // list pane, beside the map, in map view. const resultsHeader = ( @@ -797,17 +822,32 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
{resultsHeader}
+ {/* The list view's own rows, so both views show the same thing. + Clicking a row (not its links or buttons) picks its pin. */} {listPaneShown && mapListSchools.map((school) => ( - + data-urn={school.urn} + className={`${styles.mapRow} ${selectedMapSchool?.urn === school.urn ? styles.mapRowSelected : ''}`} + onClick={(e) => { + if ((e.target as HTMLElement).closest('a, button')) return; + setSelectedMapSchool(school); + }} + > + {/* The keyboard's way to pick the pin: hidden until it has + focus, since a pointer just clicks the row. The row + itself cannot be the button, as it holds links and + buttons of its own. */} + + {renderRow(school)} +
))}
@@ -831,29 +871,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed /* List View Layout */ <>
- {sortedSchools.map((school) => ( - school.attainment_8_score != null ? ( - s.urn === school.urn)} - laAvgAttainment8={school.local_authority ? laAverages[school.local_authority] ?? null : null} - /> - ) : ( - s.urn === school.urn)} - nationalAvgRwm={nationalAvgRwm} - /> - ) - ))} + {sortedSchools.map(renderRow)}
{(hasMore || allSchools.length < initialSchools.total) && ( @@ -899,14 +917,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed > × - + {renderRow(selectedMapSchool)} )} @@ -914,106 +925,3 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed ); } - -/* Compact School Item: a card in the map view's list, and the phone's bottom sheet. */ -interface CompactSchoolItemProps { - school: School; - onAddToCompare: (school: School) => void; - isInCompare: boolean; - nationalAvgRwm?: number | null; - laAverages?: Record; - isSelected?: boolean; - /** Clicking the card (not its link or button) picks its pin on the map. */ - onSelect?: (school: School) => void; - /** The bottom sheet has no list around it, so it carries its own View. */ - showView?: boolean; -} - -function CompactSchoolItem({ - school, onAddToCompare, isInCompare, nationalAvgRwm, laAverages, isSelected, onSelect, showView, -}: CompactSchoolItemProps) { - const ofstedBadge = buildOfstedListBadge(school); - const special = isSpecialSchool(school); - const href = schoolUrl(school.urn, school.school_name); - - /* - * The headline figure, then its comparison. Same rules as the list rows: - * no placeholder all-zero RWM, and no mainstream benchmark for special - * schools, PRUs or AP. - */ - let figure: React.ReactNode = null; - if (school.attainment_8_score != null) { - const laAvg = school.local_authority ? laAverages?.[school.local_authority] : undefined; - const diff = !special && laAvg != null - ? Math.round((school.attainment_8_score - laAvg) * 10) / 10 : null; - figure = ( - <> - {school.attainment_8_score.toFixed(1)} Att 8 - {diff != null && ( - = 0.5 ? styles.deltaUp : diff <= -0.5 ? styles.deltaDown : undefined}> - {' '}{diff >= 0 ? '+' : ''}{diff} vs LA - - )} - - ); - } else { - const rwm = listRwmValue(school); - if (rwm != null) { - const diff = !special && nationalAvgRwm != null ? Math.round(rwm - nationalAvgRwm) : null; - figure = ( - <> - {rwm}% RWM - {diff != null && ( - = 2 ? styles.deltaUp : diff <= -2 ? styles.deltaDown : undefined}> - {' '}{diff >= 2 ? `+${diff} pts` : diff <= -2 ? `${diff} pts` : '≈ national'} - - )} - - ); - } - } - - const handleClick = (e: React.MouseEvent) => { - if ((e.target as HTMLElement).closest('a, button')) return; - onSelect?.(school); - }; - - return ( -
-
- {school.school_name} - {school.distance != null && ( - {school.distance.toFixed(1)} mi - )} -
-
- - {ofstedBadge.label} - - {school.school_type && {school.school_type}} -
-
- - {figure} - {school.total_pupils != null && ( - <>{figure ? ' · ' : ''}{school.total_pupils.toLocaleString('en-GB')} pupils - )} - -
- {showView && View} - -
-
-
- ); -} diff --git a/nextjs-app/components/SchoolRow.module.css b/nextjs-app/components/SchoolRow.module.css index 9a1ceb8..ce61578 100644 --- a/nextjs-app/components/SchoolRow.module.css +++ b/nextjs-app/components/SchoolRow.module.css @@ -220,7 +220,16 @@ .vsNationalFlat { font-size: 0.7rem; color: var(--text-muted); } /* ── Mobile ──────────────────────────────────────────── */ -@media (max-width: 640px) { +/* + * Narrow: content full width, actions in a row beneath. Keyed to the list the + * row sits in, not the screen, because the same row fills the phone list and + * the ~430px list beside the map on desktop. HomeView makes its lists a + * `results` container; outside one, the row keeps its wide layout, which is + * why rowContainerGuard.test.ts fails if anything else renders this row. + * 608px is exact: below 769px the page pads 1rem each side, so a 640px screen + * gives a 608px list, matching the old max-width: 640px media query. + */ +@container results (max-width: 608px) { .row { flex-wrap: wrap; padding: 0.875rem; diff --git a/nextjs-app/components/SecondarySchoolRow.module.css b/nextjs-app/components/SecondarySchoolRow.module.css index 3f0a7d0..2c82faa 100644 --- a/nextjs-app/components/SecondarySchoolRow.module.css +++ b/nextjs-app/components/SecondarySchoolRow.module.css @@ -232,7 +232,16 @@ } /* ── Mobile ──────────────────────────────────────────── */ -@media (max-width: 640px) { +/* + * Narrow: content full width, actions in a row beneath. Keyed to the list the + * row sits in, not the screen, because the same row fills the phone list and + * the ~430px list beside the map on desktop. HomeView makes its lists a + * `results` container; outside one, the row keeps its wide layout, which is + * why rowContainerGuard.test.ts fails if anything else renders this row. + * 608px is exact: below 769px the page pads 1rem each side, so a 640px screen + * gives a 608px list, matching the old max-width: 640px media query. + */ +@container results (max-width: 608px) { .row { flex-wrap: wrap; padding: 0.875rem;