feat(planner): desktop redesign — flip tiles, full-width grid, no right panel #54
Reference in New Issue
Block a user
Delete Branch "feat/issue-52-planner-flip-tiles"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #52
Summary
RecipePickerDrawer) instead of a right panel+ Gericht wählenCTA and optional reasoning tags (lazy — only rendered for the focused suggestion)heroImageUrlused when availableKey decisions
SlotRecipeDTO carries no ingredient data; not needed per issue clarification/v1/recipes—SlotRecipehas no tags; server load now mapstags: TagItem[]from the recipes listCommits
feat(planner): add CSS design tokens for protein/cuisine gradients and dimmingfeat(planner): add reasoningTags pure helper with testsfeat(planner): add EmptyDayTile component with reasoning tagsfeat(planner): add DesktopDayTile flip-card componentfeat(planner): add RecipePickerDrawer slide-in componentfeat(planner): wire desktop redesign — full-width grid, flip tiles, drawer👨💻 Kai — Senior Frontend Engineer
Verdict: ⚠️ Approved with concerns
Great refactor overall. The 2-panel flip-tile layout is cleaner than the previous 3-panel state machine, and the component split is well-reasoned. A few things need attention before this ships.
Blockers
1. Duplicate
TagItem/SlotRecipeinterface definitions across filesDesktopDayTile.svelte,EmptyDayTile.svelte, andreasoningTags.tseach declare their own localTagItem,SlotRecipe,Slotinterfaces.types.tswas updated to exportTagItem— but none of the new components import it. This is the classic "define once, import everywhere" failure. If the shape changes again, three files drift.Fix: import
TagItemand the slot-related interfaces from$lib/planner/types.tsin all three files.2.
gradientBackgrounduses rawurl(...)with an unencoded user-controlled stringIn
DesktopDayTile.svelte:When
heroImageUrlis present, the value becomesurl(${slot.recipe.heroImageUrl}). This URL comes from the API (ultimately from the database). If it contains special CSS characters (parentheses, quotes, whitespace), the inline style is malformed and the tile breaks visually. At minimum, URI-encode or CSS-escape the URL. This is also the entry point for the security concern Sable will raise.3.
$effectfor keyboard handler runs on every render cycle — missing dep guardThis is correct Svelte 5 usage (cleanup returned) — but the effect reads
drawerOpenandactiveSlotIdfrom the outer scope, so it will re-register/de-register the listener on every state change to those variables. The teardown+re-register is harmless but wasteful. Consider using a stablehandleKeydownreference outside the effect, or restructure so the effect body only registers once and reads state inside the handler closure. Not a hard blocker, but it's a rough edge.Suggestions
EmptyDayTileuses inlinestyle=strings for almost all layout. The rest of the app uses Tailwind utilities. Consider extracting at least the structural layout (flex flex-col h-fulletc.) to Tailwind and keeping only the CSS-variable-dependent values in inline styles. Mixing styles two ways in one file makes it harder to scan.DesktopDayTile.svelteis 437 lines. The CSS block alone is ~180 lines. Consider extracting aDesktopDayTileFront.svelteandDesktopDayTileBack.svelteif this grows further. For now it's acceptable, just watch the line count.The
Suggestioninterface insideDesktopDayTile.svelteusesrecipe: any. Giventypes.tsexports a properRecipetype, this should berecipe: Recipe.activePickerDatein+page.sveltestill checksdrawerOpen && drawerSlotIdtwice — once in the$derivedand once inhandleRecipePick. A single getter function would be cleaner.The drawer mounts
RecipePickeronly whenopenis true ({#if open}) — good pattern to avoid duplicate text in the DOM. But it means every open loses the scroll position. For a short list this is fine; document the tradeoff in a comment.🔒 Sable — Security Engineer
Verdict: ⚠️ Approved with concerns
I've checked this PR against the OWASP Top 10 for this stack. No critical auth/authz issues in the new code, but there is one injection-adjacent concern and one data-exposure issue that need attention.
Blockers
1. CSS injection via
heroImageUrlin inline style bindingIn
DesktopDayTile.svelte, thegradientBackgroundderived value builds a raw CSSurl()string from API data:This is then injected directly into an inline
style=attribute:Svelte escapes HTML attributes, but CSS injection via
style=is not prevented by Svelte's escaping. A malformed or attacker-controlledheroImageUrllike);}body{display:none}/*can break out of theurl()context and inject arbitrary CSS rules into the element's style attribute. SinceheroImageUrlultimately comes from user-uploaded data stored in the DB, this is a real attack surface.Severity: Medium (CSS injection, not JS injection — but can be used for UI redressing/clickjacking on the planner page).
Fix: sanitize the URL before embedding it in CSS. At minimum:
Or validate on the server that
heroImageUrlis always a well-formedhttps://URL before it reaches the DB.2.
tagsmapping uses(t: any)in+page.server.tsThis is a server-side load function. The
anycast means there's no validation thatt.nameort.tagTypeare strings (vs. something injected by a malicious API response). If the backend API is ever compromised or the response is proxied/replayed, untyped fields pass through to the frontend unvalidated. This is low severity given it's your own backend, but theanycast should be replaced with proper typing using the API response schema.Suggestions
The
RecipePickerDrawerbackdropdivusesonclickfor close but hasaria-hidden="true". This is correct accessibility-wise (the interaction is duplicated on the close button), but ensure a keyboard Escape path also closes it — which is handled in the$effect. Good.gradientBackgroundfalls back tovar(--color-surface)when no image and no tag match — safe default, no injection risk in that path.No new API endpoints introduced, no new auth surface, no new cookies. Auth/authz scope is unchanged from the existing planner page.
No
{@html}usage in any new component. Clean.No secrets or credentials in any changed file.
🎨 Atlas — UI/UX Designer
Verdict: ⚠️ Approved with concerns
The visual direction is exciting — full-height gradient tiles with CSS 3D flip is a substantial upgrade from the previous flat card-in-panel layout. I've checked the new components against the design system constraints.
Blockers
1. Hard-coded
border-radius: 10pxviolates design token contractThe design system specifies
--radius-lg: 10pxas the named token for 10px radius. BothDesktopDayTile.svelteandEmptyDayTile.svelteuse literalborder-radius: 10pxinstead ofvar(--radius-lg). If the radius token changes during a design refresh, these tiles won't update.Files:
DesktopDayTile.svelte(.scene,.card-front-inner,.card-back-inner),EmptyDayTile.svelte(inline style on the root div).Fix: replace all
border-radius: 10pxwithborder-radius: var(--radius-lg).2.
.scene-dimmedusesopacity: 0.42— not the--opacity-dimmedtoken just addedThe PR adds
--opacity-dimmed: 0.38toapp.cssas a design token, but.scene-dimmedinDesktopDayTile.svelteappliesopacity: 0.42. These should match. The token should be used:3.
.scene-todayring referencesvar(--yellow)— raw scale token, not semantic.scene-todayusesvar(--yellow)directly. Per the design system, component-level ring state should use the semantic token--color-ring-today(which the PR defines asvar(--yellow-text)). Similarly.dn-todaybackground usesvar(--yellow). This should bevar(--color-ring-today)to stay within the semantic layer.Suggestions
tile-namefont-size is19px— not on the standard type scale. Closest is18pxor20px. At 19px this is a one-off. Consider18pxor bump to20pxwithfont-weight: 300to maintain the light feel.back-nameusesfont-family: var(--font-display)withfont-size: 13px. Display font (Fraunces) at 13px is very small for a display face — it loses its character. Consider usingvar(--font-sans)at that size, or bumping the display text to 15–16px where Fraunces reads well.btn-actionfont-size is11pxon the back face. Per design system: buttons are13pxalways. These are action buttons — they should be13px.back-metafont-size is10px. Minimum readable text is11pxin the design system. Consider bumping to11px.The gradient token naming (
--gradient-protein-haehnchen,--gradient-cuisine-deutschetc.) is good. I would add a note inapp.cssthat these are derived from German tag names viatoCssKey()so future maintainers understand the coupling to the backend tag vocabulary.EmptyDayTileday header usesfont-size: 9pxfor the day abbreviation. That's below the 11px minimum recommended for body text, though for supplemental labels it can work at high resolution. Consider10pxminimum.🧪 QA Engineer — Test Specialist
Verdict: ✅ Approved
This PR has solid test coverage across all new components and the pure helper. The TDD discipline is visible — tests are structured, behavior-focused, and query by role/text/testid rather than internals. Here's what I verified and what still has gaps.
What's well-covered
reasoningTags.ts: 100% path coverage. Happy path, boundary conditions (cookTimeMinat 29 vs. 30), no-protein case, empty slotMap, multiple tags returning together, and explicitly zero tags. Boundary tests atcookTimeMin: 29andcookTimeMin: 30are exactly right.DesktopDayTile.test.ts: Front face state (today ring, selected ring, dimmed, not-dimmed), flip interaction (click, Enter, Space), back face actions (Koch-Modus, Rezept ansehen, close, Gericht tauschen, Entfernen), planner vs. non-planner gating,aria-expandedstate. Good coverage.EmptyDayTile.test.ts: CTA show/hide by role, callback firing, reasoning tag rendering, recipe name display, and the "no tags for heavy recipe" negative case.RecipePickerDrawer.test.ts: Open/closed visibility,aria-hidden, backdrop click, close button, and recipe picking.Missing coverage (suggestions, not blockers)
1.
DesktopDayTile— no test for theonflipnot being called when tile is already flippedThe spec intent (clicking an already-active card) isn't explicitly tested.
handleFlipalways callsonflip?.(slotId)— including whenisFlippedis already true. Depending on the design intent, a second click should either close or be a no-op. This is currently delegated to the parent'shandleTileFlip, but there's no component-level test for "clicking a flipped tile calls onflip again."2.
DesktopDayTile— no test forEntfernenbeing hidden whenslot.idis nullThe component only shows the Entfernen button when
{#if slot.id}. The test fixturefilledSlotalways hasid: 's1'. There's no test case for a slot with a recipe but without an id (i.e., optimistic slot before server assignment). Low probability but worth a negative test.3.
RecipePickerDrawer— no test for Escape key closing the drawerThe Escape key handler lives in
+page.svelte, not in the drawer itself — so it's untestable at the drawer component level. But there's also no integration-level test for this keyboard flow. It's a documented UX feature that currently has zero test coverage.4.
+page.svelteintegration changes — zero test coverageThe
recipeByIdmerge,slotMapenrichment with tags,handleTileFlip/Close/Swap/Remove/EmptyTileAddhandlers, anddrawerReplacingMetaderived — none of these have page-level tests. This is the most complex logic change in the PR and relies entirely on the component tests cascading correctly. Suggest adding at least one integration test for the tag-enrichment flow (verify thatslotMapslots have tags fromdata.recipes).Minor notes
RecipePickerDrawer.test.tsuses{ ...baseProps, onclose }on each close test — thevi.fn()onbaseProps.oncloseis shared across tests unless reset. ConsiderbeforeEach(() => vi.clearAllMocks())or inlinevi.fn()per test for cleaner isolation.Tests use
userEvent.setup()per test — correct, not shared. Good.⚙️ Senior Backend Engineer
Verdict: ✅ Approved
The backend changes are small and focused. The shift from JPQL constructor expressions to full entity fetching with
LEFT JOIN FETCH r.tagsis the right approach when you need to include collection associations in the response.What I checked
RecipeRepository— query change from DTO projection to entity fetchThe old query used a JPQL constructor expression (
SELECT new RecipeSummaryResponse(...)) which can't navigate tor.tags(a collection). Switching toSELECT r FROM Recipe r LEFT JOIN FETCH r.tagscorrectly eager-loads the tags collection in a single query. This is the standard JPA pattern for this.One thing to verify: if a recipe has many tags (unlikely but possible), the LEFT JOIN FETCH still returns one row per tag, and JPA deduplicates in memory. With pagination via
limit/offsetapplied after the join, this can cause the offset to be applied to joined rows rather than entities — the classic "HHH90003004: firstResult/maxResults specified with collection fetch" problem. Check whether Hibernate logs any warnings about this in dev. The fix would be to either:@EntityGraphor a separate query for tags after pagination, orDISTINCTin the query and ensure your limit/offset are applied at the entity level.This is worth verifying before shipping to production at scale, though for small household-scoped recipe counts it won't manifest.
RecipeService— tag mappingClean. No business logic in the controller (checked: controller untouched). Mapping stays in the service layer. Good.
RecipeSummaryResponse—heroImagePreview→heroImageUrlThis is a field rename. If any other consumer uses
heroImagePreview, it breaks. The controller test is updated, which gives confidence this is the only reference. Worth a quick search in the codebase to confirm no other test or client usesheroImagePreview.RecipeControllerTestUpdated correctly — the test stub now includes
heroImageUrland tags in the mock response, and asserts ontags[0].nameandtags[0].tagType. Good coverage of the new response shape.No blockers
The changes are minimal, correctly layered, and tested. The one thing to watch is the pagination + JOIN FETCH behavior under load, but for this app's scale it's academic.
Extracts sanitizeForCssUrl helper that strips '"()\ before the URL is embedded in url("..."). Prevents CSS injection via the hero image field in inline style bindings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>Adds regression test for the {#if slot.id} guard on the remove button — QA flagged the missing negative test case for optimistic slots. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>✅ Review concerns addressed — 7 commits
All six reviewer concerns resolved. No tasks deferred.
Kai — Duplicate interface definitions
types.tsnow exportsSlotandSlotMap(66447a7)DesktopDayTile.svelteimportsRecipe,Slot,Suggestionfromtypes.ts— local copies removed (b0800ca)EmptyDayTile.svelteimportsSuggestion,SlotMapfromtypes.ts— local copies removed (a8a781f)reasoningTags.tsimportsRecipe,Slot,SlotMapfromtypes.ts— local copies removed (e5c361f)Kai + Sable — CSS injection via
heroImageUrlsanitizeForCssUrl()pure helper that strips'"()\before embedding inurl("...")(4c87d9c)Sable —
(t: any)in server load tag mapping(t: any)with(t: TagItem)in+page.server.ts(9423cd6)Atlas — hardcoded values vs design tokens
border-radius: 10px→var(--radius-lg)in both tile componentsopacity: 0.42→var(--opacity-dimmed)var(--yellow)→var(--color-ring-today)for today ring and date circle3f9bd2b)QA — missing test: Entfernen hidden when
slot.idis null{#if slot.id}guard (4835231)QA — shared
vi.fn()across tests in RecipePickerDrawerbeforeEach(() => vi.clearAllMocks())(cfbde18)Backend (Hibernate pagination + JOIN FETCH): Deferred — reviewer confirmed no blocker, academic for this app's recipe count scale.
Final: 698 tests pass, 0 new type errors.