From 02ed9e74fe45a4ab3b7654fb26d4ebe3ff9fa571 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 16:25:57 +0100 Subject: [PATCH] feat: connect the search field and the results of the Elements page to searchCatalog --- docs/AI_USAGE.md | 6 + src/components/elements/ElementsPage.test.tsx | 359 ++++++++++++++++-- src/components/elements/ElementsPage.tsx | 144 +++++-- src/components/elements/ResultsTable.test.tsx | 7 +- src/lib/elementList.ts | 112 ------ src/lib/searchOperations.test.ts | 65 ++++ src/lib/searchOperations.ts | 106 ++++++ src/tests/elementRows.ts | 110 ++++++ 8 files changed, 733 insertions(+), 176 deletions(-) create mode 100644 src/lib/searchOperations.test.ts create mode 100644 src/lib/searchOperations.ts create mode 100644 src/tests/elementRows.ts diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 6f45781..b82747c 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -473,3 +473,9 @@ page one at a time (the drawing, the warning, the disabled buttons, the not-found page, "Try again", "Trying again…", keeping the current element and the variant link) and checked that a test failed each time. + +## The search and the results of the Elements page (`src/components/elements/ElementsPage.tsx`, `src/lib/searchOperations.ts`) + +- **Tool:** Claude Code +- **What the team did:** Decided that making the page work is done in parts, each with its own issue and pull request, and that the first part is the search field and the results (#117). Chose that the pull request is stacked on the design (#114) until that is merged. +- **What Claude did:** Wrote the query `searchCatalog` and the page with the results, the loading, error and empty states, the debounce and the search in the address. Moved the example rows to a fixture for the tests. Wrote the tests on the real Apollo Client with a stub for `fetch`, and checked them by breaking five behaviors one at a time (the tests fail without each of them). Checked in a browser against the real backend: the first 20 of 562, a search, no result, a search from the address, and 9 words. diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index 36ed1dd..0d426d8 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -1,61 +1,163 @@ // @vitest-environment jsdom +// The page runs on the real Apollo Client, with a stub for `fetch` that answers like +// the server (see src/tests/apollo.ts and "Search, filter, sort and pagination" in +// docs/API.md). -import { cleanup, fireEvent, render, screen } from '@testing-library/react' -import { MemoryRouter } from 'react-router-dom' +import { ApolloProvider } from '@apollo/client/react' +import { + cleanup, + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react' +import { MemoryRouter, useLocation, useNavigate } from 'react-router-dom' import { afterEach, describe, expect, it } from 'vitest' -import { exampleElementRows, exampleTotalCount } from '@/lib/elementList' +import type { ElementListRow } from '@/lib/elementList' +import { MAX_SEARCH_LENGTH, PAGE_SIZE } from '@/lib/searchOperations' +import { + createTestClient, + deferred, + failure, + ok, + type Answer, + type Handler, +} from '@/tests/apollo' +import { exampleElementRows, exampleTotalCount } from '@/tests/elementRows' import ElementsPage from './ElementsPage' afterEach(cleanup) -const renderPage = () => +/** What the server says for one row (an Element has `value`, a Vault has `dValue`). */ +const nodeOf = (row: ElementListRow) => ({ + __typename: row.isVault ? 'Vault' : 'Element', + id: row.id, + code: row.code, + name: row.name, + description: row.description, + imageUrl: row.imageUrl, + elementGroup: { + __typename: 'ElementGroup', + name: row.elementGroupName, + number: row.elementGroupNumber, + apparatus: { __typename: 'Apparatus', name: row.apparatusName }, + }, + ...(row.isVault ? { dValue: row.value } : { value: row.value }), + variants: row.hasVariants + ? [{ __typename: 'Element', id: `${row.id}-x` }] + : [], +}) + +/** What the server says to `SearchCatalog`. */ +const searchAnswer = ( + rows: ElementListRow[], + totalCount = rows.length, +): Answer => + ok({ + searchCatalog: { + __typename: 'CatalogConnection', + totalCount, + pageInfo: { __typename: 'PageInfo', hasNextPage: false, endCursor: null }, + edges: rows.map((row) => ({ + __typename: 'CatalogEdge', + cursor: row.id, + node: nodeOf(row), + })), + }, + }) + +/** The server has seven rows, and finds only those with the search text in them. */ +const serverWithRows: Handler = ({ variables }) => { + const text = String(variables['search'] ?? '').toLowerCase() + const found = exampleElementRows.filter((row) => + `${row.code} ${row.name ?? ''} ${row.description}` + .toLowerCase() + .includes(text), + ) + return searchAnswer(found, text ? found.length : exampleTotalCount) +} + +/** Shows the address, so a test can see that the search is in it. */ +function Address() { + const location = useLocation() + return ( + {location.pathname + location.search} + ) +} + +/** A button that goes to another address, like the back button does. */ +function GoTo({ to }: { to: string }) { + const navigate = useNavigate() + return ( + + ) +} + +function renderPage(handler: Handler, address = '/elements', extra?: string) { + const { client, calls } = createTestClient(handler) render( - - - , + + + +
+ {extra && } + + , ) + return { calls } +} + +const field = () => screen.getByRole('searchbox') +const address = () => screen.getByTestId('address').textContent +const wait = { timeout: 3000 } -describe('ElementsPage', () => { - it('has one h1, and the results under an h2 that names the section', () => { - renderPage() +describe('ElementsPage: the page', () => { + it('has one h1, and the results under an h2 that names the section', async () => { + renderPage(serverWithRows) expect(screen.getAllByRole('heading', { level: 1 })).toHaveLength(1) expect(screen.getByRole('region', { name: 'Results' })).toBeTruthy() expect( screen.getByRole('heading', { level: 2, name: 'Results' }), ).toBeTruthy() + await screen.findByText(/Showing/) }) - it('has a labelled search field in a search landmark', () => { - renderPage() + it('has a labelled search field in a search landmark, that takes at most 80 characters', async () => { + renderPage(serverWithRows) - const search = screen.getByRole('search') - expect(search).toBeTruthy() + expect(screen.getByRole('search')).toBeTruthy() expect( screen.getByLabelText('Search elements by code, name or description'), - ).toBeTruthy() + ).toBe(field()) + expect(field().maxLength).toBe(MAX_SEARCH_LENGTH) + await screen.findByText(/Showing/) }) - it('has the filters in a labelled group, and a sort button that says what it sorts', () => { - renderPage() + it('has the filters in a labelled group, and a sort button that says what it sorts', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) const filters = screen.getByRole('group', { name: 'Filters' }) for (const name of ['Apparatus', 'Element group', 'Difficulty']) { - expect(filters.querySelector('button')?.textContent).toBeTruthy() - expect(screen.getByRole('button', { name })).toBeTruthy() + expect(within(filters).getByRole('button', { name })).toBeTruthy() } expect( screen.getByRole('button', { name: /Sort by Code\W+Ascending/ }), ).toBeTruthy() }) - it('can reach every control with the keyboard, and each one shows where the focus is', () => { - renderPage() + it('can reach every control with the keyboard, and each one shows where the focus is', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) const controls = [ ...screen.getAllByRole('button'), ...screen.getAllByRole('link'), - screen.getByRole('searchbox'), + field(), ] // The search field, the three filters, the sort button, "Load more", and each row // twice: in the table and in a card (CSS shows only one of them, and what is not @@ -76,19 +178,10 @@ describe('ElementsPage', () => { } }) - it('shows how many of the results are loaded', () => { - renderPage() - - expect( - screen.getByText( - `Showing ${exampleElementRows.length} of ${exampleTotalCount}`, - ), - ).toBeTruthy() - }) - - it('says so in a status when a control is not connected yet, and not before', () => { - renderPage() - const status = screen.getByRole('status') + it('says so in a status when a control is not connected yet, and not before', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) + const status = screen.getByTestId('controls-status') expect(status.textContent).toBe('') fireEvent.click(screen.getByRole('button', { name: 'Apparatus' })) @@ -105,8 +198,202 @@ describe('ElementsPage', () => { expect(status.textContent).toBe( 'Loading more elements is not connected yet.', ) + }) +}) + +describe('ElementsPage: the results from the API', () => { + it('asks for the first page, with no search text, and shows what the server says', async () => { + const { calls } = renderPage(serverWithRows) + + expect( + await screen.findByText( + `Showing ${exampleElementRows.length} of ${exampleTotalCount}`, + ), + ).toBeTruthy() + expect(calls).toHaveLength(1) + expect(calls[0]).toMatchObject({ + operationName: 'SearchCatalog', + variables: { search: null, first: PAGE_SIZE }, + }) + // Each row is in the table and in a card. + expect(screen.getAllByText(exampleElementRows[1].description)).toHaveLength( + 2, + ) + expect(screen.getAllByText('Yurchenko').length).toBeGreaterThan(0) + }) + + it('shows the fields of the API in each row: the group number of a vault, the value, the drawing and the variants', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) + + const vault = exampleElementRows.find((row) => row.isVault)! + const withVariants = exampleElementRows.find((row) => row.hasVariants)! + // The group of a vault is its number, and not the long title of the group. + expect( + screen.getAllByText(`Group ${vault.elementGroupNumber}`).length, + ).toBeGreaterThan(0) + expect(screen.queryByText(vault.elementGroupName)).toBeNull() + // The drawing is what the API says (imageUrl). + const images = screen.getAllByRole('img') + expect(images.map((image) => image.src)).toContain(vault.imageUrl) + // "More variants" only where the API lists variants (a card and a row). + expect(withVariants.hasVariants).toBe(true) + expect(screen.getAllByText('More variants')).toHaveLength(2) + }) + + it('says that the elements are loading, and then shows them', async () => { + const answer = deferred() + renderPage(() => answer.promise) + + expect( + (await screen.findByText('Loading elements…')).getAttribute('role'), + ).toBe('status') + expect( + screen.getByRole('region', { name: 'Results' }).getAttribute('aria-busy'), + ).toBe('true') + + answer.resolve(searchAnswer(exampleElementRows)) + expect(await screen.findByText(/Showing/)).toBeTruthy() + expect(screen.queryByText('Loading elements…')).toBeNull() + }) + + it('says so when nothing is found, in a status a screen reader reads, with what was searched for', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) + + fireEvent.change(field(), { target: { value: 'zzz' } }) + const empty = await screen.findByText('No elements match “zzz”.', {}, wait) + + expect(empty.getAttribute('role')).toBe('status') + expect(screen.queryByText(/Showing/)).toBeNull() + }) + + it('shows the error with "Try again" when the server fails, and loads again when the user tries again', async () => { + let attempt = 0 + const { calls } = renderPage(() => + ++attempt === 1 + ? failure('INTERNAL_SERVER_ERROR', 'Something broke') + : searchAnswer(exampleElementRows), + ) + const alert = await screen.findByRole('alert') + expect(alert.textContent).toBe( + 'Something went wrong on our side. Try again.', + ) + fireEvent.click(screen.getByRole('button', { name: 'Try again' })) + + expect(await screen.findByText(/Showing/)).toBeTruthy() + expect(screen.queryByRole('alert')).toBeNull() + expect(calls).toHaveLength(2) + }) + + it('shows what the server says when the search text is not allowed, without "Try again"', async () => { + renderPage(() => failure('BAD_USER_INPUT', 'Use at most 8 words.')) + + expect((await screen.findByRole('alert')).textContent).toBe( + 'Use at most 8 words.', + ) + expect(screen.queryByRole('button', { name: 'Try again' })).toBeNull() + }) +}) + +describe('ElementsPage: the search', () => { + it('sends the text when the user has stopped typing, and not for each letter', async () => { + const { calls } = renderPage(serverWithRows) + await screen.findByText(/Showing/) + + for (const text of ['y', 'yu', 'yur', 'yurchenko']) { + fireEvent.change(field(), { target: { value: text } }) + } + + await waitFor(() => expect(calls).toHaveLength(2), wait) + expect(calls[1].variables).toMatchObject({ + search: 'yurchenko', + first: PAGE_SIZE, + }) + expect(await screen.findByText('Showing 1 of 1', {}, wait)).toBeTruthy() + }) + + it('puts the search in the address, so a result can be shared, and takes it out when the field is empty', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) + expect(address()).toBe('/elements') + + fireEvent.change(field(), { target: { value: ' Yurchenko ' } }) + await waitFor( + () => expect(address()).toBe('/elements?search=Yurchenko'), + wait, + ) + + fireEvent.change(field(), { target: { value: '' } }) + await waitFor(() => expect(address()).toBe('/elements'), wait) + expect( + await screen.findByText( + `Showing ${exampleElementRows.length} of ${exampleTotalCount}`, + {}, + wait, + ), + ).toBeTruthy() + }) + + it('starts with the search of the address, in the field and in the request', async () => { + const { calls } = renderPage(serverWithRows, '/elements?search=amanar') + + expect(field().value).toBe('amanar') + expect(await screen.findByText('Showing 1 of 1')).toBeTruthy() + expect(calls[0].variables).toMatchObject({ search: 'amanar' }) + }) + + it('searches at once when the user presses Enter, and does not wait', async () => { + const { calls } = renderPage(serverWithRows) + await screen.findByText(/Showing/) + + fireEvent.change(field(), { target: { value: 'biles' } }) + fireEvent.submit(screen.getByRole('search')) + + expect(address()).toBe('/elements?search=biles') + await waitFor(() => expect(calls).toHaveLength(2)) + expect(calls[1].variables).toMatchObject({ search: 'biles' }) + }) + + it('keeps the old results while a new search is on its way', async () => { + const second = deferred() + renderPage(({ variables }) => + variables['search'] ? second.promise : searchAnswer(exampleElementRows), + ) + await screen.findByText(/Showing/) + + fireEvent.change(field(), { target: { value: 'biles' } }) fireEvent.submit(screen.getByRole('search')) - expect(status.textContent).toBe('Search is not connected yet.') + + // The new answer has not come, and the old results are still there. + await waitFor(() => expect(address()).toBe('/elements?search=biles')) + expect( + screen.getByText( + `Showing ${exampleElementRows.length} of ${exampleElementRows.length}`, + ), + ).toBeTruthy() + expect(screen.queryByText('Loading elements…')).toBeNull() + + second.resolve(searchAnswer([exampleElementRows[5]])) + expect(await screen.findByText('Showing 1 of 1')).toBeTruthy() + }) + + it('follows the address when the user goes back or forward: the field and the results change', async () => { + const { calls } = renderPage( + serverWithRows, + '/elements', + '/elements?search=amanar', + ) + await screen.findByText(/Showing/) + expect(field().value).toBe('') + + fireEvent.click( + screen.getByRole('button', { name: 'go to /elements?search=amanar' }), + ) + + await waitFor(() => expect(field().value).toBe('amanar')) + expect(await screen.findByText('Showing 1 of 1', {}, wait)).toBeTruthy() + expect(calls.at(-1)?.variables).toMatchObject({ search: 'amanar' }) }) }) diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index e6bb748..c560a4c 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -1,23 +1,70 @@ -import { useState, type FormEvent } from 'react' +import { useEffect, useRef, useState, type FormEvent } from 'react' +import { useQuery } from '@apollo/client/react' +import { useSearchParams } from 'react-router-dom' import { ArrowUpDown, ChevronDown, Plus, Search } from 'lucide-react' import ResultsTable from '@/components/elements/ResultsTable' import { Button } from '@/components/ui/button' -import { exampleElementRows, exampleTotalCount } from '@/lib/elementList' +import { describeError } from '@/lib/apollo/errors' +import { + MAX_SEARCH_LENGTH, + PAGE_SIZE, + SEARCH_CATALOG_QUERY, + SEARCH_DELAY_MS, + toListRow, +} from '@/lib/searchOperations' const PILL = 'h-10 rounded-full border-0 bg-(--button-bg) px-4 text-sm font-medium text-(--button-text) shadow-(--box-shadow) transition-colors hover:bg-(--button-hover-bg) focus-visible:bg-(--button-hover-bg) dark:border-transparent dark:bg-(--button-bg) dark:hover:bg-(--button-hover-bg) hover:text-(--button-text) dark:hover:text-(--button-text) focus-visible:ring-0 focus-visible:outline-2 focus-visible:outline-solid focus-visible:outline-offset-2 focus-visible:outline-(--text-color) motion-reduce:transition-none' const FILTERS = ['Apparatus', 'Element group', 'Difficulty'] -// TODO(#110): search, filters, sort and paging are only drawn so far. Each one +// TODO(#110): the filters, the sort and "Load more" are only drawn so far. Each one // says that it is not connected yet, like the buttons on the element page. export default function ElementsPage() { const [status, setStatus] = useState('') - const rows = exampleElementRows + const [searchParams, setSearchParams] = useSearchParams() - function search(event: FormEvent) { + // The search is in the address (?search=...), so a result can be shared and the + // back button works. `text` is what is in the field, and `search` is what the + // address says (what the search uses): the address follows the field when the user + // has stopped typing. + const search = (searchParams.get('search') ?? '').trim() + const [text, setText] = useState(searchParams.get('search') ?? '') + const written = useRef(search) + + function writeAddress(value: string) { + const trimmed = value.trim() + if (trimmed === written.current) return + written.current = trimmed + setSearchParams(trimmed ? { search: trimmed } : {}, { replace: true }) + } + + useEffect(() => { + const timer = setTimeout(() => writeAddress(text), SEARCH_DELAY_MS) + return () => clearTimeout(timer) + // writeAddress only uses what is in the dependencies and the ref. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [text]) + + // The user went back or forward to another address: the field follows it. + useEffect(() => { + if (search !== written.current) { + written.current = search + setText(search) + } + }, [search]) + + const { data, previousData, loading, error, refetch } = useQuery( + SEARCH_CATALOG_QUERY, + { variables: { search: search || null, first: PAGE_SIZE } }, + ) + // While a new search is on its way the old results stay, so the page does not jump. + const shown = (data ?? previousData)?.searchCatalog + const rows = shown?.edges.map((edge) => toListRow(edge.node)) ?? [] + + function submit(event: FormEvent) { event.preventDefault() - setStatus('Search is not connected yet.') + writeAddress(text) } return ( @@ -26,7 +73,7 @@ export default function ElementsPage() {