fix(nav): replace static back-link hrefs with history.back() + fallback #185

New Issue

marcel · 2026-04-06T19:41:58+02:00

marcel commented

2026-04-06 19:41:58 +02:00

Problem

Back navigation buttons across the app use static href values (e.g. href="/persons"). Because many pages have multiple entry points (person detail reachable from search, document view, briefwechsel, etc.), the static link often navigates to the wrong place and confuses users. The browser back button works correctly — the in-app buttons don't.

Affected files (11 locations)

File	Current static href
`persons/new/+page.svelte:10`	`/persons`
`persons/new/+page.svelte:130`	`/persons`
`persons/[id]/+page.svelte:53`	`/persons`
`persons/[id]/edit/+page.svelte:16`	`/persons/{person.id}`
`documents/[id]/edit/+page.svelte:23`	`/documents/{doc.id}`
`documents/[id]/edit/SaveBar.svelte:77`	`/documents/{docId}`
`admin/users/[id]/+page.svelte:37`	`/admin/users`
`admin/users/[id]/+page.svelte:123`	`/admin/users`
`notifications/+page.svelte:71`	`/`
`enrich/+page.svelte:13`	`/`
`enrich/[id]/+page.svelte:49`	`/enrich`

Solution

Replace each <a href="...">Zurück</a> back link with a <button> that calls history.back(), with a sensible fallback for deep-link entry (no history):

<script>
  import { goto } from '$app/navigation';

  function goBack(fallback: string) {
    if (history.length > 1) {
      history.back();
    } else {
      goto(fallback);
    }
  }
</script>

<button
  onclick={() => goBack('/persons')}
  class="inline-flex items-center text-xs font-bold uppercase tracking-widest text-gray-500 hover:text-brand-navy transition-colors group mb-4"
>
  <svg class="w-4 h-4 mr-2 transform group-hover:-translate-x-1 transition-transform" .../>
  Zurück
</button>

The fallback URL should be the natural parent route for each page (same as the current static href).

Acceptance criteria

All back buttons use history.back() as the primary action
A static fallback route is defined for each button (for deep-link entry)
Visual appearance is unchanged
Works at 320px viewport (touch target ≥ 44px)
Screen reader announces the button correctly (aria-label if icon-only)

## Problem Back navigation buttons across the app use static `href` values (e.g. `href="/persons"`). Because many pages have multiple entry points (person detail reachable from search, document view, briefwechsel, etc.), the static link often navigates to the wrong place and confuses users. The browser back button works correctly — the in-app buttons don't. ## Affected files (11 locations) | File | Current static href | |---|---| | `persons/new/+page.svelte:10` | `/persons` | | `persons/new/+page.svelte:130` | `/persons` | | `persons/[id]/+page.svelte:53` | `/persons` | | `persons/[id]/edit/+page.svelte:16` | `/persons/{person.id}` | | `documents/[id]/edit/+page.svelte:23` | `/documents/{doc.id}` | | `documents/[id]/edit/SaveBar.svelte:77` | `/documents/{docId}` | | `admin/users/[id]/+page.svelte:37` | `/admin/users` | | `admin/users/[id]/+page.svelte:123` | `/admin/users` | | `notifications/+page.svelte:71` | `/` | | `enrich/+page.svelte:13` | `/` | | `enrich/[id]/+page.svelte:49` | `/enrich` | ## Solution Replace each `<a href="...">Zurück</a>` back link with a `<button>` that calls `history.back()`, with a sensible fallback for deep-link entry (no history): ```svelte <script> import { goto } from '$app/navigation'; function goBack(fallback: string) { if (history.length > 1) { history.back(); } else { goto(fallback); } } </script> <button onclick={() => goBack('/persons')} class="inline-flex items-center text-xs font-bold uppercase tracking-widest text-gray-500 hover:text-brand-navy transition-colors group mb-4" > <svg class="w-4 h-4 mr-2 transform group-hover:-translate-x-1 transition-transform" .../> Zurück </button> ``` The fallback URL should be the natural parent route for each page (same as the current static href). ## Acceptance criteria - [ ] All back buttons use `history.back()` as the primary action - [ ] A static fallback route is defined for each button (for deep-link entry) - [ ] Visual appearance is unchanged - [ ] Works at 320px viewport (touch target ≥ 44px) - [ ] Screen reader announces the button correctly (aria-label if icon-only)

