From 3311dc29ae0e615345157b381bc0c48412ad7b36 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 08:34:01 +0200 Subject: [PATCH] feat(documents): paginate search with a Pagination control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frontend side of the /documents pagination work. The page.server.ts load reads ?page= from the URL, forwards page+size=50 to the backend, and exposes the new totalElements/pageNumber/pageSize/totalPages fields on `data`. +page.svelte renders a component below the result list; buildPageHref preserves every filter param and only updates page. The existing triggerSearch debounce flow intentionally drops `page` when any filter changes, so filter edits reset to page 0 automatically. uses plain links (not goto) so SvelteKit's default scroll restoration scrolls new pages to the top — the expected senior-UX behaviour. Decorative chevrons wrapped in aria-hidden spans, 44px touch targets, focus-visible ring, stacks vertically under 640px. The control hides itself when totalPages ≤ 1. Test coverage: 9 cases on Pagination (label, aria-current, prev/next enable/disable, makeHref invocation, decorative chevron, touch target), plus a filter-reset assertion on +page.svelte (page 5 → edit q → goto URL must drop page=). Adds i18n keys in de/en/es. Manual edit to api.ts pending a post-merge npm run generate:api against a rebuilt dev backend. (#315) Co-Authored-By: Claude Opus 4.7 --- frontend/messages/de.json | 6 +- frontend/messages/en.json | 6 +- frontend/messages/es.json | 6 +- frontend/src/lib/components/Pagination.svelte | 58 +++++++++++++ .../lib/components/Pagination.svelte.spec.ts | 84 +++++++++++++++++++ frontend/src/lib/generated/api.ts | 18 +++- frontend/src/routes/documents/+page.server.ts | 17 +++- frontend/src/routes/documents/+page.svelte | 34 +++++++- .../src/routes/documents/page.server.spec.ts | 12 +-- .../src/routes/documents/page.svelte.spec.ts | 16 ++++ 10 files changed, 243 insertions(+), 14 deletions(-) create mode 100644 frontend/src/lib/components/Pagination.svelte create mode 100644 frontend/src/lib/components/Pagination.svelte.spec.ts diff --git a/frontend/messages/de.json b/frontend/messages/de.json index f74c749a..5e56ed54 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -806,5 +806,9 @@ "chronik_load_more": "Mehr laden", "chronik_loading": "Lädt …", "chronik_load_more_announcement": "{count} weitere Einträge geladen", - "chronik_view_all": "Alle Aktivitäten →" + "chronik_view_all": "Alle Aktivitäten →", + "pagination_prev": "Zurück", + "pagination_next": "Weiter", + "pagination_page_of": "Seite {page} von {total}", + "pagination_nav_label": "Seitennavigation" } diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 0ff58ce2..5e2591ef 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -806,5 +806,9 @@ "chronik_load_more": "Load more", "chronik_loading": "Loading …", "chronik_load_more_announcement": "{count} more entries loaded", - "chronik_view_all": "All activity →" + "chronik_view_all": "All activity →", + "pagination_prev": "Previous", + "pagination_next": "Next", + "pagination_page_of": "Page {page} of {total}", + "pagination_nav_label": "Pagination" } diff --git a/frontend/messages/es.json b/frontend/messages/es.json index f968bbde..b64d0405 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -806,5 +806,9 @@ "chronik_load_more": "Cargar más", "chronik_loading": "Cargando …", "chronik_load_more_announcement": "{count} entradas más cargadas", - "chronik_view_all": "Todas las actividades →" + "chronik_view_all": "Todas las actividades →", + "pagination_prev": "Anterior", + "pagination_next": "Siguiente", + "pagination_page_of": "Página {page} de {total}", + "pagination_nav_label": "Paginación" } diff --git a/frontend/src/lib/components/Pagination.svelte b/frontend/src/lib/components/Pagination.svelte new file mode 100644 index 00000000..0edf0fb7 --- /dev/null +++ b/frontend/src/lib/components/Pagination.svelte @@ -0,0 +1,58 @@ + + +{#if totalPages > 1} + +{/if} diff --git a/frontend/src/lib/components/Pagination.svelte.spec.ts b/frontend/src/lib/components/Pagination.svelte.spec.ts new file mode 100644 index 00000000..68af1558 --- /dev/null +++ b/frontend/src/lib/components/Pagination.svelte.spec.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; + +import Pagination from './Pagination.svelte'; + +afterEach(() => { + cleanup(); +}); + +const makeHref = (p: number) => `/documents?page=${p}`; + +describe('Pagination', () => { + it('renders the page-of-total label for the current page', async () => { + render(Pagination, { page: 2, totalPages: 10, makeHref }); + + const label = page.getByTestId('pagination-page-label'); + await expect.element(label).toHaveTextContent(/3/); // page is 0-indexed, label is 1-indexed + await expect.element(label).toHaveTextContent(/10/); + }); + + it('marks the current page label with aria-current="page"', async () => { + render(Pagination, { page: 0, totalPages: 3, makeHref }); + + const label = page.getByTestId('pagination-page-label'); + await expect.element(label).toHaveAttribute('aria-current', 'page'); + }); + + it('renders prev as a link pointing at page - 1 when not on first page', async () => { + render(Pagination, { page: 4, totalPages: 10, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect.element(prev).toHaveAttribute('href', '/documents?page=3'); + }); + + it('disables prev on page 0 (no href, aria-disabled="true")', async () => { + render(Pagination, { page: 0, totalPages: 3, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect.element(prev).toHaveAttribute('aria-disabled', 'true'); + await expect.element(prev).not.toHaveAttribute('href'); + }); + + it('renders next as a link pointing at page + 1 when not on last page', async () => { + render(Pagination, { page: 0, totalPages: 3, makeHref }); + + const next = page.getByTestId('pagination-next'); + await expect.element(next).toHaveAttribute('href', '/documents?page=1'); + }); + + it('disables next on the last page (no href, aria-disabled="true")', async () => { + render(Pagination, { page: 2, totalPages: 3, makeHref }); + + const next = page.getByTestId('pagination-next'); + await expect.element(next).toHaveAttribute('aria-disabled', 'true'); + await expect.element(next).not.toHaveAttribute('href'); + }); + + it('calls makeHref with p-1 and p+1', async () => { + const spy = vi.fn(makeHref); + render(Pagination, { page: 3, totalPages: 10, makeHref: spy }); + + const calls = spy.mock.calls.map((c) => c[0]).sort((a, b) => a - b); + expect(calls).toContain(2); + expect(calls).toContain(4); + }); + + it('renders decorative chevron inside aria-hidden span so screen readers skip it', async () => { + render(Pagination, { page: 1, totalPages: 3, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect + .element(prev.getByText('«', { exact: true })) + .toHaveAttribute('aria-hidden', 'true'); + }); + + it('prev and next have min 44px touch targets', async () => { + render(Pagination, { page: 1, totalPages: 3, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect.element(prev).toHaveClass(/min-h-\[44px\]/); + await expect.element(prev).toHaveClass(/min-w-\[44px\]/); + }); +}); diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index a72acd15..aafb9c78 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -1921,7 +1921,13 @@ export interface components { DocumentSearchResult: { items: components["schemas"]["DocumentSearchItem"][]; /** Format: int64 */ - total: number; + totalElements: number; + /** Format: int32 */ + pageNumber: number; + /** Format: int32 */ + pageSize: number; + /** Format: int32 */ + totalPages: number; }; MatchOffset: { /** Format: int32 */ @@ -4032,6 +4038,16 @@ export interface operations { dir?: string; /** @description Tag operator: AND (default) or OR */ tagOp?: string; + /** + * @description Page number (0-indexed) + * @default 0 + */ + page?: number; + /** + * @description Page size (max 100) + * @default 50 + */ + size?: number; }; header?: never; path?: never; diff --git a/frontend/src/routes/documents/+page.server.ts b/frontend/src/routes/documents/+page.server.ts index ed46e46a..f9e4daf4 100644 --- a/frontend/src/routes/documents/+page.server.ts +++ b/frontend/src/routes/documents/+page.server.ts @@ -10,6 +10,8 @@ type ValidSort = (typeof VALID_SORTS)[number]; const VALID_DIRS = ['asc', 'desc'] as const; type ValidDir = (typeof VALID_DIRS)[number]; +const PAGE_SIZE = 50; + export async function load({ url, fetch }) { const q = url.searchParams.get('q') || ''; const from = url.searchParams.get('from') || ''; @@ -27,6 +29,7 @@ export async function load({ url, fetch }) { : 'desc'; const tagQ = url.searchParams.get('tagQ') || ''; const tagOp = url.searchParams.get('tagOp') === 'OR' ? 'OR' : 'AND'; + const page = Math.max(0, Number(url.searchParams.get('page') ?? '0') || 0); const api = createApiClient(fetch); @@ -44,14 +47,19 @@ export async function load({ url, fetch }) { tagQ: tagQ && !tags.length ? tagQ : undefined, tagOp: tagOp === 'OR' ? 'OR' : undefined, sort, - dir: dir || undefined + dir: dir || undefined, + page, + size: PAGE_SIZE } } }); } catch { return { items: [] as DocumentSearchItem[], - total: 0, + totalElements: 0, + pageNumber: 0, + pageSize: PAGE_SIZE, + totalPages: 0, q, from, to, @@ -77,7 +85,10 @@ export async function load({ url, fetch }) { return { items: (result.data?.items ?? []) as DocumentSearchItem[], - total: result.data?.total ?? 0, + totalElements: result.data?.totalElements ?? 0, + pageNumber: result.data?.pageNumber ?? page, + pageSize: result.data?.pageSize ?? PAGE_SIZE, + totalPages: result.data?.totalPages ?? 0, q, from, to, diff --git a/frontend/src/routes/documents/+page.svelte b/frontend/src/routes/documents/+page.svelte index 57054ff6..22f756d3 100644 --- a/frontend/src/routes/documents/+page.svelte +++ b/frontend/src/routes/documents/+page.svelte @@ -5,6 +5,7 @@ import { untrack } from 'svelte'; import { SvelteURLSearchParams } from 'svelte/reactivity'; import SearchFilterBar from '../SearchFilterBar.svelte'; import DocumentList from '../DocumentList.svelte'; +import Pagination from '$lib/components/Pagination.svelte'; import * as m from '$lib/paraglide/messages.js'; let { data } = $props(); @@ -35,6 +36,12 @@ let showAdvanced = $state(untrack(hasAdvancedFilters)); let searchTimer: ReturnType; +/** + * Rebuilds the URL from the CURRENT local filter state. `page` is intentionally + * not carried over — any filter change implicitly resets back to page 0, which + * is the expected behaviour. For page-only navigation use {@link buildPageHref} + * instead, which preserves every filter from the server `data`. + */ function triggerSearch() { const params = new SvelteURLSearchParams(); if (q) params.set('q', q); @@ -50,6 +57,29 @@ function triggerSearch() { goto(`/documents?${params.toString()}`, { keepFocus: true, noScroll: true }); } +/** + * Builds the href for a Pagination prev/next link. Preserves every current + * filter param and only updates `page`. Uses a normal (not goto) + * so SvelteKit's default scroll restoration brings the user to the top of + * the new slice — the expected behaviour for page navigation. + */ +function buildPageHref(targetPage: number): string { + const params = new SvelteURLSearchParams(); + if (data.q) params.set('q', data.q); + if (data.from) params.set('from', data.from); + if (data.to) params.set('to', data.to); + if (data.senderId) params.set('senderId', data.senderId); + if (data.receiverId) params.set('receiverId', data.receiverId); + (data.tags || []).forEach((t: string) => params.append('tag', t)); + if (data.sort) params.set('sort', data.sort); + if (data.dir) params.set('dir', data.dir); + if (data.tagQ) params.set('tagQ', data.tagQ); + if (data.tagOp === 'OR') params.set('tagOp', 'OR'); + if (targetPage > 0) params.set('page', String(targetPage)); + const qs = params.toString(); + return qs ? `/documents?${qs}` : '/documents'; +} + function handleTextSearch() { clearTimeout(searchTimer); searchTimer = setTimeout(() => triggerSearch(), 500); @@ -115,10 +145,12 @@ $effect(() => { + + diff --git a/frontend/src/routes/documents/page.server.spec.ts b/frontend/src/routes/documents/page.server.spec.ts index d345023e..d99c8d8d 100644 --- a/frontend/src/routes/documents/page.server.spec.ts +++ b/frontend/src/routes/documents/page.server.spec.ts @@ -25,7 +25,7 @@ describe('documents page load — search params', () => { it('passes q, from, to to the search API', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -49,7 +49,7 @@ describe('documents page load — search params', () => { it('passes senderId and receiverId to the search API', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -73,7 +73,7 @@ describe('documents page load — search params', () => { it('passes sort, dir, tagQ to the search API', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -103,7 +103,7 @@ describe('documents page load — search params', () => { }; const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [item], total: 42 } + data: { items: [item], totalElements: 42, pageNumber: 0, pageSize: 50, totalPages: 1 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -115,13 +115,13 @@ describe('documents page load — search params', () => { }); expect(result.items).toHaveLength(1); - expect(result.total).toBe(42); + expect(result.totalElements).toBe(42); }); it('returns filter values in the result for pre-filling the UI', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient diff --git a/frontend/src/routes/documents/page.svelte.spec.ts b/frontend/src/routes/documents/page.svelte.spec.ts index 59af31bf..1d8064c6 100644 --- a/frontend/src/routes/documents/page.svelte.spec.ts +++ b/frontend/src/routes/documents/page.svelte.spec.ts @@ -118,4 +118,20 @@ describe('documents page — URL building', () => { expect.objectContaining({ keepFocus: true, noScroll: true }) ); }); + + it('filter change does not carry the current page — goto URL drops page param', async () => { + const { goto } = await import('$app/navigation'); + vi.mocked(goto).mockClear(); + + // User is mid-way through results at page 5; change the search text. + render(Page, { data: makeData({ q: 'old', pageNumber: 5 }) }); + + const input = page.getByRole('textbox', { name: SEARCH_LABEL }); + await input.fill('Brief'); + vi.advanceTimersByTime(500); + + const [url] = vi.mocked(goto).mock.calls[0]; + expect(url).toContain('q=Brief'); + expect(url).not.toContain('page='); + }); });