From 8b76b5dd447394f993f6efa72cdb89506562ebbf Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 16:38:48 +0100 Subject: [PATCH 1/2] feat: make Load more elements fetch the next page, and disable it when everything is shown --- docs/AI_USAGE.md | 6 + src/components/elements/ElementsPage.test.tsx | 187 ++++++++++++++++-- src/components/elements/ElementsPage.tsx | 55 +++++- src/lib/searchOperations.ts | 110 ++++++++--- 4 files changed, 306 insertions(+), 52 deletions(-) diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index b82747c..c84c4bf 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -479,3 +479,9 @@ - **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. + +## Load more elements (`src/components/elements/ElementsPage.tsx`, `src/lib/searchOperations.ts`) + +- **Tool:** Claude Code +- **What the team did:** Decided that "Load more elements" is disabled when everything is shown ("Showing N of N"), and that all the work to make the page work is one issue (#110), in several pull requests. +- **What Claude did:** Wrote the query for the next page (without `totalCount`, which the API counts only when it is asked), the button with its states (loading, disabled when everything is shown, error with a new try) and the tests, and checked the tests by breaking six behaviors one at a time. Checked in a browser against the real backend: 20, 40 and 60 of 562, one result (disabled at once), and 36 of 36 for a search that has two pages. diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index 0d426d8..c785b23 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -12,10 +12,16 @@ import { waitFor, within, } from '@testing-library/react' +import { print } from 'graphql' import { MemoryRouter, useLocation, useNavigate } from 'react-router-dom' import { afterEach, describe, expect, it } from 'vitest' import type { ElementListRow } from '@/lib/elementList' -import { MAX_SEARCH_LENGTH, PAGE_SIZE } from '@/lib/searchOperations' +import { + MAX_SEARCH_LENGTH, + PAGE_SIZE, + SEARCH_CATALOG_MORE_QUERY, + SEARCH_CATALOG_QUERY, +} from '@/lib/searchOperations' import { createTestClient, deferred, @@ -53,12 +59,17 @@ const nodeOf = (row: ElementListRow) => ({ const searchAnswer = ( rows: ElementListRow[], totalCount = rows.length, + hasNextPage = false, ): Answer => ok({ searchCatalog: { __typename: 'CatalogConnection', totalCount, - pageInfo: { __typename: 'PageInfo', hasNextPage: false, endCursor: null }, + pageInfo: { + __typename: 'PageInfo', + hasNextPage, + endCursor: rows.length ? rows[rows.length - 1].id : null, + }, edges: rows.map((row) => ({ __typename: 'CatalogEdge', cursor: row.id, @@ -78,6 +89,25 @@ const serverWithRows: Handler = ({ variables }) => { return searchAnswer(found, text ? found.length : exampleTotalCount) } +/** + * A server that has the seven rows in pages of `size`: the first page, and then the + * page after the row that `after` is the cursor of (the id of a row). + */ +const pagedServer = + (size: number): Handler => + ({ variables }) => { + const after = variables['after'] + const start = after + ? exampleElementRows.findIndex((row) => row.id === after) + 1 + : 0 + const page = exampleElementRows.slice(start, start + size) + return searchAnswer( + page, + exampleElementRows.length, + start + size < exampleElementRows.length, + ) + } + /** Shows the address, so a test can see that the search is in it. */ function Address() { const location = useLocation() @@ -151,7 +181,7 @@ describe('ElementsPage: the page', () => { }) it('can reach every control with the keyboard, and each one shows where the focus is', async () => { - renderPage(serverWithRows) + renderPage(pagedServer(4)) await screen.findByText(/Showing/) const controls = [ @@ -159,10 +189,10 @@ describe('ElementsPage: the page', () => { ...screen.getAllByRole('link'), 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 - // shown cannot be reached with Tab). - expect(controls).toHaveLength(1 + 3 + 1 + 1 + 2 * exampleElementRows.length) + // The search field, the three filters, the sort button, "Load more" (there is a next + // page), and each row twice: in the table and in a card (CSS shows only one of + // them, and what is not shown cannot be reached with Tab). + expect(controls).toHaveLength(1 + 3 + 1 + 1 + 2 * 4) for (const control of controls) { expect(control.getAttribute('tabindex')).not.toBe('-1') expect(control.hasAttribute('disabled')).toBe(false) @@ -178,7 +208,7 @@ describe('ElementsPage: the page', () => { } }) - it('says so in a status when a control is not connected yet, and not before', async () => { + it('says so in a status when a filter or the sort is not connected yet, and not before', async () => { renderPage(serverWithRows) await screen.findByText(/Showing/) const status = screen.getByTestId('controls-status') @@ -193,11 +223,6 @@ describe('ElementsPage: the page', () => { screen.getByRole('button', { name: /Sort by Code\W+Ascending/ }), ) expect(status.textContent).toBe('Sorting is not connected yet.') - - fireEvent.click(screen.getByRole('button', { name: 'Load more elements' })) - expect(status.textContent).toBe( - 'Loading more elements is not connected yet.', - ) }) }) @@ -397,3 +422,139 @@ describe('ElementsPage: the search', () => { expect(calls.at(-1)?.variables).toMatchObject({ search: 'amanar' }) }) }) + +describe('ElementsPage: Load more elements', () => { + const more = () => + screen.getByRole('button', { + name: /Load more|Loading more/, + }) + const tableRows = () => document.querySelectorAll('tbody tr') + + it('fetches the next page with the cursor of the last row and adds it below, and the count follows', async () => { + const { calls } = renderPage(pagedServer(4)) + await screen.findByText('Showing 4 of 7') + expect(tableRows()).toHaveLength(4) + expect(more().disabled).toBe(false) + + fireEvent.click(more()) + + expect(await screen.findByText('Showing 7 of 7')).toBeTruthy() + expect(tableRows()).toHaveLength(7) + // The next page comes in the order of the server, after the first one. + const codes = [...tableRows()].map( + (row) => row.querySelector('td')?.textContent, + ) + expect(codes).toEqual(exampleElementRows.map((row) => row.code)) + expect(calls).toHaveLength(2) + expect(calls[1]).toMatchObject({ + operationName: 'SearchCatalogMore', + variables: { + search: null, + first: PAGE_SIZE, + after: exampleElementRows[3].id, + }, + }) + }) + + it('does not ask for the count again with the next page', () => { + expect(print(SEARCH_CATALOG_QUERY)).toContain('totalCount') + expect(print(SEARCH_CATALOG_MORE_QUERY)).not.toContain('totalCount') + }) + + it('is disabled and says so while the next page is on its way, so a double click fetches it once', async () => { + const next = deferred() + const { calls } = renderPage((operation) => + operation.variables['after'] ? next.promise : pagedServer(4)(operation), + ) + await screen.findByText('Showing 4 of 7') + + fireEvent.click(more()) + await waitFor(() => expect(more().disabled).toBe(true)) + expect(more().textContent).toBe('Loading more…') + expect(more().getAttribute('aria-busy')).toBe('true') + fireEvent.click(more()) + + next.resolve(searchAnswer(exampleElementRows.slice(4), 7)) + expect(await screen.findByText('Showing 7 of 7')).toBeTruthy() + expect(calls).toHaveLength(2) + }) + + it('is disabled when everything is shown (N of N), and the count is what describes it', async () => { + renderPage(pagedServer(10)) + const count = await screen.findByText('Showing 7 of 7') + + expect(more().disabled).toBe(true) + expect(more().getAttribute('aria-describedby')).toBe(count.id) + expect(count.id).not.toBe('') + }) + + it('becomes disabled when the last page has come', async () => { + renderPage(pagedServer(4)) + await screen.findByText('Showing 4 of 7') + + fireEvent.click(more()) + + await screen.findByText('Showing 7 of 7') + expect(more().disabled).toBe(true) + expect(more().textContent).toBe('Load more elements') + }) + + it('shows the error, keeps what is shown, and lets the user press the button again', async () => { + let attempt = 0 + const { calls } = renderPage((operation) => { + if (operation.variables['after'] && ++attempt === 1) { + return failure('INTERNAL_SERVER_ERROR', 'Something broke') + } + return pagedServer(4)(operation) + }) + await screen.findByText('Showing 4 of 7') + + fireEvent.click(more()) + + const alert = await screen.findByRole('alert') + expect(alert.textContent).toBe( + 'Something went wrong on our side. Try again. Press the button to try again.', + ) + expect(tableRows()).toHaveLength(4) + expect(more().disabled).toBe(false) + + fireEvent.click(more()) + + expect(await screen.findByText('Showing 7 of 7')).toBeTruthy() + expect(screen.queryByRole('alert')).toBeNull() + expect(calls).toHaveLength(3) + }) + + it('starts again with the first page when the search changes, and the error of the old search goes', async () => { + const { calls } = renderPage((operation) => + operation.variables['after'] && !operation.variables['search'] + ? failure('INTERNAL_SERVER_ERROR', 'Something broke') + : serverWithRowsPaged(operation), + ) + await screen.findByText('Showing 4 of 7') + fireEvent.click(more()) + await screen.findByRole('alert') + + fireEvent.change(field(), { target: { value: 'yurchenko' } }) + fireEvent.submit(screen.getByRole('search')) + + expect(await screen.findByText('Showing 1 of 1', {}, wait)).toBeTruthy() + expect(screen.queryByRole('alert')).toBeNull() + const last = calls.at(-1) + expect(last?.operationName).toBe('SearchCatalog') + expect(last?.variables).toMatchObject({ search: 'yurchenko' }) + expect(last?.variables['after']).toBeUndefined() + }) +}) + +/** A server that pages the seven rows, and also finds rows by their text. */ +function serverWithRowsPaged(operation: Parameters[0]) { + const text = String(operation.variables['search'] ?? '').toLowerCase() + if (!text) return pagedServer(4)(operation) + const found = exampleElementRows.filter((row) => + `${row.code} ${row.name ?? ''} ${row.description}` + .toLowerCase() + .includes(text), + ) + return searchAnswer(found, found.length) +} diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index c560a4c..df7a5d3 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -8,6 +8,7 @@ import { describeError } from '@/lib/apollo/errors' import { MAX_SEARCH_LENGTH, PAGE_SIZE, + SEARCH_CATALOG_MORE_QUERY, SEARCH_CATALOG_QUERY, SEARCH_DELAY_MS, toListRow, @@ -18,8 +19,8 @@ const PILL = const FILTERS = ['Apparatus', 'Element group', 'Difficulty'] -// 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. +// TODO(#110): the filters and the sort 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 [searchParams, setSearchParams] = useSearchParams() @@ -54,13 +55,40 @@ export default function ElementsPage() { } }, [search]) - const { data, previousData, loading, error, refetch } = useQuery( + const { data, previousData, loading, error, refetch, fetchMore } = 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)) ?? [] + const hasNextPage = shown?.pageInfo.hasNextPage ?? false + + // "Load more": the next page is fetched with the cursor of the last one, and the + // cache adds it to the pages that are shown (src/lib/apollo/cache.ts). An error is + // for the search it happened in, and goes away with the next search. + const [loadingMore, setLoadingMore] = useState(false) + const [moreError, setMoreError] = useState<{ + search: string + message: string + } | null>(null) + + async function loadMore() { + const after = shown?.pageInfo.endCursor + if (!after || loadingMore) return + setLoadingMore(true) + setMoreError(null) + try { + await fetchMore({ + query: SEARCH_CATALOG_MORE_QUERY, + variables: { search: search || null, first: PAGE_SIZE, after }, + }) + } catch (problem) { + setMoreError({ search, message: describeError(problem).message }) + } finally { + setLoadingMore(false) + } + } function submit(event: FormEvent) { event.preventDefault() @@ -182,24 +210,37 @@ export default function ElementsPage() {

