From 9442a9a6d1010c71d99b5a2789a206012937257e Mon Sep 17 00:00:00 2001 From: Ronja Date: Fri, 30 Oct 2026 17:04:19 +0100 Subject: [PATCH] feat: connect the sort on the Elements page --- src/components/elements/ElementsPage.test.tsx | 206 +++++++++++++++++- src/components/elements/ElementsPage.tsx | 72 ++++-- src/components/elements/SortMenu.tsx | 129 +++++++++++ src/lib/searchOperations.ts | 95 +++++++- 4 files changed, 479 insertions(+), 23 deletions(-) create mode 100644 src/components/elements/SortMenu.tsx diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index e23bac5..be16b19 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -208,7 +208,7 @@ describe('ElementsPage: the page', () => { } }) - it('says so in a status when a filter or the sort is not connected yet, and not before', async () => { + it('says so in a status when a filter is not connected yet, and not before', async () => { renderPage(serverWithRows) await screen.findByText(/Showing/) const status = screen.getByTestId('controls-status') @@ -218,11 +218,209 @@ describe('ElementsPage: the page', () => { expect(status.textContent).toBe( 'The apparatus filter is not connected yet.', ) + }) +}) - fireEvent.click( - screen.getByRole('button', { name: /Sort by Code\W+Ascending/ }), +const sortButton = (name = /Sort by Code\W+Ascending/) => + screen.getByRole('button', { name }) + +/** Opens the menu like a click does: the pointer goes down and up on the button. */ +async function openSortMenu() { + const button = sortButton(/Sort by/) + fireEvent.pointerDown(button, { button: 0, pointerType: 'mouse' }) + fireEvent.mouseDown(button, { button: 0 }) + fireEvent.pointerUp(button, { button: 0, pointerType: 'mouse' }) + fireEvent.mouseUp(button, { button: 0 }) + fireEvent.click(button, { button: 0 }) + return screen.findByRole('menu') +} + +const choose = (name: string) => + fireEvent.click(screen.getByRole('menuitemradio', { name })) + +describe('ElementsPage: the sort', () => { + it('sends no sort when it is the default (code, ascending), and keeps the address clean', async () => { + const { calls } = renderPage(serverWithRows) + await screen.findByText(/Showing/) + + expect(calls[0].variables).not.toHaveProperty('sort') + expect(address()).toBe('/elements') + }) + + it('has the fields and the directions as radio items, and the chosen ones are checked', async () => { + renderPage(serverWithRows) + await screen.findByText(/Showing/) + await openSortMenu() + + const checked = (name: string) => + screen.getByRole('menuitemradio', { name }).getAttribute('aria-checked') + for (const name of [ + 'Code', + 'Difficulty', + 'Name', + 'Ascending', + 'Descending', + ]) + expect(screen.getByRole('menuitemradio', { name })).toBeTruthy() + expect(checked('Code')).toBe('true') + expect(checked('Ascending')).toBe('true') + expect(checked('Name')).toBe('false') + expect(checked('Descending')).toBe('false') + }) + + it('sorts by name: sends it, puts it in the address and says it on the button', async () => { + const { calls } = renderPage(serverWithRows) + await screen.findByText(/Showing/) + await openSortMenu() + choose('Name') + + await waitFor(() => expect(address()).toBe('/elements?sort=name')) + await waitFor(() => + expect(calls.at(-1)!.variables).toMatchObject({ + sort: { field: 'NAME', direction: 'ASC' }, + }), + ) + expect(sortButton(/Sort by Name\W+Ascending/)).toBeTruthy() + }) + + it('sorts descending, and goes back to a clean address with the default again', async () => { + const { calls } = renderPage(serverWithRows) + await screen.findByText(/Showing/) + await openSortMenu() + choose('Descending') + + await waitFor(() => expect(address()).toBe('/elements?dir=desc')) + await waitFor(() => + expect(calls.at(-1)!.variables).toMatchObject({ + sort: { field: 'CODE', direction: 'DESC' }, + }), ) - expect(status.textContent).toBe('Sorting is not connected yet.') + + await openSortMenu() + choose('Ascending') + await waitFor(() => expect(address()).toBe('/elements')) + }) + + it('starts from the sort in the address, and keeps the search when the sort changes', async () => { + const { calls } = renderPage( + serverWithRows, + '/elements?search=amanar&sort=name&dir=desc', + ) + await screen.findByText(/Showing/) + + expect(calls[0].variables).toMatchObject({ + search: 'amanar', + sort: { field: 'NAME', direction: 'DESC' }, + }) + expect(field().value).toBe('amanar') + expect(sortButton(/Sort by Name\W+Descending/)).toBeTruthy() + + await openSortMenu() + choose('Ascending') + await waitFor(() => + expect(address()).toBe('/elements?search=amanar&sort=name'), + ) + }) + + it('keeps the sort in the address when the search changes', async () => { + renderPage(serverWithRows, '/elements?sort=name') + await screen.findByText(/Showing/) + + fireEvent.change(field(), { target: { value: 'biles' } }) + await waitFor( + () => expect(address()).toBe('/elements?sort=name&search=biles'), + wait, + ) + }) + + it('starts a new sort from the first page, and loads more with the same sort', async () => { + const { calls } = renderPage(pagedServer(4)) + await screen.findByText(/Showing 4 of/) + + fireEvent.click(screen.getByRole('button', { name: 'Load more elements' })) + await screen.findByText(/Showing 7 of/) + expect(calls.at(-1)).toMatchObject({ + operationName: 'SearchCatalogMore', + variables: { after: expect.any(String) }, + }) + + await openSortMenu() + choose('Name') + await screen.findByText(/Showing 4 of/) + const last = calls.at(-1)! + expect(last.operationName).toBe('SearchCatalog') + expect(last.variables).not.toHaveProperty('after') + expect(last.variables).toMatchObject({ sort: { field: 'NAME' } }) + + fireEvent.click(screen.getByRole('button', { name: 'Load more elements' })) + await screen.findByText(/Showing 7 of/) + expect(calls.at(-1)!.variables).toMatchObject({ + sort: { field: 'NAME', direction: 'ASC' }, + after: expect.any(String), + }) + }) + + it('cannot sort by difficulty without an apparatus, and says why', async () => { + const { calls } = renderPage(serverWithRows) + await screen.findByText(/Showing/) + await openSortMenu() + + const difficulty = screen.getByRole('menuitemradio', { name: 'Difficulty' }) + expect(difficulty.getAttribute('aria-disabled')).toBe('true') + const reason = screen.getByText( + 'Choose an apparatus to sort by difficulty.', + ) + expect(difficulty.getAttribute('aria-describedby')).toBe(reason.id) + + fireEvent.click(difficulty) + expect(address()).toBe('/elements') + expect(calls).toHaveLength(1) + }) + + it('does not send difficulty without an apparatus, even if the address says so (BAD_USER_INPUT)', async () => { + const { calls } = renderPage( + serverWithRows, + '/elements?sort=difficulty&dir=desc', + ) + await screen.findByText(/Showing/) + + expect(calls[0].variables).toMatchObject({ + sort: { field: 'CODE', direction: 'DESC' }, + }) + expect(sortButton(/Sort by Code\W+Descending/)).toBeTruthy() + }) + + it('can sort by difficulty with an apparatus, and sends the apparatus with it', async () => { + const { calls } = renderPage(serverWithRows, '/elements?apparatus=VT') + await screen.findByText(/Showing/) + await openSortMenu() + + const difficulty = screen.getByRole('menuitemradio', { name: 'Difficulty' }) + expect(difficulty.getAttribute('aria-disabled')).not.toBe('true') + expect(screen.queryByText(/Choose an apparatus/)).toBeNull() + choose('Difficulty') + + await waitFor(() => + expect(address()).toBe('/elements?apparatus=VT&sort=difficulty'), + ) + await waitFor(() => + expect(calls.at(-1)!.variables).toMatchObject({ + filter: { apparatusId: 'VT' }, + sort: { field: 'DIFFICULTY', direction: 'ASC' }, + }), + ) + expect(sortButton(/Sort by Difficulty\W+Ascending/)).toBeTruthy() + }) + + it('ignores a sort in the address that it does not know', async () => { + const { calls } = renderPage( + serverWithRows, + '/elements?sort=banana&dir=sideways', + ) + await screen.findByText(/Showing/) + + expect(calls[0].variables).not.toHaveProperty('sort') + expect(sortButton()).toBeTruthy() }) }) diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index e84e201..9bd16e7 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -1,18 +1,22 @@ 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 { ChevronDown, Plus, Search } from 'lucide-react' import ResultsTable from '@/components/elements/ResultsTable' +import SortMenu from '@/components/elements/SortMenu' import { Button } from '@/components/ui/button' import { describeError } from '@/lib/apollo/errors' import { formatTotal, MAX_SEARCH_LENGTH, PAGE_SIZE, + parseSort, SEARCH_CATALOG_MORE_QUERY, SEARCH_CATALOG_QUERY, SEARCH_DELAY_MS, + sortParams, toListRow, + type Sort, } from '@/lib/searchOperations' const PILL = @@ -20,8 +24,8 @@ const PILL = const FILTERS = ['Apparatus', 'Element group', 'Difficulty'] -// 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. +// TODO(#110): the filters are only drawn so far. Each one says that it is not connected +// yet, like the buttons on the element page. The sort is connected (#128). export default function ElementsPage() { const [status, setStatus] = useState('') const [searchParams, setSearchParams] = useSearchParams() @@ -34,11 +38,45 @@ export default function ElementsPage() { const [text, setText] = useState(searchParams.get('search') ?? '') const written = useRef(search) + // The sort is in the address too (?sort=difficulty&dir=desc, the default left out). + // Difficulty needs an apparatus; the apparatus filter will be in the address as + // ?apparatus=..., and until it is connected nothing sets it. + const apparatusId = searchParams.get('apparatus') + const sort = parseSort( + searchParams.get('sort'), + searchParams.get('dir'), + Boolean(apparatusId), + ) + + // Writing one part of the address keeps the others. function writeAddress(value: string) { const trimmed = value.trim() if (trimmed === written.current) return written.current = trimmed - setSearchParams(trimmed ? { search: trimmed } : {}, { replace: true }) + setSearchParams( + (params) => { + const next = new URLSearchParams(params) + if (trimmed) next.set('search', trimmed) + else next.delete('search') + return next + }, + { replace: true }, + ) + } + + // A new sort is a new list (the cache keys it), so it starts from the first page. + function changeSort(chosen: Sort) { + setSearchParams( + (params) => { + const next = new URLSearchParams(params) + next.delete('sort') + next.delete('dir') + for (const [key, value] of Object.entries(sortParams(chosen))) + next.set(key, value) + return next + }, + { replace: true }, + ) } useEffect(() => { @@ -56,9 +94,17 @@ export default function ElementsPage() { } }, [search]) + // What the search is sent with. The default sort is not sent, so the first request is + // the same as before the sort was connected. + const searchVariables = { + search: search || null, + ...(apparatusId && { filter: { apparatusId } }), + ...(sort.field !== 'CODE' || sort.direction !== 'ASC' ? { sort } : {}), + } + const { data, previousData, loading, error, refetch, fetchMore } = useQuery( SEARCH_CATALOG_QUERY, - { variables: { search: search || null, first: PAGE_SIZE } }, + { variables: { ...searchVariables, 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 @@ -82,7 +128,7 @@ export default function ElementsPage() { try { await fetchMore({ query: SEARCH_CATALOG_MORE_QUERY, - variables: { search: search || null, first: PAGE_SIZE, after }, + variables: { ...searchVariables, first: PAGE_SIZE, after }, }) } catch (problem) { setMoreError({ search, message: describeError(problem).message }) @@ -144,16 +190,12 @@ export default function ElementsPage() { ))} - + />

void + className?: string +}) { + const reasonId = useId() + const blocked = (needsApparatus?: boolean) => + Boolean(needsApparatus) && !hasApparatus + const reasonShown = SORT_FIELDS.some((option) => + blocked(option.needsApparatus), + ) + const fieldLabel = SORT_FIELDS.find((o) => o.field === sort.field)?.label + const directionLabel = SORT_DIRECTIONS.find( + (o) => o.direction === sort.direction, + )?.label + + return ( + + + } + > + + + + + + Sort by + + onChange({ ...sort, field: value as SortField }) + } + > + {SORT_FIELDS.map(({ field, label, needsApparatus }) => { + const disabled = blocked(needsApparatus) + return ( + + + + + + {label} + + ) + })} + + {reasonShown && ( +

+ Choose an apparatus to sort by difficulty. +

+ )} + + + + Direction + + onChange({ ...sort, direction: value as SortDirection }) + } + > + {SORT_DIRECTIONS.map(({ direction, label }) => ( + + + + + + {label} + + ))} + + + + + + + ) +} diff --git a/src/lib/searchOperations.ts b/src/lib/searchOperations.ts index d98f01b..1df8c5b 100644 --- a/src/lib/searchOperations.ts +++ b/src/lib/searchOperations.ts @@ -25,9 +25,72 @@ export const MAX_COUNTED = 1000 export const formatTotal = (totalCount: number): string => totalCount > MAX_COUNTED ? `${MAX_COUNTED}+` : String(totalCount) +export type SortField = 'CODE' | 'DIFFICULTY' | 'NAME' +export type SortDirection = 'ASC' | 'DESC' +export type Sort = { field: SortField; direction: SortDirection } + +export const DEFAULT_SORT: Sort = { field: 'CODE', direction: 'ASC' } + +/** + * The fields in the order of the menu, with the word the user sees. A new field is one + * more line here (and in the API): the menu and the address are built from this list. + * `needsApparatus` is for a field the API only allows within one apparatus. + */ +export const SORT_FIELDS: { + field: SortField + label: string + needsApparatus?: boolean +}[] = [ + { field: 'CODE', label: 'Code' }, + { field: 'DIFFICULTY', label: 'Difficulty', needsApparatus: true }, + { field: 'NAME', label: 'Name' }, +] + +export const SORT_DIRECTIONS: { direction: SortDirection; label: string }[] = [ + { direction: 'ASC', label: 'Ascending' }, + { direction: 'DESC', label: 'Descending' }, +] + +/** + * The sort as the address says it (`?sort=difficulty&dir=desc`). What is missing or not + * known is the default. Difficulty is only allowed with an apparatus (the API answers + * BAD_USER_INPUT without one: element values and vault D-values cannot be compared), so + * without one an address with `sort=difficulty` sorts by code. + */ +export function parseSort( + sort: string | null, + dir: string | null, + hasApparatus: boolean, +): Sort { + const option = SORT_FIELDS.find( + (candidate) => candidate.field.toLowerCase() === sort?.toLowerCase(), + ) + return { + field: + !option || (option.needsApparatus && !hasApparatus) + ? DEFAULT_SORT.field + : option.field, + direction: dir?.toLowerCase() === 'desc' ? 'DESC' : 'ASC', + } +} + +/** The sort as the address says it. The default is left out. */ +export function sortParams(sort: Sort): Record { + return { + ...(sort.field !== DEFAULT_SORT.field && { + sort: sort.field.toLowerCase(), + }), + ...(sort.direction !== DEFAULT_SORT.direction && { + dir: sort.direction.toLowerCase(), + }), + } +} + /** What the queries are sent with. `after` is the cursor of the page to load next. */ export type SearchVariables = { search: string | null + filter?: { apparatusId: string } | null + sort?: Sort first: number after?: string | null } @@ -101,8 +164,20 @@ export const SEARCH_CATALOG_QUERY: TypedDocumentNode< SearchCatalogResult, SearchVariables > = gql` - query SearchCatalog($search: String, $first: Int!, $after: String) { - searchCatalog(search: $search, first: $first, after: $after) { + query SearchCatalog( + $search: String + $filter: CatalogFilter + $sort: CatalogSort + $first: Int! + $after: String + ) { + searchCatalog( + search: $search + filter: $filter + sort: $sort + first: $first + after: $after + ) { totalCount pageInfo { hasNextPage @@ -127,8 +202,20 @@ 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) { + query SearchCatalogMore( + $search: String + $filter: CatalogFilter + $sort: CatalogSort + $first: Int! + $after: String + ) { + searchCatalog( + search: $search + filter: $filter + sort: $sort + first: $first + after: $after + ) { pageInfo { hasNextPage endCursor