From b13c10936bbbb444eb855e9f9733a2c83706e116 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 21:00:50 +0200 Subject: [PATCH 1/5] feat(viewer): show delete icon on annotation for direct block deletion (#339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a trash icon button (44×44 px touch target) directly on each annotation shape in transcription mode so users can delete a block without navigating through the sidebar. Includes keyboard support (Delete key), confirm dialog via ConfirmService, prop-chain wiring through DocumentViewer → PdfViewer → AnnotationLayer → AnnotationShape, and orphaned-annotation fallback (calls DELETE /annotations/{id} when no block is linked). Backend security regression test added for deleteBlock 403 on READ_ALL. Co-Authored-By: Claude Sonnet 4.6 --- .../TranscriptionBlockControllerTest.java | 7 + .../src/lib/components/AnnotationLayer.svelte | 6 +- .../components/AnnotationLayer.svelte.spec.ts | 17 +- .../src/lib/components/AnnotationShape.svelte | 52 ++++++ .../components/AnnotationShape.svelte.spec.ts | 155 ++++++++++++++++++ .../src/lib/components/DocumentViewer.svelte | 5 +- frontend/src/lib/components/PdfViewer.svelte | 3 + .../src/routes/documents/[id]/+page.svelte | 21 +++ 8 files changed, 262 insertions(+), 4 deletions(-) create mode 100644 frontend/src/lib/components/AnnotationShape.svelte.spec.ts diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java index 2231e9fb..31073057 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java @@ -260,6 +260,13 @@ class TranscriptionBlockControllerTest { .andExpect(status().isForbidden()); } + @Test + @WithMockUser(authorities = "READ_ALL") + void deleteBlock_returns403_whenUserHasOnlyReadAllPermission() throws Exception { + mockMvc.perform(delete(URL_BLOCK)) + .andExpect(status().isForbidden()); + } + @Test @WithMockUser(authorities = "WRITE_ALL") void deleteBlock_returns204_whenAuthorised() throws Exception { diff --git a/frontend/src/lib/components/AnnotationLayer.svelte b/frontend/src/lib/components/AnnotationLayer.svelte index ac7dbc7c..129fef36 100644 --- a/frontend/src/lib/components/AnnotationLayer.svelte +++ b/frontend/src/lib/components/AnnotationLayer.svelte @@ -18,7 +18,8 @@ let { dimmed = false, flashAnnotationId = null, onDraw, - onAnnotationClick + onAnnotationClick, + onDeleteRequest }: { annotations: Annotation[]; canDraw: boolean; @@ -29,6 +30,7 @@ let { flashAnnotationId?: string | null; onDraw: (rect: DrawRect) => void; onAnnotationClick?: (id: string) => void; + onDeleteRequest?: (annotationId: string) => void; } = $props(); let drawStart = $state<{ x: number; y: number } | null>(null); @@ -112,6 +114,8 @@ const containerStyle = $derived( dimmed={dimmed} blockNumber={blockNumbers[annotation.id]} isFlashing={flashAnnotationId === annotation.id} + showDelete={canDraw} + onDeleteRequest={() => onDeleteRequest?.(annotation.id)} onclick={() => onAnnotationClick?.(annotation.id)} onpointerenter={() => (hoveredId = annotation.id)} onpointerleave={() => (hoveredId = null)} diff --git a/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts b/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts index 7a4b4e07..b134c50f 100644 --- a/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts +++ b/frontend/src/lib/components/AnnotationLayer.svelte.spec.ts @@ -98,7 +98,7 @@ describe('AnnotationLayer', () => { expect(el2.style.opacity).toBe('1'); }); - it('does not show delete buttons (annotations owned by blocks)', async () => { + it('does not show delete button when annotation is not hovered or active', async () => { render(AnnotationLayer, { annotations: [makeAnnotation('ann-1')], canDraw: true, @@ -107,6 +107,19 @@ describe('AnnotationLayer', () => { }); await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); - expect(page.getByRole('button', { name: /löschen/i }).query()).toBeNull(); + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); + }); + + it('does not show delete button when canDraw is false even if annotation is active', async () => { + render(AnnotationLayer, { + annotations: [makeAnnotation('ann-1')], + canDraw: false, + color: '#00C7B1', + activeAnnotationId: 'ann-1', + onDraw: () => {} + }); + + await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); }); }); diff --git a/frontend/src/lib/components/AnnotationShape.svelte b/frontend/src/lib/components/AnnotationShape.svelte index a7c667f2..2dbc1f3d 100644 --- a/frontend/src/lib/components/AnnotationShape.svelte +++ b/frontend/src/lib/components/AnnotationShape.svelte @@ -11,6 +11,8 @@ let { blockNumber = undefined, isFlashing = false, isResizable = false, + showDelete = false, + onDeleteRequest, onclick, onpointerenter, onpointerleave @@ -23,11 +25,15 @@ let { blockNumber?: number | undefined; isFlashing?: boolean; isResizable?: boolean; + showDelete?: boolean; + onDeleteRequest?: () => void; onclick: () => void; onpointerenter: () => void; onpointerleave: () => void; } = $props(); +const deleteVisible = $derived(showDelete && (isHovered || isActive)); + function hexToRgba(hex: string, alpha: number): string { const r = parseInt(hex.slice(1, 3), 16); const g = parseInt(hex.slice(3, 5), 16); @@ -83,6 +89,7 @@ let shapeStyle = $derived( onclick={onclick} onkeydown={(e) => { if (e.key === 'Enter' || e.key === ' ') onclick(); + if (e.key === 'Delete' && showDelete) onDeleteRequest?.(); }} onpointerenter={onpointerenter} onpointerleave={onpointerleave} @@ -112,6 +119,51 @@ let shapeStyle = $derived( {blockNumber} {/if} + {#if deleteVisible} + + {/if} {#if isResizable} {/if} diff --git a/frontend/src/lib/components/AnnotationShape.svelte.spec.ts b/frontend/src/lib/components/AnnotationShape.svelte.spec.ts new file mode 100644 index 00000000..f48be08e --- /dev/null +++ b/frontend/src/lib/components/AnnotationShape.svelte.spec.ts @@ -0,0 +1,155 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; +import AnnotationShape from './AnnotationShape.svelte'; + +afterEach(cleanup); + +function makeAnnotation(id = 'ann-1') { + return { + id, + documentId: 'doc-1', + pageNumber: 1, + x: 0.1, + y: 0.1, + width: 0.3, + height: 0.2, + color: '#00C7B1', + createdAt: new Date().toISOString() + }; +} + +describe('AnnotationShape', () => { + it('renders the annotation element', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: false, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); + }); + + it('does not show delete button when showDelete is false', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: false, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); + }); + + it('does not show delete button when showDelete is true but neither hovered nor active', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: false, + showDelete: true, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + expect(page.getByTestId('annotation-delete-ann-1').query()).toBeNull(); + }); + + it('shows delete button when showDelete is true and isHovered is true', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: true, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + await expect.element(page.getByTestId('annotation-delete-ann-1')).toBeInTheDocument(); + }); + + it('shows delete button when showDelete is true and isActive is true', async () => { + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: true, + showDelete: true, + onDeleteRequest: vi.fn(), + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + await expect.element(page.getByTestId('annotation-delete-ann-1')).toBeInTheDocument(); + }); + + it('calls onDeleteRequest when delete button is clicked', async () => { + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: true, + onDeleteRequest, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const deleteBtn = page.getByTestId('annotation-delete-ann-1'); + await deleteBtn.click(); + + expect(onDeleteRequest).toHaveBeenCalledOnce(); + }); + + it('calls onDeleteRequest when Delete key is pressed on the annotation', async () => { + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: true, + showDelete: true, + onDeleteRequest, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement; + annotationEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Delete', bubbles: true })); + + expect(onDeleteRequest).toHaveBeenCalledOnce(); + }); + + it('does not call onDeleteRequest on Delete key when showDelete is false', async () => { + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: false, + isActive: true, + showDelete: false, + onDeleteRequest, + onclick: () => {}, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement; + annotationEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Delete', bubbles: true })); + + expect(onDeleteRequest).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/components/DocumentViewer.svelte b/frontend/src/lib/components/DocumentViewer.svelte index c48d1f2a..b22f3d6b 100644 --- a/frontend/src/lib/components/DocumentViewer.svelte +++ b/frontend/src/lib/components/DocumentViewer.svelte @@ -24,6 +24,7 @@ type Props = { flashAnnotationId?: string | null; onAnnotationClick: (id: string) => void; onTranscriptionDraw?: (rect: DrawRect) => void; + onDeleteAnnotationRequest?: (annotationId: string) => void; }; let { @@ -38,7 +39,8 @@ let { annotationsDimmed = false, flashAnnotationId = null, onAnnotationClick, - onTranscriptionDraw + onTranscriptionDraw, + onDeleteAnnotationRequest }: Props = $props(); @@ -98,6 +100,7 @@ let { flashAnnotationId={flashAnnotationId} onAnnotationClick={onAnnotationClick} onTranscriptionDraw={onTranscriptionDraw} + onDeleteAnnotationRequest={onDeleteAnnotationRequest} documentFileHash={doc.fileHash ?? null} /> {:else if fileUrl} diff --git a/frontend/src/lib/components/PdfViewer.svelte b/frontend/src/lib/components/PdfViewer.svelte index 844aa338..7642b197 100644 --- a/frontend/src/lib/components/PdfViewer.svelte +++ b/frontend/src/lib/components/PdfViewer.svelte @@ -18,6 +18,7 @@ let { activeAnnotationId = $bindable(null), onAnnotationClick, onTranscriptionDraw, + onDeleteAnnotationRequest, documentFileHash, annotationsDimmed = false, flashAnnotationId = null @@ -30,6 +31,7 @@ let { activeAnnotationId?: string | null; onAnnotationClick?: (id: string) => void; onTranscriptionDraw?: (rect: DrawRect) => void; + onDeleteAnnotationRequest?: (annotationId: string) => void; documentFileHash?: string | null; annotationsDimmed?: boolean; flashAnnotationId?: string | null; @@ -264,6 +266,7 @@ function handleAnnotationClick(id: string) { flashAnnotationId={flashAnnotationId} onDraw={handleDraw} onAnnotationClick={handleAnnotationClick} + onDeleteRequest={onDeleteAnnotationRequest} /> {/if} diff --git a/frontend/src/routes/documents/[id]/+page.svelte b/frontend/src/routes/documents/[id]/+page.svelte index 06a38484..cbf079b2 100644 --- a/frontend/src/routes/documents/[id]/+page.svelte +++ b/frontend/src/routes/documents/[id]/+page.svelte @@ -13,9 +13,12 @@ import { getErrorMessage } from '$lib/errors'; import { translateOcrProgress } from '$lib/ocr/translateOcrProgress'; import { createFileLoader } from '$lib/hooks/useFileLoader.svelte'; import { scrollToCommentFromQuery } from '$lib/utils/deepLinkScroll'; +import { getConfirmService } from '$lib/services/confirm.svelte.js'; let { data } = $props(); +const { confirm } = getConfirmService(); + const doc = $derived(data.document); const canWrite = $derived(data.canWrite ?? false); const currentUserId = $derived((data.user?.id as string | undefined) ?? null); @@ -105,6 +108,23 @@ async function deleteBlock(blockId: string) { annotationReloadKey++; } +async function handleAnnotationDeleteRequest(annotationId: string) { + const confirmed = await confirm({ + title: m.transcription_block_delete_confirm(), + destructive: true + }); + if (!confirmed) return; + + const block = transcriptionBlocks.find((b) => b.annotationId === annotationId); + if (block) { + await deleteBlock(block.id); + } else { + // Annotation has no linked block — delete the annotation directly + await fetch(`/api/documents/${doc.id}/annotations/${annotationId}`, { method: 'DELETE' }); + annotationReloadKey++; + } +} + async function reviewToggle(blockId: string) { const res = await fetch(`/api/documents/${doc.id}/transcription-blocks/${blockId}/review`, { method: 'PUT' @@ -381,6 +401,7 @@ onMount(() => { bind:activeAnnotationId={activeAnnotationId} onAnnotationClick={handleAnnotationClick} onTranscriptionDraw={createBlockFromDraw} + onDeleteAnnotationRequest={handleAnnotationDeleteRequest} /> -- 2.49.1 From f22596a29de581014d50b766f442a2f7f2fe4134 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 21:36:27 +0200 Subject: [PATCH 2/5] fix(viewer): check res.ok on orphaned annotation DELETE to surface errors Without the guard, a failed DELETE (4xx/5xx) was silently swallowed and annotationReloadKey was incremented anyway, leaving the annotation visible and the user with no feedback. Now matches the deleteBlock() pattern immediately above. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/routes/documents/[id]/+page.svelte | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/routes/documents/[id]/+page.svelte b/frontend/src/routes/documents/[id]/+page.svelte index cbf079b2..2cac8cf8 100644 --- a/frontend/src/routes/documents/[id]/+page.svelte +++ b/frontend/src/routes/documents/[id]/+page.svelte @@ -120,7 +120,10 @@ async function handleAnnotationDeleteRequest(annotationId: string) { await deleteBlock(block.id); } else { // Annotation has no linked block — delete the annotation directly - await fetch(`/api/documents/${doc.id}/annotations/${annotationId}`, { method: 'DELETE' }); + const res = await fetch(`/api/documents/${doc.id}/annotations/${annotationId}`, { + method: 'DELETE' + }); + if (!res.ok) throw new Error('Delete annotation failed'); annotationReloadKey++; } } -- 2.49.1 From e95a9312e8fd700bcd54c113ca14f293b3b4a34f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 21:36:51 +0200 Subject: [PATCH 3/5] test(viewer): verify delete button click does not bubble to onclick Documents the stopPropagation guarantee: clicking the trash button must not trigger the annotation's onclick (which opens the block detail panel) while the delete confirm is in progress. Co-Authored-By: Claude Sonnet 4.6 --- .../components/AnnotationShape.svelte.spec.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/frontend/src/lib/components/AnnotationShape.svelte.spec.ts b/frontend/src/lib/components/AnnotationShape.svelte.spec.ts index f48be08e..87f1cae1 100644 --- a/frontend/src/lib/components/AnnotationShape.svelte.spec.ts +++ b/frontend/src/lib/components/AnnotationShape.svelte.spec.ts @@ -113,6 +113,28 @@ describe('AnnotationShape', () => { expect(onDeleteRequest).toHaveBeenCalledOnce(); }); + it('does not call onclick when delete button is clicked', async () => { + const onclick = vi.fn(); + const onDeleteRequest = vi.fn(); + + render(AnnotationShape, { + annotation: makeAnnotation(), + isHovered: true, + isActive: false, + showDelete: true, + onDeleteRequest, + onclick, + onpointerenter: () => {}, + onpointerleave: () => {} + }); + + const deleteBtn = page.getByTestId('annotation-delete-ann-1'); + await deleteBtn.click(); + + expect(onclick).not.toHaveBeenCalled(); + expect(onDeleteRequest).toHaveBeenCalledOnce(); + }); + it('calls onDeleteRequest when Delete key is pressed on the annotation', async () => { const onDeleteRequest = vi.fn(); -- 2.49.1 From 007ec65dbdc68d6c28868dfdd8926c101a1f1991 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 21:37:17 +0200 Subject: [PATCH 4/5] fix(viewer): move delete button inside annotation bounds to prevent edge clipping Repositioning from top:-8px/right:-8px to top:4px/right:4px ensures the 44px touch target stays fully within the annotation shape. Annotations drawn near the top or right edge of the PDF page no longer risk the button being obscured or inaccessible. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/lib/components/AnnotationShape.svelte | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/components/AnnotationShape.svelte b/frontend/src/lib/components/AnnotationShape.svelte index 2dbc1f3d..31be34f4 100644 --- a/frontend/src/lib/components/AnnotationShape.svelte +++ b/frontend/src/lib/components/AnnotationShape.svelte @@ -130,8 +130,8 @@ let shapeStyle = $derived( }} style=" position: absolute; - top: -8px; - right: -8px; + top: 4px; + right: 4px; min-width: 44px; min-height: 44px; display: flex; -- 2.49.1 From 251b5503a241040c870e48030a19c09f7efc2b25 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 26 Apr 2026 21:37:41 +0200 Subject: [PATCH 5/5] test(security): add 403 permission test for annotation DELETE endpoint Confirms that DELETE /api/documents/{id}/annotations/{id} requires at least ANNOTATE_ALL; a user with only READ_ALL receives 403 Forbidden. Closes the permission audit raised during PR review. Co-Authored-By: Claude Sonnet 4.6 --- .../controller/AnnotationControllerTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java index 7a4546b6..368a21a0 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/AnnotationControllerTest.java @@ -154,6 +154,13 @@ class AnnotationControllerTest { .andExpect(status().isForbidden()); } + @Test + @WithMockUser(authorities = "READ_ALL") + void deleteAnnotation_returns403_whenUserHasOnlyReadAllPermission() throws Exception { + mockMvc.perform(delete("/api/documents/" + UUID.randomUUID() + "/annotations/" + UUID.randomUUID())) + .andExpect(status().isForbidden()); + } + @Test @WithMockUser(authorities = "ANNOTATE_ALL") void deleteAnnotation_returns204_whenHasAnnotatePermission() throws Exception { -- 2.49.1