feat(ui): responsive DocumentTopBar — TopBar B1 spec implementation #161
Reference in New Issue
Block a user
Delete Branch "%!s()"
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?
Context
The document detail page (
/documents/[id]) has aDocumentTopBarcomponent (src/lib/components/DocumentTopBar.svelte) that currently renders a single flat layout at all screen sizes. This issue implements the TopBar B1 Responsive design spec (docs/specs/document-topbar-b1-responsive.html), which defines a mobile-first, five-breakpoint layout with full dark mode support, person-chip overflow, and an annotate-mode state.Open the spec file in a browser to see all visual references referenced in this issue.
What currently exists
DocumentTopBar.sveltealready:<h1>, a compactcompactMetastring (date + sender/receiver as plain text), annotate toggle, edit link, and download linkbg-surface,border-line,text-ink, etc.) from the project's themedoc,canWrite,canAnnotate,fileUrl,annotateMode(bindable)What is missing: responsive breakpoint logic, chip-based person display, receiver overflow pill, dark mode token variants, the annotate hint strip, and the viewport-specific show/hide rules described below.
Design tokens
All colours are already present in the project theme. Map spec names to Tailwind classes as follows:
bg-surface#FFFFFF#0F1923bg-surface(alreadyvar(--color-surface))border-line#E4E2D8#1E2D3Dborder-linebg-primary/ accent bar#012851#A1DCD8border-primary/bg-primarytext-primary-fg#A1DCD8#012851text-primary-fgtext-ink#1A1A1A#EAE8E2text-inktext-ink-3(meta)#AAAAAA#3E5065text-ink-2(closest existing)bg-muted(chip bg)#F0EFE9#0A1218bg-mutedorbg-canvas#DDD9D0#1E2D3Dborder-lineAccent bar:
border-l-[3px] border-primary— always present, all breakpoints, both themes.Responsive behaviour rules
Implement these as Tailwind responsive classes. The topbar height and element visibility change at four breakpoints:
< 375px(XS)375–767px(mobile)768–1023px(tablet)≥ 1024px(laptop/desktop)h-11(44 px)h-[50px]h-[52px]h-[52px]24×24,rounded26×26,rounded-full28×2828×28text-[10px]text-[10.5px]text-[11.5px]text-[12px](text-[12.5px]at 1440)K. Raddatz)dd.mm.yyyyinline in plain-text metadd.mm.yyyydd.mm.yyyy24. Dezember 1943(usemonth: 'long')doc.locationpresent"Edit""Edit""Bearbeiten""Bearbeiten"18pxstrip below main row18pxstripPerson chips
Replace the current plain-text
compactMetareceiver string with a chip row at≥ 375px.Chip structure (one chip)
#012851(navy),#5A3080(purple),#007596(teal),#2A6040(moss),#803020(rust).375–767px: use abbreviated name — first initial + last name (K. Raddatz). At≥ 768px: full name.Arrow between sender and receiver
Omit the arrow when there is no sender (direction unknown — e.g. photos, certificates).
Overflow rule — always show sender + 1st receiver, collapse the rest
Overflow pill:
At
< 375px(XS), chips are hidden entirely. Overflow is shown as plain text appended to the meta line:"K.Raddatz → E.Raddatz +4 · 24.12.1943".Overflow tooltip
When the overflow pill is clicked/tapped (at
≥ 768px), show a floating panel:absolute top-full left-0 mt-1relative to the pillbg-surface, borderborder-line,rounded-md,shadow-lg,p-3,min-w-[160px],z-50"Weitere Empfänger"intext-[7px] font-bold uppercase tracking-wide text-ink-2, with a bottom border16×16) + full name intext-[9.5px] font-medium text-ink/persons/{id}bg-surfacedark token automaticallyOn mobile (
< 768px) the overflow pill shows "+N" only (no tooltip) — tapping it does nothing (or could navigate to the edit page to see all receivers, but that's optional scope).Active state of the pill (tooltip open or annotate mode active):
Annotate mode
When
annotateMode === true(passed as bindable prop):bg-primary text-primary-fg), label changes to"Beenden"≥ 768px):Dark mode hint:
text-primary-fgfor the label, same muted bg.XS (< 375px) plain-text meta line
At the smallest viewport, chips are not rendered. Instead, keep the existing
compactMetastring but adjust its format:K.Raddatz)+Nafter the first receiver namedd.mm.yyyy(numeric, no spaces)Status chip (≥ 768px)
The
docobject exposes astatusfield of typeDocumentStatus(PLACEHOLDER | UPLOADED | TRANSCRIBED | REVIEWED | ARCHIVED). Show a small status badge inline with the title:Use green (
bg-emerald-100 border-emerald-300 text-emerald-800) forUPLOADED/ARCHIVED, a neutral muted style for the rest. Dark mode: usedark:variants with 7–10% opacity backgrounds.Accessibility requirements
<header role="banner">or already be inside<header>— check the parent layout.aria-label="Zurück zur Dokumentenliste"(always, because the icon is the only visible element on XS).aria-haspopup="listbox",aria-expanded,aria-label="Alle Empfänger anzeigen".role="listbox"on the panel,role="option"on each person row.aria-pressed={annotateMode}.outline: nonewithout a replacement.Out of scope for this issue
AnnotationSidePanel)AnnotationSidePanelorDocumentBottomPanellocationfield onDocument— if it does not yet exist on the backend DTO, skip the location display for now and leave a// TODO: show location when availablecommentAcceptance criteria
aria-pressed,aria-haspopup,aria-expanded,aria-labelon all interactive elementssvelte-checkpasses with no new type errors/proofshotagainst the document detail page at all five viewport widths in both themesReference
Full annotated spec:
docs/specs/document-topbar-b1-responsive.html— open in a browser.Existing component:
src/lib/components/DocumentTopBar.svelteConsuming page:
src/routes/documents/[id]/+page.svelte👨💻 Felix Brandt — Senior Fullstack Developer
Component Decomposition First
The issue describes multiple distinct visual regions packed into one component. Before writing a single line of code, I'd draw these boundaries:
DocumentTopBar.svelte— orchestrator, holdsoverflowOpenand passes props downPersonChipRow.svelte— chip row with arrow, visible at≥ 375pxPersonChip.svelte— single chip (avatar + name), receivespersonandabbreviated: booleanOverflowPill.svelte— the "+N weitere" button and its tooltip panelDocumentStatusChip.svelte— status badge with dot indicatorAnnotateHintStrip.svelte— the 18px annotate-mode bannerA single component doing all of this will exceed 40 lines of template easily and become untestable. Each of these has a clear one-word name and a single visual responsibility.
TDD Questions
"Karl Raddatz"→"K. Raddatz") is a pure function — it needs a Vitest unit test written before the implementation. What are the edge cases? Single-name persons? Hyphenated last names ("K. Müller-Schmidt")?"K.Raddatz → E.Raddatz +4 · 24.12.1943") is pure formatting logic — this belongs in aformatCompactMeta()utility module, not inline in the template.statusLabel()andstatusClass()helper functions should each have unit tests covering all 5DocumentStatusvalues.Svelte 5 Concerns
viewport >= 768in the overflow pill template. If this meanswindow.innerWidthat runtime, that is a side-effect, not a derived value. It must not be computed inline in markup. Use a$derivedfrom a reactivewindowWidthstore, or — better — use CSS-only responsive classes everywhere and avoid JS viewport checks entirely.$derivedfor computed values:extraCount,visibleReceivers,abbreviatedSender,formattedDate— all of these must be$derived, never$state+$effect.{#each}: Every chip list must use(person.id)as the key, not index. Reordering receivers without a key will silently corrupt local chip state.overflowOpenstate: Fine as$stateon the orchestrator — just make sure Escape dismissal uses$effectwired to akeydownhandler, not inline template logic.Naming
overflowOpen— good boolean name (reads as a yes/no question ✓)extraCount— clear ✓compactMeta— a bit vague;xsMetaLineorplainTextMetawould better communicate that this is the XS-only fallbackSuggestions
abbreviateName(person: Person): stringandformatXsMeta(doc: Document): stringintosrc/lib/utils/personFormat.ts— these are reusable and independently testable.personAvatarColor(personId: string): stringinto the same util module.use:clickOutside) rather than wiringdocument.addEventListenerdirectly in$effect. Keeps the component clean and the action reusable.🏛️ Markus Keller — Application Architect
Scope Assessment
This is a pure frontend component change — no backend, no database, no new services. The scope is well-contained and the layering is correct: data flows in from the server load function via
docprops, the component renders. That's the right pattern.SSR vs. Client-Side Viewport Detection
My main architectural concern: the overflow pill contains
viewport >= 768 ? 'weitere' : ''— this implies a runtime JS check ofwindow.innerWidth. On SvelteKit with SSR,windowis not available during server render. If this lands in the component script without abrowserguard, it will throw on SSR.Two acceptable approaches:
window.innerWidthin JS at all. The word"weitere"can be shown/hidden purely with responsive Tailwind classes (hidden sm:inline). This is the right call — no hydration mismatch, no JS needed.$effectwithbrowserguard: If JS viewport detection is genuinely necessary (e.g. for JS-driven tooltip positioning), gate it behindimport { browser } from '$app/environment'. But I'd exhaust CSS-only options first.The
375pxCustom BreakpointTailwind's default breakpoints are
sm: 640px,md: 768px,lg: 1024px,xl: 1280px. The375pxbreakpoint used throughout this spec is non-standard. This means a custom breakpoint must be added totailwind.config.ts:Is this already in place? If not, classes like
xs:flexwill silently do nothing, and the responsive logic won't work. Worth confirming before implementation starts.annotateModeState OwnershipThe
annotateModeprop is declared as bindable (bind:annotateModefrom the parent page). This means the parent page (+page.svelte) owns the state, and the annotation side panel presumably also reads it. That's correct.My question: if multiple components read and write
annotateMode(TopBar toggles it, the annotation panel may reset it), is a simple bindable prop sufficient, or should this be a shared$statein a context or a small Svelte store? I'm not blocking implementation on this — a bindable prop is fine for now — but it's worth clarifying how the annotation panel's own close button will interact with this.Design Token Mapping
The issue maps spec tokens to Tailwind classes and notes some are approximations (e.g.
text-ink-3→text-ink-2as "closest existing"). I'd want this documented with a brief comment in the component or inlayout.cssso future maintainers understand the intentional approximation rather than treating it as a bug.No Concerns On
docvia props from server load — correct dependency direction// TODO: show location when availableplaceholder — pragmatic and clearly scoped out🧪 Sara Holt — QA Engineer & Test Strategist
Test Strategy
This feature has significant logic complexity hiding behind its visual surface. Here's how I'd layer the test coverage:
Unit tests (Vitest) — pure utility functions, fast, no DOM:
abbreviateName(person)— full name, single word name, hyphenated surnameformatXsMeta(doc)— 0 receivers, 1 receiver, 3 receivers with overflow, no senderpersonAvatarColor(personId)— deterministic: same ID always returns same colour; all IDs map to a valid palette entrystatusLabel(status)— all 5DocumentStatusvaluesstatusClass(status)— all 5 values, including dark-mode class variantsComponent tests (Vitest + @testing-library/svelte) — render + ARIA assertions:
true: Edit and Download buttons not in DOM; hint strip present; button label "Beenden"false: Edit and Download visible; hint strip absent; button label "Annotieren" (or equivalent)E2E tests (Playwright):
aria-pressedcorrectlycheckA11yruns on the document detail page in both light and dark modeMissing Acceptance Criteria
The current criteria don't specify:
doc.senderisnull(the "no sender" case is described in the spec but not in the AC checkboxes)/persons/{id}are tested for correct navigationI'd suggest adding:
/persons/{id}URLViewport Coverage
The AC mentions 320px, 375px, 768px, 1024px, and 1440px. For Playwright visual regression, I'd capture both light and dark mode at each — that's 10 screenshots to baseline. The
/proofshotskill should cover this, but make sure the Playwright config has viewport presets for all five widths and thatprefers-color-schemeor the dark mode class toggle is automated.Risk: The "click outside" dismiss
Click-outside logic is historically flaky in component tests. If using a Svelte action, test the action in isolation. If using
document.addEventListenerin an$effect, test via Playwright E2E only — JSDOM's event bubbling doesn't always match real browsers.🔐 Nora "NullX" Steiner — Application Security Engineer
Attack Surface Assessment
This is a frontend-only UI component change with no new API endpoints, no new backend logic, and no new data flows. The attack surface is narrow. Findings below are low-severity observations and smells — nothing blocking — but worth addressing cleanly.
Observation 1: CSS Injection via Avatar Color (Informational)
The spec calls for a deterministic avatar color derived from a person ID via a hash. If this hash is ever used to set an inline
styleattribute (e.g.style="background-color: {personAvatarColor(person.id)}"), and the returned value is not constrained to the predefined palette, there's a theoretical CSS injection vector.This is low risk in practice because the hash returns one of 5 hardcoded hex literals — user input never flows into the CSS value directly. But:
PALETTE[hash(id) % PALETTE.length]idinto the returned colour stringNo action needed if implemented as above — just flagging the pattern to watch for.
Observation 2:
aria-haspopupAttribute Type (Informational)The spec shows:
The ARIA spec defines
aria-haspopupas an enumerated token (menu,listbox,tree,grid,dialog, ortrue). Svelte may renderaria-haspopup="true"as the string"true"rather than the boolean. Given the tooltip is acting as alistbox, the correct value is:This is also what the issue body specifies in the Accessibility section — just making sure the code matches the spec, not the snippet in the "Overflow pill" section which says
"true".Observation 3: Tooltip Links to
/persons/{id}— Confirm ID SourceThe tooltip renders
<a href="/persons/{person.id}">for each overflow receiver. Confirm that:person.idvalues come from the authenticated server load response (they do, per the current architecture)UUID hrefs are safe. If the person model ever gains a slug field and that slug is used here without sanitization, that would need a second look. Not a current concern, just a note for future development.
No Concerns On
annotateModebindable prop is internal UI state, not URL-derived — no risk of external manipulation🎨 Leonie Voss — UI/UX Design Lead
Critical: Font Sizes Below Any Accessible Threshold
This is my primary concern with the spec as written. Several text sizes specified are dangerously small:
text-[5.5px]text-[6.5px]text-[5px]text-[5.5px]text-[5.5px]text-[10px]–text-[12px]WCAG SC 1.4.4 (Resize Text) requires that text can be scaled to 200% without loss of functionality. Text at 6–7px is effectively unreadable at 1× and becomes only marginally usable at 200%. More practically: no user, regardless of age or visual ability, can read 6.5px chip text on a physical device. This appears to be a spec artifact from designing at high zoom in Figma.
My recommendation: Scale up all chip and label text to a minimum of
text-[9px](chip names) andtext-[8px](status labels), with the topbar title attext-xs(text-[12px]) as the minimum. The topbar will need to accommodate this — consider increasing theh-[50px]heights slightly, or reducing horizontal padding to give text more room.Status Chip: Color Is Not the Only Cue — But Check the Implementation
The spec uses green (
bg-emerald-100) forUPLOADED/ARCHIVEDand muted styles for others. Green vs. muted is a color-only distinction for status. This is acceptable only if the status label text is always visible (which it is per the spec —statusLabel(doc.status)renders text). The colored dot adds reinforcement. This passes WCAG 1.4.1 (Use of Color) as long as the text label is always present alongside the color.However: what are the German label strings for each status? The issue doesn't specify.
PLACEHOLDER,UPLOADED, etc. are English enum values — the rendered label should be in German and translated via Paraglide.Touch Targets
The spec states all touch targets must be ≥ 44×44px on mobile. Checking the defined elements:
24×24px— fails the 44×44 requirement. Must be padded to 44×44 minimum withp-2.5or similar, so the visual icon stays small but the hit area is large.min-h-[44px]or padding equivalent on mobile.Overflow Tooltip Accessibility
The ARIA pattern described (role="listbox", role="option") is correct for a non-modal popup. One addition: when the tooltip opens, focus should move to the first
role="option"element (or the panel itself), not remain on the pill. The spec says "Escape should dismiss and return focus to the pill" — for that return to work, focus must have moved into the tooltip first.Dark Mode Tokens
The token mapping table is clear and uses CSS custom properties throughout — this is the right approach. I'd just confirm that
bg-primary/4(used in the hint strip dark mode) is a valid Tailwind opacity modifier.bg-primary/5is listed for light mode; dark uses/4. Both should work as long as--color-primaryis defined as an RGB value (not a hex), which is required for Tailwind's opacity modifiers to function.Minor: Arrow Character for Sender → Receiver
The
→arrow (→) is used inline as text. This should havearia-hidden="true"(already noted in the spec ✓) and should not be a hyphen-minus or ASCII>— the spec uses the correct Unicode directional arrow. Just flagging this so it isn't accidentally simplified during implementation.⚙️ Tobias Wendt — DevOps & Platform Engineer
No Infrastructure Changes Required
This is a pure frontend component change. No new services, no new environment variables, no new Docker volumes or containers, no schema migrations. Clean from an ops perspective — this deploys with the normal frontend build and does not require a coordinated release.
Custom Tailwind Breakpoint — Check Before CI Runs
If a
xs: 375pxbreakpoint is added totailwind.config.ts(as Markus noted), make sure thenpm run checkandnpm run buildsteps in the CI workflow still pass cleanly. Tailwind 4 handles custom screen tokens differently from v3 — confirm the syntax is correct for the version in use, otherwise the build will silently produce wrong output without failing.Dark Mode in CI/Playwright
The acceptance criteria requires visual proof in both light and dark mode. Two questions:
How is dark mode toggled in this project? If it's class-based (
darkclass on<html>) rather thanprefers-color-scheme, Playwright tests need to explicitly add that class rather than usingcolorScheme: 'dark'in the browser context config. These are different mechanisms and need the right approach for tests to actually test dark mode.Does the
/proofshotskill handle multi-theme screenshots automatically? If not, the acceptance criterion "visual proof in both themes" will require manual steps, which should be documented in the PR description so reviewers know what to verify.svelte-checkin CIThe AC includes "svelte-check passes with no new type errors." Confirm this runs as part of the existing CI workflow (it should —
npm run checkis typically included). If the new components introduce any missing prop types or untyped event handlers, it'll catch them before merge. No action needed if it's already wired up — just confirming.Deployment Risk
None. The component is self-contained, server-rendered, and carries no state to the database. A rollback is as simple as reverting the frontend container. Low risk, standard deployment.
One Flag:
proofshot-artifacts/in Git StatusI notice
proofshot-artifacts/directories are showing as untracked in the repo. These should be in.gitignoreif they aren't already — artifact directories from visual proof sessions shouldn't land in the working tree permanently. Not related to this issue, but worth a quick check.🎨 Leonie Voss — Design Review Discussion
Follow-up discussion resolving the open questions from my earlier review comment. All seven items were worked through with the team.
✅ Resolved
1. Font sizes
text-[9px]for all informational text (chip names, hint strip, tooltip header)text-[8px]text-[11px]at XS (nottext-[10px]per spec)h-12(48px) at XS,h-14(56px) at mobile/tablet. Implementer verifies against actual render.2. Status chip — labels removed
title={doc.status}for hover tooltip andaria-labelfor screen readers.3. Back button touch target
w-11 h-11(44×44px).-ml-2) to preserve visual alignment to the left edge.4. Overflow tooltip keyboard flow
role="option"(first person link).≥ 768pxonly — tooltip does not exist on mobile.5. Edit button on small screens
"Bearbeiten"label.aria-label="Bearbeiten"always present on the element.6. Overflow pill on mobile (
< 768px)<span>witharia-hidden="true".7.
--color-primaryCSS format#012851) — Tailwind opacity modifiers (/4,/20) will silently break.--color-primaryto raw RGB channels (1 40 81) inlayout.css.var(--color-primary)to ensure nothing expects a hex string.Overall: the spec is solid and the implementation path is clear. The font size and CSS token issues are the two things that must be addressed before implementation starts — everything else can be handled inline during the build.