diff --git a/backend/src/catalog/structure.ts b/backend/src/catalog/structure.ts index ef902b0..463070d 100644 --- a/backend/src/catalog/structure.ts +++ b/backend/src/catalog/structure.ts @@ -18,6 +18,8 @@ export type CatalogStructure = { * cost does not depend on how many requests there are. */ difficultyValuesByApparatus: ReadonlyMap + /** The same for each group, from the same kind of `groupBy`. */ + difficultyValuesByGroup: ReadonlyMap } const cache = new WeakMap< @@ -26,25 +28,38 @@ const cache = new WeakMap< >() async function load(prisma: PrismaClient): Promise { - const [apparatus, groups, difficulties] = await Promise.all([ - prisma.apparatus.findMany({ select: { id: true } }), - prisma.elementGroup.findMany({ orderBy: { number: 'asc' } }), - // The index on (apparatus_id, difficulty, id) serves this. - prisma.catalogEntry.groupBy({ - by: ['apparatusId', 'difficulty'], - orderBy: { difficulty: 'asc' }, - }), - ]) + const [apparatus, groups, difficulties, groupDifficulties] = + await Promise.all([ + prisma.apparatus.findMany({ select: { id: true } }), + prisma.elementGroup.findMany({ orderBy: { number: 'asc' } }), + // The index on (apparatus_id, difficulty, id) serves this. + prisma.catalogEntry.groupBy({ + by: ['apparatusId', 'difficulty'], + orderBy: { difficulty: 'asc' }, + }), + // The index on (element_group_id) is enough: the table is a few hundred rows. + prisma.catalogEntry.groupBy({ + by: ['elementGroupId', 'difficulty'], + orderBy: { difficulty: 'asc' }, + }), + ]) const difficultyValuesByApparatus = new Map() for (const { apparatusId, difficulty } of difficulties) { const values = difficultyValuesByApparatus.get(apparatusId) ?? [] values.push(Number(difficulty)) difficultyValuesByApparatus.set(apparatusId, values) } + const difficultyValuesByGroup = new Map() + for (const { elementGroupId, difficulty } of groupDifficulties) { + const values = difficultyValuesByGroup.get(elementGroupId) ?? [] + values.push(Number(difficulty)) + difficultyValuesByGroup.set(elementGroupId, values) + } return { apparatusIds: new Set(apparatus.map((a) => a.id)), groupsById: new Map(groups.map((g) => [g.id, g])), difficultyValuesByApparatus, + difficultyValuesByGroup, } } diff --git a/backend/src/schema.graphql b/backend/src/schema.graphql index 1a7ce8f..7ca8da4 100644 --- a/backend/src/schema.graphql +++ b/backend/src/schema.graphql @@ -64,6 +64,12 @@ type ElementGroup { number: Int! name: String! apparatus: Apparatus! + """ + The difficulty values that exist in this group, lowest first. A filter on this group + together with a difficulty that is not in the list has no results, so the interface can + show that choice as unavailable. Read once and kept for ten minutes. + """ + difficultyValues: [Float!]! } """ diff --git a/backend/src/server/resolvers/catalog.ts b/backend/src/server/resolvers/catalog.ts index 7730b16..db4162b 100644 --- a/backend/src/server/resolvers/catalog.ts +++ b/backend/src/server/resolvers/catalog.ts @@ -151,6 +151,15 @@ export const catalogResolvers = { throw new Error(`The apparatus ${group.apparatusId} does not exist.`) return apparatus }, + difficultyValues: async ( + group: ElementGroup, + _args: unknown, + ctx: Context, + ): Promise => [ + ...((await catalogStructure(ctx.prisma)).difficultyValuesByGroup.get( + group.id, + ) ?? []), + ], }, CatalogConnection: { diff --git a/backend/src/server/server.test.ts b/backend/src/server/server.test.ts index 0cd7248..d4cdb15 100644 --- a/backend/src/server/server.test.ts +++ b/backend/src/server/server.test.ts @@ -133,6 +133,30 @@ describe.runIf(hasDatabase)('the GraphQL server on the seeded catalog', () => { expect(Math.min(...vault.difficultyValues)).toBeGreaterThan(1) }) + it('lists the difficulty values that exist in each group, so a filter that has no result can be known before it is made', async () => { + const { data, errors } = await run<{ + apparatus: { + id: string + groups: { id: string; difficultyValues: number[] }[] + }[] + }>('{ apparatus { id groups { id difficultyValues } } }') + + expect(errors).toBeUndefined() + for (const { groups } of data!.apparatus) { + for (const { id, difficultyValues } of groups) { + const expected = ( + await prisma.catalogEntry.findMany({ + where: { elementGroupId: id }, + select: { difficulty: true }, + distinct: ['difficulty'], + orderBy: { difficulty: 'asc' }, + }) + ).map((e) => Number(e.difficulty)) + expect(difficultyValues).toEqual(expected) + } + } + }) + it('gives an element with its value, drawing, group and removed flag', async () => { const row = await prisma.element.findFirstOrThrow({ orderBy: { id: 'asc' }, diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 44e324f..d6a59ee 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -596,3 +596,10 @@ - **What the team did:** Took the peer feedback that the README (about 59 kB) is too long to find your way in, wrote issue #182, and asked for the reasoning to be kept. - **What Claude did:** Moved four long sections unchanged to their own documents (`ELEMENTS_PAGE.md`, `DATABASE_SETUP.md`, `CATALOG_SEARCH.md`, `E2E_TESTS.md`) with a short summary and a link in the README (now about half the size). Added a "Where is what" table that points from each feature to the file where it starts, and a list of contents at the top of `docs/API.md`. A script checked that every relative link and anchor in the changed files works. - **Limits:** The text of the moved sections was not shortened or checked again, so what was outdated in them is still outdated (that is issue #148). A first run of the move emptied `README.md` because a formatting command was cut off in a pipe; it was restored from git and run again. + +## Filter choices that lead to an empty result (`ElementGroup.difficultyValues`, `FilterMenu`) + +- **Tool:** Claude Code +- **What the team did:** Took the peer feedback that a filter combination can lead to an empty result, wrote issue #170, and asked for the choices that have no result to be shown as unavailable. +- **What Claude did:** Added `difficultyValues` to `ElementGroup` in the API (the values that exist in the group, from the in-memory structure), so the page knows this before a filter is made and needs no extra request. The menus now show a choice with no result as disabled, with the reason read by a screen reader (`Reason` in `FilterMenu.tsx`): a group that has none of the chosen difficulties, and a difficulty that does not exist in the chosen group. A chosen value stays possible to uncheck. Vault is left out, because its difficulty is a band of D-values. Tests: a server test that compares the field with the database, unit tests of `groupHasDifficulty` and `difficultyInGroup`, a page test, and one end-to-end test. +- **Limits:** A screen reader was not used; that has to be done by a person. The end-to-end test covers one direction (a difficulty that does not exist in the chosen group); the other direction (a group without the chosen difficulty) is only covered by unit and page tests. diff --git a/e2e/unavailable-filters.e2e.ts b/e2e/unavailable-filters.e2e.ts new file mode 100644 index 0000000..f9843cf --- /dev/null +++ b/e2e/unavailable-filters.e2e.ts @@ -0,0 +1,23 @@ +// A filter choice that has no result is shown as unavailable, so the visitor does not have +// to find out by trying. In the seeded catalog the group "4" of balance beam only has the +// difficulties A, B and C. + +import { expect, test } from '@playwright/test' +import { openMenu, resultsLoaded } from './helpers.ts' + +test('a difficulty that does not exist in the chosen group is disabled, and says why', async ({ + page, +}) => { + await page.goto('elements?apparatus=BB') + await resultsLoaded(page) + + await openMenu(page, /^Element group/) + await page.getByRole('menuitemradio').nth(4).click() // 0 is "All groups" + await openMenu(page, /^Difficulty/) + + await expect( + page.getByRole('menuitemcheckbox', { name: /^C$/ }), + ).toBeEnabled() + const h = page.getByRole('menuitemcheckbox', { name: /^H \(no elements/ }) + await expect(h).toBeDisabled() +}) diff --git a/src/components/elements/FilterMenu.tsx b/src/components/elements/FilterMenu.tsx index c170ce0..54c7589 100644 --- a/src/components/elements/FilterMenu.tsx +++ b/src/components/elements/FilterMenu.tsx @@ -5,12 +5,13 @@ import { Check, ChevronDown } from 'lucide-react' import { Button } from '@/components/ui/button' const ITEM = - 'flex min-h-10 cursor-default items-center gap-2 rounded-md px-3 text-sm font-medium text-(--text-color) outline-none select-none data-[highlighted]:bg-(--button-hover-bg)' + 'flex min-h-10 cursor-default items-center gap-2 rounded-md px-3 text-sm font-medium text-(--text-color) outline-none select-none data-[highlighted]:bg-(--button-hover-bg) data-[disabled]:opacity-50' const POPUP = 'z-50 max-h-(--available-height) min-w-48 max-w-[min(32rem,90vw)] overflow-y-auto rounded-lg bg-(--surface) p-1 shadow-(--box-shadow) outline-none' -type Option = { value: string; label: string } +/** `unavailable` is why a choice has no result now: it is shown as disabled, with the reason. */ +type Option = { value: string; label: string; unavailable?: string } type Props = { /** The name of the filter, also the name of the menu. */ @@ -137,6 +138,13 @@ const Indicator = ({ children }: { children: ReactNode }) => ( {children} ) +/** + * Why a choice is disabled, for a screen reader (the item is also dimmed, which is not the only + * sign: WCAG 1.4.1). It is part of the name of the item, so it is read with it. + */ +const Reason = ({ text }: { text?: string }) => + text ? ({text}) : null + /** One choice at a time. "All" (the empty value) takes the filter away. */ export function SingleFilterMenu({ value, @@ -155,6 +163,7 @@ export function SingleFilterMenu({ @@ -164,6 +173,7 @@ export function SingleFilterMenu({ {option.label} + ))} @@ -188,6 +198,9 @@ export function MultiFilterMenu({ key={option.value} checked={values.includes(option.value)} onCheckedChange={(checked) => onToggle(option.value, checked)} + disabled={ + Boolean(option.unavailable) && !values.includes(option.value) + } className={ITEM} > @@ -196,6 +209,7 @@ export function MultiFilterMenu({ {option.label} + ))} diff --git a/src/lib/elementFilters.ts b/src/lib/elementFilters.ts index 208193e..bd20e1c 100644 --- a/src/lib/elementFilters.ts +++ b/src/lib/elementFilters.ts @@ -13,7 +13,13 @@ import { type DifficultyLetter, } from '@/lib/difficulty' -export type StructureGroup = { id: string; number: number; name: string } +export type StructureGroup = { + id: string + number: number + name: string + /** The difficulty values that exist in the group, lowest first. */ + difficultyValues: number[] +} export type StructureApparatus = { id: string name: string @@ -39,6 +45,7 @@ export const STRUCTURE_QUERY: TypedDocumentNode<{ id number name + difficultyValues } } } @@ -98,6 +105,33 @@ export function difficultyLabel(apparatusId: string | null, value: number) { const difficultyParam = (apparatusId: string | null, value: number) => apparatusId === VAULT_ID ? String(value) : difficultyLabel(null, value) +const tenth = (value: number) => Math.round(value * 10) + +/** + * Whether choosing this group together with one of these difficulties can have a result. True + * when no difficulty is chosen. Vault is left out (its difficulty is a band of D-values, not a + * value of its own), so it is always true there. + */ +export function groupHasDifficulty( + group: StructureGroup, + difficulty: number[], + apparatusId: string | null, +): boolean { + if (!difficulty.length || apparatusId === VAULT_ID) return true + const wanted = difficulty.map(tenth) + return group.difficultyValues.some((value) => wanted.includes(tenth(value))) +} + +/** Whether a difficulty exists in the chosen group (true when no group is chosen, and for vault). */ +export function difficultyInGroup( + group: StructureGroup | undefined, + value: number, + apparatusId: string | null, +): boolean { + if (!group || apparatusId === VAULT_ID) return true + return group.difficultyValues.some((v) => tenth(v) === tenth(value)) +} + /** * What the Difficulty menu offers. For vault the bands that have a D-value in them, in * ascending order. For everything else the letters A-J (also with no apparatus chosen). diff --git a/src/pages/ElementsPage.tsx b/src/pages/ElementsPage.tsx index 4add436..8e3713e 100644 --- a/src/pages/ElementsPage.tsx +++ b/src/pages/ElementsPage.tsx @@ -19,8 +19,10 @@ import { Button } from '@/components/ui/button' import { describeError } from '@/lib/apollo/errors' import { asksForFilters, + difficultyInGroup, difficultyLabel, difficultyOptions, + groupHasDifficulty, groupName, hasFilters, NO_FILTERS, @@ -280,6 +282,13 @@ export default function ElementsPage() { options={(apparatus?.groups ?? []).map((g) => ({ value: g.id, label: groupName(apparatus!.id, g), + unavailable: groupHasDifficulty( + g, + filters.difficulty, + filters.apparatusId, + ) + ? undefined + : 'no elements at the chosen difficulty', }))} unavailable={ apparatus ? undefined : 'Choose an apparatus to choose a group.' @@ -298,6 +307,13 @@ export default function ElementsPage() { options={difficultyOptions(apparatus).map((o) => ({ value: String(o.value), label: o.label, + unavailable: difficultyInGroup( + group, + o.value, + filters.apparatusId, + ) + ? undefined + : 'no elements in the chosen group', }))} className={PILL} onToggle={(value, checked) => diff --git a/src/tests/lib/elementFilters.test.ts b/src/tests/lib/elementFilters.test.ts new file mode 100644 index 0000000..37857a8 --- /dev/null +++ b/src/tests/lib/elementFilters.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' +import { + difficultyInGroup, + groupHasDifficulty, + type StructureGroup, +} from '@/lib/elementFilters' + +const group: StructureGroup = { + id: 'BB-4', + number: 4, + name: 'Turns', + difficultyValues: [0.1, 0.2, 0.3], +} + +describe('a filter that has no result is known before it is made', () => { + it('says that a group has a difficulty only when it has an element at it', () => { + expect(difficultyInGroup(group, 0.2, 'BB')).toBe(true) + expect(difficultyInGroup(group, 0.8, 'BB')).toBe(false) + }) + + it('compares the values as tenths, so 0.1 + 0.2 and 0.3 are the same', () => { + expect(difficultyInGroup(group, 0.1 + 0.2, 'BB')).toBe(true) + }) + + it('says that a group can be chosen when no difficulty is chosen, or when one of them is in it', () => { + expect(groupHasDifficulty(group, [], 'BB')).toBe(true) + expect(groupHasDifficulty(group, [0.8, 0.3], 'BB')).toBe(true) + expect(groupHasDifficulty(group, [0.8, 0.9], 'BB')).toBe(false) + }) + + it('does not decide for vault (its difficulty is a band) or for no group', () => { + expect(difficultyInGroup(group, 0.8, 'VT')).toBe(true) + expect(groupHasDifficulty(group, [0.8], 'VT')).toBe(true) + expect(difficultyInGroup(undefined, 0.8, 'BB')).toBe(true) + }) +}) diff --git a/src/tests/pages/ElementsPage.test.tsx b/src/tests/pages/ElementsPage.test.tsx index c75b6df..d78482e 100644 --- a/src/tests/pages/ElementsPage.test.tsx +++ b/src/tests/pages/ElementsPage.test.tsx @@ -140,12 +140,14 @@ const structure = [ id: 'VT-1', number: 1, name: 'Long vault title', + difficultyValues: [1.6, 2], }, { __typename: 'ElementGroup', id: 'VT-2', number: 2, name: 'Another long title', + difficultyValues: [2.4, 3.2], }, ], }, @@ -155,8 +157,21 @@ const structure = [ name: 'Uneven bars', difficultyValues: [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1], groups: [ - { __typename: 'ElementGroup', id: 'UB-1', number: 1, name: 'Mounts' }, - { __typename: 'ElementGroup', id: 'UB-2', number: 2, name: 'Dismounts' }, + { + __typename: 'ElementGroup', + id: 'UB-1', + number: 1, + name: 'Mounts', + difficultyValues: [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1], + }, + { + __typename: 'ElementGroup', + id: 'UB-2', + number: 2, + name: 'Dismounts', + // Short on purpose: the difficulties above C do not exist in this group. + difficultyValues: [0.1, 0.2, 0.3], + }, ], }, ]