marcel added the ui label 2026-04-06 19:42:03 +02:00

marcel commented

2026-04-15 12:38:23 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

history.length > 1 is a leaky signal. It tells you "has any navigation happened in this tab" — not "can I navigate back within this app." A user who arrived from an external link (email, Google) and navigated once will have history.length === 2, so history.back() fires instead of the fallback — and sends them off-site. That's identical to the browser back button's behavior, which is probably fine, but it's worth a conscious comment in the code so the next developer understands the trade-off.
SSR guard: history is a browser-only API. It won't be evaluated in an onclick handler (which only fires in the browser), so there's no SSR crash risk as written. But if history.length ever moves into a $derived, a Svelte $effect, or a lifecycle hook, a browser guard from $app/environment will be needed.
history.back() bypasses SvelteKit's nav lifecycle. beforeNavigate and afterNavigate hooks won't fire when the user clicks back. This is fine for the current app, but document edit pages (documents/[id]/edit, persons/[id]/edit) may grow unsaved-changes guards later. history.back() would silently skip them. Worth a // NOTE: comment on those specific pages.
The 4 "check" locations need auditing. users/[id], notifications/, enrich/, enrich/[id] are listed as "check" without a confirmed fallback URL. The issue can't be fully closed until those fallbacks are defined.

Suggestions

Extract to $lib/navigation.ts. Ten duplications of the same three-line function isn't clean. A shared goBack(fallback: string) in $lib/navigation.ts reduces it to a single source. A JSDoc comment explaining the history.length trade-off belongs there too.
Or go one level further: a tiny <BackButton fallback={string} label={string}> component that co-locates the arrow SVG, class list, and function call. If the visual design ever changes (e.g. adding a keyboard shortcut or changing the animation), it changes in one place.
Verify the edit-page fallback is semantically correct. persons/[id]/edit → fallback persons/{person.id} (detail page) is right. documents/[id]/edit → fallback documents/{doc.id} is right. Confirm the SaveBar's duplicate fallback (SaveBar.svelte:77) resolves identically to the top-of-page button.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Questions & Observations - **`history.length > 1` is a leaky signal.** It tells you "has any navigation happened in this tab" — not "can I navigate back within this app." A user who arrived from an external link (email, Google) and navigated once will have `history.length === 2`, so `history.back()` fires instead of the fallback — and sends them off-site. That's identical to the browser back button's behavior, which is probably fine, but it's worth a conscious comment in the code so the next developer understands the trade-off. - **SSR guard:** `history` is a browser-only API. It won't be evaluated in an `onclick` handler (which only fires in the browser), so there's no SSR crash risk as written. But if `history.length` ever moves into a `$derived`, a Svelte `$effect`, or a lifecycle hook, a `browser` guard from `$app/environment` will be needed. - **`history.back()` bypasses SvelteKit's nav lifecycle.** `beforeNavigate` and `afterNavigate` hooks won't fire when the user clicks back. This is fine for the current app, but document edit pages (`documents/[id]/edit`, `persons/[id]/edit`) may grow unsaved-changes guards later. `history.back()` would silently skip them. Worth a `// NOTE:` comment on those specific pages. - **The 4 "check" locations need auditing.** `users/[id]`, `notifications/`, `enrich/`, `enrich/[id]` are listed as "check" without a confirmed fallback URL. The issue can't be fully closed until those fallbacks are defined. ### Suggestions - **Extract to `$lib/navigation.ts`.** Ten duplications of the same three-line function isn't clean. A shared `goBack(fallback: string)` in `$lib/navigation.ts` reduces it to a single source. A JSDoc comment explaining the `history.length` trade-off belongs there too. - **Or go one level further:** a tiny `<BackButton fallback={string} label={string}>` component that co-locates the arrow SVG, class list, and function call. If the visual design ever changes (e.g. adding a keyboard shortcut or changing the animation), it changes in one place. - **Verify the edit-page fallback is semantically correct.** `persons/[id]/edit` → fallback `persons/{person.id}` (detail page) is right. `documents/[id]/edit` → fallback `documents/{doc.id}` is right. Confirm the SaveBar's duplicate fallback (`SaveBar.svelte:77`) resolves identically to the top-of-page button.

