Compare commits

...

5 Commits

Author SHA1 Message Date
Marcel
74124dcc5d polish(transcribe): review nits — kbd size, focus ring, guard, action doc (#327)
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m33s
CI / OCR Service Tests (pull_request) Successful in 25s
CI / Backend Unit Tests (pull_request) Successful in 3m31s
CI / fail2ban Regex (pull_request) Successful in 45s
CI / Semgrep Security Scan (pull_request) Successful in 25s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m6s
Review follow-up (Leonie, Felix, Markus): bump cheatsheet key caps to text-sm
for the 60+ audience, add a focus-visible ring to the close button, simplify
the draw-hint guard to {#if drawArmed} (the $effect already clears it outside
edit mode), and document why the transcribeShortcuts action ignores its node
and binds to window.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-04 17:17:25 +02:00
Marcel
f7bc3ccfc1 test(transcribe): prove Delete fires once via real shape + action (#327)
Review follow-up (Sara): the prior single-owner evidence was two separate
unit facts against an inert DOM stub. This renders a real AnnotationShape,
attaches the live transcribeShortcuts action, focuses the region, and presses
Delete once — asserting deleteCurrentRegion fires exactly once. A genuine
integration guard against re-introducing a double-bind.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-04 17:16:03 +02:00
Marcel
194340c716 refactor(transcribe): extract region navigation into a tested pure helper (#327)
Review follow-up (Sara): j/k wrap-around and fresh-entry had no direct
coverage — the logic lived inline in the page where the action spec only
mocks the callbacks. Extracted to a pure stepRegion() with 9 unit tests
(empty list, forward/back, both wraps, fresh-entry null + unknown id,
length-1). Also replaces the inline nested ternary Felix flagged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-04 17:14:39 +02:00
Marcel
39ef5f2d83 fix(transcribe): hide the "?" hint on touch-only devices (#327)
Review follow-up (Requirements Engineer, Leonie) — closes the unmet
acceptance row. The coach card's "press ?" tip rendered unconditionally, so
a touch-only tablet transcriber (no hardware keyboard) was told to press a
key they don't have. The hint is now gated behind a fine-pointer media
query ([@media(pointer:coarse)]:hidden); the cheatsheet itself only opens
via the "?" key, so it already never surfaces without a keyboard. Also bumps
the key cap from 11px to text-xs for the 60+ audience.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-04 17:12:52 +02:00
Marcel
7199575a11 fix(transcribe): localise Delete key cap + annotation label, clarify Esc row (#327)
Review follow-up (Leonie, Requirements Engineer): the Delete key cap was a
hardcoded German "Entf" shown to EN/ES users — now driven by key_cap_delete
(Entf/Del/Supr). The annotation read-only aria-label was a hardcoded German
"Block anzeigen" in all locales — now annotation_view_label. Renamed the Esc
row label from "Bereich schließen" to "Panel schließen" so it no longer
collides with "Bereich" (= region) used elsewhere in the cheatsheet.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-04 17:11:29 +02:00
12 changed files with 161 additions and 17 deletions

View File

@@ -935,12 +935,14 @@
"shortcut_new_region": "Neuen Bereich zeichnen",
"shortcut_toggle_training": "Für Training markieren",
"shortcut_delete_region": "Aktuellen Bereich löschen",
"shortcut_close_panel": "Bereich schließen",
"shortcut_close_panel": "Panel schließen",
"shortcut_help": "Tastaturkürzel anzeigen",
"shortcut_draw_hint": "Ziehen Sie mit der Maus einen Bereich auf.",
"key_cap_delete": "Entf",
"cheatsheet_title": "Tastaturkürzel",
"cheatsheet_close": "Kürzelübersicht schließen",
"cheatsheet_autosave_hint": "Änderungen werden automatisch gespeichert.",
"annotation_view_label": "Block anzeigen",
"annotation_label_with_delete": "Block anzeigen, Entf zum Löschen.",
"transcription_mode_help_label": "Lese- und Bearbeitungsmodus",
"transcription_mode_help_body": "Lesen zeigt die Transkription als fließenden Text. Bearbeiten öffnet die Textfelder für jede Passage.",

View File

@@ -938,9 +938,11 @@
"shortcut_close_panel": "Close panel",
"shortcut_help": "Show keyboard shortcuts",
"shortcut_draw_hint": "Drag a region with your mouse.",
"key_cap_delete": "Del",
"cheatsheet_title": "Keyboard shortcuts",
"cheatsheet_close": "Close shortcut overview",
"cheatsheet_autosave_hint": "Changes are saved automatically.",
"annotation_view_label": "View block",
"annotation_label_with_delete": "Show block, press Delete to remove.",
"transcription_mode_help_label": "Read and edit mode",
"transcription_mode_help_body": "Read shows the transcription as flowing text. Edit opens the text fields for each passage.",

View File

@@ -938,9 +938,11 @@
"shortcut_close_panel": "Cerrar panel",
"shortcut_help": "Mostrar atajos de teclado",
"shortcut_draw_hint": "Arrastre una región con el ratón.",
"key_cap_delete": "Supr",
"cheatsheet_title": "Atajos de teclado",
"cheatsheet_close": "Cerrar el resumen de atajos",
"cheatsheet_autosave_hint": "Los cambios se guardan automáticamente.",
"annotation_view_label": "Ver bloque",
"annotation_label_with_delete": "Mostrar bloque, pulse Supr para eliminar.",
"transcription_mode_help_label": "Modo lectura y edición",
"transcription_mode_help_body": "Lectura muestra la transcripción como texto continuo. Edición abre los campos de texto para cada pasaje.",

View File

@@ -36,7 +36,9 @@ let {
// When deletion is available (transcribe mode), announce the otherwise-hidden
// Delete affordance to assistive tech (issue #327). The transcribeShortcuts
// action is the single owner of the key itself.
const ariaLabel = $derived(showDelete ? m.annotation_label_with_delete() : 'Block anzeigen');
const ariaLabel = $derived(
showDelete ? m.annotation_label_with_delete() : m.annotation_view_label()
);
function hexToRgba(hex: string, alpha: number): string {
const r = parseInt(hex.slice(1, 3), 16);

View File

@@ -2,9 +2,32 @@ 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';
import {
transcribeShortcuts,
type TranscribeShortcutOptions
} from '$lib/shared/actions/transcribeShortcuts';
afterEach(cleanup);
function noopShortcutOptions(
overrides: Partial<TranscribeShortcutOptions> = {}
): TranscribeShortcutOptions {
return {
isPanelOpen: () => true,
isCheatsheetOpen: () => false,
panelMode: () => 'edit',
goToNextRegion: () => {},
goToPrevRegion: () => {},
toggleMode: () => {},
closePanel: () => {},
startDrawMode: () => {},
toggleTrainingMark: () => {},
deleteCurrentRegion: () => {},
openCheatsheet: () => {},
...overrides
};
}
function makeAnnotation(id = 'ann-1') {
return {
id,
@@ -128,4 +151,29 @@ describe('AnnotationShape', () => {
expect(onfocus).toHaveBeenCalledOnce();
});
// Integration: a real rendered shape + the live transcribeShortcuts action.
// Pressing Delete on the focused region must delete exactly once — proving the
// action is the single owner and the shape contributes no competing handler.
it('with the transcribeShortcuts action active, Delete deletes the focused region exactly once', () => {
const deleteCurrentRegion = vi.fn();
render(AnnotationShape, {
annotation: makeAnnotation(),
isHovered: false,
isActive: true,
showDelete: true,
onclick: () => {},
onpointerenter: () => {},
onpointerleave: () => {}
});
const annotationEl = page.getByTestId('annotation-ann-1').element() as HTMLElement;
const action = transcribeShortcuts(annotationEl, noopShortcutOptions({ deleteCurrentRegion }));
annotationEl.focus();
annotationEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Delete', bubbles: true }));
expect(deleteCurrentRegion).toHaveBeenCalledTimes(1);
action.destroy();
});
});

View File

@@ -16,7 +16,7 @@ const groups = [
{ cap: 'e', label: m.shortcut_toggle_mode() },
{ cap: 'n', label: m.shortcut_new_region() },
{ cap: 't', label: m.shortcut_toggle_training() },
{ cap: 'Entf', label: m.shortcut_delete_region() }
{ cap: m.key_cap_delete(), label: m.shortcut_delete_region() }
],
[
{ cap: 'Esc', label: m.shortcut_close_panel() },
@@ -57,7 +57,7 @@ function handleBackdropClick(event: MouseEvent) {
type="button"
onclick={onClose}
aria-label={m.cheatsheet_close()}
class="flex h-11 w-11 items-center justify-center rounded-sm text-ink-2 hover:bg-muted"
class="flex h-11 w-11 items-center justify-center rounded-sm text-ink-2 hover:bg-muted focus-visible:ring-2 focus-visible:ring-brand-mint focus-visible:outline-none"
>
<svg class="h-5 w-5" viewBox="0 0 24 24" fill="none" stroke="currentColor" aria-hidden="true">
<path stroke-linecap="round" stroke-width="2" d="M6 6l12 12M18 6L6 18" />
@@ -71,7 +71,7 @@ function handleBackdropClick(event: MouseEvent) {
{#each group as shortcut (shortcut.cap)}
<div class="flex items-center justify-between gap-4">
<kbd
class="rounded border border-line bg-muted px-1.5 py-0.5 font-mono text-xs text-ink shadow-sm"
class="rounded border border-line bg-muted px-2 py-0.5 font-mono text-sm text-ink shadow-sm"
>{shortcut.cap}</kbd
>
<span class="flex-1 text-right font-serif text-sm text-ink">{shortcut.label}</span>

View File

@@ -0,0 +1,48 @@
import { describe, it, expect } from 'vitest';
import { stepRegion } from './regionNavigation';
describe('stepRegion', () => {
const ids = ['a', 'b', 'c'];
it('returns null for an empty list', () => {
expect(stepRegion([], null, 1)).toBe(null);
expect(stepRegion([], 'a', -1)).toBe(null);
});
it('steps forward from the middle', () => {
expect(stepRegion(ids, 'a', 1)).toBe('b');
expect(stepRegion(ids, 'b', 1)).toBe('c');
});
it('steps backward from the middle', () => {
expect(stepRegion(ids, 'c', -1)).toBe('b');
expect(stepRegion(ids, 'b', -1)).toBe('a');
});
it('wraps forward past the last region to the first', () => {
expect(stepRegion(ids, 'c', 1)).toBe('a');
});
it('wraps backward past the first region to the last', () => {
expect(stepRegion(ids, 'a', -1)).toBe('c');
});
it('lands on the first region when entering fresh (no active) going forward', () => {
expect(stepRegion(ids, null, 1)).toBe('a');
});
it('lands on the last region when entering fresh (no active) going backward', () => {
expect(stepRegion(ids, null, -1)).toBe('c');
});
it('treats an unknown active id as a fresh entry', () => {
expect(stepRegion(ids, 'zzz', 1)).toBe('a');
expect(stepRegion(ids, 'zzz', -1)).toBe('c');
});
it('returns the single region for both directions (wrap of length 1)', () => {
expect(stepRegion(['only'], 'only', 1)).toBe('only');
expect(stepRegion(['only'], 'only', -1)).toBe('only');
expect(stepRegion(['only'], null, 1)).toBe('only');
});
});

View File

@@ -0,0 +1,33 @@
/**
* Region navigation for the transcribe keyboard shortcuts (j/k) — issue #327.
*
* Pure and side-effect free so the wrap-around / fresh-entry branches are
* unit-testable without mounting the page.
*/
/**
* Pick the annotation id one step from the active region, wrapping around the
* ends. Entering fresh (no active region, or an unknown id) lands on the first
* region going forward and the last going backward.
*
* @param orderedAnnotationIds region annotation ids in display order
* @param activeId the currently active region, or null
* @param delta +1 for next (j), -1 for previous (k)
* @returns the next annotation id, or null when there are no regions
*/
export function stepRegion(
orderedAnnotationIds: string[],
activeId: string | null,
delta: 1 | -1
): string | null {
const count = orderedAnnotationIds.length;
if (count === 0) return null;
const current = activeId === null ? -1 : orderedAnnotationIds.indexOf(activeId);
if (current === -1) {
return delta > 0 ? orderedAnnotationIds[0] : orderedAnnotationIds[count - 1];
}
const next = (current + delta + count) % count;
return orderedAnnotationIds[next];
}

View File

@@ -27,6 +27,10 @@ function isEditableTarget(target: EventTarget | null): boolean {
return tag === 'INPUT' || tag === 'TEXTAREA' || target.isContentEditable;
}
// `node` is unused: the listener is global (window) so a shortcut fires no
// matter where focus sits on the page. It is still authored as a Svelte action
// (`use:transcribeShortcuts`) so its lifecycle is tied to the host element's
// mount/unmount and `destroy()` reliably removes the listener.
export function transcribeShortcuts(_node: HTMLElement, initial: TranscribeShortcutOptions) {
let options = initial;

View File

@@ -72,10 +72,10 @@ import TranscribeDragDemo from './TranscribeDragDemo.svelte';
{m.transcribe_coach_footer_richtlinien()}
<span class="ml-1 text-[11px] text-ink-3">{m.common_opens_new_tab()}</span>
</a>
<p class="w-full text-ink-3">
<p class="w-full text-ink-3 [@media(pointer:coarse)]:hidden">
{m.transcribe_coach_shortcut_hint_before()}
<kbd
class="rounded border border-line bg-muted px-1.5 py-0.5 font-mono text-[11px] text-ink shadow-sm"
class="rounded border border-line bg-muted px-1.5 py-0.5 font-mono text-xs text-ink shadow-sm"
>?</kbd
>
{m.transcribe_coach_shortcut_hint_after()}

View File

@@ -72,6 +72,14 @@ describe('TranscribeCoachEmptyState', () => {
expect(kbd?.textContent).toBe('?');
});
it('hides the keyboard hint on touch-only (coarse-pointer) devices', async () => {
render(TranscribeCoachEmptyState);
const hint = document.querySelector('kbd')?.closest('p');
// The hint is gated behind a fine-pointer media query so touch-only
// transcribers are never told to press a key they do not have (#327).
expect(hint?.className).toContain('pointer:coarse');
});
it('renders the drag demo animation region inside step 1', async () => {
render(TranscribeCoachEmptyState);
const demo = page.getByRole('img', { name: /Rahmen ziehen|Animation/i });

View File

@@ -12,6 +12,7 @@ import ShortcutCheatsheet from '$lib/document/transcription/ShortcutCheatsheet.s
import { transcribeShortcuts } from '$lib/shared/actions/transcribeShortcuts';
import { createOcrJob } from '$lib/ocr/useOcrJob.svelte';
import { createTranscriptionBlocks } from '$lib/document/transcription/useTranscriptionBlocks.svelte';
import { stepRegion } from '$lib/document/transcription/regionNavigation';
import { createFileLoader } from '$lib/document/viewer/useFileLoader.svelte';
import { scrollToCommentFromQuery } from '$lib/shared/utils/deepLinkScroll';
import { getConfirmService } from '$lib/shared/services/confirm.svelte';
@@ -102,15 +103,9 @@ async function createBlockFromDraw(rect: {
const sortedBlocks = $derived([...transcription.blocks].sort((a, b) => a.sortOrder - b.sortOrder));
function goToRegion(delta: 1 | -1) {
if (sortedBlocks.length === 0) return;
const current = sortedBlocks.findIndex((b) => b.annotationId === activeAnnotationId);
const next =
current === -1
? delta > 0
? 0
: sortedBlocks.length - 1
: (current + delta + sortedBlocks.length) % sortedBlocks.length;
activeAnnotationId = sortedBlocks[next].annotationId;
const ids = sortedBlocks.map((b) => b.annotationId);
const next = stepRegion(ids, activeAnnotationId, delta);
if (next) activeAnnotationId = next;
}
function toggleMode() {
@@ -427,7 +422,7 @@ onMount(() => {
{/if}
</div>
{#if drawArmed && panelMode === 'edit'}
{#if drawArmed}
<div
class="pointer-events-none absolute bottom-4 left-1/2 z-50 -translate-x-1/2 rounded-full bg-ink px-4 py-2 font-sans text-xs text-white shadow-lg"
role="status"