feat(settings): implement /settings hub page (E1) — Kachel-Ansicht #55
Reference in New Issue
Block a user
Delete Branch "feat/issue-49-settings-kachel-hub"
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 #49
Summary
/settingshub page — three cards (Vorräte, Haushalt, Profil) in arepeat(2, 1fr)grid, SSR-only, no client-side state+page.server.ts— parallel calls toGET /v1/ingredients(stapleCount) andGET /v1/households/mine(memberCount); fallback to 0 on API errorSettingsCard.svelte— reusable card component (title, href, cta, meta, accent props)(app)/household/staples/— AppShell sidebar/nav now present; onboarding usesposition: fixed; inset: 0overlay to cover AppShell chromectx=settingsremoved — default view is settings; onlyctx=onboardingchanges behaviourNavItem.extraPathsadded; "Einstellungen" is active on/household/staplesin both DesktopSidebar and MobileTabBar--radius-xl,p-[28px], display font H1 28px, stat number 28px weight-300, content padding40px 56pxdesktopCommits
feat(settings): add +page.server.ts loading stapleCount, memberCount, userNamefeat(settings): add SettingsCard componentfeat(settings): implement settings hub page with three cardsfeat(settings): add ← Einstellungen back-link on D3 staples pagefeat(settings): add autosave hint text on D3feat(nav): add extraPaths to NavItemfeat(nav): pass extraPaths in DesktopSidebar and MobileTabBarrefactor(staples): move household/staples route into (app) groupfix(settings): align hub page with V2 spec stylesrefactor(staples): remove ctx=settingsfix(settings): remove green left border from Vorräte cardTest plan
npm run test)/settingsshows three equal cards, 2-col grid on desktop, 1-col on mobile/household/stapleshas sidebar nav (Einstellungen active)/household/staples?ctx=onboardingshows full-screen onboarding wizard (no sidebar)/settingsand/household/staples👨💻 Kai — Frontend Engineer
Verdict: ⚠️ Approved with concerns
Blockers
Redundant
{#if !isOnboarding}guards in the else block (frontend/src/routes/(app)/household/staples/+page.svelte)The outer
{:else}already meansisOnboardingis false. The inner{#if !isOnboarding}guards around the back-link and hint text are always-true dead conditions:These should just be unconditional. Not a runtime bug, but misleading and the dead branch can't be reached by tests either.
Suggestions
SettingsCard.accentprop is dead code (frontend/src/lib/components/SettingsCard.svelte:17)The
accentprop and itsstyle="border-left: 3px solid var(--green-dark)"are never used on the settings hub. If this was removed intentionally after the design decision to drop the green border, the prop and the inline style branch should be deleted too. Keeping it invites someone to accidentally use it later.Style duplication across three card instances (
+page.svelte,SettingsCard.svelte)The Vorräte and Haushalt cards duplicate the full card class string from SettingsCard (hover styles, transitions, focus-visible ring). If the card hover style changes, it has to be updated in three places. Consider passing a snippet to SettingsCard for the stat block, or extracting a shared CSS class via
@apply. For now it's fine, but worth flagging before this pattern spreads to more cards.Svelte 5 runes usage — all correct.
$props(),$derived()used properly. No legacyexport letor$:reactive statements. Good.🧪 QA Engineer — Test Coverage Review
Verdict: ⚠️ Approved with concerns
Blockers
isActiveRoutewithextraPathshas no tests (frontend/src/lib/nav/nav.ts)The
extraPathsfeature was added toisActiveRouteand wired into both nav components, but there are no tests for it. This is new branching logic that could silently break. The function is pure, so tests are trivial to write:isActiveRoute('/settings', '/household/staples', ['/household/staples'])→trueisActiveRoute('/settings', '/household/staples/edit', ['/household/staples'])→true(prefix match)isActiveRoute('/settings', '/household/other', ['/household/staples'])→falseisActiveRoute('/settings', '/household/staples', undefined)→false(no extraPaths)SettingsCard.sveltehas no component testsThe generic
SettingsCardcomponent is used for the Profil card and is a reusable building block, but it has no test file. At minimum: renders title, href, cta, meta when provided; skips meta block when prop is absent; renders data-accent attribute whenaccent=true.Suggestions
Staples page — API error fallback not tested (
frontend/src/routes/(app)/settings/+page.server.ts)stapleCountandmemberCountfall back to0when the API fails (via?? 0). This is a reasonable UX choice, but there's no test that exercises the fallback path. Consider a server load unit test that mocks a failed API response and asserts the returned values are0.Good coverage overall:
settings/page.test.ts: all 10 scenarios covered includingstapleCount=0empty state and both CTA variants ✅household/staples/page.test.ts: 12 scenarios across both contexts, including the back-link, hint text, and onboarding nav ✅data-testidplacements onstaple-countandmember-countmake the tests stable and behavior-focused ✅🏗️ Backend Engineer — Server Load & Architecture Review
Verdict: ✅ Approved
Observations
locals.benutzer!.namenon-null assertion (frontend/src/routes/(app)/settings/+page.server.ts:18)The bang operator assumes
benutzeris always set. This is only safe ifhooks.server.tsunconditionally guards all(app)routes and redirects unauthenticated users before any page load runs. If that guarantee exists (which it should), this is fine. If that guard is ever relaxed or the route moves outside(app), this crashes with a null dereference instead of a graceful redirect.Consider
locals.benutzer?.name ?? ''as a more defensive fallback, or at minimum leave a comment:householdRes.data?.data?.members?.length ?? 0— the double.datapath (data?.data) is a smell from the OpenAPI wrapper'sApiResponse<T>envelope leaking into the load function. The type chain is technically correct but fragile to read. This is a pre-existing API client design issue, not introduced in this PR. Worth noting for a future clean-up.Parallel
Promise.allfor the two API calls — correct and efficient. No sequential dependency, no reason to await them separately. ✅Silent fallback to 0 on API failure — returning
0when ingredient or household APIs fail is a reasonable UX choice for a summary/hub page. The settings page still renders and the user isn't blocked. Acceptable trade-off. ✅No server-side mutations — this is a pure read-only load function with no side effects. No transaction concerns. ✅
🎨 Atlas — UI/UX & Design System Review
Verdict: ⚠️ Approved with concerns
Blockers
Hardcoded hex
#C0BFB8in hover state — appears in all three card definitions:settings/+page.svelteVorräte card (line ~22)settings/+page.svelteHaushalt card (line ~51)SettingsCard.svelte(line 16)hover:border-[#C0BFB8]should be a semantic token. This color sits between--color-borderand--color-text-mutedon the neutral scale. Either add a--color-border-hovertoken to the design system or use the closest existing token. Hardcoded hex values in components break the theming contract.SettingsCard.accentdead prop with inline style (SettingsCard.svelte:17)This feature was removed from the design (correctly — there's no active state on the hub page), but the code remains. Dead design system surface should be removed. Every unused prop is a question future devs will ask.
Observations — all correct ✅
--radius-xlfor card border radius ✅p-[28px]card padding ✅font-[var(--font-display)] text-[28px] font-medium tracking-[-0.02em]— matches V2 spec ✅text-[16px] font-medium✅text-[13px] text-[var(--color-text-muted)]✅text-[12px] font-medium text-[var(--green-dark)]✅font-[var(--font-display)] text-[28px] font-light leading-[1] tracking-[-0.02em] text-[var(--green-dark)]— nice use of display font for the count ✅shadow-[var(--shadow-card)]andhover:shadow-[var(--shadow-raised)]— correct elevation transitions ✅focus-visible:outline-[var(--green-dark)]— keyboard accessibility maintained ✅p-[16px_20px] md:p-[40px_56px]✅grid-cols-1 sm:grid-cols-2 gap-4 max-w-[820px]✅Overall the V2 spec translation is accurate. Fix the hardcoded hex and clean up the dead prop.
🔒 Sable — Security Review
Verdict: ✅ Approved
Observations
locals.benutzer!.name— non-null assertion (settings/+page.server.ts:18)Not a security issue per se, but the
!bypasses TypeScript's null check. Ifhooks.server.tsever fails to redirect an unauthenticated request before this load function runs, the process crashes with an unhandled exception rather than issuing a clean 401. The crash itself doesn't leak data, but a misconfigured hook could in theory expose the settings page to an unauthenticated user with a different failure mode. Confirm that(app)/+layout.server.tsorhooks.server.tshard-redirects before any load in this group executes.Route group move:
/household/staplesnow inside(app)— positive change. The staples page was previously outside the(app)route group, meaning it may have been served without the AppShell authentication guards. Moving it inside ensures the same session validation middleware applies. ✅No
{@html}usage —userName,stapleCount, andmemberCountare all rendered as text content, not raw HTML. No XSS surface. ✅Data scoping —
api.GET('/v1/ingredients')andapi.GET('/v1/households/mine')are called with the server-side fetch that carries the user's session cookie. Data is scoped to the authenticated user's household. No IDOR risk introduced here. ✅No secrets in load return — the serialized page data contains
stapleCount,memberCount, anduserNameonly. No session tokens, password hashes, or internal identifiers exposed to the client. ✅No form actions — this is a read-only page. No CSRF surface to audit. ✅
The outer {:else} already guarantees isOnboarding is false — inner guards were always-true dead conditions unreachable by tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>Review concerns addressed
All reviewer blockers and accepted suggestions resolved in 4 commits:
{#if !isOnboarding}guards in staples{:else}blockaf27564accentprop + inline style removed fromSettingsCard.svelte; stale test data cleaned up98c8aa9--color-border-hover: #c0bfb8token added to design system;hover:border-[#C0BFB8]replaced in all 3 card definitionsd66120blocals.benutzer!.namenon-null assertion5904102733/733 tests green. Type errors in
recipes/andplanner/files are pre-existing and unrelated to this PR.