marcel commented

2026-04-15 12:38:36 +02:00

🏗️ Markus Keller — Application Architect

Questions & Observations

This is a pure frontend refactor with no backend surface and no data model changes. The scope is tight and well-defined — good.
history.back() and SvelteKit's navigation model are separate systems. When history.back() fires, SvelteKit's beforeNavigate / afterNavigate hooks do not run. Today that's harmless. But if navigation guards (e.g., unsaved-changes confirmation dialogs) are introduced later, they will work for goto() and link-clicks but silently skip history.back(). This is a trade-off, not a blocker — but it should be a deliberate decision with a comment, not an accidental omission discovered later.
Ten duplications signal a missing module boundary. The goBack(fallback) logic belongs in $lib/navigation.ts, not inlined in 10 components. At this scale, extraction is worth the one-time cost. Keep the function small and well-named: navigateBack(fallback: string): void.
The fallback URLs are hardcoded strings. That's fine and intentional — there's nothing dynamic here. But if route paths ever change (e.g. /persons → /people), 10 fallback strings become a maintenance task. A route constants module ($lib/routes.ts) exporting PERSONS = '/persons' etc. would make a future rename a one-line change. Whether that's worth doing now depends on how stable the route structure is.

Suggestions

Extract goBack to $lib/navigation.ts with a brief JSDoc comment explaining the history.length heuristic and its known edge case (off-site history entries).
Consider a $lib/routes.ts constants file for the fallback URLs, particularly for routes that appear in more than one fallback (e.g. /persons appears at least 3 times across the 10 locations).
No ADR needed — this is an implementation detail, not an architectural decision. A code comment at the extraction site is sufficient.

## 🏗️ Markus Keller — Application Architect ### Questions & Observations - This is a pure frontend refactor with no backend surface and no data model changes. The scope is tight and well-defined — good. - **`history.back()` and SvelteKit's navigation model are separate systems.** When `history.back()` fires, SvelteKit's `beforeNavigate` / `afterNavigate` hooks do not run. Today that's harmless. But if navigation guards (e.g., unsaved-changes confirmation dialogs) are introduced later, they will work for `goto()` and link-clicks but silently skip `history.back()`. This is a trade-off, not a blocker — but it should be a deliberate decision with a comment, not an accidental omission discovered later. - **Ten duplications signal a missing module boundary.** The `goBack(fallback)` logic belongs in `$lib/navigation.ts`, not inlined in 10 components. At this scale, extraction is worth the one-time cost. Keep the function small and well-named: `navigateBack(fallback: string): void`. - **The fallback URLs are hardcoded strings.** That's fine and intentional — there's nothing dynamic here. But if route paths ever change (e.g. `/persons` → `/people`), 10 fallback strings become a maintenance task. A route constants module (`$lib/routes.ts`) exporting `PERSONS = '/persons'` etc. would make a future rename a one-line change. Whether that's worth doing now depends on how stable the route structure is. ### Suggestions - Extract `goBack` to `$lib/navigation.ts` with a brief JSDoc comment explaining the `history.length` heuristic and its known edge case (off-site history entries). - Consider a `$lib/routes.ts` constants file for the fallback URLs, particularly for routes that appear in more than one fallback (e.g. `/persons` appears at least 3 times across the 10 locations). - No ADR needed — this is an implementation detail, not an architectural decision. A code comment at the extraction site is sufficient.

marcel commented

2026-04-15 12:38:51 +02:00

🧪 Sara Holt — QA Engineer

Questions & Observations

