fix(transcription): remove annotation canvas delete button that obscured text (#722)
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m21s
CI / OCR Service Tests (pull_request) Successful in 25s
CI / Backend Unit Tests (pull_request) Successful in 3m45s
CI / fail2ban Regex (pull_request) Successful in 47s
CI / Semgrep Security Scan (pull_request) Successful in 24s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m6s
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m21s
CI / OCR Service Tests (pull_request) Successful in 25s
CI / Backend Unit Tests (pull_request) Successful in 3m45s
CI / fail2ban Regex (pull_request) Successful in 47s
CI / Semgrep Security Scan (pull_request) Successful in 24s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m6s
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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: () => {}
|
||||
});
|
||||
|
||||
@@ -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}
|
||||
</div>
|
||||
{/if}
|
||||
{#if deleteVisible}
|
||||
<button
|
||||
data-testid="annotation-delete-{annotation.id}"
|
||||
type="button"
|
||||
aria-label="Löschen"
|
||||
onclick={(e) => {
|
||||
e.stopPropagation();
|
||||
onDeleteRequest?.();
|
||||
}}
|
||||
style="
|
||||
position: absolute;
|
||||
top: 4px;
|
||||
right: 4px;
|
||||
min-width: 44px;
|
||||
min-height: 44px;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
border-radius: 50%;
|
||||
background-color: #fff;
|
||||
border: 1px solid var(--color-error, #e53e3e);
|
||||
color: var(--color-error, #e53e3e);
|
||||
cursor: pointer;
|
||||
pointer-events: auto;
|
||||
box-shadow: 0 1px 4px rgba(0,0,0,0.2);
|
||||
z-index: 10;
|
||||
"
|
||||
>
|
||||
<svg
|
||||
width="16"
|
||||
height="16"
|
||||
viewBox="0 0 24 24"
|
||||
fill="none"
|
||||
stroke="currentColor"
|
||||
stroke-width="1.5"
|
||||
aria-hidden="true"
|
||||
>
|
||||
<path
|
||||
stroke-linecap="round"
|
||||
stroke-linejoin="round"
|
||||
d="M19 7l-.867 12.142A2 2 0 0116.138 21H7.862a2 2 0 01-1.995-1.858L5 7m5 4v6m4-6v6m1-10V4a1 1 0 00-1-1h-4a1 1 0 00-1 1v3M4 7h16"
|
||||
/>
|
||||
</svg>
|
||||
</button>
|
||||
{/if}
|
||||
{#if isResizable}
|
||||
<AnnotationEditOverlay annotation={annotation} />
|
||||
{/if}
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in New Issue
Block a user