feat(viewer): show delete icon directly on transcription annotation #339

New Issue

marcel · 2026-04-26T19:56:16+02:00

marcel commented

2026-04-26 19:56:16 +02:00

User story

As a transcriber, I want to delete a transcription block directly from its annotation in the PDF viewer, so that I don't have to navigate to Bearbeiten → Block → Block löschen.

Context

Current flow is 3+ steps: open Bearbeiten tab → find the block in the list → click Delete. Users expect a trash icon directly on (or adjacent to) the highlighted annotation region, making deletion a single gesture.

Acceptance criteria

Given a transcription block is visible as an annotation, when the user hovers or focuses the annotation, then a delete (trash) icon appears on or adjacent to it.
Given the icon is visible, when the user clicks it, then the block is deleted (with a confirmation or undo-toast to prevent accidental loss).
The existing Bearbeiten → Block → Block löschen path remains functional as a secondary path.

NFRs

Deletion is keyboard-accessible (e.g. Delete key on a focused annotation).
Touch target ≥ 44 px on tablet.

## User story As a transcriber, I want to delete a transcription block directly from its annotation in the PDF viewer, so that I don't have to navigate to Bearbeiten → Block → Block löschen. ## Context Current flow is 3+ steps: open Bearbeiten tab → find the block in the list → click Delete. Users expect a trash icon directly on (or adjacent to) the highlighted annotation region, making deletion a single gesture. ## Acceptance criteria - Given a transcription block is visible as an annotation, when the user hovers or focuses the annotation, then a delete (trash) icon appears on or adjacent to it. - Given the icon is visible, when the user clicks it, then the block is deleted (with a confirmation or undo-toast to prevent accidental loss). - The existing Bearbeiten → Block → Block löschen path remains functional as a secondary path. ## NFRs - Deletion is keyboard-accessible (e.g. Delete key on a focused annotation). - Touch target ≥ 44 px on tablet.

marcel added this to the (deleted) milestone 2026-04-26 19:56:16 +02:00

marcel added the P2-medium feature ui labels 2026-04-26 19:56:52 +02:00

marcel commented

2026-04-26 19:59:08 +02:00

👨‍💼 Markus Keller — Application Architect

Observations

The feature is purely frontend-side: TranscriptionBlockController.deleteBlock() already exists at DELETE /api/documents/{documentId}/transcription-blocks/{blockId}, and deleteBlock() in +page.svelte already wires it correctly. No backend change is needed.
The cross-concern here is between AnnotationShape.svelte (the clickable PDF overlay) and TranscriptionBlock.svelte (the sidebar card). The delete gesture must be initiated on the annotation region but the block ID lives in the sidebar data model, linked via annotationId in TranscriptionBlockData.
The AnnotationLayer.svelte → AnnotationShape.svelte → +page.svelte event chain already carries annotationId. The +page.svelte has transcriptionBlocks in scope. The architectural path is: AnnotationShape emits onDeleteRequest(annotationId), +page.svelte resolves blockId from annotationId, calls existing deleteBlock(blockId).
No new props need to pass through DocumentViewer.svelte → PdfViewer.svelte → AnnotationLayer.svelte → AnnotationShape.svelte unless a full delete capability is wired into each layer. An alternative is to keep the delete icon purely in the AnnotationShape, but only render it in transcribe-edit mode (controlled by the existing canDraw prop already passed through the chain).

Recommendations

Add onDeleteRequest?: (annotationId: string) => void to AnnotationLayer and AnnotationShape. Wire it through DocumentViewer from the onTranscriptionDraw-like slot that +page.svelte already manages. The canDraw flag already gates transcribe-edit mode — use it as the condition to show the delete icon.
Keep the confirmation modal in +page.svelte (where deleteBlock already lives with access to the confirm service context), not inside AnnotationShape. AnnotationShape should only emit the intent; the page orchestrates the confirmation and API call.
The annotationsDimmed flag (passed when in read mode) can double as the "hide delete icon" signal — if annotations are dimmed, the delete affordance should not appear.

Open Decisions

Should onDeleteRequest wire through all four layers (PdfViewer → AnnotationLayer → AnnotationShape → +page.svelte), or should AnnotationShape receive the block ID directly so it can trigger confirmation inline? The prop-drilling path preserves the layering boundary; inline would reduce the chain at the cost of a new dependency between annotation and transcription domains.

