fix(journey-editor): note UX, error codes, list semantics, a11y labels

- handleNotePatch routes failures through failureMessage() so a backend
  JOURNEY_NOTE_TOO_LONG renders its specific message in the row
- handleNoteBlur: '' vs undefined no-op guard (no spurious PATCH
  {note:null}), empty blur collapses the textarea, clearing an existing
  note collapses after the PATCH lands (spec LE-3)
- 'Notiz hinzufügen' toggle gets aria-expanded and moves focus into the
  revealed textarea
- journey_remove_item_aria interpolates the item title (de/en/es); dead
  journey_drag_aria_label key deleted from all locales
- editor item list is an <ol> (screen readers announce the ordering)
- editor title input gets maxlength=255 + border-danger error cue; intro
  textarea gets maxlength=4000
- Briefmeta line (date · von X an Y) under document titles in the editor
  row via the shared formatDocumentMetaLine (spec LE-2)
- new specs: successful save clears the unsaved warning; item add does
  not arm the guard; server-confirmed order after successful reorder;
  blur-without-typing no-op; focus hand-off into the note textarea

Review round 3: Sara (1-3), Felix, Elicit (LE-2/LE-3), Leonie.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-11 08:28:38 +02:00
parent b9bed19610
commit 8995b6e922
7 changed files with 237 additions and 79 deletions

View File

