feat(recipes): B1 — Recipe Library page with search and effort filtering #36

Merged
marcel merged 6 commits from feat/issue-22-recipe-library into master 2026-04-03 09:53:39 +02:00
Owner

Summary

  • Adds RecipeCard component with hero image/placeholder, name, cookTime, effort
  • Adds FilterChipRow for effort filtering (Alle/Leicht/Mittel/Schwer) with aria-pressed accessibility
  • Adds RecipeGrid with 2-col mobile / 4-col desktop layout and empty state
  • Adds +page.server.ts fetching recipes from GET /v1/recipes
  • Adds +page.svelte with client-side search + effort filtering, link to add recipe

Closes #22

Test plan

  • 260 unit tests passing
  • npm run check — 0 errors, 0 warnings
  • All 5 components have dedicated test files
  • Effort chip filter maps labels (Leicht/Mittel/Schwer) to API values (Easy/Medium/Hard)

🤖 Generated with Claude Code

## Summary - Adds `RecipeCard` component with hero image/placeholder, name, cookTime, effort - Adds `FilterChipRow` for effort filtering (Alle/Leicht/Mittel/Schwer) with aria-pressed accessibility - Adds `RecipeGrid` with 2-col mobile / 4-col desktop layout and empty state - Adds `+page.server.ts` fetching recipes from `GET /v1/recipes` - Adds `+page.svelte` with client-side search + effort filtering, link to add recipe Closes #22 ## Test plan - [ ] 260 unit tests passing - [ ] `npm run check` — 0 errors, 0 warnings - [ ] All 5 components have dedicated test files - [ ] Effort chip filter maps labels (Leicht/Mittel/Schwer) to API values (Easy/Medium/Hard) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
marcel added 5 commits 2026-04-03 09:50:03 +02:00
Author
Owner

👨‍💻 Kai — Frontend Engineer

Verdict: 🚫 Changes requested

Blockers

RecipeSummary type defined in three places (RecipeCard.svelte, RecipeGrid.svelte, +page.svelte)
These three files each independently define an identical local type:

type RecipeSummary = { id: string; name: string; cookTimeMin?: number; effort?: string; heroImageUrl?: string }

This will diverge the moment any field changes. Extract it to a shared module ($lib/recipes/types.ts) or derive it from the OpenAPI-generated types. Keeping three copies is a maintenance time-bomb.

+page.svelte "Rezept hinzufügen" link uses Skeleton UI classes

<a href="/recipes/new" class="btn variant-filled-primary">Rezept hinzufügen</a>

btn and variant-filled-primary are Skeleton UI utility classes — we don't use Skeleton. This will render as an unstyled text link. The RecipeGrid.svelte empty-state link shows the correct pattern: font-sans text-[13px] font-medium tracking-[0.04em] rounded-[var(--radius-md)] bg-[var(--green-dark)] px-[24px] py-[12px] text-white. Use that.

Suggestions

Unused imports in page.test.tsvi, beforeEach, afterEach are imported but not used. Clean these up.

effortMap in +page.svelte — the mapping lives only in the page, while FilterChipRow just emits raw label strings. If effort labels ever change, the mapping breaks silently. Consider co-locating the chips array with their API values inside FilterChipRow, or exporting the map from a shared constants file so the page imports it instead of defining it inline.

## 👨‍💻 Kai — Frontend Engineer **Verdict: 🚫 Changes requested** ### Blockers **`RecipeSummary` type defined in three places** (`RecipeCard.svelte`, `RecipeGrid.svelte`, `+page.svelte`) These three files each independently define an identical local type: ```ts type RecipeSummary = { id: string; name: string; cookTimeMin?: number; effort?: string; heroImageUrl?: string } ``` This will diverge the moment any field changes. Extract it to a shared module (`$lib/recipes/types.ts`) or derive it from the OpenAPI-generated types. Keeping three copies is a maintenance time-bomb. **`+page.svelte` "Rezept hinzufügen" link uses Skeleton UI classes** ```html <a href="/recipes/new" class="btn variant-filled-primary">Rezept hinzufügen</a> ``` `btn` and `variant-filled-primary` are Skeleton UI utility classes — we don't use Skeleton. This will render as an unstyled text link. The `RecipeGrid.svelte` empty-state link shows the correct pattern: `font-sans text-[13px] font-medium tracking-[0.04em] rounded-[var(--radius-md)] bg-[var(--green-dark)] px-[24px] py-[12px] text-white`. Use that. ### Suggestions **Unused imports in `page.test.ts`** — `vi`, `beforeEach`, `afterEach` are imported but not used. Clean these up. **`effortMap` in `+page.svelte`** — the mapping lives only in the page, while `FilterChipRow` just emits raw label strings. If effort labels ever change, the mapping breaks silently. Consider co-locating the chips array with their API values inside `FilterChipRow`, or exporting the map from a shared constants file so the page imports it instead of defining it inline.
Author
Owner