history.back() is tricky to test. Vitest/JSDOM does not implement history.back() — calling it is a no-op. The behavior can only be meaningfully verified in a real browser (Playwright). The acceptance criteria need Playwright coverage, not just visual inspection.
Missing AC: what does "no history" actually look like? history.length === 1 is the cold-open case. But there's a gap: what if the user navigated via an external URL with history.length === 2? The fallback won't trigger, and history.back() navigates off-site. Is this tested as an accepted behavior, or does the team want to handle it differently?
The "4 to check" locations block sign-off. users/[id], notifications/, enrich/, enrich/[id] need confirmed fallback routes before this can be called done. The AC says "A static fallback route is defined for each button" — that means every single location must be verified, not just the 6 listed ones.
No existing regression test for back navigation. The current static <a> links have no test. This change is an opportunity to add baseline coverage.

Suggestions

Add 2 Playwright scenarios per back-button surface (can be grouped):
1. Navigate to page via parent → click back → assert URL returns to parent.
2. Open page directly via URL → click back → assert URL is the fallback route (not going back to a blank/external page).
Assert the touch target size programmatically via Playwright rather than via manual check: expect(await backButton.boundingBox()).toHaveProperty('height', expect.toBeGreaterThanOrEqual(44)).
The AC line "Screen reader announces the button correctly" should have a corresponding axe-core scan in the E2E suite — this won't be caught by visual QA.
If goBack is extracted to $lib/navigation.ts, add a Vitest unit test for the fallback branch by mocking history as { length: 1, back: vi.fn() } and verifying goto is called with the fallback string.

## 🧪 Sara Holt — QA Engineer ### Questions & Observations - **`history.back()` is tricky to test.** Vitest/JSDOM does not implement `history.back()` — calling it is a no-op. The behavior can only be meaningfully verified in a real browser (Playwright). The acceptance criteria need Playwright coverage, not just visual inspection. - **Missing AC: what does "no history" actually look like?** `history.length === 1` is the cold-open case. But there's a gap: what if the user navigated via an external URL with `history.length === 2`? The fallback won't trigger, and `history.back()` navigates off-site. Is this tested as an accepted behavior, or does the team want to handle it differently? - **The "4 to check" locations block sign-off.** `users/[id]`, `notifications/`, `enrich/`, `enrich/[id]` need confirmed fallback routes before this can be called done. The AC says "A static fallback route is defined for each button" — that means every single location must be verified, not just the 6 listed ones. - **No existing regression test for back navigation.** The current static `<a>` links have no test. This change is an opportunity to add baseline coverage. ### Suggestions - **Add 2 Playwright scenarios per back-button surface** (can be grouped): 1. Navigate to page via parent → click back → assert URL returns to parent. 2. Open page directly via URL → click back → assert URL is the fallback route (not going back to a blank/external page). - **Assert the touch target size** programmatically via Playwright rather than via manual check: `expect(await backButton.boundingBox()).toHaveProperty('height', expect.toBeGreaterThanOrEqual(44))`. - The AC line "Screen reader announces the button correctly" should have a corresponding `axe-core` scan in the E2E suite — this won't be caught by visual QA. - If `goBack` is extracted to `$lib/navigation.ts`, add a Vitest unit test for the fallback branch by mocking `history` as `{ length: 1, back: vi.fn() }` and verifying `goto` is called with the fallback string.

marcel commented

2026-04-15 12:39:04 +02:00

🔐 Nora Steiner — Security Engineer

Questions & Observations

Minimal security surface. This is a navigation UX change with no server-side component and no user input flowing into any URL construction. No injection risk, no auth/authz changes.
Open-redirect check (ruled out). The goto(fallback) call uses a hardcoded string literal as the fallback (e.g., '/persons'). As long as the fallback value is never derived from $page.url, URL search params, route data, or any user-controlled input, there is no open-redirect risk. The issue spec confirms this: "same as the current static href." Verify in implementation that no fallback is constructed dynamically.
history.back() can navigate off-site. If the user arrived from an external URL, history.back() sends them back to that external page. This is identical to the browser's native back button — it's expected behavior, not a security issue. Confirming it's intentional.
<a> → <button> is a minor security improvement. The original <a href="/persons"> could be crawled, pre-fetched by the browser, or indexed by link checkers as an explicit navigation target. A <button onclick> with no href has no pre-fetch surface. Small win.

Suggestions

