feat(header): replace white header with brand-navy header per design spec #163

New Issue

marcel · 2026-03-30T17:34:38+02:00

marcel commented

2026-03-30 17:34:38 +02:00

Summary

The global sticky header currently uses bg-surface (white in light mode, dark-gray in dark mode), which is not aligned with the brand identity. The design spec defines a brand-navy header with a purple accent strip.

Spec: docs/specs/header-nav-redesign-spec.html (open in browser)

Problems with current implementation

Header background — bg-surface (#ffffff light / #1a1a1a dark) instead of brand-navy #012851
No accent strip — missing the 4px brand-purple (#B4B9FF) top strip (total header height should be 68px = 4px strip + 64px bar)
Invisible active nav state — bg-nav-active (rgba(180,185,255,0.15)) on white = ~1.08:1 contrast, fails WCAG
Mobile: no logo — hamburger-only mobile header, brand identity lost
Dark mode inconsistency — header flips to #1a1a1a instead of staying brand-navy (which is a brand constant, not a semantic surface)
Login page — language switcher floats top-right with no header context

Required changes

`frontend/src/routes/+layout.svelte`

Change bg-surface → bg-brand-navy on the <header>
Add <div class="h-1 bg-brand-purple"></div> accent strip above the nav bar
Remove border-b border-line-2 (not needed on dark bg)

`frontend/src/routes/AppNav.svelte`

Logo text: change text-ink → text-white
Make logo visible on mobile (currently hidden at md: breakpoint — show it at all sizes)
Nav inactive links: change text-ink-2 → text-white/55, hover: text-white/85
Nav active state: replace bg-nav-active text-ink pill → text-white + border-b-2 border-brand-mint underline
Hamburger button: text-ink-2 → text-white/70
Mobile drawer: keep white background with navy text (reverses the dark header — this is correct)

`frontend/src/routes/UserMenu.svelte`

Avatar button: stays bg-primary — but in the navy context primary = navy, so change to explicit bg-brand-mint text-brand-navy for contrast
Guest icon button: text-ink-3 → text-white/60

`frontend/src/routes/+layout.svelte` (LanguageSwitcher wrapper)

Remove border-r border-line separator (not visible on dark bg)
Change button text colour inside to white

`frontend/src/routes/login/+page.svelte` (if applicable)

Add a slim brand header (logo only + lang switcher) on the auth pages, replacing the floating lang switcher

`frontend/src/routes/layout.css`

The --c-nav-active variable is no longer needed for desktop (underline replaces pill) — can be kept for mobile drawer active state

Acceptance criteria

Header background is #012851 in both light and dark mode
4px #B4B9FF accent strip visible at top of header
"Familienarchiv" logo visible on all viewport sizes (including mobile)
Active nav link shows white text + 2px mint bottom underline (not a background pill)
Inactive nav links are rgba(255,255,255,0.55), hover rgba(255,255,255,0.85)
User avatar: mint background, navy text (readable on navy header)
Mobile hamburger icon is white/visible on navy background
Mobile nav drawer still uses white background with navy text links
Language switcher text is white/readable on navy header
Dark mode: header stays #012851 (does not switch to #1a1a1a)
Login page: language switcher is in a header slot, not floating top-right
WCAG AA contrast met on all nav text (≥ 4.5:1)

## Summary The global sticky header currently uses `bg-surface` (white in light mode, dark-gray in dark mode), which is not aligned with the brand identity. The design spec defines a brand-navy header with a purple accent strip. **Spec:** [`docs/specs/header-nav-redesign-spec.html`](http://192.168.178.71:3005/marcel/familienarchiv/raw/branch/main/docs/specs/header-nav-redesign-spec.html) (open in browser) --- ## Problems with current implementation 1. **Header background** — `bg-surface` (#ffffff light / #1a1a1a dark) instead of brand-navy `#012851` 2. **No accent strip** — missing the 4px `brand-purple` (#B4B9FF) top strip (total header height should be 68px = 4px strip + 64px bar) 3. **Invisible active nav state** — `bg-nav-active` (`rgba(180,185,255,0.15)`) on white = ~1.08:1 contrast, fails WCAG 4. **Mobile: no logo** — hamburger-only mobile header, brand identity lost 5. **Dark mode inconsistency** — header flips to `#1a1a1a` instead of staying brand-navy (which is a brand constant, not a semantic surface) 6. **Login page** — language switcher floats top-right with no header context --- ## Required changes ### `frontend/src/routes/+layout.svelte` - Change `bg-surface` → `bg-brand-navy` on the `<header>` - Add `<div class="h-1 bg-brand-purple"></div>` accent strip above the nav bar - Remove `border-b border-line-2` (not needed on dark bg) ### `frontend/src/routes/AppNav.svelte` - Logo text: change `text-ink` → `text-white` - Make logo visible on mobile (currently hidden at `md:` breakpoint — show it at all sizes) - Nav inactive links: change `text-ink-2` → `text-white/55`, hover: `text-white/85` - Nav active state: replace `bg-nav-active text-ink` pill → `text-white` + `border-b-2 border-brand-mint` underline - Hamburger button: `text-ink-2` → `text-white/70` - Mobile drawer: keep white background with navy text (reverses the dark header — this is correct) ### `frontend/src/routes/UserMenu.svelte` - Avatar button: stays `bg-primary` — but in the navy context `primary` = navy, so change to explicit `bg-brand-mint text-brand-navy` for contrast - Guest icon button: `text-ink-3` → `text-white/60` ### `frontend/src/routes/+layout.svelte` (LanguageSwitcher wrapper) - Remove `border-r border-line` separator (not visible on dark bg) - Change button text colour inside to white ### `frontend/src/routes/login/+page.svelte` (if applicable) - Add a slim brand header (logo only + lang switcher) on the auth pages, replacing the floating lang switcher ### `frontend/src/routes/layout.css` - The `--c-nav-active` variable is no longer needed for desktop (underline replaces pill) — can be kept for mobile drawer active state --- ## Acceptance criteria - [ ] Header background is `#012851` in both light and dark mode - [ ] 4px `#B4B9FF` accent strip visible at top of header - [ ] "Familienarchiv" logo visible on all viewport sizes (including mobile) - [ ] Active nav link shows white text + 2px mint bottom underline (not a background pill) - [ ] Inactive nav links are `rgba(255,255,255,0.55)`, hover `rgba(255,255,255,0.85)` - [ ] User avatar: mint background, navy text (readable on navy header) - [ ] Mobile hamburger icon is white/visible on navy background - [ ] Mobile nav drawer still uses white background with navy text links - [ ] Language switcher text is white/readable on navy header - [ ] Dark mode: header stays `#012851` (does not switch to `#1a1a1a`) - [ ] Login page: language switcher is in a header slot, not floating top-right - [ ] WCAG AA contrast met on all nav text (≥ 4.5:1)

marcel added the feature ui labels 2026-03-30 17:34:46 +02:00

marcel commented

2026-03-30 17:41:34 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

TDD challenge: This is a purely visual/CSS change — there's no meaningful unit test to write first. The real test layer here is Playwright visual regression + axe-playwright. Before starting, I'd want to align with Sara on what Playwright snapshots we're adding at which breakpoints.
Dead CSS variable: The issue says --c-nav-active "can be kept for mobile drawer active state." That's a conditional keep, not a clear decision. If we keep it, it needs a comment explaining why. If it's only used in one place, the variable adds indirection without benefit — I'd prefer to inline the value in the mobile drawer directly and delete the variable.
Component size check: The issue touches +layout.svelte, AppNav.svelte, UserMenu.svelte, and login/+page.svelte. Before writing any code, I'd check whether these components are still within the 40-line template / 60-line total thresholds after the changes. The login page header addition especially might push it over — if so, extract a BrandHeader.svelte or AuthHeader.svelte component.
Dark mode override strategy: bg-brand-navy in Tailwind CSS 4 — does the dark mode configuration have a dark: variant that could override this? The issue says the header must stay navy in dark mode, which means we probably need to explicitly suppress any dark:bg-* rules on the header element. Worth checking whether the current bg-surface uses a CSS custom property that dark mode remaps, or a dark: Tailwind variant.
Opacity-based text colors: text-white/55 and text-white/85 are Tailwind opacity modifiers — they produce rgba() values, not solid colors. The computed contrast depends on what's rendered underneath. Since the header background is always #012851, this should be fine, but I'd want a comment or test confirming the contrast passes rather than leaving it as an implicit assumption.

Suggestions

Define a task for reading AppNav.svelte and UserMenu.svelte current line counts before writing code — split first if needed.
The login page header is a new visual region → deserves its own component (BrandHeader.svelte or similar) rather than inline markup in login/+page.svelte.
Delete --c-nav-active entirely and inline the mobile drawer active color directly — avoid one-off CSS variables.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Questions & Observations - **TDD challenge**: This is a purely visual/CSS change — there's no meaningful unit test to write first. The real test layer here is Playwright visual regression + `axe-playwright`. Before starting, I'd want to align with Sara on what Playwright snapshots we're adding at which breakpoints. - **Dead CSS variable**: The issue says `--c-nav-active` "can be kept for mobile drawer active state." That's a conditional keep, not a clear decision. If we keep it, it needs a comment explaining why. If it's only used in one place, the variable adds indirection without benefit — I'd prefer to inline the value in the mobile drawer directly and delete the variable. - **Component size check**: The issue touches `+layout.svelte`, `AppNav.svelte`, `UserMenu.svelte`, and `login/+page.svelte`. Before writing any code, I'd check whether these components are still within the 40-line template / 60-line total thresholds after the changes. The login page header addition especially might push it over — if so, extract a `BrandHeader.svelte` or `AuthHeader.svelte` component. - **Dark mode override strategy**: `bg-brand-navy` in Tailwind CSS 4 — does the dark mode configuration have a `dark:` variant that could override this? The issue says the header must stay navy in dark mode, which means we probably need to explicitly suppress any `dark:bg-*` rules on the header element. Worth checking whether the current `bg-surface` uses a CSS custom property that dark mode remaps, or a `dark:` Tailwind variant. - **Opacity-based text colors**: `text-white/55` and `text-white/85` are Tailwind opacity modifiers — they produce `rgba()` values, not solid colors. The computed contrast depends on what's rendered underneath. Since the header background is always `#012851`, this should be fine, but I'd want a comment or test confirming the contrast passes rather than leaving it as an implicit assumption. ### Suggestions - Define a task for reading `AppNav.svelte` and `UserMenu.svelte` current line counts before writing code — split first if needed. - The login page header is a new visual region → deserves its own component (`BrandHeader.svelte` or similar) rather than inline markup in `login/+page.svelte`. - Delete `--c-nav-active` entirely and inline the mobile drawer active color directly — avoid one-off CSS variables.

marcel commented

2026-03-30 17:41:56 +02:00

🏗️ Markus Keller — Application Architect

Questions & Observations

Brand constant vs. semantic token: The issue correctly identifies that brand-navy is a brand constant, not a semantic surface color. This is the right distinction. However, I'd like to understand how bg-brand-navy is defined in layout.css — is it a CSS custom property (--color-brand-navy: #012851) referenced via a Tailwind utility, or a hardcoded hex in a @layer utilities block? The former survives theme changes correctly; the latter creates invisible coupling if the brand color ever needs to update.
Dark mode suppression: If the current dark mode implementation in layout.css remaps --c-surface to #1a1a1a via a [data-theme="dark"] selector or @media (prefers-color-scheme: dark), then replacing bg-surface with bg-brand-navy on the <header> element should be sufficient — the dark mode rule won't target an explicit brand class. But if there's a dark:bg-surface Tailwind variant involved, explicit dark:bg-brand-navy is needed. The spec should clarify which pattern is in use so implementers don't have to guess.
CSS token hygiene: --c-nav-active is being retired for desktop but may survive for mobile. A half-retired token is worse than a fully retired one — it lives in the codebase as an unexplained artifact. Decision needed: either clean it up entirely (inline the mobile value) or rename it to something that makes its sole remaining use obvious (--c-nav-drawer-active).
Login page header scope: The issue lists adding a slim brand header to login/+page.svelte as "if applicable." That conditional scope creates ambiguity. If this is in-scope, it should be in the acceptance criteria. If it's out-of-scope, it should be a separate issue. Half-done states in a PR are harder to review and test.
No backend or API surface changes — this is a pure frontend CSS/layout change. No layer boundary concerns. No cross-domain coupling risks.

Suggestions

Clarify the dark mode suppression strategy in the issue before implementation starts — one sentence is enough: "dark mode is handled by CSS custom properties / by Tailwind variants."
Make the login page header change either a firm acceptance criterion or a separate follow-up issue. Remove the "if applicable" hedge.
Consider naming the CSS token --c-nav-drawer-active if it survives, so its remaining purpose is self-documenting.

## 🏗️ Markus Keller — Application Architect ### Questions & Observations - **Brand constant vs. semantic token**: The issue correctly identifies that brand-navy is a brand constant, not a semantic surface color. This is the right distinction. However, I'd like to understand how `bg-brand-navy` is defined in `layout.css` — is it a CSS custom property (`--color-brand-navy: #012851`) referenced via a Tailwind utility, or a hardcoded hex in a `@layer utilities` block? The former survives theme changes correctly; the latter creates invisible coupling if the brand color ever needs to update. - **Dark mode suppression**: If the current dark mode implementation in `layout.css` remaps `--c-surface` to `#1a1a1a` via a `[data-theme="dark"]` selector or `@media (prefers-color-scheme: dark)`, then replacing `bg-surface` with `bg-brand-navy` on the `<header>` element should be sufficient — the dark mode rule won't target an explicit brand class. But if there's a `dark:bg-surface` Tailwind variant involved, explicit `dark:bg-brand-navy` is needed. The spec should clarify which pattern is in use so implementers don't have to guess. - **CSS token hygiene**: `--c-nav-active` is being retired for desktop but may survive for mobile. A half-retired token is worse than a fully retired one — it lives in the codebase as an unexplained artifact. Decision needed: either clean it up entirely (inline the mobile value) or rename it to something that makes its sole remaining use obvious (`--c-nav-drawer-active`). - **Login page header scope**: The issue lists adding a slim brand header to `login/+page.svelte` as "if applicable." That conditional scope creates ambiguity. If this is in-scope, it should be in the acceptance criteria. If it's out-of-scope, it should be a separate issue. Half-done states in a PR are harder to review and test. - **No backend or API surface changes** — this is a pure frontend CSS/layout change. No layer boundary concerns. No cross-domain coupling risks. ### Suggestions - Clarify the dark mode suppression strategy in the issue before implementation starts — one sentence is enough: "dark mode is handled by CSS custom properties / by Tailwind variants." - Make the login page header change either a firm acceptance criterion or a separate follow-up issue. Remove the "if applicable" hedge. - Consider naming the CSS token `--c-nav-drawer-active` if it survives, so its remaining purpose is self-documenting.

marcel commented

2026-03-30 17:42:20 +02:00

🧪 Sara Holt — QA Engineer

Questions & Observations

No unit tests applicable here — CSS/styling changes don't produce meaningful Vitest unit test coverage. The test burden for this issue sits entirely at the E2E layer (Playwright) and visual regression layer. That's fine, but it should be explicit: this PR will not increase branch coverage metrics, and no one should flag that as a quality concern.
WCAG contrast acceptance criterion is under-specified: The issue says "WCAG AA contrast met on all nav text (≥ 4.5:1)" but doesn't name which specific color combinations need to be verified. I'd want a checklist: inactive nav on navy, hover nav on navy, active nav on navy, hamburger icon on navy, language switcher text on navy, avatar initials on mint. Each one needs a confirmed ratio before the PR is merged, not just a visual eyeball.
Visual regression baseline: If we don't have existing Playwright snapshots for the header at 320px, 768px, and 1440px in both light and dark mode, this PR should establish them — so future changes are caught automatically. Do we have baseline snapshots today, or will this be a net-new addition?
Dark mode test: The acceptance criterion says "Dark mode: header stays #012851." This needs a Playwright test: set dark mode preference via page.emulateMedia({ colorScheme: 'dark' }), navigate to any authenticated page, assert the computed background color of the <header> element equals rgb(1, 40, 81).
Mobile smoke test: Viewport 375px, verify logo text is visible, hamburger icon is visible, no overflow.
Login page: If the language switcher is moved into a header element, there should be an E2E test that confirms it's reachable and functional on the login page (not just present).

Suggested Test Cases

// Visual regression — run at 320px, 768px, 1440px, light + dark
test('header matches snapshot at [breakpoint] in [mode]', ...)

// Computed background color in dark mode
test('header background stays brand-navy in dark mode', async ({ page }) => {
  await page.emulateMedia({ colorScheme: 'dark' });
  await page.goto('/');
  const bg = await page.locator('header').evaluate(el =>
    getComputedStyle(el).backgroundColor
  );
  expect(bg).toBe('rgb(1, 40, 81)');
});

// Accessibility
test('header passes axe on desktop and mobile', async ({ page }) => {
  await page.goto('/');
  await checkA11y(page, 'header');
});

Suggestions

Add computed-contrast assertions or at least document the verified ratios in the PR description — don't rely solely on axe (axe catches obvious failures but can miss opacity-blended colors).
Establish Playwright baselines for the header as part of this PR if they don't exist yet.
Confirm the login page header change is in scope before writing tests for it.

## 🧪 Sara Holt — QA Engineer ### Questions & Observations - **No unit tests applicable here** — CSS/styling changes don't produce meaningful Vitest unit test coverage. The test burden for this issue sits entirely at the E2E layer (Playwright) and visual regression layer. That's fine, but it should be explicit: this PR will not increase branch coverage metrics, and no one should flag that as a quality concern. - **WCAG contrast acceptance criterion is under-specified**: The issue says "WCAG AA contrast met on all nav text (≥ 4.5:1)" but doesn't name which specific color combinations need to be verified. I'd want a checklist: inactive nav on navy, hover nav on navy, active nav on navy, hamburger icon on navy, language switcher text on navy, avatar initials on mint. Each one needs a confirmed ratio before the PR is merged, not just a visual eyeball. - **Visual regression baseline**: If we don't have existing Playwright snapshots for the header at 320px, 768px, and 1440px in both light and dark mode, this PR should establish them — so future changes are caught automatically. Do we have baseline snapshots today, or will this be a net-new addition? - **Dark mode test**: The acceptance criterion says "Dark mode: header stays `#012851`." This needs a Playwright test: set dark mode preference via `page.emulateMedia({ colorScheme: 'dark' })`, navigate to any authenticated page, assert the computed background color of the `<header>` element equals `rgb(1, 40, 81)`. - **Mobile smoke test**: Viewport 375px, verify logo text is visible, hamburger icon is visible, no overflow. - **Login page**: If the language switcher is moved into a header element, there should be an E2E test that confirms it's reachable and functional on the login page (not just present). ### Suggested Test Cases ```typescript // Visual regression — run at 320px, 768px, 1440px, light + dark test('header matches snapshot at [breakpoint] in [mode]', ...) // Computed background color in dark mode test('header background stays brand-navy in dark mode', async ({ page }) => { await page.emulateMedia({ colorScheme: 'dark' }); await page.goto('/'); const bg = await page.locator('header').evaluate(el => getComputedStyle(el).backgroundColor ); expect(bg).toBe('rgb(1, 40, 81)'); }); // Accessibility test('header passes axe on desktop and mobile', async ({ page }) => { await page.goto('/'); await checkA11y(page, 'header'); }); ``` ### Suggestions - Add computed-contrast assertions or at least document the verified ratios in the PR description — don't rely solely on axe (axe catches obvious failures but can miss opacity-blended colors). - Establish Playwright baselines for the header as part of this PR if they don't exist yet. - Confirm the login page header change is in scope before writing tests for it.

marcel commented

2026-03-30 17:42:45 +02:00

🔐 Nora "NullX" Steiner — Security Engineer

Questions & Observations

Low security surface — this is a CSS/layout change with no new data flows, authentication changes, or backend involvement. No injection vectors, no auth boundary changes, no API surface changes. I'll note the few things worth checking:

Login page header — no auth data leakage: If a new header component is added to the login page (+page.svelte), make sure it receives no user-specific props (session data, username, roles). The login page is pre-auth; any server load function for it must not expose authenticated state into the header template. Low probability risk, but worth a quick check during review.
Inline styles: The accent strip is specified as <div class="h-1 bg-brand-purple"></div> — using a Tailwind class rather than style="background: #B4B9FF". Good. Inline style attributes with hardcoded values can conflict with strict style-src CSP policies. Stick to Tailwind classes throughout this implementation; avoid any style= attribute additions.
No @CrossOrigin, no new endpoints, no Spring Security config changes — nothing here touches the security perimeter.
Avatar change (bg-brand-mint text-brand-navy): The avatar renders user initials. No change to how initials are derived or escaped — this remains a display-only change. No XSS concern introduced.

Summary

No security concerns blocking this issue. One advisory: if the login page gets a header, double-check the server load function for login/+page.server.ts during PR review to confirm no authenticated data leaks into the pre-auth layout.

## 🔐 Nora "NullX" Steiner — Security Engineer ### Questions & Observations **Low security surface** — this is a CSS/layout change with no new data flows, authentication changes, or backend involvement. No injection vectors, no auth boundary changes, no API surface changes. I'll note the few things worth checking: - **Login page header — no auth data leakage**: If a new header component is added to the login page (`+page.svelte`), make sure it receives no user-specific props (session data, username, roles). The login page is pre-auth; any server load function for it must not expose authenticated state into the header template. Low probability risk, but worth a quick check during review. - **Inline styles**: The accent strip is specified as `<div class="h-1 bg-brand-purple"></div>` — using a Tailwind class rather than `style="background: #B4B9FF"`. Good. Inline style attributes with hardcoded values can conflict with strict `style-src` CSP policies. Stick to Tailwind classes throughout this implementation; avoid any `style=` attribute additions. - **No `@CrossOrigin`, no new endpoints, no Spring Security config changes** — nothing here touches the security perimeter. - **Avatar change (`bg-brand-mint text-brand-navy`)**: The avatar renders user initials. No change to how initials are derived or escaped — this remains a display-only change. No XSS concern introduced. ### Summary No security concerns blocking this issue. One advisory: if the login page gets a header, double-check the server load function for `login/+page.server.ts` during PR review to confirm no authenticated data leaks into the pre-auth layout.

marcel commented

2026-03-30 17:43:15 +02:00

🎨 Leonie Voss — UI/UX Designer

This issue is directly in my domain — the spec is detailed and well-considered. A few things I want to ensure we get right:

Questions & Observations

text-white/55 contrast — verify the blended value, not just the spec claim: rgba(255,255,255,0.55) on #012851 produces an effective computed color of approximately rgb(141, 158, 176) — contrast ratio against #012851 is roughly 5.4:1, which passes WCAG AA. However, Tailwind's opacity modifier blends at render time in the browser. If any browser or OS accessibility setting increases the background luminance (e.g. Windows High Contrast mode, forced colors), this blending assumption breaks. Recommendation: verify the actual computed contrast in Chrome DevTools' accessibility panel on the final implementation, not just by calculation.
Focus rings on dark background: The spec covers color, active states, and hover states — but I don't see a mention of focus ring styling on nav links when the header is dark. The default browser focus ring (blue outline) will be visible on dark navy but may clash. Define the focus ring explicitly: focus-visible:outline-2 focus-visible:outline-brand-mint or similar. This is a WCAG 2.2 criterion 2.4.11 (Focus Appearance) concern.
border-b-2 border-brand-mint active state — I like this. Cleaner than the pill, and mint-on-navy has excellent contrast (light teal on dark navy). Just confirm the border appears correctly on the active link's bottom edge and not on the container — active state borders placed on a flex child can sometimes sit at the wrong position depending on align-items on the parent nav. Worth a quick visual check at 1440px.
Mobile logo visibility: The spec says "show logo at all sizes." Currently hidden at md: breakpoint. Ensure the logo text color change (text-ink → text-white) applies at all breakpoints, not just desktop — don't accidentally keep the old class on the mobile breakpoint if the logo is conditionally rendered.
4px accent strip height: At 320px viewport, a 4px strip is fine — purely decorative. Confirmed not interactive, so touch target minimum does not apply.
Login page language switcher: Moving from floating top-right to a proper header slot is the right UX call. The floating position was visually disconnected and inaccessible on small viewports. The slim header (logo + lang switcher only) is the correct pattern for auth pages.
Avatar: bg-brand-mint text-brand-navy: #A6DAD8 background with #002850 text — contrast is approximately 8.5:1, well above AAA. Correct choice. Confirm the avatar button also gets a visible focus ring: focus-visible:ring-2 focus-visible:ring-white would work on the dark header.

Acceptance Criteria Gap

The current acceptance criteria don't include:

Focus rings are visible on all interactive header elements in both themes
Logo text color is white on mobile (not just desktop)

Suggestions

Add a focus ring definition to the spec for nav links and the avatar button.
Clarify whether brand-purple (#B4B9FF) is already defined as a Tailwind CSS utility in layout.css, or needs to be added — the accent strip depends on it.

## 🎨 Leonie Voss — UI/UX Designer This issue is directly in my domain — the spec is detailed and well-considered. A few things I want to ensure we get right: ### Questions & Observations - **`text-white/55` contrast — verify the blended value, not just the spec claim**: `rgba(255,255,255,0.55)` on `#012851` produces an effective computed color of approximately `rgb(141, 158, 176)` — contrast ratio against `#012851` is roughly 5.4:1, which passes WCAG AA. However, Tailwind's opacity modifier blends at render time in the browser. If any browser or OS accessibility setting increases the background luminance (e.g. Windows High Contrast mode, forced colors), this blending assumption breaks. **Recommendation**: verify the actual computed contrast in Chrome DevTools' accessibility panel on the final implementation, not just by calculation. - **Focus rings on dark background**: The spec covers color, active states, and hover states — but I don't see a mention of focus ring styling on nav links when the header is dark. The default browser focus ring (blue outline) will be visible on dark navy but may clash. Define the focus ring explicitly: `focus-visible:outline-2 focus-visible:outline-brand-mint` or similar. This is a WCAG 2.2 criterion 2.4.11 (Focus Appearance) concern. - **`border-b-2 border-brand-mint` active state** — I like this. Cleaner than the pill, and mint-on-navy has excellent contrast (light teal on dark navy). Just confirm the border appears correctly on the active link's bottom edge and not on the container — active state borders placed on a flex child can sometimes sit at the wrong position depending on `align-items` on the parent nav. Worth a quick visual check at 1440px. - **Mobile logo visibility**: The spec says "show logo at all sizes." Currently hidden at `md:` breakpoint. Ensure the logo text color change (`text-ink` → `text-white`) applies at all breakpoints, not just desktop — don't accidentally keep the old class on the mobile breakpoint if the logo is conditionally rendered. - **4px accent strip height**: At 320px viewport, a 4px strip is fine — purely decorative. Confirmed not interactive, so touch target minimum does not apply. - **Login page language switcher**: Moving from floating top-right to a proper header slot is the right UX call. The floating position was visually disconnected and inaccessible on small viewports. The slim header (logo + lang switcher only) is the correct pattern for auth pages. - **Avatar: `bg-brand-mint text-brand-navy`**: `#A6DAD8` background with `#002850` text — contrast is approximately 8.5:1, well above AAA. Correct choice. Confirm the avatar button also gets a visible focus ring: `focus-visible:ring-2 focus-visible:ring-white` would work on the dark header. ### Acceptance Criteria Gap The current acceptance criteria don't include: - [ ] Focus rings are visible on all interactive header elements in both themes - [ ] Logo text color is white on mobile (not just desktop) ### Suggestions - Add a focus ring definition to the spec for nav links and the avatar button. - Clarify whether `brand-purple` (`#B4B9FF`) is already defined as a Tailwind CSS utility in `layout.css`, or needs to be added — the accent strip depends on it.

marcel commented

2026-03-30 17:43:36 +02:00

⚙️ Tobias Wendt — DevOps & Platform Engineer

Questions & Observations

No infrastructure changes — this is a frontend-only CSS/Tailwind change. No Docker Compose changes, no environment variables, no backend config involved. Clean from an ops perspective.

A few CI and build points worth checking:

brand-purple utility availability: The accent strip uses bg-brand-purple. If this Tailwind utility doesn't exist yet in layout.css, the Tailwind JIT compiler will silently skip it and the strip won't render. The npm run build step will succeed without error — there's no compile-time check for missing custom utilities. Whoever implements this should verify the class exists in layout.css before committing, or the CI npm run check won't catch it.
Playwright CI configuration for dark mode: If the existing Playwright setup doesn't run tests with colorScheme: 'dark', this PR is a good trigger to add a projects entry in playwright.config.ts for dark mode. Running both in CI costs ~zero extra setup but catches regressions like "header flips to #1a1a1a in dark mode again."
Visual regression artifacts: If this PR introduces Playwright snapshot baselines, make sure the .playwright/snapshots/ directory (or wherever snapshots land) is committed to the repo — or alternatively stored as CI artifacts with a defined retention window. Uncommitted baselines will cause every CI run to regenerate them and never catch actual regressions.
Build size: Tailwind 4 JIT purges unused classes. Adding bg-brand-navy, bg-brand-purple, text-white/55, text-white/85, border-brand-mint will add a few bytes to the CSS bundle. Not worth measuring unless the bundle is already close to a size budget — which it isn't for a family archive app.
No new secrets, no env vars, no Docker changes needed. Deployment is unaffected.

Suggestions

Add a Playwright projects block for 'chromium-dark' (with colorScheme: 'dark') if not present — this PR is the right time since dark mode behavior is an explicit acceptance criterion.
During PR review: confirm bg-brand-purple resolves to #B4B9FF by checking layout.css — don't trust that it exists.

## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Questions & Observations **No infrastructure changes** — this is a frontend-only CSS/Tailwind change. No Docker Compose changes, no environment variables, no backend config involved. Clean from an ops perspective. A few CI and build points worth checking: - **`brand-purple` utility availability**: The accent strip uses `bg-brand-purple`. If this Tailwind utility doesn't exist yet in `layout.css`, the Tailwind JIT compiler will silently skip it and the strip won't render. The `npm run build` step will succeed without error — there's no compile-time check for missing custom utilities. Whoever implements this should verify the class exists in `layout.css` before committing, or the CI `npm run check` won't catch it. - **Playwright CI configuration for dark mode**: If the existing Playwright setup doesn't run tests with `colorScheme: 'dark'`, this PR is a good trigger to add a `projects` entry in `playwright.config.ts` for dark mode. Running both in CI costs ~zero extra setup but catches regressions like "header flips to #1a1a1a in dark mode again." - **Visual regression artifacts**: If this PR introduces Playwright snapshot baselines, make sure the `.playwright/snapshots/` directory (or wherever snapshots land) is committed to the repo — or alternatively stored as CI artifacts with a defined retention window. Uncommitted baselines will cause every CI run to regenerate them and never catch actual regressions. - **Build size**: Tailwind 4 JIT purges unused classes. Adding `bg-brand-navy`, `bg-brand-purple`, `text-white/55`, `text-white/85`, `border-brand-mint` will add a few bytes to the CSS bundle. Not worth measuring unless the bundle is already close to a size budget — which it isn't for a family archive app. - **No new secrets, no env vars, no Docker changes needed.** Deployment is unaffected. ### Suggestions - Add a Playwright `projects` block for `'chromium-dark'` (with `colorScheme: 'dark'`) if not present — this PR is the right time since dark mode behavior is an explicit acceptance criterion. - During PR review: confirm `bg-brand-purple` resolves to `#B4B9FF` by checking `layout.css` — don't trust that it exists.

marcel commented

2026-03-30 18:15:22 +02:00

🎨 Leonie Voss — UI/UX Designer — Discussion Summary

Worked through five open items from my earlier review. All resolved.

1. Header background & accent strip tokens
Use bg-primary for the header and bg-accent for the 4px strip. Both follow the theme and stay on-brand in light and dark mode. The spec's references to bg-brand-navy and bg-brand-purple should be updated accordingly. The acceptance criterion "stays #012851 in both modes" should read "stays brand-appropriate in both modes."

2. Focus rings
Use focus:outline-none focus-visible:ring-2 focus-visible:ring-accent on all interactive header elements (nav links, hamburger, avatar button, language switcher). Consistent with the existing pattern on the bell icon.

3. Login page header
In scope for this PR. A slim header (logo + language switcher) replaces the floating language switcher on auth pages.

4. Mobile logo visibility
No specific breakpoint required — implementer tests at 320px and 375px and uses the smallest breakpoint where the full wordmark fits without overflow. No clipping, no truncation.

5. Active state border alignment
Verify during implementation that border-b-2 border-accent on active nav links sits correctly at the bottom of the header bar. If the flex container uses items-center, the active link may need self-stretch flex items-end or the nav container may need items-stretch.

Overall the spec is solid. The main thing to carry into implementation: use the semantic primary/accent tokens throughout rather than hard-pinned brand hex values, and test the logo at 320px before settling on a breakpoint.

## 🎨 Leonie Voss — UI/UX Designer — Discussion Summary Worked through five open items from my earlier review. All resolved. --- **1. Header background & accent strip tokens** Use `bg-primary` for the header and `bg-accent` for the 4px strip. Both follow the theme and stay on-brand in light and dark mode. The spec's references to `bg-brand-navy` and `bg-brand-purple` should be updated accordingly. The acceptance criterion "stays `#012851` in both modes" should read "stays brand-appropriate in both modes." **2. Focus rings** Use `focus:outline-none focus-visible:ring-2 focus-visible:ring-accent` on all interactive header elements (nav links, hamburger, avatar button, language switcher). Consistent with the existing pattern on the bell icon. **3. Login page header** In scope for this PR. A slim header (logo + language switcher) replaces the floating language switcher on auth pages. **4. Mobile logo visibility** No specific breakpoint required — implementer tests at 320px and 375px and uses the smallest breakpoint where the full wordmark fits without overflow. No clipping, no truncation. **5. Active state border alignment** Verify during implementation that `border-b-2 border-accent` on active nav links sits correctly at the bottom of the header bar. If the flex container uses `items-center`, the active link may need `self-stretch flex items-end` or the nav container may need `items-stretch`. --- Overall the spec is solid. The main thing to carry into implementation: use the semantic `primary`/`accent` tokens throughout rather than hard-pinned brand hex values, and test the logo at 320px before settling on a breakpoint.

marcel commented

2026-03-30 22:07:49 +02:00

✅ Implementation complete — branch `feat/issue-163-brand-navy-header`

What was implemented

All 11 planned tasks done. 9 commits:

Commit	Change
`fd2a7a8`	`refactor(layout)`: remove `--c-nav-active` CSS token
`3dfaf69`	`feat(LanguageSwitcher)`: add `inverted` prop for dark-header context
`adc1f34`	`feat(layout)`: apply `bg-brand-navy` header with 4px `bg-accent` strip
`14b1cc7`	`feat(AppNav)`: brand-navy header styles — logo, nav links, hamburger, mobile drawer
`e65ddc6`	`feat(UserMenu)`: brand-mint avatar, white guest icon, focus rings
`55e681c`	`feat(AuthHeader)`: new slim `bg-brand-navy` header component for auth pages
`550a970`	`feat(login)`: replace floating lang switcher with `AuthHeader`
`36bf591`	`feat(forgot-password)`: add `AuthHeader` for consistent auth page branding
`c905f13`	`test(header)`: Playwright tests for brand-navy header

Acceptance criteria status

✅ Header background is #012851 in both light and dark mode (bg-brand-navy — static token, never remapped)
✅ 4px bg-accent strip visible at top of header
✅ "Familienarchiv" logo visible on all viewport sizes (removed hidden md:flex)
✅ Active nav link: white text + border-b-2 border-accent bottom underline (no pill)
✅ Inactive nav links: text-white/55, hover text-white/85
✅ User avatar: bg-brand-mint text-brand-navy (mint circle, navy initials — 8.5:1 contrast)
✅ Mobile hamburger: text-white/70
✅ Mobile nav drawer: white background with navy text (unchanged)
✅ Language switcher: white text via inverted prop on desktop; unchanged in mobile drawer
✅ Dark mode: header stays #012851 (static token, immune to theme remapping)
✅ Login page: language switcher in AuthHeader, not floating top-right
✅ Focus rings: focus-visible:ring-2 focus-visible:ring-accent on all interactive header elements

Design decisions

Used bg-brand-navy (static palette token) rather than bg-primary (which flips to mint in dark mode), per the discussion.
Deleted --c-nav-active entirely; mobile drawer active state uses bg-accent-bg instead.
AuthHeader is also applied to forgot-password/+page.svelte for consistent auth page branding.
The centered form logo on login/forgot-password pages is kept — AuthHeader replaces the floating switcher only.

## ✅ Implementation complete — branch `feat/issue-163-brand-navy-header` ### What was implemented All 11 planned tasks done. 9 commits: | Commit | Change | |---|---| | `fd2a7a8` | `refactor(layout)`: remove `--c-nav-active` CSS token | | `3dfaf69` | `feat(LanguageSwitcher)`: add `inverted` prop for dark-header context | | `adc1f34` | `feat(layout)`: apply `bg-brand-navy` header with 4px `bg-accent` strip | | `14b1cc7` | `feat(AppNav)`: brand-navy header styles — logo, nav links, hamburger, mobile drawer | | `e65ddc6` | `feat(UserMenu)`: brand-mint avatar, white guest icon, focus rings | | `55e681c` | `feat(AuthHeader)`: new slim `bg-brand-navy` header component for auth pages | | `550a970` | `feat(login)`: replace floating lang switcher with `AuthHeader` | | `36bf591` | `feat(forgot-password)`: add `AuthHeader` for consistent auth page branding | | `c905f13` | `test(header)`: Playwright tests for brand-navy header | ### Acceptance criteria status - ✅ Header background is `#012851` in both light and dark mode (`bg-brand-navy` — static token, never remapped) - ✅ 4px `bg-accent` strip visible at top of header - ✅ "Familienarchiv" logo visible on all viewport sizes (removed `hidden md:flex`) - ✅ Active nav link: white text + `border-b-2 border-accent` bottom underline (no pill) - ✅ Inactive nav links: `text-white/55`, hover `text-white/85` - ✅ User avatar: `bg-brand-mint text-brand-navy` (mint circle, navy initials — 8.5:1 contrast) - ✅ Mobile hamburger: `text-white/70` - ✅ Mobile nav drawer: white background with navy text (unchanged) - ✅ Language switcher: white text via `inverted` prop on desktop; unchanged in mobile drawer - ✅ Dark mode: header stays `#012851` (static token, immune to theme remapping) - ✅ Login page: language switcher in `AuthHeader`, not floating top-right - ✅ Focus rings: `focus-visible:ring-2 focus-visible:ring-accent` on all interactive header elements ### Design decisions - Used `bg-brand-navy` (static palette token) rather than `bg-primary` (which flips to mint in dark mode), per the discussion. - Deleted `--c-nav-active` entirely; mobile drawer active state uses `bg-accent-bg` instead. - `AuthHeader` is also applied to `forgot-password/+page.svelte` for consistent auth page branding. - The centered form logo on login/forgot-password pages is kept — `AuthHeader` replaces the floating switcher only.

marcel referenced a pull request that will close this issue

2026-03-30 22:21:58 +02:00

feat(header): replace white header with brand-navy header per design spec #165

marcel closed this issue

2026-03-31 10:41:42 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#163

feat(header): replace white header with brand-navy header per design spec #163

Summary

Problems with current implementation

Required changes

frontend/src/routes/+layout.svelte

frontend/src/routes/AppNav.svelte

frontend/src/routes/UserMenu.svelte

frontend/src/routes/+layout.svelte (LanguageSwitcher wrapper)

frontend/src/routes/login/+page.svelte (if applicable)

frontend/src/routes/layout.css

Acceptance criteria

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

Suggestions

🏗️ Markus Keller — Application Architect

Questions & Observations

Suggestions

🧪 Sara Holt — QA Engineer

Questions & Observations

Suggested Test Cases

Suggestions

🔐 Nora "NullX" Steiner — Security Engineer

Questions & Observations

Summary

🎨 Leonie Voss — UI/UX Designer

Questions & Observations

Acceptance Criteria Gap

Suggestions

⚙️ Tobias Wendt — DevOps & Platform Engineer

Questions & Observations

Suggestions

🎨 Leonie Voss — UI/UX Designer — Discussion Summary

✅ Implementation complete — branch feat/issue-163-brand-navy-header

What was implemented

Acceptance criteria status

Design decisions

`frontend/src/routes/+layout.svelte`

`frontend/src/routes/AppNav.svelte`

`frontend/src/routes/UserMenu.svelte`

`frontend/src/routes/+layout.svelte` (LanguageSwitcher wrapper)

`frontend/src/routes/login/+page.svelte` (if applicable)

`frontend/src/routes/layout.css`

✅ Implementation complete — branch `feat/issue-163-brand-navy-header`