Build Artikel-Modal with reservation and cancel actions #7

New Issue

marcel · 2026-05-05T10:57:36+02:00

marcel commented

2026-05-05 10:57:36 +02:00

Task 7 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md`

User story (US-GAL-003 + US-RES-001 + US-RES-002):

As a family member, I tap an article card to see all its photos and details in a modal.
I can reserve a free article.
I can cancel my own reservation.

Acceptance criteria

Bottom-sheet modal opens when a gallery card is tapped (Svelte reactive state)
Modal shows: photo gallery (swipeable with prev/next buttons and "1 / 3" counter), category badge, optional title, optional note (only if present), reservation status section
Photo navigation: ‹ / › buttons cycle through photos; counter shows current position
Status: Frei → green "Reservieren" button (full width)
Status: Meine → green badge "✓ Meine Reservierung" + red outline "Reservierung aufheben" button
Status: Belegt → "Reserviert von [Name]" paragraph + disabled "Nicht verfügbar" button
Reservation is atomic: UNIQUE constraint on reservierungen.artikel_id handles race condition — no application-level locking needed; failed INSERT is caught silently (first-come-first-served)
aufheben action checks WHERE code_id = locals.familyCode.id — users cannot cancel others' reservations (NFR-SEC-003)
Backdrop click closes modal
Modal works without JavaScript via ?id= URL param (server renders selectedArtikel from query param)
After reservation action: redirects back to /galerie?id={id} to re-render modal with updated status

Files to create/modify

src/lib/components/ArtikelModal.svelte (new)
src/routes/artikel/[id]/+page.server.ts (new — reservieren and aufheben actions)
src/routes/galerie/+page.server.ts (modify — add selectedArtikel from ?id= param)
src/routes/galerie/+page.svelte (modify — replace placeholder with ArtikelModal component)

Depends on: #6 | Size: M | Spec: reservierung-design §4.3, views spec View 03, NFR-SEC-003, NFR-DATA-002

## Task 7 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md` **User story (US-GAL-003 + US-RES-001 + US-RES-002):** - As a family member, I tap an article card to see all its photos and details in a modal. - I can reserve a free article. - I can cancel my own reservation. ### Acceptance criteria - [ ] Bottom-sheet modal opens when a gallery card is tapped (Svelte reactive state) - [ ] Modal shows: photo gallery (swipeable with prev/next buttons and "1 / 3" counter), category badge, optional title, optional note (only if present), reservation status section - [ ] Photo navigation: ‹ / › buttons cycle through photos; counter shows current position - [ ] **Status: Frei** → green "Reservieren" button (full width) - [ ] **Status: Meine** → green badge "✓ Meine Reservierung" + red outline "Reservierung aufheben" button - [ ] **Status: Belegt** → "Reserviert von [Name]" paragraph + disabled "Nicht verfügbar" button - [ ] Reservation is atomic: UNIQUE constraint on `reservierungen.artikel_id` handles race condition — no application-level locking needed; failed INSERT is caught silently (first-come-first-served) - [ ] `aufheben` action checks `WHERE code_id = locals.familyCode.id` — users cannot cancel others' reservations (NFR-SEC-003) - [ ] Backdrop click closes modal - [ ] Modal works without JavaScript via `?id=` URL param (server renders `selectedArtikel` from query param) - [ ] After reservation action: redirects back to `/galerie?id={id}` to re-render modal with updated status ### Files to create/modify - `src/lib/components/ArtikelModal.svelte` (new) - `src/routes/artikel/[id]/+page.server.ts` (new — `reservieren` and `aufheben` actions) - `src/routes/galerie/+page.server.ts` (modify — add `selectedArtikel` from `?id=` param) - `src/routes/galerie/+page.svelte` (modify — replace placeholder with `ArtikelModal` component) **Depends on:** #6 | **Size:** M | **Spec:** reservierung-design §4.3, views spec View 03, NFR-SEC-003, NFR-DATA-002

marcel added this to the v1.0 — MVP milestone 2026-05-05 10:57:36 +02:00

marcel commented

2026-05-05 11:26:27 +02:00

👤 Markus Keller — Application Architect

Observations

The route split is correct: artikel/[id]/+page.server.ts for the two Form Actions (reservieren, aufheben) and galerie/+page.server.ts for loading selectedArtikel via ?id= query param. This aligns exactly with the system design spec section 6.
The UNIQUE constraint on reservierungen.artikel_id is the complete concurrency strategy. No application-level lock, no SELECT-before-INSERT. The issue correctly specifies catching the failed INSERT silently — that pattern is right.
The no-JS fallback via ?id= URL param and server-side rendering of selectedArtikel is architecturally sound and consistent with SSR-first design. After reservation actions, redirecting back to /galerie?id={id} re-opens the modal with fresh server state — no client-side cache to invalidate.
ArtikelModal.svelte is a new component — not a route. Modal state lives in galerie/+page.svelte as a $state variable (the selected article ID). The component receives the article as a prop, not as a page load.
The aufheben action must verify WHERE code_id = locals.familyCode.id server-side. The issue specifies this (NFR-SEC-003). Do not add a client-side guard as a substitute — the server check is the real guard.

Recommendations

