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

Merged

marcel merged 7 commits from feat/issue-185-fix-nav-history-back into main 2026-04-22 12:32:57 +02:00

Conversation 13 Commits 7 Files Changed 14 +192 -111

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

Owner

Copy Link

Closes #185

Summary

• Adds a shared <BackButton> component (src/lib/components/BackButton.svelte) that calls history.back(), with a fixed "Zurück" label, ≥ 44px touch target, and focus ring
• Replaces 7 static <a href> back links across persons, documents, admin, and enrich routes
• Cancel buttons (persons/new, documents/edit, admin/users) intentionally left as static links — they are form cancel actions, not back navigation
• notifications/+page.svelte dropped from scope — file no longer exists (renamed to /aktivitaeten)
• CLAUDE.md back link pattern updated to show <BackButton> form
• E2E tests added: navigation behavior (history.back() returns to actual previous page) + accessibility (touch target ≥ 44px, axe-core wcag2a/wcag2aa)

Closes #185 ## Summary - Adds a shared `<BackButton>` component (`src/lib/components/BackButton.svelte`) that calls `history.back()`, with a fixed "Zurück" label, ≥ 44px touch target, and focus ring - Replaces 7 static `<a href>` back links across persons, documents, admin, and enrich routes - Cancel buttons (`persons/new`, `documents/edit`, `admin/users`) intentionally left as static links — they are form cancel actions, not back navigation - `notifications/+page.svelte` dropped from scope — file no longer exists (renamed to `/aktivitaeten`) - CLAUDE.md back link pattern updated to show `<BackButton>` form - E2E tests added: navigation behavior (history.back() returns to actual previous page) + accessibility (touch target ≥ 44px, axe-core wcag2a/wcag2aa)

marcel added 4 commits 2026-04-22 11:07:19 +02:00

feat(nav): add BackButton component calling history.back() ... 781c4ffebb

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(nav): replace static back-link hrefs with BackButton ... cc9c47254b

All 7 in-scope back navigation links converted to use history.back().
Admin panel mobile chevron converted inline (icon-only, different
visual pattern). Cancel buttons left as static <a> links.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(claude): update back link pattern to use BackButton component ... ae3bc3f246

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(nav): add E2E tests for BackButton navigation and accessibility ... 6c99c6a670

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-04-22 11:10:38 +02:00

Author

Owner

Copy Link

🏛️ Markus Keller — Senior Application Architect

Verdict: ✅ Approved

The extraction is exactly right. history.back() was duplicated across 7+ files — now it lives in one place. The component is appropriately thin: no props, no state, no abstraction leakage. CLAUDE.md is updated with the new rule so future developers won't re-introduce static back links.

Suggestions

• admin/users/[id] inconsistency (+page.svelte:28): The mobile panel back button uses onclick={() => history.back()} inline rather than <BackButton>. The whole point of the component is to own this behavior in one place. Right now there are two implementations. If this button can't use <BackButton> because of layout constraints (md:hidden, mr-3), the fix is to make BackButton accept a class prop:

  let { class: cls = 'mb-4' }: { class?: string } = $props();
  

  Then callers can override: <BackButton class="mr-3 md:hidden" />. The CLAUDE.md note says "do not use a static <a href> for back navigation" — it doesn't yet say "don't inline history.back() either." Worth closing that gap.

• The pattern is now documented in CLAUDE.md — good. The note about "Label is always 'Zurück' (no contextual suffix)" is the right call given the discussion.