In the shared goBack(fallback: string) utility (if extracted), add a brief comment: // fallback must be a static route string — never pass URL params or user input. This makes the invariant explicit for future developers and auditors, preventing a future "let me make this dynamic" refactor that would introduce an open-redirect.

## 🔐 Nora Steiner — Security Engineer ### Questions & Observations - **Minimal security surface.** This is a navigation UX change with no server-side component and no user input flowing into any URL construction. No injection risk, no auth/authz changes. - **Open-redirect check (ruled out).** The `goto(fallback)` call uses a hardcoded string literal as the fallback (e.g., `'/persons'`). As long as the fallback value is never derived from `$page.url`, URL search params, route data, or any user-controlled input, there is no open-redirect risk. The issue spec confirms this: "same as the current static href." **Verify in implementation that no fallback is constructed dynamically.** - **`history.back()` can navigate off-site.** If the user arrived from an external URL, `history.back()` sends them back to that external page. This is identical to the browser's native back button — it's expected behavior, not a security issue. Confirming it's intentional. - **`<a>` → `<button>` is a minor security improvement.** The original `<a href="/persons">` could be crawled, pre-fetched by the browser, or indexed by link checkers as an explicit navigation target. A `<button onclick>` with no `href` has no pre-fetch surface. Small win. ### Suggestions - In the shared `goBack(fallback: string)` utility (if extracted), add a brief comment: `// fallback must be a static route string — never pass URL params or user input`. This makes the invariant explicit for future developers and auditors, preventing a future "let me make this dynamic" refactor that would introduce an open-redirect.

marcel commented

2026-04-15 12:39:22 +02:00

🎨 Leonie Voss — UI/UX & Accessibility

Questions & Observations

Touch target is missing from the code example. The existing class list (inline-flex items-center text-xs font-bold uppercase tracking-widest text-gray-500 hover:text-brand-navy transition-colors group mb-4) has no min-h-[44px]. The AC correctly requires ≥ 44px, but the sample implementation doesn't include it. This is the most important change for the senior audience — needs to be added explicitly.
"Zurück" vs "Zurück zur Übersicht". The CLAUDE.md back link pattern shows Zurück zur Übersicht as the full visible text. The issue example shows just Zurück. Shorter label = less screen reader context. If a page has only one back button, "Zurück" is fine. But if multiple interactive elements compete for attention, screen reader users benefit from "Zurück zur Personenübersicht" (for example). Is this a deliberate simplification, or was the existing contextual text being dropped unintentionally?
Focus ring is absent. The class list doesn't include focus-visible:ring-2 focus-visible:ring-brand-navy. Tailwind CSS resets the browser's default focus outline, so a <button> with no explicit focus style is invisible to keyboard users. All interactive elements in this project need a visible focus ring.
Semantic correctness: ✅ Replacing <a href> with <button> is exactly right. Back navigation is an action, not a link to a stable URL. This also eliminates the incorrect semantic of a link with a destination that may not reflect where you'll actually end up.
Hover animation: group-hover:-translate-x-1 on the arrow SVG carries over cleanly to <button class="... group">. No issue.

Suggestions

Add to the button class: min-h-[44px] focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-2 outline-none
Clarify the visible label: if CLAUDE.md's canonical pattern shows "Zurück zur Übersicht", keep contextual text. If simplifying to "Zurück" is intentional, add aria-label="Zurück zur [entity]Übersicht" per page so screen readers retain context.
Update the back link pattern in CLAUDE.md to reflect the <button> form once implemented — the current docs show an <a> pattern that will be obsolete after this change.

