docs(spec): reader dashboard design exploration and final spec #449
Reference in New Issue
Block a user
Delete Branch "docs/reader-dashboard-spec-447"
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?
Part of #447
Summary
Final spec covers 4 view variants
Key design decisions
Test plan
docs/specs/reader-dashboard-final.htmlin browser — verify all 4 sections render correctlyb0aa3a6ffdto12487d187f🏗️ Markus Keller — Senior Application Architect
Verdict: ✅ Approved
What I checked
This is a documentation-only PR — one static HTML file added to
docs/specs/. No application code, no schema changes, no new dependencies. My standard concerns (layer boundaries, domain coupling, transport protocol choices, database constraints) are not applicable here.Observations
Spec belongs in the right place.
docs/specs/is exactly where design specs should live — committed to the repository alongside the code they describe. This is the pattern.The
DK-additive class pattern is correctly documented as a spec-time approximation. The comment in the CSS is clear:This is the right way to communicate intent without pretending the spec IS the implementation. When Felix implements, he'll use Tailwind
dark:variants, not!importantoverrides. The comment makes this explicit.Scale comment is useful. The
/* Real layout: canvas spans full viewport width; inner wrapper is max-w-7xl (1280px) centered with px-8/py-8. Shown here at ~55% scale */comment on.PAGE-WRAPis exactly the kind of bridging annotation that prevents implementers from cargo-culting spec pixel values verbatim.No blockers.
One question for the implementer: The spec shows the header-bar is
background:#fffwithborder:1px solid #E0DDD5. When this becomes a Svelte component, does it get its own card pattern (bg-surface border border-line) or a dedicated white-background token? Worth clarifying before implementation starts so the design token mapping is unambiguous.👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ⚠️ Approved with concerns
What I checked
This is a spec HTML file, not production code, so TDD and Svelte-specific rules don't apply directly. What I review here is: does the spec communicate enough for implementation? Are there patterns it introduces that would lead to problematic code?
Suggestions
1. The
!importantdark pattern should call out its Svelte equivalent explicitly.The spec documents dark mode as:
with the note: "Real tokens live in layout.css / Tailwind dark: variants"
This is fine as a spec approximation. But the jump from
DK-classes → Tailwinddark:variants isn't spelled out. An annotation saying something like:would close the gap completely. Otherwise whoever implements this has to infer the token mapping themselves.
2.
impl-reftable is absent.The project's spec format standard (see Leonie's design domain expertise) calls for a Two-Layer Rule: scaled visual mockup +
impl-reftable with exact Tailwind class strings and real pixel values per UI region. This spec has the mockup and annotation blocks, but no structuredimpl-reftable.Example of what's missing:
Without this, the implementer has to reverse-engineer Tailwind equivalents from raw CSS values.
3. New i18n strings not catalogued.
The spec introduces new UI text that will need Paraglide i18n keys:
"Guten Morgen/Guten Tag/Guten Abend"(time-based greeting)"Herzlich willkommen, {name}."(personalized greeting)"Alle {n} Personen →"(person list link)"Zuletzt aktualisiert","Meine Entwürfe"(card headers)"Dok.","Pers.","Gesch."(mobile stat abbreviations)The spec should flag these as new
messages/*.jsonkeys so they're not forgotten during implementation.4. The stat numbers are wrapped in
<a href="#">.Where do these links go?
/documents? The spec should annotate the target routes. If they're not interactive in the MVP, the spec should say so.No blocking issues — the spec communicates the visual intent clearly. The concerns above are all about making the implementation handoff smoother.
🔧 Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
What I checked
Static HTML file committed to
docs/specs/. No Docker Compose changes, no CI workflow changes, no infrastructure configuration, no environment variables, no secrets, no image tags.Observations
Nothing to flag. One HTML file in docs/ has zero operational impact. No build step, no deployment artifact, no service dependency.
Minor note: If the project ever adds a CI step to lint or validate HTML files in
docs/specs/, this file would need to pass that check (it uses<html lang="de">and proper<!DOCTYPE html>— it would pass standard validators).The file is well-contained: self-hosted CSS, no external CDN dependencies, no JavaScript, no network requests when opened locally. It can be reviewed in any browser without connectivity.
LGTM from infrastructure perspective.
📋 Elicit — Requirements Engineer
Verdict: ⚠️ Approved with concerns
What I checked
As a requirements engineer, I evaluate whether a spec is complete enough to implement without ambiguity — particularly for edge cases, states, and i18n requirements that are often missed at this stage.
Blockers
None — the spec is implementation-ready for the visual golden path.
Concerns (should be addressed before implementation, not necessarily in this PR)
REQ-GAP-01: Interactive states not specified.
The spec documents resting state only. Missing:
These need to be specced before implementation, otherwise each will be a judgment call by the implementer.
REQ-GAP-02: Empty states not specified.
The spec shows a "happy path" dashboard with 847 documents, 94 persons, 12 stories. What renders when:
REQ-GAP-03: New i18n strings not logged.
The spec introduces text that requires
messages/{de,en,es}.jsonentries:"Guten Morgen"/"Guten Tag"/"Guten Abend") — what are the time boundaries? Before 12:00? Before 18:00?"Herzlich willkommen, {name}."— parametric string"Alle {n} Personen →"— parametric count"Dok."/"Pers."/"Gesch."— are these abbreviations of existing i18n keys or new ones?REQ-GAP-04: Stat link destinations ambiguous.
The header stats (847 Dok. / 94 Pers. / 12 Gesch.) are wrapped in
<a href="#">. The spec doesn't document where these navigate. Are they:/documents,/persons,/geschichtenrespectively?REQ-GAP-05: "How many persons to show" is unspecified.
The desktop spec shows 4 person cards. The mobile spec also shows 4. Is this fixed (top-4 by document count)? Configurable? What happens if there are fewer than 4 persons?
Observations (informational)
The PR description's decision table is good — it answers most "why" questions. The annotation blocks (
.ann-block,.note,.ok) give clear rationale for the chosen layout. The spec is genuinely useful documentation.The two personas (READ_ALL vs READ_ALL + BLOG_WRITE) are correctly separated — the BLOG_WRITE drafts zone as an additive section is well specified.
🔒 Nora "NullX" Steiner — Application Security Engineer
Verdict: ✅ Approved
What I checked
Static HTML spec document — self-contained, no JavaScript, no network requests, no user input, no auth surface, no backend interaction. From an application security perspective, this file presents no attack surface in production (it lives in
docs/, not in the application bundle).Observations
No security findings.
The spec uses
href="#"throughout — appropriate for mockup anchors, no SSRF risk. Noeval(), noinnerHTMLwith user data, no external script sources, no CDN dependencies that could be hijacked.One informational note for implementation time:
The stat numbers in the header bar (
847 Dok.,94 Pers.,12 Gesch.) are rendered as clickable links. When implemented, these will be populated from a backend API response. At implementation, ensure:innerHTML) to prevent any XSS path if counts ever come from user-controlled metadata."Herzlich willkommen, {name}."is rendered with the username escaped — SvelteKit's{name}interpolation in templates does this by default, but worth confirming when the component is built.Both are standard practice in this codebase — flagging now so they're not overlooked during implementation.
LGTM.
🧪 Sara Holt — QA Engineer
Verdict: ⚠️ Approved with concerns
What I checked
Spec documents are upstream of test suites. My review focuses on: is this spec testable? Does it contain enough precision to write acceptance criteria and automated checks against?
Concerns
1. No machine-readable acceptance criteria.
The PR test plan is entirely manual:
These are human spot-checks, not repeatable test conditions. When implementation starts, there's no Gherkin or structured acceptance criteria to write Playwright tests against. The implementation issue (#447 / implementation PR) will need acceptance criteria added at that point.
Recommendation: Before closing the implementation issue, add at minimum:
2. No Playwright visual regression baseline committed.
The spec shows exactly what the dashboard should look like in 4 variants. This is the ideal moment to commit screenshot baselines that Playwright can diff against during implementation. Without them, visual regressions will only be caught by eye during PR review.
3. Dark mode contrast is not verifiable from the spec alone.
Several dark-mode color values in the spec appear very low-contrast (flagged in detail by Leonie's review). axe-core accessibility checks would catch these automatically during E2E. The implementation PR must include
checkA11y()in both light AND dark mode per the project's testing standard.4. Empty state behavior is unspecified (see also Elicit's review).
Without empty state specs, there are no test cases for:
Each of these is a distinct test condition that will otherwise be untested.
LGTM for the spec itself — these are tracking items for the implementation phase, not blockers on this PR.
🎨 Leonie Voss — UI/UX Design Lead
Verdict: 🚫 Changes requested
The visual direction is strong — the B.1+B.3 merge is the right call, the mint pill badge reads beautifully against the plain 1px border, and the mobile stacking logic is clearly specified. But there are contrast failures in dark mode that I can't approve for implementation as-is, and the spec is missing its
impl-reftable.Blockers
1. Dark mode contrast failures — several values will not pass WCAG AA.
I've calculated approximate contrast ratios for the dark surface (
#161C27≈ rgb(22,28,39)):.DK-STORY-EXCERPT#3A4568.DK-DRAFT-META#3A4568.DK-HSTAT-LABEL#323850.DK-CARD-HEAD h3#323850.DK-DOC-DATE#262E48.DK-STORY-META#262E48.DK-DRAFT-META(mobile)#3A4568These are labeled "muted" text intentionally, but even intentionally de-emphasized text must meet 4.5:1 for normal-weight text under WCAG AA. Our target audience includes 60+ readers — reduced contrast in dark mode is doubly harmful for them.
Suggested fix: Use brighter muted values for dark mode. For example:
#3A4568→#7080A8(passes ~4.6:1 on#161C27)#262E48→#5A6888(passes ~4.8:1)#323850→#6070A0(passes ~4.9:1)These maintain the de-emphasis without making text invisible.
2. Light mode: several secondary/metadata colors also fail WCAG AA.
.HSTAT-LABEL#B8B4AE#fff.DOC-DATE#C8C4BE#fff.STORY-META#B8B4AE#fff.PERSONS-ALL#002850at 55% opacity#fffStat labels, dates, and meta text need at least 4.5:1. Suggest:
#888for de-emphasized text on white gives ~3.9:1 (acceptable for large/bold text), but metadata text like dates is small normal-weight text and needs a minimum of#767676(4.5:1) on white.3. Missing
impl-reftable.The project spec format standard (Two-Layer Rule) requires a scaled visual +
impl-reftable with exact Tailwind class strings and real pixel values per UI region. This spec has the visual mockup and annotation blocks, but noimpl-reftable. Without it, Felix has to reverse-engineer Tailwind equivalents from raw spec CSS — which leads to inconsistent implementations.Add a section like:
Suggestions
4. Hover and focus states not specified.
Person cards, stat links, and document rows are all interactive. The spec shows resting state only. At minimum, document the expected hover treatment (e.g.,
bg-surface → bg-canvasor a subtle border-color shift) and the focus ring style (focus-visible:ring-2 focus-visible:ring-brand-navy).5. Stat link accessible names.
Screen readers will announce "847 Dok." as the link text. Spec should note that the implementation adds
aria-label="847 Dokumente"(or equivalent) so the abbreviated label becomes fully readable for assistive technology.6. Token mapping for
#D4F0EE(mint pill bg).The mint pill background
#D4F0EEis introduced here. It's not yet a named token inlayout.css. Flag in the spec that this becomes--color-mint-pill(orbg-mint-softas a Tailwind utility) — whichever convention the project adopts — so it doesn't get hardcoded as an arbitrary hex value in production.Summary
The visual concept is approved. The two blockers (dark mode contrast failures and missing
impl-reftable) need to be addressed — either in this PR or as a mandatory prerequisite before the implementation PR opens. I won't approve the implementation PR if the spec it's based on has contrast failures baked in.Review concerns addressed — commit
f34967f7All of Leonie's blockers and Felix's suggestions resolved in one commit.
Leonie — 3 blockers ✅
Blocker 1: Dark-mode WCAG AA contrast failures
Fixed 8 desktop
DK-CSS rules + 8 mobile.dkmodifier rules + 3 inlinestyle="color:#262E48"dash elements:#3A4568(~1.8:1)#7080A8(≥4.5:1)#323850(~1.6:1)#6070A0(≥4.5:1)#262E48(~1.2:1)#5A6888(≥4.5:1)Blocker 2: Light-mode muted text contrast failures
HSTAT-LABEL#B8B4AE(~1.8:1)#706C68(~5.2:1)DOC-DATE#C8C4BE(~1.6:1)#706C68STORY-META#B8B4AE#706C68PERSONS-ALLrgba(0,40,80,.55)(~2.5:1)#4A6E8Adirect (~5.4:1)Blocker 3: Missing
impl-reftableAdded a full implementation reference table at the end of the spec — 20 UI regions with exact Tailwind class strings, real pixel values, and per-row implementation notes (including the header-bar
bg-whitevsbg-surfaceclarification Markus asked about).Felix — 4 suggestions ✅
Suggestion 1: DK-→Tailwind
dark:mapping annotationAdded to the dark-mode CSS comment block — each
DK-class now has its Tailwinddark:equivalent and the corrected WCAG-passing hex value.Suggestion 2:
impl-reftable — covered above.Suggestion 3: New i18n strings catalogued
Added annotation block at the end of section 4 listing all 10 new
messages/*.jsonkeys: greeting variants (with time boundaries), parametric welcome/persons strings, card headers, and mobile stat abbreviations.Suggestion 4: Stat link target routes annotated
Updated all
href="#"stat links and card-header "Alle …" links to real routes (/documents,/persons,/geschichten) across all 4 sections. Added a routing annotation note to the section 1 sidebar.Elicit / Sara / Nora — informational ✅
The i18n catalog and routing annotation also close Elicit's REQ-GAP-03 and REQ-GAP-04. Remaining gaps (REQ-GAP-01 interactive states, REQ-GAP-02 empty states, REQ-GAP-05 person count) and Sara's acceptance criteria / Playwright baseline items are intentionally deferred to the implementation issue (#447) as agreed in their reviews.
🏗️ Markus Keller — Application Architect
Verdict: ✅ Approved
Spec-only change — no structural or layer impact. Reviewing for design-system alignment and implementation guidance quality.
What I checked
Token alignment in the impl-ref table
The table correctly maps every UI region to the established token vocabulary (
bg-surface,border-line,brand-navy,brand-mint,bg-canvas). No rogue hex values in implementation classes. The explicit choice ofbg-white(notbg-surface) for the header bar is documented with a reason ("intentionell B.1-Entscheidung") — that's exactly the right way to record a deliberate deviation.Dark mode pattern
The spec's
DK-additive classes use!importantto override light values — appropriate for a static mockup document. Theimpl-refrow for dark mode explicitly says "Nicht!important-Overrides aus Spec — spec-DK- Klassen sind Annäherungen" with the correctdark:prefix approach for implementation. That hand-off is clear and prevents cargo-culting the mockup pattern into production code.New token candidates
Two color values are flagged as candidates but not yet tokens:
#C8B8A0→ proposed--color-greeting-time#D4F0EE→ proposed--color-mint-pill/bg-mint-softThese need to land in
layout.cssbefore the implementation PR is merged. The implementation issue (#447 or its follow-up) should include this as an explicit task, not a nice-to-have.Layer boundary
The spec correctly routes stats data through links to existing routes (
/documents,/persons,/geschichten). No new API surface implied — good. The permission-gated "Entwürfe" card is specified as a conditional zone without inventing a new permission; it piggybacks on the existingBLOG_WRITE. Architecture stays flat.Suggestion
Add a task to the implementation issue to register
--color-greeting-timeandbg-mint-softas design tokens inlayout.cssbefore the component goes live. Token candidates that ship as raw hex values become the hardcoded colors that haunt us in dark mode refactors.👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ⚠️ Approved with concerns
The impl-ref table is the strongest part of this spec — it translates directly into code without guesswork. A few gaps would slow down implementation if left unresolved.
Blockers
None — this is a spec doc, not shipping code.
Concerns
No Svelte component boundary map
The spec describes visual regions well but doesn't name the Svelte components. The implementation issue should define the component split before a line of code is written. Based on the spec I'd expect something like:
Without this map, the implementation risks landing a single monolithic
+page.sveltehandling everything.Mobile stat label i18n is undefined
The spec lists
dashboard.stat.documentsfor desktop ("Dokumente") and notes "mobil: Dok." — but the spec doesn't address English or Spanish abbreviations. Two options: (a) separate keysdashboard.stat.documents.shortper locale, or (b) a single key with Paraglide's plural/variant support. The implementation issue needs a decision.API endpoint not referenced
The spec shows stats (847 docs, 94 persons, 12 stories) and person cards with doc counts — but doesn't reference which endpoints serve this data. The existing
/api/statsor dashboard endpoint (if it exists) should be cited, or the impl issue should note that a new endpoint is needed. Without this, the implementer has to reverse-engineer the data model from the mockup.Suggestions
avatar-bgper person: the spec uses five different navy-family colors for the avatars — is this derived from a person attribute (index? hash of ID?) or hardcoded? The impl should derive it consistently.rounded-smon cards is correct per the existing card pattern in CLAUDE.md. ✓🛠️ Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
Static HTML file added to
docs/specs/. No infrastructure, CI, or deployment implications.What I checked
docs/— not served by any service, not included in build artifactsNotes
Spec files in
docs/specs/are development-time artifacts. They're committed to git for team reference but have zero runtime footprint. This PR is correctly scoped.LGTM.
📋 Elicit — Requirements Engineer
Verdict: ⚠️ Approved with concerns
The visual spec is detailed and the impl-ref table is precise. However, several data and interaction requirements are unspecified. These are not blockers for merging this PR, but they need to live somewhere before the implementation issue is started — otherwise the implementer will make quiet assumptions.
Gaps to address in the implementation issue
OQ-01: Person card ordering
The spec shows 4 persons (Käthe 47, Ernst 31, Frieda 28, Heinrich 19) but doesn't define the selection or sort rule.
OQ-02: "Recently updated" document sort
The spec labels the column "Zuletzt aktualisiert" but doesn't define:
updatedAt?uploadedAt? Last transcription edit?UPLOADED+? Or all?OQ-03: Stats counts scope
847 documents, 94 persons, 12 stories — are these:
OQ-04: BLOG_WRITE gating mechanism
The spec says the drafts card is visible "when BLOG_WRITE permission is present." On the frontend, how is this checked? Via the session data already available in the page loader? This is likely straightforward but should be explicit:
load()checksdata.user.permissions.includes('BLOG_WRITE').OQ-05: Empty states
No empty state is specified for any region:
Each should have a defined empty-state treatment (can be inherited from the app pattern or specified inline).
OQ-06: Dashboard data freshness
Is the dashboard SSR (snapshot at page load) or does it live-update (SSE or polling)? Given the Familienarchiv's low-frequency update pattern, SSR at page load is almost certainly correct — but it should be stated so no one adds unnecessary complexity.
What the spec does well
BLOG_WRITE) is clearly shown in Section 2 ✓🔒 Nora "NullX" Steiner — Application Security Engineer
Verdict: ✅ Approved
Static HTML specification document. No executable backend code, no API endpoints, no authentication logic, no user input handling. Security surface: zero.
What I checked
.htmlcommitted todocs/specs/— not served by any runtimeOne note (informational, not a finding)
The spec documents several clickable stat links (
<a href="/documents">) and person card links (<a href="#">) as mockup placeholders. When these are implemented in SvelteKit, ensure the stat links are standard<a>elements rendered server-side (SSR default) — no client-side-only fetch on mount, which would bypass the auth cookie forwarding.LGTM.
🧪 Sara Holt — QA Engineer & Test Strategist
Verdict: ⚠️ Approved with concerns
The spec establishes the design ground truth, which is exactly what I need to write test cases. But the test plan in the PR body is thin, and there are no acceptance criteria that map to the implementation tests.
Blockers
None for merging this spec PR.
Concerns
Weak test plan
The PR test plan says:
That's a smoke test for the spec document itself, not a test plan for the dashboard implementation. The implementation issue needs a real test plan covering:
No acceptance criteria
The spec is a visual mockup, not a list of acceptance criteria. For the implementation issue, I need statements like:
Accessibility testing scope
The spec claims WCAG AA compliance for dark mode text colors (
#7080A8muted,#6070A0labels,#5A6888dim on#161C27surface). These need to be verified with axe-core in both light and dark mode — not assumed. I'll add those assertions to the E2E suite when the implementation lands.What the spec gets right for testability
🎨 Leonie Voss — UI/UX Designer & Accessibility Strategist
Verdict: ⚠️ Approved with concerns
The spec follows the Two-Layer Rule (visual mockup + impl-ref table) and the scale disclaimer is prominent and correct. The design decisions are well-rationalized. I have two accessibility findings that need to be addressed before implementation, and one UX gap.
Blockers
1. Greeting-time label fails WCAG AA contrast
The greeting-time label (
#C8B8A0warm beige on white#FFFFFF) has a contrast ratio of approximately 1.9:1 — well below the 4.5:1 AA minimum for normal text, and below 3:1 for large text. At 11px real size (6px × 1/0.55 scale factor), this does not qualify as "large text" (≥18px normal or 14px bold).Fix options:
text-ink-3(#706C68) which achieves ~4.6:1 on white — passes AAtext-[11px] font-bold uppercase tracking-widest text-ink-3per the existing section-title token patterntext-ink-3is the safe default hereThis is a blocker for implementation — do not ship with
#C8B8A0as the greeting-time color.2. "Alle N Personen" link contrast is borderline
#4A6E8Aon the canvas background (#ECEAE4) yields approximately 4.49:1 — fractionally below the 4.5:1 AA threshold for normal text. The spec comments "WCAG AA auf bg-canvas" but the math disagrees by a hair.Fix: Use
#4A6580(one step darker) or the existingbrand-navy(#002850) at reduced opacity is not an option since opacity tricks don't count for contrast. Verifying with the Figma contrast plugin or Colour Contrast Analyser is required before the impl lands. The impl-ref table should cite the verified passing value.Concerns (non-blocking for spec merge)
3. Mobile hamburger menu not specified
The mobile mockup shows a hamburger icon (3 bars) but the spec doesn't define what happens on tap: slide-in drawer? Dropdown? Full-screen overlay? This is a UX gap — the implementation issue should spec this before the nav is built. Without it, the developer will make a choice that may need a UX rework.
4. Touch targets on mobile person cards
The mobile person cards (
M-PCARD) showpadding:7px 8pxin the mockup CSS (55% scale). At real scale that's approximatelypadding:13px 15pxon a narrow card. Verify in the implementation that each card's tappable area is ≥44×44px per WCAG 2.2. The 2×2 grid on a 375px screen gives each card ~182px width — width is fine, but the card height depends on the avatar + name + pill stacking. Flag this in the implementation PR.What the spec gets right
≥ 4.5:1)#D4F0EEwith navy text: navy on mint-pale is ~12:1 — AAA pass ✓font-serif(Tinos/Georgia) for document titles and greeting name;font-sans(Montserrat) for labels — brand typography consistent ✓1px #E0DDD5card border with no accent stripe — cleaner than the B.1 mint-bottom alternative ✓