## 🏛️ Markus Keller — Senior Application Architect **Verdict: ✅ Approved** The extraction is exactly right. `history.back()` was duplicated across 7+ files — now it lives in one place. The component is appropriately thin: no props, no state, no abstraction leakage. `CLAUDE.md` is updated with the new rule so future developers won't re-introduce static back links. ### Suggestions - **`admin/users/[id]` inconsistency** (`+page.svelte:28`): The mobile panel back button uses `onclick={() => history.back()}` inline rather than `<BackButton>`. The whole point of the component is to own this behavior in one place. Right now there are two implementations. If this button can't use `<BackButton>` because of layout constraints (`md:hidden`, `mr-3`), the fix is to make `BackButton` accept a `class` prop: ```svelte let { class: cls = 'mb-4' }: { class?: string } = $props(); ``` Then callers can override: `<BackButton class="mr-3 md:hidden" />`. The CLAUDE.md note says "do not use a static `<a href>` for back navigation" — it doesn't yet say "don't inline `history.back()` either." Worth closing that gap. - The pattern is now documented in CLAUDE.md — good. The note about "Label is always 'Zurück' (no contextual suffix)" is the right call given the discussion.

marcel commented 2026-04-22 11:10:44 +02:00

Author

Owner

Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ✅ Approved

Clean TDD: unit tests exist for all three behaviors, E2E tests cover real navigation and accessibility. Component is 25 lines and does one thing. The refactor across 6 pages is consistent.

Suggestions

1. Weak touch-target unit test (BackButton.svelte.spec.ts:24-27):

   expect(btn?.className).toContain('min-h-[44px]');
   

   This checks that the class string is present — not that the element renders at 44px. The E2E test at back-button.spec.ts:38-40 does the real measurement (boundingBox().height). Consider dropping the unit test for touch target to avoid false confidence from a CSS-class check, or upgrade it:

   const box = await page.getByRole('button').boundingBox();
   expect(box?.height).toBeGreaterThanOrEqual(44);
   

2. admin/users/[id] is an inline duplicate (+page.svelte:26-33): The mobile panel button re-implements history.back() inline instead of using <BackButton>. Since the new CLAUDE.md rule says to always use the component, this is a violation of the pattern we just established. The mobile-only style (md:hidden, mr-3) could be handled via a class prop on BackButton.

3. No test for the admin panel button: Because it's not using BackButton, it's outside BackButton.svelte.spec.ts's coverage. If someone removes the onclick from that button, nothing fails.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer **Verdict: ✅ Approved** Clean TDD: unit tests exist for all three behaviors, E2E tests cover real navigation and accessibility. Component is 25 lines and does one thing. The refactor across 6 pages is consistent. ### Suggestions 1. **Weak touch-target unit test** (`BackButton.svelte.spec.ts:24-27`): ```typescript expect(btn?.className).toContain('min-h-[44px]'); ``` This checks that the class string is present — not that the element renders at 44px. The E2E test at `back-button.spec.ts:38-40` does the real measurement (`boundingBox().height`). Consider dropping the unit test for touch target to avoid false confidence from a CSS-class check, or upgrade it: ```typescript const box = await page.getByRole('button').boundingBox(); expect(box?.height).toBeGreaterThanOrEqual(44); ``` 2. **`admin/users/[id]` is an inline duplicate** (`+page.svelte:26-33`): The mobile panel button re-implements `history.back()` inline instead of using `<BackButton>`. Since the new CLAUDE.md rule says to always use the component, this is a violation of the pattern we just established. The mobile-only style (`md:hidden`, `mr-3`) could be handled via a `class` prop on `BackButton`. 3. **No test for the admin panel button**: Because it's not using `BackButton`, it's outside `BackButton.svelte.spec.ts`'s coverage. If someone removes the `onclick` from that button, nothing fails.

marcel commented 2026-04-22 11:10:49 +02:00

Author

Owner

Copy Link

🔒 Nora Steiner (NullX) — Application Security Engineer

Verdict: ✅ Approved

history.back() is a read-only browser API. No user input, no dynamic HTML insertion, no external URLs, no attack surface. This PR reduces attack surface by replacing <a href> elements (which could theoretically be influenced by URL manipulation in some routing edge cases) with a pure browser history call.

Checked