@@ -18,6 +18,7 @@ type JourneyItemView = components['schemas']['JourneyItemView'];
interface Props {
geschichte: GeschichteView;
/** Must reject when the save failed — the editor keeps its dirty state then. */
onSubmit: (payload: {
title: string;
body: string;
@@ -203,7 +204,9 @@ async function handleNotePatch(itemId: string, note: string | null) {
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ note: note })
});
if (!res.ok) throw new Error('note patch failed');
// Carry the backend error code's message so the row can show the specific
// reason (e.g. JOURNEY_NOTE_TOO_LONG) instead of a generic alert.
if (!res.ok) throw new Error(await failureMessage(res));
const updated: JourneyItemView = await res.json();
items = items.map((i) => (i.id === itemId ? updated : i));
}
@@ -266,13 +269,16 @@ async function save(nextStatus: 'DRAFT' | 'PUBLISHED') {
<input
type="text"
bind:value={title}
maxlength="255"
oninput={() => unsaved.markDirty()}
onblur={() => (titleTouched = true)}
placeholder={m.geschichte_editor_title_placeholder()}
aria-label={m.journey_title_aria_label()}
aria-invalid={showTitleError}
aria-describedby={showTitleError ? 'journey-title-error' : undefined}
class="block w-full rounded border border-line bg-surface px-3 py-3 font-serif text-2xl font-bold text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
class="block w-full rounded border {showTitleError
? 'border-danger'
: 'border-line'} bg-surface px-3 py-3 font-serif text-2xl font-bold text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
/>
{#if showTitleError}
<p id="journey-title-error" class="mt-1 text-sm text-danger" role="alert">
@@ -285,6 +291,7 @@ async function save(nextStatus: 'DRAFT' | 'PUBLISHED') {
<div>
<textarea
bind:value={body}
maxlength="4000"
oninput={() => unsaved.markDirty()}
placeholder={m.journey_intro_placeholder()}
aria-label={m.journey_intro_aria_label()}
@@ -312,44 +319,43 @@ async function save(nextStatus: 'DRAFT' | 'PUBLISHED') {
</p>
{/if}
<!-- pointer events managed by createBlockDragDrop; keyboard reorder available via move-up/down buttons on each item -->
<!-- svelte-ignore a11y_no_static_element_interactions -->
<div
bind:this={listEl}
onpointermove={(e) => dragDrop.handlePointerMove(e)}
onpointerup={() => dragDrop.handlePointerUp()}
class="flex flex-col gap-2"
>
{#if items.length === 0}
<p class="font-sans text-sm text-ink-3">{m.journey_empty_state()}</p>
{/if}
{#each items as item, i (item.id)}
<!-- pointerdown initiates drag; the drag handle button inside is the semantic interactive element -->
<!-- svelte-ignore a11y_no_static_element_interactions -->
<div
data-block-wrapper
onpointerdown={(e) => dragDrop.handleGripDown(e, item.id)}
class="transition-all duration-150 {dragDrop.draggedBlockId === item.id ? 'z-10 rounded-lg shadow-xl ring-2 ring-focus-ring/40' : ''}"
style={dragDrop.draggedBlockId === item.id
? `transform: translateY(${dragDrop.dragOffsetY}px) scale(1.02); opacity: 0.9;`
: ''}
>
{#if dragDrop.dropTargetIdx === i}
<div class="mb-1 h-1 rounded-full bg-accent transition-all"></div>
{/if}
<JourneyItemRow
item={item}
index={i}
total={items.length}
pendingRemove={pendingRemoveIds.includes(item.id)}
onMoveUp={() => handleMoveUp(i)}
onMoveDown={() => handleMoveDown(i)}
onRemove={() => handleRemove(item.id)}
onNotePatch={(note) => handleNotePatch(item.id, note)}
/>
</div>
{/each}
</div>
{#if items.length === 0}
<p class="font-sans text-sm text-ink-3">{m.journey_empty_state()}</p>
{:else}
<!-- pointer events managed by createBlockDragDrop; keyboard reorder available via move-up/down buttons on each item -->
<ol
bind:this={listEl}
onpointermove={(e) => dragDrop.handlePointerMove(e)}
onpointerup={() => dragDrop.handlePointerUp()}
class="m-0 flex list-none flex-col gap-2 p-0"
>
{#each items as item, i (item.id)}
<!-- pointerdown initiates drag; the drag handle button inside is the semantic interactive element -->
<li
data-block-wrapper
onpointerdown={(e) => dragDrop.handleGripDown(e, item.id)}
class="transition-all duration-150 {dragDrop.draggedBlockId === item.id ? 'z-10 rounded-lg shadow-xl ring-2 ring-focus-ring/40' : ''}"
style={dragDrop.draggedBlockId === item.id
? `transform: translateY(${dragDrop.dragOffsetY}px) scale(1.02); opacity: 0.9;`
: ''}
>
{#if dragDrop.dropTargetIdx === i}
<div class="mb-1 h-1 rounded-full bg-accent transition-all"></div>
{/if}
<JourneyItemRow
item={item}
index={i}
total={items.length}
pendingRemove={pendingRemoveIds.includes(item.id)}
onMoveUp={() => handleMoveUp(i)}
onMoveDown={() => handleMoveDown(i)}
onRemove={() => handleRemove(item.id)}
onNotePatch={(note) => handleNotePatch(item.id, note)}
/>
</li>
{/each}
</ol>
{/if}
<JourneyAddBar
alreadyAddedIds={alreadyAddedIds}

View File

@@ -338,7 +338,9 @@ describe('JourneyEditor — remove with pending state', () => {
render(JourneyEditor, defaultProps({ geschichte: makeGeschichte({ items }) }));
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief A' }) })
);
// Row still present, marked as pending (text appears in the row AND the live region,
// so scope the query to the row instead of using a page-wide locator)
@@ -364,7 +366,9 @@ describe('JourneyEditor — remove with pending state', () => {
render(JourneyEditor, defaultProps({ geschichte: makeGeschichte({ items }) }));
// Click remove (no note → direct remove)
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief A' }) })
);
await expect.element(page.getByRole('alert')).toBeInTheDocument();
await expect.element(page.getByText('Brief A')).toBeInTheDocument();
@@ -376,7 +380,9 @@ describe('JourneyEditor — remove with pending state', () => {
render(JourneyEditor, defaultProps({ geschichte: makeGeschichte({ items }) }));
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief A' }) })
);
await expect.element(page.getByText('Brief A')).not.toBeInTheDocument();
});
@@ -387,7 +393,9 @@ describe('JourneyEditor — remove with pending state', () => {
render(JourneyEditor, defaultProps({ geschichte: makeGeschichte({ items }) }));
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief A' }) })
);
await expect.element(page.getByText('Brief A')).not.toBeInTheDocument();
await vi.waitFor(() => {
@@ -460,6 +468,36 @@ describe('JourneyEditor — reorder via move buttons', () => {
});
});
it('renders the server-confirmed order after a successful reorder', async () => {
const items = [
{ id: 'i1', position: 0, document: docSummary('d1', 'Brief A') },
{ id: 'i2', position: 1, document: docSummary('d2', 'Brief B') }
];
// Server response deliberately NOT pre-sorted — pins items = updated.sort(...)
vi.stubGlobal(
'fetch',
vi.fn().mockResolvedValue({
ok: true,
json: vi.fn().mockResolvedValue([
{ id: 'i1', position: 20, document: docSummary('d1', 'Brief A') },
{ id: 'i2', position: 10, document: docSummary('d2', 'Brief B') }
])
})
);
render(JourneyEditor, defaultProps({ geschichte: makeGeschichte({ items }) }));
await userEvent.click(page.getByRole('button', { name: /Brief A.*nach unten verschieben/ }));
await vi.waitFor(() => {
const briefB = page.getByText('Brief B').element();
const briefA = page.getByText('Brief A').element();
expect(
briefB.compareDocumentPosition(briefA) & Node.DOCUMENT_POSITION_FOLLOWING
).toBeTruthy();
});
});
it('restores the original DOM order and shows an alert on failed reorder (non-ok)', async () => {
const items = [
{ id: 'i1', position: 0, document: docSummary('d1', 'Brief A') },
@@ -630,7 +668,7 @@ describe('JourneyEditor — unsaved warning banner', () => {
it('banner is absent before any edit or navigation attempt', async () => {
render(JourneyEditor, defaultProps());
expect(document.querySelector('[class*="amber"]')).toBeNull();
expect(page.getByText(m.admin_unsaved_warning()).query()).toBeNull();
});
it('banner appears when dirty and a navigation is attempted', async () => {
@@ -673,9 +711,52 @@ describe('JourneyEditor — unsaved warning banner', () => {
// Banner must still be visible (isDirty was not cleared)
await vi.waitFor(() => {
expect(document.querySelector('[class*="amber"]')).toBeTruthy();
expect(page.getByText(m.admin_unsaved_warning()).query()).toBeTruthy();
});
});
it('successful save clears the unsaved warning (navigation unblocked after onSubmit resolves)', async () => {
// Regression guard for clearOnSuccess(): without it, a curator who edits the
// title and saves successfully stays trapped — the page goto() gets cancelled
// by the still-armed guard and the banner appears after a *successful* save.
const onSubmit = vi.fn().mockResolvedValue(undefined);
render(JourneyEditor, defaultProps({ onSubmit }));
// Mark dirty
const titleInput = page.getByPlaceholder(/Titel/);
await titleInput.element().dispatchEvent(new InputEvent('input', { bubbles: true }));
// Dirty state blocks navigation
expect(triggerNavigationAttempt()).toHaveBeenCalled();
// Save succeeds
await userEvent.click(page.getByRole('button', { name: m.geschichte_editor_save_draft() }));
await vi.waitFor(() => expect(onSubmit).toHaveBeenCalled());
// Guard is disarmed again — navigation passes and no banner shows
await vi.waitFor(() => {
expect(triggerNavigationAttempt()).not.toHaveBeenCalled();
});
expect(page.getByText(m.admin_unsaved_warning()).query()).toBeNull();
});
it('item add does not arm the unsaved-changes guard (items persist immediately)', async () => {
mockCsrfFetch(() => ({ id: 'i-new', position: 10, note: 'Zwischentext' }));
render(JourneyEditor, defaultProps());
await userEvent.click(page.getByText(m.journey_add_interlude()));
await userEvent.fill(page.getByPlaceholder(m.journey_interlude_placeholder()), 'Zwischentext');
await userEvent.click(
page.getByRole('button', { name: m.journey_add_interlude_confirm(), exact: true })
);
// The new interlude row renders its note textarea once the POST resolved
await expect
.element(page.getByRole('textbox', { name: /Kuratoren-Notiz/ }))
.toBeInTheDocument();
// The item was persisted by its own POST — navigating away loses nothing
expect(triggerNavigationAttempt()).not.toHaveBeenCalled();
});
});
describe('JourneyEditor — selectedPersons marks dirty', () => {

View File

@@ -2,6 +2,7 @@
import { tick } from 'svelte';
import type { components } from '$lib/generated/api';
import { m } from '$lib/paraglide/messages.js';
import { formatDocumentMetaLine } from './utils';
type JourneyItemView = components['schemas']['JourneyItemView'];
@@ -29,6 +30,8 @@ let {
const isInterlude = $derived(!item.document);
const itemTitle = $derived(item.document?.title ?? m.journey_interlude_label());
// Spec LE-2 "Briefmeta": date · von X an Y disambiguates near-identical titles.
const metaLine = $derived(item.document ? formatDocumentMetaLine(item.document) : '');
const needsConfirmOnRemove = $derived(!!item.note);
let rootEl: HTMLElement | null = $state(null);
@@ -40,8 +43,14 @@ let showRemoveConfirm = $state(false);
async function handleNoteBlur() {
if (noteSaving) return;
if (noteDraft === item.note) return;
if (isInterlude && noteDraft.trim().length === 0) {
const normalizedDraft = noteDraft.trim().length === 0 ? null : noteDraft;
// '' and undefined both mean "no note" — never PATCH a no-op.
if (normalizedDraft === (item.note ?? null)) {
// Opened "Notiz hinzufügen" and blurred without typing → collapse again.
if (!isInterlude && normalizedDraft === null) showNote = false;
return;
}
if (isInterlude && normalizedDraft === null) {
// Interludes must keep a note — restore the draft so the UI doesn't show
// an emptied text that the server still holds.
noteDraft = item.note ?? '';
@@ -51,9 +60,11 @@ async function handleNoteBlur() {
noteSaving = true;
noteError = '';
try {
await onNotePatch(noteDraft.trim().length === 0 ? null : noteDraft);
} catch {
noteError = m.journey_note_error();
await onNotePatch(normalizedDraft);
// Clearing an existing note collapses the textarea after the PATCH lands.
if (normalizedDraft === null) showNote = false;
} catch (e) {
noteError = e instanceof Error && e.message ? e.message : m.journey_note_error();
} finally {
noteSaving = false;
}
@@ -67,13 +78,20 @@ async function handleNoteRemove() {
noteError = '';
try {
await onNotePatch(null);
} catch {
} catch (e) {
noteDraft = prevDraft;
showNote = prevShowNote;
noteError = m.journey_note_error();
noteError = e instanceof Error && e.message ? e.message : m.journey_note_error();
}
}
async function handleNoteOpen() {
showNote = true;
// Spec LE-3: focus moves into the revealed textarea.
await tick();
rootEl?.querySelector<HTMLTextAreaElement>('textarea')?.focus();
}
async function handleRemoveClick() {
if (needsConfirmOnRemove) {
showRemoveConfirm = true;
@@ -103,7 +121,7 @@ async function handleRemoveCancel() {
'flex min-w-0 flex-col rounded border transition-colors',
pendingRemove ? 'opacity-60' : '',
isInterlude
? 'border-l-4 border-[var(--color-interlude-border)] bg-[var(--color-interlude-bg)]'
? 'border-l-4 border-line border-l-interlude-border bg-interlude-bg'
: 'border-line bg-surface'
].join(' ')}
>
@@ -150,15 +168,15 @@ async function handleRemoveCancel() {
<!-- Content (title + note inline) -->
<div class="min-w-0 flex-1 py-1 break-words">
{#if isInterlude}
<span
class="font-sans text-xs font-bold tracking-widest uppercase"
style="color: var(--color-interlude-label);"
>
<span class="font-sans text-xs font-bold tracking-widest text-interlude-label uppercase">
{m.journey_interlude_label()}
</span>
{:else}
<span class="font-sans text-xs text-ink-3">{index + 1}.</span>
<span class="ml-1 font-serif text-sm text-ink">{item.document!.title}</span>
{#if metaLine}
<p class="mt-0.5 font-sans text-xs text-ink-3">{metaLine}</p>
{/if}
{/if}
{#if showNote}
@@ -190,9 +208,8 @@ async function handleRemoveCancel() {
{:else if !isInterlude}
<button
type="button"
onclick={() => {
showNote = true;
}}
onclick={handleNoteOpen}
aria-expanded={showNote}
class="mt-0.5 inline-flex min-h-[44px] items-center font-sans text-xs text-ink-3 underline hover:text-accent focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
>
{m.journey_note_add()}
@@ -232,7 +249,7 @@ async function handleRemoveCancel() {
type="button"
data-remove-btn
onclick={handleRemoveClick}
aria-label={m.journey_remove_item_aria()}
aria-label={m.journey_remove_item_aria({ title: itemTitle })}
class="-m-1 inline-flex min-h-[44px] min-w-[44px] items-center justify-center rounded p-3 text-ink-3 hover:text-danger focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring"
>
<svg

View File

@@ -66,6 +66,35 @@ describe('JourneyItemRow — note textarea', () => {
.toBeInTheDocument();
});
it('blur without typing does not call onNotePatch and collapses the textarea', async () => {
// '' (untouched draft) and undefined (no note) both mean "no note" — a
// spurious PATCH {note: null} must not fire, and the empty textarea closes.
const onNotePatch = vi.fn().mockResolvedValue(undefined);
render(JourneyItemRow, { item: docItem(), ...defaultProps({ onNotePatch }) });
await userEvent.click(page.getByText(m.journey_note_add()));
const textarea = page.getByRole('textbox', { name: /Kuratoren-Notiz für Brief von Karl/ });
await textarea.element().dispatchEvent(new FocusEvent('blur'));
expect(onNotePatch).not.toHaveBeenCalled();
await expect.element(page.getByText(m.journey_note_add())).toBeInTheDocument();
});
it('moves focus into the textarea when "Notiz hinzufügen" opens it', async () => {
render(JourneyItemRow, { item: docItem(), ...defaultProps() });
const toggle = page.getByText(m.journey_note_add());
expect(toggle.element().getAttribute('aria-expanded')).toBe('false');
await userEvent.click(toggle);
await vi.waitFor(() => {
const textarea = page
.getByRole('textbox', { name: /Kuratoren-Notiz für Brief von Karl/ })
.element();
expect(document.activeElement).toBe(textarea);
});
});
it('calls onNotePatch on textarea blur with non-empty value', async () => {
const onNotePatch = vi.fn().mockResolvedValue(undefined);
render(JourneyItemRow, { item: docItem(), ...defaultProps({ onNotePatch }) });
@@ -167,7 +196,9 @@ describe('JourneyItemRow — remove confirm', () => {
});
// Click remove (x button)
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
await expect.element(page.getByText(m.journey_remove_confirm())).toBeInTheDocument();
await expect
@@ -182,7 +213,9 @@ describe('JourneyItemRow — remove confirm', () => {
...defaultProps({ onRemove })
});
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
await userEvent.click(page.getByRole('button', { name: m.journey_remove_confirm_yes() }));
expect(onRemove).toHaveBeenCalledTimes(1);
@@ -195,13 +228,17 @@ describe('JourneyItemRow — remove confirm', () => {
...defaultProps({ onRemove })
});
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
await userEvent.click(page.getByRole('button', { name: m.journey_remove_confirm_cancel() }));
expect(onRemove).not.toHaveBeenCalled();
// The remove button should be back
await expect
.element(page.getByRole('button', { name: m.journey_remove_item_aria() }))
.element(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
)
.toBeInTheDocument();
});
@@ -211,11 +248,15 @@ describe('JourneyItemRow — remove confirm', () => {
...defaultProps()
});
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
await userEvent.click(page.getByRole('button', { name: m.journey_remove_confirm_cancel() }));
await vi.waitFor(() => {
const removeBtn = page.getByRole('button', { name: m.journey_remove_item_aria() }).element();
const removeBtn = page
.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
.element();
expect(document.activeElement).toBe(removeBtn);
});
});
@@ -228,7 +269,9 @@ describe('JourneyItemRow — remove confirm a11y', () => {
...defaultProps()
});
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
const group = document.querySelector('[role="group"]');
expect(group).toBeTruthy();
@@ -241,7 +284,9 @@ describe('JourneyItemRow — remove confirm a11y', () => {
...defaultProps()
});
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
await vi.waitFor(() => {
const cancelBtn = page
@@ -257,7 +302,9 @@ describe('JourneyItemRow — remove confirm a11y', () => {
...defaultProps()
});
await userEvent.click(page.getByRole('button', { name: m.journey_remove_item_aria() }));
await userEvent.click(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
);
await vi.waitFor(() => {
const cancelBtn = page
.getByRole('button', { name: m.journey_remove_confirm_cancel() })
@@ -268,7 +315,9 @@ describe('JourneyItemRow — remove confirm a11y', () => {
await userEvent.keyboard('{Escape}');
await vi.waitFor(() => {
const removeBtn = page.getByRole('button', { name: m.journey_remove_item_aria() }).element();
const removeBtn = page
.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
.element();
expect(document.activeElement).toBe(removeBtn);
});
await expect.element(page.getByText(m.journey_remove_confirm())).not.toBeInTheDocument();
@@ -284,7 +333,9 @@ describe('JourneyItemRow — pending remove state', () => {
await expect.element(page.getByText(m.journey_item_pending_remove())).toBeInTheDocument();
await expect
.element(page.getByRole('button', { name: m.journey_remove_item_aria() }))
.element(
page.getByRole('button', { name: m.journey_remove_item_aria({ title: 'Brief von Karl' }) })
)
.not.toBeInTheDocument();
const root = document.querySelector('[data-block-id="item-1"]')!;