## 🎨 Leonie Voss — UI/UX & Accessibility ### Questions & Observations - **Touch target is missing from the code example.** The existing class list (`inline-flex items-center text-xs font-bold uppercase tracking-widest text-gray-500 hover:text-brand-navy transition-colors group mb-4`) has no `min-h-[44px]`. The AC correctly requires ≥ 44px, but the sample implementation doesn't include it. This is the most important change for the senior audience — needs to be added explicitly. - **"Zurück" vs "Zurück zur Übersicht".** The CLAUDE.md back link pattern shows `Zurück zur Übersicht` as the full visible text. The issue example shows just `Zurück`. Shorter label = less screen reader context. If a page has only one back button, "Zurück" is fine. But if multiple interactive elements compete for attention, screen reader users benefit from "Zurück zur Personenübersicht" (for example). Is this a deliberate simplification, or was the existing contextual text being dropped unintentionally? - **Focus ring is absent.** The class list doesn't include `focus-visible:ring-2 focus-visible:ring-brand-navy`. Tailwind CSS resets the browser's default focus outline, so a `<button>` with no explicit focus style is invisible to keyboard users. All interactive elements in this project need a visible focus ring. - **Semantic correctness: ✅** Replacing `<a href>` with `<button>` is exactly right. Back navigation is an action, not a link to a stable URL. This also eliminates the incorrect semantic of a link with a destination that may not reflect where you'll actually end up. - **Hover animation:** `group-hover:-translate-x-1` on the arrow SVG carries over cleanly to `<button class="... group">`. No issue. ### Suggestions - Add to the button class: `min-h-[44px] focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-2 outline-none` - Clarify the visible label: if CLAUDE.md's canonical pattern shows "Zurück zur Übersicht", keep contextual text. If simplifying to "Zurück" is intentional, add `aria-label="Zurück zur [entity]Übersicht"` per page so screen readers retain context. - Update the back link pattern in CLAUDE.md to reflect the `<button>` form once implemented — the current docs show an `<a>` pattern that will be obsolete after this change.

marcel commented

2026-04-15 12:39:33 +02:00

⚙️ Tobias Wendt — DevOps & Platform

Questions & Observations

Zero infra impact. No Docker Compose changes, no new environment variables, no CI pipeline changes, no new npm dependencies. history is a browser API; goto is already bundled via $app/navigation.
Build pipeline: npm run check (svelte-check + TypeScript) and npm run lint will catch type errors and formatting issues in the changed files. No new CI steps needed.
SSR note (for completeness): history is not available in the Node.js SSR context. Since the onclick handler only runs in the browser, this is safe as written. If the goBack utility is ever called outside an event handler, wrap it with a browser guard from $app/environment. Not a current risk.
No new npm packages. The implementation as described uses only existing runtime APIs. Good — no supply chain surface added.

Suggestions

Nothing from my end. This is a clean, self-contained frontend change with no footprint beyond the modified .svelte files. Standard PR checks cover it.

## ⚙️ Tobias Wendt — DevOps & Platform ### Questions & Observations - **Zero infra impact.** No Docker Compose changes, no new environment variables, no CI pipeline changes, no new npm dependencies. `history` is a browser API; `goto` is already bundled via `$app/navigation`. - **Build pipeline:** `npm run check` (svelte-check + TypeScript) and `npm run lint` will catch type errors and formatting issues in the changed files. No new CI steps needed. - **SSR note (for completeness):** `history` is not available in the Node.js SSR context. Since the `onclick` handler only runs in the browser, this is safe as written. If the `goBack` utility is ever called outside an event handler, wrap it with a `browser` guard from `$app/environment`. Not a current risk. - **No new npm packages.** The implementation as described uses only existing runtime APIs. Good — no supply chain surface added. ### Suggestions - Nothing from my end. This is a clean, self-contained frontend change with no footprint beyond the modified `.svelte` files. Standard PR checks cover it.

marcel commented

2026-04-22 10:40:09 +02:00

🎯 Discussion Resolutions

After reviewing the persona feedback with the user, here are the agreed decisions:

Theme 1 — Scope: affected locations

Decision: 9 locations are in scope. Two are excluded.
admin/users/[id]:129 (labeled Cancel) stays as a static <a href="/admin/users"> — it is a form cancel button, not back navigation. Replacing it with history.back() would be semantically wrong.
notifications/+page.svelte is dropped — the file no longer exists (route was renamed to /aktivitaeten, which has no back button).

Decision: Use history.back() with no fallback and no history.length check.
A button that silently does nothing on deep-link entry (no history) is acceptable. The added complexity of a fallback or a heuristic is not justified.
Off-site navigation (user arrived from external link) is accepted behaviour — identical to the browser's own back button.

Theme 3 — SvelteKit lifecycle guards