Keep artikel/[id]/+page.server.ts purely as a Form Action file — no load() function needed there. The load for the gallery is in galerie/+page.server.ts.
The galerie/+page.server.ts load function should query selectedArtikel with a join to artikel_fotos and reservierungen in a single prepared statement when url.searchParams.get('id') is present. Avoid a second round-trip.
Prepared statements for both actions belong in lib/db.ts, defined at module load: stmtInsertReservierung, stmtDeleteReservierung, stmtReservierungByArtikel (for ownership check), stmtArtikelById (for the modal query in galerie load).
Catch only UNIQUE constraint failed from the INSERT error; rethrow everything else. Do not swallow unexpected database errors.
The redirect after action should use SvelteKit's redirect(303, '/galerie?id=' + artikelId). This ensures the browser re-GETs the page, the load function re-queries the DB, and the modal opens with current state.

Open Decisions

Modal close URL — After closing the modal (backdrop click or swipe), should the browser update the URL to /galerie (removing ?id=)? Options: (a) pushState client-side — clean URL but requires JS; (b) do nothing — URL stays as /galerie?id=X until navigation, which means refresh re-opens the modal. Both are acceptable for this app; (b) is simpler. (Raised by: Markus Keller)

## 👤 Markus Keller — Application Architect ### Observations - The route split is correct: `artikel/[id]/+page.server.ts` for the two Form Actions (`reservieren`, `aufheben`) and `galerie/+page.server.ts` for loading `selectedArtikel` via `?id=` query param. This aligns exactly with the system design spec section 6. - The UNIQUE constraint on `reservierungen.artikel_id` is the complete concurrency strategy. No application-level lock, no SELECT-before-INSERT. The issue correctly specifies catching the failed INSERT silently — that pattern is right. - The no-JS fallback via `?id=` URL param and server-side rendering of `selectedArtikel` is architecturally sound and consistent with SSR-first design. After reservation actions, redirecting back to `/galerie?id={id}` re-opens the modal with fresh server state — no client-side cache to invalidate. - `ArtikelModal.svelte` is a new component — not a route. Modal state lives in `galerie/+page.svelte` as a `$state` variable (the selected article ID). The component receives the article as a prop, not as a page load. - The `aufheben` action must verify `WHERE code_id = locals.familyCode.id` server-side. The issue specifies this (NFR-SEC-003). Do not add a client-side guard as a substitute — the server check is the real guard. ### Recommendations - Keep `artikel/[id]/+page.server.ts` purely as a Form Action file — no `load()` function needed there. The load for the gallery is in `galerie/+page.server.ts`. - The `galerie/+page.server.ts` load function should query `selectedArtikel` with a join to `artikel_fotos` and `reservierungen` in a single prepared statement when `url.searchParams.get('id')` is present. Avoid a second round-trip. - Prepared statements for both actions belong in `lib/db.ts`, defined at module load: `stmtInsertReservierung`, `stmtDeleteReservierung`, `stmtReservierungByArtikel` (for ownership check), `stmtArtikelById` (for the modal query in galerie load). - Catch only `UNIQUE constraint failed` from the INSERT error; rethrow everything else. Do not swallow unexpected database errors. - The redirect after action should use SvelteKit's `redirect(303, '/galerie?id=' + artikelId)`. This ensures the browser re-GETs the page, the load function re-queries the DB, and the modal opens with current state. ### Open Decisions - **Modal close URL** — After closing the modal (backdrop click or swipe), should the browser update the URL to `/galerie` (removing `?id=`)? Options: (a) pushState client-side — clean URL but requires JS; (b) do nothing — URL stays as `/galerie?id=X` until navigation, which means refresh re-opens the modal. Both are acceptable for this app; (b) is simpler. _(Raised by: Markus Keller)_

marcel commented

2026-05-05 11:26:45 +02:00

👤 Felix Brandt — Fullstack Developer

Observations