Showing {rows.length} of {shown.totalCount}

+ {/* Not pressable while the next page is on its way (a double click would + fetch it twice), and when everything is shown (Showing N of N): the + count says why. */}
+ {moreError?.search === search && ( +

+ {moreError.message} Press the button to try again. +

+ )} )} diff --git a/src/lib/searchOperations.ts b/src/lib/searchOperations.ts index fa29b0a..9a60e90 100644 --- a/src/lib/searchOperations.ts +++ b/src/lib/searchOperations.ts @@ -14,6 +14,13 @@ export const SEARCH_DELAY_MS = 300 /** The longest search text the API takes (more gives BAD_USER_INPUT). */ export const MAX_SEARCH_LENGTH = 80 +/** What the queries are sent with. `after` is the cursor of the page to load next. */ +export type SearchVariables = { + search: string | null + first: number + after?: string | null +} + type NodeFields = { id: string code: string @@ -34,20 +41,57 @@ export type SearchNode = NodeFields & | { __typename: 'Vault'; dValue: number; variants: { id: string }[] } ) +type Page = { + pageInfo: { hasNextPage: boolean; endCursor: string | null } + edges: { cursor: string; node: SearchNode }[] +} + +/** The first page of a search, with the number of results in all. */ export type SearchCatalogResult = { - searchCatalog: { - totalCount: number - pageInfo: { hasNextPage: boolean; endCursor: string | null } - edges: { cursor: string; node: SearchNode }[] - } + searchCatalog: Page & { totalCount: number } } +/** A later page of a search. The count is not asked for again. */ +export type SearchCatalogMoreResult = { searchCatalog: Page } + +// The fields of one result, the same in both queries. +const RESULT = ` + id + code + name + description + imageUrl + elementGroup { + name + number + apparatus { + name + } + } + ... on Element { + value + variants { + id + } + } + ... on Vault { + dValue + variants { + id + } + } +` + +/** + * The first page of a search. It asks for `totalCount`, which the API counts only when + * it is asked (counting is expensive on a large result). + */ export const SEARCH_CATALOG_QUERY: TypedDocumentNode< SearchCatalogResult, - { search: string | null; first: number } + SearchVariables > = gql` - query SearchCatalog($search: String, $first: Int!) { - searchCatalog(search: $search, first: $first) { + query SearchCatalog($search: String, $first: Int!, $after: String) { + searchCatalog(search: $search, first: $first, after: $after) { totalCount pageInfo { hasNextPage @@ -56,30 +100,32 @@ export const SEARCH_CATALOG_QUERY: TypedDocumentNode< edges { cursor node { - id - code - name - description - imageUrl - elementGroup { - name - number - apparatus { - name - } - } - ... on Element { - value - variants { - id - } - } - ... on Vault { - dValue - variants { - id - } - } + ${RESULT} + } + } + } + } +` + +/** + * The next page of a search ("Load more"). It is the same field with `after`, so the + * cache adds it to the pages that are shown (see src/lib/apollo/cache.ts), and it + * does not ask for `totalCount` again: the cache keeps the first one. + */ +export const SEARCH_CATALOG_MORE_QUERY: TypedDocumentNode< + SearchCatalogMoreResult, + SearchVariables +> = gql` + query SearchCatalogMore($search: String, $first: Int!, $after: String) { + searchCatalog(search: $search, first: $first, after: $after) { + pageInfo { + hasNextPage + endCursor + } + edges { + cursor + node { + ${RESULT} } } } From 7d6295b8f228580b3c1a6f80ea87032e428863f4 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 16:44:16 +0100 Subject: [PATCH 2/2] feat: say 1000+ when the server counts more than 1000 --- src/components/elements/ElementsPage.test.tsx | 17 +++++++++++++++ src/components/elements/ElementsPage.tsx | 3 ++- src/lib/searchOperations.test.ts | 21 ++++++++++++++++++- src/lib/searchOperations.ts | 11 ++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index c785b23..e23bac5 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -247,6 +247,23 @@ describe('ElementsPage: the results from the API', () => { expect(screen.getAllByText('Yurchenko').length).toBeGreaterThan(0) }) + it('says 1000+ and not the number when the server counts more than 1000 (it stops at 1001)', async () => { + renderPage(() => searchAnswer(exampleElementRows, 1001, true)) + + expect( + await screen.findByText(`Showing ${exampleElementRows.length} of 1000+`), + ).toBeTruthy() + expect(screen.queryByText(/1001/)).toBeNull() + }) + + it('says the number as it is up to 1000', async () => { + renderPage(() => searchAnswer(exampleElementRows, 1000, true)) + + expect( + await screen.findByText(`Showing ${exampleElementRows.length} of 1000`), + ).toBeTruthy() + }) + 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/) diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index df7a5d3..e84e201 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -6,6 +6,7 @@ import ResultsTable from '@/components/elements/ResultsTable' import { Button } from '@/components/ui/button' import { describeError } from '@/lib/apollo/errors' import { + formatTotal, MAX_SEARCH_LENGTH, PAGE_SIZE, SEARCH_CATALOG_MORE_QUERY, @@ -215,7 +216,7 @@ export default function ElementsPage() { aria-live="polite" className="text-sm font-medium text-(--text-color)" > - Showing {rows.length} of {shown.totalCount} + Showing {rows.length} of {formatTotal(shown.totalCount)}

