marcel/familienarchiv
1
0
Fork 0
Code Issues 117 Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

refactor: Replace native confirm() dialogs with a shared confirmation modal service #207

New Issue
Closed
opened 2026-04-07 16:34:47 +02:00 by marcel · 10 comments
marcel commented 2026-04-07 16:34:47 +02:00
Owner
Copy Link

Problem

The app uses three different confirmation patterns:

  1. Native confirm() - ugly browser dialog, inconsistent with the app's design, no i18n control over button labels
  2. Inline confirmation - toggle-based UI within the same component (e.g. document delete, person merge)
  3. Custom modal - overlay with styled buttons (e.g. alias delete)

Each component reinvents its own confirmation flow with local state (showDeleteModal, confirmDelete, showMergeConfirm). The alias delete modal in NameHistoryEditCard is the newest and closest to a reusable pattern, but it's hardcoded to that component.

Native confirm() calls to replace

File Usage Message
TranscriptionBlock.svelte:94 Delete transcription block + comments transcription_block_delete_confirm
admin/users/[id]/+page.svelte:58 Delete user admin_user_delete_confirm
admin/groups/[id]/+page.svelte:66 Delete group admin_group_delete_confirm

Custom inline confirmations (could also migrate)

File Pattern Usage
documents/[id]/edit/SaveBar.svelte confirmDelete toggle, inline buttons Delete document
PersonMergePanel.svelte showMergeConfirm toggle, two-step inline UI Merge person
NameHistoryEditCard.svelte showDeleteModal + fixed overlay modal Delete alias

Proposal

Build a shared ConfirmModal.svelte component (or a Svelte context-based confirmation service) that any component can call without managing its own modal state.

Option A - Component with bindable props:

<ConfirmModal
  bind:open={showConfirm}
  title="Block loeschen?"
  body="Block und alle Kommentare werden entfernt."
  confirmLabel="Loeschen"
  destructive={true}
  onConfirm={() => deleteBlock()}
/>

Option B - Context-based service (more flexible):

const confirmed = await confirm({
  title: m.transcription_block_delete_confirm(),
  body: '...',
  confirmLabel: m.btn_delete(),
  destructive: true
});
if (confirmed) deleteBlock();

Option B is cleaner for form use:enhance cancel patterns where you need to await the user's decision before proceeding.

Acceptance criteria

  • Shared confirmation component/service exists in $lib/components/
  • All 3 native confirm() calls replaced
  • Alias delete modal in NameHistoryEditCard uses the shared component
  • Consistent visual style: overlay, title, body text, Cancel + Confirm buttons
  • Destructive confirmations use red confirm button
  • All button labels come from i18n keys
  • Touch targets >= 44px on confirm/cancel buttons
## Problem The app uses three different confirmation patterns: 1. **Native `confirm()`** - ugly browser dialog, inconsistent with the app's design, no i18n control over button labels 2. **Inline confirmation** - toggle-based UI within the same component (e.g. document delete, person merge) 3. **Custom modal** - overlay with styled buttons (e.g. alias delete) Each component reinvents its own confirmation flow with local state (`showDeleteModal`, `confirmDelete`, `showMergeConfirm`). The alias delete modal in `NameHistoryEditCard` is the newest and closest to a reusable pattern, but it's hardcoded to that component. ## Native `confirm()` calls to replace | File | Usage | Message | |------|-------|---------| | `TranscriptionBlock.svelte:94` | Delete transcription block + comments | `transcription_block_delete_confirm` | | `admin/users/[id]/+page.svelte:58` | Delete user | `admin_user_delete_confirm` | | `admin/groups/[id]/+page.svelte:66` | Delete group | `admin_group_delete_confirm` | ## Custom inline confirmations (could also migrate) | File | Pattern | Usage | |------|---------|-------| | `documents/[id]/edit/SaveBar.svelte` | `confirmDelete` toggle, inline buttons | Delete document | | `PersonMergePanel.svelte` | `showMergeConfirm` toggle, two-step inline UI | Merge person | | `NameHistoryEditCard.svelte` | `showDeleteModal` + fixed overlay modal | Delete alias | ## Proposal Build a shared `ConfirmModal.svelte` component (or a Svelte context-based confirmation service) that any component can call without managing its own modal state. **Option A - Component with bindable props:** ```svelte <ConfirmModal bind:open={showConfirm} title="Block loeschen?" body="Block und alle Kommentare werden entfernt." confirmLabel="Loeschen" destructive={true} onConfirm={() => deleteBlock()} /> ``` **Option B - Context-based service (more flexible):** ```svelte const confirmed = await confirm({ title: m.transcription_block_delete_confirm(), body: '...', confirmLabel: m.btn_delete(), destructive: true }); if (confirmed) deleteBlock(); ``` Option B is cleaner for form `use:enhance` cancel patterns where you need to `await` the user's decision before proceeding. ## Acceptance criteria - [ ] Shared confirmation component/service exists in `$lib/components/` - [ ] All 3 native `confirm()` calls replaced - [ ] Alias delete modal in NameHistoryEditCard uses the shared component - [ ] Consistent visual style: overlay, title, body text, Cancel + Confirm buttons - [ ] Destructive confirmations use red confirm button - [ ] All button labels come from i18n keys - [ ] Touch targets >= 44px on confirm/cancel buttons
marcel added the refactorui labels 2026-04-07 16:34:54 +02:00
marcel commented 2026-04-12 12:20:46 +02:00
Author
Owner
Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

  • Option B (async service) is the right call — but the implementation is non-trivial in Svelte 5. An async confirm() that returns a Promise<boolean> requires a deferred Promise pattern: the service holds a resolver function in $state, the modal calls resolve(true/false) when the user clicks, and the caller awaits. Worth sketching this before committing to Option B, because the reactive state machine is subtle:

    // Rough outline — needs careful handling of concurrent calls
let resolve: ((value: boolean) => void) | null = $state(null);
let options: ConfirmOptions | null = $state(null);

