From cf3c773f86fd0f96a9a8eab8398d34f8ffe3ea96 Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 2 Oct 2026 09:31:53 +0100 Subject: [PATCH 1/2] feat(search): filter on phones through one sheet On phones the results toolbar's filters were a sideways-scrolling row led by "More filters", so phase showed only in part and school type not at all, and the panel "More filters" opened held neither of them. Phones now get a single Filters button beside the folded search summary, counting every applied filter. It opens a bottom sheet with every filter: distance as five segments, then phase, school type, local authority and the secondary-only filters. Changes apply at once, as on desktop, so the footer's "Show N schools" only closes the sheet. Applied filters show as removable chips on a second line, which appears only when something is applied. Desktop and tablet are unchanged. Modal gains dialog semantics, a pinned footer and focus handling, and moves above the pinned toolbar, the floating List/Map button and the comparison toast, which its old z-index sat beneath. Co-Authored-By: Claude Opus 5.5 --- e2e/tests/journeys.spec.ts | 66 ++- .../__tests__/components/FilterSheet.test.tsx | 156 ++++++ .../components/ResultsToolbar.test.tsx | 16 +- nextjs-app/components/FilterBar.module.css | 128 ++++- nextjs-app/components/FilterBar.tsx | 470 ++++++++++++------ nextjs-app/components/FilterSheet.module.css | 96 ++++ nextjs-app/components/FilterSheet.tsx | 121 +++++ nextjs-app/components/HomeView.tsx | 1 + nextjs-app/components/Modal.module.css | 16 +- nextjs-app/components/Modal.tsx | 31 +- 10 files changed, 933 insertions(+), 168 deletions(-) create mode 100644 nextjs-app/__tests__/components/FilterSheet.test.tsx create mode 100644 nextjs-app/components/FilterSheet.module.css create mode 100644 nextjs-app/components/FilterSheet.tsx diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 7dd7030..b5f7f9e 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -712,12 +712,13 @@ for (const width of [360, 390, 402, 430]) { // scrollWidth alone cannot see this page's overflow: .main clips on x, so // the search summary ran 40px off an iPhone 17 screen with scrollWidth // still equal to the viewport. Measure the toolbar's own edges instead; - // the controls row scrolls by design, so only its box is held to the edge. + // the applied-filter chips scroll by design, so only their line's box is + // held to the edge. const offscreen = await page.evaluate(() => { const toolbar = document.querySelector('[class*="resultsToolbar"]'); return [...(toolbar?.querySelectorAll('*') ?? [])] .filter((el) => (el as HTMLElement).offsetParent - && !el.parentElement?.closest('[class*="controlsRow"]')) + && !el.parentElement?.closest('[class*="chipsLine"]')) .map((el) => ({ el: (el.className?.toString() || el.tagName).slice(0, 40), right: Math.round(el.getBoundingClientRect().right) })) .filter((o) => o.right > window.innerWidth); @@ -749,6 +750,67 @@ for (const width of [360, 390, 402, 430]) { }); } +/* + * Phones filter through one Filters button and a sheet with every filter in + * it. The row it replaced scrolled sideways with "More filters" first, so + * phase showed only in part, school type not at all, and the panel "More + * filters" opened held neither. + */ +test('a phone filters through one sheet, and sees what it applied as chips', async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await page.goto(LONG_LIST); + const trigger = page.getByRole('button', { name: 'Filters', exact: true }); + await expect(trigger).toBeInViewport({ timeout: 15_000 }); + // The desktop row is gone at this width: no phase select until the sheet. + await expect(page.getByRole('combobox', { name: 'Phase', exact: true })).toHaveCount(0); + + await trigger.click(); + const sheet = page.getByRole('dialog', { name: 'Filters' }); + await expect(sheet).toBeVisible(); + await expect(sheet.getByRole('radio', { name: 'Within 1 mile' })).toBeChecked(); + for (const name of ['Phase', 'School type', 'Local authority']) { + await expect(sheet.getByRole('combobox', { name, exact: true })).toBeVisible(); + } + + // A change applies at once, and the sheet stays open for the next one. + await sheet.getByRole('combobox', { name: 'Phase', exact: true }).selectOption('primary'); + await page.waitForURL(/[?&]phase=primary(&|$)/); + await expect(sheet).toBeVisible(); + const show = sheet.getByRole('button', { name: /^(Show [\d,]+ schools?|No schools match)$/ }); + await expect(show).toBeVisible(); + + // MOBILE.md: the sheet clears the bottom tab bar, and its targets are 44px. + // It slides up over 0.3s, so poll for where it comes to rest. + await expect.poll(async () => { + const box = (await show.boundingBox())!; + return Math.round(box.y + box.height); + }).toBeLessThanOrEqual(844); + const small = await sheet.evaluate((el) => + [...el.querySelectorAll('button, select, label:has(input)')] + .filter((n): n is HTMLElement => !!(n as HTMLElement).offsetParent) + .map((n) => ({ t: n.innerText?.trim().slice(0, 24) || n.getAttribute('aria-label'), + w: n.getBoundingClientRect().width, h: n.getBoundingClientRect().height })) + .filter((o) => o.w < 44 || o.h < 44)); + expect(small).toEqual([]); + + await show.click(); + await expect(sheet).toBeHidden(); + + // What was applied shows under the search, counted on the button, and comes + // off with a tap, keeping the search. + await expect(page.getByRole('button', { name: 'Filters, 1 applied' })).toBeInViewport(); + const chip = page.getByRole('group', { name: 'Applied filters' }) + .getByRole('button', { name: 'Remove filter: Primary' }); + await expect(chip).toBeInViewport(); + expect(await page.evaluate(() => document.documentElement.scrollWidth - window.innerWidth)) + .toBe(0); + await chip.click(); + await expect(page).not.toHaveURL(/[?&]phase=/); + await expect(page).toHaveURL(/[?&]postcode=B1(%20|\+)1BB/); + await expect(page.getByRole('group', { name: 'Applied filters' })).toHaveCount(0); + await expect(trigger).toBeInViewport(); +}); + test('comparing two schools shows the parent-first sections side by side', async ({ page }) => { // Two same-phase (pure primary) schools so both stay on one tab. const [urn0, urn1] = await twoPrimaryUrns(page); diff --git a/nextjs-app/__tests__/components/FilterSheet.test.tsx b/nextjs-app/__tests__/components/FilterSheet.test.tsx new file mode 100644 index 0000000..ca17bf0 --- /dev/null +++ b/nextjs-app/__tests__/components/FilterSheet.test.tsx @@ -0,0 +1,156 @@ +import { fireEvent, render, screen, within } from '@testing-library/react'; +import { FilterBar } from '@/components/FilterBar'; + +/* + * Phones filter through one "Filters" button and a bottom sheet holding every + * filter, rather than a sideways-scrolling row whose later chips (phase, type) + * sat off-screen beside a "More filters" panel that did not contain them. + * Which markup shows at which width is CSS and invisible to jsdom; these pin + * the behaviour and the accessible names. + */ + +let params = new URLSearchParams('postcode=SW196AR&radius=1'); +const push = jest.fn(); +jest.mock('next/navigation', () => ({ + useSearchParams: () => params, + usePathname: () => '/', + useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }), +})); +jest.mock('@/lib/analytics', () => ({ track: jest.fn() })); + +const filters = { + local_authorities: ['Wandsworth', 'Merton'], school_types: ['Community school'], years: [], + phases: ['Primary', 'Secondary'], genders: ['Girls', 'Mixed'], admissions_policies: [], +}; + +const pushedParams = () => new URLSearchParams(push.mock.calls.at(-1)![0].split('?')[1]); + +beforeEach(() => { + params = new URLSearchParams('postcode=SW196AR&radius=1'); + push.mockClear(); +}); + +describe('the phone Filters button', () => { + it('sits beside the folded search summary', () => { + render(); + const summary = screen.getByRole('button', { name: /Edit search/ }); + expect(summary.parentElement).toContainElement(screen.getByRole('button', { name: 'Filters' })); + }); + + it('counts every applied filter, phase and type included', () => { + params = new URLSearchParams('postcode=SW196AR&radius=1&phase=primary&school_type=Community+school'); + render(); + expect(screen.getByRole('button', { name: 'Filters, 2 applied' })).toBeInTheDocument(); + }); + + it('is still offered before anything has been searched', () => { + params = new URLSearchParams('local_authority=Wandsworth'); + render(); + expect(screen.getByRole('button', { name: 'Filters, 1 applied' })).toBeInTheDocument(); + }); + + it('stays out of the hero', () => { + render(); + expect(screen.queryByRole('button', { name: /^Filters/ })).not.toBeInTheDocument(); + }); +}); + +describe('the filter sheet', () => { + const openSheet = () => { + fireEvent.click(screen.getByRole('button', { name: /^Filters/ })); + return screen.getByRole('dialog', { name: 'Filters' }); + }; + + it('holds every filter in one place', () => { + render(); + const sheet = openSheet(); + expect(within(sheet).getByRole('radiogroup', { name: 'Distance' })).toBeInTheDocument(); + for (const name of ['Phase', 'School type', 'Local authority']) { + expect(within(sheet).getByRole('combobox', { name })).toBeInTheDocument(); + } + }); + + it('shows the secondary-only filters once they apply', () => { + params = new URLSearchParams('postcode=SW196AR&radius=1&phase=secondary'); + render(); + const sheet = openSheet(); + for (const name of ['Gender', 'Sixth form']) { + expect(within(sheet).getByRole('combobox', { name })).toBeInTheDocument(); + } + }); + + it('offers distance only for a postcode search', () => { + params = new URLSearchParams('search=southmead'); + render(); + expect(within(openSheet()).queryByRole('radiogroup', { name: 'Distance' })).not.toBeInTheDocument(); + }); + + it('changes the distance', () => { + render(); + const distance = within(openSheet()).getByRole('radiogroup', { name: 'Distance' }); + expect(within(distance).getByRole('radio', { name: 'Within 1 mile' })).toBeChecked(); + fireEvent.click(within(distance).getByRole('radio', { name: 'Within 3 miles' })); + expect(pushedParams().get('radius')).toBe('3'); + }); + + it('applies a change straight away and stays open for the next one', () => { + render(); + const sheet = openSheet(); + fireEvent.change(within(sheet).getByRole('combobox', { name: 'Phase' }), { target: { value: 'primary' } }); + expect(pushedParams().get('phase')).toBe('primary'); + expect(screen.getByRole('dialog', { name: 'Filters' })).toBeInTheDocument(); + }); + + it('closes on the results button, which gives the count', () => { + render(); + fireEvent.click(within(openSheet()).getByRole('button', { name: 'Show 12 schools' })); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('says so when nothing matches', () => { + render(); + expect(within(openSheet()).getByRole('button', { name: 'No schools match' })).toBeInTheDocument(); + }); + + it('clears the filters but keeps the search', () => { + params = new URLSearchParams('postcode=SW196AR&radius=1&phase=primary&local_authority=Wandsworth'); + render(); + fireEvent.click(within(openSheet()).getByRole('button', { name: 'Clear all' })); + const next = pushedParams(); + expect(next.get('phase')).toBeNull(); + expect(next.get('local_authority')).toBeNull(); + expect(next.get('postcode')).toBe('SW196AR'); + expect(next.get('radius')).toBe('1'); + }); +}); + +describe('the applied-filter chips', () => { + it('appear only when something is applied', () => { + render(); + expect(screen.queryByRole('group', { name: 'Applied filters' })).not.toBeInTheDocument(); + }); + + it('name each filter by its label and remove it on tap', () => { + params = new URLSearchParams('postcode=SW196AR&radius=1&phase=primary&gender=girls&has_sixth_form=no'); + render(); + const chips = screen.getByRole('group', { name: 'Applied filters' }); + for (const label of ['Primary', 'Girls', 'Without sixth form']) { + expect(within(chips).getByRole('button', { name: `Remove filter: ${label}` })).toBeInTheDocument(); + } + fireEvent.click(within(chips).getByRole('button', { name: 'Remove filter: Primary' })); + const next = pushedParams(); + expect(next.get('phase')).toBeNull(); + expect(next.get('gender')).toBe('girls'); + expect(next.get('postcode')).toBe('SW196AR'); + }); + + it('carry a Clear all that keeps the search', () => { + params = new URLSearchParams('search=southmead&school_type=Community+school'); + render(); + fireEvent.click(within(screen.getByRole('group', { name: 'Applied filters' })) + .getByRole('button', { name: 'Clear all' })); + const next = pushedParams(); + expect(next.get('school_type')).toBeNull(); + expect(next.get('search')).toBe('southmead'); + }); +}); diff --git a/nextjs-app/__tests__/components/ResultsToolbar.test.tsx b/nextjs-app/__tests__/components/ResultsToolbar.test.tsx index 563a252..c59842e 100644 --- a/nextjs-app/__tests__/components/ResultsToolbar.test.tsx +++ b/nextjs-app/__tests__/components/ResultsToolbar.test.tsx @@ -105,11 +105,21 @@ describe('the toolbar filters', () => { }); }); -describe('the phone filter row', () => { +describe('the phone chips line', () => { it('drops its "more this way" fade when nothing is left to scroll', () => { + params = new URLSearchParams('postcode=SW196AR&radius=1&phase=primary'); render(); - // jsdom lays nothing out, so the row reads as not overflowing at all. - expect(screen.getByRole('group', { name: 'Filters' }).className).toMatch(/controlsAtEnd/); + // jsdom lays nothing out, so the line reads as not overflowing at all. + const line = screen.getByRole('group', { name: 'Applied filters' }).parentElement!; + expect(line.className).toMatch(/controlsAtEnd/); + }); +}); + +describe('the phone filter sheet', () => { + it('offers the results total', () => { + render(); + fireEvent.click(screen.getByRole('button', { name: 'Filters' })); + expect(screen.getByRole('button', { name: 'Show 1 school' })).toBeInTheDocument(); }); }); diff --git a/nextjs-app/components/FilterBar.module.css b/nextjs-app/components/FilterBar.module.css index 4d44b08..ea2062b 100644 --- a/nextjs-app/components/FilterBar.module.css +++ b/nextjs-app/components/FilterBar.module.css @@ -95,8 +95,10 @@ } } -/* Only phones fold the form away; see the 640px block. */ -.searchSummary { +/* Only phones fold the form away and filter through the sheet; see the + 640px block. */ +.summaryRow, +.chipsLine { display: none; } @@ -348,6 +350,11 @@ font-weight: 500; } +/* In the phone sheet a select is a full-width field and a whole touch target. */ +.sheetSelect { + min-height: 2.75rem; +} + /* A pill, 44px tall: these are the page's main controls now, not fine print, and a phone needs the full touch target. */ .controlSelect { @@ -553,16 +560,16 @@ /* * Phones: the results toolbar is pinned, so it is held to two short lines. * - * After a search the form folds into a one-line summary ("SW196AR · within - * 1 mile Edit") and the controls become a single row that scrolls sideways. - * "More filters" leads the row there: it is the one control that opens - * everything else, so it must never be the chip scrolled out of sight. + * After a search the form folds into a one-line summary ("SW196AR · 1 mi + * Edit") with the Filters button beside it, and every filter lives in + * the sheet that button opens. A second line, of the applied filters as + * chips, appears only once something is applied. */ @media (max-width: 640px) { /* * nowrap matters as much as column. The desktop rule wraps, and in a * wrapping flex container each line is as wide as its widest item's content, - * not the container: the search summary ("SW196AR · within 1 mile Edit", + * not the container: the search summary (then "SW196AR · within 1 mile Edit", * about 410px) stretched the line, and the controls row with it, 40px past a * 402px iPhone 17 screen. Single-line, stretch means the container's width. */ @@ -622,13 +629,73 @@ display: none; } + /* The desktop row and its panel give way to the Filters button, the + applied-filter chips and the sheet. */ + .filterBar:not(.heroMode) .controlsRow, + .filterBar:not(.heroMode) .filters { + display: none; + } + + .summaryRow { + display: flex; + align-items: stretch; + gap: 0.5rem; + } + + .summaryRow .searchSummary { + flex: 1 1 auto; + min-width: 0; + } + + .sheetTrigger { + flex: 0 0 auto; + display: inline-flex; + align-items: center; + gap: 0.375rem; + min-height: 2.75rem; + padding: 0 0.875rem; + background: var(--bg-card); + border: 1px solid var(--text-secondary); + border-radius: var(--radius-md); + font-family: var(--font-ui); + font-size: var(--step--1); + font-weight: 600; + line-height: 1; + color: var(--text-primary); + cursor: pointer; + white-space: nowrap; + } + + /* The same brand chip as "More filters" on desktop: the list is narrowed. */ + .sheetTriggerActive { + border-color: var(--brand); + background: var(--brand-bg); + color: var(--brand); + } + + .sheetTriggerCount { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 1.25rem; + height: 1.25rem; + padding: 0 0.3rem; + border-radius: 999px; + background: var(--brand); + color: var(--bg-card); + font-size: 0.75rem; + font-weight: 700; + } + /* Bleeds to the screen edge so a chip scrolls out from under it, rather than being cut off at the toolbar's padding. The toolbar's inline padding is 1rem at this width (HomeView.module.css, .resultsToolbar). The 4px of block padding is room for focus rings, which the scroll clip would otherwise cut off above and below the chips. */ - .controlsRow { - flex-wrap: nowrap; + .chipsLine { + display: flex; + align-items: center; + gap: 0.5rem; overflow-x: auto; margin: -4px -1rem; padding: 4px 1rem; @@ -640,15 +707,50 @@ mask-image: none; } - .controlsRow::-webkit-scrollbar { + .chipsLine::-webkit-scrollbar { display: none; } - .controlsRow > * { + .chipsLine > *, + .chips > * { flex: 0 0 auto; } - .controlsRow .advancedToggle { - order: -1; + .chips { + display: flex; + align-items: center; + gap: 0.5rem; + } + + .chip { + display: inline-flex; + align-items: center; + gap: 0.375rem; + min-height: 2.75rem; + padding: 0 0.75rem 0 1rem; + background: rgba(var(--sage-rgb), 0.38); + border: 1px solid var(--brand); + border-radius: 999px; + font-family: var(--font-ui); + font-size: var(--step--1); + font-weight: 600; + line-height: 1; + color: var(--brand-strong); + cursor: pointer; + white-space: nowrap; + } + + /* A school type can run to "Academy special sponsor led". */ + .chipLabel { + max-width: 11rem; + overflow: hidden; + text-overflow: ellipsis; + } + + .chipsClear { + min-height: 2.75rem; + padding: 0 0.75rem; + font-size: var(--step--1); + font-weight: 500; } } diff --git a/nextjs-app/components/FilterBar.tsx b/nextjs-app/components/FilterBar.tsx index 25fde82..f10ab28 100644 --- a/nextjs-app/components/FilterBar.tsx +++ b/nextjs-app/components/FilterBar.tsx @@ -7,6 +7,7 @@ import { DEFAULT_RADIUS_MILES, isValidPostcode, schoolUrl } from "@/lib/utils"; import { track } from "@/lib/analytics"; import { useSchoolSuggest } from "@/hooks/useSchoolSuggest"; import { SuggestList, suggestOptionId } from "./SuggestList"; +import { FilterSheet, SheetField, RADIUS_OPTIONS, radiusLabel as milesLabel } from "./FilterSheet"; import type { Suggestion } from "@/lib/suggest"; import type { Filters, ResultFilters } from "@/lib/types"; import styles from "./FilterBar.module.css"; @@ -28,6 +29,8 @@ interface FilterBarProps { * runs the full width instead of stopping short of the switch. */ viewSwitch?: ReactNode; + /** The results' total, for the phone filter sheet's "Show N schools". */ + resultCount?: number; } /** @@ -52,6 +55,37 @@ function SelectShell({ ); } +/** The filters a phone sees as chips and counts on its Filters button. */ +const FILTER_KEYS = [ + "phase", + "school_type", + "local_authority", + "gender", + "has_sixth_form", + "admissions_policy", +] as const; +type FilterKey = (typeof FILTER_KEYS)[number]; + +function SlidersIcon() { + return ( + + ); +} + export function FilterBar({ filters, isHero, @@ -61,6 +95,7 @@ export function FilterBar({ geoError, autosuggest = false, viewSwitch, + resultCount, }: FilterBarProps) { const router = useRouter(); const pathname = usePathname(); @@ -172,9 +207,9 @@ export function FilterBar({ setOmniValue(currentQuery); } - // The phone row's right-edge fade says "more this way"; once there is no - // more, it only dims the last chip. Same rule as the school page's section - // nav (MOBILE.md, "Right-edge scroll-fade"). + // The phone chips line's right-edge fade says "more this way"; once there is + // no more, it only dims the last chip. Same rule as the school page's + // section nav (MOBILE.md, "Right-edge scroll-fade"). const controlsRowRef = useRef(null); const [controlsAtEnd, setControlsAtEnd] = useState(false); const updateControlsAtEnd = useCallback(() => { @@ -187,8 +222,8 @@ export function FilterBar({ window.addEventListener("resize", updateControlsAtEnd); return () => window.removeEventListener("resize", updateControlsAtEnd); }, [updateControlsAtEnd]); - // Chips come and go with the search (distance, Clear), so re-measure after - // every render rather than only on resize. + // Chips come and go with the filters, so re-measure after every render + // rather than only on resize. useEffect(updateControlsAtEnd); const openSearch = () => { setSearchOpen(true); @@ -281,6 +316,12 @@ export function FilterBar({ updateURL({ [key]: value }); }; + // Every filter at once, keeping the search and its distance: what "Clear + // all" means beside the applied filters, where the search is not one of them. + const handleClearFilterValues = () => { + updateURL(Object.fromEntries(FILTER_KEYS.map((k) => [k, ""]))); + }; + const handleClearFilters = () => { setOmniValue(""); startTransition(() => { @@ -317,13 +358,194 @@ export function FilterBar({ // only ever additive, so the control's behaviour is untouched. const activeIf = (value: string) => (value ? ` ${styles.selectActive}` : ""); - const radiusLabel = `${currentRadius} mile${currentRadius === "1" ? "" : "s"}`; + const radiusLabel = milesLabel(currentRadius); + + /* + * Phones filter through one button and a sheet holding every filter. The + * desktop row's phone version scrolled sideways with "More filters" first, + * so phase showed only in part and school type not at all, and the panel + * "More filters" opened did not contain either of them. + */ + const [sheetOpen, setSheetOpen] = useState(false); + const values: Record = { + phase: currentPhase, + school_type: currentType, + local_authority: currentLA, + gender: currentGender, + has_sixth_form: currentHasSixthForm, + admissions_policy: currentAdmissionsPolicy, + }; + const labelFor = (key: FilterKey, value: string) => { + if (key === "has_sixth_form") { + return value === "yes" ? "With sixth form" : "Without sixth form"; + } + // Phase, gender and admissions values are lowercased option names; the + // rest are the names themselves. + const named: Partial> = { + phase: phaseOptions, + gender: [...genderOptions, ...(filters.genders ?? [])], + admissions_policy: [ + ...admissionsPolicyOptions, + ...(filters.admissions_policies ?? []), + ], + }; + return (named[key] ?? []).find((o) => o.toLowerCase() === value.toLowerCase()) ?? value; + }; + const applied = FILTER_KEYS.filter((k) => values[k]).map((key) => ({ + key, + label: labelFor(key, values[key]), + })); + const appliedCount = applied.length; + + /* + * Each select is built here once and drawn twice: as a pill in the desktop + * row or panel, and as a full-width labelled field in the phone sheet. Only + * one copy is ever showing; the sheet's is mounted only while it is open. + */ + type Look = "pill" | "panel" | "sheet"; + const selectClass = (look: Look, value: string) => + `${look === "pill" ? styles.controlSelect : styles.filterSelect}${look === "sheet" ? ` ${styles.sheetSelect}` : ""}${activeIf(value)}`; + + const phaseSelect = (look: Look) => ( + + + + ); + + const typeSelect = (look: Look) => ( + + + + ); + + const laSelect = (look: Look) => ( + + + + ); + + const genderSelect = (look: Look) => ( + + + + ); + + const sixthFormSelect = (look: Look) => ( + + + + ); + + const admissionsSelect = (look: Look) => ( + + + + ); + + const sheetTrigger = ( + + ); + // Beside the folded search summary; on the chips line when there is no + // summary to sit beside. While the search is being edited it steps aside, + // and comes back when the form folds. + const triggerBesideSummary = canFold && !searchOpen; + const triggerOnChipsLine = !canFold; return (
- {canFold && !searchOpen && ( + {triggerBesideSummary && ( +
+ {sheetTrigger} +
)}
{viewSwitch}
)} - {/* Every control here is a real or + + ))} + + + )} + + )} + + setSheetOpen(false)} + radius={currentPostcode ? currentRadius : null} + onRadiusChange={(r) => updateURL({ radius: r })} + onClearAll={appliedCount > 0 ? handleClearFilterValues : undefined} + resultCount={resultCount} + isPending={isPending} + > + {phaseOptions.length > 0 && ( + {phaseSelect("sheet")} + )} + {typeSelect("sheet")} + {laSelect("sheet")} + {isSecondaryMode && ( + <> + {genderOptions.length > 0 && ( + {genderSelect("sheet")} + )} + {sixthFormSelect("sheet")} + {admissionsPolicyOptions.length > 0 && ( + {admissionsSelect("sheet")} + )} + + )} + )} diff --git a/nextjs-app/components/FilterSheet.module.css b/nextjs-app/components/FilterSheet.module.css new file mode 100644 index 0000000..73491be --- /dev/null +++ b/nextjs-app/components/FilterSheet.module.css @@ -0,0 +1,96 @@ +/* ── Phone filter sheet ───────────────────────────────────────────── */ + +.body { + display: flex; + flex-direction: column; + gap: 1rem; + padding: 1rem; +} + +.field { + display: flex; + flex-direction: column; + gap: 0.375rem; +} + +.fieldLabel { + font-family: var(--font-ui); + font-size: var(--step--1); + font-weight: 600; + color: var(--text-primary); +} + +/* Distance: five joined segments, a full-width row. At 360px each is about + 65px wide, clear of the 44px minimum. */ +.segments { + display: flex; + border: 1px solid var(--border-strong); + border-radius: var(--radius-sm); + overflow: hidden; +} + +.segment { + position: relative; + flex: 1 1 0; + display: flex; +} + +.segment + .segment { + border-left: 1px solid var(--border); +} + +/* The real radio stays in the tree for keyboard and screen readers; the span + is what is drawn. */ +.segment input { + position: absolute; + inset: 0; + margin: 0; + opacity: 0; + cursor: pointer; +} + +.segment span { + flex: 1; + display: flex; + align-items: center; + justify-content: center; + min-height: 2.75rem; + font-family: var(--font-ui); + font-size: var(--step--1); + font-weight: 500; + color: var(--text-primary); + background: var(--bg-card); + white-space: nowrap; + transition: background-color var(--transition), color var(--transition); +} + +.segment input:checked + span { + background: rgba(var(--sage-rgb), 0.38); + color: var(--brand-strong); + font-weight: 700; +} + +.segment input:focus-visible + span { + box-shadow: inset 0 0 0 2px var(--brand); +} + +.segment input:disabled { + cursor: progress; +} + +.footer { + display: flex; + align-items: center; + gap: 0.75rem; +} + +.clearAll { + min-height: 2.75rem; + padding: 0 1rem; +} + +.show { + flex: 1; + min-height: 2.75rem; + justify-content: center; +} diff --git a/nextjs-app/components/FilterSheet.tsx b/nextjs-app/components/FilterSheet.tsx new file mode 100644 index 0000000..6229a09 --- /dev/null +++ b/nextjs-app/components/FilterSheet.tsx @@ -0,0 +1,121 @@ +"use client"; + +import { useId } from "react"; +import type { ReactNode } from "react"; +import { Modal } from "./Modal"; +import styles from "./FilterSheet.module.css"; + +export const RADIUS_OPTIONS = ["0.25", "0.5", "1", "3", "5"] as const; + +export const radiusLabel = (r: string) => `${r} mile${r === "1" ? "" : "s"}`; + +interface FilterSheetProps { + open: boolean; + onClose: () => void; + /** The current distance, or null when the search is not by postcode. */ + radius: string | null; + onRadiusChange: (radius: string) => void; + /** The filter selects, each already labelled; FilterBar owns their state. */ + children: ReactNode; + /** Shown only when something is applied. */ + onClearAll?: () => void; + resultCount?: number; + isPending: boolean; +} + +/* + * Every filter in one place, for phones. A change applies at once, as it does + * in the desktop toolbar, so the results behind the sheet are already the + * filtered ones and the footer button only has to close it. + */ +export function FilterSheet({ + open, + onClose, + radius, + onRadiusChange, + children, + onClearAll, + resultCount, + isPending, +}: FilterSheetProps) { + const distanceId = useId(); + + const showLabel = isPending + ? "Updating…" + : resultCount === undefined + ? "Show schools" + : resultCount === 0 + ? "No schools match" + : `Show ${resultCount.toLocaleString()} school${resultCount === 1 ? "" : "s"}`; + + return ( + + {onClearAll && ( + + )} + + + } + > +
+ {radius !== null && ( +
+ + Distance + +
+ {RADIUS_OPTIONS.map((r) => ( + + ))} +
+
+ )} + {children} +
+ + ); +} + +/** One labelled row of the sheet. */ +export function SheetField({ label, children }: { label: string; children: ReactNode }) { + return ( + + ); +} diff --git a/nextjs-app/components/HomeView.tsx b/nextjs-app/components/HomeView.tsx index 3ea0b4f..7e0c0fb 100644 --- a/nextjs-app/components/HomeView.tsx +++ b/nextjs-app/components/HomeView.tsx @@ -712,6 +712,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed filters={filters} isHero={false} resultFilters={initialSchools.result_filters} + resultCount={initialSchools.total} onNearMe={handleNearMe} geoState={geoState} geoError={geoError} diff --git a/nextjs-app/components/Modal.module.css b/nextjs-app/components/Modal.module.css index 05b8275..d335a75 100644 --- a/nextjs-app/components/Modal.module.css +++ b/nextjs-app/components/Modal.module.css @@ -5,7 +5,9 @@ display: flex; align-items: center; justify-content: center; - z-index: 1000; + /* Above the pinned results toolbar (1001), the floating List/Map button + (1002) and the comparison toast (2000): a modal covers the page. */ + z-index: 2100; padding: 1rem; animation: fadeIn 0.2s ease; } @@ -31,6 +33,10 @@ border: 1px solid var(--border); } +.modal:focus { + outline: none; +} + @keyframes slideIn { from { transform: translateY(-20px); @@ -96,6 +102,14 @@ flex: 1; } +/* Clears the iPhone home indicator when the modal is a bottom sheet. */ +.footer { + flex-shrink: 0; + padding: 0.75rem 1rem calc(0.75rem + env(safe-area-inset-bottom, 0px)); + border-top: 1px solid var(--border); + background: var(--bg-card); +} + /* Scrollbar styles */ .content::-webkit-scrollbar { width: 8px; diff --git a/nextjs-app/components/Modal.tsx b/nextjs-app/components/Modal.tsx index bac310f..78582ed 100644 --- a/nextjs-app/components/Modal.tsx +++ b/nextjs-app/components/Modal.tsx @@ -5,7 +5,7 @@ 'use client'; -import { useEffect, useCallback, useRef } from 'react'; +import { useEffect, useCallback, useId, useRef } from 'react'; import { createPortal } from 'react-dom'; import styles from './Modal.module.css'; @@ -15,10 +15,14 @@ interface ModalProps { children: React.ReactNode; title?: string; size?: 'small' | 'medium' | 'large'; + /** Pinned below the scrolling content, so its actions stay in reach. */ + footer?: React.ReactNode; } -export function Modal({ isOpen, onClose, children, title, size = 'medium' }: ModalProps) { +export function Modal({ isOpen, onClose, children, title, size = 'medium', footer }: ModalProps) { const overlayRef = useRef(null); + const dialogRef = useRef(null); + const titleId = useId(); const handleEscape = useCallback((e: KeyboardEvent) => { if (e.key === 'Escape') { @@ -41,6 +45,17 @@ export function Modal({ isOpen, onClose, children, title, size = 'medium' }: Mod }; }, [isOpen, handleEscape]); + // Focus moves into the dialog when it opens and back to whatever opened it + // when it closes. A child that has already taken focus (an autoFocus input) + // keeps it: children's effects run before this one. + useEffect(() => { + if (!isOpen) return; + const opener = document.activeElement as HTMLElement | null; + const dialog = dialogRef.current; + if (dialog && !dialog.contains(document.activeElement)) dialog.focus(); + return () => opener?.focus?.(); + }, [isOpen]); + // Pin the overlay to the VISUAL viewport, not the layout viewport. On mobile // the on-screen keyboard shrinks the visual viewport but not the layout one, // so a `position: fixed; inset: 0` overlay keeps full height — leaving the @@ -77,9 +92,16 @@ export function Modal({ isOpen, onClose, children, title, size = 'medium' }: Mod return createPortal(
-
+
- {title &&

{title}

} + {title &&

{title}

}
+ {footer &&
{footer}
}
, document.body From fa491641438c070d1719602241c7910104ac8c6a Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 2 Oct 2026 09:40:29 +0100 Subject: [PATCH 2/2] fix(search): keep the filter sheet usable while a change lands The sheet's controls were disabled while a filter change navigated, and a control disabled under focus drops it to , out of the dialog. They now stay enabled, with aria-busy on the sheet instead. The disabling had also been covering a race: updateURL built from useSearchParams, which only catches up once a navigation lands, so a second change made before then undid the first. It now builds on the URL the navigation in flight is heading to. The sheet also closes if the screen widens past phone width while it is open, so its selects and the desktop row's are never both showing. Co-Authored-By: Claude Opus 5.5 --- .../components/FilterSheetPending.test.tsx | 80 +++++++++++++++++++ nextjs-app/components/FilterBar.tsx | 42 ++++++++-- nextjs-app/components/FilterSheet.module.css | 4 - nextjs-app/components/FilterSheet.tsx | 6 +- 4 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 nextjs-app/__tests__/components/FilterSheetPending.test.tsx diff --git a/nextjs-app/__tests__/components/FilterSheetPending.test.tsx b/nextjs-app/__tests__/components/FilterSheetPending.test.tsx new file mode 100644 index 0000000..19dd2e7 --- /dev/null +++ b/nextjs-app/__tests__/components/FilterSheetPending.test.tsx @@ -0,0 +1,80 @@ +import { act, fireEvent, render, screen, within } from '@testing-library/react'; +import { FilterBar } from '@/components/FilterBar'; + +/* + * While a filter change is navigating, the sheet's controls stay enabled: a + * control disabled under the user's focus drops it to , and a keyboard or + * screen-reader user is thrown out of the sheet after every change. A second + * change made before the first lands must build on the first, not on the URL + * useSearchParams still reports. + */ + +// Every transition stays pending, as a slow server render would. +jest.mock('react', () => ({ + ...jest.requireActual('react'), + useTransition: () => [true, (fn: () => void) => fn()], +})); + +const params = new URLSearchParams('postcode=SW196AR&radius=1'); +const push = jest.fn(); +jest.mock('next/navigation', () => ({ + useSearchParams: () => params, + usePathname: () => '/', + useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }), +})); +jest.mock('@/lib/analytics', () => ({ track: jest.fn() })); + +const filters = { + local_authorities: ['Wandsworth'], school_types: ['Community school'], years: [], + phases: ['Primary', 'Secondary'], genders: [], admissions_policies: [], +}; + +const pushedParams = () => new URLSearchParams(push.mock.calls.at(-1)![0].split('?')[1]); + +const openSheet = () => { + fireEvent.click(screen.getByRole('button', { name: /^Filters/ })); + return screen.getByRole('dialog', { name: 'Filters' }); +}; + +beforeEach(() => push.mockClear()); + +it('keeps the sheet usable, and says it is busy, while a change lands', () => { + render(); + const sheet = openSheet(); + for (const name of ['Phase', 'School type', 'Local authority']) { + expect(within(sheet).getByRole('combobox', { name })).toBeEnabled(); + } + expect(within(sheet).getByRole('radio', { name: 'Within 3 miles' })).toBeEnabled(); + expect(sheet.querySelector('[aria-busy="true"]')).not.toBeNull(); +}); + +it('builds a second change on the first, not on the URL it has not reached', () => { + render(); + const sheet = openSheet(); + fireEvent.change(within(sheet).getByRole('combobox', { name: 'Phase' }), { target: { value: 'primary' } }); + fireEvent.change(within(sheet).getByRole('combobox', { name: 'School type' }), + { target: { value: 'Community school' } }); + const next = pushedParams(); + expect(next.get('phase')).toBe('primary'); + expect(next.get('school_type')).toBe('Community school'); + expect(next.get('postcode')).toBe('SW196AR'); +}); + +describe('a screen that widens past phone width', () => { + let listeners: ((e: { matches: boolean }) => void)[] = []; + beforeEach(() => { + listeners = []; + window.matchMedia = jest.fn().mockImplementation((query: string) => ({ + matches: true, media: query, + addEventListener: (_: string, l: (e: { matches: boolean }) => void) => listeners.push(l), + removeEventListener: jest.fn(), + })); + }); + + it('closes the sheet, leaving the desktop row as the only filters', () => { + render(); + openSheet(); + act(() => listeners.forEach((l) => l({ matches: false }))); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); +}); diff --git a/nextjs-app/components/FilterBar.tsx b/nextjs-app/components/FilterBar.tsx index f10ab28..036b7fa 100644 --- a/nextjs-app/components/FilterBar.tsx +++ b/nextjs-app/components/FilterBar.tsx @@ -247,9 +247,21 @@ export function FilterBar({ return () => document.removeEventListener("keydown", handleKeyDown); }, []); + /* + * Where the navigation in flight is headed. useSearchParams only catches up + * once a navigation lands, so a second change made before then (the phone + * sheet stays usable while one lands) would otherwise be built on the old + * URL and undo the first. Dropped once nothing is pending, when + * searchParams is current again. + */ + const pendingQueryRef = useRef(null); + useEffect(() => { + if (!isPending) pendingQueryRef.current = null; + }, [isPending]); + const updateURL = useCallback( (updates: Record) => { - const params = new URLSearchParams(searchParams); + const params = new URLSearchParams(pendingQueryRef.current ?? searchParams); Object.entries(updates).forEach(([key, value]) => { if (value && value !== "") { @@ -260,6 +272,7 @@ export function FilterBar({ }); params.delete("page"); + pendingQueryRef.current = params.toString(); startTransition(() => { router.push(`${pathname}?${params.toString()}`); @@ -324,6 +337,7 @@ export function FilterBar({ const handleClearFilters = () => { setOmniValue(""); + pendingQueryRef.current = ""; startTransition(() => { router.push(pathname); }); @@ -367,6 +381,18 @@ export function FilterBar({ * "More filters" opened did not contain either of them. */ const [sheetOpen, setSheetOpen] = useState(false); + // The sheet is phone furniture: if the screen widens past phone width while + // it is open (rotation, a resized window), close it, or its selects and the + // desktop row's would both be showing. + useEffect(() => { + if (!sheetOpen || typeof window.matchMedia !== "function") return; + const phone = window.matchMedia("(max-width: 640px)"); + const onChange = (e: { matches: boolean }) => { + if (!e.matches) setSheetOpen(false); + }; + phone.addEventListener("change", onChange); + return () => phone.removeEventListener("change", onChange); + }, [sheetOpen]); const values: Record = { phase: currentPhase, school_type: currentType, @@ -401,6 +427,8 @@ export function FilterBar({ * Each select is built here once and drawn twice: as a pill in the desktop * row or panel, and as a full-width labelled field in the phone sheet. Only * one copy is ever showing; the sheet's is mounted only while it is open. + * The sheet's stay enabled while a change lands: disabling the control + * under the user's focus would drop focus to , out of the dialog. */ type Look = "pill" | "panel" | "sheet"; const selectClass = (look: Look, value: string) => @@ -413,7 +441,7 @@ export function FilterBar({ onChange={(e) => handleFilterChange("phase", e.target.value)} className={selectClass(look, currentPhase)} aria-label="Phase" - disabled={isPending} + disabled={isPending && look !== "sheet"} > {phaseOptions.map((p) => ( @@ -432,7 +460,7 @@ export function FilterBar({ onChange={(e) => handleFilterChange("school_type", e.target.value)} className={selectClass(look, currentType)} aria-label="School type" - disabled={isPending} + disabled={isPending && look !== "sheet"} > {typeOptions.map((type) => ( @@ -451,7 +479,7 @@ export function FilterBar({ onChange={(e) => handleFilterChange("local_authority", e.target.value)} className={selectClass(look, currentLA)} aria-label="Local authority" - disabled={isPending} + disabled={isPending && look !== "sheet"} > {laOptions.map((la) => ( @@ -470,7 +498,7 @@ export function FilterBar({ onChange={(e) => handleFilterChange("gender", e.target.value)} className={selectClass(look, currentGender)} aria-label="Gender" - disabled={isPending} + disabled={isPending && look !== "sheet"} > {genderOptions.map((g) => ( @@ -489,7 +517,7 @@ export function FilterBar({ onChange={(e) => handleFilterChange("has_sixth_form", e.target.value)} className={selectClass(look, currentHasSixthForm)} aria-label="Sixth form" - disabled={isPending} + disabled={isPending && look !== "sheet"} > @@ -505,7 +533,7 @@ export function FilterBar({ onChange={(e) => handleFilterChange("admissions_policy", e.target.value)} className={selectClass(look, currentAdmissionsPolicy)} aria-label="Admissions" - disabled={isPending} + disabled={isPending && look !== "sheet"} > {admissionsPolicyOptions.map((p) => ( diff --git a/nextjs-app/components/FilterSheet.module.css b/nextjs-app/components/FilterSheet.module.css index 73491be..95c24db 100644 --- a/nextjs-app/components/FilterSheet.module.css +++ b/nextjs-app/components/FilterSheet.module.css @@ -74,10 +74,6 @@ box-shadow: inset 0 0 0 2px var(--brand); } -.segment input:disabled { - cursor: progress; -} - .footer { display: flex; align-items: center; diff --git a/nextjs-app/components/FilterSheet.tsx b/nextjs-app/components/FilterSheet.tsx index 6229a09..df72421 100644 --- a/nextjs-app/components/FilterSheet.tsx +++ b/nextjs-app/components/FilterSheet.tsx @@ -61,7 +61,6 @@ export function FilterSheet({ type="button" className={`btn btn-tertiary ${styles.clearAll}`} onClick={onClearAll} - disabled={isPending} > Clear all @@ -76,7 +75,9 @@ export function FilterSheet({
} > -
+ {/* Controls stay enabled while a change lands, so focus is never + dropped out of the dialog; aria-busy says the results are updating. */} +
{radius !== null && (
@@ -95,7 +96,6 @@ export function FilterSheet({ value={r} checked={r === radius} onChange={() => onRadiusChange(r)} - disabled={isPending} aria-label={`Within ${radiusLabel(r)}`} /> {r} mi