🎨 Atlas — UI/UX Designer

Verdict: 🚫 Changes requested

Blockers

+page.svelte header link broken styling
The "Rezept hinzufügen" link in the page header uses class="btn variant-filled-primary" — not design system tokens. This renders as an unstyled link. The empty-state link in RecipeGrid.svelte gets it right:

font-sans text-[13px] font-medium tracking-[0.04em] rounded-[var(--radius-md)] bg-[var(--green-dark)] px-[24px] py-[12px] text-white

Apply the same treatment to the header link.

FilterChipRow.svelte font size violates design system
Chips use text-[11px]:

class="font-sans text-[11px] font-medium ..."

The design system rule is non-negotiable: all button-role elements use text-[13px]. These chips are <button> elements — the rule applies. Change to text-[13px].

Suggestions

<h1> in +page.svelte doesn't use display font

<h1 class="text-2xl font-medium">Rezepte</h1>

Page headings should use font-[var(--font-display)] (Fraunces). Recommend: font-[var(--font-display)] text-[28px] font-medium text-[var(--color-text)]. The text-2xl Tailwind utility equals 24px which is close but doesn't use the correct font family.

RecipeCard metadata text text-[11px]
cookTimeMin · effort is rendered at 11px in RecipeCard.svelte. This is secondary metadata, not a button, so the 13px button rule doesn't strictly apply — but 11px is below 12px which is generally considered the minimum for readable body text. Consider text-[12px].

## 🎨 Atlas — UI/UX Designer **Verdict: 🚫 Changes requested** ### Blockers **`+page.svelte` header link broken styling** The "Rezept hinzufügen" link in the page header uses `class="btn variant-filled-primary"` — not design system tokens. This renders as an unstyled link. The empty-state link in `RecipeGrid.svelte` gets it right: ``` font-sans text-[13px] font-medium tracking-[0.04em] rounded-[var(--radius-md)] bg-[var(--green-dark)] px-[24px] py-[12px] text-white ``` Apply the same treatment to the header link. **`FilterChipRow.svelte` font size violates design system** Chips use `text-[11px]`: ```html class="font-sans text-[11px] font-medium ..." ``` The design system rule is non-negotiable: **all button-role elements use `text-[13px]`**. These chips are `<button>` elements — the rule applies. Change to `text-[13px]`. ### Suggestions **`<h1>` in `+page.svelte` doesn't use display font** ```html <h1 class="text-2xl font-medium">Rezepte</h1> ``` Page headings should use `font-[var(--font-display)]` (Fraunces). Recommend: `font-[var(--font-display)] text-[28px] font-medium text-[var(--color-text)]`. The `text-2xl` Tailwind utility equals 24px which is close but doesn't use the correct font family. **RecipeCard metadata text `text-[11px]`** `cookTimeMin · effort` is rendered at 11px in `RecipeCard.svelte`. This is secondary metadata, not a button, so the 13px button rule doesn't strictly apply — but 11px is below 12px which is generally considered the minimum for readable body text. Consider `text-[12px]`.
Author
Owner

🧪 QA Engineer

Verdict: ⚠️ Approved with concerns

Coverage Summary

Overall test coverage is solid — all five new components have dedicated test files following the right patterns (query by role/text/label, userEvent for interactions). The TDD structure is evident. Good work.

Concerns (non-blocking but should be addressed)

Missing: combined search + effort filter test
There's a test for search-only and a test for effort-chip-only, but no test for applying both simultaneously. This is a real user interaction (filter by "Leicht" then search within results). The $derived chain handles it, but it's an untested code path. Example:

it('applies search and effort filter together', async () => {
  await user.click(screen.getByRole('button', { name: 'Leicht' }));
  await user.type(searchInput, 'Gemüse');
  expect(screen.getByText('Gemüsesuppe')).toBeInTheDocument();
  expect(screen.queryByText('Spaghetti Bolognese')).not.toBeInTheDocument();
});

Missing: test for clearing effort filter back to "Alle"
There's no test that clicking "Alle" after an active filter resets to showing all recipes. This validates the toggle/reset UX.

page.test.ts unused imports
vi, beforeEach, and afterEach are imported but not used. Minor, but test files should be kept clean — they serve as documentation.

page.server.test.ts uses load: any
Type assertion let load: any suppresses TypeScript safety on the test. This is a known pattern for dynamic imports in Vitest but worth calling out — if the load function signature changes, the test won't catch it through types.

What's Good

  • All components tested from user perspective (role/text/label selectors, no CSS class snooping — except the necessary grid layout and styling tests)
  • userEvent.setup() used throughout (not fireEvent)
  • Mocking pattern in page.server.test.ts is clean: vi.resetModules() + dynamic import ensures isolation
  • Empty state tested both ways (no recipes at all vs. no matches after filter)
## 🧪 QA Engineer **Verdict: ⚠️ Approved with concerns** ### Coverage Summary Overall test coverage is solid — all five new components have dedicated test files following the right patterns (query by role/text/label, `userEvent` for interactions). The TDD structure is evident. Good work. ### Concerns (non-blocking but should be addressed) **Missing: combined search + effort filter test** There's a test for search-only and a test for effort-chip-only, but no test for applying *both* simultaneously. This is a real user interaction (filter by "Leicht" then search within results). The `$derived` chain handles it, but it's an untested code path. Example: ```ts it('applies search and effort filter together', async () => { await user.click(screen.getByRole('button', { name: 'Leicht' })); await user.type(searchInput, 'Gemüse'); expect(screen.getByText('Gemüsesuppe')).toBeInTheDocument(); expect(screen.queryByText('Spaghetti Bolognese')).not.toBeInTheDocument(); }); ``` **Missing: test for clearing effort filter back to "Alle"** There's no test that clicking "Alle" after an active filter resets to showing all recipes. This validates the toggle/reset UX. **`page.test.ts` unused imports** `vi`, `beforeEach`, and `afterEach` are imported but not used. Minor, but test files should be kept clean — they serve as documentation. **`page.server.test.ts` uses `load: any`** Type assertion `let load: any` suppresses TypeScript safety on the test. This is a known pattern for dynamic imports in Vitest but worth calling out — if the load function signature changes, the test won't catch it through types. ### What's Good - All components tested from user perspective (role/text/label selectors, no CSS class snooping — except the necessary grid layout and styling tests) - `userEvent.setup()` used throughout (not `fireEvent`) ✅ - Mocking pattern in `page.server.test.ts` is clean: `vi.resetModules()` + dynamic import ensures isolation ✅ - Empty state tested both ways (no recipes at all vs. no matches after filter) ✅
Author
Owner

🔒 Sable — Security Engineer

Verdict: Approved

Checked this PR against the OWASP Top 10 and project-specific threat model. No blockers.

What I Verified

Access control: The recipe library page is read-only and serves data for authenticated household members. Auth is centralized in hooks.server.ts — the load function correctly does not duplicate auth logic.

Injection / XSS: No {@html} usage anywhere in the new components. Recipe name, cookTimeMin, and effort values are rendered as text interpolations (auto-escaped by Svelte).

heroImageUrl in RecipeCard: The <img src={recipe.heroImageUrl}> renders an image from a backend-provided URL. The backend controls this value (stored via API, not directly from user input at render time), so SSRF and javascript: scheme injection are the backend's responsibility to prevent at write time. Acceptable.

Information leakage: The load function returns { recipes: [] } on any API error — no status codes, stack traces, or internal error messages leak to the client.

No client-side filtering secrets: The effortMap and search logic in +page.svelte operate entirely on data already delivered to the client. No server-side bypass concern.

Multi-tenancy: The GET /v1/recipes call uses SvelteKit's server-side fetch which carries the session cookie. The backend enforces household isolation — the frontend is not responsible for that here.

