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

fix(transcription): remove annotation canvas delete button that obscured text (#722) #723

Merged
marcel merged 4 commits from fix/issue-722-remove-annotation-canvas-delete-button into main 2026-06-04 12:28:18 +02:00
Conversation 8 Commits 4 Files Changed 7 +69 -156
7 changed files with 69 additions and 156 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
frontend/e2e/documents.spec.ts
View File

@@ -382,17 +382,32 @@ 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();
// Guard against a missing seed: without this, a count of 0 would turn the
// post-delete assertion into toHaveCount(-1) and fail with a misleading timeout.
expect(countBefore).toBeGreaterThan(0);
// 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();
// Capture the identity of the specific annotation we delete so we can assert
// that exact element is gone afterwards — a count drop alone is identity-blind.
const deletedTestId = await annotation.getAttribute('data-testid');
expect(deletedTestId).toBeTruthy();
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
});
// Identity check: the specific annotation we deleted must no longer exist.
await expect(page.locator(`[data-testid="${deletedTestId}"]`)).toHaveCount(0, {
timeout: 8000
});
await page.screenshot({ path: 'test-results/e2e/annotation-deleted.png' });
});

20
frontend/src/lib/document/annotation/AnnotationLayer.svelte.spec.ts
View File

@@ -98,28 +98,22 @@ 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: () => {}
});
await expect.element(page.getByTestId('annotation-ann-1')).toBeInTheDocument();
// Positive control: the previously-removed testid must stay absent.
await expect.element(page.getByTestId('annotation-delete-ann-1')).not.toBeInTheDocument();
// Real invariant: the annotation must contain no clickable delete control at all.
const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement;
expect(annotationEl.querySelectorAll('button').length).toBe(0);
});
});

2
frontend/src/lib/document/annotation/AnnotationLayer.svelte.test.ts
View File

@@ -240,7 +240,7 @@ describe('AnnotationLayer', () => {
expect(document.querySelector('[data-testid="annotation-ann-1"]')).not.toBeNull();
});
it('renders without throwing when canDraw is true (delete button visible)', async () => {
it('renders without throwing when canDraw is true (no delete button)', async () => {
expect(() =>
render(AnnotationLayer, {
annotations: [annotation],

47
frontend/src/lib/document/annotation/AnnotationShape.svelte
View File

@@ -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}

100
frontend/src/lib/document/annotation/AnnotationShape.svelte.spec.ts
View File

@@ -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,12 @@ 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();
// Positive control: the previously-removed testid must stay absent.
await expect.element(page.getByTestId('annotation-delete-ann-1')).not.toBeInTheDocument();
// Real invariant: the annotation must contain no clickable delete control at all.
const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement;
expect(annotationEl.querySelectorAll('button').length).toBe(0);
});
it('calls onDeleteRequest when Delete key is pressed on the annotation', async () => {

4
frontend/src/lib/document/transcription/TranscriptionBlock.svelte
View File

@@ -231,7 +231,7 @@ async function handleDelete() {
<!-- Review toggle -->
<button
type="button"
class="cursor-pointer transition-colors {reviewed ? 'text-turquoise hover:text-turquoise/70' : 'text-ink-3 hover:text-turquoise'}"
class="-my-2 inline-flex min-h-[44px] min-w-[44px] cursor-pointer items-center justify-center transition-colors {reviewed ? 'text-turquoise hover:text-turquoise/70' : 'text-ink-3 hover:text-turquoise'}"
aria-label={reviewed ? m.transcription_block_unreview() : m.transcription_block_review()}
title={reviewed ? m.transcription_block_unreview() : m.transcription_block_review()}
onclick={onReviewToggle}
@@ -254,7 +254,7 @@ async function handleDelete() {
<!-- Delete button -->
<button
type="button"
class="hover:text-error cursor-pointer text-ink-3 transition-colors"
class="hover:text-error -my-2 -mr-2 inline-flex min-h-[44px] min-w-[44px] cursor-pointer items-center justify-center text-ink-3 transition-colors"
aria-label={m.btn_delete()}
onclick={handleDelete}
>

29
frontend/src/lib/document/transcription/TranscriptionBlock.svelte.spec.ts
View File

@@ -170,6 +170,35 @@ describe('TranscriptionBlock — reorder controls', () => {
});
});
// ─── Touch targets (WCAG 2.2 §2.5.8) ──────────────────────────────────────────
// After #722 removed the on-canvas delete button, the panel footer's icon-only
// actions are the only touch-reachable controls for those operations. They must
// meet the 44×44px minimum target size in both dimensions.
//
// We assert the sizing utility classes rather than measuring getBoundingClientRect:
// the component-test browser env (src/test-setup.ts) loads no Tailwind stylesheet,
// so class-based layout has no effect there and a rendered element collapses to its
// icon (16px). The `min-h/min-w-[44px]` classes are the exact mechanism that yields
// the WCAG target size in the real app; the compiled px size is covered by e2e.
describe('TranscriptionBlock — footer action touch targets', () => {
it('delete button carries the 44px minimum-target-size classes', async () => {
renderBlock();
const btn = (await page.getByRole('button', { name: 'Löschen' }).element()) as HTMLElement;
expect(btn.className).toContain('min-h-[44px]');
expect(btn.className).toContain('min-w-[44px]');
});
it('review toggle button carries the 44px minimum-target-size classes', async () => {
renderBlock({ reviewed: false });
const btn = (await page
.getByRole('button', { name: 'Als geprüft markieren' })
.element()) as HTMLElement;
expect(btn.className).toContain('min-h-[44px]');
expect(btn.className).toContain('min-w-[44px]');
});
});
// ─── Delete confirmation ──────────────────────────────────────────────────────
function renderBlockWithService(overrides: Record<string, unknown> = {}) {