async function confirm(opts: ConfirmOptions): Promise<boolean> {
    options = opts;
    return new Promise(r => resolve = r);
}

    If two components call confirm() simultaneously, the second overwrites the first. The service needs to queue or reject concurrent calls — define which behavior is wanted.

  • The ACs are incomplete on the "could also migrate" patterns. SaveBar.svelte (delete document) and PersonMergePanel.svelte (merge) are explicitly listed as candidates but not in the acceptance criteria. If they're out of scope for this issue, say so explicitly — otherwise implementers will differ on whether migrating them counts as "done."

  • Component naming. ConfirmModal works, but the HTML <dialog> element is the native semantic element for this pattern and provides focus trapping, Escape key handling, and ::backdrop styling for free. The name ConfirmDialog.svelte would signal that semantic choice. Worth considering before naming is locked in.

  • Testing the service requires a host component. Unit testing the async service in isolation with Vitest is awkward — you need to mount a host that provides the context, open the modal, simulate a button click, and assert the Promise resolves. Write the test structure before implementation to confirm the design is testable.

Suggestions

  • Write one failing Vitest test first: mount a host with the confirmation service in context, call confirm({ title: 'test' }), click the Confirm button, assert the returned Promise resolves to true. This test drives the minimal API surface.
  • Decide on concurrent call behavior (queue vs. reject) and add it to the acceptance criteria — it's a real edge case that form submit handlers can trigger.
## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Questions & Observations - **Option B (async service) is the right call — but the implementation is non-trivial in Svelte 5.** An async `confirm()` that returns a `Promise<boolean>` requires a deferred Promise pattern: the service holds a resolver function in `$state`, the modal calls `resolve(true/false)` when the user clicks, and the caller awaits. Worth sketching this before committing to Option B, because the reactive state machine is subtle: ```typescript // Rough outline — needs careful handling of concurrent calls let resolve: ((value: boolean) => void) | null = $state(null); let options: ConfirmOptions | null = $state(null); async function confirm(opts: ConfirmOptions): Promise<boolean> { options = opts; return new Promise(r => resolve = r); } ``` If two components call `confirm()` simultaneously, the second overwrites the first. The service needs to queue or reject concurrent calls — define which behavior is wanted. - **The ACs are incomplete on the "could also migrate" patterns.** `SaveBar.svelte` (delete document) and `PersonMergePanel.svelte` (merge) are explicitly listed as candidates but not in the acceptance criteria. If they're out of scope for this issue, say so explicitly — otherwise implementers will differ on whether migrating them counts as "done." - **Component naming.** `ConfirmModal` works, but the HTML `<dialog>` element is the native semantic element for this pattern and provides focus trapping, `Escape` key handling, and `::backdrop` styling for free. The name `ConfirmDialog.svelte` would signal that semantic choice. Worth considering before naming is locked in. - **Testing the service requires a host component.** Unit testing the async service in isolation with Vitest is awkward — you need to mount a host that provides the context, open the modal, simulate a button click, and assert the Promise resolves. Write the test structure before implementation to confirm the design is testable. ### Suggestions - Write one failing Vitest test first: mount a host with the confirmation service in context, call `confirm({ title: 'test' })`, click the Confirm button, assert the returned Promise resolves to `true`. This test drives the minimal API surface. - Decide on concurrent call behavior (queue vs. reject) and add it to the acceptance criteria — it's a real edge case that form submit handlers can trigger.
marcel commented 2026-04-12 12:21:06 +02:00
Author
Owner
Copy Link

🏛️ Markus Keller — Application Architect

Questions & Observations

  • Option B is architecturally superior — but it requires a contract. A context-based service inverts control cleanly: callers express intent ("did the user confirm?") without owning modal state. The trade-off is that the service must be provided at the layout level, and any component that calls confirm() outside that context will fail silently at runtime. The implementation should include a guard:

    export function getConfirmService() {
    const service = getContext<ConfirmService>(CONFIRM_KEY);
    if (!service) throw new Error('ConfirmService not found — mount ConfirmProvider in +layout.svelte');
    return service;
}

    This converts a silent runtime failure into a clear developer error.

  • Use the native <dialog> element. The HTML <dialog> element with showModal() / close() provides focus trapping, Escape handling, ::backdrop CSS hook, and top-layer stacking (bypasses z-index wars) for free. In 2026 this has full browser support. Building a custom overlay with z-index: 9999 duplicates platform behavior and introduces accessibility bugs. The <dialog> is the correct primitive here.

  • SSR safety. Svelte context (setContext/getContext) is SSR-safe — context is scoped to the component tree per request, not global. However, the deferred Promise pattern (storing a resolve function in reactive state) should be client-side only. Confirm the service doesn't attempt to instantiate the Promise on the server — or add a browser guard.

  • Module boundary. Where does this service live? $lib/components/ConfirmModal.svelte (the view) and $lib/services/confirm.ts (the logic) are distinct concerns. The service module exports getConfirmService(), the component renders the modal. They're coupled but separable — don't collapse them into one file.

  • The "could also migrate" patterns are a scope creep risk. PersonMergePanel.svelte uses a two-step inline confirmation that is deliberately different — it shows additional context ("you are merging X into Y") inline rather than in a modal. Migrating it to the shared service may lose that contextual richness. I'd keep it out of scope for this issue and revisit separately.

Suggestions

  • Document the provider pattern in a comment at the top of +layout.svelte where the service is mounted — future contributors need to know why it's there.
  • Consider whether the service should expose confirmDestructive() as a named shorthand for confirm({ destructive: true }) — keeps call sites readable without extra boilerplate.