## 🔒 Sable — Security Engineer **Verdict: ✅ Approved** Checked this PR against the OWASP Top 10 and project-specific threat model. No blockers. ### What I Verified **Access control**: The recipe library page is read-only and serves data for authenticated household members. Auth is centralized in `hooks.server.ts` — the load function correctly does not duplicate auth logic. ✅ **Injection / XSS**: No `{@html}` usage anywhere in the new components. Recipe name, cookTimeMin, and effort values are rendered as text interpolations (auto-escaped by Svelte). ✅ **`heroImageUrl` in RecipeCard**: The `<img src={recipe.heroImageUrl}>` renders an image from a backend-provided URL. The backend controls this value (stored via API, not directly from user input at render time), so SSRF and `javascript:` scheme injection are the backend's responsibility to prevent at write time. Acceptable. ✅ **Information leakage**: The load function returns `{ recipes: [] }` on any API error — no status codes, stack traces, or internal error messages leak to the client. ✅ **No client-side filtering secrets**: The `effortMap` and search logic in `+page.svelte` operate entirely on data already delivered to the client. No server-side bypass concern. ✅ **Multi-tenancy**: The `GET /v1/recipes` call uses SvelteKit's server-side `fetch` which carries the session cookie. The backend enforces household isolation — the frontend is not responsible for that here. ✅
Author
Owner

🖥️ Backend Engineer

Verdict: Approved

This PR is frontend-only and I have limited concerns from the backend perspective. Here's what I checked:

What Looks Good

+page.server.ts is clean and idiomatic

const { data, error } = await api.GET('/v1/recipes', {});
if (error || !data?.data) return { recipes: [] };

The load function is thin — it fetches, maps, and returns. No business logic, no leaking internal API shapes. The defensive fallback to { recipes: [] } on error is correct for a read-only page.

Server-side data fetching: All recipe data is loaded server-side via +page.server.ts using the authenticated fetch. No client-side API calls, no exposed API keys.

Field mapping is explicit: Only the fields actually needed by the UI are mapped — id, name, cookTimeMin, effort, heroImageUrl. Internal fields from the API response (if any) are stripped.

Observation

Client-side filtering vs server-side filtering
GET /v1/recipes is called with no query params ({}), and search/effort filtering is done client-side in $derived. This is fine for small datasets. If the recipe count grows significantly, the backend already supports search and effort query params — migrating filtering server-side later would be a clean lift. No action needed now, just noting the tradeoff is understood.

## 🖥️ Backend Engineer **Verdict: ✅ Approved** This PR is frontend-only and I have limited concerns from the backend perspective. Here's what I checked: ### What Looks Good **`+page.server.ts` is clean and idiomatic** ```ts const { data, error } = await api.GET('/v1/recipes', {}); if (error || !data?.data) return { recipes: [] }; ``` The load function is thin — it fetches, maps, and returns. No business logic, no leaking internal API shapes. The defensive fallback to `{ recipes: [] }` on error is correct for a read-only page. ✅ **Server-side data fetching**: All recipe data is loaded server-side via `+page.server.ts` using the authenticated `fetch`. No client-side API calls, no exposed API keys. ✅ **Field mapping is explicit**: Only the fields actually needed by the UI are mapped — `id`, `name`, `cookTimeMin`, `effort`, `heroImageUrl`. Internal fields from the API response (if any) are stripped. ✅ ### Observation **Client-side filtering vs server-side filtering** `GET /v1/recipes` is called with no query params (`{}`), and search/effort filtering is done client-side in `$derived`. This is fine for small datasets. If the recipe count grows significantly, the backend already supports `search` and `effort` query params — migrating filtering server-side later would be a clean lift. No action needed now, just noting the tradeoff is understood.
marcel added 1 commit 2026-04-03 09:53:36 +02:00
- Extract RecipeSummary type to $lib/recipes/types.ts (was duplicated in 3 files)
- Fix +page.svelte header link: replace Skeleton UI classes with design system tokens
- Fix h1: use font-[var(--font-display)] and correct size
- Fix FilterChipRow: text-[11px] → text-[13px] + tracking-[0.04em] per design system
- Fix RecipeCard metadata: text-[11px] → text-[12px] for readability
- Remove unused imports (vi, beforeEach, afterEach) from page.test.ts
- Add combined search + effort filter test
- Add reset-to-Alle filter test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
marcel merged commit a34c6f30f2 into master 2026-04-03 09:53:39 +02:00
marcel deleted branch feat/issue-22-recipe-library 2026-04-03 09:53:40 +02:00
Sign in to join this conversation.