{/* Not pressable while the next page is on its way (a double click would fetch it twice), and when everything is shown (Showing N of N): the diff --git a/src/lib/searchOperations.test.ts b/src/lib/searchOperations.test.ts index 0c06240..819bd44 100644 --- a/src/lib/searchOperations.test.ts +++ b/src/lib/searchOperations.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { toListRow, type SearchNode } from './searchOperations' +import { + formatTotal, + MAX_COUNTED, + toListRow, + type SearchNode, +} from './searchOperations' const group = { name: 'Mounts', @@ -63,3 +68,17 @@ describe('toListRow', () => { }) }) }) + +describe('formatTotal', () => { + it('says the number as it is up to 1000', () => { + expect(formatTotal(0)).toBe('0') + expect(formatTotal(562)).toBe('562') + expect(formatTotal(MAX_COUNTED)).toBe('1000') + }) + + it('says 1000+ for more than 1000, which is what the server gives (1001), and for anything above', () => { + expect(MAX_COUNTED).toBe(1000) + expect(formatTotal(1001)).toBe('1000+') + expect(formatTotal(250000)).toBe('1000+') + }) +}) diff --git a/src/lib/searchOperations.ts b/src/lib/searchOperations.ts index 9a60e90..d98f01b 100644 --- a/src/lib/searchOperations.ts +++ b/src/lib/searchOperations.ts @@ -14,6 +14,17 @@ export const SEARCH_DELAY_MS = 300 /** The longest search text the API takes (more gives BAD_USER_INPUT). */ export const MAX_SEARCH_LENGTH = 80 +/** + * The most results that the server counts. A search with more matches gets one more + * (1001) as its `totalCount`, which means "more than this many" (see "Search, filter, + * sort and pagination" in docs/API.md). + */ +export const MAX_COUNTED = 1000 + +/** The number of results as the page says it: 562, or 1000+ when there are more than 1000. */ +export const formatTotal = (totalCount: number): string => + totalCount > MAX_COUNTED ? `${MAX_COUNTED}+` : String(totalCount) + /** What the queries are sent with. `after` is the cursor of the page to load next. */ export type SearchVariables = { search: string | null