## 🏛️ Markus Keller — Application Architect ### Questions & Observations - **Option B is architecturally superior — but it requires a contract.** A context-based service inverts control cleanly: callers express intent ("did the user confirm?") without owning modal state. The trade-off is that the service must be provided at the layout level, and any component that calls `confirm()` outside that context will fail silently at runtime. The implementation should include a guard: ```typescript export function getConfirmService() { const service = getContext<ConfirmService>(CONFIRM_KEY); if (!service) throw new Error('ConfirmService not found — mount ConfirmProvider in +layout.svelte'); return service; } ``` This converts a silent runtime failure into a clear developer error. - **Use the native `<dialog>` element.** The HTML `<dialog>` element with `showModal()` / `close()` provides focus trapping, `Escape` handling, `::backdrop` CSS hook, and top-layer stacking (bypasses z-index wars) for free. In 2026 this has full browser support. Building a custom overlay with `z-index: 9999` duplicates platform behavior and introduces accessibility bugs. The `<dialog>` is the correct primitive here. - **SSR safety.** Svelte context (`setContext`/`getContext`) is SSR-safe — context is scoped to the component tree per request, not global. However, the deferred Promise pattern (storing a `resolve` function in reactive state) should be client-side only. Confirm the service doesn't attempt to instantiate the Promise on the server — or add a `browser` guard. - **Module boundary.** Where does this service live? `$lib/components/ConfirmModal.svelte` (the view) and `$lib/services/confirm.ts` (the logic) are distinct concerns. The service module exports `getConfirmService()`, the component renders the modal. They're coupled but separable — don't collapse them into one file. - **The "could also migrate" patterns are a scope creep risk.** `PersonMergePanel.svelte` uses a two-step inline confirmation that is deliberately different — it shows additional context ("you are merging X into Y") inline rather than in a modal. Migrating it to the shared service may lose that contextual richness. I'd keep it out of scope for this issue and revisit separately. ### Suggestions - Document the provider pattern in a comment at the top of `+layout.svelte` where the service is mounted — future contributors need to know why it's there. - Consider whether the service should expose `confirmDestructive()` as a named shorthand for `confirm({ destructive: true })` — keeps call sites readable without extra boilerplate.
marcel commented 2026-04-12 12:21:36 +02:00
Author
Owner
Copy Link

🧪 Sara Holt — QA Engineer

