diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 7d34f7b..dc25940 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -2094,12 +2094,32 @@ test('a place page lists its schools alphabetically', async ({ page }) => { expect(town).toBeTruthy(); await page.goto(`/schools/${town.slug}`); - const names = await page.locator('a[href^="/school/"]').allTextContents(); - expect(names.length).toBeGreaterThan(1); - const sorted = [...names].sort((a, b) => - a.toLowerCase().localeCompare(b.toLowerCase())); - expect(names).toEqual(sorted); + /* + * Per table, not per page. + * + * An unphased place page renders one table per phase, and an all-through + * school legitimately appears in both — so the page's school links are not + * one alphabetical run and never were. This assertion used to collect them + * all together and only passed because no town it picked happened to hold an + * all-through school; when the data gave Abbots Langley one, Breakspeare + * School showed up in the primary table and again in the secondary, and the + * test failed on correct behaviour. + */ + const tables = page.locator('table'); + const tableCount = await tables.count(); + expect(tableCount).toBeGreaterThan(0); + + let checked = 0; + for (let i = 0; i < tableCount; i++) { + const names = await tables.nth(i).locator('a[href^="/school/"]').allTextContents(); + if (names.length < 2) continue; // a one-row table says nothing about order + const sorted = [...names].sort((a, b) => + a.toLowerCase().localeCompare(b.toLowerCase())); + expect(names, `table ${i + 1} is not alphabetical`).toEqual(sorted); + checked++; + } + expect(checked, 'no table had enough rows to check the ordering').toBeGreaterThan(0); }); test('the rankings page still orders by score, not name', async ({ page }) => { @@ -2198,6 +2218,30 @@ test('the whole dropdown is reachable, not clipped by the hero', async ({ page } + '— an ancestor is clipping or covering the dropdown').toBeTruthy(); }); +test('the dropdown does not survive into the results it produced', async ({ page }) => { + /* + * The bug that took the staging gate down, and it was not a test problem: + * after a search the results-page bar still holds the term, so the dropdown + * reopened on top of the results and swallowed the click on the first one. + * Playwright reported it as "
  • intercepts pointer events"; a + * reader would simply have found their first result unclickable. + */ + test.skip(!(await autosuggestIsOn(page)), + 'the school_autosuggest flag is off in this environment'); + + await page.goto('/'); + await page.getByRole('combobox').first().fill('school'); + await expect(page.getByRole('option').first()).toBeVisible(); + + await page.getByRole('button', { name: /Search/i }).first().click(); + await page.waitForURL(/search=school/); + + await expect(page.getByRole('listbox')).toHaveCount(0); + // And the results underneath are actually reachable, which is the point. + await page.locator('a[href^="/school/"]').first().click({ timeout: 15_000 }); + await expect(page).toHaveURL(/\/school\//); +}); + test('with autosuggest off, the search box is a plain input', async ({ page }) => { test.skip(await autosuggestIsOn(page), 'the school_autosuggest flag is on in this environment'); diff --git a/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx b/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx index 70ec8ac..8e2c962 100644 --- a/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx +++ b/nextjs-app/__tests__/components/FilterBarSuggest.test.tsx @@ -3,10 +3,11 @@ import userEvent from '@testing-library/user-event'; import { FilterBar } from '@/components/FilterBar'; const push = jest.fn(); +let searchParams = new URLSearchParams(); jest.mock('next/navigation', () => ({ useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }), usePathname: () => '/', - useSearchParams: () => new URLSearchParams(), + useSearchParams: () => searchParams, })); const FILTERS = { @@ -24,6 +25,7 @@ beforeEach(() => { phase: 'Primary', school_type: 'Community school' }] }), })) as unknown as typeof fetch; push.mockClear(); + searchParams = new URLSearchParams(); }); afterEach(() => { global.fetch = realFetch; }); @@ -74,3 +76,35 @@ describe('FilterBar autosuggest', () => { expect.stringContaining('search=brecknock'))); }); }); + +describe('FilterBar autosuggest does not reopen over results', () => { + it('stays shut when the input arrives pre-filled from the URL', async () => { + /* + * The results-page bar renders with the search term already in the input. + * Opening on that would drop the dropdown on top of the results the search + * just produced — which is exactly what happened: the first result became + * unclickable, because the list sat over it and swallowed the pointer. + * + * Suggestions answer typing, not the presence of a value. + */ + searchParams = new URLSearchParams('search=brecknock'); + render(); + + expect(screen.getByRole('combobox')).toHaveValue('brecknock'); + await new Promise((r) => setTimeout(r, 300)); // past the 200ms debounce + expect(global.fetch).not.toHaveBeenCalled(); + expect(screen.queryByRole('listbox')).not.toBeInTheDocument(); + }); + + it('closes the dropdown when the search is submitted', async () => { + render(); + const input = screen.getByRole('combobox'); + + await userEvent.type(input, 'brecknock'); + expect(await screen.findByRole('listbox')).toBeInTheDocument(); + + await userEvent.type(input, '{Enter}'); + await waitFor(() => + expect(screen.queryByRole('listbox')).not.toBeInTheDocument()); + }); +}); diff --git a/nextjs-app/components/FilterBar.tsx b/nextjs-app/components/FilterBar.tsx index 5cb7fa1..74e3eb4 100644 --- a/nextjs-app/components/FilterBar.tsx +++ b/nextjs-app/components/FilterBar.tsx @@ -69,14 +69,28 @@ export function FilterBar({ const [omniValue, setOmniValue] = useState(initialOmniValue); const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`; + + /* + * Suggestions answer typing, not the mere presence of a value. + * + * Without this the results-page bar reopened the dropdown over the results: + * after a search the input still holds the term, so on every render the + * query was >= 2 characters and the list opened again — on top of the very + * results the search had just produced, swallowing the click on the first + * one. The E2E gate caught it as "
  • intercepts pointer + * events", but a reader would just have found the page unclickable. + */ + const [hasTyped, setHasTyped] = useState(false); + // Suppressed once the value parses as a postcode: the box takes a school // name OR a postcode, and suggesting schools during postcode entry fights // the user rather than helping them. - const suggestEnabled = autosuggest && !isValidPostcode(omniValue); + const suggestEnabled = autosuggest && hasTyped && !isValidPostcode(omniValue); const { suggestions, open, activeIndex, setActiveIndex, close } = useSchoolSuggest(omniValue, suggestEnabled); const pickSuggestion = (s: Suggestion) => { + setHasTyped(false); close(); track('search_submitted', { query: s.school_name.toLowerCase(), @@ -169,6 +183,9 @@ export function FilterBar({ const handleSearchSubmit = (e: React.FormEvent) => { e.preventDefault(); + // The search has been made; the suggestions that led to it are spent. + setHasTyped(false); + close(); if (!omniValue.trim()) { updateURL({ search: "", postcode: "", radius: "" }); return; @@ -271,7 +288,7 @@ export function FilterBar({ ref={inputRef} type="search" value={omniValue} - onChange={(e) => setOmniValue(e.target.value)} + onChange={(e) => { setOmniValue(e.target.value); setHasTyped(true); }} onKeyDown={handleOmniKeyDown} onBlur={close} placeholder="School name or postcode"