## 👨‍💼 Markus Keller — Application Architect ### Observations - The feature is purely frontend-side: `TranscriptionBlockController.deleteBlock()` already exists at `DELETE /api/documents/{documentId}/transcription-blocks/{blockId}`, and `deleteBlock()` in `+page.svelte` already wires it correctly. No backend change is needed. - The cross-concern here is between `AnnotationShape.svelte` (the clickable PDF overlay) and `TranscriptionBlock.svelte` (the sidebar card). The delete gesture must be initiated on the annotation region but the block ID lives in the sidebar data model, linked via `annotationId` in `TranscriptionBlockData`. - The `AnnotationLayer.svelte` → `AnnotationShape.svelte` → `+page.svelte` event chain already carries `annotationId`. The `+page.svelte` has `transcriptionBlocks` in scope. The architectural path is: `AnnotationShape` emits `onDeleteRequest(annotationId)`, `+page.svelte` resolves `blockId` from `annotationId`, calls existing `deleteBlock(blockId)`. - No new props need to pass through `DocumentViewer.svelte` → `PdfViewer.svelte` → `AnnotationLayer.svelte` → `AnnotationShape.svelte` unless a full delete capability is wired into each layer. An alternative is to keep the delete icon purely in the `AnnotationShape`, but only render it in transcribe-edit mode (controlled by the existing `canDraw` prop already passed through the chain). ### Recommendations - Add `onDeleteRequest?: (annotationId: string) => void` to `AnnotationLayer` and `AnnotationShape`. Wire it through `DocumentViewer` from the `onTranscriptionDraw`-like slot that `+page.svelte` already manages. The `canDraw` flag already gates transcribe-edit mode — use it as the condition to show the delete icon. - Keep the confirmation modal in `+page.svelte` (where `deleteBlock` already lives with access to the confirm service context), not inside `AnnotationShape`. `AnnotationShape` should only emit the intent; the page orchestrates the confirmation and API call. - The `annotationsDimmed` flag (passed when in read mode) can double as the "hide delete icon" signal — if annotations are dimmed, the delete affordance should not appear. ### Open Decisions - Should `onDeleteRequest` wire through all four layers (`PdfViewer` → `AnnotationLayer` → `AnnotationShape` → `+page.svelte`), or should `AnnotationShape` receive the block ID directly so it can trigger confirmation inline? The prop-drilling path preserves the layering boundary; inline would reduce the chain at the cost of a new dependency between annotation and transcription domains.

marcel commented

2026-04-26 19:59:21 +02:00

👩‍💻 Felix Brandt — Fullstack Developer

Observations

AnnotationShape.svelte already renders a numbered badge in the top-left corner. The delete icon should follow the same pattern: absolutely positioned, pointer-events: auto, conditionally rendered on hover/focus.
The isHovered and isActive props are already tracked in AnnotationLayer.svelte via the hoveredId state and passed into AnnotationShape. Show the trash icon when isHovered || isActive and canDraw is true — no new state needed.
The keyboard requirement ("Delete key on a focused annotation") is almost free: AnnotationShape already handles onkeydown for Enter/Space. Add e.key === 'Delete' to the same handler.
The confirm modal call pattern is established in TranscriptionBlock.svelte (the existing handleDelete function uses getConfirmService()). Reuse the same pattern. The translation key transcription_block_delete_confirm already exists in de.json.
deleteBlock in +page.svelte takes a blockId, not an annotationId. The wiring must resolve via transcriptionBlocks.find(b => b.annotationId === annotationId) — the same lookup that handleAnnotationClick already does.

Recommendations

In AnnotationShape.svelte, add a trash button inside the existing <div> conditionally when isHovered || isActive. Place it top-right to avoid collision with the numbered badge (top-left). Example structure:

{#if showDelete && (isHovered || isActive)}
  <button
    type="button"
    aria-label="Block löschen"
    onclick={(e) => { e.stopPropagation(); onDeleteRequest?.(); }}
    class="..."
    style="position:absolute; top:4px; right:4px; ..."
  >
    <!-- trash SVG -->
  </button>
{/if}

Add showDelete?: boolean and onDeleteRequest?: () => void to AnnotationShape props rather than a full annotationId — keeps the component's API surface minimal.

Add e.key === 'Delete' to the existing onkeydown handler in AnnotationShape:

onkeydown={(e) => {
  if (e.key === 'Enter' || e.key === ' ') onclick();
  if (e.key === 'Delete') onDeleteRequest?.();
}}

In AnnotationLayer.svelte, pass showDelete={canDraw} and onDeleteRequest={() => onDeleteRequest?.(annotation.id)} through to each AnnotationShape.
Wire onDeleteRequest through DocumentViewer and handle it in +page.svelte using the existing handleAnnotationClick lookup pattern followed by deleteBlock.

Open Decisions (omit if none)

The existing TranscriptionBlock.svelte delete flow uses a modal confirm. The issue mentions "confirmation or undo-toast." Picking one interaction model is a product decision — see Decision Queue.

## 👩‍💻 Felix Brandt — Fullstack Developer ### Observations - `AnnotationShape.svelte` already renders a numbered badge in the top-left corner. The delete icon should follow the same pattern: absolutely positioned, `pointer-events: auto`, conditionally rendered on hover/focus. - The `isHovered` and `isActive` props are already tracked in `AnnotationLayer.svelte` via the `hoveredId` state and passed into `AnnotationShape`. Show the trash icon when `isHovered || isActive` and `canDraw` is true — no new state needed. - The keyboard requirement ("Delete key on a focused annotation") is almost free: `AnnotationShape` already handles `onkeydown` for Enter/Space. Add `e.key === 'Delete'` to the same handler. - The confirm modal call pattern is established in `TranscriptionBlock.svelte` (the existing `handleDelete` function uses `getConfirmService()`). Reuse the same pattern. The translation key `transcription_block_delete_confirm` already exists in `de.json`. - `deleteBlock` in `+page.svelte` takes a `blockId`, not an `annotationId`. The wiring must resolve via `transcriptionBlocks.find(b => b.annotationId === annotationId)` — the same lookup that `handleAnnotationClick` already does. ### Recommendations - In `AnnotationShape.svelte`, add a trash button inside the existing `<div>` conditionally when `isHovered || isActive`. Place it top-right to avoid collision with the numbered badge (top-left). Example structure: ```svelte {#if showDelete && (isHovered || isActive)} <button type="button" aria-label="Block löschen" onclick={(e) => { e.stopPropagation(); onDeleteRequest?.(); }} class="..." style="position:absolute; top:4px; right:4px; ..." >  </button> {/if} ``` - Add `showDelete?: boolean` and `onDeleteRequest?: () => void` to `AnnotationShape` props rather than a full `annotationId` — keeps the component's API surface minimal. - Add `e.key === 'Delete'` to the existing `onkeydown` handler in `AnnotationShape`: ```svelte onkeydown={(e) => { if (e.key === 'Enter' || e.key === ' ') onclick(); if (e.key === 'Delete') onDeleteRequest?.(); }} ``` - In `AnnotationLayer.svelte`, pass `showDelete={canDraw}` and `onDeleteRequest={() => onDeleteRequest?.(annotation.id)}` through to each `AnnotationShape`. - Wire `onDeleteRequest` through `DocumentViewer` and handle it in `+page.svelte` using the existing `handleAnnotationClick` lookup pattern followed by `deleteBlock`. ### Open Decisions _(omit if none)_ - The existing `TranscriptionBlock.svelte` delete flow uses a modal confirm. The issue mentions "confirmation or undo-toast." Picking one interaction model is a product decision — see Decision Queue.

marcel commented

2026-04-26 19:59:31 +02:00

🛡️ Nora "NullX" Steiner — Security Expert

Observations

The backend delete endpoint DELETE /api/documents/{documentId}/transcription-blocks/{blockId} is already protected with @RequirePermission(Permission.WRITE_ALL). No new backend permission surface is introduced by this feature.
The client-side deleteBlock() in +page.svelte correctly checks res.ok before mutating local state. No change needed there.
The canWrite flag (derived from data.canWrite) gates transcribe mode and the edit panel. However, I don't see canWrite currently being threaded into AnnotationLayer to suppress the delete affordance for read-only users. The transcribeMode && !ocrRunning flag is passed as transcribeMode to DocumentViewer, which maps to canDraw in AnnotationLayer. If canDraw is false, the delete icon should not appear — this is the correct guard.
Verify that canDraw is false for users with only READ_ALL permission. The current transcribeMode state starts as false and is toggled by DocumentTopBar, but canWrite must control whether the mode can even be entered.

Recommendations

Explicitly gate the delete icon behind canDraw (not just hover state). Since canDraw already reflects transcribeMode && !ocrRunning, adding showDelete={canDraw} as a prop is sufficient — do not add a separate permission check inside AnnotationShape, which has no business knowing about permissions.
Add a @WebMvcTest test verifying that DELETE /api/documents/{id}/transcription-blocks/{blockId} returns 403 when the authenticated user has only READ_ALL. This is the standard security regression test for any write endpoint and should be added alongside the implementation.

## 🛡️ Nora "NullX" Steiner — Security Expert ### Observations - The backend delete endpoint `DELETE /api/documents/{documentId}/transcription-blocks/{blockId}` is already protected with `@RequirePermission(Permission.WRITE_ALL)`. No new backend permission surface is introduced by this feature. - The client-side `deleteBlock()` in `+page.svelte` correctly checks `res.ok` before mutating local state. No change needed there. - The `canWrite` flag (derived from `data.canWrite`) gates transcribe mode and the edit panel. However, I don't see `canWrite` currently being threaded into `AnnotationLayer` to suppress the delete affordance for read-only users. The `transcribeMode && !ocrRunning` flag is passed as `transcribeMode` to `DocumentViewer`, which maps to `canDraw` in `AnnotationLayer`. If `canDraw` is `false`, the delete icon should not appear — this is the correct guard. - Verify that `canDraw` is `false` for users with only `READ_ALL` permission. The current `transcribeMode` state starts as `false` and is toggled by `DocumentTopBar`, but `canWrite` must control whether the mode can even be entered. ### Recommendations - Explicitly gate the delete icon behind `canDraw` (not just hover state). Since `canDraw` already reflects `transcribeMode && !ocrRunning`, adding `showDelete={canDraw}` as a prop is sufficient — do not add a separate permission check inside `AnnotationShape`, which has no business knowing about permissions. - Add a `@WebMvcTest` test verifying that `DELETE /api/documents/{id}/transcription-blocks/{blockId}` returns 403 when the authenticated user has only `READ_ALL`. This is the standard security regression test for any write endpoint and should be added alongside the implementation.

marcel commented

2026-04-26 19:59:47 +02:00

🎨 Leonie Voss — UI/UX Expert

Observations

The primary audience for the transcription panel is 60+ transcribers working on laptop/tablet. Hover-only affordances are a high-risk interaction pattern for this group — they may never discover the trash icon if it only appears on hover.
The numbered badge in AnnotationShape is 20×20 px with inline styles. The new trash icon button must be at minimum 44×44 px touch target (WCAG 2.2 SC 2.5.8) — this is the stated NFR in the issue. The 20 px badge is decorative (number label), but the delete button is interactive.
The annotation-flash animation already respects prefers-reduced-motion with a fallback outline. Any hover/appear animation on the delete icon must do the same.
Placing the icon "on or adjacent to" the annotation region is correct. Top-right corner avoids collision with the existing numbered badge (top-left). For very narrow annotations (common in single-line Kurrent text), both a 44 px badge and a 44 px trash icon will not fit side-by-side. Consider positioning the trash icon outside the annotation shape, offset to the right, on a small floating chip.
The current AnnotationShape uses inline style for color via hexToRgba. The delete button's background and focus ring must use brand tokens, not hardcoded hex. Use var(--color-error) for the destructive hover state.

Recommendations

Render the delete icon as a small floating button offset to the top-right of the annotation bounding box (not inside the colored region). Use position: absolute; top: -8px; right: -8px; mirroring the badge pattern. Set min-width: 44px; min-height: 44px with display: flex; align-items: center; justify-content: center.
Show the delete button on isActive (tab focus or click) unconditionally, and on hover only as a secondary trigger. An annotation that has focus must always show the affordance — keyboard users cannot rely on hover.
Add aria-label={m.btn_delete()} to the trash button (the existing btn_delete key translates to "Löschen").
Use focus-visible:ring-2 focus-visible:ring-error for the focus state of the trash button, consistent with the project's focus indicator convention.
The @media (prefers-reduced-motion: reduce) block in AnnotationShape already exists — add the delete icon's appear animation to that same block.

Open Decisions

For very narrow annotations (width < 40 px, common for short Kurrent lines), the trash icon cannot fit without overlapping the PDF content. Define the minimum annotation width below which the icon is hidden and only the keyboard Delete key remains available.

## 🎨 Leonie Voss — UI/UX Expert ### Observations - The primary audience for the transcription panel is 60+ transcribers working on laptop/tablet. Hover-only affordances are a high-risk interaction pattern for this group — they may never discover the trash icon if it only appears on hover. - The numbered badge in `AnnotationShape` is 20×20 px with inline styles. The new trash icon button must be at minimum 44×44 px touch target (WCAG 2.2 SC 2.5.8) — this is the stated NFR in the issue. The 20 px badge is decorative (number label), but the delete button is interactive. - The `annotation-flash` animation already respects `prefers-reduced-motion` with a fallback outline. Any hover/appear animation on the delete icon must do the same. - Placing the icon "on or adjacent to" the annotation region is correct. Top-right corner avoids collision with the existing numbered badge (top-left). For very narrow annotations (common in single-line Kurrent text), both a 44 px badge and a 44 px trash icon will not fit side-by-side. Consider positioning the trash icon outside the annotation shape, offset to the right, on a small floating chip. - The current `AnnotationShape` uses inline `style` for color via `hexToRgba`. The delete button's background and focus ring must use brand tokens, not hardcoded hex. Use `var(--color-error)` for the destructive hover state. ### Recommendations - Render the delete icon as a small floating button offset to the top-right of the annotation bounding box (not inside the colored region). Use `position: absolute; top: -8px; right: -8px;` mirroring the badge pattern. Set `min-width: 44px; min-height: 44px` with `display: flex; align-items: center; justify-content: center`. - Show the delete button on `isActive` (tab focus or click) unconditionally, and on hover only as a secondary trigger. An annotation that has focus must always show the affordance — keyboard users cannot rely on hover. - Add `aria-label={m.btn_delete()}` to the trash button (the existing `btn_delete` key translates to "Löschen"). - Use `focus-visible:ring-2 focus-visible:ring-error` for the focus state of the trash button, consistent with the project's focus indicator convention. - The `@media (prefers-reduced-motion: reduce)` block in `AnnotationShape` already exists — add the delete icon's appear animation to that same block. ### Open Decisions - For very narrow annotations (width < 40 px, common for short Kurrent lines), the trash icon cannot fit without overlapping the PDF content. Define the minimum annotation width below which the icon is hidden and only the keyboard Delete key remains available.

marcel commented

2026-04-26 20:00:00 +02:00

🧪 Sara Holt — QA Engineer

Observations

The existing TranscriptionBlock.svelte delete path already has an integration path in +page.svelte (deleteBlock → fetch DELETE → filter transcriptionBlocks). No new API behavior is introduced — only a new trigger point.
The data-testid="annotation-{annotation.id}" attribute on AnnotationShape provides a stable Playwright selector. The delete button should carry data-testid="annotation-delete-{annotation.id}" for test targeting.
The ConfirmDialog is already tested via confirm.svelte.test.ts. The new flow reuses the same getConfirmService() pattern — no new modal infrastructure to test.
The keyboard path (Delete key on focused annotation) is easy to miss in E2E. Playwright's page.keyboard.press('Delete') handles this cleanly after a focus() call.

Recommendations

Unit / component level:

Add a Vitest browser-mode test for AnnotationShape verifying that:
1. The trash button appears when isHovered=true and showDelete=true.
2. The trash button is absent when showDelete=false.
3. onDeleteRequest is called when the trash button is clicked.
4. onDeleteRequest is called when Delete key is pressed while the annotation has focus.

Integration / E2E level:

Add a Playwright test:
1. Open a document in transcribe edit mode.
2. Hover [data-testid="annotation-{id}"] → verify [data-testid="annotation-delete-{id}"] becomes visible.
3. Click the trash button → verify the confirm dialog appears.
4. Confirm → verify the annotation is removed from the PDF overlay and the block count decreases.
5. Repeat for keyboard path: focus the annotation via Tab, press Delete, confirm.

Regression guard:

Add a @WebMvcTest test for the 403 response when a READ_ALL-only user attempts DELETE /api/documents/{id}/transcription-blocks/{blockId} (this is currently missing from TranscriptionBlockControllerTest).
The secondary path (Bearbeiten → Block → Block löschen) must remain tested — existing tests for TranscriptionBlock.svelte cover this and should not be removed.

Open Decisions

If an undo-toast is chosen over a confirm dialog, a new test scenario is needed: undo within the toast timer window must restore the block. The current deleteBlock implementation calls fetch immediately — an undo-toast would require buffering the delete. Define which pattern before implementation starts.

## 🧪 Sara Holt — QA Engineer ### Observations - The existing `TranscriptionBlock.svelte` delete path already has an integration path in `+page.svelte` (`deleteBlock` → fetch DELETE → filter `transcriptionBlocks`). No new API behavior is introduced — only a new trigger point. - The `data-testid="annotation-{annotation.id}"` attribute on `AnnotationShape` provides a stable Playwright selector. The delete button should carry `data-testid="annotation-delete-{annotation.id}"` for test targeting. - The `ConfirmDialog` is already tested via `confirm.svelte.test.ts`. The new flow reuses the same `getConfirmService()` pattern — no new modal infrastructure to test. - The keyboard path (Delete key on focused annotation) is easy to miss in E2E. Playwright's `page.keyboard.press('Delete')` handles this cleanly after a `focus()` call. ### Recommendations **Unit / component level:** - Add a Vitest browser-mode test for `AnnotationShape` verifying that: 1. The trash button appears when `isHovered=true` and `showDelete=true`. 2. The trash button is absent when `showDelete=false`. 3. `onDeleteRequest` is called when the trash button is clicked. 4. `onDeleteRequest` is called when Delete key is pressed while the annotation has focus. **Integration / E2E level:** - Add a Playwright test: 1. Open a document in transcribe edit mode. 2. Hover `[data-testid="annotation-{id}"]` → verify `[data-testid="annotation-delete-{id}"]` becomes visible. 3. Click the trash button → verify the confirm dialog appears. 4. Confirm → verify the annotation is removed from the PDF overlay and the block count decreases. 5. Repeat for keyboard path: focus the annotation via Tab, press Delete, confirm. **Regression guard:** - Add a `@WebMvcTest` test for the 403 response when a `READ_ALL`-only user attempts `DELETE /api/documents/{id}/transcription-blocks/{blockId}` (this is currently missing from `TranscriptionBlockControllerTest`). - The secondary path (Bearbeiten → Block → Block löschen) must remain tested — existing tests for `TranscriptionBlock.svelte` cover this and should not be removed. ### Open Decisions - If an undo-toast is chosen over a confirm dialog, a new test scenario is needed: undo within the toast timer window must restore the block. The current `deleteBlock` implementation calls fetch immediately — an undo-toast would require buffering the delete. Define which pattern before implementation starts.

marcel commented

2026-04-26 20:00:07 +02:00

🏗️ Tobias Wendt — DevOps

Observations

This is a purely frontend UI change. No Docker Compose, CI pipeline, or infrastructure changes are required.
The delete endpoint is already exercised in TranscriptionBlockControllerTest. CI will pick up any new tests without configuration changes.
No new environment variables, secrets, or service dependencies are introduced.

Recommendations

Ensure the Playwright E2E test added for this feature runs within the existing <8 minutes E2E budget. A hover + confirm + verify flow is fast; no concern there.
If the undo-toast pattern is chosen, make sure the toast timer is not a setTimeout-based flakiness risk in Playwright tests. Use page.waitForSelector with explicit visibility assertions instead of time-based waits.

Nothing else to flag on the infrastructure side — straightforward frontend change with an existing, tested backend endpoint.

## 🏗️ Tobias Wendt — DevOps ### Observations - This is a purely frontend UI change. No Docker Compose, CI pipeline, or infrastructure changes are required. - The delete endpoint is already exercised in `TranscriptionBlockControllerTest`. CI will pick up any new tests without configuration changes. - No new environment variables, secrets, or service dependencies are introduced. ### Recommendations - Ensure the Playwright E2E test added for this feature runs within the existing `<8 minutes` E2E budget. A hover + confirm + verify flow is fast; no concern there. - If the undo-toast pattern is chosen, make sure the toast timer is not a `setTimeout`-based flakiness risk in Playwright tests. Use `page.waitForSelector` with explicit visibility assertions instead of time-based waits. Nothing else to flag on the infrastructure side — straightforward frontend change with an existing, tested backend endpoint.

marcel commented

2026-04-26 20:00:24 +02:00

📋 Elicit — Requirements Engineer

Observations

Story coverage is solid for the main path. The three acceptance criteria map cleanly to: (1) affordance visibility trigger, (2) delete action with safety net, (3) secondary path preservation.
NFR gap — touch target: The ≥ 44 px touch target is stated but not measurably specified. Where does it apply — on the trash icon button only, or on the annotation region itself? The annotation region is user-drawn and can be arbitrarily small (a 5 × 5 px polygon is valid). Clarify: the 44 px requirement applies to the trash button only, not the annotation region.
NFR gap — "confirmation or undo-toast": This is an OR, which means the acceptance criteria are ambiguous. A modal confirm and an undo-toast are different interactions with different error-recovery semantics. An undo-toast implies a buffered delete (the server call happens after the timer expires or is cancelled client-side), which adds implementation complexity. A modal confirm uses the already-present ConfirmService. The current phrasing leaves this as an open implementation choice that will be revisited in code review — it should be resolved before implementation starts.
Missing edge case — OCR running: The issue doesn't address whether the delete icon appears while ocrRunning is true. The current code passes transcribeMode && !ocrRunning as transcribeMode to DocumentViewer, which controls canDraw. If canDraw drives showDelete, this edge case is already handled. Confirm this is intentional.
Missing edge case — annotation without a linked block: An annotation can exist on the PDF without a corresponding TranscriptionBlock (e.g., a newly drawn region that hasn't been typed into yet). The delete icon in this case deletes the annotation record via the transcription block delete. If no block is linked, the icon must either be hidden or trigger annotation-only deletion. The issue doesn't address this.
i18n: The transcription_block_delete_confirm key already exists in de.json, en.json, and es.json. The trash button's aria-label should use btn_delete (also already present). No new translation keys needed if the confirm-dialog pattern is chosen. An undo-toast would need a new "Rückgängig" key — btn_undo does not appear in de.json.

Recommendations

Close the OR in the acceptance criteria: pick confirm-dialog (reuses existing infrastructure, zero new i18n keys) or undo-toast (lower friction but adds buffering complexity and a new i18n key).
Add a concrete AC for the annotation-without-block edge case: "Given an annotation exists with no linked transcription block, the delete icon does not appear (or deletes the annotation only — decide which)."
Replace the vague "a confirmation or undo-toast" phrasing with the chosen pattern and a measurable reversal window if undo-toast is chosen (e.g., "undo available for 5 seconds").

Open Decisions

Confirm-dialog vs. undo-toast: Modal confirm reuses ConfirmService with zero new infrastructure. Undo-toast is lower friction but requires buffered delete logic and a new i18n key. Which fits the Demo Day timeline?
Annotation without a linked block: Should the trash icon appear for orphaned annotations (no block), and if so, what does it delete?

## 📋 Elicit — Requirements Engineer ### Observations - **Story coverage is solid** for the main path. The three acceptance criteria map cleanly to: (1) affordance visibility trigger, (2) delete action with safety net, (3) secondary path preservation. - **NFR gap — touch target:** The ≥ 44 px touch target is stated but not measurably specified. Where does it apply — on the trash icon button only, or on the annotation region itself? The annotation region is user-drawn and can be arbitrarily small (a 5 × 5 px polygon is valid). Clarify: the 44 px requirement applies to the trash button only, not the annotation region. - **NFR gap — "confirmation or undo-toast":** This is an OR, which means the acceptance criteria are ambiguous. A modal confirm and an undo-toast are different interactions with different error-recovery semantics. An undo-toast implies a buffered delete (the server call happens after the timer expires or is cancelled client-side), which adds implementation complexity. A modal confirm uses the already-present `ConfirmService`. The current phrasing leaves this as an open implementation choice that will be revisited in code review — it should be resolved before implementation starts. - **Missing edge case — OCR running:** The issue doesn't address whether the delete icon appears while `ocrRunning` is true. The current code passes `transcribeMode && !ocrRunning` as `transcribeMode` to `DocumentViewer`, which controls `canDraw`. If `canDraw` drives `showDelete`, this edge case is already handled. Confirm this is intentional. - **Missing edge case — annotation without a linked block:** An annotation can exist on the PDF without a corresponding `TranscriptionBlock` (e.g., a newly drawn region that hasn't been typed into yet). The delete icon in this case deletes the annotation record via the transcription block delete. If no block is linked, the icon must either be hidden or trigger annotation-only deletion. The issue doesn't address this. - **i18n:** The `transcription_block_delete_confirm` key already exists in `de.json`, `en.json`, and `es.json`. The trash button's `aria-label` should use `btn_delete` (also already present). No new translation keys needed if the confirm-dialog pattern is chosen. An undo-toast would need a new "Rückgängig" key — `btn_undo` does not appear in `de.json`. ### Recommendations - Close the OR in the acceptance criteria: pick confirm-dialog (reuses existing infrastructure, zero new i18n keys) or undo-toast (lower friction but adds buffering complexity and a new i18n key). - Add a concrete AC for the annotation-without-block edge case: "Given an annotation exists with no linked transcription block, the delete icon does not appear (or deletes the annotation only — decide which)." - Replace the vague "a confirmation or undo-toast" phrasing with the chosen pattern and a measurable reversal window if undo-toast is chosen (e.g., "undo available for 5 seconds"). ### Open Decisions - **Confirm-dialog vs. undo-toast:** Modal confirm reuses `ConfirmService` with zero new infrastructure. Undo-toast is lower friction but requires buffered delete logic and a new i18n key. Which fits the Demo Day timeline? - **Annotation without a linked block:** Should the trash icon appear for orphaned annotations (no block), and if so, what does it delete?

marcel commented

2026-04-26 20:00:45 +02:00

🗳️ Decision Queue

Three genuine tradeoffs that require a human decision before implementation starts.

1. Confirm-dialog vs. undo-toast

Raised by: Felix, Sara, Elicit

The acceptance criteria say "confirmation or undo-toast" — but these are architecturally different:

	Modal confirm	Undo-toast
Infrastructure	Reuses existing `ConfirmService`	Requires buffered delete (server call delayed)
i18n keys	None new (`transcription_block_delete_confirm` + `btn_delete` exist)	Needs `btn_undo` (not in `de.json`)
Friction	Higher (modal interrupts flow)	Lower (inline, dismissable)
Test complexity	Straightforward	Timer-based undo window needs Playwright care
Demo Day fit	Faster to implement	More polish, more work

Recommendation from the team: Modal confirm — it is already used for the same action in the sidebar (TranscriptionBlock.svelte), costs nothing new, and is consistent. Undo-toast is a worthwhile upgrade but a separate issue.

2. Prop-drilling depth for `onDeleteRequest`

Raised by: Markus

The event must travel: +page.svelte → DocumentViewer → PdfViewer → AnnotationLayer → AnnotationShape. That is four component layers.

Option A — Full prop chain: Each component passes onDeleteRequest down. Preserves strict layering. Four small prop additions.

Option B — AnnotationShape receives block ID directly and calls confirm inline: Reduces the chain but couples annotation rendering to the transcription-block domain and the ConfirmService context.

Recommendation from the team: Option A (full prop chain). It mirrors the existing onAnnotationClick and onTranscriptionDraw patterns already in place. The chain is mechanical, not complex.

3. Orphaned annotation (no linked block) — show or hide the trash icon?

Raised by: Elicit

A user can draw an annotation region on the PDF but leave the text field empty (or the block may not yet be saved). If the trash icon appears on these orphaned annotations, clicking it must either:

A) Be hidden entirely (icon only shows when blockNumbers[annotation.id] is defined).
B) Shown and deletes only the annotation record (not a transcription block).
C) Shown and deletes the transcription block record (which also deletes the annotation via cascade).