Questions & Observations

  • The acceptance criteria are a solid starting point but missing behavioral edge cases. The 7 items cover structure and style, but don't define expected behavior for:

    • What happens when Escape is pressed? (Should cancel — equivalent to clicking Cancel)
    • What happens when the backdrop is clicked? (Cancel, or ignored for destructive actions?)
    • What if confirm is called while a confirmation is already open? (Queue, replace, or throw?)
    • Does the Promise reject or resolve to false if the component is unmounted before the user responds?

  • Three well-documented replacement targets make regression testing straightforward. For each of the 3 native confirm() callsites, I'd write an E2E test that:

    1. Triggers the action that previously used confirm()
    2. Asserts the native browser dialog is not shown (Playwright's page.on('dialog') listener should not fire)
    3. Asserts the custom modal is visible with the correct title and i18n-rendered button labels
    4. Clicks Cancel, asserts no deletion occurred
    5. Repeats, clicks Confirm, asserts deletion occurred

  • The use:enhance cancel pattern needs a test. The issue mentions Option B is cleaner for form enhance cancel patterns. This is a meaningful integration: use:enhance with a submit callback that awaits confirm() before proceeding is a non-trivial interaction. It needs its own test to verify the form doesn't submit while the modal is open, and does submit after confirmation.

  • Visual regression tests. The modal has two visual states: default (neutral confirm button) and destructive (red confirm button). Both states should be captured in Playwright visual regression tests at 320px, 768px, and 1440px — the destructive state especially, since a styling bug could make a dangerous action look safe.

Suggested Test Matrix

Test Layer Key assertion
Service resolves true on confirm click Vitest (unit) await confirm(opts) returns true
Service resolves false on cancel click Vitest (unit) Returns false
Service resolves false on Escape key Vitest (unit) Returns false
confirm() outside context throws Vitest (unit) Error message contains "mount ConfirmProvider"
Delete transcription block — modal shown, no browser dialog Playwright (E2E) dialog event not fired, modal visible
Delete user — confirm then cancel Playwright (E2E) User still exists after cancel
Delete group — confirm then confirm Playwright (E2E) Group removed after confirmation
Destructive button is visually red Playwright (visual) Screenshot diff at 768px
## 🧪 Sara Holt — QA Engineer ### Questions & Observations - **The acceptance criteria are a solid starting point but missing behavioral edge cases.** The 7 items cover structure and style, but don't define expected behavior for: - What happens when `Escape` is pressed? (Should cancel — equivalent to clicking Cancel) - What happens when the backdrop is clicked? (Cancel, or ignored for destructive actions?) - What if confirm is called while a confirmation is already open? (Queue, replace, or throw?) - Does the Promise reject or resolve to `false` if the component is unmounted before the user responds? - **Three well-documented replacement targets make regression testing straightforward.** For each of the 3 native `confirm()` callsites, I'd write an E2E test that: 1. Triggers the action that previously used `confirm()` 2. Asserts the native browser dialog is *not* shown (Playwright's `page.on('dialog')` listener should not fire) 3. Asserts the custom modal *is* visible with the correct title and i18n-rendered button labels 4. Clicks Cancel, asserts no deletion occurred 5. Repeats, clicks Confirm, asserts deletion occurred - **The `use:enhance` cancel pattern needs a test.** The issue mentions Option B is cleaner for form enhance cancel patterns. This is a meaningful integration: `use:enhance` with a `submit` callback that awaits `confirm()` before proceeding is a non-trivial interaction. It needs its own test to verify the form doesn't submit while the modal is open, and does submit after confirmation. - **Visual regression tests.** The modal has two visual states: default (neutral confirm button) and destructive (red confirm button). Both states should be captured in Playwright visual regression tests at 320px, 768px, and 1440px — the destructive state especially, since a styling bug could make a dangerous action look safe. ### Suggested Test Matrix | Test | Layer | Key assertion | |---|---|---| | Service resolves `true` on confirm click | Vitest (unit) | `await confirm(opts)` returns `true` | | Service resolves `false` on cancel click | Vitest (unit) | Returns `false` | | Service resolves `false` on Escape key | Vitest (unit) | Returns `false` | | `confirm()` outside context throws | Vitest (unit) | Error message contains "mount ConfirmProvider" | | Delete transcription block — modal shown, no browser dialog | Playwright (E2E) | `dialog` event not fired, modal visible | | Delete user — confirm then cancel | Playwright (E2E) | User still exists after cancel | | Delete group — confirm then confirm | Playwright (E2E) | Group removed after confirmation | | Destructive button is visually red | Playwright (visual) | Screenshot diff at 768px |
marcel commented 2026-04-12 12:21:56 +02:00
Author
Owner
Copy Link

🔐 Nora "NullX" Steiner — Application Security Engineer

Questions & Observations

  • XSS risk in title and body props. If the title or body strings passed to ConfirmModal ever include user-controlled data (e.g. "Delete alias 'X'?" where X is the alias name), they must be rendered via Svelte's text interpolation ({title}, not {@html title}). Svelte escapes text interpolation by default — so as long as no {@html} is used in the component, this is safe. Make it a hard rule in the component: no {@html} in ConfirmModal.svelte, ever. Add a comment to that effect so future contributors don't accidentally "fix" it.

    The existing alias delete modal in NameHistoryEditCard is the migration target — check if it currently displays the alias name in the confirmation text. If so, the new shared modal will inherit that pattern and it must stay as text interpolation.

  • The native confirm() replacement eliminates a phishing vector. window.confirm() dialogs display the origin in Chrome/Firefox, making it clear to the user which site is asking. Custom modals don't have this — this is a minor regression in origin transparency, but it is standard practice for web apps and not a concern here. Just noting it for completeness.

  • Focus trapping is a security-adjacent concern. Without focus trapping, a user can Tab away from the modal and interact with the page behind it (including clicking other destructive actions). If the native <dialog> element with showModal() is used, focus trapping is provided by the browser. If a custom overlay is used, an explicit focus trap library (or manual implementation) is required. This should be an explicit acceptance criterion.

  • CSRF. The actual delete/confirm actions behind these modals are SvelteKit form actions — CSRF protection is handled by SvelteKit's built-in token mechanism. No concerns here; the modal is purely a UI gate, not an auth boundary.

  • confirmLabel coming from i18n keys — good. Static i18n strings mean there is no way for user input to appear in button labels. No XSS surface there.

Summary

No critical security concerns with this refactor. The main thing to enforce: {@html} must never be used in the confirmation modal component, even if the call site passes a string that looks like static content today. Enforce this with a linting comment in the component and ideally a Semgrep rule:

# Semgrep rule sketch
rules:
  - id: no-html-in-confirm-modal
    pattern: "{@html ...}"
    paths:
      include: ["**/ConfirmModal.svelte"]
    message: "ConfirmModal must not use {@html} — use text interpolation only"
## 🔐 Nora "NullX" Steiner — Application Security Engineer ### Questions & Observations - **XSS risk in `title` and `body` props.** If the `title` or `body` strings passed to `ConfirmModal` ever include user-controlled data (e.g. "Delete alias 'X'?" where X is the alias name), they must be rendered via Svelte's text interpolation (`{title}`, not `{@html title}`). Svelte escapes text interpolation by default — so as long as no `{@html}` is used in the component, this is safe. Make it a hard rule in the component: **no `{@html}` in `ConfirmModal.svelte`, ever.** Add a comment to that effect so future contributors don't accidentally "fix" it. The existing alias delete modal in `NameHistoryEditCard` is the migration target — check if it currently displays the alias name in the confirmation text. If so, the new shared modal will inherit that pattern and it must stay as text interpolation. - **The native `confirm()` replacement eliminates a phishing vector.** `window.confirm()` dialogs display the origin in Chrome/Firefox, making it clear to the user which site is asking. Custom modals don't have this — this is a minor regression in origin transparency, but it is standard practice for web apps and not a concern here. Just noting it for completeness. - **Focus trapping is a security-adjacent concern.** Without focus trapping, a user can Tab away from the modal and interact with the page behind it (including clicking other destructive actions). If the native `<dialog>` element with `showModal()` is used, focus trapping is provided by the browser. If a custom overlay is used, an explicit focus trap library (or manual implementation) is required. This should be an explicit acceptance criterion. - **CSRF.** The actual delete/confirm actions behind these modals are SvelteKit form actions — CSRF protection is handled by SvelteKit's built-in token mechanism. No concerns here; the modal is purely a UI gate, not an auth boundary. - **`confirmLabel` coming from i18n keys — good.** Static i18n strings mean there is no way for user input to appear in button labels. No XSS surface there. ### Summary No critical security concerns with this refactor. The main thing to enforce: `{@html}` must never be used in the confirmation modal component, even if the call site passes a string that looks like static content today. Enforce this with a linting comment in the component and ideally a Semgrep rule: ```yaml # Semgrep rule sketch rules: - id: no-html-in-confirm-modal pattern: "{@html ...}" paths: include: ["**/ConfirmModal.svelte"] message: "ConfirmModal must not use {@html} — use text interpolation only" ```
marcel commented 2026-04-12 12:22:17 +02:00
Author
Owner
Copy Link

🎨 Leonie Voss — UI/UX Designer

Questions & Observations

  • "Destructive confirmations use red confirm button" — which red? The brand palette doesn't include a standard danger red. The app uses brand-navy, brand-mint, and brand-sand. For a destructive action button, a common choice is Tailwind's bg-red-600 or bg-red-700, but this should be an explicit decision — not left to the implementer. Define the exact color (hex or Tailwind class) in the acceptance criteria before implementation. If red is introduced as a new semantic color, add it to layout.css as a named utility (e.g. --color-danger) rather than hardcoding red-600 at every callsite.

  • cancelLabel is missing from the proposed API. The issue shows confirmLabel as a prop but doesn't mention cancelLabel. In German/ES localization, "Abbrechen" and "Cancelar" are fine defaults, but for specialized confirmations the cancel label might need to be different (e.g. "Zurück" instead of "Abbrechen"). Add cancelLabel as an optional prop with a sensible i18n default.

  • Backdrop click behavior needs a design decision. Should clicking outside the modal cancel the confirmation? For destructive actions (delete user, delete group), closing via backdrop click should probably behave identically to Cancel — but this needs to be stated, not assumed.

  • The <dialog> element gives you ::backdrop for free. This is the correct CSS pseudo-element for styling the overlay behind the modal. It supports backdrop-filter: blur() and opacity, and doesn't require a separate DOM node. Use it rather than a manually rendered overlay <div>.

  • Animation. The current app's overlay (alias delete modal in NameHistoryEditCard) presumably has some entry/exit behavior. The shared modal should match it. Even a simple transition:fade on the <dialog> content and transition:opacity on the backdrop creates a polished feel. If the existing modal has no animation, this is an opportunity to add it consistently once rather than per-component.

  • Touch target check — already in ACs, good. >= 44px on both buttons. Confirm the buttons also have sufficient horizontal spacing so they're not accidentally tappable together on narrow screens (minimum 8px gap, prefer 12px).

Suggested additions to acceptance criteria

  • cancelLabel prop supported (optional, defaults to m.btn_cancel())
  • Danger color defined as a named CSS variable/utility in layout.css
  • Backdrop click closes the modal (same as Cancel)
  • Escape key closes the modal (same as Cancel)
  • Modal uses native <dialog> element with showModal()
  • Entry animation on open (e.g. transition:scale|fade)
## 🎨 Leonie Voss — UI/UX Designer ### Questions & Observations - **"Destructive confirmations use red confirm button" — which red?** The brand palette doesn't include a standard danger red. The app uses `brand-navy`, `brand-mint`, and `brand-sand`. For a destructive action button, a common choice is Tailwind's `bg-red-600` or `bg-red-700`, but this should be an explicit decision — not left to the implementer. Define the exact color (hex or Tailwind class) in the acceptance criteria before implementation. If red is introduced as a new semantic color, add it to `layout.css` as a named utility (e.g. `--color-danger`) rather than hardcoding `red-600` at every callsite. - **`cancelLabel` is missing from the proposed API.** The issue shows `confirmLabel` as a prop but doesn't mention `cancelLabel`. In German/ES localization, "Abbrechen" and "Cancelar" are fine defaults, but for specialized confirmations the cancel label might need to be different (e.g. "Zurück" instead of "Abbrechen"). Add `cancelLabel` as an optional prop with a sensible i18n default. - **Backdrop click behavior needs a design decision.** Should clicking outside the modal cancel the confirmation? For destructive actions (delete user, delete group), closing via backdrop click should probably behave identically to Cancel — but this needs to be stated, not assumed. - **The `<dialog>` element gives you `::backdrop` for free.** This is the correct CSS pseudo-element for styling the overlay behind the modal. It supports `backdrop-filter: blur()` and opacity, and doesn't require a separate DOM node. Use it rather than a manually rendered overlay `<div>`. - **Animation.** The current app's overlay (alias delete modal in `NameHistoryEditCard`) presumably has some entry/exit behavior. The shared modal should match it. Even a simple `transition:fade` on the `<dialog>` content and `transition:opacity` on the backdrop creates a polished feel. If the existing modal has no animation, this is an opportunity to add it consistently once rather than per-component. - **Touch target check — already in ACs, good.** `>= 44px` on both buttons. Confirm the buttons also have sufficient horizontal spacing so they're not accidentally tappable together on narrow screens (minimum 8px gap, prefer 12px). ### Suggested additions to acceptance criteria - [ ] `cancelLabel` prop supported (optional, defaults to `m.btn_cancel()`) - [ ] Danger color defined as a named CSS variable/utility in `layout.css` - [ ] Backdrop click closes the modal (same as Cancel) - [ ] `Escape` key closes the modal (same as Cancel) - [ ] Modal uses native `<dialog>` element with `showModal()` - [ ] Entry animation on open (e.g. `transition:scale|fade`)
marcel commented 2026-04-12 12:22:31 +02:00
Author
Owner
Copy Link

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

This is a pure frontend refactor — no infrastructure changes, no new services, no config required. From a deployment and operations angle, there's nothing blocking here.

A few things worth noting for completeness:

  • No SSR concerns for Svelte context. setContext/getContext in SvelteKit is SSR-safe — context is per-request, not global state. The deferred Promise pattern (storing a resolve callback in $state) is client-side by nature since it requires user interaction, but it should either be initialized lazily or guarded with if (browser) to avoid server-side instantiation of a Promise that can never resolve.

  • No CI pipeline changes needed. This refactor replaces existing component patterns — no new build steps, no new environment variables, no new Docker layers. The existing Vitest and Playwright pipelines will cover it.

  • Bundle size. Replacing three window.confirm() calls with a shared modal component adds a small amount of JS to the bundle. A shared component is better than three separate inline implementations from a bundle perspective — no concern here. Worth checking with npm run build that the bundle doesn't grow unexpectedly (it shouldn't).

  • The <dialog> element recommendation from the team. If the native <dialog> is used with showModal(), note that Playwright's test utilities interact with it differently from a custom div-based overlay — use page.getByRole('dialog') rather than a CSS selector, which is actually cleaner. No DevOps impact, just a heads-up for the test authors.