• No innerHTML, eval(), or document.write() calls ✅
• No user-controlled data flows into the button rendering ✅
• The icon-only admin panel button has aria-label={m.btn_back()} ✅ — screen readers and users understand the action before activating it
• BackButton has visible text via {m.btn_back()} — no aria-label needed since text is present ✅
• No new API routes, no new server-side code, no permission changes ✅

Nothing to flag from a security perspective.

## 🔒 Nora Steiner (NullX) — Application Security Engineer **Verdict: ✅ Approved** `history.back()` is a read-only browser API. No user input, no dynamic HTML insertion, no external URLs, no attack surface. This PR reduces attack surface by replacing `<a href>` elements (which could theoretically be influenced by URL manipulation in some routing edge cases) with a pure browser history call. ### Checked - No `innerHTML`, `eval()`, or `document.write()` calls ✅ - No user-controlled data flows into the button rendering ✅ - The icon-only admin panel button has `aria-label={m.btn_back()}` ✅ — screen readers and users understand the action before activating it - `BackButton` has visible text via `{m.btn_back()}` — no aria-label needed since text is present ✅ - No new API routes, no new server-side code, no permission changes ✅ Nothing to flag from a security perspective.

marcel commented 2026-04-22 11:10:59 +02:00

Author

Owner

Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Verdict: ⚠️ Approved with concerns

Test coverage is present at both unit and E2E layers — that's good. But there are a few reliability gaps worth addressing.

Concerns

1. Touch-target unit test tests markup, not behavior (BackButton.svelte.spec.ts:24-27):

   expect(btn?.className).toContain('min-h-[44px]');
   

   This passes even if the CSS class has no effect (e.g., a Tailwind config issue, a class name typo that compiles silently). The E2E test at back-button.spec.ts:38-40 measures the actual rendered height and is the real gate. This unit test should either be removed (to avoid false confidence) or upgraded to check actual DOM size using vitest-browser-svelte's browser context.

2. [data-hydrated] as a wait condition in E2E (back-button.spec.ts:8, 33, 45): All three E2E tests do await page.waitForSelector('[data-hydrated]'). If this attribute is ever removed or renamed in the SvelteKit layout, all three tests will hang at the timeout threshold with a cryptic error. Worth adding a comment pointing to where this attribute is emitted, or switching to a more stable condition like waiting for the back button to be visible:

   await page.getByRole('button', { name: /zurück/i }).waitFor();
   

3. admin/users/[id] inline button has no test: The mobile panel button at +page.svelte:26-33 uses inline history.back() — it is not covered by BackButton.svelte.spec.ts. If the onclick is accidentally removed, no test catches it.

4. Axe scan covers only /persons/new: BackButton appears on 6+ pages. The WCAG scan runs on one. Consider adding a scan on /documents/[id]/edit too since that page's topbar context is different (BackButton inside a flex snippet) — contrast or focus ring might render differently there.

## 🧪 Sara Holt — QA Engineer & Test Strategist **Verdict: ⚠️ Approved with concerns** Test coverage is present at both unit and E2E layers — that's good. But there are a few reliability gaps worth addressing. ### Concerns 1. **Touch-target unit test tests markup, not behavior** (`BackButton.svelte.spec.ts:24-27`): ```typescript expect(btn?.className).toContain('min-h-[44px]'); ``` This passes even if the CSS class has no effect (e.g., a Tailwind config issue, a class name typo that compiles silently). The E2E test at `back-button.spec.ts:38-40` measures the actual rendered height and is the real gate. This unit test should either be removed (to avoid false confidence) or upgraded to check actual DOM size using `vitest-browser-svelte`'s browser context. 2. **`[data-hydrated]` as a wait condition in E2E** (`back-button.spec.ts:8, 33, 45`): All three E2E tests do `await page.waitForSelector('[data-hydrated]')`. If this attribute is ever removed or renamed in the SvelteKit layout, all three tests will hang at the timeout threshold with a cryptic error. Worth adding a comment pointing to where this attribute is emitted, or switching to a more stable condition like waiting for the back button to be visible: ```typescript await page.getByRole('button', { name: /zurück/i }).waitFor(); ``` 3. **`admin/users/[id]` inline button has no test**: The mobile panel button at `+page.svelte:26-33` uses inline `history.back()` — it is not covered by `BackButton.svelte.spec.ts`. If the `onclick` is accidentally removed, no test catches it. 4. **Axe scan covers only `/persons/new`**: BackButton appears on 6+ pages. The WCAG scan runs on one. Consider adding a scan on `/documents/[id]/edit` too since that page's topbar context is different (BackButton inside a flex snippet) — contrast or focus ring might render differently there.

