From 34592fbc438c8a34c1ac258402452b2c39d89b54 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 13:42:10 +0100 Subject: [PATCH 01/32] feat: add the table row color and let DifficultyBadge hide its value --- src/components/ui/DifficultyBadge.tsx | 24 +++++++++++++++++++----- src/styles/variables.css | 3 +++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/components/ui/DifficultyBadge.tsx b/src/components/ui/DifficultyBadge.tsx index d49d82a..e87ee8e 100644 --- a/src/components/ui/DifficultyBadge.tsx +++ b/src/components/ui/DifficultyBadge.tsx @@ -3,8 +3,17 @@ import { getDifficultyValue } from '@/lib/difficulty' export default function DifficultyBadge({ difficulty, + label = 'Difficulty', + showValue = true, + compact = false, }: { difficulty: string | number + /** What a screen reader says before the badge: "Difficulty" or "D-value". */ + label?: string + /** False leaves out the value under a letter (a screen reader still gets it). */ + showValue?: boolean + /** A smaller circle, for table rows. */ + compact?: boolean }) { const letter = typeof difficulty === 'string' ? difficulty.toUpperCase() : undefined @@ -18,15 +27,20 @@ export default function DifficultyBadge({ would be read a second time, and a tooltip only works with a mouse. */} - Difficulty + {`${label} `} {displayDifficulty} - {value !== undefined && ,} + {value !== undefined && ( + + {showValue ? ',' : `, Value ${value.toFixed(1)}`} + + )} - {value !== undefined && ( + {value !== undefined && showValue && ( Value {value.toFixed(1)} diff --git a/src/styles/variables.css b/src/styles/variables.css index f6cac9a..5605332 100644 --- a/src/styles/variables.css +++ b/src/styles/variables.css @@ -24,6 +24,9 @@ --box-stroke: #888394; --box-shadow: 0 2px 5px rgba(3, 2, 2, 0.1); + /* Results table: the purple of every other row */ + --table-row-alt: #D9D4DB; + /* D-value badge */ --badge-d-value-bg: #BDE981; --badge-d-value-text: var(--text-color-dark); From 7309b22531957054c6104ed474f1399762bb3672 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 13:42:11 +0100 Subject: [PATCH 02/32] feat: build the Elements page with the results table --- index.html | 2 +- src/App.tsx | 7 +- src/components/elements/ElementsPage.test.tsx | 85 ++++++++++++ src/components/elements/ElementsPage.tsx | 110 +++++++++++++++ src/components/elements/ResultsTable.test.tsx | 67 +++++++++ src/components/elements/ResultsTable.tsx | 130 ++++++++++++++++++ src/lib/elementList.ts | 102 ++++++++++++++ src/styles/global.css | 24 ++++ 8 files changed, 522 insertions(+), 5 deletions(-) create mode 100644 src/components/elements/ElementsPage.test.tsx create mode 100644 src/components/elements/ElementsPage.tsx create mode 100644 src/components/elements/ResultsTable.test.tsx create mode 100644 src/components/elements/ResultsTable.tsx create mode 100644 src/lib/elementList.ts diff --git a/index.html b/index.html index 8cd354a..626211a 100644 --- a/index.html +++ b/index.html @@ -7,7 +7,7 @@ t29-project-2 diff --git a/src/App.tsx b/src/App.tsx index c474b73..2f2c7d2 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -3,15 +3,14 @@ import './index.css' import './styles/variables.css' import './styles/global.css' import ElementDetailPage from './components/elements/ElementDetailPage' +import ElementsPage from './components/elements/ElementsPage' import YourGymnastsPage from './components/gymnasts/YourGymnastsPage' function App() { return ( - } - /> + } /> + } /> } /> } /> } /> diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx new file mode 100644 index 0000000..e1c71b4 --- /dev/null +++ b/src/components/elements/ElementsPage.test.tsx @@ -0,0 +1,85 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { MemoryRouter } from 'react-router-dom' +import { afterEach, describe, expect, it } from 'vitest' +import { exampleElementRows, exampleTotalCount } from '@/lib/elementList' +import ElementsPage from './ElementsPage' + +afterEach(cleanup) + +const renderPage = () => + render( + + + , + ) + +describe('ElementsPage', () => { + it('has one h1, and the results under an h2 that names the section', () => { + renderPage() + + expect(screen.getAllByRole('heading', { level: 1 })).toHaveLength(1) + expect(screen.getByRole('region', { name: 'Results' })).toBeTruthy() + expect( + screen.getByRole('heading', { level: 2, name: 'Results' }), + ).toBeTruthy() + }) + + it('has a labelled search field in a search landmark', () => { + renderPage() + + const search = screen.getByRole('search') + expect(search).toBeTruthy() + expect( + screen.getByLabelText('Search elements by code, name or description'), + ).toBeTruthy() + }) + + it('has the filters in a labelled group, and a sort button that says what it sorts', () => { + renderPage() + + 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( + screen.getByRole('button', { name: /Sort by Code\W+Ascending/ }), + ).toBeTruthy() + }) + + 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') + expect(status.textContent).toBe('') + + fireEvent.click(screen.getByRole('button', { name: 'Apparatus' })) + expect(status.textContent).toBe( + 'The apparatus filter is not connected yet.', + ) + + fireEvent.click( + 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.', + ) + + fireEvent.submit(screen.getByRole('search')) + expect(status.textContent).toBe('Search is not connected yet.') + }) +}) diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx new file mode 100644 index 0000000..c4dba93 --- /dev/null +++ b/src/components/elements/ElementsPage.tsx @@ -0,0 +1,110 @@ +import { useState, type FormEvent } from 'react' +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' + +const PILL = + 'h-10 rounded-full border-0 bg-(--background-color-element) px-4 text-sm font-medium text-(--text-color) shadow-(--box-shadow) hover:bg-(--background-color-element) focus-visible:ring-0 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-(--text-color)' + +const FILTERS = ['Apparatus', 'Element group', 'Difficulty'] + +// TODO(#110): search, filters, sort and paging 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 + + function search(event: FormEvent) { + event.preventDefault() + setStatus('Search is not connected yet.') + } + + return ( +
+

Elements

+ +
+ +
+ ) +} diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx new file mode 100644 index 0000000..85c3d5b --- /dev/null +++ b/src/components/elements/ResultsTable.test.tsx @@ -0,0 +1,67 @@ +import { renderToString } from 'react-dom/server' +import { MemoryRouter } from 'react-router-dom' +import { describe, expect, it } from 'vitest' +import { exampleElementRows, type ElementListRow } from '@/lib/elementList' +import ResultsTable from './ResultsTable' + +const render = (rows: ElementListRow[]) => + renderToString( + + + , + ) + +const [withVariants, plain, named] = exampleElementRows +const element = exampleElementRows.find((row) => !row.isVault)! + +describe('ResultsTable', () => { + it('is a table with a caption and the four column headings', () => { + const markup = render(exampleElementRows) + + expect(markup).toMatch(/]*>[^<]*Elements and vaults/) + for (const heading of ['Code', 'Element', 'Apparatus', 'Value']) { + expect(markup).toMatch( + new RegExp(`]*scope="col"[^>]*>${heading}`), + ) + } + }) + + it('has one row for each element, and each row links to its page', () => { + const markup = render(exampleElementRows) + + expect(markup.match(/]*role="row"[^>]*class="group/g)).toHaveLength( + exampleElementRows.length, + ) + expect(markup).toContain(`href="/elements/${plain.id}"`) + expect(markup).toContain(plain.description) + }) + + it('shows the name of the gymnast above the description', () => { + const markup = render([named]) + + expect(markup).toMatch( + new RegExp(`${named.name}]*>${named.description}`), + ) + expect(render([plain])).not.toContain('font-heading') + }) + + it('says "More variants" only on a row that has variants', () => { + expect(render([withVariants])).toContain('More variants') + expect(render([plain, named])).not.toContain('More variants') + }) + + it('reads out the D-value of a vault, and the letter and value of an element', () => { + const vault = render([plain]) + expect(vault).toContain('D-value 2.6') + + const letter = render([element]) + expect(letter).toContain('Difficulty A') + expect(letter).toContain(', Value 0.1') + }) + + it('does not show the value under the letter, as the circle is the value', () => { + const letter = render([element]) + + expect(letter).not.toMatch(/text-xs font-medium leading-none/) + }) +}) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx new file mode 100644 index 0000000..aae2c4b --- /dev/null +++ b/src/components/elements/ResultsTable.tsx @@ -0,0 +1,130 @@ +import { Link } from 'react-router-dom' +import DifficultyBadge from '@/components/ui/DifficultyBadge' +import { getDifficultyLetter } from '@/lib/difficulty' +import type { ElementListRow } from '@/lib/elementList' + +// Below the "md" breakpoint a row is a card (code, text and apparatus on the +// left, value on the right), because four columns do not fit a phone. The roles +// are written out because a table that is not displayed as a table loses them +// in some browsers. +const CELL = 'align-middle md:px-4 md:py-3' +const COLUMN_HEADER = + 'px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--background-color-element)' + +/** The circle: the letter of an element, or the D-value of a vault. */ +function ValueBadge({ row }: { row: ElementListRow }) { + const letter = row.isVault ? undefined : getDifficultyLetter(row.value) + + return ( + + ) +} + +export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { + return ( +
+ + + {/* A phone shows cards, so the headings are only read out there. */} + + + + + + + + + + {rows.map((row) => ( + + + + + + + ))} + +
+ Elements and vaults in the Code of Points, one row for each code +
+ Code + + Element + + Apparatus + + Value +
+ {row.code} + +
+ {/* The link covers the whole row (the ::after), so a click + anywhere in the row opens the element. */} + + {row.name && ( + + {row.name} + + )} + + {row.description} + + + {row.hasVariants && ( + + More variants + + )} +
+ 		+ + {row.apparatusName} + + + {row.elementGroupName} + + + +
+
+ ) +} diff --git a/src/lib/elementList.ts b/src/lib/elementList.ts new file mode 100644 index 0000000..fd6d223 --- /dev/null +++ b/src/lib/elementList.ts @@ -0,0 +1,102 @@ +/** One row of the results table on the Elements page. */ +export type ElementListRow = { + id: string + code: string + /** The gymnast the element is named after, if any. */ + name: string | null + description: string + apparatusName: string + elementGroupName: string + /** The value of an element, or the D-value of a vault. */ + value: number + isVault: boolean + hasVariants: boolean +} + +/** How many results there are in all, for "Showing 7 of 32". */ +export const exampleTotalCount = 32 + +// TODO(#110): replace with the result of `searchCatalog`. These rows only show +// the design until the page is connected to the API. +export const exampleElementRows: ElementListRow[] = [ + { + id: 'VT-1.10-a', + code: '1.10', + name: null, + description: 'Handspring forward on, salto forward tucked off', + apparatusName: 'Vault', + elementGroupName: 'Handsprings', + value: 2.6, + isVault: true, + hasVariants: true, + }, + { + id: 'VT-1.22-a', + code: '1.22', + name: null, + description: + 'Handspring forward on, salto forward piked with 1/2 twist off', + apparatusName: 'Vault', + elementGroupName: 'Handsprings', + value: 2.6, + isVault: true, + hasVariants: false, + }, + { + id: 'VT-3.20-a', + code: '3.20', + name: 'Yurchenko', + description: 'Round-off, flic-flac on, salto backward stretched off', + apparatusName: 'Vault', + elementGroupName: 'Round-off entries', + value: 2.6, + isVault: true, + hasVariants: false, + }, + { + id: 'VT-3.30-a', + code: '3.30', + name: null, + description: + 'Round-off, flic-flac on, salto backward stretched with 1/1 twist off', + apparatusName: 'Vault', + elementGroupName: 'Round-off entries', + value: 2.6, + isVault: true, + hasVariants: false, + }, + { + id: 'VT-3.50-a', + code: '3.50', + name: 'Amanar', + description: + 'Round-off, flic-flac on, salto backward stretched with 5/2 twist off', + apparatusName: 'Vault', + elementGroupName: 'Round-off entries', + value: 2.6, + isVault: true, + hasVariants: false, + }, + { + id: 'VT-4.20-a', + code: '4.20', + name: 'Tsukahara', + description: 'Handspring with 1/2 turn on, salto backward stretched off', + apparatusName: 'Vault', + elementGroupName: 'Handspring with 1/2 turn on', + value: 2.6, + isVault: true, + hasVariants: false, + }, + { + id: 'UB-1.101-a', + code: '1.101', + name: null, + description: 'Glide kip to front support on low bar', + apparatusName: 'Uneven bars', + elementGroupName: 'Mounts', + value: 0.1, + isVault: false, + hasVariants: false, + }, +] diff --git a/src/styles/global.css b/src/styles/global.css index 4db4f32..65fc1dd 100644 --- a/src/styles/global.css +++ b/src/styles/global.css @@ -2,6 +2,30 @@ color: var(--text-color-dark); } +/* The Elements page uses the whole width of the window, not the 1126px of #root. */ +#root:has(.elements-page) { + width: 100%; + border-inline: 0; +} + +#root .elements-page :is(h1, h2) { + color: var(--text-color); + font-weight: 700; + letter-spacing: -0.01em; + line-height: 1.1; + margin: 0; +} + +#root .elements-page h1 { + font-size: 2.125rem; + font-weight: 800; +} + +#root .elements-page h2 { + font-size: 1.25rem; + margin-bottom: 0.75rem; +} + #root .your-gymnasts-page :is(h1, h2) { color: var(--text-color-dark); } From cdda5974e3897c68c0888852f0e68e5c5d4e688d Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 13:42:11 +0100 Subject: [PATCH 03/32] docs: describe the Elements page in AI_USAGE.md --- docs/AI_USAGE.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 2a405f9..43d6354 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -446,3 +446,17 @@ browser against the real backend that an element, a variant link, an unknown id, a stopped server and "Try again" work. The JavaScript bundle went from 665 kB to 522 kB. + +## The Elements page (`src/components/elements/ElementsPage.tsx`, `ResultsTable.tsx`) + +- **Tool:** Claude Code +- **What the team did:** Drew the page in a design (`elements_page.png`) and + chose that it should look exactly like it, with a gray line between the rows + and the same line everywhere. Decided that this pull request is only the + design, and that the search, filters, sort and paging are connected in #110. +- **What Claude did:** Measured the colors in the design and used the existing + tokens for all of them but the purple of every other row (new: + `--table-row-alt`). Wrote the page and the table with the existing badge for + the circle. Checked the page at 390, 768, 927 and 1440 pixels wide in a + browser, and chose how the table becomes cards on a phone and how the whole + row is one link. Wrote the tests. From 486c4e312f4eb118522206775ba992a0be9eef43 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 13:50:19 +0100 Subject: [PATCH 04/32] fix: remove the image-copy plugin that a merge brought back and broke tsc --- vite.config.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/vite.config.ts b/vite.config.ts index f3a5f2f..3044119 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -4,20 +4,7 @@ import babel from '@rolldown/plugin-babel' import { fileURLToPath } from 'node:url' import { defineConfig } from 'vitest/config' -// The drawings in data/images are served by Vite in `npm run dev`, but are not -// in public/, so a build would leave them out of dist/. Copy them there until -// the images come from the server (IMAGE_BASE_URL). -const copyCatalogImages = { - name: 'copy-catalog-images', - apply: 'build' as const, - closeBundle() { - cpSync( - fileURLToPath(new URL('./data/images', import.meta.url)), - fileURLToPath(new URL('./dist/data/images', import.meta.url)), - { recursive: true }, - ) - }, -} +// https://vite.dev/config/ export default defineConfig({ base: '/project2/', plugins: [ From 8c6fc86cc90b01b428ce76b0e8f14c8b295eb665 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 13:50:20 +0100 Subject: [PATCH 05/32] feat: rename Elements to Search in the navbar and point the logo to the same page --- src/App.test.tsx | 48 ++++++++++++++++++++++----- src/components/navbar/Navbar.test.tsx | 14 ++++++-- src/components/navbar/Navbar.tsx | 4 +-- 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/App.test.tsx b/src/App.test.tsx index b1102a3..c32166e 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -1,23 +1,39 @@ // @vitest-environment jsdom +import { ApolloProvider } from '@apollo/client/react'; import { fireEvent, render, screen } from '@testing-library/react'; import { BrowserRouter } from 'react-router-dom'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { apolloClient } from '@/lib/apollo/client'; import App from './App'; /** The app with the router that main.tsx gives it. */ const renderApp = () => render( - - - , + + + + + , ); beforeEach(() => { + // The Your gymnasts page asks the API who is logged in; here nobody is. + vi.stubGlobal( + 'fetch', + vi.fn(() => + Promise.resolve( + new Response(JSON.stringify({ data: { me: null } }), { + headers: { 'content-type': 'application/json' }, + }), + ), + ), + ); window.localStorage.clear(); window.history.replaceState({}, '', import.meta.env.BASE_URL); }); afterEach(() => { + vi.unstubAllGlobals(); delete document.documentElement.dataset.theme; window.localStorage.clear(); window.history.replaceState({}, '', import.meta.env.BASE_URL); @@ -74,14 +90,30 @@ describe('App', () => { expect(screen.getAllByRole('main')).toHaveLength(1); }); + it('opens the search page from the start address, the logo and the Search link', () => { + renderApp(); + expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}elements`); + expect(screen.getByRole('heading', { level: 1, name: 'Elements' })).toBeTruthy(); + + for (const link of [ + () => screen.getByRole('link', { name: /wag cop planner home/i }), + () => screen.getByRole('link', { name: 'Search' }), + ]) { + fireEvent.click(screen.getByRole('link', { name: 'About' })); + expect(screen.getByRole('heading', { name: 'About' })).toBeTruthy(); + + fireEvent.click(link()); + expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}elements`); + expect(screen.getByRole('heading', { level: 1, name: 'Elements' })).toBeTruthy(); + } + }); + it('guards the Gymnasts route and makes it available after login', () => { window.history.replaceState({}, '', `${import.meta.env.BASE_URL}gymnasts`); renderApp(); - // Logged out, /gymnasts goes to the start page, which shows an element. - expect(window.location.pathname).toBe( - `${import.meta.env.BASE_URL}elements/UB-1.101-a`, - ); + // Logged out, /gymnasts goes to the start page, which is the Elements page. + expect(window.location.pathname).toBe(`${import.meta.env.BASE_URL}elements`); expect(screen.queryByRole('link', { name: 'Gymnasts' })).toBeNull(); fireEvent.click(screen.getByRole('button', { name: 'Log in' })); diff --git a/src/components/navbar/Navbar.test.tsx b/src/components/navbar/Navbar.test.tsx index 595acc2..157b893 100644 --- a/src/components/navbar/Navbar.test.tsx +++ b/src/components/navbar/Navbar.test.tsx @@ -44,7 +44,7 @@ describe('Navbar', () => { 'min-[720px]:grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)]', ); expect(header.className).toContain('relative'); - expect(screen.getByRole('link', { name: 'Elements' })).toBeTruthy(); + expect(screen.getByRole('link', { name: 'Search' })).toBeTruthy(); expect(screen.getByRole('link', { name: 'Calculator' })).toBeTruthy(); expect(screen.getByRole('link', { name: 'About' })).toBeTruthy(); expect(screen.queryByRole('link', { name: 'Gymnasts' })).toBeNull(); @@ -102,10 +102,20 @@ describe('Navbar', () => { expect(navLink.className).toContain('focus-visible:outline-(--text-color)'); }); + it('sends the logo and the Search link to the same page, the search page', () => { + renderNavbar(); + + const logo = screen.getByRole('link', { name: /wag cop planner home/i }); + const search = screen.getByRole('link', { name: 'Search' }); + expect(logo.getAttribute('href')).toBe('/elements'); + expect(search.getAttribute('href')).toBe('/elements'); + }); + it('keeps the underline visible on the current page link', () => { + window.history.replaceState({}, '', '/elements'); renderNavbar(); - const currentPageLink = screen.getByRole('link', { name: 'Elements' }); + const currentPageLink = screen.getByRole('link', { name: 'Search' }); expect(currentPageLink.className).toContain('after:scale-x-100'); expect(screen.getByRole('navigation').className).not.toContain('pb-[6px]'); }); diff --git a/src/components/navbar/Navbar.tsx b/src/components/navbar/Navbar.tsx index caea231..a413049 100644 --- a/src/components/navbar/Navbar.tsx +++ b/src/components/navbar/Navbar.tsx @@ -19,7 +19,7 @@ export const Navbar: React.FC = ({ const focusRing = 'focus-visible:outline-2 focus-visible:outline-(--text-color) focus-visible:outline-offset-4'; const navLinks = [ - { path: '/', label: 'Elements' }, + { path: '/elements', label: 'Search' }, { path: '/calculator', label: 'Calculator' }, { path: '/about', label: 'About' }, ...(isLoggedIn ? [{ path: '/gymnasts', label: 'Gymnasts' }] : []), @@ -28,7 +28,7 @@ export const Navbar: React.FC = ({ return (
Date: Thu, 29 Oct 2026 14:02:08 +0100 Subject: [PATCH 06/32] refactor: use DifficultyBadge as it is, without new props --- src/components/ui/DifficultyBadge.tsx | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/src/components/ui/DifficultyBadge.tsx b/src/components/ui/DifficultyBadge.tsx index e87ee8e..d49d82a 100644 --- a/src/components/ui/DifficultyBadge.tsx +++ b/src/components/ui/DifficultyBadge.tsx @@ -3,17 +3,8 @@ import { getDifficultyValue } from '@/lib/difficulty' export default function DifficultyBadge({ difficulty, - label = 'Difficulty', - showValue = true, - compact = false, }: { difficulty: string | number - /** What a screen reader says before the badge: "Difficulty" or "D-value". */ - label?: string - /** False leaves out the value under a letter (a screen reader still gets it). */ - showValue?: boolean - /** A smaller circle, for table rows. */ - compact?: boolean }) { const letter = typeof difficulty === 'string' ? difficulty.toUpperCase() : undefined @@ -27,20 +18,15 @@ export default function DifficultyBadge({ would be read a second time, and a tooltip only works with a mouse. */} - {`${label} `} + Difficulty {displayDifficulty} - {value !== undefined && ( - - {showValue ? ',' : `, Value ${value.toFixed(1)}`} - - )} + {value !== undefined && ,} - {value !== undefined && showValue && ( + {value !== undefined && ( Value {value.toFixed(1)} From 6040e5335b681aee2fd71bc868ba7118a53238f5 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:02:08 +0100 Subject: [PATCH 07/32] feat: let the whole row grow on hover and focus instead of underlining the text --- src/components/elements/ResultsTable.test.tsx | 27 ++++++----- src/components/elements/ResultsTable.tsx | 46 +++++++++---------- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index 85c3d5b..a8d24e6 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -29,13 +29,22 @@ describe('ResultsTable', () => { it('has one row for each element, and each row links to its page', () => { const markup = render(exampleElementRows) - expect(markup.match(/]*role="row"[^>]*class="group/g)).toHaveLength( - exampleElementRows.length, - ) + expect( + markup.match(/]*role="row"[^>]*class="relative cursor-pointer/g), + ).toHaveLength(exampleElementRows.length) expect(markup).toContain(`href="/elements/${plain.id}"`) expect(markup).toContain(plain.description) }) + it('answers on the whole row, by growing a little, and not by underlining the text', () => { + const markup = render(exampleElementRows) + + expect(markup).toContain('motion-safe:hover:scale-[1.02]') + expect(markup).toContain('motion-safe:focus-within:scale-[1.02]') + expect(markup).toContain('motion-reduce:hover:outline-2') + expect(markup).not.toContain('underline') + }) + it('shows the name of the gymnast above the description', () => { const markup = render([named]) @@ -50,18 +59,12 @@ describe('ResultsTable', () => { expect(render([plain, named])).not.toContain('More variants') }) - it('reads out the D-value of a vault, and the letter and value of an element', () => { + it('uses the difficulty badge: the D-value of a vault, and the letter and value of an element', () => { const vault = render([plain]) - expect(vault).toContain('D-value 2.6') + expect(vault).toContain('Difficulty 2.6') const letter = render([element]) expect(letter).toContain('Difficulty A') - expect(letter).toContain(', Value 0.1') - }) - - it('does not show the value under the letter, as the circle is the value', () => { - const letter = render([element]) - - expect(letter).not.toMatch(/text-xs font-medium leading-none/) + expect(letter).toContain('Value 0.1') }) }) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index aae2c4b..7daef9b 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -9,34 +9,32 @@ import type { ElementListRow } from '@/lib/elementList' // in some browsers. const CELL = 'align-middle md:px-4 md:py-3' const COLUMN_HEADER = - 'px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--background-color-element)' + 'bg-(--text-color) px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--background-color-element) first:rounded-tl-lg last:rounded-tr-lg' -/** The circle: the letter of an element, or the D-value of a vault. */ -function ValueBadge({ row }: { row: ElementListRow }) { - const letter = row.isVault ? undefined : getDifficultyLetter(row.value) +// The whole row is one link, so the whole row answers when the pointer or the +// keyboard is on it: it grows a little and comes in front of the next rows (a +// row that grows inside a clipped box would be cut off, so nothing clips the +// table). Without motion (prefers-reduced-motion) it gets an outline instead. +const ROW_EFFECT = + 'relative cursor-pointer transition-transform duration-150 ease-out hover:z-10 focus-within:z-10 motion-safe:hover:scale-[1.02] motion-safe:focus-within:scale-[1.02] motion-reduce:transition-none motion-reduce:hover:outline-2 motion-reduce:hover:-outline-offset-2 motion-reduce:hover:outline-(--text-color) focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' - return ( - - ) -} +// The colors and lines are on the cells in a table (so the corners can be +// round), and on the row when it is a card. +const ROW_LOOK = + 'md:not-last:bg-(--background-color-element) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--background-color-element) md:even:*:bg-(--table-row-alt) md:not-first:*:border-t md:not-first:*:border-(--box-stroke) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--background-color-element) max-md:even:bg-(--table-row-alt) max-md:not-first:border-t max-md:not-first:border-(--box-stroke) max-md:first:rounded-t-lg max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { return ( -
+
{/* A phone shows cards, so the headings are only read out there. */} - + {rows.map((row) => ( - + ))} From 84ac1788aff8748372d3101676ff22f672098a40 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:14:31 +0100 Subject: [PATCH 08/32] feat: make the hovered row taller instead of wider --- src/components/elements/ResultsTable.test.tsx | 10 ++++++---- src/components/elements/ResultsTable.tsx | 9 +++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index a8d24e6..d11e5e8 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -36,12 +36,14 @@ describe('ResultsTable', () => { expect(markup).toContain(plain.description) }) - it('answers on the whole row, by growing a little, and not by underlining the text', () => { + it('answers on the whole row, by getting taller and not wider, and not by underlining the text', () => { const markup = render(exampleElementRows) - expect(markup).toContain('motion-safe:hover:scale-[1.02]') - expect(markup).toContain('motion-safe:focus-within:scale-[1.02]') - expect(markup).toContain('motion-reduce:hover:outline-2') + expect(markup).toContain('md:hover:*:shadow-[0_-5px_0_0_var(--row-bg)') + expect(markup).toContain( + 'md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg)', + ) + expect(markup).not.toContain('scale-') expect(markup).not.toContain('underline') }) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 7daef9b..8771e0e 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -12,11 +12,12 @@ const COLUMN_HEADER = 'bg-(--text-color) px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--background-color-element) first:rounded-tl-lg last:rounded-tr-lg' // The whole row is one link, so the whole row answers when the pointer or the -// keyboard is on it: it grows a little and comes in front of the next rows (a -// row that grows inside a clipped box would be cut off, so nothing clips the -// table). Without motion (prefers-reduced-motion) it gets an outline instead. +// keyboard is on it: it gets taller (5 px above and below, in its own color, in +// front of the next rows) and its lines go away. Only the height changes, so +// nothing moves and the table does not get wider. A shadow does it, so the text +// stays where it is. (The classes are written out, so Tailwind can find them.) const ROW_EFFECT = - 'relative cursor-pointer transition-transform duration-150 ease-out hover:z-10 focus-within:z-10 motion-safe:hover:scale-[1.02] motion-safe:focus-within:scale-[1.02] motion-reduce:transition-none motion-reduce:hover:outline-2 motion-reduce:hover:-outline-offset-2 motion-reduce:hover:outline-(--text-color) focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' + 'relative cursor-pointer [--row-bg:var(--background-color-element)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 hover:*:border-transparent! focus-within:*:border-transparent! max-md:hover:border-transparent! max-md:focus-within:border-transparent! md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] *:transition-shadow *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' // The colors and lines are on the cells in a table (so the corners can be // round), and on the row when it is a card. From 093709604bce59775ce3a868cddd57be56012c04 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:16:08 +0100 Subject: [PATCH 09/32] feat: remove the lines between the table rows --- docs/AI_USAGE.md | 3 +-- src/components/elements/ResultsTable.tsx | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 43d6354..db4211a 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -451,8 +451,7 @@ - **Tool:** Claude Code - **What the team did:** Drew the page in a design (`elements_page.png`) and - chose that it should look exactly like it, with a gray line between the rows - and the same line everywhere. Decided that this pull request is only the + chose that it should look exactly like it, without lines between the rows. Decided that this pull request is only the design, and that the search, filters, sort and paging are connected in #110. - **What Claude did:** Measured the colors in the design and used the existing tokens for all of them but the purple of every other row (new: diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 8771e0e..9d80612 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -13,16 +13,16 @@ const COLUMN_HEADER = // The whole row is one link, so the whole row answers when the pointer or the // keyboard is on it: it gets taller (5 px above and below, in its own color, in -// front of the next rows) and its lines go away. Only the height changes, so +// front of the next rows). Only the height changes, so // nothing moves and the table does not get wider. A shadow does it, so the text // stays where it is. (The classes are written out, so Tailwind can find them.) const ROW_EFFECT = - 'relative cursor-pointer [--row-bg:var(--background-color-element)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 hover:*:border-transparent! focus-within:*:border-transparent! max-md:hover:border-transparent! max-md:focus-within:border-transparent! md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] *:transition-shadow *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' + 'relative cursor-pointer [--row-bg:var(--background-color-element)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] *:transition-shadow *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' -// The colors and lines are on the cells in a table (so the corners can be +// The colors are on the cells in a table (so the corners can be // round), and on the row when it is a card. const ROW_LOOK = - 'md:not-last:bg-(--background-color-element) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--background-color-element) md:even:*:bg-(--table-row-alt) md:not-first:*:border-t md:not-first:*:border-(--box-stroke) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--background-color-element) max-md:even:bg-(--table-row-alt) max-md:not-first:border-t max-md:not-first:border-(--box-stroke) max-md:first:rounded-t-lg max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' + 'md:not-last:bg-(--background-color-element) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--background-color-element) md:even:*:bg-(--table-row-alt) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--background-color-element) max-md:even:bg-(--table-row-alt) max-md:first:rounded-t-lg max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { return ( From a87f27b22ae6a8abb5370f76c1e0ec1b1a57aa0c Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:25:38 +0100 Subject: [PATCH 10/32] feat: make the text of the hovered row look bolder without moving it --- src/components/elements/ResultsTable.test.tsx | 5 +++++ src/components/elements/ResultsTable.tsx | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index d11e5e8..f6a9402 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -43,6 +43,11 @@ describe('ResultsTable', () => { expect(markup).toContain( 'md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg)', ) + expect(markup).toContain('hover:[text-shadow:0.35px_0_0_currentColor') + expect(markup).toContain( + 'focus-within:[text-shadow:0.35px_0_0_currentColor', + ) + expect(markup).not.toContain('font-bold hover') expect(markup).not.toContain('scale-') expect(markup).not.toContain('underline') }) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 9d80612..0bdc43e 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -15,9 +15,11 @@ const COLUMN_HEADER = // keyboard is on it: it gets taller (5 px above and below, in its own color, in // front of the next rows). Only the height changes, so // nothing moves and the table does not get wider. A shadow does it, so the text -// stays where it is. (The classes are written out, so Tailwind can find them.) +// stays where it is. The text looks bolder in the same way (a thin shadow in the +// color of the text), because real bold makes the words wider and the lines +// break in other places. (The classes are written out, so Tailwind can find them.) const ROW_EFFECT = - 'relative cursor-pointer [--row-bg:var(--background-color-element)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] *:transition-shadow *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' + 'relative cursor-pointer [--row-bg:var(--background-color-element)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] hover:[text-shadow:0.35px_0_0_currentColor,-0.35px_0_0_currentColor] focus-within:[text-shadow:0.35px_0_0_currentColor,-0.35px_0_0_currentColor] transition-[text-shadow] duration-150 *:transition-shadow *:duration-150 motion-reduce:transition-none motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' // The colors are on the cells in a table (so the corners can be // round), and on the row when it is a card. From b06e8bb30e201c9370a687141f0f2caaad4bc687 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:42:51 +0100 Subject: [PATCH 11/32] feat: add the value column, chevron, bold code, focus rings and a dark theme to the Elements page --- src/components/elements/ElementsPage.test.tsx | 21 ++++++++ src/components/elements/ElementsPage.tsx | 6 +-- src/components/elements/ResultsTable.test.tsx | 50 ++++++++++++++++++- src/components/elements/ResultsTable.tsx | 47 +++++++++++------ src/styles/global.css | 7 +++ src/styles/variables.css | 23 ++++++++- 6 files changed, 134 insertions(+), 20 deletions(-) diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index e1c71b4..9665d24 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -49,6 +49,27 @@ describe('ElementsPage', () => { ).toBeTruthy() }) + it('can reach every control with the keyboard, and each one shows where the focus is', () => { + renderPage() + + const controls = [ + ...screen.getAllByRole('button'), + ...screen.getAllByRole('link'), + screen.getByRole('searchbox'), + ] + // The search field, the three filters, the sort button, the 7 rows and "Load more". + expect(controls).toHaveLength(1 + 3 + 1 + exampleElementRows.length + 1) + for (const control of controls) { + expect(control.getAttribute('tabindex')).not.toBe('-1') + expect(control.hasAttribute('disabled')).toBe(false) + } + for (const button of screen.getAllByRole('button')) { + // outline-none on the base button hides the ring unless the style is set again. + expect(button.className).toContain('focus-visible:outline-solid') + expect(button.className).toContain('hover:bg-(--surface-hover)') + } + }) + it('shows how many of the results are loaded', () => { renderPage() diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index c4dba93..5200ff9 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -5,7 +5,7 @@ import { Button } from '@/components/ui/button' import { exampleElementRows, exampleTotalCount } from '@/lib/elementList' const PILL = - 'h-10 rounded-full border-0 bg-(--background-color-element) px-4 text-sm font-medium text-(--text-color) shadow-(--box-shadow) hover:bg-(--background-color-element) focus-visible:ring-0 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-(--text-color)' + 'h-10 rounded-full border-0 bg-(--surface) px-4 text-sm font-medium text-(--text-color) shadow-(--box-shadow) transition-colors hover:bg-(--surface-hover) focus-visible:bg-(--surface-hover) 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'] @@ -21,7 +21,7 @@ export default function ElementsPage() { } return ( -
+

Elements

diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index f6a9402..35edc40 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -21,7 +21,9 @@ describe('ResultsTable', () => { expect(markup).toMatch(/]*>[^<]*Elements and vaults/) for (const heading of ['Code', 'Element', 'Apparatus', 'Value']) { expect(markup).toMatch( - new RegExp(`]*scope="col"[^>]*>${heading}`), + new RegExp( + `]*scope="col"[^>]*>(]*>)?(]*>)?${heading}()?()?`, + ), ) } }) @@ -30,7 +32,9 @@ describe('ResultsTable', () => { const markup = render(exampleElementRows) expect( - markup.match(/]*role="row"[^>]*class="relative cursor-pointer/g), + markup.match( + /]*role="row"[^>]*class="group relative cursor-pointer/g, + ), ).toHaveLength(exampleElementRows.length) expect(markup).toContain(`href="/elements/${plain.id}"`) expect(markup).toContain(plain.description) @@ -52,6 +56,48 @@ describe('ResultsTable', () => { expect(markup).not.toContain('underline') }) + it('has a chevron in each row that only appears on hover and focus, and is hidden from a screen reader', () => { + const markup = render(exampleElementRows) + + const chevrons = markup.match(/]*lucide-chevron-right[^>]*>/g) ?? [] + + expect(chevrons).toHaveLength(exampleElementRows.length) + for (const chevron of chevrons) { + expect(chevron).toContain('aria-hidden="true"') + expect(chevron).toContain('group-hover:opacity-100') + expect(chevron).toContain('group-focus-within:opacity-100') + } + }) + + it('puts the chevron next to the badge, not on top of it', () => { + const markup = render([plain]) + + // The [ and ] of the class are escaped, as they mean something in a regex. + const layout = 'grid grid-cols-[1fr_auto_1fr] items-center'.replace( + /[[\]]/g, + '\\$&', + ) + // The badge is in the middle column and the chevron in the one to the right. + expect(markup).toMatch( + new RegExp( + `
.*Value
`, + ), + ) + expect(markup).not.toContain('absolute top-1/2 right-2') + }) + + it('shows the code in bold', () => { + expect(render([plain])).toMatch( + new RegExp(`]*font-bold[^>]*>${plain.code}`), + ) + }) + it('shows the name of the gymnast above the description', () => { const markup = render([named]) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 0bdc43e..0a14e93 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -1,3 +1,4 @@ +import { ChevronRight } from 'lucide-react' import { Link } from 'react-router-dom' import DifficultyBadge from '@/components/ui/DifficultyBadge' import { getDifficultyLetter } from '@/lib/difficulty' @@ -8,8 +9,12 @@ import type { ElementListRow } from '@/lib/elementList' // are written out because a table that is not displayed as a table loses them // in some browsers. const CELL = 'align-middle md:px-4 md:py-3' +// The badge is in the middle of the cell, and the chevron has the column to the +// right of it, so it never ends up behind the badge, and the heading above can +// use the same columns to be in the middle over the badge. +const VALUE_LAYOUT = 'grid grid-cols-[1fr_auto_1fr] items-center' const COLUMN_HEADER = - 'bg-(--text-color) px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--background-color-element) first:rounded-tl-lg last:rounded-tr-lg' + 'bg-(--table-head-bg) px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--table-head-text) first:rounded-tl-lg last:rounded-tr-lg' // The whole row is one link, so the whole row answers when the pointer or the // keyboard is on it: it gets taller (5 px above and below, in its own color, in @@ -19,12 +24,12 @@ const COLUMN_HEADER = // color of the text), because real bold makes the words wider and the lines // break in other places. (The classes are written out, so Tailwind can find them.) const ROW_EFFECT = - 'relative cursor-pointer [--row-bg:var(--background-color-element)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] hover:[text-shadow:0.35px_0_0_currentColor,-0.35px_0_0_currentColor] focus-within:[text-shadow:0.35px_0_0_currentColor,-0.35px_0_0_currentColor] transition-[text-shadow] duration-150 *:transition-shadow *:duration-150 motion-reduce:transition-none motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' + 'group relative cursor-pointer [--row-bg:var(--surface)] even:[--row-bg:var(--table-row-alt)] hover:z-10 focus-within:z-10 md:hover:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:hover:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-bg),0_5px_0_0_var(--row-bg)] hover:[text-shadow:0.35px_0_0_currentColor,-0.35px_0_0_currentColor] focus-within:[text-shadow:0.35px_0_0_currentColor,-0.35px_0_0_currentColor] transition-[text-shadow] duration-150 *:transition-shadow *:duration-150 motion-reduce:transition-none motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' // The colors are on the cells in a table (so the corners can be // round), and on the row when it is a card. const ROW_LOOK = - 'md:not-last:bg-(--background-color-element) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--background-color-element) md:even:*:bg-(--table-row-alt) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--background-color-element) max-md:even:bg-(--table-row-alt) max-md:first:rounded-t-lg max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' + 'md:not-last:bg-(--surface) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--surface) md:even:*:bg-(--table-row-alt) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--surface) max-md:even:bg-(--table-row-alt) max-md:first:rounded-t-lg max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { return ( @@ -59,9 +64,13 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) {
@@ -70,7 +79,7 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { @@ -98,7 +107,7 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { {row.hasVariants && ( - + More variants )} @@ -116,13 +125,23 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { role="cell" className={`${CELL} max-md:col-start-2 max-md:row-span-3 max-md:row-start-1 max-md:self-center md:text-center`} > - {/* The letter of an element, or the D-value of a vault. */} - +
+ {/* The letter of an element, or the D-value of a vault. */} + + + + {/* Shown on hover and focus, with its own place so nothing + moves when it shows. A phone does not show it. */} +
))} diff --git a/src/styles/global.css b/src/styles/global.css index 65fc1dd..ca8baee 100644 --- a/src/styles/global.css +++ b/src/styles/global.css @@ -16,6 +16,13 @@ margin: 0; } +/* The value under the letter in the difficulty badge is dark navy, which cannot + be read on the dark table. Only that text changes, and only in dark mode (the + circle and its letter keep their colors, and the badge is not changed). */ +[data-theme='dark'] #root .elements-page td .inline-flex.flex-col > span + span { + color: var(--text-color); +} + #root .elements-page h1 { font-size: 2.125rem; font-weight: 800; diff --git a/src/styles/variables.css b/src/styles/variables.css index 12887b7..e3065bc 100644 --- a/src/styles/variables.css +++ b/src/styles/variables.css @@ -26,8 +26,17 @@ --box-stroke: #888394; --box-shadow: 0 2px 5px rgba(3, 2, 2, 0.1); - /* Results table: the purple of every other row */ + /* The Elements page. Each color has a light and a dark value (the dark + ones are in the dark block below), so the page follows the theme. */ + --page-bg: var(--secondary-color); + --surface: var(--background-color-element); + --surface-hover: var(--primary-color); + /* The purple of every other row in the results table */ --table-row-alt: #D9D4DB; + --table-head-bg: var(--text-color); + --table-head-text: var(--background-color-element); + --pill-bg: var(--text-color); + --pill-text: var(--background-color-element); /* D-value badge */ --badge-d-value-bg: #BDE981; @@ -67,4 +76,16 @@ --border-subtle: #2A3363; --badge-d-value-bg: #82C43C; /* Slightly deeper green for contrast */ --badge-d-value-text: #090F2F; + + /* The Elements page, from the color inspiration (inspo.png): a gray slate + backdrop, darker slate rows, the navbar color in the table heading and a + warm tan for the "More variants" mark. */ + --page-bg: #40444F; + --surface: #2D3343; + --surface-hover: #434A60; + --table-row-alt: #373E52; + --table-head-bg: var(--primary-color); + --table-head-text: var(--text-color); + --pill-bg: #C19F83; + --pill-text: var(--primary-color); } From 2f848829af3954aba304c53d8e75528e82d004fc Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:46:23 +0100 Subject: [PATCH 12/32] feat: use the light More-variants color on the buttons and the value badge in dark mode --- src/components/elements/ElementsPage.test.tsx | 2 +- src/components/elements/ElementsPage.tsx | 2 +- src/styles/global.css | 7 +++++++ src/styles/variables.css | 8 ++++++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index 9665d24..46ba91e 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -66,7 +66,7 @@ describe('ElementsPage', () => { for (const button of screen.getAllByRole('button')) { // outline-none on the base button hides the ring unless the style is set again. expect(button.className).toContain('focus-visible:outline-solid') - expect(button.className).toContain('hover:bg-(--surface-hover)') + expect(button.className).toContain('hover:bg-(--button-hover-bg)') } }) diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index 5200ff9..c9c8466 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -5,7 +5,7 @@ import { Button } from '@/components/ui/button' import { exampleElementRows, exampleTotalCount } from '@/lib/elementList' const PILL = - 'h-10 rounded-full border-0 bg-(--surface) px-4 text-sm font-medium text-(--text-color) shadow-(--box-shadow) transition-colors hover:bg-(--surface-hover) focus-visible:bg-(--surface-hover) 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' + '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) 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'] diff --git a/src/styles/global.css b/src/styles/global.css index ca8baee..557181a 100644 --- a/src/styles/global.css +++ b/src/styles/global.css @@ -23,6 +23,13 @@ color: var(--text-color); } +/* The circle of the badge gets the light color of "More variants" in dark mode, + with dark text, so it stands out from the dark rows. */ +[data-theme='dark'] #root .elements-page td .inline-flex.flex-col > span:first-child { + background-color: var(--pill-bg); + color: var(--pill-text); +} + #root .elements-page h1 { font-size: 2.125rem; font-weight: 800; diff --git a/src/styles/variables.css b/src/styles/variables.css index e3065bc..9bff291 100644 --- a/src/styles/variables.css +++ b/src/styles/variables.css @@ -37,6 +37,11 @@ --table-head-text: var(--background-color-element); --pill-bg: var(--text-color); --pill-text: var(--background-color-element); + /* The buttons: light in the design. In dark mode they use the light color of + "More variants", so they stand out from the dark page. */ + --button-bg: var(--surface); + --button-hover-bg: var(--surface-hover); + --button-text: var(--text-color); /* D-value badge */ --badge-d-value-bg: #BDE981; @@ -88,4 +93,7 @@ --table-head-text: var(--text-color); --pill-bg: #C19F83; --pill-text: var(--primary-color); + --button-bg: var(--pill-bg); + --button-hover-bg: color-mix(in srgb, var(--pill-bg), white 25%); + --button-text: var(--pill-text); } From 7b22db4174a263957e348c63b1475786cc3b524d Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 14:48:10 +0100 Subject: [PATCH 13/32] feat: give the search field the light button color --- src/components/elements/ElementsPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/elements/ElementsPage.tsx b/src/components/elements/ElementsPage.tsx index c9c8466..a70cf04 100644 --- a/src/components/elements/ElementsPage.tsx +++ b/src/components/elements/ElementsPage.tsx @@ -34,7 +34,7 @@ export default function ElementsPage() { - `), diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 43dcf27..5644052 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -13,9 +13,10 @@ const CELL = 'align-middle md:px-4 md:py-3' // The badge is in the middle of the cell, and the chevron has the column to the // right of it, so it never ends up behind the badge, and the heading above can // use the same columns to be in the middle over the badge. -// The mark "More variants" and the LabelBadge have the same colors (--pill-*). +// The mark "More variants" is as tall as the pills of the LabelBadge (the same text +// size, line height, padding and a border that cannot be seen). const MORE_VARIANTS = - 'rounded-full bg-(--pill-bg) px-3 py-1 text-xs font-bold whitespace-nowrap text-(--pill-text)' + 'rounded-full border border-transparent bg-(--pill-bg) px-3 py-1 text-[0.8125rem] leading-snug font-bold whitespace-nowrap text-(--pill-text)' const VALUE_LAYOUT = 'grid grid-cols-[1fr_auto_1fr] items-center' const COLUMN_HEADER = 'bg-(--table-head-bg) px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--table-head-text) first:rounded-tl-lg last:rounded-tr-lg' @@ -33,7 +34,7 @@ const ROW_EFFECT = // The colors are on the cells in a table (so the corners can be // round), and on the row when it is a card. const ROW_LOOK = - 'md:not-last:bg-(--surface) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--surface) md:even:*:bg-(--table-row-alt) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--surface) max-md:even:bg-(--table-row-alt) max-md:first:rounded-t-lg max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' + 'md:not-last:bg-(--surface) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--surface) md:even:*:bg-(--table-row-alt) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--surface) max-md:even:bg-(--table-row-alt) max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { return ( @@ -45,23 +46,32 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { - {/* A phone shows cards, so the headings are only read out there. */} - - + {/* A phone shows cards, but the navy heading stays, with only two sides: + Element and D-value. Code and Apparatus are in the cards (the apparatus + has its own label), so their headings are only read out there. */} + + - @@ -73,7 +83,8 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { {/* The same three columns as the cell, so the heading is exactly over the badge. */}
- Value + Value + D-value
From 9868ce98e428e5565f569f327edfb359bb90b887 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 15:49:29 +0100 Subject: [PATCH 25/32] feat: show the results as cards on a small screen, with the drawing when there is room, and separate apparatus and group columns in the table --- src/components/elements/ElementsPage.test.tsx | 6 +- src/components/elements/ResultsTable.test.tsx | 240 +++++++++++------- src/components/elements/ResultsTable.tsx | 208 +++++++++------ src/components/ui/DifficultyBadge.test.tsx | 40 +++ src/components/ui/DifficultyBadge.tsx | 9 +- src/components/ui/LabelBadge.test.tsx | 49 ---- src/components/ui/LabelBadge.tsx | 39 --- src/lib/elementList.ts | 86 ++++--- src/styles/global.css | 14 +- src/styles/variables.css | 7 +- 10 files changed, 398 insertions(+), 300 deletions(-) create mode 100644 src/components/ui/DifficultyBadge.test.tsx delete mode 100644 src/components/ui/LabelBadge.test.tsx delete mode 100644 src/components/ui/LabelBadge.tsx diff --git a/src/components/elements/ElementsPage.test.tsx b/src/components/elements/ElementsPage.test.tsx index 73f0b6c..36ed1dd 100644 --- a/src/components/elements/ElementsPage.test.tsx +++ b/src/components/elements/ElementsPage.test.tsx @@ -57,8 +57,10 @@ describe('ElementsPage', () => { ...screen.getAllByRole('link'), screen.getByRole('searchbox'), ] - // The search field, the three filters, the sort button, the 7 rows and "Load more". - expect(controls).toHaveLength(1 + 3 + 1 + exampleElementRows.length + 1) + // 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) for (const control of controls) { expect(control.getAttribute('tabindex')).not.toBe('-1') expect(control.hasAttribute('disabled')).toBe(false) diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index 38fa2ac..dcf35fc 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -11,75 +11,71 @@ const render = (rows: ElementListRow[]) => , ) -const [withVariants, plain, named] = exampleElementRows +/** The cards (a phone) and the table (a wide screen) are both in the page. */ +const cardsOf = (markup: string) => + /]*md:hidden[^>]*>.*<\/ul>/s.exec(markup)?.[0] ?? '' +const tableOf = (markup: string) => //s.exec(markup)?.[0] ?? '' + +const withVariants = exampleElementRows.find((row) => row.hasVariants)! +const plain = exampleElementRows.find((row) => row.id === 'VT-1.22-a')! +const named = exampleElementRows.find((row) => row.name)! const element = exampleElementRows.find((row) => !row.isVault)! -describe('ResultsTable', () => { - it('is a table with a caption and the four column headings', () => { - const markup = render(exampleElementRows) +describe('ResultsTable: the table (a wide screen)', () => { + it('is a table with a caption and the five column headings', () => { + const table = tableOf(render(exampleElementRows)) - expect(markup).toMatch(/]*>[^<]*Elements and vaults/) - for (const heading of ['Code', 'Element', 'Apparatus', 'Value']) { - expect(markup).toMatch( + expect(table).toMatch(/]*>[^<]*Elements and vaults/) + for (const heading of ['Code', 'Element', 'Apparatus', 'Group', 'Value']) { + expect(table).toMatch( new RegExp( - `]*scope="col"[^>]*>(]*>)?(]*>)?${heading}()?(]*>D-value)?()?`, + `]*scope="col"[^>]*>(]*>)?(]*>)?${heading}()?()?`, ), ) } }) - it('keeps the navy heading on a phone, with Element on one side and D-value on the other', () => { + it('is not shown on a phone, where the cards are', () => { const markup = render(exampleElementRows) - // The row is the navy bar on a phone, and the table's own heading is not hidden. - const headRow = /]*>]*)>/.exec(markup)?.[1] ?? '' - expect(headRow).toContain('max-md:bg-(--table-head-bg)') - expect(headRow).toContain('max-md:justify-between') - expect(markup).not.toMatch(/]*max-md:sr-only/) - // Element is shown on a phone, Code and Apparatus are only read out (their texts are - // in the card), and the value heading is "D-value" on a phone and "Value" in a table. - expect(markup).toMatch(/]*max-md:flex-1[^>]*>Element<\/th>/) - expect(markup).toMatch(/]*max-md:sr-only[^>]*>Code<\/th>/) - expect(markup).toMatch(/]*max-md:sr-only[^>]*>Apparatus<\/th>/) - expect(markup).toMatch(/max-md:hidden">Value<\/span>/) - expect(markup).toMatch(/md:hidden">D-value<\/span>/) + expect(markup).toMatch(/
Elements and vaults in the Code of Points, one row for each code
{row.name && ( @@ -119,7 +113,13 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { role="cell" className={`${CELL} max-md:col-start-2 max-md:row-span-3 max-md:row-start-1 max-md:self-center md:text-center`} > - + {/* The letter of an element, or the D-value of a vault. */} +
- Value + {/* The same three columns as the cell, so the heading is exactly + over the badge. */} +
+ Value +
{row.code}
- - {row.apparatusName} - - - {row.elementGroupName} - + {/* In the table the column heading says what it is, so the two + texts are plain. In a card there is no heading to see, so each + text is a label. Only one of them is shown (display: none), so + a screen reader reads it once. */} +
+ + {row.apparatusName} + + + {row.elementGroupName} + +
+
+ + + {/* In a card it is with the labels, below the text. */} + {row.hasVariants && ( + More variants + )} +
{ + it('shows the label in front of the value, and the label can be anything', () => { + const apparatus = renderToString( + , + ) + const group = renderToString() + + expect(apparatus).toMatch(/>Apparatus<\/span>]*>Uneven barsGroup<\/span>]*>Mounts { + const markup = renderToString() + + expect(markup).toMatch(/ + {/* In the table the column heading says what it is, so the two texts are plain. In a card there is no heading to see, so each text is a label. Only one of them is shown (display: none), so @@ -131,7 +131,7 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) {
-
+
From 885d052fd28066ba21d51aa38c415125a72e87a6 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 15:24:12 +0100 Subject: [PATCH 23/32] feat: indent the labels over the apparatus and group pills a little --- src/components/ui/LabelBadge.test.tsx | 4 ++++ src/components/ui/LabelBadge.tsx | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/ui/LabelBadge.test.tsx b/src/components/ui/LabelBadge.test.tsx index 97557d5..a36ee01 100644 --- a/src/components/ui/LabelBadge.test.tsx +++ b/src/components/ui/LabelBadge.test.tsx @@ -32,6 +32,10 @@ describe('LabelBadge', () => { expect(markup).toContain('flex-col') expect(markup).toContain('gap-0.5') + // The label is a little to the right of the pill (the pill itself stays to the + // left), which looks better. + expect(markup).toContain('aria-hidden="true" class="pl-2 ') + expect(markup).not.toContain('ml-2') }) it('takes more classes, which win over the ones it has', () => { diff --git a/src/components/ui/LabelBadge.tsx b/src/components/ui/LabelBadge.tsx index 174a630..0246d55 100644 --- a/src/components/ui/LabelBadge.tsx +++ b/src/components/ui/LabelBadge.tsx @@ -27,7 +27,7 @@ export default function LabelBadge({ > From a7e46e3ed2c59ca77c4caa62688488a268b74826 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 15:30:40 +0100 Subject: [PATCH 24/32] feat: keep the navy table heading on a phone (Element and D-value), and make More variants as tall as the label pills --- src/components/elements/ResultsTable.test.tsx | 41 ++++++++++++++++++- src/components/elements/ResultsTable.tsx | 31 +++++++++----- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index 122ea41..38fa2ac 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -22,12 +22,29 @@ describe('ResultsTable', () => { for (const heading of ['Code', 'Element', 'Apparatus', 'Value']) { expect(markup).toMatch( new RegExp( - `]*scope="col"[^>]*>(]*>)?(]*>)?${heading}()?(
)?`, + `]*scope="col"[^>]*>(]*>)?(]*>)?${heading}()?(]*>D-value)?(
)?`, ), ) } }) + it('keeps the navy heading on a phone, with Element on one side and D-value on the other', () => { + const markup = render(exampleElementRows) + + // The row is the navy bar on a phone, and the table's own heading is not hidden. + const headRow = /]*>]*)>/.exec(markup)?.[1] ?? '' + expect(headRow).toContain('max-md:bg-(--table-head-bg)') + expect(headRow).toContain('max-md:justify-between') + expect(markup).not.toMatch(/]*max-md:sr-only/) + // Element is shown on a phone, Code and Apparatus are only read out (their texts are + // in the card), and the value heading is "D-value" on a phone and "Value" in a table. + expect(markup).toMatch(/]*max-md:flex-1[^>]*>Element<\/th>/) + expect(markup).toMatch(/]*max-md:sr-only[^>]*>Code<\/th>/) + expect(markup).toMatch(/]*max-md:sr-only[^>]*>Apparatus<\/th>/) + expect(markup).toMatch(/max-md:hidden">Value<\/span>/) + expect(markup).toMatch(/md:hidden">D-value<\/span>/) + }) + it('has one row for each element, and each row links to its page', () => { const markup = render(exampleElementRows) @@ -89,7 +106,7 @@ describe('ResultsTable', () => { // The heading has the same columns, so it is over the badge. expect(markup).toMatch( new RegExp( - `
Value
`, + `
ValueD-value
`, ), ) expect(markup).not.toContain('absolute top-1/2 right-2') @@ -129,6 +146,26 @@ describe('ResultsTable', () => { ) }) + it('makes More variants as tall as the pills of the labels', () => { + const markup = render([withVariants]) + // The same text size, line height and padding as the pill of the LabelBadge, and a + // border that cannot be seen (the pill has a border). + const pill = new RegExp( + String.raw`More variants`, + ).exec(markup)?.[1] + + expect(pill).toBeTruthy() + for (const part of [ + 'border', + 'border-transparent', + 'py-1', + 'text-[0.8125rem]', + 'leading-snug', + ]) { + expect(pill?.split(' ')).toContain(part) + } + }) + it('shows the code in bold', () => { expect(render([plain])).toMatch( new RegExp(`]*font-bold[^>]*>${plain.code}
 Elements and vaults in the Code of Points, one row for each code
Code + Element Apparatus
{ - const markup = render(exampleElementRows) + const table = tableOf(render(exampleElementRows)) expect( - markup.match( + table.match( /]*role="row"[^>]*class="group relative cursor-pointer/g, ), ).toHaveLength(exampleElementRows.length) - expect(markup).toContain(`href="/elements/${plain.id}"`) - expect(markup).toContain(plain.description) + expect(table).toContain(`href="/elements/${plain.id}"`) + expect(table).toContain(plain.description) }) it('answers on the whole row, by getting taller and not wider, and not by underlining the text', () => { - const markup = render(exampleElementRows) + const table = tableOf(render(exampleElementRows)) - expect(markup).toContain('md:hover:*:shadow-[0_-5px_0_0_var(--row-hover)') - expect(markup).toContain( - 'md:focus-within:*:shadow-[0_-5px_0_0_var(--row-hover)', + expect(table).toContain('hover:*:shadow-[0_-5px_0_0_var(--row-hover)') + expect(table).toContain( + 'focus-within:*:shadow-[0_-5px_0_0_var(--row-hover)', ) // The text only gets a very thin shadow in its own color (a little bolder). More // than that makes it harder to read. - expect(markup).toContain('hover:[text-shadow:0.15px_0_0_currentColor,') - expect(markup).toContain( + expect(table).toContain('hover:[text-shadow:0.15px_0_0_currentColor,') + expect(table).toContain( 'focus-within:[text-shadow:0.15px_0_0_currentColor,', ) - expect(markup).not.toContain('0.35px') - expect(markup).not.toContain('hover:font-') - expect(markup).not.toContain('scale-') - expect(markup).not.toContain('underline') + expect(table).not.toContain('0.35px') + expect(table).not.toContain('hover:font-') + expect(table).not.toContain('scale-') + expect(table).not.toContain('underline') }) it('has a chevron in each row that only appears on hover and focus, and is hidden from a screen reader', () => { - const markup = render(exampleElementRows) + const table = tableOf(render(exampleElementRows)) - const chevrons = markup.match(/]*lucide-chevron-right[^>]*>/g) ?? [] + const chevrons = table.match(/]*lucide-chevron-right[^>]*>/g) ?? [] expect(chevrons).toHaveLength(exampleElementRows.length) for (const chevron of chevrons) { @@ -89,8 +85,8 @@ describe('ResultsTable', () => { } }) - it('puts the chevron next to the badge, not on top of it', () => { - const markup = render([plain]) + it('puts the chevron next to the badge, not on top of it, and the heading over the badge', () => { + const table = tableOf(render([plain])) // The [ and ] of the class are escaped, as they mean something in a regex. const layout = 'grid grid-cols-[1fr_auto_1fr] items-center'.replace( @@ -98,100 +94,154 @@ describe('ResultsTable', () => { '\\$&', ) // The badge is in the middle column and the chevron in the one to the right. - expect(markup).toMatch( + expect(table).toMatch( new RegExp( `
.*ValueD-value
`, + `
Value
`, ), ) - expect(markup).not.toContain('absolute top-1/2 right-2') + expect(table).not.toContain('absolute top-1/2 right-2') }) - it('shows the apparatus and the group as plain text in the table, and as labels in a card', () => { - const markup = render([plain]) + it('has the apparatus and the group in two columns of their own, not in one cell', () => { + const table = tableOf(render([plain])) - // The table: no label, the column heading says what it is. - expect(markup).toMatch( + expect(table).toMatch( new RegExp( - `
]*>${plain.apparatusName}]*>${plain.elementGroupName}
`, + `]*>${plain.apparatusName}]*>${plain.elementGroupName}`, ), ) - // The card: each text has its label in front, and the table's text is not shown. - expect(markup).toMatch( + expect(table).not.toMatch( new RegExp( - `
.*>Apparatus]*>${plain.apparatusName}.*>Group]*>${plain.elementGroupName}`, + `${plain.apparatusName}]*>${plain.elementGroupName}`, ), ) }) - it('puts More variants with the labels in a card, below them, and in the row in a table', () => { - const markup = render([withVariants]) + it('shows the code in bold', () => { + expect(tableOf(render([plain]))).toMatch( + new RegExp(`]*font-bold[^>]*>${plain.code}`), + ) + }) + + it('shows the name of the gymnast above the description', () => { + const table = tableOf(render([named])) + + expect(table).toMatch( + new RegExp(`${named.name}]*>${named.description}`), + ) + expect(tableOf(render([plain]))).not.toContain('font-heading') + }) + + it('says "More variants" in the row only on a row that has variants', () => { + expect(tableOf(render([withVariants]))).toContain('More variants') + expect(tableOf(render([plain, named]))).not.toContain('More variants') + }) + + it('uses the difficulty badge: the D-value of a vault, and the letter and value of an element', () => { + const vault = tableOf(render([plain])) + expect(vault).toContain('Difficulty 2.6') + + const letter = tableOf(render([element])) + expect(letter).toContain('Difficulty A') + expect(letter).toContain('Value 0.1') + }) +}) + +describe('ResultsTable: the cards (a phone and a small screen)', () => { + it('are a list of cards below each other, only on a small screen, and without a table heading', () => { + const markup = render(exampleElementRows) + const cards = cardsOf(markup) - // In the table: with the text, and not in a card. - expect(markup).toMatch( + expect(cards).toMatch(/^
    { + const plainCard = cardsOf(render([plain])) + const namedCard = cardsOf(render([named])) + + expect(plainCard).toMatch( new RegExp( - String.raw`More variants`, + `]*>]*href="/elements/${plain.id}"[^>]*>${plain.code}`, ), ) - // In a card: the last of the labels, below the apparatus and the group. - expect(markup).toMatch( + expect(namedCard).toMatch( new RegExp( - String.raw`
    .*>Group.*
    More variants
`, + `]*>]*>${named.code}]*>${named.name}`, ), ) }) - it('makes More variants as tall as the pills of the labels', () => { - const markup = render([withVariants]) - // The same text size, line height and padding as the pill of the LabelBadge, and a - // border that cannot be seen (the pill has a border). - const pill = new RegExp( - String.raw`More variants`, - ).exec(markup)?.[1] - - expect(pill).toBeTruthy() - for (const part of [ - 'border', - 'border-transparent', - 'py-1', - 'text-[0.8125rem]', - 'leading-snug', - ]) { - expect(pill?.split(' ')).toContain(part) - } + it('have the description under the title, then the rows downward (apparatus, group, value with its badge), then the drawing', () => { + const card = cardsOf(render([plain])) + + const order = [ + 'Apparatus:', + `>${plain.apparatusName}`, + '>Group:', + `>${plain.elementGroupName}`, + '>D-value:', + 'Difficulty 2.6', + ' card.indexOf(part)) + + expect(order.every((position) => position >= 0)).toBe(true) + expect([...order].sort((a, b) => a - b)).toEqual(order) }) - it('shows the code in bold', () => { - expect(render([plain])).toMatch( - new RegExp(`]*font-bold[^>]*>${plain.code}`), + it('show the drawing with a text for a screen reader, loaded when it is near', () => { + const plainCard = cardsOf(render([plain])) + const namedCard = cardsOf(render([named])) + + expect(plainCard).toContain(`src="${plain.imageUrl}"`) + expect(plainCard).toContain(`alt="Illustration for ${plain.code}"`) + expect(namedCard).toContain( + `alt="Illustration for ${named.code}: ${named.name}"`, ) + expect(plainCard).toContain('loading="lazy"') }) - it('shows the name of the gymnast above the description', () => { - const markup = render([named]) + it('show the drawing to the right only when the card is wide enough, and not at all on a narrow card', () => { + const card = cardsOf(render([plain])) + + // The card is a container, and the drawing is hidden until the card is 30rem wide. + expect(card).toContain('@container') + // Then the drawing has two fifths of the card, and the text the rest (so the + // description can go over two lines). + const image = /]*>/.exec(card)?.[0] ?? '' + expect(image.split(/[ "]/)).toContain('hidden') + expect(image).toContain('@[30rem]:block') + expect(card).toContain('@[30rem]:grid-cols-[3fr_2fr]') + }) - expect(markup).toMatch( - new RegExp(`${named.name}]*>${named.description}`), - ) - expect(render([plain])).not.toContain('font-heading') + it('call the value "Difficulty" for an element, and "D-value" for a vault', () => { + expect(cardsOf(render([element]))).toContain('>Difficulty:') + expect(cardsOf(render([plain]))).toContain('>D-value:') }) - it('says "More variants" only on a row that has variants', () => { - expect(render([withVariants])).toContain('More variants') - expect(render([plain, named])).not.toContain('More variants') + it('link as a whole card to the element page, and show where the focus is', () => { + const card = cardsOf(render([plain])) + + expect(card).toContain(`href="/elements/${plain.id}"`) + expect(card).toContain('after:absolute') + expect(card).toContain('focus-within:outline-2') + expect(card).toContain('focus-within:outline-solid') }) - it('uses the difficulty badge: the D-value of a vault, and the letter and value of an element', () => { - const vault = render([plain]) - expect(vault).toContain('Difficulty 2.6') + it('say "More variants" only on a card that has variants', () => { + const card = cardsOf(render([withVariants])) - const letter = render([element]) - expect(letter).toContain('Difficulty A') - expect(letter).toContain('Value 0.1') + expect(card).toContain('More variants') + expect(cardsOf(render([plain, named]))).not.toContain('More variants') }) }) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 5644052..5c97ce6 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -1,22 +1,107 @@ import { ChevronRight } from 'lucide-react' import { Link } from 'react-router-dom' import DifficultyBadge from '@/components/ui/DifficultyBadge' -import LabelBadge from '@/components/ui/LabelBadge' import { getDifficultyLetter } from '@/lib/difficulty' import type { ElementListRow } from '@/lib/elementList' -// Below the "md" breakpoint a row is a card (code, text and apparatus on the -// left, value on the right), because four columns do not fit a phone. The roles -// are written out because a table that is not displayed as a table loses them -// in some browsers. -const CELL = 'align-middle md:px-4 md:py-3' +// On a wide screen the results are a table. On a phone and a small screen (below +// the "md" breakpoint) they are cards below each other instead, because four +// columns do not fit. Both are always in the page, and CSS shows one of them, so a +// screen reader gets each result once. + +/** The letter of an element, or the D-value of a vault, for the badge. */ +const badgeValue = (row: ElementListRow) => + (!row.isVault && getDifficultyLetter(row.value)) || row.value + +const MORE_VARIANTS = + 'rounded-full bg-(--pill-bg) px-3 py-1 text-xs font-bold whitespace-nowrap text-(--pill-text)' + +// --------------------------------------------------------------------------- +// The cards (a phone and a small screen) +// --------------------------------------------------------------------------- + +/** + * A card is a small table with the rows downward: the title (the code, and the + * name if the element has one), the description, the apparatus, the group and the + * value with its badge. The whole card is one link (the title). + * + * The drawing is to the right, but only when the card is wide enough for it (a + * container query on the card, so it is the width of the card that counts and not + * the width of the window). On a narrow card it is not shown at all, and a hidden + * image is not loaded either. + */ +function ResultCards({ rows }: { rows: ElementListRow[] }) { + return ( +
    + {rows.map((row) => ( +
  • +
    +
    +

    + {/* The link covers the whole card (the ::after). */} + + {row.code} + {row.name && ( + <> + + + {row.name} + + + )} + +

    +

    {row.description}

    +
    +
    Apparatus:
    +
    {row.apparatusName}
    +
    Group:
    +
    {row.elementGroupName}
    +
    + {row.isVault ? 'D-value:' : 'Difficulty:'} +
    +
    + +
    +
    + {row.hasVariants && ( + + More variants + + )} +
    + {/* If the drawing is missing, nothing is shown, and not a broken image. */} + {`Illustration { + event.currentTarget.hidden = true + }} + className="hidden h-auto w-full self-start rounded-md bg-white object-contain @[30rem]:block" + /> +
    +
    • + ))} +
+ ) +} + +// --------------------------------------------------------------------------- +// The table (a wide screen) +// --------------------------------------------------------------------------- + +const CELL = 'align-middle px-4 py-3' // The badge is in the middle of the cell, and the chevron has the column to the // right of it, so it never ends up behind the badge, and the heading above can // use the same columns to be in the middle over the badge. -// The mark "More variants" is as tall as the pills of the LabelBadge (the same text -// size, line height, padding and a border that cannot be seen). -const MORE_VARIANTS = - 'rounded-full border border-transparent bg-(--pill-bg) px-3 py-1 text-[0.8125rem] leading-snug font-bold whitespace-nowrap text-(--pill-text)' const VALUE_LAYOUT = 'grid grid-cols-[1fr_auto_1fr] items-center' const COLUMN_HEADER = 'bg-(--table-head-bg) px-4 py-3 text-left text-[0.8125rem] font-bold uppercase tracking-wide text-(--table-head-text) first:rounded-tl-lg last:rounded-tr-lg' @@ -29,62 +114,57 @@ const COLUMN_HEADER = // the words wider and moves the line breaks, and more than a little makes the // text harder to read. (The classes are written out, so Tailwind can find them.) const ROW_EFFECT = - 'group relative cursor-pointer [--row-bg:var(--surface)] even:[--row-bg:var(--table-row-alt)] [--row-hover:color-mix(in_srgb,var(--row-bg),var(--text-color)_var(--row-hover-amount))] md:hover:*:bg-(--row-hover) md:focus-within:*:bg-(--row-hover) md:not-last:hover:bg-(--row-hover) md:not-last:focus-within:bg-(--row-hover) max-md:hover:bg-(--row-hover) max-md:focus-within:bg-(--row-hover) hover:z-10 focus-within:z-10 md:hover:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] md:focus-within:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] max-md:hover:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] max-md:focus-within:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] hover:[text-shadow:0.15px_0_0_currentColor,-0.15px_0_0_currentColor] focus-within:[text-shadow:0.15px_0_0_currentColor,-0.15px_0_0_currentColor] *:transition-[box-shadow,background-color] *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' + 'group relative cursor-pointer [--row-bg:var(--surface)] even:[--row-bg:var(--table-row-alt)] [--row-hover:color-mix(in_srgb,var(--row-bg),var(--text-color)_var(--row-hover-amount))] hover:*:bg-(--row-hover) focus-within:*:bg-(--row-hover) not-last:hover:bg-(--row-hover) not-last:focus-within:bg-(--row-hover) hover:z-10 focus-within:z-10 hover:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] focus-within:*:shadow-[0_-5px_0_0_var(--row-hover),0_5px_0_0_var(--row-hover)] hover:[text-shadow:0.15px_0_0_currentColor,-0.15px_0_0_currentColor] focus-within:[text-shadow:0.15px_0_0_currentColor,-0.15px_0_0_currentColor] *:transition-[box-shadow,background-color] *:duration-150 motion-reduce:*:transition-none focus-within:outline-2 focus-within:-outline-offset-2 focus-within:outline-(--text-color)' -// The colors are on the cells in a table (so the corners can be -// round), and on the row when it is a card. +// The colors are on the cells (so the corners can be round). const ROW_LOOK = - 'md:not-last:bg-(--surface) md:not-last:even:bg-(--table-row-alt) md:*:bg-(--surface) md:even:*:bg-(--table-row-alt) md:last:*:first:rounded-bl-lg md:last:*:last:rounded-br-lg max-md:bg-(--surface) max-md:even:bg-(--table-row-alt) max-md:last:rounded-b-lg max-md:grid max-md:grid-cols-[1fr_auto] max-md:gap-x-4 max-md:gap-y-1 max-md:px-4 max-md:py-3' + 'not-last:bg-(--surface) not-last:even:bg-(--table-row-alt) *:bg-(--surface) even:*:bg-(--table-row-alt) last:*:first:rounded-bl-lg last:*:last:rounded-br-lg' -export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { +function ResultTableView({ rows }: { rows: ElementListRow[] }) { return ( -
+
- {/* A phone shows cards, but the navy heading stays, with only two sides: - Element and D-value. Code and Apparatus are in the cards (the apparatus - has its own label), so their headings are only read out there. */} - + + @@ -92,13 +172,10 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { {rows.map((row) => ( - - - + + @@ -181,3 +236,12 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { ) } + +export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { + return ( + <> + + + + ) +} diff --git a/src/components/ui/DifficultyBadge.test.tsx b/src/components/ui/DifficultyBadge.test.tsx new file mode 100644 index 0000000..978bf15 --- /dev/null +++ b/src/components/ui/DifficultyBadge.test.tsx @@ -0,0 +1,40 @@ +import { renderToString } from 'react-dom/server' +import { describe, expect, it } from 'vitest' +import DifficultyBadge from './DifficultyBadge' + +/** The classes of the circle (the first span with rounded-full in the badge). */ +const circle = (markup: string) => + / { + it('always shows a number with one decimal', () => { + for (const [value, shown] of [ + [2, '2.0'], + [2.6, '2.6'], + [6.4, '6.4'], + [0.5, '0.5'], + [3.04, '3.0'], + ] as const) { + expect(renderToString()).toContain( + `Difficulty ${shown}`, + ) + } + }) + + it('has a smaller text for a number, and the same size as before for a letter', () => { + const number = circle(renderToString()) + const letter = circle(renderToString()) + + expect(number?.[1].split(' ')).toContain('text-base') + expect(number?.[1].split(' ')).not.toContain('text-lg') + expect(letter?.[1].split(' ')).toContain('text-lg') + expect(letter?.[1].split(' ')).not.toContain('text-base') + }) + + it('shows a letter as a letter, with its value under it', () => { + const markup = renderToString() + + expect(markup).toContain('Difficulty B') + expect(markup).toContain('Value 0.2') + }) +}) diff --git a/src/components/ui/DifficultyBadge.tsx b/src/components/ui/DifficultyBadge.tsx index d49d82a..cce604c 100644 --- a/src/components/ui/DifficultyBadge.tsx +++ b/src/components/ui/DifficultyBadge.tsx @@ -9,7 +9,11 @@ export default function DifficultyBadge({ const letter = typeof difficulty === 'string' ? difficulty.toUpperCase() : undefined const value = letter ? getDifficultyValue(letter) : undefined - const displayDifficulty = value !== undefined ? letter : difficulty + // A number (the D-value of a vault) always has one decimal, so 2 is 2.0, and its text + // is a little smaller than a letter, so it fits in the circle. + const isNumber = typeof difficulty === 'number' + const displayDifficulty = + value !== undefined ? letter : isNumber ? difficulty.toFixed(1) : difficulty return ( @@ -19,7 +23,8 @@ export default function DifficultyBadge({ Difficulty diff --git a/src/components/ui/LabelBadge.test.tsx b/src/components/ui/LabelBadge.test.tsx deleted file mode 100644 index a36ee01..0000000 --- a/src/components/ui/LabelBadge.test.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { renderToString } from 'react-dom/server' -import { describe, expect, it } from 'vitest' -import LabelBadge from './LabelBadge' - -describe('LabelBadge', () => { - it('shows the label above the value, and the label can be anything', () => { - const apparatus = renderToString( - , - ) - const group = renderToString() - - expect(apparatus).toMatch(/>Apparatus<\/span>]*>Uneven barsGroup<\/span>]*>Mounts { - const markup = renderToString() - - expect(markup).toMatch(/]*>${plain.elementGroupName}`, + `]*>${plain.apparatusName}]*>${groupLabel(plain)}`, ), ) }) + it('shows a vault by the number of its group, and not by the long title of the group', () => { + const table = tableOf(render([plain, element])) + + expect(plain.elementGroupName.length).toBeGreaterThan(40) + expect(table).toContain(`>Group ${plain.elementGroupNumber}`) + expect(table).not.toContain(plain.elementGroupName) + // An element is shown by the name of its group, which is short. + expect(table).toContain(`>${element.elementGroupName}`) + }) + it('shows the code in bold', () => { expect(tableOf(render([plain]))).toMatch( new RegExp(`]*font-bold[^>]*>${plain.code}`), @@ -132,9 +141,11 @@ describe('ResultsTable: the table (a wide screen)', () => { it('shows the name of the gymnast above the description', () => { const table = tableOf(render([named])) - expect(table).toMatch( - new RegExp(`${named.name}]*>${named.description}`), - ) + // The description has characters (the brackets) that mean something in a regex, so + // it is not put in one. + const afterName = table.slice(table.indexOf(`${named.name}`)) + expect(afterName).toMatch(/^[^<]*<\/span>]*>/) + expect(afterName).toContain(`>${named.description}`) expect(tableOf(render([plain]))).not.toContain('font-heading') }) @@ -164,6 +175,13 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect(cards).not.toContain(' { + const cards = cardsOf(render(exampleElementRows)) + + expect(cards).toContain('grid-cols-1') + expect(cards).toContain('min-[600px]:grid-cols-2') + }) + it('have the title first: the code, and the name if there is one', () => { const plainCard = cardsOf(render([plain])) const namedCard = cardsOf(render([named])) @@ -180,7 +198,7 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { ) }) - it('have the description under the title, then the rows downward (apparatus, group, value with its badge), then the drawing', () => { + it('have the description under the title, then the rows downward (apparatus, group, value with its badge), then the drawing below', () => { const card = cardsOf(render([plain])) const order = [ @@ -189,9 +207,9 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { '>Apparatus:', `>${plain.apparatusName}`, '>Group:', - `>${plain.elementGroupName}`, + `>${groupLabel(plain)}`, '>D-value:', - 'Difficulty 2.6', + 'Difficulty ', ' card.indexOf(part)) @@ -199,29 +217,27 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect([...order].sort((a, b) => a - b)).toEqual(order) }) - it('show the drawing with a text for a screen reader, loaded when it is near', () => { + it('show a vault by the number of its group, and not by the long title of the group', () => { + const card = cardsOf(render([plain])) + + expect(card).toContain(`>Group ${plain.elementGroupNumber}`) + expect(card).not.toContain(plain.elementGroupName) + }) + + it('show the drawing below the text, always, with a text for a screen reader, loaded when it is near', () => { const plainCard = cardsOf(render([plain])) const namedCard = cardsOf(render([named])) - expect(plainCard).toContain(`src="${plain.imageUrl}"`) + const image = /]*>/.exec(plainCard)?.[0] ?? '' + expect(image).toContain(`src="${plain.imageUrl}"`) + expect(image).toContain('loading="lazy"') + expect(image.split(/[ "]/)).not.toContain('hidden') expect(plainCard).toContain(`alt="Illustration for ${plain.code}"`) expect(namedCard).toContain( `alt="Illustration for ${named.code}: ${named.name}"`, ) - expect(plainCard).toContain('loading="lazy"') - }) - - it('show the drawing to the right only when the card is wide enough, and not at all on a narrow card', () => { - const card = cardsOf(render([plain])) - - // The card is a container, and the drawing is hidden until the card is 30rem wide. - expect(card).toContain('@container') - // Then the drawing has two fifths of the card, and the text the rest (so the - // description can go over two lines). - const image = /]*>/.exec(card)?.[0] ?? '' - expect(image.split(/[ "]/)).toContain('hidden') - expect(image).toContain('@[30rem]:block') - expect(card).toContain('@[30rem]:grid-cols-[3fr_2fr]') + // Not beside the text: there is no column for it. + expect(plainCard).not.toContain('@container') }) it('call the value "Difficulty" for an element, and "D-value" for a vault', () => { @@ -238,6 +254,31 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect(card).toContain('focus-within:outline-solid') }) + it('answer on hover, to show that they can be pressed: they move up a little and get a stronger shadow', () => { + const card = cardsOf(render([plain])) + + expect(card).toContain('cursor-pointer') + expect(card).toContain('motion-safe:hover:-translate-y-0.5') + expect(card).toContain('hover:shadow-[') + // In dark mode the card is also a little lighter, as the rows of the table are + // (the amount is 0 % in light mode, where nothing changes color on hover). + expect(card).toContain('var(--row-hover-amount)') + expect(card).toContain('hover:bg-(--card-hover)') + }) + + it('have an arrow like the rows of the table: in on hover and focus, always there without hover, and hidden from a screen reader', () => { + const cards = cardsOf(render(exampleElementRows)) + + const arrows = cards.match(/]*lucide-chevron-right[^>]*>/g) ?? [] + expect(arrows).toHaveLength(exampleElementRows.length) + for (const arrow of arrows) { + expect(arrow).toContain('aria-hidden="true"') + expect(arrow).toContain('group-hover:opacity-100') + expect(arrow).toContain('group-focus-within:opacity-100') + expect(arrow).toContain('[@media(hover:none)]:opacity-100') + } + }) + it('say "More variants" only on a card that has variants', () => { const card = cardsOf(render([withVariants])) diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 5c97ce6..cc42a21 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -2,7 +2,7 @@ import { ChevronRight } from 'lucide-react' import { Link } from 'react-router-dom' import DifficultyBadge from '@/components/ui/DifficultyBadge' import { getDifficultyLetter } from '@/lib/difficulty' -import type { ElementListRow } from '@/lib/elementList' +import { groupLabel, type ElementListRow } from '@/lib/elementList' // On a wide screen the results are a table. On a phone and a small screen (below // the "md" breakpoint) they are cards below each other instead, because four @@ -23,71 +23,76 @@ const MORE_VARIANTS = /** * A card is a small table with the rows downward: the title (the code, and the * name if the element has one), the description, the apparatus, the group and the - * value with its badge. The whole card is one link (the title). + * value with its badge, and the drawing below. The whole card is one link (the + * title). The cards are in two columns when the screen is wide enough for it. * - * The drawing is to the right, but only when the card is wide enough for it (a - * container query on the card, so it is the width of the card that counts and not - * the width of the window). On a narrow card it is not shown at all, and a hidden - * image is not loaded either. + * To show that a card can be pressed it moves up a little and gets a stronger + * shadow on hover (in dark mode it is also a little lighter, as the rows of the + * table are), and it has an arrow, like the rows of the table. The arrow comes in + * on hover and focus, and is always there on a screen without hover (a phone), where + * nothing else would show it. */ function ResultCards({ rows }: { rows: ElementListRow[] }) { return ( -
    +
      {rows.map((row) => (
    • -
      -
      -

      - {/* The link covers the whole card (the ::after). */} - - {row.code} - {row.name && ( - <> - - - {row.name} - - - )} - -

      -

      {row.description}

      -
      -
      Apparatus:
      -
      {row.apparatusName}
      -
      Group:
      -
      {row.elementGroupName}
      -
      - {row.isVault ? 'D-value:' : 'Difficulty:'} -
      -
      - -
      -
      - {row.hasVariants && ( - - More variants - - )} -
      - {/* If the drawing is missing, nothing is shown, and not a broken image. */} - {`Illustration { - event.currentTarget.hidden = true - }} - className="hidden h-auto w-full self-start rounded-md bg-white object-contain @[30rem]:block" +
      +

      + {/* The link covers the whole card (the ::after). */} + + {row.code} + {row.name && ( + <> + + + {row.name} + + + )} + +

      +
      +

      {row.description}

      +
      +
      Apparatus:
      +
      {row.apparatusName}
      +
      Group:
      +
      {groupLabel(row)}
      +
      + {row.isVault ? 'D-value:' : 'Difficulty:'} +
      +
      + +
      +
      + {row.hasVariants && ( + + More variants + + )} + {/* The drawing is below the text. If it is missing, nothing is shown, + and not a broken image. */} + {`Illustration { + event.currentTarget.hidden = true + }} + className="mt-4 block h-auto max-h-56 w-full rounded-md bg-white object-contain" + />
      • ))}
    @@ -213,7 +218,7 @@ function ResultTableView({ rows }: { rows: ElementListRow[] }) { role="cell" className={`${CELL} text-[0.8125rem] leading-snug`} > - {row.elementGroupName} + {groupLabel(row)}
@@ -220,7 +220,7 @@ function ResultTableView({ rows }: { rows: ElementListRow[] }) {
Elements and vaults in the Code of Points, one row for each code
Code + Element + - Element + Apparatus - Apparatus + Group {/* The same three columns as the cell, so the heading is exactly over the badge. */}
- Value - D-value + Value
+ {row.code} +
{/* The link covers the whole row (the ::after), so a click anywhere in the row opens the element. */} @@ -121,32 +198,6 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) { {row.description} - {row.hasVariants && ( - - More variants - - )} -
- 		- {/* In the table the column heading says what it is, so the two - texts are plain. In a card there is no heading to see, so each - text is a label. Only one of them is shown (display: none), so - a screen reader reads it once. */} -
- - {row.apparatusName} - - - {row.elementGroupName} - -
-
-
- - -
- {/* In a card it is with the labels, below them. */} {row.hasVariants && ( More variants )} @@ -154,23 +205,27 @@ export default function ResultsTable({ rows }: { rows: ElementListRow[] }) {
+ {row.apparatusName} + + {row.elementGroupName} +
{/* The letter of an element, or the D-value of a vault. */} - + {/* Shown on hover and focus, with its own place so nothing - moves when it shows. A phone does not show it. */} + moves when it shows. */}
diff --git a/src/lib/elementList.ts b/src/lib/elementList.ts index 1454a87..65d1b95 100644 --- a/src/lib/elementList.ts +++ b/src/lib/elementList.ts @@ -7,6 +7,8 @@ export type ElementListRow = { description: string apparatusName: string elementGroupName: string + /** The number of the group on the apparatus (the API's `elementGroup.number`). */ + elementGroupNumber: number /** The value of an element, or the D-value of a vault. */ value: number isVault: boolean @@ -30,38 +32,42 @@ const imageOf = (id: string) => `${IMAGE_BASE}/${id.slice(0, 2)}/${id}.webp` // and only show the design until the page is connected to the API. const exampleRowsWithoutImage: Omit[] = [ { - id: 'VT-1.10-a', - code: '1.10', + id: 'VT-1.22-a', + code: '1.22', name: null, - description: 'Yamashita', + description: 'Handspring fwd with ½ turn (180°) on – 1/1 turn (360°) off', apparatusName: 'Vault', elementGroupName: 'Handspring, Yamashita, round-off with or without turn in 1st and/or 2nd flight phase', - value: 2.0, + elementGroupNumber: 1, + value: 2.6, isVault: true, hasVariants: false, }, { - id: 'VT-1.22-a', - code: '1.22', - name: null, - description: 'Handspring fwd with ½ turn (180°) on – 1/1 turn (360°) off', + id: 'VT-2.31-a', + code: '2.31', + name: 'Wang', + description: + 'Handspring fwd on – stretched salto fwd with ½ twist (180°) off', apparatusName: 'Vault', elementGroupName: - 'Handspring, Yamashita, round-off with or without turn in 1st and/or 2nd flight phase', - value: 2.6, + 'Handspring fwd with/without 1/1 turn (360°) in 1st flight phase – salto fwd/bwd with/without twist in 2nd flight phase', + elementGroupNumber: 2, + value: 4.6, isVault: true, hasVariants: false, }, { - id: 'VT-3.20-a', - code: '3.20', - name: null, - description: 'Tsukahara piked', + id: 'VT-2.50-a', + code: '2.50', + name: 'Produnova', + description: 'Handspring fwd on – tucked double salto fwd off', apparatusName: 'Vault', elementGroupName: - 'Handspring with ¼ - ½ turn (90°-180°) in 1st flight phase (Tsukahara) – salto bwd with/without twist in 2nd flight phase', - value: 3.4, + 'Handspring fwd with/without 1/1 turn (360°) in 1st flight phase – salto fwd/bwd with/without twist in 2nd flight phase', + elementGroupNumber: 2, + value: 6, isVault: true, hasVariants: false, }, @@ -73,7 +79,8 @@ const exampleRowsWithoutImage: Omit[] = [ apparatusName: 'Vault', elementGroupName: 'Round-off (Yurchenko) with/wo ¾ turn (270°) in 1st flight phase – salto bwd with/without twist in 2nd flight phase', - value: 3.0, + elementGroupNumber: 4, + value: 3, isVault: true, hasVariants: false, }, @@ -86,6 +93,7 @@ const exampleRowsWithoutImage: Omit[] = [ apparatusName: 'Vault', elementGroupName: 'Round-off (Yurchenko) with/wo ¾ turn (270°) in 1st flight phase – salto bwd with/without twist in 2nd flight phase', + elementGroupNumber: 4, value: 5.4, isVault: true, hasVariants: false, @@ -98,6 +106,7 @@ const exampleRowsWithoutImage: Omit[] = [ apparatusName: 'Vault', elementGroupName: 'Round-off (Yurchenko) with/wo ¾ turn (270°) in 1st flight phase – salto bwd with/without twist in 2nd flight phase', + elementGroupNumber: 4, value: 6.4, isVault: true, hasVariants: false, @@ -109,6 +118,7 @@ const exampleRowsWithoutImage: Omit[] = [ description: 'Glide kip to support on LB', apparatusName: 'Uneven bars', elementGroupName: 'Mounts', + elementGroupNumber: 1, value: 0.1, isVault: false, hasVariants: true, @@ -118,3 +128,14 @@ const exampleRowsWithoutImage: Omit[] = [ export const exampleElementRows: ElementListRow[] = exampleRowsWithoutImage.map( (row) => ({ ...row, imageUrl: imageOf(row.id) }), ) + +/** + * The group to show. A vault group has no short name: the name in the Code of + * Points is the long title of the group (for example "Round-off (Yurchenko) with/wo + * ¾ turn (270°) in 1st flight phase – salto bwd with/without twist in 2nd flight + * phase"), which is a description and does not fit as a label. A vault is + * therefore shown by the number of its group ("Group 4"), and an element by the name + * of its group ("Mounts"). + */ +export const groupLabel = (row: ElementListRow): string => + row.isVault ? `Group ${row.elementGroupNumber}` : row.elementGroupName From 02ed9e74fe45a4ab3b7654fb26d4ebe3ff9fa571 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 16:25:57 +0100 Subject: [PATCH 27/32] 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() {
)?`, @@ -206,9 +212,9 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { plain.description, '>Apparatus:', `>${plain.apparatusName}`, - '>Group:', + '>Element group:', `>${groupLabel(plain)}`, - '>D-value:', + '>Value:', 'Difficulty ', ' card.indexOf(part)) @@ -240,9 +246,20 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect(plainCard).not.toContain('@container') }) - it('call the value "Difficulty" for an element, and "D-value" for a vault', () => { - expect(cardsOf(render([element]))).toContain('>Difficulty:') - expect(cardsOf(render([plain]))).toContain('>D-value:') + it('call the value "Value" in every card, and the group "Element group", each as the label of its value', () => { + for (const row of [element, plain]) { + const card = cardsOf(render([row])) + + expect(card).toContain('>Value:') + expect(card).not.toContain('>Difficulty:') + expect(card).not.toContain('>D-value:') + expect(card).toContain('>Element group:') + // A label (dt) and its value (dd) are in one description list, so they are + // connected for a screen reader. + expect(card).toMatch( + /]*>.*>Element group:<\/dt>]*>[^<]*<\/dd>.*<\/dl>/s, + ) + } }) it('link as a whole card to the element page, and show where the focus is', () => { diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index cc42a21..47ba032 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -64,14 +64,12 @@ function ResultCards({ rows }: { rows: ElementListRow[] }) { />

{row.description}

-
+
Apparatus:
{row.apparatusName}
-
Group:
+
Element group:
{groupLabel(row)}
-
- {row.isVault ? 'D-value:' : 'Difficulty:'} -
+
Value:
@@ -159,7 +157,7 @@ function ResultTableView({ rows }: { rows: ElementListRow[] }) { scope="col" className={`${COLUMN_HEADER} w-[19%]`} > - Group + Element group
Date: Thu, 29 Oct 2026 16:38:48 +0100 Subject: [PATCH 29/32] 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 d2cd7601becf911071176b938a5fc699e4e9b512 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 16:42:26 +0100 Subject: [PATCH 30/32] feat: stop counting the matches of a search at 1001 --- backend/src/catalog/search.test.ts | 51 ++++++++++++++++++++++++++++++ backend/src/catalog/search.ts | 30 +++++++++++++++--- backend/src/schema.graphql | 3 +- docs/AI_USAGE.md | 6 ++++ docs/API.md | 2 +- 5 files changed, 85 insertions(+), 7 deletions(-) diff --git a/backend/src/catalog/search.test.ts b/backend/src/catalog/search.test.ts index 71adfbb..1cb75a0 100644 --- a/backend/src/catalog/search.test.ts +++ b/backend/src/catalog/search.test.ts @@ -12,6 +12,7 @@ import { BadUserInputError, type CatalogSearchArgs, listElementGroups, + MAX_TOTAL_COUNT, searchCatalog, searchTerms, } from './search.js' @@ -502,4 +503,54 @@ describe.runIf(hasDatabase)('searchCatalog on the seeded catalog', () => { const page = await searchCatalog(prisma, { first: 1 }) expect(page.totalCount).toBeUndefined() }) + + describe('the count stops at the most that is counted', () => { + const count = async (maxTotalCount?: number, search?: string) => + ( + await searchCatalog(prisma, { + first: 1, + search, + withTotalCount: true, + maxTotalCount, + }) + ).totalCount + + it('counts up to 1000 as it is, and the catalog is smaller than that', async () => { + expect(MAX_TOTAL_COUNT).toBe(1000) + expect(totalEntries).toBeLessThanOrEqual(MAX_TOTAL_COUNT) + expect(await count()).toBe(totalEntries) + }) + + it('stops at one more than the most, so a count above it means "more than that"', async () => { + expect(totalEntries).toBeGreaterThan(10) + expect(await count(10)).toBe(11) + expect(await count(1)).toBe(2) + }) + + it('is not capped when the most is as big as the number of matches, or bigger', async () => { + expect(await count(totalEntries)).toBe(totalEntries) + expect(await count(totalEntries + 500)).toBe(totalEntries) + // The most that is one less than the number of matches: one more than it is the + // number of matches, which is above the most. + expect(await count(totalEntries - 1)).toBe(totalEntries) + }) + + it('caps a search too, and a search with fewer matches than the most is counted as it is', async () => { + const all = await searchCatalog(prisma, { + first: 100, + search: 'handspring', + withTotalCount: true, + }) + expect(all.totalCount).toBeGreaterThan(5) + + expect(await count(5, 'handspring')).toBe(6) + expect(await count(all.totalCount! + 10, 'handspring')).toBe( + all.totalCount, + ) + }) + + it('counts nothing for text with no searchable words, also with a cap', async () => { + expect(await count(10, '%')).toBe(0) + }) + }) }) diff --git a/backend/src/catalog/search.ts b/backend/src/catalog/search.ts index 6115d75..3b806fe 100644 --- a/backend/src/catalog/search.ts +++ b/backend/src/catalog/search.ts @@ -36,19 +36,33 @@ export type CatalogSearchArgs = { /** Page size, 1–100. */ first?: number | null after?: string | null - /** Count all matches. Only set it when the client asks for totalCount. */ + /** Count the matches. Only set it when the client asks for totalCount. */ withTotalCount?: boolean + /** + * The most matches that are counted, MAX_TOTAL_COUNT if it is left out. The count + * stops at one more than this, so a count above it means "more than this many". + */ + maxTotalCount?: number } export type CatalogSearchResult = { edges: { cursor: string; node: CatalogEntry }[] pageInfo: { hasNextPage: boolean; endCursor: string | null } - /** Only set when withTotalCount was true. */ + /** + * Only set when withTotalCount was true. At most maxTotalCount + 1: a value above + * maxTotalCount means that there are more matches than that. + */ totalCount?: number } const DIRECTIONS: readonly SortDirection[] = ['ASC', 'DESC'] const MAX_PAGE_SIZE = 100 +/** + * How many matches `totalCount` counts. A search that has more matches gets one more + * than this (1001), which the interface shows as "1000+". The database stops counting + * there, so the cost of the count does not grow with the catalog. + */ +export const MAX_TOTAL_COUNT = 1000 /** Limits for the search text. Far more than anyone types, and each word adds a condition to the query. */ export const MAX_SEARCH_LENGTH = 80 export const MAX_SEARCH_WORDS = 8 @@ -237,9 +251,15 @@ export async function searchCatalog( endCursor: edges.at(-1)?.cursor ?? null, }, } - // Counting reads every match, so it only runs when the client asks for it. - if (args.withTotalCount) - result.totalCount = await prisma.catalogEntry.count({ where }) + // Counting reads the matches, so it only runs when the client asks for it, and it + // stops after one more than the most that is counted (take). + if (args.withTotalCount) { + const max = args.maxTotalCount ?? MAX_TOTAL_COUNT + result.totalCount = await prisma.catalogEntry.count({ + where, + take: max + 1, + }) + } return result } diff --git a/backend/src/schema.graphql b/backend/src/schema.graphql index 6dfa941..d3180e6 100644 --- a/backend/src/schema.graphql +++ b/backend/src/schema.graphql @@ -205,7 +205,8 @@ type CatalogConnection { """ Number of matches in the whole result set, not only this page. Counting is expensive on a large result, so the server only counts when the client asks - for this field. + for this field, and it stops at 1001: a value above 1000 means that there are + more than 1000 matches (the interface shows "1000+"). """ totalCount: Int! } diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 57ddbf7..184e762 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -460,3 +460,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 count of the search stops at 1001 (`backend/src/catalog/search.ts`) + +- **Tool:** Claude Code +- **What the team did:** Decided that the page says "1000+" when there are more than 1000 results, and that the server must not count more than that. +- **What Claude did:** Made the database stop counting at 1001 (`count` with `take`), kept the schema the same (a `totalCount` above 1000 means more than 1000), wrote the tests (the count stops at one more than the most, it is not capped below that, and a search is capped too) and broke the cap to check that they fail without it. diff --git a/docs/API.md b/docs/API.md index e4bc931..7b61a51 100644 --- a/docs/API.md +++ b/docs/API.md @@ -158,7 +158,7 @@ The requirement is that the solution works with any amount of data. The catalog - **Filter and sort need indexes.** `schema.prisma` declares an index for each sort field combined with `id` (`codeSort`, `difficulty`, and `hasName` with `nameSort`), and for the filter columns (apparatus, group). Without them the database sorts the whole catalog for every request. The name sort puts named items first in both directions (`hasName` descending), so ascending and descending each have their own index. The apparatus filter is often combined with a sort, so there are also indexes on apparatus with `difficulty` and with `codeSort`. The difficulty sort always has an apparatus filter, so it uses the index on apparatus and `difficulty`. The difficulty filter without an apparatus uses the index on `difficulty`. Without them, a filtered page walks past the rows of the other apparatus first, which is slow in one direction (for example vault sorted by ascending difficulty). - **Sort by name needs a rule for items without a name.** Most items have no name. A cursor does not accept `nulls` placement, so the sort uses a plain `hasName` field and a `nameSort` field that is never `null` (see Search without raw SQL). - **The text search must not scan the table.** A search for `%word%` cannot use an ordinary index. The search uses a word table with a B-tree index and a prefix match, which finds the rows that have a word starting with the search term without reading every row. A search that is correct but reads every row works on 500 rows and not on 5 million. -- **`totalCount` is only calculated when the client asks for it.** Counting a large result takes time: about 30 ms for the whole catalog and about 100 ms for a search in a test with 225 000 entries, and the cost grows with the catalog. The resolver for the field runs only if `totalCount` is in the query, so a client that scrolls without showing a total does not pay for it. The client should therefore ask for `totalCount` only with the first page of a new search or filter, and not with the pages that are loaded while scrolling. Together with the debounce on the search field, this keeps the count from running on every keystroke. If the count is still too slow, it can be capped (for example "1000+"), which needs a change in the schema. +- **`totalCount` is only calculated when the client asks for it.** Counting a large result takes time: about 30 ms for the whole catalog and about 100 ms for a search in a test with 225 000 entries, and the cost grows with the catalog. The resolver for the field runs only if `totalCount` is in the query, so a client that scrolls without showing a total does not pay for it. The client should therefore ask for `totalCount` only with the first page of a new search or filter, and not with the pages that are loaded while scrolling. Together with the debounce on the search field, this keeps the count from running on every keystroke. **The count is capped:** it stops at 1001 (`MAX_TOTAL_COUNT` is 1000, and the database counts one more), so the cost does not grow with the catalog. A `totalCount` above 1000 means that there are more than 1000 matches, and the interface shows "1000+". The schema is the same (`Int!`), only the meaning of a value above 1000 is new. - **Elements and vaults are different tables, but one list.** A query cannot combine two tables into one sorted and paged result. The data model keeps them apart because they work differently, so the list is read from `CatalogEntry`, a read-only table with the shared columns that the seed script fills. It never gets out of date because the catalog never changes between seed runs. - **Variants are few.** A variant group has two or three members, so `variants` does not need paging. From 7d6295b8f228580b3c1a6f80ea87032e428863f4 Mon Sep 17 00:00:00 2001 From: Ronja Date: Thu, 29 Oct 2026 16:44:16 +0100 Subject: [PATCH 31/32] 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 From 98423d05c10e1cff52f8251fa7c4664d7eeca3de Mon Sep 17 00:00:00 2001 From: Ronja Date: Fri, 30 Oct 2026 12:22:28 +0100 Subject: [PATCH 32/32] refactor: call the value and the D-value "Difficulty" everywhere --- src/components/elements/ElementMetadataCard.tsx | 4 ++-- src/components/elements/ResultsTable.test.tsx | 14 +++++++------- src/components/elements/ResultsTable.tsx | 8 ++++---- src/components/ui/DifficultyBadge.tsx | 2 +- src/components/ui/ElementVariants.tsx | 2 +- src/lib/elementList.ts | 2 +- src/lib/searchOperations.test.ts | 2 +- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/components/elements/ElementMetadataCard.tsx b/src/components/elements/ElementMetadataCard.tsx index da87a2c..19768e0 100644 --- a/src/components/elements/ElementMetadataCard.tsx +++ b/src/components/elements/ElementMetadataCard.tsx @@ -38,10 +38,10 @@ export default function ElementMetadataCard({
- + {element.isVault || !difficulty ? ( ) : ( diff --git a/src/components/elements/ResultsTable.test.tsx b/src/components/elements/ResultsTable.test.tsx index 51015cb..ecab212 100644 --- a/src/components/elements/ResultsTable.test.tsx +++ b/src/components/elements/ResultsTable.test.tsx @@ -32,7 +32,7 @@ describe('ResultsTable: the table (a wide screen)', () => { 'Element', 'Apparatus', 'Element group', - 'Value', + 'Difficulty', ]) { expect(table).toMatch( new RegExp( @@ -109,7 +109,7 @@ describe('ResultsTable: the table (a wide screen)', () => { // The heading has the same columns, so it is over the badge. expect(table).toMatch( new RegExp( - `
Value
`, + `
Difficulty
`, ), ) expect(table).not.toContain('absolute top-1/2 right-2') @@ -157,7 +157,7 @@ describe('ResultsTable: the table (a wide screen)', () => { expect(tableOf(render([plain, named]))).not.toContain('More variants') }) - it('uses the difficulty badge: the D-value of a vault, and the letter and value of an element', () => { + it('uses the difficulty badge: the difficulty of a vault, and the letter and value of an element', () => { const vault = tableOf(render([plain])) expect(vault).toContain('Difficulty 2.6') @@ -211,7 +211,7 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { `>${plain.apparatusName}`, '>Element group:', `>${groupLabel(plain)}`, - '>Value:', + '>Difficulty:', 'Difficulty ', ' card.indexOf(part)) @@ -243,12 +243,12 @@ describe('ResultsTable: the cards (a phone and a small screen)', () => { expect(plainCard).not.toContain('@container') }) - it('call the value "Value" in every card, and the group "Element group", each as the label of its value', () => { + it('call the difficulty "Difficulty" in every card, and the group "Element group", each as the label of its value', () => { for (const row of [element, plain]) { const card = cardsOf(render([row])) - expect(card).toContain('>Value:') - expect(card).not.toContain('>Difficulty:') + expect(card).toContain('>Difficulty:') + expect(card).not.toContain('>Value:') expect(card).not.toContain('>D-value:') expect(card).toContain('>Element group:') // A label (dt) and its value (dd) are in one description list, so they are diff --git a/src/components/elements/ResultsTable.tsx b/src/components/elements/ResultsTable.tsx index 47ba032..ab91b33 100644 --- a/src/components/elements/ResultsTable.tsx +++ b/src/components/elements/ResultsTable.tsx @@ -9,7 +9,7 @@ import { groupLabel, type ElementListRow } from '@/lib/elementList' // columns do not fit. Both are always in the page, and CSS shows one of them, so a // screen reader gets each result once. -/** The letter of an element, or the D-value of a vault, for the badge. */ +/** The letter of an element, or the difficulty of a vault, for the badge. */ const badgeValue = (row: ElementListRow) => (!row.isVault && getDifficultyLetter(row.value)) || row.value @@ -69,7 +69,7 @@ function ResultCards({ rows }: { rows: ElementListRow[] }) {
{row.apparatusName}
Element group:
{groupLabel(row)}
-
Value:
+
Difficulty:
@@ -167,7 +167,7 @@ function ResultTableView({ rows }: { rows: ElementListRow[] }) { {/* The same three columns as the cell, so the heading is exactly over the badge. */}
- Value + Difficulty
- {/* The letter of an element, or the D-value of a vault. */} + {/* The letter of an element, or the difficulty of a vault. */} diff --git a/src/components/ui/DifficultyBadge.tsx b/src/components/ui/DifficultyBadge.tsx index cce604c..cda52be 100644 --- a/src/components/ui/DifficultyBadge.tsx +++ b/src/components/ui/DifficultyBadge.tsx @@ -9,7 +9,7 @@ export default function DifficultyBadge({ const letter = typeof difficulty === 'string' ? difficulty.toUpperCase() : undefined const value = letter ? getDifficultyValue(letter) : undefined - // A number (the D-value of a vault) always has one decimal, so 2 is 2.0, and its text + // A number (the difficulty of a vault) always has one decimal, so 2 is 2.0, and its text // is a little smaller than a letter, so it fits in the circle. const isNumber = typeof difficulty === 'number' const displayDifficulty = diff --git a/src/components/ui/ElementVariants.tsx b/src/components/ui/ElementVariants.tsx index 63996b7..00b069c 100644 --- a/src/components/ui/ElementVariants.tsx +++ b/src/components/ui/ElementVariants.tsx @@ -51,7 +51,7 @@ export default function ElementVariants({ ) : ( )} diff --git a/src/lib/elementList.ts b/src/lib/elementList.ts index 6097628..e0142d9 100644 --- a/src/lib/elementList.ts +++ b/src/lib/elementList.ts @@ -9,7 +9,7 @@ export type ElementListRow = { elementGroupName: string /** The number of the group on the apparatus (the API's `elementGroup.number`). */ elementGroupNumber: number - /** The value of an element, or the D-value of a vault. */ + /** The value of an element, or the difficulty of a vault. */ value: number isVault: boolean hasVariants: boolean diff --git a/src/lib/searchOperations.test.ts b/src/lib/searchOperations.test.ts index 819bd44..c83da47 100644 --- a/src/lib/searchOperations.test.ts +++ b/src/lib/searchOperations.test.ts @@ -57,7 +57,7 @@ describe('toListRow', () => { }) }) - it('makes a row of a vault: its D-value, the name of the gymnast, and no variants', () => { + it('makes a row of a vault: its difficulty, the name of the gymnast, and no variants', () => { expect(toListRow(vault)).toMatchObject({ name: 'Amanar', apparatusName: 'Vault',