Nothing blocking from my side. Ship it.

## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations This is a pure frontend refactor — no infrastructure changes, no new services, no config required. From a deployment and operations angle, there's nothing blocking here. A few things worth noting for completeness: - **No SSR concerns for Svelte context.** `setContext`/`getContext` in SvelteKit is SSR-safe — context is per-request, not global state. The deferred Promise pattern (storing a `resolve` callback in `$state`) is client-side by nature since it requires user interaction, but it should either be initialized lazily or guarded with `if (browser)` to avoid server-side instantiation of a Promise that can never resolve. - **No CI pipeline changes needed.** This refactor replaces existing component patterns — no new build steps, no new environment variables, no new Docker layers. The existing Vitest and Playwright pipelines will cover it. - **Bundle size.** Replacing three `window.confirm()` calls with a shared modal component adds a small amount of JS to the bundle. A shared component is better than three separate inline implementations from a bundle perspective — no concern here. Worth checking with `npm run build` that the bundle doesn't grow unexpectedly (it shouldn't). - **The `<dialog>` element recommendation from the team.** If the native `<dialog>` is used with `showModal()`, note that Playwright's test utilities interact with it differently from a custom `div`-based overlay — use `page.getByRole('dialog')` rather than a CSS selector, which is actually cleaner. No DevOps impact, just a heads-up for the test authors. Nothing blocking from my side. Ship it.
marcel commented 2026-04-12 12:47:33 +02:00
Author
Owner
Copy Link

🎨 Leonie Voss — UI/UX Design Discussion

This comment captures the outcomes of a design walkthrough for the shared confirmation modal. All seven open items were resolved.

Resolved decisions

1. Danger color — new design token
Add a full --c-danger / --c-danger-fg token pair to layout.css, following the same pattern as --c-primary / --c-primary-fg:

  • Light mode: --c-danger: #c0392b (5.1:1 on white — WCAG AA ✓), --c-danger-fg: #ffffff
  • Dark mode: --c-danger: #e55347 (4.7:1 on #011526 — WCAG AA ✓), --c-danger-fg: #ffffff
  • Expose as Tailwind utilities via @theme inline: --color-danger: var(--c-danger) / --color-danger-fg: var(--c-danger-fg)
  • The existing bg-red-600 in NameHistoryEditCard migrates to bg-danger text-danger-fg

2. cancelLabel prop
Optional prop, defaults to m.btn_cancel(). Only passed when a non-standard label is needed.