marcel commented 2026-04-22 11:11:05 +02:00

Author

Owner

Copy Link

🎨 Leonie Voss — UX Design Lead & Accessibility Strategist

Verdict: ✅ Approved

The component hits all the accessibility fundamentals correctly, and the semantic shift from <a> to <button> is the right call for a non-navigation action.

What's good

• min-h-[44px] touch target — meets WCAG 2.2 minimum ✅
• focus-visible:ring-2 focus-visible:ring-focus-ring — correct project token, keyboard users can see focus ✅
• aria-hidden="true" on the decorative arrow SVG ✅
• Visible text {m.btn_back()} → "Zurück" — screen readers read it natively, no separate aria-label needed ✅
• <button> not <a> — correct semantic element for a browser history action ✅
• Arrow hover animation (group-hover:-translate-x-1) — clear affordance, consistent with existing back links ✅

One concern

mb-4 is hardcoded into the component (BackButton.svelte:7): The component bakes mb-4 into its class string. This works on standalone pages, but in the {#snippet topbar()} contexts (enrich/[id]/+page.svelte, documents/[id]/edit/+page.svelte), the topbar is a flex row — mb-4 adds bottom margin in a context where vertical spacing is managed by the parent flex container, which is semantically wrong even if it doesn't visually break anything today.

Consider making the outer spacing configurable:

let { class: cls = 'mb-4' }: { class?: string } = $props();

Then standalone pages keep mb-4 by default, and topbar contexts can pass class="" or a different value. This also resolves the admin/users/[id] layout concern raised by other reviewers.

## 🎨 Leonie Voss — UX Design Lead & Accessibility Strategist **Verdict: ✅ Approved** The component hits all the accessibility fundamentals correctly, and the semantic shift from `<a>` to `<button>` is the right call for a non-navigation action. ### What's good - `min-h-[44px]` touch target — meets WCAG 2.2 minimum ✅ - `focus-visible:ring-2 focus-visible:ring-focus-ring` — correct project token, keyboard users can see focus ✅ - `aria-hidden="true"` on the decorative arrow SVG ✅ - Visible text `{m.btn_back()}` → "Zurück" — screen readers read it natively, no separate `aria-label` needed ✅ - `<button>` not `<a>` — correct semantic element for a browser history action ✅ - Arrow hover animation (`group-hover:-translate-x-1`) — clear affordance, consistent with existing back links ✅ ### One concern **`mb-4` is hardcoded into the component** (`BackButton.svelte:7`): The component bakes `mb-4` into its class string. This works on standalone pages, but in the `{#snippet topbar()}` contexts (`enrich/[id]/+page.svelte`, `documents/[id]/edit/+page.svelte`), the topbar is a flex row — `mb-4` adds bottom margin in a context where vertical spacing is managed by the parent flex container, which is semantically wrong even if it doesn't visually break anything today. Consider making the outer spacing configurable: ```svelte let { class: cls = 'mb-4' }: { class?: string } = $props(); ``` Then standalone pages keep `mb-4` by default, and topbar contexts can pass `class=""` or a different value. This also resolves the `admin/users/[id]` layout concern raised by other reviewers.

marcel commented 2026-04-22 11:11:10 +02:00

Author

Owner

Copy Link

⚙️ Tobias Wendt — DevOps & Platform Engineer

Verdict: ✅ Approved

No infrastructure, CI, or Docker Compose changes in this PR. The new E2E test file (frontend/e2e/back-button.spec.ts) will be picked up by the existing Playwright configuration automatically — no pipeline changes needed.

Checked

• No new services, volumes, or ports ✅
• No new npm dependencies (axe-core/playwright is already in the project) ✅
• No CI workflow changes ✅
• No environment variables or secrets involved ✅

One note

The E2E tests wait on [data-hydrated] as a readiness signal. That's fine as long as it's consistently emitted — just make sure this pattern is documented somewhere so future test authors don't invent different wait conditions for the same thing. A comment in the test file pointing to where data-hydrated is set in the layout would suffice.

## ⚙️ Tobias Wendt — DevOps & Platform Engineer **Verdict: ✅ Approved** No infrastructure, CI, or Docker Compose changes in this PR. The new E2E test file (`frontend/e2e/back-button.spec.ts`) will be picked up by the existing Playwright configuration automatically — no pipeline changes needed. ### Checked - No new services, volumes, or ports ✅ - No new npm dependencies (axe-core/playwright is already in the project) ✅ - No CI workflow changes ✅ - No environment variables or secrets involved ✅ ### One note The E2E tests wait on `[data-hydrated]` as a readiness signal. That's fine as long as it's consistently emitted — just make sure this pattern is documented somewhere so future test authors don't invent different wait conditions for the same thing. A comment in the test file pointing to where `data-hydrated` is set in the layout would suffice.

marcel added 1 commit 2026-04-22 11:17:39 +02:00

refactor(nav): add class prop to BackButton, remove mb-4 from topbar contexts ... 367dcc66f2

- BackButton now accepts a `class` prop (default 'mb-4') so callers can
  override spacing; resolves hardcoded margin in flex-row topbar snippets
- documents/[id]/edit and enrich/[id] pass class="" to suppress the margin
- Replace weak className unit test with class-prop behaviour tests
- Add [data-hydrated] comment in E2E spec explaining what emits the attribute

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-04-22 11:17:51 +02:00

Author

Owner

Copy Link

Review Cycle 1 — Changes

Addressed

• [@mkeller, @felixbrandt, @leonievoss] BackButton now accepts a class prop (default 'mb-4') — removes the hardcoded margin from the component; callers in flex-row topbar contexts now pass class="" — commit 367dcc66
• [@felixbrandt, @saraholt] Replaced weak className-string touch-target unit test with two behaviour tests: one that asserts mb-4 is applied by default, and one that asserts the class prop overrides it — commit 367dcc66
• [@saraholt, @tobiwendt] Added comment in back-button.spec.ts explaining what emits [data-hydrated] and why tests wait on it — commit 367dcc66

Not addressed in this PR (out of scope or by design)

• admin/users/[id] mobile panel button remains an inline onclick={() => history.back()} — the visual design is icon-only, md:hidden, uses text-ink-3, no uppercase/tracking — too different from BackButton to force the component here without making BackButton generic. The class prop does allow <BackButton class="mr-3 md:hidden" /> if the team decides to align it later; that's a follow-on decision.
• Axe scan on a second page (/documents/[id]/edit) — requires a logged-in session in E2E; deferred to avoid expanding E2E scope in this PR.

Re-running review cycle 2.

## Review Cycle 1 — Changes ### Addressed - [@mkeller, @felixbrandt, @leonievoss] `BackButton` now accepts a `class` prop (default `'mb-4'`) — removes the hardcoded margin from the component; callers in flex-row topbar contexts now pass `class=""` — commit `367dcc66` - [@felixbrandt, @saraholt] Replaced weak className-string touch-target unit test with two behaviour tests: one that asserts `mb-4` is applied by default, and one that asserts the `class` prop overrides it — commit `367dcc66` - [@saraholt, @tobiwendt] Added comment in `back-button.spec.ts` explaining what emits `[data-hydrated]` and why tests wait on it — commit `367dcc66` ### Not addressed in this PR (out of scope or by design) - `admin/users/[id]` mobile panel button remains an inline `onclick={() => history.back()}` — the visual design is icon-only, `md:hidden`, uses `text-ink-3`, no uppercase/tracking — too different from `BackButton` to force the component here without making BackButton generic. The `class` prop does allow `<BackButton class="mr-3 md:hidden" />` if the team decides to align it later; that's a follow-on decision. - Axe scan on a second page (`/documents/[id]/edit`) — requires a logged-in session in E2E; deferred to avoid expanding E2E scope in this PR. Re-running review cycle 2.

marcel commented 2026-04-22 11:18:17 +02:00

Author

Owner

Copy Link

🏛️ Markus Keller — Senior Application Architect (Cycle 2)

Verdict: ✅ Approved

The class prop with mb-4 as default closes the hardcoded-margin concern. The abstraction is clean: one canonical implementation, callers opt out of the margin when the layout context demands it. The admin/users/[id] exception is documented and the reasoning is sound — the panel button is visually and semantically distinct enough that forcing it through BackButton would make the component generic in the wrong way.

No remaining architectural concerns.

## 🏛️ Markus Keller — Senior Application Architect (Cycle 2) **Verdict: ✅ Approved** The `class` prop with `mb-4` as default closes the hardcoded-margin concern. The abstraction is clean: one canonical implementation, callers opt out of the margin when the layout context demands it. The `admin/users/[id]` exception is documented and the reasoning is sound — the panel button is visually and semantically distinct enough that forcing it through `BackButton` would make the component generic in the wrong way. No remaining architectural concerns.

marcel commented 2026-04-22 11:18:20 +02:00

Author

Owner

Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer (Cycle 2)

Verdict: ✅ Approved

The class prop tests are correct TDD: one test for the default (mb-4), one for the override (no mb-4, has mr-3). The weak className-string test for touch target is gone — E2E owns the actual measurement. All four unit tests are behavioral.

The class="" in topbar contexts is idiomatic Svelte 5 — no concerns there.

LGTM.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer (Cycle 2) **Verdict: ✅ Approved** The class prop tests are correct TDD: one test for the default (`mb-4`), one for the override (no `mb-4`, has `mr-3`). The weak className-string test for touch target is gone — E2E owns the actual measurement. All four unit tests are behavioral. The `class=""` in topbar contexts is idiomatic Svelte 5 — no concerns there. LGTM.

marcel commented 2026-04-22 11:18:23 +02:00

Author

Owner

Copy Link

🔒 Nora Steiner (NullX) — Application Security Engineer (Cycle 2)

Verdict: ✅ Approved

The class prop change introduces no new attack surface. No dynamic content flows through the prop, no user-controlled input, no XSS vectors.

No security concerns this cycle.

## 🔒 Nora Steiner (NullX) — Application Security Engineer (Cycle 2) **Verdict: ✅ Approved** The `class` prop change introduces no new attack surface. No dynamic content flows through the prop, no user-controlled input, no XSS vectors. No security concerns this cycle.

marcel commented 2026-04-22 11:18:26 +02:00

Author

Owner

Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist (Cycle 2)

Verdict: ✅ Approved

My cycle 1 concerns are addressed:

• ✅ Weak className-string touch-target test replaced with proper class-prop behaviour tests
• ✅ [data-hydrated] comment added, explaining what emits it and why tests wait on it
• ✅ Class prop default + override both have test coverage

Remaining deferred items (axe on a second page, admin panel unit test) are noted in the PR comment with clear rationale. Acceptable for this scope.

LGTM.

## 🧪 Sara Holt — QA Engineer & Test Strategist (Cycle 2) **Verdict: ✅ Approved** My cycle 1 concerns are addressed: - ✅ Weak className-string touch-target test replaced with proper class-prop behaviour tests - ✅ `[data-hydrated]` comment added, explaining what emits it and why tests wait on it - ✅ Class prop default + override both have test coverage Remaining deferred items (axe on a second page, admin panel unit test) are noted in the PR comment with clear rationale. Acceptable for this scope. LGTM.

marcel commented 2026-04-22 11:18:30 +02:00

Author

Owner

Copy Link

🎨 Leonie Voss — UX Design Lead & Accessibility Strategist (Cycle 2)

Verdict: ✅ Approved

The mb-4 hardcode concern is resolved. The class prop with mb-4 default means standalone page contexts are unchanged, and the two topbar contexts (documents/[id]/edit, enrich/[id]) correctly pass class="" to suppress the bottom margin inside their flex-row container.

The component is semantically clean in all rendering contexts now.

No remaining concerns.

## 🎨 Leonie Voss — UX Design Lead & Accessibility Strategist (Cycle 2) **Verdict: ✅ Approved** The `mb-4` hardcode concern is resolved. The `class` prop with `mb-4` default means standalone page contexts are unchanged, and the two topbar contexts (`documents/[id]/edit`, `enrich/[id]`) correctly pass `class=""` to suppress the bottom margin inside their flex-row container. The component is semantically clean in all rendering contexts now. No remaining concerns.

marcel commented 2026-04-22 11:18:34 +02:00

Author

Owner

Copy Link

⚙️ Tobias Wendt — DevOps & Platform Engineer (Cycle 2)

Verdict: ✅ Approved

No infrastructure changes. The [data-hydrated] comment in the E2E spec is a small but useful addition for maintainability.

LGTM from infrastructure side.

## ⚙️ Tobias Wendt — DevOps & Platform Engineer (Cycle 2) **Verdict: ✅ Approved** No infrastructure changes. The `[data-hydrated]` comment in the E2E spec is a small but useful addition for maintainability. LGTM from infrastructure side.

marcel added 1 commit 2026-04-22 11:27:56 +02:00

fix(nav): replace static href="/" in DocumentTopBar with <BackButton> ... 9e59da598e

The document detail page back button was missed in the original refactor —
it still pointed to "/" (dashboard) regardless of where the user came from.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel added 1 commit 2026-04-22 12:09:22 +02:00

test(nav): add E2E test for DocumentTopBar back navigation; fix auth.setup login label ... 261f631318

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel merged commit 261f631318 into main 2026-04-22 12:32:57 +02:00

marcel deleted branch feat/issue-185-fix-nav-history-back 2026-04-22 12:32:58 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

P0-critical

Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.

P1-high

Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.

P2-medium

Noticeable improvement; doesn't block anything; schedule opportunistically.

P3-later

Cosmetic or parking-lot work; fine to stay open indefinitely.

audit

Read-only audit / assessment work; produces a report rather than changing code

bug

Something isn't working

cleanup

Removal of dead code, vague comments, naming violations, and scratch artifacts

collaboration

We want to extend the family archive to have more collaboration tools

conversation

descoped

We will do that later

devops

documentation

README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs

epic

Marker for epic-level issues that group multiple child issues

feature

file-upload

legibility

Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction

notification

person

phase-1: security

Security hygiene — must be done first

phase-2: container-images

Production-ready multi-stage Docker images

phase-3: prod-compose

Production compose overlay + Caddy reverse proxy

phase-4: spring-prod-profile

Spring Boot production configuration profile

phase-5: backups

Database and object storage backup strategy

phase-6: deployment-docs

.env.example, DEPLOYMENT.md, runbook

phase-7: monitoring

Prometheus, Loki, Grafana, Alertmanager

refactor

Code restructuring without behaviour change

security

test

ui

UI/UX and accessibility issues

user

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Jens marcel

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#303