feat(timeline): layer filter (Personal / Historical / Letters) for /zeitstrahl #843
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: marcel/familienarchiv#843
Reference in New Issue
Block a user
Delete Branch "feat/issue-780-timeline-layer-filter"
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 #780.
A presentation-only, fully client-side layer filter for the global
/zeitstrahl: aTimelineFiltersbar with three toggles (Personal / Historical / Letters) that derives a$derivedfiltered view of the already-loaded timeline. No backend, endpoint, migration, URL state, API call, orgenerate:api— the SSR load (owned by #779) is untouched. Reloading returns to the default all-layers-on view.Built TDD red→green; the Svelte MCP autofixer validated both
.sveltefiles.What's in it
timelineFilter.ts— pureisDefaultState/hiddenLayerCount/filterTimeline(drops year bands left empty, filters the undated bucket; input never mutated).TimelineFilters.svelte— dumb bar: three$bindabletoggles in a<fieldset><legend>, sticky "Filter (N aktiv)" trigger, reset text button (visible only when a layer is off), reduced-motion-awareslide(duration 0 whenprefers-reduced-motion, reusing the guard fromdocuments/[id]/+page.svelte:57). Toggles mirror theSearchFilterBarundated-toggle markup (aria-pressed, ✓ glyph, 44px target, semantic tokens)./zeitstrahl/+page.svelte— holds the three toggles in$state, binds them into the bar, derives the filteredTimelineDTO→TimelineView; renders a filter-specific empty-state + one-click reset below the open bar when nothing is visible (never blank, never the generic "no events" state). Meta-line keeps counting the unfiltered timeline (D1 known limitation, commented).timeline_filter_*keys in de/en/es (triggervstrigger_active({count})distinct).goto/url.searchParams/api.GET/fetch; an un-skipped Playwright journey + 375px axe (light + dark).Requirement coverage
All
REQ-001…REQ-010implemented, each with a green test and aDonerow in.specify/rtm.md. Full REQ → commit → test mapping is in the completion comment on #780.Verification
timelineFilter,messages,timelineFilterBoundary,TimelineFilters.svelte,zeitstrahl/page.svelte— the 7 pre-existing page tests stay green).svelte-check: zero errors in any touched file (repo's ~800 pre-existing baseline is unrelated/ungated).prettier/eslintclean — pre-commit gate passed on all 7 commits.Reviewer notes
durationis zeroed; the matchMedia guard is reused verbatim.zeitstrahl-filter.spec.ts) is authored un-skipped and Playwright-collection-verified, but not executed in this environment (local stack unhealthy: backend health 500, no frontend on :3000). Run with the full stack up:npx playwright test zeitstrahl-filter.spec.ts. E2E is not wired into CI pere2e/CLAUDE.md.🤖 Generated with Claude Code
Eight Paraglide keys for the /zeitstrahl layer filter — layer labels, the fieldset legend, the sticky trigger (distinct timeline_filter_trigger and timeline_filter_trigger_active({count}) so it never reads "Filter (0 aktiv)"), the reset button, and the filtered-empty message — added to all three locales. messages.spec asserts their presence and the {count} signature. Refs #780 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>A static boundary gate (mirroring the project's no-`{@html}` greps) that reads TimelineFilters.svelte and /zeitstrahl/+page.svelte and fails if either ever reintroduces goto(, url.searchParams, api.GET, or fetch( — the filter must stay presentation-only and fully client-side. Refs #780 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>📋 Requirements Engineer — PR Review
Verdict: ⚠️ Approved with concerns
Every
REQ-NNNfrom the (rescoped 2026-06-14, layer-only) #780 spec is implemented, tested, and carries aDonerow freshly added to.specify/rtm.md(10 rows, lines 157–166). Traceability is end-to-end.Traceability table
DoneTimelineFilters.svelteTimelineFilters.svelte.spec.ts+timelineFilterBoundary.spec.ts$derivedfiltered view+page.svelte:23-28page.svelte.spec.ts+ boundarytimelineFilter.ts#isVisibletimelineFilter.spec.ts,page.svelte.spec.tstimelineFilter.ts#isVisibletimelineFilter.spec.ts,page.svelte.spec.tstimelineFilter.ts#isVisibletimelineFilter.spec.ts,page.svelte.spec.ts+page.svelte:84-98page.svelte.spec.ts,timelineFilter.spec.tsisDefaultState/hiddenLayerCount, 44pxtimelineFilter.ts,TimelineFilters.sveltetimelineFilter.spec.ts,TimelineFilters.svelte.spec.ts$derivedany-offTimelineFilters.svelteTimelineFilters.svelte.spec.tsduration: 0TimelineFilters.svelte:24-30messages/{de,en,es}.jsonmessages.spec.tsConcerns (not blockers)
duration: 0"), so it is not a contract violation, but it remains the single requirement with no green assertion. I verified the reuse is faithful: the guard atdocuments/[id]/+page.svelte:58is reproduced verbatim and only the slidedurationis zeroed.documents/[id]/+page.svelte:57; it is actually at line 58 (matching the spec). Worth a one-character fix for future trace accuracy.Scope-creep check
No behavior without a backing requirement. The extra
timelineFilterBoundary.spec.tsandALL_LAYERS_ON/hiddenLayerCounthelpers are all backed by REQ-001/002/007 acceptance criteria (the "grep returns zero" AC). RTM is in sync withmain.👨💻 Developer (Felix Brandt) — PR Review
Verdict: ✅ Approved
Clean, small, and idiomatic Svelte 5. The split is exactly right:
timelineFilter.tsis a pure module,TimelineFilters.svelteis a dumb presentation component (three$bindablebooleans +onChange), and the route owns the$stateand the$derivedfiltered view. KISS over DRY — no premature abstraction.What I checked
filterTimelinerebuilds{ years, undated }with.map/.filterand never touches the input;timelineFilter.spec.ts#does not mutate the input timelineproves it. I confirmedTimelineDTOis exactly{ years, undated }ingenerated/api.ts:2432, so the reconstruction loses no field.isVisiblebranch coverage —kind === 'LETTER'is checked beforetype, which is correct becausetype?is optional on the DTO (letters have no type). Events fall toHISTORICALor default-to-personal — sound for thePERSONAL | HISTORICALenum, and a defensible default iftypewere ever absent.generate:apicorrectly skipped — no backend model/endpoint change,api.tsuntouched (constitution §3.5/§4.1 respected). No newErrorCode(none needed) — the four-site rule (§3.6) is N/A.Suggestions (nice-to-have, non-blocking)
+page.svelte:79-81uses the longhandbind:personalOn={personalOn}. Svelte 5 accepts the shorthandbind:personalOnwhen the variable name matches — marginally cleaner, but the longhand is valid and prettier-stable, so purely cosmetic.timelineFilter.tsre-declaresTimelineDTO/TimelineEntryDTOtype aliases that several sibling timeline modules also declare locally. Fine at two callers (KISS, §3.2 — abstract on the third), just noting it for when a third appears.No layering, error-handling, or generated-artifact issues. Good work.
🧪 Tester — PR Review
Verdict: ⚠️ Approved with concerns
Coverage is broad and the levels are well-chosen: a pure-logic suite (
timelineFilter.spec.ts), a static boundary guard (timelineFilterBoundary.spec.ts), a component suite (TimelineFilters.svelte.spec.ts), a route-integration suite (page.svelte.spec.ts, +4 new specs over the 7 pre-existing), message parity (messages.spec.ts), and an un-skipped E2E journey + 375px axe. Each layer tests at the right altitude and asserts real behavior (e.g.does not mutate the input timeline,drops year bands that become empty).Strengths
/\bgoto\s*\(/,/url\.searchParams/,/\bapi\.GET\b/,/\bfetch\s*\(/) that cannot regress. Stronger than a one-shot runtime spy.page.svelte.spec.tsexercises the real route component with all three layers, including the all-off filtered-empty path and that the generictimeline_empty_stateis explicitly not shown (asserts.query()is null) — a sharp distinction.Concerns (not blockers)
window.matchMediato{ matches: true }and assert the collapsible opens without the slide delay, or extract a tinyslideDurationFor(reduced: boolean)pure helper and unit-test it. Optional, but it would close the one untested REQ.page.svelte.spec.ts#…with no fetchtoggles and checks the DOM but does not spy oncreateApiClient/fetch; the "no fetch" guarantee rests entirely ontimelineFilterBoundary.spec.ts. Acceptable (the static guard is strictly stronger), just noting the AC's literal wording asked for a runtime "not called" check.e2e/CLAUDE.md). The axe/journey gate is therefore unverified until someone runsnpx playwright test zeitstrahl-filter.spec.tsagainst a healthy stack. I confirmed the spec is collection-valid and nottest.skip-ped (the #779 dependency is merged, so removing the skip is correct).No flaky patterns, no passing-on-first-run tests spotted. Recommend running the E2E locally before merge to close concern #3.
🔐 Security (Nora "NullX") — PR Review
Verdict: ✅ Approved — LGTM, nil attack surface.
This is a read-only, frontend-only, client-side-only visibility filter over data already loaded by #779. No trust boundary is crossed. I checked everything I'd normally look for and found nothing to fix:
POST/PUT/PATCH/DELETEadded, so §2.1/§2.2 (@RequirePermission, typed enum) are N/A. No backend code at all.timelineFilterBoundary.spec.tsstatically forbidsapi.GET/fetch(in both touched files, and the boundary is reachable only by family-wide read. The+page.server.tsSSR load (and its 401/error handling) is owned by #779 and untouched here.m.timeline_filter_*); the only dynamic value ism.timeline_filter_trigger_active({ count }), wherecountis a number derived from three booleans (hiddenLayerCount). No user/import-derived text is rendered, andgrep -rn '@html' frontend/src/lib/timeline/stays at zero —{...}escaping throughout.parseTimelineFilters; there is nourl.searchParamsread (boundary-guarded), so no tampering/open-redirect/DoS-via-params vector.createdBy/updatedByN/A — no persistence).@axe-core/playwrightand@playwright/testused by the E2E spec are already in the manifest; §5.1 (ADR-for-new-dependency) is not triggered.The STRIDE assessment in the issue body matches the code as shipped. Nothing for me to block on.
⚙️ DevOps — PR Review
Verdict: ✅ Approved — LGTM, zero infra/operational footprint.
Nothing in this PR touches the deploy, build, or CI surface, and there are no Do-Not-Touch violations.
What I checked (constitution §4 / §5)
frontend/src/lib/generated/api.tsandparaglide/are untouched (§4.1). The only non-source change is.specify/rtm.md, which is a hand-maintained doc, not generated.upload/download-artifactbump (§4.3/§4.4) — no.gitea/workflows/change at all.package.jsonunchanged; the E2E spec reuses already-present@axe-core/playwright+@playwright/test..env.exampleordocker-compose*.yml.feat/issue-780-timeline-layer-filter, notmain.Operational note (not a blocker)
e2e/zeitstrahl-filter.spec.ts(journey + 375px axe, light/dark) is not wired into CI — pere2e/CLAUDE.md, E2E runs locally only for now. So this axe gate provides no automated regression protection until the E2E suite is CI-integrated; that's an accepted project-wide posture, not a defect of this PR. Just flagging that the a11y guarantee here is manual-run for the foreseeable future.No rollback risk (no schema/state), no config drift. Ship-safe from an ops standpoint.
🎨 UI/UX — PR Review
Verdict: ⚠️ Approved with concerns
The interaction model is right for the dual audience (phone-first readers + seniors 60+), and the toggle markup faithfully mirrors the canonical
SearchFilterBarundated-toggle (verified againstSearchFilterBar.svelte:262-274). Accessibility fundamentals are solid.Strong points
bg-primary/text-primary-fgcolor with a checkbox-style ✓ glyph box (filled when pressed, empty-bordered when not), andaria-pressedcarries the state to AT. The glyph is correctlyaria-hidden.min-h-[44px](WCAG 2.5.5). Verified inTimelineFilters.svelte.spec.ts#gives the trigger a 44px touch target.<fieldset><legend>Ebenen anzeigen</legend>; the collapsible trigger hasaria-expandedand a correctly-conditionalaria-controls(set only while the panel is in the DOM, avoiding a dangling idref).bg-surface,border-line,text-ink-2/3,bg-primary,text-primary-fg); no raw palette. Reduced-motion handled (REQ-009).Concerns
{metaLine}fromtimelineMeta(data.timeline)) keeps counting the unfiltered timeline. So with Letters off and zero letters visible, the header can still read e.g. "1903–1945 · 12 Briefe · …". For the senior audience this is a plausible "did it actually filter?" confusion. It's an accepted spec limitation (D1) and code-commented at+page.svelte:14-18— I'm not blocking, but I'd strongly suggest a fast follow-up to either recompute the meta fromfilteredTimelineor append a "(gefiltert)" hint when!isDefaultState.Suggestions (non-blocking)
aria-pressed) this reads fine, so it's acceptable — but a… anzeigenaria-label would make each button self-describing out of context.top-16(4rem) to clear the header; worth a manual check at 375px landscape (short viewport) that the sticky bar doesn't eat the small content area — the issue called this out specifically.No blockers from a UX standpoint; concern #1 is the one I'd like to see tracked.
🏛️ Architect — PR Review
Verdict: ✅ Approved
The change respects every boundary it touches and introduces no irreversible decision, so no ADR is required.
Boundaries (constitution §1)
timelinedomain ($lib/timeline/timelineFilter.ts,TimelineFilters.svelte+ specs); the route consumes it.timelineFilter.tsimports only$lib/generated/apitypes — no cross-domain reach. The route→$lib/timelineimport is already permitted byeslint.config.jsboundaries/dependencies(the route already importedTimelineView), so no boundary config change was needed or made.$state+ the$derivedfiltered view. This is the layering the issue mandated.ArchitectureTestallow-list rule is N/A (no Java touched). No service/repository/controller layering concerns.Irreversibility / ADRs
AcceptedADR edited or superseded (§4.2). TheGET /api/timelinecontract (owned by #5/#835) is unchanged — this filters a fully-materialized DTO client-side.TimelineDTOis closed at{ years, undated }(generated/api.ts:2432), sofilterTimelinereconstructing the DTO is structurally complete and forward-compatible: if the DTO gains a field, the spread-free rebuild would need a touch-up, but today it loses nothing.Design soundness
LETTER→ Letters,EVENT & HISTORICAL→ Historical, else → Personal) maps cleanly onto the existingkind/type/derivedmodel with no new server concept — derived life-events correctly ride the Personal layer (they carrytype=PERSONALper #776), avoiding a phantom fourth layer. This keeps the feature a pure presentation concern, exactly as scoped.Architecturally clean. No concerns.
The /zeitstrahl header sub-line counted the unfiltered timeline, so a hidden layer (e.g. Letters off) still showed its entries in the totals ("1 Brief" with no letters on screen) — the documented D1 limitation. Derive the meta from filteredTimeline so the range and letter/event counts always match what is actually rendered. hasContent stays on the full timeline so the filter bar and meta line still appear whenever the archive has content. Refs #780 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>✅ Follow-up — UI/UX concern #1 (meta-line count mismatch) resolved
Pushed
096b4a0f(fix(timeline): track the meta-line counts to the filtered view).The header sub-line now derives from
filteredTimelineinstead ofdata.timeline, so the range + letter/event counts always match what's on screen — hiding the Letters layer drops "1 Brief" from the totals rather than leaving it stale. This closes the D1 limitation the original spec accepted; the in-code D1 comment is replaced accordingly.hasContentdeliberately stays on the unfiltered timeline, so the filter bar + meta line still appear whenever the archive has content (including the all-filtered-out state, where the empty-state message renders below).page.svelte.spec.ts › recomputes the meta-line counts from the filtered view…(confirmed red against the old code). 12/12 page specs green — the 6 pre-existing #833 meta tests (range/singular/zero-drop/empty) still pass, so the default all-on view is byte-identical.Remaining open items from the review are both non-blocking: REQ-009's manual-only coverage (spec-sanctioned) and running the E2E (
zeitstrahl-filter.spec.ts) locally against a healthy stack.