3. Backdrop click behavior
Configurable via a closeOnBackdrop prop. Defaults to !destructive — so destructive confirmations block backdrop-click-to-close automatically, without requiring an explicit override.

4. Escape key
Always cancels — no configuration, no exceptions. Blocking Escape on a modal is an accessibility anti-pattern; the safety concern for destructive actions is covered by the backdrop behavior above.

5. Animation
Simple transition:fade on both the backdrop and the dialog panel. Matches the general app animation style without adding complexity.

6. Native <dialog> element
Use <dialog> with showModal() / close(). Provides focus trapping, Escape handling, top-layer rendering, and ::backdrop for free. The existing div-based overlay in NameHistoryEditCard is replaced.

7. Visual appearance
Match the existing alias delete modal exactly — no redesign:

  • Container: max-w-sm rounded-sm border border-line bg-surface p-6 shadow-lg
  • Title: font-serif text-lg text-ink
  • Body: text-sm text-ink-2
  • Cancel: border border-line px-4 py-2 text-sm font-medium text-ink-2 hover:bg-muted
  • Confirm (default): bg-primary text-primary-fg px-4 py-2 text-sm font-medium
  • Confirm (destructive): bg-danger text-danger-fg px-4 py-2 text-sm font-medium
  • Button layout: right-aligned row, gap-3
  • Add aria-labelledby wiring the title element to the <dialog>: <dialog aria-labelledby="confirm-title">

The design system is ready to support this component. The --c-danger token should be added in the same commit as the modal — it's a prerequisite, and other components (NameHistoryEditCard delete button, any future destructive UI) should migrate to it at the same time.

## 🎨 Leonie Voss — UI/UX Design Discussion This comment captures the outcomes of a design walkthrough for the shared confirmation modal. All seven open items were resolved. --- ### Resolved decisions **1. Danger color — new design token** Add a full `--c-danger` / `--c-danger-fg` token pair to `layout.css`, following the same pattern as `--c-primary` / `--c-primary-fg`: - Light mode: `--c-danger: #c0392b` (5.1:1 on white — WCAG AA ✓), `--c-danger-fg: #ffffff` - Dark mode: `--c-danger: #e55347` (4.7:1 on `#011526` — WCAG AA ✓), `--c-danger-fg: #ffffff` - Expose as Tailwind utilities via `@theme inline`: `--color-danger: var(--c-danger)` / `--color-danger-fg: var(--c-danger-fg)` - The existing `bg-red-600` in `NameHistoryEditCard` migrates to `bg-danger text-danger-fg` **2. `cancelLabel` prop** Optional prop, defaults to `m.btn_cancel()`. Only passed when a non-standard label is needed. **3. Backdrop click behavior** Configurable via a `closeOnBackdrop` prop. Defaults to `!destructive` — so destructive confirmations block backdrop-click-to-close automatically, without requiring an explicit override. **4. Escape key** Always cancels — no configuration, no exceptions. Blocking Escape on a modal is an accessibility anti-pattern; the safety concern for destructive actions is covered by the backdrop behavior above. **5. Animation** Simple `transition:fade` on both the backdrop and the dialog panel. Matches the general app animation style without adding complexity. **6. Native `<dialog>` element** Use `<dialog>` with `showModal()` / `close()`. Provides focus trapping, Escape handling, top-layer rendering, and `::backdrop` for free. The existing div-based overlay in `NameHistoryEditCard` is replaced. **7. Visual appearance** Match the existing alias delete modal exactly — no redesign: - Container: `max-w-sm rounded-sm border border-line bg-surface p-6 shadow-lg` - Title: `font-serif text-lg text-ink` - Body: `text-sm text-ink-2` - Cancel: `border border-line px-4 py-2 text-sm font-medium text-ink-2 hover:bg-muted` - Confirm (default): `bg-primary text-primary-fg px-4 py-2 text-sm font-medium` - Confirm (destructive): `bg-danger text-danger-fg px-4 py-2 text-sm font-medium` - Button layout: right-aligned row, `gap-3` - Add `aria-labelledby` wiring the title element to the `<dialog>`: `<dialog aria-labelledby="confirm-title">` --- The design system is ready to support this component. The `--c-danger` token should be added in the same commit as the modal — it's a prerequisite, and other components (`NameHistoryEditCard` delete button, any future destructive UI) should migrate to it at the same time.
marcel commented 2026-04-12 12:57:09 +02:00
Author
Owner
Copy Link

🏛️ Markus Keller — Architecture Discussion

This comment captures the outcomes of an architectural walkthrough for the shared confirmation service. All six open items were resolved.

Resolved decisions

1. Option A vs. Option B — Option B chosen
Context-based async service returning Promise<boolean>. Callers use await confirm(...) in event handlers or use:enhance submit callbacks. Modal state is owned entirely by the service — no local state in consuming components.

2. Module boundary — split into two files

  • $lib/services/confirm.ts — service logic: context setup, confirm() function, deferred Promise lifecycle
  • $lib/components/ConfirmDialog.svelte — view only: reads from context, renders the <dialog>, calls resolve on button click
  • Keeping them separate makes the service independently testable and the component a pure rendering concern.

3. Context provider location — root layout
<ConfirmDialog> is mounted once in the root +layout.svelte. All routes share it — no nested instances needed.

4. Missing context guard — add to ACs
getConfirmService() must throw a descriptive error if called outside the provider tree:

if (!service) throw new Error('ConfirmService not found — mount <ConfirmDialog> in +layout.svelte');

Add to acceptance criteria: calling confirm() outside the provider throws a clear developer error.

5. SSR safety
confirm() returns Promise.resolve(false) when called server-side (!browser). Safe non-interactive fallback — code paths that await confirm() during SSR proceed as if the user cancelled.

6. Scope of "could also migrate" patterns

  • SaveBar.svelte (delete document) — included in this issue. Straightforward confirmDelete toggle migration.
  • PersonMergePanel.svelte (merge person) — explicitly out of scope. Its two-step inline flow shows contextual merge information that a generic modal would lose. Track as a separate issue.

AC additions from this discussion

  • $lib/services/confirm.ts and $lib/components/ConfirmDialog.svelte are separate files
  • getConfirmService() throws a descriptive error if called outside the provider tree
  • confirm() returns Promise.resolve(false) when called server-side
  • SaveBar.svelte delete document confirmation uses the shared service
  • PersonMergePanel.svelte explicitly excluded — tracked separately