Decision: No code change needed. Guards continue to work.
history.back() triggers a browser popstate event, which SvelteKit intercepts. beforeNavigate fires normally for in-app navigation.
The existing useUnsavedWarning hook is compatible as-is.

Theme 4 — Shared component

Decision: Extract to a <BackButton> component in src/lib/components/.
The component calls history.back() internally and encapsulates the arrow SVG, class list, touch target, focus ring, and hover animation.
No label prop, no fallback prop.

Theme 5 — Testing

Decision: Two Playwright E2E tests in frontend/e2e/:
1. Navigate to a page via its parent → click back → assert URL returns to parent.
2. Component accessibility: touch target ≥ 44px (boundingBox()) + axe-core scan.

Theme 6 — Accessibility

Decision: <BackButton> class list includes min-h-[44px] focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-2 outline-none.
Label is always "Zurück" — no contextual suffix (e.g. "zur Übersicht") since the destination is unknown when using history.back().

Theme 7 — Security

No action needed. No fallback strings, no injectable parameters. The concern is fully mitigated by Option B.

Theme 8 — CLAUDE.md back link pattern

Decision: Update the back link pattern in CLAUDE.md from the current <a href> form to the new <BackButton> component form once implemented.

Open / Skipped

None — all raised points were resolved.

These resolutions are the authoritative design for implementation. The implement skill will read this comment alongside the original issue.

# 🎯 Discussion Resolutions After reviewing the persona feedback with the user, here are the agreed decisions: ## Theme 1 — Scope: affected locations - **Decision**: 9 locations are in scope. Two are excluded. - `admin/users/[id]:129` (labeled *Cancel*) stays as a static `<a href="/admin/users">` — it is a form cancel button, not back navigation. Replacing it with `history.back()` would be semantically wrong. - `notifications/+page.svelte` is dropped — the file no longer exists (route was renamed to `/aktivitaeten`, which has no back button). ## Theme 2 — Navigation behaviour - **Decision**: Use `history.back()` with no fallback and no `history.length` check. - A button that silently does nothing on deep-link entry (no history) is acceptable. The added complexity of a fallback or a heuristic is not justified. - Off-site navigation (user arrived from external link) is accepted behaviour — identical to the browser's own back button. ## Theme 3 — SvelteKit lifecycle guards - **Decision**: No code change needed. Guards continue to work. - `history.back()` triggers a browser `popstate` event, which SvelteKit intercepts. `beforeNavigate` fires normally for in-app navigation. - The existing `useUnsavedWarning` hook is compatible as-is. ## Theme 4 — Shared component - **Decision**: Extract to a `<BackButton>` component in `src/lib/components/`. - The component calls `history.back()` internally and encapsulates the arrow SVG, class list, touch target, focus ring, and hover animation. - No `label` prop, no `fallback` prop. ## Theme 5 — Testing - **Decision**: Two Playwright E2E tests in `frontend/e2e/`: 1. Navigate to a page via its parent → click back → assert URL returns to parent. 2. Component accessibility: touch target ≥ 44px (`boundingBox()`) + axe-core scan. ## Theme 6 — Accessibility - **Decision**: `<BackButton>` class list includes `min-h-[44px] focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-offset-2 outline-none`. - Label is always "Zurück" — no contextual suffix (e.g. "zur Übersicht") since the destination is unknown when using `history.back()`. ## Theme 7 — Security - **No action needed.** No fallback strings, no injectable parameters. The concern is fully mitigated by Option B. ## Theme 8 — CLAUDE.md back link pattern - **Decision**: Update the back link pattern in CLAUDE.md from the current `<a href>` form to the new `<BackButton>` component form once implemented. --- ## Open / Skipped _None — all raised points were resolved._ --- These resolutions are the authoritative design for implementation. The `implement` skill will read this comment alongside the original issue.

marcel commented

2026-04-22 11:03:41 +02:00

✅ Implementation Complete

All tasks from the implementation plan are done. Here's what was built:

Commits

