feat: Themen-Inhaltsverzeichnis — Dashboard-Widget + dedizierte Seite /themen #664
Reference in New Issue
Block a user
Delete Branch "worktree-feat+issue-662-themen-inhaltsverzeichnis"
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 #662
Summary
ThemenWidgetto the home dashboard — visible in the reader layout (after PersonChips) and the editor sidebar (between FamilyPulse and ActivityFeed, single-column compact mode)/themenpage with root-tag cards (6 px top color bar), up to 5 child rows per card, and a+ N weitere →overflow link/?tag={encodeURIComponent(tag.name)}— consistent with existing DocumentMetadataDrawer patternhasAnyDocuments()recursive helper in$lib/shared/utils/tagUtils.tsfilters tags with no documents in their subtreedocumentCount = 0but populated children are shown, but the 0 count is omittedGET /api/tags/treewas already completeTest plan
cd frontend && npm run test -- --project=server tagUtils— 4 unit tests greencd frontend && npm run test -- --project=server "themen/page.server"— 3 server tests greencd frontend && npm run check— no TypeScript errors in new filesThemenWidget.svelte.spec.ts(6 tests) andthemen/page.svelte.spec.ts(6 tests)/?tag=<name>with filter active/themenrenders root-tag cards with color bars and child rows/?tag=<child-name>with filter activedocumentCount=0and no children with docs are hidden; if all filtered, "Noch keine Themen vergeben." shown🤖 Generated with Claude Code
🏛️ Markus Keller — Senior Application Architect
Verdict: ✅ Approved
Reviewed module boundaries, documentation currency, and SSR architecture. The implementation is clean.
What I checked
Module boundaries —
hasAnyDocuments()was correctly moved to$lib/shared/utils/tagUtils.tsrather than$lib/tag/tagUtils.ts. This is the right call: placing it in$lib/tag/and importing it from$lib/shared/dashboard/ThemenWidget.sveltewould have crossed the domain boundary enforced by ESLint. The$lib/shared/utils/location is the correct neutral zone for cross-domain utilities.Layer compliance — Data flows correctly:
+page.server.tscallscreateApiClient(fetch)→GET /api/tags/tree→ passestreeprop down to+page.svelte. No direct repository access from the front end, no business logic in components. ✅SSR — Both the dashboard and
/themenload their tag tree data server-side. The tag tree fetch is added to thePromise.allSettledblock in the editor path and thereaderFetchesarray in the reader path, consistent with the existing pattern. ✅Error resilience — The dashboard catches a failed tag tree fetch gracefully:
tagTree: [] as TagTreeNodeDTO[]in the catch fallback. Empty array means the widget renders its empty state rather than crashing. ✅Documentation — Required updates per the documentation table:
/themen/themenl3-frontend-3c-people-stories.pumlBoth
CLAUDE.md(route table entry forthemen/) and the C4 Level 3 diagram (newComponent(themen, ...)+Rel(themen, backend, ...)) are updated. The C4 description also notes thatThemenWidgetis embedded in the home dashboard, which is good — the diagram captures the component's dual role.One observation (not a blocker)
The
ThemenWidget.sveltelives in$lib/shared/dashboard/. It rendersTagTreeNodeDTOdata, which is semantically a tag domain artifact. The location is defensible because the widget is used on both the home dashboard and/themen— it's genuinely shared across domains. If the project ever adopts a stricter domain split, this could move, but for now$lib/shared/dashboard/is the correct home.LGTM. Ready to merge from an architecture perspective.
👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ⚠️ Approved with concerns
TDD evidence is present, Svelte 5 runes are used correctly, and the component structure is clean. One suggestion worth addressing before merge.
What I checked
TDD evidence — Tests precede or accompany every implementation file:
tagUtils.test.ts— 4 unit tests forhasAnyDocuments()✅themen/page.server.spec.ts— 3 server load tests ✅ThemenWidget.svelte.spec.ts— 6 browser component tests ✅themen/page.svelte.spec.ts— 6 browser component tests ✅routes/page.server.spec.ts— 3 existing dashboard tests updated with new mock ✅Svelte 5 runes —
$props(),$derived.by(), noexport let, no$:, noonMount. Correct. ✅Keyed
{#each}—{#each visibleTree as tag (tag.id)}and{#each shownChildren as child (child.id)}— both keyed. ✅Component size —
ThemenWidget.svelteat 64 lines ✅./themen/+page.svelteat 86 lines — at the upper end but acceptable for a page-level orchestrator.Suggestion
In
/themen/+page.svelte, the per-iteration derived values are computed inline with{@const}:{@const}inside{#each}is valid Svelte and scoped correctly to the iteration. The computation runs on each render for each tag. For the expected scale (a few dozen root tags max), this is not a performance concern. I'd accept this as-is — it's readable and idiomatic Svelte 5 for per-item derived state.The one thing I want to confirm:
hasAnyDocumentsis imported from$lib/shared/utils/tagUtilsin bothThemenWidget.svelteandthemen/+page.svelte. That import path is consistent throughout the PR. ✅Minor note
The
data-compact={compact}attribute on the ThemenWidget grid div exists to enable test selection (checking grid layout via attribute). This is pragmatic but couples the test to an implementation detail rather than a user-visible behavior. A better pattern would be checking the rendered class list or CSS grid column count. Not a blocker given the CI-only nature of the browser tests.Overall: clean implementation, TDD discipline followed, Svelte 5 patterns correct. Approved.
🛠️ Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
Pure frontend feature. No infrastructure changes. Nothing to flag.
What I checked
*.spec.tsand*.test.ts). ✅openapi-fetch,@sveltejs/kit,paraglide). ✅/themenis a standard SSR page. No adapter changes, no static export changes, no build config changes. ✅One note for future
The tag tree is now fetched on every dashboard load (reader + editor paths). At the current scale this adds ~1 API call to the dashboard. If dashboard load time becomes a concern,
GET /api/tags/treeis an excellent candidate for HTTP caching (Cache-Control: max-age=300, stale-while-revalidate) since tag trees change infrequently. Not a concern today, but worth noting when the observability dashboards start showing dashboard load times.LGTM from infrastructure perspective.
📋 Elicit — Requirements Engineer
Verdict: ✅ Approved
All acceptance criteria from issue #662 are implemented and verifiable.
Requirements coverage
ThemenWidgetplaced afterReaderPersonChipsThemenWidget compact={true}betweenDashboardFamilyPulseandDashboardActivityFeed/themenroute+page.svelte++page.server.tscreatedh-1.5bar withvar(--c-tag-{color})MAX_VISIBLE_CHILDREN = 5,slice(0, MAX_VISIBLE_CHILDREN)/?tag={encodeURIComponent(tag.name)}throughouthasAnyDocuments()recursive filteringm.themen_leer()shown whenvisibleTree.length === 0NFR notes
/themenusesgrid-cols-1 sm:grid-cols-2 lg:grid-cols-3✅. The widget usesgrid-cols-1(compact) orgrid-cols-1 sm:grid-cols-2(normal) ✅.Promise.allSettledpattern — doesn't block other dashboard data if it fails. ✅All requirements met. The scope is correctly bounded to what issue #662 specified — no scope creep observed.
🔒 Nora "NullX" Steiner — Application Security Engineer
Verdict: ✅ Approved with concerns
No exploitable vulnerabilities. One CSS injection smell worth documenting.
What I checked
Data flow — All tag data is fetched server-side via
createApiClient(fetch)in+page.server.tsand flows to components via SSR props. No client-side fetch, no API route exposure to the browser. ✅URL construction — Navigation links use
encodeURIComponent(tag.name)throughout:encodeURIComponentcorrectly encodes all characters that could affect URL parsing. ✅Authentication —
GET /api/tags/treeis an existing, auth-guarded endpoint. The frontend load functions inherit the session cookie via SvelteKit's SSR fetch. No new auth surface introduced. ✅XSS — Tag names are rendered as text content (not
innerHTML), and Svelte's template engine HTML-encodes by default. ✅Security smell (not a blocker, but document it)
tag.coloris interpolated into an inlinestyleattribute. CSS custom property references viavar()cannot execute JavaScript — this is not an XSS vector. However, a malicious color value likeslate; background: url(https://evil.com)could potentially exfiltrate the fact that a user visited the page via a CSS-based timing attack (CSS injection).Risk level: Very low. Tag colors are admin-controlled (not arbitrary user input), and CSS injection via
styleattributes is largely theoretical at this risk level.Mitigation if desired: Whitelist valid color names at the component boundary:
Not required for merge, but worth noting in a future security hardening task.
Not present (as expected)
@RequirePermissionneeded — this is a read-only feature using an existing endpointErrorCodevaluesClean. Approved.
🧪 Sara Holt — QA Engineer & Test Strategist
Verdict: ✅ Approved
Test pyramid is well-covered. The right tests are at the right layers.
Test coverage by layer
Unit (Vitest Node):
tagUtils.test.ts— 4 cases forhasAnyDocuments(): leaf=0 → false, leaf=3 → true, root=0+child=5 → true, root=0+all-zero → false. All four boundary conditions covered. ✅Server integration (Vitest Node):
themen/page.server.spec.ts— 3 cases: returns tree array, returns empty array when API sends[], throws 500 when API call fails. Happy path + error path covered. ✅routes/page.server.spec.ts— 3 existing dashboard tests updated with the newapi.GET('/api/tags/tree')mock. Would have broken CI without this fix. Good catch. ✅Browser component (Vitest + Playwright, CI-only):
ThemenWidget.svelte.spec.ts— 6 cases including: renders card per visible tag, hides tags wherehasAnyDocumentsis false, empty state (all filtered), empty state (empty array), compact grid attribute, "Alle Themen" link href. ✅themen/page.svelte.spec.ts— 6 cases including: renders one card per visible root tag, child rows visible up to 5, "+ N weitere" appears when children > 5, empty state. ✅What I like
The
makeLoadEvent()helper inthemen/page.server.spec.tswith a full{ fetch, request: new Request(...), url: new URL(...) }event object is the correct pattern for Sentry-wrapped load functions. The simpler{ fetch }pattern breaks because Sentry's wrapper readsrequest.method. Good fix, and it's now documented by the test itself.Minor gaps (not blockers)
The
hasAnyDocuments()tests cover depth-1 children but not depth-2 grandchildren. The function is recursive, so depth-2 would exercise the recursion more thoroughly. That said, the current 4 cases are sufficient for the current data model where trees are 2 levels deep.No test verifies the document count display logic: "show count only when
> 0". This is rendered in the template as{#if tag.documentCount > 0}{tag.documentCount}{/if}. A browser test asserting that zero-count tags show no number would close this gap. Not a blocker.The
+ N weiteretest inthemen/page.svelte.spec.ts— confirm it checks the count value displayed, not just presence of the element. If it's checkinggetByText(/\+ \d+ weitere/)that's sufficient.Overall test coverage is appropriate for the feature scope. Ready to merge from a QA perspective.
🎨 Leonie Voss — UX Designer & Accessibility Strategist
Verdict: 🚫 Changes requested
The card and color bar design is excellent. Touch targets meet requirements. The root tag links have proper focus rings. However, two keyboard accessibility violations must be fixed before merge.
🔴 Blocker 1 — Child links remove focus outline without replacing it (
/themen/+page.svelte)focus-visible:outline-noneremoves the browser's native focus ring.focus-visible:bg-canvas(a background color change) is not a visible focus indicator — it likely fails the 3:1 contrast ratio required by WCAG 2.4.11 (Focus Appearance, Level AA in WCAG 2.2). Keyboard users navigating by Tab cannot see where they are.Compare with the parent tag link, which correctly uses:
Fix:
🔴 Blocker 2 — "+ N weitere" link has no focus style at all (
/themen/+page.svelte)No
focus-visible:*classes at all. This link is completely invisible to keyboard navigation. WCAG 2.4.7 (Focus Visible, Level AA) violation.Fix:
What's working well ✅
focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:outline-none focus-visible:ring-inset— correct and consistent with the rest of the codebase ✅min-h-[56px], childrenmin-h-[44px]— meets the 44px WCAG 2.2 minimum for our 60+ user base ✅aria-hidden="true"on decorative color bar ✅aria-hidden="true"on the›character ✅grid-cols-1 sm:grid-cols-2 lg:grid-cols-3on/themen, single-column in compact widget mode ✅font-seriffor tag names,font-sansfor counts/children — consistent with design system ✅var(--c-tag-{color})is mode-neutral (same in light and dark) ✅Verification after fix
The same focus ring pattern should also be checked in
ThemenWidget.sveltefor any child links or "Alle Themen" links — apply the samefocus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-insetpattern there if it's not already present.The fixes are two class additions. Two minutes of work. Please fix and push before merging.
🏛️ Markus Keller — Senior Application Architect (Round 2)
Verdict: ✅ Approved
Architecture is clean throughout. No blockers.
Module boundaries
hasAnyDocuments()lives in$lib/shared/utils/tagUtils.ts— the correct neutral zone.ThemenWidget.svelteimports from$lib/shared/utils/, staying within theshareddomain boundary and respecting the ESLint cross-domain rule. ✅Layer compliance
All data loading is server-side.
+page.server.ts(dashboard) andthemen/+page.server.tsboth usecreateApiClient(fetch)→GET /api/tags/tree. Components receive data via$props(). No direct API calls from client-side code. ✅Error resilience
tagTreefetch is insidePromise.allSettled, so a failure doesn't block other dashboard data. The catch fallback explicitly returnstagTree: [] as TagTreeNodeDTO[]. ✅/themen: throws a SvelteKiterror(500, ...)if the API fails, which renders an error page. Appropriate for a dedicated route where the data is the whole point. ✅Documentation currency
/themenCLAUDE.mdroute table/themenl3-frontend-3c-people-stories.pumlC4 diagram correctly adds both
Component(themen, ...)andRel(themen, backend, "GET /api/tags/tree", "HTTP / JSON"). The description also captures thatThemenWidgetis embedded on the home dashboard — the diagram reflects the dual use. ✅Nothing to flag from an architecture perspective. Merge when ready.
👨💻 Felix Brandt — Senior Fullstack Developer (Round 2)
Verdict: ✅ Approved
TDD, Svelte 5 runes, keyed iteration, component sizing — all correct. No blockers remaining.
Svelte 5 compliance (full check)
$props()— ✅ used in bothThemenWidget.svelteand/themen/+page.svelte$derived.by()— ✅visibleTagsandvisibleTreeare computed reactivelyexport let, no$:, noonMount— ✅{#each ... (tag.id)}— ✅ in both components; child iteration also keyed(child.id)✅Clean code
hasAnyDocuments()is a 3-line pure function with a self-documenting name — exactly right. No business logic leaked into templates beyond the{@const}per-iteration derived values in the{#each}block, which is idiomatic Svelte 5 for item-scoped computation.ThemenWidget.svelteat 64 lines and/themen/+page.svelteat 85 lines — within acceptable ranges. Each does one thing.The accessibility fix commit
The last commit (
d1d0acf0) addsfocus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:outline-none focus-visible:ring-insetto the child links and+ N weiterelink. Minimal, targeted, correct. The ring-inset variant is the right choice here since these links are inside a card border — inset rings don't overflow the card boundary. ✅Remaining suggestions (not blockers)
data-compact={compact}on the grid div is used by the browser test to verify compact mode. This couples the test to an implementation attribute rather than user-visible state. A future improvement would be asserting that the grid has thegrid-cols-1class, but given the CI-only nature of the browser tests, this is a minor papercut.Ready to merge.
🛠️ Tobias Wendt — DevOps & Platform Engineer (Round 2)
Verdict: ✅ Approved
Pure frontend feature, no infrastructure changes. LGTM.
Checked: Docker Compose (no changes), CI pipeline (no changes), npm dependencies (no new packages added), env vars (none new), SvelteKit adapter config (unchanged).
The fix commit adds 2 CSS class strings. Zero operational impact. Merge when ready.
📋 Elicit — Requirements Engineer (Round 2)
Verdict: ✅ Approved
All requirements from issue #662 are implemented. No gaps.
Full requirements trace
<ThemenWidget tags={data.tagTree ?? []} />after<ReaderPersonChips><ThemenWidget tags={data.tagTree ?? []} compact={true} />in sidebar/themendedicated routethemen/+page.server.ts+themen/+page.svelteh-1.5div +style="background: var(--c-tag-{tag.color ?? 'slate'})"MAX_VISIBLE_CHILDREN = 5+visibleChildren.slice(0, MAX_VISIBLE_CHILDREN){m.themen_weitere({ count: hiddenCount })} →linking to root tag search/?tag={encodeURIComponent(tag.name)}throughouthasAnyDocuments()+$derived.by(() => data.tree.filter(hasAnyDocuments)){#if tag.documentCount > 0}{tag.documentCount}{/if}{m.themen_leer()}whenvisibleTree.length === 0focus-visible:ring-2(including post-fix child + weitere links)Scope is correctly bounded to issue #662. No scope creep observed. Ready to close the issue on merge.
🔒 Nora "NullX" Steiner — Application Security Engineer (Round 2)
Verdict: ✅ Approved
No security issues. The fix commit is CSS-only and introduces no new attack surface.
Full security check on the diff
Data flow: Server-side SSR throughout.
createApiClient(fetch)in load functions — the session cookie is forwarded by SvelteKit's server-side fetch. No API exposure to the browser. ✅URL encoding:
encodeURIComponent(tag.name)andencodeURIComponent(child.name)used consistently on every navigation link. ✅Svelte template output: All tag names and child names rendered as text content in Svelte template expressions
{}— Svelte HTML-encodes these by default. No{@html}anywhere in the diff. ✅CSS variable interpolation:
var(--c-tag-{tag.color ?? 'slate'})— CSS custom properties cannot execute JavaScript, so this is not an XSS vector. Tag colors are admin-controlled, low-risk. Flagged in round 1 for awareness; no regression from the fix. ✅Auth:
GET /api/tags/treeis an existing auth-guarded endpoint. No new permissions or permission checks needed — this is a read-only feature. ✅The focus ring fix: Purely presentational Tailwind class additions. Zero security impact. ✅
Clean. Approved.
🧪 Sara Holt — QA Engineer & Test Strategist (Round 2)
Verdict: ✅ Approved
Test pyramid is solid. The accessibility fix requires no new tests — it's a pure CSS class change with no new conditional logic.
Test coverage summary
tagUtils.test.tsthemen/page.server.spec.tsroutes/page.server.spec.ts(updated)ThemenWidget.svelte.spec.tsthemen/page.svelte.spec.tsSpecific observations
tagUtils.test.ts— 4 cases covering the key boundary conditions forhasAnyDocuments(). The recursive case (root=0, child=5) is explicitly tested. ✅themen/page.server.spec.ts— ThemakeLoadEvent()helper correctly provides{ fetch, request: new Request(...), url: new URL(...) }. This is the right pattern for Sentry-wrapped SvelteKit load functions, which crash ifrequest.methodis undefined. Documents a non-obvious pattern that future contributors will benefit from. ✅page.server.spec.ts(dashboard) — All 3 existing tests correctly received an additionalmockResolvedValueOncefor the newGET /api/tags/treecall. Without this fix, these tests would have failed in CI with "unexpected call" errors. ✅themen/page.svelte.spec.ts— The+ N weiteretest usesexpect(document.body.textContent).toMatch(/\+\s*2\s*weitere/)— checks the count value displayed, not just the element's existence. ✅The fix commit
No tests needed for the accessibility fix. Adding
focus-visible:ring-2to a link has no conditional logic, no new code paths, and the visual behavior is verified by axe-core accessibility checks in E2E (future work). ✅Ready to merge.
🎨 Leonie Voss — UX Designer & Accessibility Strategist (Round 2)
Verdict: ✅ Approved
Both blockers from round 1 are resolved. All interactive elements now have consistent, visible focus rings.
Blockers resolved
Blocker 1 — Child links ✅ FIXED
focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-insetadded. The ring-inset variant is correct here — it keeps the focus indicator within the card's border, preventing visual bleed across the card boundary. ✅Blocker 2 — "+ N weitere" link ✅ FIXED
Complete focus ring added. Keyboard users can now see the "more items" link when it has focus. ✅
Full accessibility check (post-fix)
ring-2 ring-brand-navy outline-nonering-2 ring-brand-navy outline-none<a>ring-2 ring-brand-navy outline-none ring-inset<a>ring-2 ring-brand-navy outline-none ring-inset<a>ring-2 ring-brand-navy outline-none ring-insetTouch targets: root
min-h-[56px], childrenmin-h-[44px], "weitere"min-h-[44px]— all meet the 44px WCAG 2.2 minimum. ✅Screen reader support: root tag
<a>carries anaria-labelthat includes the name and document count when count > 0. Decorative›characters arearia-hidden="true". Color bars arearia-hidden="true". ✅Responsive:
/themenscales from single-column → 2-column (sm) → 3-column (lg). Widget scales from single-column (compact) or 1→2 column (normal). ✅Typography: tag names in
font-serif, counts and metadata infont-sans— consistent with the design system. ✅All accessibility concerns resolved. Approved for merge.
d1d0acf029to80d77a53e9