The architecture is straightforward. The deferred Promise pattern is the only subtle piece — write the failing test for it first before touching any consuming component.

## 🏛️ Markus Keller — Architecture Discussion This comment captures the outcomes of an architectural walkthrough for the shared confirmation service. All six open items were resolved. --- ### Resolved decisions **1. Option A vs. Option B — Option B chosen** Context-based async service returning `Promise<boolean>`. Callers use `await confirm(...)` in event handlers or `use:enhance` submit callbacks. Modal state is owned entirely by the service — no local state in consuming components. **2. Module boundary — split into two files** - `$lib/services/confirm.ts` — service logic: context setup, `confirm()` function, deferred Promise lifecycle - `$lib/components/ConfirmDialog.svelte` — view only: reads from context, renders the `<dialog>`, calls resolve on button click - Keeping them separate makes the service independently testable and the component a pure rendering concern. **3. Context provider location — root layout** `<ConfirmDialog>` is mounted once in the root `+layout.svelte`. All routes share it — no nested instances needed. **4. Missing context guard — add to ACs** `getConfirmService()` must throw a descriptive error if called outside the provider tree: ```typescript if (!service) throw new Error('ConfirmService not found — mount <ConfirmDialog> in +layout.svelte'); ``` Add to acceptance criteria: calling `confirm()` outside the provider throws a clear developer error. **5. SSR safety** `confirm()` returns `Promise.resolve(false)` when called server-side (`!browser`). Safe non-interactive fallback — code paths that await `confirm()` during SSR proceed as if the user cancelled. **6. Scope of "could also migrate" patterns** - `SaveBar.svelte` (delete document) — **included in this issue**. Straightforward `confirmDelete` toggle migration. - `PersonMergePanel.svelte` (merge person) — **explicitly out of scope**. Its two-step inline flow shows contextual merge information that a generic modal would lose. Track as a separate issue. --- ### AC additions from this discussion - [ ] `$lib/services/confirm.ts` and `$lib/components/ConfirmDialog.svelte` are separate files - [ ] `getConfirmService()` throws a descriptive error if called outside the provider tree - [ ] `confirm()` returns `Promise.resolve(false)` when called server-side - [ ] `SaveBar.svelte` delete document confirmation uses the shared service - [ ] `PersonMergePanel.svelte` explicitly excluded — tracked separately --- The architecture is straightforward. The deferred Promise pattern is the only subtle piece — write the failing test for it first before touching any consuming component.
marcel commented 2026-04-12 13:03:25 +02:00
Author
Owner
Copy Link

👨‍💻 Felix Brandt — Developer Discussion

This comment captures implementation decisions from a developer walkthrough. All five open items resolved.

Resolved decisions

1. Concurrent calls — return false immediately
If confirm() is called while a confirmation is already open, return Promise.resolve(false) immediately. Guard at the top of confirm():

if (resolve !== null) return Promise.resolve(false);

Two simultaneous confirmations is a UI bug, not a case the service should recover from.

2. ConfirmOptions interface — pinned

interface ConfirmOptions {
    title: string;
    body?: string;               // optional — some confirmations are self-explanatory
    confirmLabel?: string;       // defaults to m.btn_confirm()
    cancelLabel?: string;        // defaults to m.btn_cancel()
    destructive?: boolean;       // defaults to false
    closeOnBackdrop?: boolean;   // defaults to !destructive
}

Prerequisite: btn_confirm i18n key does not exist yet — must be added to all three message files before implementation:

  • de.json: "btn_confirm": "Bestätigen"
  • en.json: "btn_confirm": "Confirm"
  • es.json: "btn_confirm": "Confirmar"

3. First failing test — round-trip
The first test drives the entire deferred Promise design:

it('resolves true when the user clicks confirm', async () => {
    render(TestHost);  // mounts ConfirmDialog + provides context
    const resultPromise = getConfirmService().confirm({ title: 'Test?' });
    await screen.findByRole('dialog');
    await userEvent.click(screen.getByRole('button', { name: 'Bestätigen' }));
    expect(await resultPromise).toBe(true);
});

TestHost is a minimal inline Svelte component — not a shared fixture. Write this test first. It fails until the service, component, and context wiring all exist.

4. use:enhance integration pattern — canonical approach

<form method="POST" action="?/deleteDocument" use:enhance={async ({ cancel }) => {
    const confirmed = await confirm({
        title: m.confirm_delete_document_title(),
        destructive: true
    });
    if (!confirmed) cancel();
}}>

cancel() aborts the form submission if the user cancels. No local state, no toggle. This pattern goes in a JSDoc usage comment in confirm.ts.

5. confirmDestructive() shorthand — rejected
Four callsites don't justify the abstraction. confirm({ destructive: true }) is readable enough. KISS over DRY.

AC additions from this discussion

  • btn_confirm i18n key added to de.json, en.json, es.json
  • confirm() returns Promise.resolve(false) immediately if already open
  • confirm.ts includes a JSDoc usage example for the use:enhance pattern
  • First commit is the failing round-trip test — no production code before it

The deferred Promise test is the north star. Everything else — the component, the migrations, the use:enhance wiring — follows from making that test green.