feat(nav): add BackButton component calling history.back()
- src/lib/components/BackButton.svelte — button with inline SVG arrow, m.btn_back() label, history.back() on click, min-h-[44px] touch target, focus-visible:ring-focus-ring focus ring
- src/lib/components/BackButton.svelte.spec.ts — 3 unit tests: renders "Zurück", calls history.back() on click, has min-h-[44px] class
refactor(nav): replace static back-link hrefs with BackButton
- persons/new/+page.svelte:10 ✅
- persons/[id]/+page.svelte:53 ✅
- persons/[id]/edit/+page.svelte:16 ✅
- documents/[id]/edit/+page.svelte:29 ✅
- admin/users/[id]/+page.svelte:26 ✅ (inline <button onclick> — icon-only panel chevron, different layout)
- enrich/+page.svelte:12 ✅
- enrich/[id]/+page.svelte:17 ✅
docs(claude): update back link pattern to use BackButton component
- CLAUDE.md updated from <a href> pattern to <BackButton> pattern
test(nav): add E2E tests for BackButton navigation and accessibility
- e2e/back-button.spec.ts — navigation test (history.back() returns to actual previous URL, not static href) + accessibility test (touch target ≥ 44px + axe-core wcag2a/wcag2aa scan)

Scope adjustments vs. issue

admin/users/[id]:129 (Cancel button) — kept as static <a>, not back navigation
persons/new/+page.svelte:130 (Cancel button) — kept as static <a>, same reason
documents/[id]/edit/+page.svelte:75 (Cancel button) — kept as static <a>, same reason
notifications/+page.svelte — dropped (file no longer exists; route renamed to /aktivitaeten which has no back button)

Test results

Unit suite: 1141/1142 passing (1 pre-existing ChronikErrorCard failure unrelated to this change)
E2E: written, require running dev server to execute

Branch: feat/issue-185-fix-nav-history-back

## ✅ Implementation Complete All tasks from the implementation plan are done. Here's what was built: ### Commits 1. **`feat(nav): add BackButton component calling history.back()`** - `src/lib/components/BackButton.svelte` — button with inline SVG arrow, `m.btn_back()` label, `history.back()` on click, `min-h-[44px]` touch target, `focus-visible:ring-focus-ring` focus ring - `src/lib/components/BackButton.svelte.spec.ts` — 3 unit tests: renders "Zurück", calls `history.back()` on click, has `min-h-[44px]` class 2. **`refactor(nav): replace static back-link hrefs with BackButton`** - `persons/new/+page.svelte:10` ✅ - `persons/[id]/+page.svelte:53` ✅ - `persons/[id]/edit/+page.svelte:16` ✅ - `documents/[id]/edit/+page.svelte:29` ✅ - `admin/users/[id]/+page.svelte:26` ✅ (inline `<button onclick>` — icon-only panel chevron, different layout) - `enrich/+page.svelte:12` ✅ - `enrich/[id]/+page.svelte:17` ✅ 3. **`docs(claude): update back link pattern to use BackButton component`** - CLAUDE.md updated from `<a href>` pattern to `<BackButton>` pattern 4. **`test(nav): add E2E tests for BackButton navigation and accessibility`** - `e2e/back-button.spec.ts` — navigation test (history.back() returns to actual previous URL, not static href) + accessibility test (touch target ≥ 44px + axe-core wcag2a/wcag2aa scan) ### Scope adjustments vs. issue - `admin/users/[id]:129` (Cancel button) — kept as static `<a>`, not back navigation - `persons/new/+page.svelte:130` (Cancel button) — kept as static `<a>`, same reason - `documents/[id]/edit/+page.svelte:75` (Cancel button) — kept as static `<a>`, same reason - `notifications/+page.svelte` — dropped (file no longer exists; route renamed to `/aktivitaeten` which has no back button) ### Test results - Unit suite: 1141/1142 passing (1 pre-existing `ChronikErrorCard` failure unrelated to this change) - E2E: written, require running dev server to execute Branch: `feat/issue-185-fix-nav-history-back`

marcel referenced a pull request that will close this issue

2026-04-22 11:07:18 +02:00

fix(nav): replace static back-link hrefs with history.back() + fallback #303

marcel closed this issue

2026-04-22 12:32:57 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#185