diff --git a/docs/superpowers/plans/2026-09-03-paginate-catalogue.md b/docs/superpowers/plans/2026-09-03-paginate-catalogue.md new file mode 100644 index 0000000..cb4eb61 --- /dev/null +++ b/docs/superpowers/plans/2026-09-03-paginate-catalogue.md @@ -0,0 +1,753 @@ +# Catalogue Pagination Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Page the storefront catalogue instead of rendering every item, with a visible total and a page size the customer chooses. + +**Architecture:** Client-side paging only. `useCatalogue` keeps returning every matching item; the page number lives in the URL beside the existing filters, and the page size is a per-viewer preference in `localStorage`. Ant Design's `Pagination` renders the controls and the total. Pure helpers hold every decision worth testing. + +**Tech Stack:** React + TypeScript, antd (`antd/es/...` deep imports), React Router `useSearchParams`, Vitest (node environment), Playwright. + +**Spec:** issue #269 and its decisions comment — https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/269#issuecomment-2611. There is no separate spec document; the issue is the spec. + +## Global Constraints + +- **Numbered pages using Ant Design's `Pagination`.** Not "load more", not infinite scroll. +- **No "Go to … page" jump box.** `showQuickJumper` stays off. +- **Page size defaults to 10**, and the customer may choose **10, 20, 50 or 100**. +- **The total item count is shown.** +- **The page size does not change with viewport.** The issue asked; the decision was a fixed default of 10 plus a chooser, and nothing about screen size. One size on every device is the simpler thing and it is what the customer's own choice then overrides — a size that moved on rotation would fight the preference they just set. The grid itself stays responsive, as it already is (`xs={24} sm={12} md={8} lg={6}`); only how many items make a page is fixed. +- **The API is out of scope.** `GET /api/items` keeps returning everything while the client asks for everything. Do not add `LIMIT`/`OFFSET`, do not touch `backend/`. +- **antd imports are deep and from `es`**: `import Pagination from 'antd/es/pagination';`. Never `import { Pagination } from 'antd'` — a barrel import breaks tree-shaking. +- **The URL is the single source of truth for view state.** `frontend/src/App.tsx:159` already says so for filters; the page number joins them there. The page size does **not** — see Task 2's reasoning. +- Vitest runs with `environment: 'node'`, includes only `tests/unit/**/*.test.ts`, and the project has **no jsdom and no testing-library**. Only pure functions are unit-testable. Do not add those dependencies; React wiring is covered by Playwright. +- **Verify with `npm run build`, never a bare `npx tsc --noEmit`** — the app tsconfig excludes `tests/`, and a green bare `tsc` once broke a deploy here. +- **Branch:** `feature/269-paginate-catalogue`, already created off `main`. Commit there. Subjects end `(#269)`. Commit bodies are **not hard-wrapped** — one long line per paragraph, blank lines between. Do not push; the user pushes. +- End every commit message with: + `Co-Authored-By: Claude Opus 5 ` + +--- + +### Task 1: The pure pagination helpers + +**Files:** +- Create: `frontend/src/pagination.ts` +- Test: `frontend/tests/unit/pagination.test.ts` (create) + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `DEFAULT_PAGE_SIZE: 10` + - `PAGE_SIZE_OPTIONS: readonly [10, 20, 50, 100]` + - `readPageSize(raw: string | null): number` — a stored or supplied value, or the default + - `pageFromSearchParams(params: URLSearchParams): number` + - `clampPage(page: number, total: number, pageSize: number): number` + - `pageSlice(items: readonly T[], page: number, pageSize: number): T[]` + - `readStoredPageSize(storage: Pick | null): number` + - `writeStoredPageSize(storage: Pick | null, size: number): void` + - `PAGE_SIZE_STORAGE_KEY: 'catalogue:pageSize'` + +- [ ] **Step 1: Write the failing test** + +Create `frontend/tests/unit/pagination.test.ts`: + +```typescript +import { describe, it, expect } from 'vitest'; +import { + DEFAULT_PAGE_SIZE, + PAGE_SIZE_OPTIONS, + PAGE_SIZE_STORAGE_KEY, + clampPage, + pageFromSearchParams, + pageSlice, + readPageSize, + readStoredPageSize, + writeStoredPageSize +} from '../../src/pagination'; + +describe('the page size a customer may choose', () => { + it('defaults to 10, the conservative choice', () => { + expect(DEFAULT_PAGE_SIZE).toBe(10); + }); + + it('offers exactly the four sizes the issue settled on', () => { + expect([...PAGE_SIZE_OPTIONS]).toEqual([10, 20, 50, 100]); + }); + + it('accepts a size that is on the list', () => { + expect(readPageSize('50')).toBe(50); + }); + + // Anything else falls back rather than being honoured. A hand-edited value + // of 5000 would render the whole catalogue, which is the thing this issue + // exists to stop. + it('falls back to the default for a size that is not on the list', () => { + expect(readPageSize('5000')).toBe(DEFAULT_PAGE_SIZE); + expect(readPageSize('7')).toBe(DEFAULT_PAGE_SIZE); + }); + + it('falls back to the default for nonsense and for nothing at all', () => { + expect(readPageSize('abc')).toBe(DEFAULT_PAGE_SIZE); + expect(readPageSize('')).toBe(DEFAULT_PAGE_SIZE); + expect(readPageSize(null)).toBe(DEFAULT_PAGE_SIZE); + }); +}); + +describe('the page number in the URL', () => { + it('is 1 when the parameter is absent', () => { + expect(pageFromSearchParams(new URLSearchParams())).toBe(1); + }); + + it('reads a sensible page', () => { + expect(pageFromSearchParams(new URLSearchParams('page=3'))).toBe(3); + }); + + // A mangled link falls back to the first page rather than to an empty view, + // the same rule filtersFromSearchParams follows for a mangled filter. + it('falls back to 1 for zero, negatives and nonsense', () => { + expect(pageFromSearchParams(new URLSearchParams('page=0'))).toBe(1); + expect(pageFromSearchParams(new URLSearchParams('page=-4'))).toBe(1); + expect(pageFromSearchParams(new URLSearchParams('page=abc'))).toBe(1); + expect(pageFromSearchParams(new URLSearchParams('page=2.5'))).toBe(1); + }); +}); + +describe('keeping a page number inside the result set', () => { + // The case that matters: someone shares a link to page 7, the catalogue + // shrinks, and the link must show the last page rather than nothing at all. + it('clamps a page past the end back to the last page', () => { + expect(clampPage(7, 25, 10)).toBe(3); + }); + + it('leaves a page inside the range alone', () => { + expect(clampPage(2, 25, 10)).toBe(2); + }); + + // An empty result set still has a first page to show the empty state on. + it('is 1 when there is nothing to show', () => { + expect(clampPage(3, 0, 10)).toBe(1); + }); + + it('is 1 when everything fits on one page', () => { + expect(clampPage(4, 6, 10)).toBe(1); + }); +}); + +describe('taking one page out of the items', () => { + const items = Array.from({ length: 25 }, (_, i) => i + 1); + + it('takes the first page', () => { + expect(pageSlice(items, 1, 10)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + }); + + it('takes a middle page', () => { + expect(pageSlice(items, 2, 10)).toEqual([11, 12, 13, 14, 15, 16, 17, 18, 19, 20]); + }); + + it('takes a short last page', () => { + expect(pageSlice(items, 3, 10)).toEqual([21, 22, 23, 24, 25]); + }); + + it('is empty past the end rather than throwing', () => { + expect(pageSlice(items, 9, 10)).toEqual([]); + }); +}); + +/** A Storage that is present and works. */ +function fakeStorage(initial: Record = {}) { + const data = { ...initial }; + return { + getItem: (key: string) => data[key] ?? null, + setItem: (key: string, value: string) => { + data[key] = value; + }, + read: () => data + }; +} + +describe('remembering the choice', () => { + it('reads a stored size', () => { + expect(readStoredPageSize(fakeStorage({ [PAGE_SIZE_STORAGE_KEY]: '20' }))).toBe(20); + }); + + it('defaults when nothing was stored', () => { + expect(readStoredPageSize(fakeStorage())).toBe(DEFAULT_PAGE_SIZE); + }); + + it('writes the choice under the agreed key', () => { + const storage = fakeStorage(); + writeStoredPageSize(storage, 50); + expect(storage.read()[PAGE_SIZE_STORAGE_KEY]).toBe('50'); + }); + + // Storage is absent during server rendering and throws outright in some + // privacy modes. Neither may take the catalogue down over a preference. + it('defaults rather than throwing when storage is missing', () => { + expect(readStoredPageSize(null)).toBe(DEFAULT_PAGE_SIZE); + }); + + it('defaults rather than throwing when storage refuses to be read', () => { + const hostile = { + getItem: () => { + throw new Error('access denied'); + } + }; + expect(readStoredPageSize(hostile)).toBe(DEFAULT_PAGE_SIZE); + }); + + it('gives up quietly when storage refuses to be written', () => { + const hostile = { + setItem: () => { + throw new Error('quota exceeded'); + } + }; + expect(() => writeStoredPageSize(hostile, 20)).not.toThrow(); + }); + + it('does nothing when there is no storage to write to', () => { + expect(() => writeStoredPageSize(null, 20)).not.toThrow(); + }); +}); +``` + +- [ ] **Step 2: Run it to verify it fails** + +```bash +cd frontend && npx vitest run tests/unit/pagination.test.ts +``` + +Expected: FAIL — cannot resolve `../../src/pagination`. + +- [ ] **Step 3: Write the implementation** + +Create `frontend/src/pagination.ts`: + +```typescript +/** + * Paging the catalogue: every decision worth testing, as pure functions. + * + * Pure because that is the only thing this project can unit-test — vitest runs + * in a node environment with no jsdom and no testing-library, so a hook or a + * component can only be exercised through Playwright. Keeping the rules here + * means the rules have tests and the React wrapper is thin enough not to need + * any. Same split `filters.ts` already uses for the URL. + * + * Paging is client-side. `GET /api/items` still returns everything and the + * client still asks for everything; moving it server-side is a separate change + * and should be measured rather than assumed, since the filter endpoints + * already do work per request. See #269. + */ + +/** + * Ten, because it is the conservative default and someone who wants a denser + * grid can say so once and be remembered. + */ +export const DEFAULT_PAGE_SIZE = 10; + +/** The sizes offered in the control. A value outside this list is not honoured. */ +export const PAGE_SIZE_OPTIONS = [10, 20, 50, 100] as const; + +export const PAGE_SIZE_STORAGE_KEY = 'catalogue:pageSize'; + +/** + * A page size, from whatever was supplied. + * + * Checked against the list rather than merely parsed as a number. A stored or + * hand-edited 5000 would render the entire catalogue in one page, which is the + * exact failure #269 exists to prevent, so an unrecognised size is refused + * rather than clamped — clamping would quietly honour a value nobody offered. + */ +export function readPageSize(raw: string | null): number { + const parsed = Number(raw); + const allowed = (PAGE_SIZE_OPTIONS as readonly number[]).includes(parsed); + return raw !== null && raw !== '' && allowed ? parsed : DEFAULT_PAGE_SIZE; +} + +/** + * The page a URL asks for. + * + * Falls back to the first page for anything unreadable, which is the rule + * `filtersFromSearchParams` already applies to a mangled filter: a bad link + * lands somewhere sensible rather than on an error. + */ +export function pageFromSearchParams(params: URLSearchParams): number { + const parsed = Number(params.get('page')); + return Number.isSafeInteger(parsed) && parsed >= 1 ? parsed : 1; +} + +/** + * The page actually shown, given how much there is to show. + * + * A shared link to page 7 of a catalogue that has since shrunk to two pages + * shows the last page rather than an empty grid — an empty page would read as + * "this shop has nothing", which is the confusion the empty state exists to + * avoid. + */ +export function clampPage(page: number, total: number, pageSize: number): number { + const lastPage = Math.max(1, Math.ceil(total / pageSize)); + return Math.min(Math.max(1, page), lastPage); +} + +/** One page of items. Past the end this is empty rather than an error. */ +export function pageSlice(items: readonly T[], page: number, pageSize: number): T[] { + const start = (page - 1) * pageSize; + return items.slice(start, start + pageSize); +} + +/** + * The remembered page size. + * + * Every access is guarded. `localStorage` is absent when there is no window at + * all and throws outright in some privacy modes, and neither is a reason for a + * customer to lose the catalogue — the worst acceptable outcome of a broken + * preference is the default. + */ +export function readStoredPageSize(storage: Pick | null): number { + if (storage === null) return DEFAULT_PAGE_SIZE; + try { + return readPageSize(storage.getItem(PAGE_SIZE_STORAGE_KEY)); + } catch { + return DEFAULT_PAGE_SIZE; + } +} + +/** Remembers a choice, or quietly does not. See `readStoredPageSize`. */ +export function writeStoredPageSize( + storage: Pick | null, + size: number +): void { + if (storage === null) return; + try { + storage.setItem(PAGE_SIZE_STORAGE_KEY, String(size)); + } catch { + // A preference that cannot be remembered is not worth an error. The + // customer's current page size still works for this visit. + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +cd frontend && npx vitest run tests/unit/pagination.test.ts +``` + +Expected: PASS, 23 tests. + +- [ ] **Step 5: Commit** + +```bash +git add frontend/src/pagination.ts frontend/tests/unit/pagination.test.ts +git commit -F- <<'EOF' +feat(storefront): the rules for paging the catalogue (#269) + +Every decision paging needs, as pure functions: which page a URL is asking for, which page is actually showable given how much there is, which slice of the items that is, and what page size to use. Pure because that is the only thing this project can unit-test — vitest runs in a node environment with no jsdom and no testing-library, so a hook or a component is only reachable through Playwright. Keeping the rules here means the rules have tests and the React wrapper stays thin enough not to need any, which is the same split filters.ts already uses for the URL. + +An unrecognised page size is refused rather than clamped. A stored or hand-edited 5000 would render the entire catalogue in one page, which is the exact failure this issue exists to prevent, and clamping would quietly honour a value nobody offered. Storage access is guarded on both sides because localStorage is absent when there is no window and throws outright in some privacy modes, and neither is a reason for a customer to lose the catalogue — the worst acceptable outcome of a broken preference is the default. + +Co-Authored-By: Claude Opus 5 +EOF +``` + +--- + +### Task 2: Paging the catalogue + +**Files:** +- Modify: `frontend/src/App.tsx` (the `Catalogue` component around lines 40–108, and `App` around lines 149–200) +- Test: covered by Task 3's Playwright cases; nothing unit-testable is added here (no jsdom). + +**Interfaces:** +- Consumes: everything Task 1 produced. +- Produces: a `page` search parameter on `/`; the catalogue grid showing one page; a `Pagination` control carrying the total and the page-size chooser. + +- [ ] **Step 1: Add the page-size preference to `App`** + +In `frontend/src/App.tsx`, add the imports beside the existing ones: + +```typescript +import Pagination from 'antd/es/pagination'; +import { + DEFAULT_PAGE_SIZE, + PAGE_SIZE_OPTIONS, + clampPage, + pageFromSearchParams, + pageSlice, + readStoredPageSize, + writeStoredPageSize +} from './pagination'; +``` + +`useState` and `useCallback` are already imported at line 1; `useEffect` is not — add it to that import only if you end up needing it, and prefer not to. + +Inside `App`, beside the other state: + +```typescript + /** + * The page size is a preference, not view state, so it lives in storage + * rather than in the URL. Putting it in the URL would mean sharing a link to + * an item also imposed your page size on whoever opened it, which is not + * yours to decide for them. It is read once with a lazy initialiser so a + * throwing storage cannot be hit on every render. + */ + const [pageSize, setPageSize] = useState(() => + readStoredPageSize(typeof window === 'undefined' ? null : window.localStorage) + ); + + const choosePageSize = useCallback((size: number) => { + setPageSize(size); + writeStoredPageSize(typeof window === 'undefined' ? null : window.localStorage, size); + }, []); +``` + +- [ ] **Step 2: Derive the current page and the visible items** + +Still inside `App`, after the existing `const { items, loading, failed, ... } = useCatalogue(...)` call: + +```typescript + // Clamped against what there actually is, so a shared link to a page that no + // longer exists shows the last page rather than an empty grid. + const page = clampPage(pageFromSearchParams(searchParams), items.length, pageSize); + const visibleItems = useMemo(() => pageSlice(items, page, pageSize), [items, page, pageSize]); + + const goToPage = useCallback( + (next: number) => { + const params = new URLSearchParams(searchParams); + // Page one is the absence of the parameter, so the plain catalogue URL + // stays clean and two links to the same first page are the same string. + if (next <= 1) params.delete('page'); + else params.set('page', String(next)); + // push, not replace: paging is navigation, and the back button should + // return to the page you came from. Filters use replace for the opposite + // reason — dragging a slider must not bury the previous view. + setSearchParams(params); + }, + [searchParams, setSearchParams] + ); +``` + +`useMemo` is already imported at line 1. + +**Note the free behaviour here, and do not break it:** `applyFilters` calls `setSearchParams(filtersToSearchParams(next))`, and `filtersToSearchParams` builds a **fresh** `URLSearchParams` (`frontend/src/filters.ts:97-108`). So changing any filter drops `page` and returns to page one, which is what should happen — landing on page 7 of a two-page result is the bug that behaviour prevents. `goToPage` above copies the existing params precisely so that paging keeps the filters, while filtering discards the page. + +- [ ] **Step 3: Pass the page through to `Catalogue`** + +Extend `CatalogueProps` (around line 40): + +```typescript +type CatalogueProps = Readonly<{ + failed: boolean; + loading: boolean; + items: Item[]; + total: number; + page: number; + pageSize: number; + onPageChange: (page: number) => void; + onPageSizeChange: (size: number) => void; + filtered: boolean; + needsFavoritesAuth: boolean; + onRetry: () => void; + onSignIn: () => void; + onClearFilters: () => void; + onChanged: () => void; +}>; +``` + +`items` is now the visible page and `total` is how many there are altogether. Update the destructuring in the function signature to match, adding `total`, `page`, `pageSize`, `onPageChange` and `onPageSizeChange`. + +At the `` call site in `App`'s JSX, pass: + +```tsx + items={visibleItems} + total={items.length} + page={page} + pageSize={pageSize} + onPageChange={goToPage} + onPageSizeChange={choosePageSize} +``` + +leaving the other props as they are. **`items` must become `visibleItems`** — leaving it as `items` renders the whole catalogue on every page and the feature silently does nothing. + +- [ ] **Step 4: Render the control** + +Replace the `Catalogue` component's final `return` (the `` block around lines 100–108) with: + +```tsx + return ( + <> + + {items.map(item => ( + + + + ))} + + onPageSizeChange(size)} + // Deliberately absent: the jump box earns its place on a table of + // thousands of rows, not on a catalogue somebody is browsing (#269). + showQuickJumper={false} + // Shown even when everything fits on one page, because the count is a + // requirement in its own right and hiding the control would hide it. + hideOnSinglePage={false} + showTotal={(count) => `${count} ${count === 1 ? 'item' : 'items'}`} + /> + + ); +``` + +`Row` and `Col` are already imported. `PAGE_SIZE_OPTIONS` is spread into a mutable array because antd's prop type is not `readonly`. + +The three early returns above it — the load failure, the sign-in prompt and the empty state — are unchanged and must stay unchanged. None of them should carry a pagination control: there is nothing to page. + +- [ ] **Step 5: Verify it builds and the existing tests still pass** + +```bash +cd frontend && npm run build && npm run lint && npm run test:unit +``` + +`npm run build`, not a bare `npx tsc --noEmit` — the app tsconfig excludes `tests/`. + +Expected: clean build, clean lint, unit tests pass. The build is what catches a prop added to `CatalogueProps` but not passed at the call site. + +- [ ] **Step 6: Commit** + +```bash +git add frontend/src/App.tsx +git commit -F- <<'EOF' +feat(storefront): page the catalogue instead of rendering all of it (#269) + +The page number joins the filters in the URL, which is already the single source of truth for what the storefront is showing. That is the whole reason numbered pages were chosen over infinite scroll: a page is a place you can send someone, and a scroll position is not. The page size deliberately does not go there — it is a preference belonging to one person, and putting it in the URL would mean sharing a link to an item also imposed your page size on whoever opened it. + +Changing a filter returns to page one, and it does so for free: filtersToSearchParams builds a fresh URLSearchParams, so applying filters drops the page parameter while paging copies the existing params and keeps the filters. That is the behaviour worth having rather than an accident to tidy up — landing on page seven of a two-page result is exactly the state a customer cannot get out of without understanding the URL. + +Co-Authored-By: Claude Opus 5 +EOF +``` + +--- + +### Task 3: The end-to-end cases, and the two assertions this unblocks + +**Files:** +- Modify: `frontend/tests/e2e/pages/StorefrontPage.ts` +- Modify: `frontend/tests/e2e/filters.spec.ts` (the case at line 156) +- Modify: `frontend/tests/e2e/favorites-filter.spec.ts` (the case at line 92) +- Create: `frontend/tests/e2e/pagination.spec.ts` + +**Interfaces:** +- Consumes: the `Pagination` control from Task 2. +- Produces: `StorefrontPage.resultCount`, `StorefrontPage.totalItems()`, `StorefrontPage.pagination`. + +**Why this task exists.** #269 names two assertions that pagination breaks, and it is right. `filters.spec.ts:165` does `await expect(storefront.card(NAMES.deepItem)).toBeVisible()` against the *unfiltered* catalogue — with ten items to a page over a development database of thousands, that fixture will not be on page one and the test will fail. `favorites-filter.spec.ts:106` has the same shape. Both were only ever proxies for "the result set got bigger", and a visible total lets them say that directly. + +- [ ] **Step 1: Add the locators to the page object** + +In `frontend/tests/e2e/pages/StorefrontPage.ts`, add to the `readonly` declarations near the top: + +```typescript + readonly pagination: Locator; + readonly resultCount: Locator; +``` + +and in the constructor, beside the existing assignments: + +```typescript + this.pagination = page.locator('.ant-pagination'); + // antd renders showTotal's output into this element. A class locator + // rather than a role, matching how this file already reaches .ant-tag and + // .ant-card — antd gives these no role of their own. + this.resultCount = page.locator('.ant-pagination-total-text'); +``` + +Then add a method beside the other helpers: + +```typescript + /** + * How many items the catalogue says it has. + * + * The number rather than the text, so a test can assert a set grew without + * knowing what it grew from — which is the whole reason #269 added a count. + */ + async totalItems(): Promise { + const text = (await this.resultCount.textContent()) ?? ''; + const digits = /(\d+)/.exec(text); + if (digits === null) throw new Error(`no count in the pagination total: "${text}"`); + return Number(digits[1]); + } +``` + +- [ ] **Step 2: Rewrite the two brittle assertions** + +In `frontend/tests/e2e/filters.spec.ts`, replace the body of `'removing a chip widens the results again'` (line 156) with: + +```typescript + // Asserts what it means, rather than that one fixture is on screen. It used + // to check a named item was visible in the unfiltered grid, which a + // paginated catalogue cannot promise — that item is on some page, not + // necessarily the first (#269). + test('removing a chip widens the results again', async ({ storefront, filterDrawer }) => { + await storefront.goto(); + const unfiltered = await storefront.totalItems(); + + await storefront.openFilters(); + await filterDrawer.chooseCategory(NAMES.decor); + await filterDrawer.close(); + + const filtered = await storefront.totalItems(); + expect(filtered).toBeLessThan(unfiltered); + + await storefront.removeFilterChip(NAMES.decor).click(); + + await expect.poll(() => storefront.totalItems()).toBe(unfiltered); + }); +``` + +`expect.poll` rather than a bare read, because the count changes when the refetch lands and a single read races it. + +In `frontend/tests/e2e/favorites-filter.spec.ts`, the case at line 92 asserts the same thing about favourites. Read it in full first, then replace its closing assertion — the one expecting the full catalogue back after removing the chip — with the same shape: capture the unfiltered total before filtering, and `await expect.poll(() => storefront.totalItems()).toBe(unfiltered)` after removing the chip. Keep every other assertion in that test, including the `activeFilters` check and the hidden-button check at line 102, which do not depend on paging. + +- [ ] **Step 3: Write the new pagination cases** + +Create `frontend/tests/e2e/pagination.spec.ts`: + +```typescript +import { test, expect } from './fixtures'; + +/** + * Paging the catalogue (#269). + * + * Every assertion here is about the control and the URL, never about a + * particular item being on a particular page. The dev database never + * truncates, so which item lands where is not something a test may rely on — + * that is the same trap the two rewritten assertions in filters.spec.ts and + * favorites-filter.spec.ts fell into. + */ +test.describe('Paging the catalogue', () => { + test('shows how many items there are', async ({ storefront }) => { + await storefront.goto(); + + await expect(storefront.resultCount).toBeVisible(); + expect(await storefront.totalItems()).toBeGreaterThan(0); + }); + + // Ten by default, which is the decision on the issue. A denser grid is + // something a customer asks for, not something they get by accident. + test('shows ten items to a page by default', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful'); + + await expect(page.locator('.ant-card')).toHaveCount(10); + }); + + // The reason numbered pages were chosen over infinite scroll: a page is a + // place you can send someone. + test('puts the page in the URL so it can be linked', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful'); + + await storefront.pagination.getByRole('listitem', { name: '2' }).click(); + + await expect(page).toHaveURL(/[?&]page=2\b/); + }); + + // Page one is the absence of the parameter, so the plain URL stays clean. + test('leaves page one out of the URL', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful'); + + await storefront.pagination.getByRole('listitem', { name: '2' }).click(); + await expect(page).toHaveURL(/[?&]page=2\b/); + + await storefront.pagination.getByRole('listitem', { name: '1' }).click(); + await expect(page).not.toHaveURL(/[?&]page=/); + }); + + // A link to a page that no longer exists lands on the last one. An empty + // grid would read as "this shop has nothing". + test('clamps a page past the end rather than showing nothing', async ({ page, storefront }) => { + await page.goto('/?page=99999'); + + await expect(storefront.resultCount).toBeVisible(); + await expect(page.locator('.ant-card').first()).toBeVisible(); + }); + + // The jump box was deliberately left off. + test('offers no "go to page" box', async ({ storefront }) => { + await storefront.goto(); + + await expect(storefront.pagination.locator('.ant-pagination-options-quick-jumper')).toHaveCount(0); + }); + + test('remembers a chosen page size', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 20, 'needs more than twenty items for a size change to show'); + + await storefront.pagination.locator('.ant-select').click(); + await page.getByTitle('20 / page').click(); + await expect(page.locator('.ant-card')).toHaveCount(20); + + // The preference is the point: it survives a reload, without being in the + // URL, because it belongs to this person and not to a shared link. + await page.reload(); + await expect(page.locator('.ant-card')).toHaveCount(20); + await expect(page).not.toHaveURL(/pageSize/); + }); +}); +``` + +- [ ] **Step 4: Run the affected e2e specs** + +The local stack must be running. **Do not start it yourself** if you are an agent — `scripts/start-local.ps1` prompts for elevation and must be run by the user. + +```bash +cd frontend && npx playwright test pagination filters favorites-filter --project=chromium +``` + +Expected: PASS. If `pagination.spec.ts` skips most cases, the local catalogue has fewer than eleven items — say so in your report rather than deleting the skips. + +- [ ] **Step 5: Run the whole e2e suite** + +```bash +cd frontend && npx playwright test --project=chromium +``` + +Expected: everything green. This is the step that finds any *other* spec that quietly assumed the whole catalogue was on screen — there may be some, and if there are, they need the same treatment as the two in Step 2. + +- [ ] **Step 6: Commit** + +```bash +git add frontend/tests/e2e/pages/StorefrontPage.ts frontend/tests/e2e/filters.spec.ts frontend/tests/e2e/favorites-filter.spec.ts frontend/tests/e2e/pagination.spec.ts +git commit -F- <<'EOF' +test(storefront): cover paging, and say what the filter tests meant (#269) + +Two assertions named a fixture and expected it visible in the unfiltered grid. No paginated catalogue can promise that — the item is on some page, not necessarily the first — so both would have started failing the moment paging landed. They were only ever proxies for "the result set got bigger", and the visible total lets them say that directly, which is what the issue predicted when it asked for a count. + +The new cases assert the control and the URL rather than which item is on which page. The development database never truncates, so which item lands where is not something a test may rely on — that is the same trap the two rewritten assertions fell into, and repeating it in new tests would be worse than leaving them alone. + +Co-Authored-By: Claude Opus 5 +EOF +``` + +--- + +## After the plan + +- The branch is `feature/269-paginate-catalogue`. **Do not push** — the user pushes and merges. +- The PR closes #269 and should say plainly that paging is client-side, and that moving it into `GET /api/items` remains a separate, measurable change. +- Per standing practice, follow with a separate SonarQube cleanup issue and PR — hotspots, duplication, debt, coverage — never folded into this branch. diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 2785366..f1cfdfc 100755 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -10,6 +10,7 @@ import theme from 'antd/es/theme'; import Badge from 'antd/es/badge'; import Empty from 'antd/es/empty'; import Alert from 'antd/es/alert'; +import Pagination from 'antd/es/pagination'; import { ShoppingCartOutlined } from '@ant-design/icons'; import { Link, useLocation, useSearchParams } from 'react-router-dom'; import { Item } from './api'; @@ -18,6 +19,14 @@ import ItemCard from './components/ItemCard'; import BrandMark from './components/BrandMark'; import FilterBar from './components/filters/FilterBar'; import { chipsFor } from './components/filters/dimension'; +import { + PAGE_SIZE_OPTIONS, + clampPage, + pageFromSearchParams, + pageSlice, + readStoredPageSize, + writeStoredPageSize +} from './pagination'; import { availabilityDimension, categoryDimension, @@ -40,7 +49,14 @@ const { Title } = Typography; type CatalogueProps = Readonly<{ failed: boolean; loading: boolean; + /** One page of items, not the whole result set — see `total`. */ items: Item[]; + /** How many items match the filters altogether, across every page. */ + total: number; + page: number; + pageSize: number; + onPageChange: (page: number) => void; + onPageSizeChange: (size: number) => void; filtered: boolean; needsFavoritesAuth: boolean; onRetry: () => void; @@ -58,6 +74,11 @@ function Catalogue({ failed, loading, items, + total, + page, + pageSize, + onPageChange, + onPageSizeChange, filtered, needsFavoritesAuth, onRetry, @@ -98,13 +119,39 @@ function Catalogue({ } return ( - - {items.map(item => ( - - - - ))} - + <> + + {items.map(item => ( + + + + ))} + + {/* Not while there is nothing to count. A genuinely empty catalogue + returns early above with the empty state, so the only way to reach + here with a total of zero is mid-load — and flashing "0 items" at + somebody while their catalogue is still arriving says something + untrue. */} + {total > 0 && ( + onPageSizeChange(size)} + // Deliberately off: the jump box earns its place on a table of + // thousands of rows, not on a catalogue somebody is browsing (#269). + showQuickJumper={false} + // Shown even when everything fits on one page, because the count is a + // requirement in its own right and hiding the control would hide it. + hideOnSinglePage={false} + showTotal={(count) => `${count} ${count === 1 ? 'item' : 'items'}`} + /> + )} + ); } @@ -163,6 +210,42 @@ export default function App() { const { items, loading, failed, options, needsFavoritesAuth, reload, retry, filterKey } = useCatalogue(filters, openAuthModal); + /** + * The page size is a preference, not view state, so it lives in storage + * rather than in the URL. Putting it in the URL would mean sharing a link to + * an item also imposed your page size on whoever opened it, which is not + * yours to decide for them. Read through a lazy initialiser so a storage + * that throws is not hit on every render. + */ + const [pageSize, setPageSize] = useState(() => + readStoredPageSize(typeof window === 'undefined' ? null : window.localStorage) + ); + + const choosePageSize = useCallback((size: number) => { + setPageSize(size); + writeStoredPageSize(typeof window === 'undefined' ? null : window.localStorage, size); + }, []); + + // Clamped against what there actually is, so a shared link to a page that no + // longer exists shows the last page rather than an empty grid. + const page = clampPage(pageFromSearchParams(searchParams), items.length, pageSize); + const visibleItems = useMemo(() => pageSlice(items, page, pageSize), [items, page, pageSize]); + + const goToPage = useCallback( + (next: number) => { + const params = new URLSearchParams(searchParams); + // Page one is the absence of the parameter, so the plain catalogue URL + // stays clean and two links to the same first page are the same string. + if (next <= 1) params.delete('page'); + else params.set('page', String(next)); + // push, not replace: paging is navigation, and the back button should + // return to the page you came from. Filters use replace for the opposite + // reason — dragging a slider must not bury the previous view. + setSearchParams(params); + }, + [searchParams, setSearchParams] + ); + const applyFilters = useCallback( (next: ItemFilters) => { // replace, not push: dragging a slider shouldn't bury the previous page @@ -264,7 +347,12 @@ export default function App() { = 1 ? parsed : 1; +} + +/** + * The page actually shown, given how much there is to show. + * + * A shared link to page 7 of a catalogue that has since shrunk to two pages + * shows the last page rather than an empty grid — an empty page would read as + * "this shop has nothing", which is the confusion the empty state exists to + * avoid. + */ +export function clampPage(page: number, total: number, pageSize: number): number { + const lastPage = Math.max(1, Math.ceil(total / pageSize)); + return Math.min(Math.max(1, page), lastPage); +} + +/** One page of items. Past the end this is empty rather than an error. */ +export function pageSlice(items: readonly T[], page: number, pageSize: number): T[] { + const start = (page - 1) * pageSize; + return items.slice(start, start + pageSize); +} + +/** + * The remembered page size. + * + * Every access is guarded. `localStorage` is absent when there is no window at + * all and throws outright in some privacy modes, and neither is a reason for a + * customer to lose the catalogue — the worst acceptable outcome of a broken + * preference is the default. + */ +export function readStoredPageSize(storage: Pick | null): number { + if (storage === null) return DEFAULT_PAGE_SIZE; + try { + return readPageSize(storage.getItem(PAGE_SIZE_STORAGE_KEY)); + } catch { + return DEFAULT_PAGE_SIZE; + } +} + +/** Remembers a choice, or quietly does not. See `readStoredPageSize`. */ +export function writeStoredPageSize( + storage: Pick | null, + size: number +): void { + if (storage === null) return; + try { + storage.setItem(PAGE_SIZE_STORAGE_KEY, String(size)); + } catch { + // A preference that cannot be remembered is not worth an error. The + // customer's current page size still works for this visit. + } +} diff --git a/frontend/tests/e2e/favorites-filter.spec.ts b/frontend/tests/e2e/favorites-filter.spec.ts index 45a6040..b3c365d 100644 --- a/frontend/tests/e2e/favorites-filter.spec.ts +++ b/frontend/tests/e2e/favorites-filter.spec.ts @@ -98,13 +98,20 @@ test.describe('Filtering the storefront by favorites', () => { await storefront.gotoSignedIn(); await favorite(storefront, favoritePrompt, KEPT); + // The unfiltered total, captured before filtering, is what "the full + // catalogue" means here. It used to mean "OTHER is on screen", which a + // paginated catalogue cannot promise — OTHER is on some page, not + // necessarily the first (#269). + await storefront.goto(); + const unfiltered = await storefront.totalItems(); + await page.goto('/?favorites=1'); await expect(storefront.addToFavoritesButton(OTHER)).toBeHidden(); await expect(storefront.activeFilters).toContainText('My favorites'); await storefront.removeFilterChip('My favorites').click(); - await expect(storefront.addToFavoritesButton(OTHER)).toBeVisible(); + await expect.poll(() => storefront.totalItems()).toBe(unfiltered); await expect(page).not.toHaveURL(/favorites/); }); diff --git a/frontend/tests/e2e/filters.spec.ts b/frontend/tests/e2e/filters.spec.ts index 55d550a..51528bf 100644 --- a/frontend/tests/e2e/filters.spec.ts +++ b/frontend/tests/e2e/filters.spec.ts @@ -153,16 +153,26 @@ test.describe('Storefront filters', () => { await expect(storefront.card(NAMES.dearItem)).toBeHidden(); }); + // Asserts what it means, rather than that one fixture happens to be on + // screen. It used to check a named item was visible in the unfiltered grid, + // which a paginated catalogue cannot promise — that item is on some page, not + // necessarily the first (#269). test('removing a chip widens the results again', async ({ storefront, filterDrawer }) => { await storefront.goto(); + const unfiltered = await storefront.totalItems(); + await storefront.openFilters(); await filterDrawer.chooseCategory(NAMES.decor); await filterDrawer.close(); await expect(storefront.card(NAMES.deepItem)).toBeHidden(); + await expect.poll(() => storefront.totalItems()).toBeLessThan(unfiltered); await storefront.removeFilterChip(NAMES.decor).click(); - await expect(storefront.card(NAMES.deepItem)).toBeVisible(); + + // Polled rather than read once: the count changes when the refetch lands, + // and a single read races it. + await expect.poll(() => storefront.totalItems()).toBe(unfiltered); }); test('clear all removes every active filter', async ({ page, storefront, filterDrawer }) => { diff --git a/frontend/tests/e2e/pages/StorefrontPage.ts b/frontend/tests/e2e/pages/StorefrontPage.ts index a7b4f7b..70d1558 100644 --- a/frontend/tests/e2e/pages/StorefrontPage.ts +++ b/frontend/tests/e2e/pages/StorefrontPage.ts @@ -38,6 +38,16 @@ export class StorefrontPage { /** What the nearest error boundary renders when the grid itself throws. */ readonly catalogueBoundaryHeading: Locator; + /** The paging control under the grid (#269). */ + readonly pagination: Locator; + /** + * Where the catalogue says how many items it has. + * + * A class locator rather than a role, matching how this file already reaches + * `.item-card` — antd gives the total no role of its own, and this is the one + * place in the suite that knows where it lives. + */ + readonly resultCount: Locator; constructor(private readonly page: Page) { this.header = new Header(page); @@ -52,12 +62,35 @@ export class StorefrontPage { this.retryButton = page.getByRole('button', { name: 'Retry' }); this.catalogueBoundaryHeading = page.getByRole('heading', { name: "The item list didn't load" }); + + this.pagination = page.locator('.ant-pagination'); + this.resultCount = page.locator('.ant-pagination-total-text'); } async goto(): Promise { await this.page.goto('/'); } + /** + * How many items the catalogue says it has, in total, across every page. + * + * The number rather than the text, so a test can assert a result set grew or + * shrank without knowing what it grew from. That is the whole reason #269 + * added a visible count: two assertions used to name a fixture and expect it + * in the unfiltered grid, which no paginated catalogue can promise. + */ + async totalItems(): Promise { + // Waits rather than reading straight away. The control is not rendered + // until there is something to count, so reading during the initial load + // used to return 0 and quietly make "the result set shrank" assertions + // compare against nothing. + await this.resultCount.waitFor(); + const text = (await this.resultCount.textContent()) ?? ''; + const digits = /(\d+)/.exec(text); + if (digits === null) throw new Error(`no count in the pagination total: "${text}"`); + return Number(digits[1]); + } + /** * Goes to the storefront and waits for the session to settle. * diff --git a/frontend/tests/e2e/pagination.spec.ts b/frontend/tests/e2e/pagination.spec.ts new file mode 100644 index 0000000..2d5bea9 --- /dev/null +++ b/frontend/tests/e2e/pagination.spec.ts @@ -0,0 +1,105 @@ +import { test, expect } from './fixtures'; + +/* eslint-disable sonarjs/no-skipped-tests -- + * The `test.skip(total <= 10, ...)` calls below are conditional guards, not + * disabled tests: they run whenever the catalogue is large enough to have a + * second page, which it is on any real database. The rule cannot tell a + * runtime condition from a permanently ignored test, and its own message asks + * for an explanation rather than removal — this is it. + * + * The guards are honest about a real limit, though: against a catalogue of ten + * items or fewer these cases prove nothing. If the e2e database is ever seeded + * that thinly, they should be given fixtures of their own rather than left to + * skip quietly. + */ + +/** + * Paging the catalogue (#269). + * + * Every assertion here is about the control and the URL, never about a + * particular item being on a particular page. The development database never + * truncates, so which item lands where is not something a test may rely on — + * that is exactly the trap the two rewritten assertions in filters.spec.ts and + * favorites-filter.spec.ts had fallen into. + */ +test.describe('Paging the catalogue', () => { + test('shows how many items there are', async ({ storefront }) => { + await storefront.goto(); + + await expect(storefront.resultCount).toBeVisible(); + expect(await storefront.totalItems()).toBeGreaterThan(0); + }); + + // Ten by default, which is the decision on the issue. A denser grid is + // something a customer asks for, not something they get by accident. + test('shows ten items to a page by default', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful'); + + await expect(page.locator('.item-card')).toHaveCount(10); + }); + + // The reason numbered pages were chosen over infinite scroll: a page is a + // place you can send someone. + test('puts the page in the URL so it can be linked', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful'); + + await storefront.pagination.getByRole('listitem', { name: '2', exact: true }).click(); + + await expect(page).toHaveURL(/[?&]page=2\b/); + }); + + // Page one is the absence of the parameter, so the plain URL stays clean. + test('leaves page one out of the URL', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful'); + + await storefront.pagination.getByRole('listitem', { name: '2', exact: true }).click(); + await expect(page).toHaveURL(/[?&]page=2\b/); + + await storefront.pagination.getByRole('listitem', { name: '1', exact: true }).click(); + await expect(page).not.toHaveURL(/[?&]page=/); + }); + + // A link to a page that no longer exists lands on the last one. An empty + // grid would read as "this shop has nothing". + test('clamps a page past the end rather than showing nothing', async ({ page, storefront }) => { + await page.goto('/?page=99999'); + + await expect(storefront.resultCount).toBeVisible(); + await expect(page.locator('.item-card').first()).toBeVisible(); + }); + + // The jump box was deliberately left off. + test('offers no "go to page" box', async ({ storefront }) => { + await storefront.goto(); + + await expect( + storefront.pagination.locator('.ant-pagination-options-quick-jumper') + ).toHaveCount(0); + }); + + test('remembers a chosen page size', async ({ page, storefront }) => { + await storefront.goto(); + + const total = await storefront.totalItems(); + test.skip(total <= 20, 'needs more than twenty items for a size change to show'); + + await storefront.pagination.locator('.ant-select').click(); + await page.getByTitle('20 / page').click(); + await expect(page.locator('.item-card')).toHaveCount(20); + + // The preference is the point: it survives a reload without being in the + // URL, because it belongs to this person and not to a shared link. + await page.reload(); + await expect(page.locator('.item-card')).toHaveCount(20); + await expect(page).not.toHaveURL(/pageSize/); + }); +}); diff --git a/frontend/tests/unit/pagination.test.ts b/frontend/tests/unit/pagination.test.ts new file mode 100644 index 0000000..ebe17e4 --- /dev/null +++ b/frontend/tests/unit/pagination.test.ts @@ -0,0 +1,156 @@ +import { describe, it, expect } from 'vitest'; +import { + DEFAULT_PAGE_SIZE, + PAGE_SIZE_OPTIONS, + PAGE_SIZE_STORAGE_KEY, + clampPage, + pageFromSearchParams, + pageSlice, + readPageSize, + readStoredPageSize, + writeStoredPageSize +} from '../../src/pagination'; + +describe('the page size a customer may choose', () => { + it('defaults to 10, the conservative choice', () => { + expect(DEFAULT_PAGE_SIZE).toBe(10); + }); + + it('offers exactly the four sizes the issue settled on', () => { + expect([...PAGE_SIZE_OPTIONS]).toEqual([10, 20, 50, 100]); + }); + + it('accepts a size that is on the list', () => { + expect(readPageSize('50')).toBe(50); + }); + + // Anything else falls back rather than being honoured. A hand-edited value + // of 5000 would render the whole catalogue, which is the thing this issue + // exists to stop. + it('falls back to the default for a size that is not on the list', () => { + expect(readPageSize('5000')).toBe(DEFAULT_PAGE_SIZE); + expect(readPageSize('7')).toBe(DEFAULT_PAGE_SIZE); + }); + + it('falls back to the default for nonsense and for nothing at all', () => { + expect(readPageSize('abc')).toBe(DEFAULT_PAGE_SIZE); + expect(readPageSize('')).toBe(DEFAULT_PAGE_SIZE); + expect(readPageSize(null)).toBe(DEFAULT_PAGE_SIZE); + }); +}); + +describe('the page number in the URL', () => { + it('is 1 when the parameter is absent', () => { + expect(pageFromSearchParams(new URLSearchParams())).toBe(1); + }); + + it('reads a sensible page', () => { + expect(pageFromSearchParams(new URLSearchParams('page=3'))).toBe(3); + }); + + // A mangled link falls back to the first page rather than to an empty view, + // the same rule filtersFromSearchParams follows for a mangled filter. + it('falls back to 1 for zero, negatives and nonsense', () => { + expect(pageFromSearchParams(new URLSearchParams('page=0'))).toBe(1); + expect(pageFromSearchParams(new URLSearchParams('page=-4'))).toBe(1); + expect(pageFromSearchParams(new URLSearchParams('page=abc'))).toBe(1); + expect(pageFromSearchParams(new URLSearchParams('page=2.5'))).toBe(1); + }); +}); + +describe('keeping a page number inside the result set', () => { + // The case that matters: someone shares a link to page 7, the catalogue + // shrinks, and the link must show the last page rather than nothing at all. + it('clamps a page past the end back to the last page', () => { + expect(clampPage(7, 25, 10)).toBe(3); + }); + + it('leaves a page inside the range alone', () => { + expect(clampPage(2, 25, 10)).toBe(2); + }); + + // An empty result set still has a first page to show the empty state on. + it('is 1 when there is nothing to show', () => { + expect(clampPage(3, 0, 10)).toBe(1); + }); + + it('is 1 when everything fits on one page', () => { + expect(clampPage(4, 6, 10)).toBe(1); + }); +}); + +describe('taking one page out of the items', () => { + const items = Array.from({ length: 25 }, (_, i) => i + 1); + + it('takes the first page', () => { + expect(pageSlice(items, 1, 10)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + }); + + it('takes a middle page', () => { + expect(pageSlice(items, 2, 10)).toEqual([11, 12, 13, 14, 15, 16, 17, 18, 19, 20]); + }); + + it('takes a short last page', () => { + expect(pageSlice(items, 3, 10)).toEqual([21, 22, 23, 24, 25]); + }); + + it('is empty past the end rather than throwing', () => { + expect(pageSlice(items, 9, 10)).toEqual([]); + }); +}); + +/** A Storage that is present and works. */ +function fakeStorage(initial: Record = {}) { + const data = { ...initial }; + return { + getItem: (key: string) => data[key] ?? null, + setItem: (key: string, value: string) => { + data[key] = value; + }, + read: () => data + }; +} + +describe('remembering the choice', () => { + it('reads a stored size', () => { + expect(readStoredPageSize(fakeStorage({ [PAGE_SIZE_STORAGE_KEY]: '20' }))).toBe(20); + }); + + it('defaults when nothing was stored', () => { + expect(readStoredPageSize(fakeStorage())).toBe(DEFAULT_PAGE_SIZE); + }); + + it('writes the choice under the agreed key', () => { + const storage = fakeStorage(); + writeStoredPageSize(storage, 50); + expect(storage.read()[PAGE_SIZE_STORAGE_KEY]).toBe('50'); + }); + + // Storage is absent during server rendering and throws outright in some + // privacy modes. Neither may take the catalogue down over a preference. + it('defaults rather than throwing when storage is missing', () => { + expect(readStoredPageSize(null)).toBe(DEFAULT_PAGE_SIZE); + }); + + it('defaults rather than throwing when storage refuses to be read', () => { + const hostile = { + getItem: () => { + throw new Error('access denied'); + } + }; + expect(readStoredPageSize(hostile)).toBe(DEFAULT_PAGE_SIZE); + }); + + it('gives up quietly when storage refuses to be written', () => { + const hostile = { + setItem: () => { + throw new Error('quota exceeded'); + } + }; + expect(() => writeStoredPageSize(hostile, 20)).not.toThrow(); + }); + + it('does nothing when there is no storage to write to', () => { + expect(() => writeStoredPageSize(null, 20)).not.toThrow(); + }); +});