## 👨‍💻 Felix Brandt — Developer Discussion This comment captures implementation decisions from a developer walkthrough. All five open items resolved. --- ### Resolved decisions **1. Concurrent calls — return false immediately** If `confirm()` is called while a confirmation is already open, return `Promise.resolve(false)` immediately. Guard at the top of `confirm()`: ```typescript if (resolve !== null) return Promise.resolve(false); ``` Two simultaneous confirmations is a UI bug, not a case the service should recover from. **2. `ConfirmOptions` interface — pinned** ```typescript interface ConfirmOptions { title: string; body?: string; // optional — some confirmations are self-explanatory confirmLabel?: string; // defaults to m.btn_confirm() cancelLabel?: string; // defaults to m.btn_cancel() destructive?: boolean; // defaults to false closeOnBackdrop?: boolean; // defaults to !destructive } ``` **Prerequisite:** `btn_confirm` i18n key does not exist yet — must be added to all three message files before implementation: - `de.json`: `"btn_confirm": "Bestätigen"` - `en.json`: `"btn_confirm": "Confirm"` - `es.json`: `"btn_confirm": "Confirmar"` **3. First failing test — round-trip** The first test drives the entire deferred Promise design: ```typescript it('resolves true when the user clicks confirm', async () => { render(TestHost); // mounts ConfirmDialog + provides context const resultPromise = getConfirmService().confirm({ title: 'Test?' }); await screen.findByRole('dialog'); await userEvent.click(screen.getByRole('button', { name: 'Bestätigen' })); expect(await resultPromise).toBe(true); }); ``` `TestHost` is a minimal inline Svelte component — not a shared fixture. Write this test first. It fails until the service, component, and context wiring all exist. **4. `use:enhance` integration pattern — canonical approach** ```svelte <form method="POST" action="?/deleteDocument" use:enhance={async ({ cancel }) => { const confirmed = await confirm({ title: m.confirm_delete_document_title(), destructive: true }); if (!confirmed) cancel(); }}> ``` `cancel()` aborts the form submission if the user cancels. No local state, no toggle. This pattern goes in a JSDoc usage comment in `confirm.ts`. **5. `confirmDestructive()` shorthand — rejected** Four callsites don't justify the abstraction. `confirm({ destructive: true })` is readable enough. KISS over DRY. --- ### AC additions from this discussion - [ ] `btn_confirm` i18n key added to `de.json`, `en.json`, `es.json` - [ ] `confirm()` returns `Promise.resolve(false)` immediately if already open - [ ] `confirm.ts` includes a JSDoc usage example for the `use:enhance` pattern - [ ] First commit is the failing round-trip test — no production code before it --- The deferred Promise test is the north star. Everything else — the component, the migrations, the `use:enhance` wiring — follows from making that test green.
marcel commented 2026-04-12 14:21:52 +02:00
Author
Owner
Copy Link

Implementierung abgeschlossen ✅

Branch: feat/issue-207-confirm-modal-service

Was wurde umgesetzt

ConfirmService-Infrastruktur

  • src/lib/services/confirm.svelte.ts — createConfirmService(), provideConfirmService(), getConfirmService(), Deferred-Promise-Pattern mit $state
  • src/lib/components/ConfirmDialog.svelte — natives <dialog>-Element mit showModal(), backdrop-close, Escape-Handling, WCAG-AA-konforme Farben (bg-danger text-danger-fg)
  • src/lib/services/confirm.test-host.svelte + confirm.svelte.test.ts — 5 Servicetests (confirm/cancel/Escape/concurrent/kein Provider)
  • src/routes/+layout.svelte — provideConfirmService() + <ConfirmDialog /> global eingebunden

CSS-Design-Tokens (layout.css)

  • --c-danger / --c-danger-fg für Light und Dark Mode (WCAG AA)
  • btn_confirm i18n-Schlüssel in de/en/es

Migrationen (alle window.confirm() und inline Modals ersetzt)

Datei Vorher Nachher
TranscriptionBlock.svelte window.confirm() await confirm({ destructive: true })
admin/users/[id]/+page.svelte window.confirm() in use:enhance handleDelete() → requestSubmit()
admin/groups/[id]/+page.svelte window.confirm() in use:enhance handleDelete() → requestSubmit()
documents/[id]/edit/SaveBar.svelte Inline confirm-Toggle (confirmDelete state) await confirm({ destructive: true })
persons/[id]/edit/NameHistoryEditCard.svelte Inline custom <dialog>-Overlay await confirm(...) + hidden form

Commits

  • 775d49f — Service + ConfirmDialog + Layout
  • (weitere) — Migration aller Komponenten

Tests

  • 671 Tests grün, 4 vorher bestehende Flaky-Failures (CommentThread, TranscriptionBlock quote, TranscriptionEditView blur, Conversations)
  • Neue Tests: ConfirmService (5), TranscriptionBlock delete (2), TranscriptionEditView delete (2), admin user delete (3), admin group delete (3), SaveBar delete (3), NameHistoryEditCard delete (4+)
## Implementierung abgeschlossen ✅ Branch: `feat/issue-207-confirm-modal-service` ### Was wurde umgesetzt **ConfirmService-Infrastruktur** - `src/lib/services/confirm.svelte.ts` — `createConfirmService()`, `provideConfirmService()`, `getConfirmService()`, Deferred-Promise-Pattern mit `$state` - `src/lib/components/ConfirmDialog.svelte` — natives `<dialog>`-Element mit `showModal()`, backdrop-close, Escape-Handling, WCAG-AA-konforme Farben (`bg-danger text-danger-fg`) - `src/lib/services/confirm.test-host.svelte` + `confirm.svelte.test.ts` — 5 Servicetests (confirm/cancel/Escape/concurrent/kein Provider) - `src/routes/+layout.svelte` — `provideConfirmService()` + `<ConfirmDialog />` global eingebunden **CSS-Design-Tokens** (`layout.css`) - `--c-danger` / `--c-danger-fg` für Light und Dark Mode (WCAG AA) - `btn_confirm` i18n-Schlüssel in de/en/es **Migrationen (alle `window.confirm()` und inline Modals ersetzt)** | Datei | Vorher | Nachher | |---|---|---| | `TranscriptionBlock.svelte` | `window.confirm()` | `await confirm({ destructive: true })` | | `admin/users/[id]/+page.svelte` | `window.confirm()` in `use:enhance` | `handleDelete()` → `requestSubmit()` | | `admin/groups/[id]/+page.svelte` | `window.confirm()` in `use:enhance` | `handleDelete()` → `requestSubmit()` | | `documents/[id]/edit/SaveBar.svelte` | Inline confirm-Toggle (`confirmDelete` state) | `await confirm({ destructive: true })` | | `persons/[id]/edit/NameHistoryEditCard.svelte` | Inline custom `<dialog>`-Overlay | `await confirm(...)` + hidden form | ### Commits - `775d49f` — Service + ConfirmDialog + Layout - `(weitere)` — Migration aller Komponenten ### Tests - 671 Tests grün, 4 vorher bestehende Flaky-Failures (CommentThread, TranscriptionBlock quote, TranscriptionEditView blur, Conversations) - Neue Tests: ConfirmService (5), TranscriptionBlock delete (2), TranscriptionEditView delete (2), admin user delete (3), admin group delete (3), SaveBar delete (3), NameHistoryEditCard delete (4+)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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 refactor ui
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#207
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?