From ad820955fda89f964a1f39f16b5aacfe6e6fec11 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 3 Jun 2026 22:38:37 +0200 Subject: [PATCH] fix(transcription): remove annotation canvas delete button that obscured text (#722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-annotation delete button (a 44px circular control pinned to the box's top-right) overlapped the box below and obscured the underlying document text. It was redundant: every user-drawn annotation has a transcription block, and the right-hand panel already offers a non-overlapping delete per block that cascades to the annotation. Remove the visible button and its `deleteVisible` derived. Keep the keyboard Delete shortcut (and its `showDelete`/`onDeleteRequest`/ `deleteAnnotation` wiring) — it obscures nothing and remains a power-user path and the only cleanup route for orphan annotations. Tests: replace the button-render/click specs with contract tests asserting no delete button ever renders; repoint the e2e delete flow to the keyboard shortcut + confirm dialog. Co-Authored-By: Claude Opus 4.8 --- frontend/e2e/documents.spec.ts | 12 ++- .../annotation/AnnotationLayer.svelte.spec.ts | 16 +--- .../annotation/AnnotationShape.svelte | 47 --------- .../annotation/AnnotationShape.svelte.spec.ts | 96 ++----------------- 4 files changed, 18 insertions(+), 153 deletions(-) diff --git a/frontend/e2e/documents.spec.ts b/frontend/e2e/documents.spec.ts index 242d3515..790d539c 100644 --- a/frontend/e2e/documents.spec.ts +++ b/frontend/e2e/documents.spec.ts @@ -383,12 +383,16 @@ test.describe('PDF annotations — admin', () => { // Record count now — the draw test may have created more than one annotation const countBefore = await page.locator('[data-testid^="annotation-"]').count(); - // Enable annotate mode to show delete buttons + // Enable annotate mode — deletion is only available while annotating await page.getByRole('button', { name: /^annotieren$/i }).click(); - const deleteBtn = page.getByRole('button', { name: /annotation löschen/i }).first(); - await expect(deleteBtn).toBeVisible({ timeout: 8000 }); - await deleteBtn.click(); + // The on-canvas delete button was removed (issue #722). Delete via the + // kept keyboard shortcut: focus an annotation, press Delete, confirm. + const annotation = page.locator('[data-testid^="annotation-"]').first(); + await annotation.click(); + await annotation.press('Delete'); + + await page.getByRole('button', { name: /^bestätigen$/i }).click(); await expect(page.locator('[data-testid^="annotation-"]')).toHaveCount(countBefore - 1, { timeout: 8000 diff --git a/frontend/src/lib/document/annotation/AnnotationLayer.svelte.spec.ts b/frontend/src/lib/document/annotation/AnnotationLayer.svelte.spec.ts index 3c5aea0b..12adfd7e 100644 --- a/frontend/src/lib/document/annotation/AnnotationLayer.svelte.spec.ts +++ b/frontend/src/lib/document/annotation/AnnotationLayer.svelte.spec.ts @@ -98,23 +98,13 @@ describe('AnnotationLayer', () => { expect(el2.style.opacity).toBe('1'); }); - it('does not show delete button when annotation is not hovered or active', async () => { + // The on-canvas delete button was removed (issue #722). Even in annotate mode + // with an active annotation, no delete button must render over the document. + it('never renders a delete button, even in annotate mode with an active annotation', async () => { render(AnnotationLayer, { annotations: [makeAnnotation('ann-1')], canDraw: true, color: '#00C7B1', - onDraw: () => {} - }); - - await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); - await expect.element(page.getByTestId('annotation-delete-ann-1')).not.toBeInTheDocument(); - }); - - 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: () => {} }); diff --git a/frontend/src/lib/document/annotation/AnnotationShape.svelte b/frontend/src/lib/document/annotation/AnnotationShape.svelte index 8bfc7c7b..c0cb985f 100644 --- a/frontend/src/lib/document/annotation/AnnotationShape.svelte +++ b/frontend/src/lib/document/annotation/AnnotationShape.svelte @@ -32,8 +32,6 @@ let { 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); @@ -119,51 +117,6 @@ let shapeStyle = $derived( {blockNumber} {/if} - {#if deleteVisible} - - {/if} {#if isResizable} {/if} diff --git a/frontend/src/lib/document/annotation/AnnotationShape.svelte.spec.ts b/frontend/src/lib/document/annotation/AnnotationShape.svelte.spec.ts index a896489b..d9ec0ffc 100644 --- a/frontend/src/lib/document/annotation/AnnotationShape.svelte.spec.ts +++ b/frontend/src/lib/document/annotation/AnnotationShape.svelte.spec.ts @@ -33,55 +33,14 @@ describe('AnnotationShape', () => { await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); }); - it('does not show delete button when showDelete is false', async () => { + // The on-canvas delete button was removed (issue #722) because it overlapped + // the document text. Deletion now happens via the transcription panel or the + // keyboard Delete shortcut. No visible delete button must ever render — even + // when hovered and active in delete mode. + it('never renders a delete button, even when hovered and active in delete mode', async () => { render(AnnotationShape, { annotation: makeAnnotation(), isHovered: true, - isActive: false, - showDelete: false, - onDeleteRequest: vi.fn(), - onclick: () => {}, - onpointerenter: () => {}, - onpointerleave: () => {} - }); - - await expect.element(page.getByTestId('annotation-delete-ann-1')).not.toBeInTheDocument(); - }); - - 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: () => {} - }); - - await expect.element(page.getByTestId('annotation-delete-ann-1')).not.toBeInTheDocument(); - }); - - 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(), @@ -90,49 +49,8 @@ describe('AnnotationShape', () => { 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('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(); + await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument(); + await expect.element(page.getByTestId('annotation-delete-ann-1')).not.toBeInTheDocument(); }); it('calls onDeleteRequest when Delete key is pressed on the annotation', async () => {