Recommendation from the team: Option A — hide the trash icon for annotations without a block number. The block number badge is already suppressed in this case ({#if !dimmed && blockNumber}). Apply the same condition to the delete icon. This keeps the feature scope tight for Demo Day.

## 🗳️ Decision Queue Three genuine tradeoffs that require a human decision before implementation starts. --- ### 1. Confirm-dialog vs. undo-toast **Raised by:** Felix, Sara, Elicit The acceptance criteria say "confirmation or undo-toast" — but these are architecturally different: | | Modal confirm | Undo-toast | |---|---|---| | Infrastructure | Reuses existing `ConfirmService` | Requires buffered delete (server call delayed) | | i18n keys | None new (`transcription_block_delete_confirm` + `btn_delete` exist) | Needs `btn_undo` (not in `de.json`) | | Friction | Higher (modal interrupts flow) | Lower (inline, dismissable) | | Test complexity | Straightforward | Timer-based undo window needs Playwright care | | Demo Day fit | Faster to implement | More polish, more work | **Recommendation from the team:** Modal confirm — it is already used for the same action in the sidebar (`TranscriptionBlock.svelte`), costs nothing new, and is consistent. Undo-toast is a worthwhile upgrade but a separate issue. --- ### 2. Prop-drilling depth for `onDeleteRequest` **Raised by:** Markus The event must travel: `+page.svelte` → `DocumentViewer` → `PdfViewer` → `AnnotationLayer` → `AnnotationShape`. That is four component layers. **Option A — Full prop chain:** Each component passes `onDeleteRequest` down. Preserves strict layering. Four small prop additions. **Option B — `AnnotationShape` receives block ID directly and calls confirm inline:** Reduces the chain but couples annotation rendering to the transcription-block domain and the `ConfirmService` context. **Recommendation from the team:** Option A (full prop chain). It mirrors the existing `onAnnotationClick` and `onTranscriptionDraw` patterns already in place. The chain is mechanical, not complex. --- ### 3. Orphaned annotation (no linked block) — show or hide the trash icon? **Raised by:** Elicit A user can draw an annotation region on the PDF but leave the text field empty (or the block may not yet be saved). If the trash icon appears on these orphaned annotations, clicking it must either: - **A)** Be hidden entirely (icon only shows when `blockNumbers[annotation.id]` is defined). - **B)** Shown and deletes only the annotation record (not a transcription block). - **C)** Shown and deletes the transcription block record (which also deletes the annotation via cascade). **Recommendation from the team:** Option A — hide the trash icon for annotations without a block number. The block number badge is already suppressed in this case (`{#if !dimmed && blockNumber}`). Apply the same condition to the delete icon. This keeps the feature scope tight for Demo Day.

marcel commented

2026-04-26 20:16:36 +02:00

Modal confirm, we currently have it.
A, prop chain
C, an annotation should always be deletable event if it does not have a transcription block

1. Modal confirm, we currently have it. 2. A, prop chain 3. C, an annotation should always be deletable event if it does not have a transcription block

marcel referenced this issue from a commit

2026-04-26 21:01:28 +02:00

feat(viewer): show delete icon on annotation for direct block deletion (#339)

marcel commented

2026-04-26 21:01:39 +02:00

Implemented on branch feat/issue-339-delete-icon-on-annotation — commit b13c1093.

Changes (8 files):

AnnotationShape.svelte — trash button (44×44 px, top-right corner, teal on hover) with showDelete/onDeleteRequest props; Delete key support; button hidden when not hovered/active
AnnotationLayer.svelte — passes showDelete={canDraw} and onDeleteRequest down to each shape
PdfViewer.svelte — accepts and forwards onDeleteAnnotationRequest prop
DocumentViewer.svelte — accepts and forwards onDeleteAnnotationRequest prop
+page.svelte — handleAnnotationDeleteRequest(): confirm dialog → delete block (if linked) or DELETE /annotations/{id} directly (orphaned annotation fallback) → reload annotations
AnnotationShape.svelte.spec.ts (new) — 8 browser-mode unit tests covering all visibility/interaction cases
AnnotationLayer.svelte.spec.ts — updated tests to match new showDelete prop behaviour
TranscriptionBlockControllerTest.java — added security regression: deleteBlock_returns403_whenUserHasOnlyReadAllPermission

All tests green: 15/15 frontend (AnnotationShape + AnnotationLayer), 31/31 backend (TranscriptionBlockControllerTest).

Implemented on branch `feat/issue-339-delete-icon-on-annotation` — commit b13c1093. **Changes (8 files):** - `AnnotationShape.svelte` — trash button (44×44 px, top-right corner, teal on hover) with `showDelete`/`onDeleteRequest` props; Delete key support; button hidden when not hovered/active - `AnnotationLayer.svelte` — passes `showDelete={canDraw}` and `onDeleteRequest` down to each shape - `PdfViewer.svelte` — accepts and forwards `onDeleteAnnotationRequest` prop - `DocumentViewer.svelte` — accepts and forwards `onDeleteAnnotationRequest` prop - `+page.svelte` — `handleAnnotationDeleteRequest()`: confirm dialog → delete block (if linked) or DELETE `/annotations/{id}` directly (orphaned annotation fallback) → reload annotations - `AnnotationShape.svelte.spec.ts` (new) — 8 browser-mode unit tests covering all visibility/interaction cases - `AnnotationLayer.svelte.spec.ts` — updated tests to match new `showDelete` prop behaviour - `TranscriptionBlockControllerTest.java` — added security regression: `deleteBlock_returns403_whenUserHasOnlyReadAllPermission` All tests green: 15/15 frontend (AnnotationShape + AnnotationLayer), 31/31 backend (TranscriptionBlockControllerTest).

marcel referenced a pull request that will close this issue

2026-04-26 21:07:26 +02:00

feat(viewer): show delete icon directly on transcription annotation (#339) #348

marcel closed this issue

2026-04-26 21:56:38 +02:00

marcel referenced this issue from a commit

2026-04-26 21:56:40 +02:00

feat(viewer): show delete icon on annotation for direct block deletion (#339)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#339

feat(viewer): show delete icon directly on transcription annotation #339

User story

Context

Acceptance criteria

NFRs

👨‍💼 Markus Keller — Application Architect

Observations

Recommendations

Open Decisions

👩‍💻 Felix Brandt — Fullstack Developer

Observations

Recommendations

Open Decisions (omit if none)

🛡️ Nora "NullX" Steiner — Security Expert

Observations

Recommendations

🎨 Leonie Voss — UI/UX Expert

Observations

Recommendations

Open Decisions

🧪 Sara Holt — QA Engineer

Observations

Recommendations

Open Decisions

🏗️ Tobias Wendt — DevOps

Observations

Recommendations

📋 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions

🗳️ Decision Queue

1. Confirm-dialog vs. undo-toast

2. Prop-drilling depth for onDeleteRequest

3. Orphaned annotation (no linked block) — show or hide the trash icon?

2. Prop-drilling depth for `onDeleteRequest`