ArtikelModal.svelte is listed as a new file — this is the correct split. The gallery page is the orchestrator; the modal is a single nameable visual region. Photo navigation state (currentFotoIndex) is local $state inside the modal component.
The issue specifies Svelte reactive state for modal open/close. In Svelte 5 this is let selectedArtikel = $state<ArtikelWithStatus | null>(null) in galerie/+page.svelte, passed as a prop to ArtikelModal.
Three reservation status branches map cleanly to $derived: const status = $derived.by(() => { if (!selectedArtikel.reservierung) return 'frei'; if (selectedArtikel.reservierung.code_id === data.familyCode.id) return 'meins'; return 'anderer'; }). Extract this — do not inline the condition in template markup.
Both Form Actions (reservieren, aufheben) must use use:enhance for client-side UX while remaining functional without JS. The submitting state from the enhance callback prevents double-submits.
The {#each fotos as foto, i (foto.id)} key expression is mandatory — keyed iteration for the photo strip.
After a Form Action completes (success or fail), SvelteKit invalidates the load function. The redirect to /galerie?id={id} triggers a new load, which is correct. Do not use invalidate() manually.

Recommendations

Photo navigation: implement with let currentIndex = $state(0) inside ArtikelModal. Prev/next buttons call currentIndex = (currentIndex - 1 + fotos.length) % fotos.length. Reset to 0 when selectedArtikel changes via $effect(() => { currentIndex = 0; }) — this is one of the few valid $effect uses (syncing external state on prop change).
The counter display {currentIndex + 1} / {fotos.length} should be hidden when fotos.length === 1 (per the spec's "hide counter for single photo" note).
Map form?.error to German user strings — never expose raw error keys: 'already_reserved' → 'Dieser Artikel wurde soeben von jemand anderem reserviert.', 'not_your_reservation' → 'Diese Reservierung gehört dir nicht.'
The backdrop click handler: <div role="button" tabindex="0" on:click={closeModal} on:keydown={e => e.key === 'Escape' && closeModal()} — keyboard accessible, not just mouse.
Guard the aufheben action with if (!locals.familyCode) redirect(303, '/') before any DB call. Same for reservieren.

Open Decisions

Swipe gesture implementation — The spec calls for swipeable photo navigation (touch-action: pan-x). Options: (a) CSS-only touch-action with JS pointer event listeners (lightweight, no dependency); (b) a small touch library. For a family app with no bundle size pressure, (a) is the right call — but the implementation complexity vs. just prev/next buttons is a real tradeoff worth confirming before coding starts. (Raised by: Felix Brandt)

## 👤 Felix Brandt — Fullstack Developer ### Observations - `ArtikelModal.svelte` is listed as a new file — this is the correct split. The gallery page is the orchestrator; the modal is a single nameable visual region. Photo navigation state (`currentFotoIndex`) is local `$state` inside the modal component. - The issue specifies Svelte reactive state for modal open/close. In Svelte 5 this is `let selectedArtikel = $state<ArtikelWithStatus | null>(null)` in `galerie/+page.svelte`, passed as a prop to `ArtikelModal`. - Three reservation status branches map cleanly to `$derived`: `const status = $derived.by(() => { if (!selectedArtikel.reservierung) return 'frei'; if (selectedArtikel.reservierung.code_id === data.familyCode.id) return 'meins'; return 'anderer'; })`. Extract this — do not inline the condition in template markup. - Both Form Actions (`reservieren`, `aufheben`) must use `use:enhance` for client-side UX while remaining functional without JS. The `submitting` state from the enhance callback prevents double-submits. - The `{#each fotos as foto, i (foto.id)}` key expression is mandatory — keyed iteration for the photo strip. - After a Form Action completes (success or fail), SvelteKit invalidates the load function. The redirect to `/galerie?id={id}` triggers a new load, which is correct. Do not use `invalidate()` manually. ### Recommendations - Photo navigation: implement with `let currentIndex = $state(0)` inside `ArtikelModal`. Prev/next buttons call `currentIndex = (currentIndex - 1 + fotos.length) % fotos.length`. Reset to 0 when `selectedArtikel` changes via `$effect(() => { currentIndex = 0; })` — this is one of the few valid `$effect` uses (syncing external state on prop change). - The counter display `{currentIndex + 1} / {fotos.length}` should be hidden when `fotos.length === 1` (per the spec's "hide counter for single photo" note). - Map `form?.error` to German user strings — never expose raw error keys: `'already_reserved'` → `'Dieser Artikel wurde soeben von jemand anderem reserviert.'`, `'not_your_reservation'` → `'Diese Reservierung gehört dir nicht.'` - The backdrop click handler: `<div role="button" tabindex="0" on:click={closeModal} on:keydown={e => e.key === 'Escape' && closeModal()}` — keyboard accessible, not just mouse. - Guard the `aufheben` action with `if (!locals.familyCode) redirect(303, '/')` before any DB call. Same for `reservieren`. ### Open Decisions - **Swipe gesture implementation** — The spec calls for swipeable photo navigation (`touch-action: pan-x`). Options: (a) CSS-only touch-action with JS pointer event listeners (lightweight, no dependency); (b) a small touch library. For a family app with no bundle size pressure, (a) is the right call — but the implementation complexity vs. just prev/next buttons is a real tradeoff worth confirming before coding starts. _(Raised by: Felix Brandt)_

marcel commented

2026-05-05 11:27:02 +02:00

👤 Nora "NullX" Steiner — Security Engineer

Observations

NFR-SEC-003 (IDOR) is the primary risk for this issue. The aufheben action deletes a reservation. Without an ownership check, any authenticated family member with the article ID can cancel any other member's reservation by crafting a POST to /artikel/[id]?/aufheben with artikel_id in the form body.
The issue correctly specifies WHERE code_id = locals.familyCode.id — this is the mandatory check. It must be implemented as a server-side query, not a client-side condition on a hidden form field.
The reservieren action's UNIQUE constraint is the correct atomicity mechanism. No pre-check SELECT is needed or safe. The catch block must specifically detect UNIQUE constraint failed (CWE-362 — Race Condition) and return fail(409).
Both actions read locals.familyCode (set by hooks.server.ts) — correct pattern. No route should read the raw family_code cookie.
The modal renders reserviert_von (the display name of the reserver). This is safe — it is a display name set by an admin, not user-controlled input. No XSS risk in Svelte's template system (auto-escaped).
Photo navigation does not involve file reads in this issue, so the upload path traversal vector is not in scope here.

Recommendations

Implement the aufheben ownership check as follows — exactly this pattern, no shortcut:
```
const reservierung = stmtReservierungByArtikel.get(artikelId) as ReservierungRow | undefined;
if (!reservierung || reservierung.code_id !== locals.familyCode.id) {
  return fail(403, { error: 'not_your_reservation' });
}
stmtDeleteReservierung.run(artikelId);
```
The ownership check and the delete must be in this order. Do not combine into a DELETE WHERE artikel_id = ? AND code_id = ? without first confirming the row exists — the affectedRows count is unreliable as the sole feedback mechanism.
Both actions must guard if (!locals.familyCode) redirect(303, '/') as the very first line. No DB call before auth check.
The form?.error values exposed to the template must never contain the actual code value, reservation ID, or internal DB details. 'already_reserved' and 'not_your_reservation' are safe opaque keys.
Add a security comment in the aufheben action explaining the threat: // NFR-SEC-003: prevents IDOR — any family member could cancel any reservation without this check.

Open Decisions

Rate limiting on reservation actions — The reservation form is accessible to all valid cookie holders. There is no rate limit specified. For this family-scale app, the SQLite UNIQUE constraint prevents meaningful abuse (you can only reserve each article once). No rate limiting is needed. This is a non-decision — documenting it as intentional is sufficient. (Raised by: Nora Steiner)

## 👤 Nora "NullX" Steiner — Security Engineer ### Observations - **NFR-SEC-003 (IDOR) is the primary risk for this issue.** The `aufheben` action deletes a reservation. Without an ownership check, any authenticated family member with the article ID can cancel any other member's reservation by crafting a POST to `/artikel/[id]?/aufheben` with `artikel_id` in the form body. - The issue correctly specifies `WHERE code_id = locals.familyCode.id` — this is the mandatory check. It must be implemented as a server-side query, not a client-side condition on a hidden form field. - The `reservieren` action's UNIQUE constraint is the correct atomicity mechanism. No pre-check SELECT is needed or safe. The catch block must specifically detect `UNIQUE constraint failed` (CWE-362 — Race Condition) and return `fail(409)`. - Both actions read `locals.familyCode` (set by `hooks.server.ts`) — correct pattern. No route should read the raw `family_code` cookie. - The modal renders `reserviert_von` (the display name of the reserver). This is safe — it is a display name set by an admin, not user-controlled input. No XSS risk in Svelte's template system (auto-escaped). - Photo navigation does not involve file reads in this issue, so the upload path traversal vector is not in scope here. ### Recommendations - Implement the `aufheben` ownership check as follows — exactly this pattern, no shortcut: ```typescript const reservierung = stmtReservierungByArtikel.get(artikelId) as ReservierungRow | undefined; if (!reservierung || reservierung.code_id !== locals.familyCode.id) { return fail(403, { error: 'not_your_reservation' }); } stmtDeleteReservierung.run(artikelId); ``` The ownership check and the delete must be in this order. Do not combine into a `DELETE WHERE artikel_id = ? AND code_id = ?` without first confirming the row exists — the affectedRows count is unreliable as the sole feedback mechanism. - Both actions must guard `if (!locals.familyCode) redirect(303, '/')` as the very first line. No DB call before auth check. - The `form?.error` values exposed to the template must never contain the actual code value, reservation ID, or internal DB details. `'already_reserved'` and `'not_your_reservation'` are safe opaque keys. - Add a security comment in the `aufheben` action explaining the threat: `// NFR-SEC-003: prevents IDOR — any family member could cancel any reservation without this check`. ### Open Decisions - **Rate limiting on reservation actions** — The reservation form is accessible to all valid cookie holders. There is no rate limit specified. For this family-scale app, the SQLite UNIQUE constraint prevents meaningful abuse (you can only reserve each article once). No rate limiting is needed. This is a non-decision — documenting it as intentional is sufficient. _(Raised by: Nora Steiner)_

marcel commented

2026-05-05 11:27:21 +02:00

👤 Sara Holt — QA Engineer

Observations

This issue involves three testable layers: (1) the reservieren and aufheben Form Actions in artikel/[id]/+page.server.ts — pure server logic, testable with :memory: SQLite; (2) the ArtikelModal.svelte component — three rendering states testable with vitest-browser-svelte; (3) the E2E journey: tap card → modal opens → reserve → see "Meine Reservierung".
The UNIQUE constraint behavior (double-reservation) is the most important integration test. The issue explicitly calls this out — it must have its own dedicated test, not be bundled with happy-path tests.
The aufheben IDOR check (NFR-SEC-003) must have a security regression test: code B attempting to cancel code A's reservation must return fail(403).
The no-JS path (server renders selectedArtikel from ?id=) means the modal must render correctly on SSR, which Playwright E2E will verify by navigating directly to /galerie?id=X.
The form?.error display in the modal (race condition message) needs a component test, not just an E2E test — it needs to be fast and deterministic.

Recommendations

Integration tests for Form Actions (Vitest + :memory: SQLite):
- reservieren returns 409 when article is already reserved — insert a reservation first, then attempt a second INSERT, expect UNIQUE constraint failed.
- reservieren succeeds and inserts a row — verify the row exists with the correct code_id.
- aufheben returns 403 when code_id does not match — reservation owned by code A, attempt cancel with code B.
- aufheben deletes the row when code_id matches — verify row is gone after successful cancel.
- reservieren returns 400 for non-integer artikel_id — send 'abc' as artikel_id.
Component tests (vitest-browser-svelte):
- shows "Reservieren" button when status is frei
- shows "✓ Meine Reservierung" badge and "Aufheben" button when status is meins
- shows "Reserviert von [Name]" and disabled "Nicht verfügbar" button when status is anderer
- hides photo counter when only one photo
- shows race condition error message when form.error is "already_reserved"
E2E journeys (Playwright — add to existing critical journeys list):
- Galerie → Karte antippen → Modal öffnet → Reservieren → Badge "Meine Reservierung" sichtbar
- Galerie → Modal öffnet über ?id= URL-Parameter (kein JS-Klick)
- Galerie → Eigene Reservierung aufheben → Status kehrt zu "Frei" zurück
Each integration test must use a fresh :memory: database in beforeEach. Never share a database instance across test cases.

Open Decisions

Component test scope for photo navigation — The swipe/prev-next logic in ArtikelModal could be tested at the component level (click next → counter increments) or only at E2E level. Component tests are faster and more reliable here. Recommend adding: clicking next button advances photo counter from "1 / 3" to "2 / 3". (Raised by: Sara Holt)

## 👤 Sara Holt — QA Engineer ### Observations - This issue involves three testable layers: (1) the `reservieren` and `aufheben` Form Actions in `artikel/[id]/+page.server.ts` — pure server logic, testable with `:memory:` SQLite; (2) the `ArtikelModal.svelte` component — three rendering states testable with `vitest-browser-svelte`; (3) the E2E journey: tap card → modal opens → reserve → see "Meine Reservierung". - The UNIQUE constraint behavior (double-reservation) is the most important integration test. The issue explicitly calls this out — it must have its own dedicated test, not be bundled with happy-path tests. - The `aufheben` IDOR check (NFR-SEC-003) must have a security regression test: code B attempting to cancel code A's reservation must return `fail(403)`. - The no-JS path (server renders `selectedArtikel` from `?id=`) means the modal must render correctly on SSR, which Playwright E2E will verify by navigating directly to `/galerie?id=X`. - The `form?.error` display in the modal (race condition message) needs a component test, not just an E2E test — it needs to be fast and deterministic. ### Recommendations - **Integration tests for Form Actions** (Vitest + `:memory:` SQLite): - `reservieren returns 409 when article is already reserved` — insert a reservation first, then attempt a second INSERT, expect `UNIQUE constraint failed`. - `reservieren succeeds and inserts a row` — verify the row exists with the correct `code_id`. - `aufheben returns 403 when code_id does not match` — reservation owned by code A, attempt cancel with code B. - `aufheben deletes the row when code_id matches` — verify row is gone after successful cancel. - `reservieren returns 400 for non-integer artikel_id` — send `'abc'` as `artikel_id`. - **Component tests** (`vitest-browser-svelte`): - `shows "Reservieren" button when status is frei` - `shows "✓ Meine Reservierung" badge and "Aufheben" button when status is meins` - `shows "Reserviert von [Name]" and disabled "Nicht verfügbar" button when status is anderer` - `hides photo counter when only one photo` - `shows race condition error message when form.error is "already_reserved"` - **E2E journeys** (Playwright — add to existing critical journeys list): - `Galerie → Karte antippen → Modal öffnet → Reservieren → Badge "Meine Reservierung" sichtbar` - `Galerie → Modal öffnet über ?id= URL-Parameter (kein JS-Klick)` - `Galerie → Eigene Reservierung aufheben → Status kehrt zu "Frei" zurück` - Each integration test must use a fresh `:memory:` database in `beforeEach`. Never share a database instance across test cases. ### Open Decisions - **Component test scope for photo navigation** — The swipe/prev-next logic in `ArtikelModal` could be tested at the component level (click next → counter increments) or only at E2E level. Component tests are faster and more reliable here. Recommend adding: `clicking next button advances photo counter from "1 / 3" to "2 / 3"`. _(Raised by: Sara Holt)_

marcel commented

2026-05-05 11:27:47 +02:00

👤 Leonie Voss — UI/UX Design Lead

Observations

View 03 of the spec is the exact reference for this issue. All three modal states (Frei / Meine / Belegt) are fully designed with implementation tables in 2026-05-05-erbstuecke-wannsee-views.html.
The bottom sheet structure: fixed inset-0 bg-black/45 flex items-end z-50 for the overlay, bg-surface rounded-t-xl w-full max-h-[88dvh] overflow-y-auto for the sheet. The 88dvh max-height prevents the sheet from covering the full screen on short phones.
The drag handle (w-9 h-1 bg-line rounded mx-auto mt-2.5 mb-3.5) is a visual affordance only — the swipe-to-close behavior is the real interaction.
The photo gallery is aspect-[4/3] — not aspect-square. The counter absolute bottom-2 right-2.5 bg-black/45 text-white text-[9px] font-bold px-1.5 py-0.5 rounded-full must be hidden when there is only one photo (per spec note).
The category badge uses the green pill style: inline-block text-[9px] font-extrabold uppercase tracking-wider bg-[#DFF0E6] text-[#2E6645] px-2.5 py-0.5 rounded-full border border-[#A8D5B8] mb-1.5 — this is a different green than the nav (--color-primary). Do not mix them up.
The "Meine Reservierung" badge in the modal is displayed as a block-level element with font-size: 12px; padding: 7px 14px; border-radius: 8px; text-align: center — larger and more prominent than the card-level badge (which uses the pill form). These are intentionally different sizes.
The notiz field must only render when artikel.notiz is present — {#if artikel.notiz}. Rendering an empty italic <p> wastes space.
The "Nicht verfügbar" button must have disabled attribute set — not just visual styling. cursor-not-allowed without disabled is a styling hack, not true inaccessibility prevention.

Recommendations

Use <dialog> or role="dialog" aria-modal="true" on the sheet element. The overlay div needs aria-label="Artikel-Details" or equivalent. Without this, screen readers do not know the modal exists.
Add aria-live="polite" around the reservation status section. When a Form Action completes and the status changes (e.g., from Frei to Meine), the screen reader announces the change without disrupting focus.
The Artikel-Titel uses font-serif text-[18px] font-bold text-ink leading-snug mb-1.5 (Lora). Do not use Inter for the title — it must be Lora per the type system.
Prev/next photo buttons: these are not in the spec mockup but are required by the acceptance criteria. Style them as icon-only buttons (aria-label="Vorheriges Foto" / aria-label="Nächstes Foto") with bg-black/45 text-white overlaid at the left/right edges of the photo — min 44×44px touch target. Never render them outside the photo area as text links.
The backdrop click-to-close: the outer overlay div needs on:click={closeModal} but the sheet itself needs on:click|stopPropagation to prevent clicks inside the modal from closing it.
Font size floor: the photo counter at text-[9px] is the only element below 10px. This is acceptable per spec for this specific element — but do not introduce any other sub-10px text in the modal.
Verify contrast: #2E6645 (category badge text) on #DFF0E6 (badge background) — check with WebAIM. If it fails 4.5:1, darken the text to #1E4D2E.

Open Decisions

Swipe-to-close gesture on the sheet handle — The spec says "Swipe-Down schließt Modal". Implementing a genuine drag-to-dismiss with velocity detection is non-trivial. Options: (a) Swipe handle is purely decorative — close only via backdrop click or a close button (simpler, acceptable for a family app); (b) implement pointer event drag with a 100px downward threshold. The spec mentions it as an interaction but does not require it in the acceptance criteria. Recommend clarifying before implementation. (Raised by: Leonie Voss)

## 👤 Leonie Voss — UI/UX Design Lead ### Observations - View 03 of the spec is the exact reference for this issue. All three modal states (Frei / Meine / Belegt) are fully designed with implementation tables in `2026-05-05-erbstuecke-wannsee-views.html`. - The bottom sheet structure: `fixed inset-0 bg-black/45 flex items-end z-50` for the overlay, `bg-surface rounded-t-xl w-full max-h-[88dvh] overflow-y-auto` for the sheet. The `88dvh` max-height prevents the sheet from covering the full screen on short phones. - The drag handle (`w-9 h-1 bg-line rounded mx-auto mt-2.5 mb-3.5`) is a visual affordance only — the swipe-to-close behavior is the real interaction. - The photo gallery is `aspect-[4/3]` — not `aspect-square`. The counter `absolute bottom-2 right-2.5 bg-black/45 text-white text-[9px] font-bold px-1.5 py-0.5 rounded-full` must be hidden when there is only one photo (per spec note). - The category badge uses the green pill style: `inline-block text-[9px] font-extrabold uppercase tracking-wider bg-[#DFF0E6] text-[#2E6645] px-2.5 py-0.5 rounded-full border border-[#A8D5B8] mb-1.5` — this is a different green than the nav (`--color-primary`). Do not mix them up. - The "Meine Reservierung" badge in the modal is displayed as a block-level element with `font-size: 12px; padding: 7px 14px; border-radius: 8px; text-align: center` — larger and more prominent than the card-level badge (which uses the pill form). These are intentionally different sizes. - The `notiz` field must only render when `artikel.notiz` is present — `{#if artikel.notiz}`. Rendering an empty italic `<p>` wastes space. - The "Nicht verfügbar" button must have `disabled` attribute set — not just visual styling. `cursor-not-allowed` without `disabled` is a styling hack, not true inaccessibility prevention. ### Recommendations - Use `<dialog>` or `role="dialog" aria-modal="true"` on the sheet element. The overlay div needs `aria-label="Artikel-Details"` or equivalent. Without this, screen readers do not know the modal exists. - Add `aria-live="polite"` around the reservation status section. When a Form Action completes and the status changes (e.g., from Frei to Meine), the screen reader announces the change without disrupting focus. - The Artikel-Titel uses `font-serif text-[18px] font-bold text-ink leading-snug mb-1.5` (Lora). Do not use Inter for the title — it must be Lora per the type system. - Prev/next photo buttons: these are not in the spec mockup but are required by the acceptance criteria. Style them as icon-only buttons (`aria-label="Vorheriges Foto"` / `aria-label="Nächstes Foto"`) with `bg-black/45 text-white` overlaid at the left/right edges of the photo — min 44×44px touch target. Never render them outside the photo area as text links. - The backdrop click-to-close: the outer overlay div needs `on:click={closeModal}` but the sheet itself needs `on:click|stopPropagation` to prevent clicks inside the modal from closing it. - Font size floor: the photo counter at `text-[9px]` is the only element below 10px. This is acceptable per spec for this specific element — but do not introduce any other sub-10px text in the modal. - Verify contrast: `#2E6645` (category badge text) on `#DFF0E6` (badge background) — check with WebAIM. If it fails 4.5:1, darken the text to `#1E4D2E`. ### Open Decisions - **Swipe-to-close gesture on the sheet handle** — The spec says "Swipe-Down schließt Modal". Implementing a genuine drag-to-dismiss with velocity detection is non-trivial. Options: (a) Swipe handle is purely decorative — close only via backdrop click or a close button (simpler, acceptable for a family app); (b) implement pointer event drag with a 100px downward threshold. The spec mentions it as an interaction but does not require it in the acceptance criteria. Recommend clarifying before implementation. _(Raised by: Leonie Voss)_

marcel commented

2026-05-05 11:28:09 +02:00

👤 Elicit — Requirements Engineer

Observations

The acceptance criteria are well-structured and testable — Given-When-Then is implicit but verifiable. The three modal states (Frei, Meine, Belegt) are exhaustive and unambiguous. NFR-SEC-003 is explicitly linked. This issue is ready.
US-GAL-003, US-RES-001, and US-RES-002 are all addressed in scope. The issue correctly groups them as a single M-sized delivery unit — they share the same component and the same Form Action file.
One ambiguity in the accepted criteria: "Photo navigation: ‹ / › buttons cycle through photos" — the spec View 03 shows a counter and a swipe gesture but does not show explicit prev/next buttons in the mockup. The acceptance criteria adds buttons not present in the mockup. This needs to be confirmed: are the ‹ / › buttons rendered always, or only as a fallback when swipe is not available?
The no-JS requirement is well-handled by the ?id= URL param pattern. However: the acceptance criteria say "Modal works without JavaScript via ?id= URL param (server renders selectedArtikel)" — this implies that on a no-JS load, the modal appears inline in the page (not as a floating sheet), since the overlay and sheet positioning require JS to trigger the $state change. The server-rendered version should display the article details in a plain section or card, not attempt to render the bottom-sheet. This distinction is not explicit in the issue.
The redirect pattern POST → redirect(303) → GET /galerie?id={id} is the correct PRG (Post-Redirect-Get) pattern. It prevents form resubmission on browser back/refresh. This is good — it is implied but worth naming explicitly.

Recommendations

Add one acceptance criterion explicitly covering the no-JS modal rendering mode: "When ?id= is present and JS is not available, the article details are rendered inline on the gallery page (not as a bottom sheet overlay)."
Clarify in the issue whether ‹ / › navigation buttons are: (a) always rendered alongside the photo; (b) only rendered when fotos.length > 1. The spec's "hide counter at 1 photo" note implies (b) for the counter — apply the same rule to the buttons.
The form?.error for the race condition (already_reserved) should display inside the modal, not as a top-level page alert. The acceptance criteria do not specify where the error appears — recommend making this explicit: "Error appears in the modal's reservation status section, replacing the action button."
NFR-DATA-002 (atomic reservation) is fully satisfied by the UNIQUE constraint. Consider adding a brief note in the issue body confirming this is intentional — future readers may not realize the atomicity is at the DB layer, not in application code.

Open Decisions

No-JS modal rendering mode — When a user navigates directly to /galerie?id=5 without JS, the server renders selectedArtikel. But the gallery page's bottom-sheet modal is a JS-driven component. Should the server-rendered selectedArtikel appear as: (a) a full-page article detail at the top of the gallery; (b) a static HTML version of the modal sheet; (c) nothing special — the ?id= param only serves as a re-open signal after POST redirect, and no-JS users just see the gallery. Option (c) is the simplest and defensible since the app requires cookies (which implies a browser capable of JS in practice). (Raised by: Elicit)

## 👤 Elicit — Requirements Engineer ### Observations - The acceptance criteria are well-structured and testable — Given-When-Then is implicit but verifiable. The three modal states (Frei, Meine, Belegt) are exhaustive and unambiguous. NFR-SEC-003 is explicitly linked. This issue is ready. - US-GAL-003, US-RES-001, and US-RES-002 are all addressed in scope. The issue correctly groups them as a single M-sized delivery unit — they share the same component and the same Form Action file. - **One ambiguity in the accepted criteria:** "Photo navigation: ‹ / › buttons cycle through photos" — the spec View 03 shows a counter and a swipe gesture but does not show explicit prev/next buttons in the mockup. The acceptance criteria adds buttons not present in the mockup. This needs to be confirmed: are the ‹ / › buttons rendered always, or only as a fallback when swipe is not available? - The no-JS requirement is well-handled by the `?id=` URL param pattern. However: the acceptance criteria say "Modal works without JavaScript via `?id=` URL param (server renders `selectedArtikel`)" — this implies that on a no-JS load, the modal appears inline in the page (not as a floating sheet), since the overlay and sheet positioning require JS to trigger the `$state` change. The server-rendered version should display the article details in a plain section or card, not attempt to render the bottom-sheet. This distinction is not explicit in the issue. - The redirect pattern `POST → redirect(303) → GET /galerie?id={id}` is the correct PRG (Post-Redirect-Get) pattern. It prevents form resubmission on browser back/refresh. This is good — it is implied but worth naming explicitly. ### Recommendations - Add one acceptance criterion explicitly covering the no-JS modal rendering mode: "When `?id=` is present and JS is not available, the article details are rendered inline on the gallery page (not as a bottom sheet overlay)." - Clarify in the issue whether ‹ / › navigation buttons are: (a) always rendered alongside the photo; (b) only rendered when `fotos.length > 1`. The spec's "hide counter at 1 photo" note implies (b) for the counter — apply the same rule to the buttons. - The `form?.error` for the race condition (already_reserved) should display inside the modal, not as a top-level page alert. The acceptance criteria do not specify where the error appears — recommend making this explicit: "Error appears in the modal's reservation status section, replacing the action button." - NFR-DATA-002 (atomic reservation) is fully satisfied by the UNIQUE constraint. Consider adding a brief note in the issue body confirming this is intentional — future readers may not realize the atomicity is at the DB layer, not in application code. ### Open Decisions - **No-JS modal rendering mode** — When a user navigates directly to `/galerie?id=5` without JS, the server renders `selectedArtikel`. But the gallery page's bottom-sheet modal is a JS-driven component. Should the server-rendered `selectedArtikel` appear as: (a) a full-page article detail at the top of the gallery; (b) a static HTML version of the modal sheet; (c) nothing special — the `?id=` param only serves as a re-open signal after POST redirect, and no-JS users just see the gallery. Option (c) is the simplest and defensible since the app requires cookies (which implies a browser capable of JS in practice). _(Raised by: Elicit)_

marcel commented

2026-05-05 11:28:27 +02:00

🗳️ Decision Queue — Action Required

5 decisions need your input before implementation starts.

Modal close URL — After closing the modal via backdrop click, should the URL update from /galerie?id=X back to /galerie? Option (a): history.pushState client-side — clean URL, requires JS. Option (b): do nothing — URL retains ?id=X, refresh re-opens the modal. Option (b) is simpler and acceptable; option (a) is cleaner UX. (Raised by: Markus Keller)
No-JS modal rendering mode — When /galerie?id=5 is loaded without JS, selectedArtikel is server-rendered. Should it appear as: (a) article details at the top of the gallery; (b) a static HTML sheet; (c) nothing — ?id= is only a post-redirect signal, no-JS users just see the gallery. Option (c) is simplest and defensible (cookie-based auth implies a JS-capable browser in practice). (Raised by: Elicit)

Swipe gesture for photos — The spec requires touch-action: pan-x swipe for photo navigation. Options: (a) implement pointer event drag (lightweight, no library, moderate complexity); (b) skip swipe — only use ‹ / › prev/next buttons. The acceptance criteria explicitly lists ‹ / › buttons; swipe is an enhancement. Confirm whether swipe is a hard requirement or a nice-to-have before coding starts. (Raised by: Felix Brandt)
Swipe-to-close the bottom sheet — The spec describes "Swipe-Down schließt Modal" but the acceptance criteria only lists "Backdrop click closes modal." Options: (a) swipe handle is decorative — close only via backdrop click (simpler); (b) implement drag-to-dismiss with a 100px downward threshold. Confirm which is required. (Raised by: Leonie Voss)

Prev/Next Buttons Visibility

When to show ‹ / › navigation buttons — The acceptance criteria adds ‹ / › buttons not present in the spec mockup. Should they: (a) always render; (b) only render when fotos.length > 1 (same rule as the counter). Option (b) is consistent with the spec's "hide counter at 1 photo" note and avoids rendering non-functional buttons. (Raised by: Elicit)

## 🗳️ Decision Queue — Action Required _5 decisions need your input before implementation starts._ --- ### Modal URL & Navigation - **Modal close URL** — After closing the modal via backdrop click, should the URL update from `/galerie?id=X` back to `/galerie`? Option (a): `history.pushState` client-side — clean URL, requires JS. Option (b): do nothing — URL retains `?id=X`, refresh re-opens the modal. Option (b) is simpler and acceptable; option (a) is cleaner UX. _(Raised by: Markus Keller)_ - **No-JS modal rendering mode** — When `/galerie?id=5` is loaded without JS, `selectedArtikel` is server-rendered. Should it appear as: (a) article details at the top of the gallery; (b) a static HTML sheet; (c) nothing — `?id=` is only a post-redirect signal, no-JS users just see the gallery. Option (c) is simplest and defensible (cookie-based auth implies a JS-capable browser in practice). _(Raised by: Elicit)_ --- ### Photo Navigation - **Swipe gesture for photos** — The spec requires `touch-action: pan-x` swipe for photo navigation. Options: (a) implement pointer event drag (lightweight, no library, moderate complexity); (b) skip swipe — only use ‹ / › prev/next buttons. The acceptance criteria explicitly lists ‹ / › buttons; swipe is an enhancement. Confirm whether swipe is a hard requirement or a nice-to-have before coding starts. _(Raised by: Felix Brandt)_ - **Swipe-to-close the bottom sheet** — The spec describes "Swipe-Down schließt Modal" but the acceptance criteria only lists "Backdrop click closes modal." Options: (a) swipe handle is decorative — close only via backdrop click (simpler); (b) implement drag-to-dismiss with a 100px downward threshold. Confirm which is required. _(Raised by: Leonie Voss)_ --- ### Prev/Next Buttons Visibility - **When to show ‹ / › navigation buttons** — The acceptance criteria adds ‹ / › buttons not present in the spec mockup. Should they: (a) always render; (b) only render when `fotos.length > 1` (same rule as the counter). Option (b) is consistent with the spec's "hide counter at 1 photo" note and avoids rendering non-functional buttons. _(Raised by: Elicit)_

marcel referenced this issue

2026-05-05 11:29:06 +02:00

Build Galerie — responsive article grid with category filter #6

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/wannsee-kram#7

Build Artikel-Modal with reservation and cancel actions #7

Task 7 — Plan reference: docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md

Acceptance criteria

Files to create/modify

👤 Markus Keller — Application Architect

Observations

Recommendations

Open Decisions

👤 Felix Brandt — Fullstack Developer

Observations

Recommendations

Open Decisions

👤 Nora "NullX" Steiner — Security Engineer

Observations

Recommendations

Open Decisions

👤 Sara Holt — QA Engineer

Observations

Recommendations

Open Decisions

👤 Leonie Voss — UI/UX Design Lead

Observations

Recommendations

Open Decisions

👤 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions

🗳️ Decision Queue — Action Required

Modal URL & Navigation

Photo Navigation

Prev/Next Buttons Visibility

Task 7 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md`