From 77135df2515b63b6253a9a2ccb09eafffb5f7004 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 19:16:53 +0200 Subject: [PATCH 1/9] feat(i18n): add re-edit @mention keys (edit/editing-announce/dismiss) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keys for the re-edit affordance landing in #628: - person_mention_edit_label — pencil button aria-label - person_mention_editing_announce — aria-live editing context - person_mention_dismiss_label — dropdown close button aria-label Co-Authored-By: Claude Opus 4.8 --- frontend/messages/de.json | 3 +++ frontend/messages/en.json | 3 +++ frontend/messages/es.json | 3 +++ 3 files changed, 9 insertions(+) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 21fc48aa..0c2d734a 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -506,6 +506,9 @@ "person_mention_create_new": "Neue Person anlegen", "person_mention_results_count_singular": "1 Person gefunden", "person_mention_results_count_plural": "{count} Personen gefunden", + "person_mention_edit_label": "Erwähnung bearbeiten", + "person_mention_editing_announce": "Erwähnung wird bearbeitet: {displayName}", + "person_mention_dismiss_label": "Suche schließen", "transcription_editor_aria_label": "Transkriptionstext", "person_born_name_prefix": "geb.", "page_title_home": "Archiv", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 55b12b51..45f3bc39 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -506,6 +506,9 @@ "person_mention_create_new": "Create new person", "person_mention_results_count_singular": "1 person found", "person_mention_results_count_plural": "{count} persons found", + "person_mention_edit_label": "Edit mention", + "person_mention_editing_announce": "Editing mention: {displayName}", + "person_mention_dismiss_label": "Close search", "transcription_editor_aria_label": "Transcription text", "person_born_name_prefix": "née", "page_title_home": "Archive", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index cdebf5db..ed304b14 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -506,6 +506,9 @@ "person_mention_create_new": "Crear nueva persona", "person_mention_results_count_singular": "1 persona encontrada", "person_mention_results_count_plural": "{count} personas encontradas", + "person_mention_edit_label": "Editar mención", + "person_mention_editing_announce": "Editando mención: {displayName}", + "person_mention_dismiss_label": "Cerrar búsqueda", "transcription_editor_aria_label": "Texto de transcripción", "person_born_name_prefix": "n.", "page_title_home": "Archivo", -- 2.49.1 From cf1d34657e0d3d50918287307077b7f53aaab2d4 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 19:22:27 +0200 Subject: [PATCH 2/9] refactor(transcription): lift @mention dropdown lifecycle into a single controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulls mountedDropdown / requestId / debouncedSearch / dropdownState ownership out of Tiptap's suggestion.render() closure into one createMentionController(). render() becomes a thin adapter: onStart→open, onUpdate→update, onExit→close. This is the single-owner structure #628 needs for the AC-6 single-dropdown invariant — the upcoming pencil re-edit affordance opens via the same controller.open() instead of racing the suggestion plugin over module state. open() now also bumps the request token so an open-A→open-B sequence discards A's in-flight fetch (preserved increment-on-open semantics). No behaviour change for the fresh-@ path — existing browser suite is the regression guard. Co-Authored-By: Claude Opus 4.8 --- .../discussion/PersonMentionEditor.svelte | 241 ++++++++++-------- 1 file changed, 140 insertions(+), 101 deletions(-) diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte index 39261606..98e19632 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte @@ -63,6 +63,133 @@ type DropdownExports = { onKeyDown: (event: KeyboardEvent) => boolean; }; +type CommitFn = (item: Person) => void; +type RectGetter = (() => DOMRect | null) | null; + +// Tiptap's SuggestionProps types `command` against the default MentionNodeAttrs +// (id/label). Our custom Mention extension uses personId/displayName, so the +// render() adapter casts the renderProps to this looser shape locally. +type LooseRenderProps = { + items: unknown; + command: (props: { personId: string; displayName: string }) => void; + query: string; + clientRect?: (() => DOMRect | null) | null; +}; + +// Single owner of the dropdown lifecycle. Both entry points route through one +// controller — Tiptap's fresh-`@` suggestion (via the render() adapter below) +// and (#628) the pencil re-edit affordance — so at most one dropdown is ever +// mounted (the AC-6 single-dropdown invariant). open() closes any prior +// dropdown first; render() is a thin adapter over open()/update()/close(). +type MentionController = { + open: (clientRect: RectGetter, query: string, commit: CommitFn) => void; + update: (clientRect: RectGetter, query: string, commit: CommitFn) => void; + close: () => void; + onKeyDown: (event: KeyboardEvent) => boolean; +}; + +function createMentionController(): MentionController { + // Request-token guard: every search AND every open bumps `requestId`; + // runSearch captures the id active when its fetch starts and discards the + // response if a newer search/open has happened since. Without this, a late + // response can repopulate a dropdown the user already moved on from (e.g. + // open A → open B → A's stale response). Sara on PR #629. + let requestId = 0; + let exports: DropdownExports | null = null; + + const runSearch = async (query: string) => { + const id = requestId; + try { + // Defensive client-side cap — server-side enforcement is tracked + // separately. Markus on PR #629. + const res = await fetch( + `/api/persons?review=true&q=${encodeURIComponent(query)}&size=${SEARCH_RESULT_LIMIT}` + ); + if (id !== requestId) return; + if (!res.ok) { + dropdownState.items = []; + return; + } + const body = (await res.json()) as { items?: Person[] }; + if (id !== requestId) return; + dropdownState.items = (body.items ?? []).slice(0, SEARCH_RESULT_LIMIT); + } catch { + if (id !== requestId) return; + dropdownState.items = []; + } + }; + + const debouncedSearch = debounce(runSearch, SEARCH_DEBOUNCE_MS); + // Hoisted so onDestroy can cancel any pending fetch even after the editor is + // gone — a trailing debounced search would otherwise pollute later tests. + cancelPendingSearch = () => debouncedSearch.cancel(); + + const onSearch = (query: string) => { + requestId++; + if (query.trim() === '') { + debouncedSearch.cancel(); + dropdownState.items = []; + return; + } + debouncedSearch(query); + }; + + const close = () => { + debouncedSearch.cancel(); + if (mountedDropdown) { + unmount(mountedDropdown); + mountedDropdown = null; + exports = null; + } + }; + + // Clip the query once so the dropdown's editor-mirror sees the capped value + // (CWE-400 amplification — Nora #1 / Felix #3 on PR #629). The commit closure + // is supplied by the caller, so the fresh-insert path keeps clipping the + // inserted displayName while the relink path (#628) preserves the stored + // displayName by construction. + const writeState = (clientRect: RectGetter, query: string, commit: CommitFn) => { + dropdownState.command = commit; + dropdownState.clientRect = clientRect; + dropdownState.editorQuery = query.slice(0, MAX_QUERY_LENGTH); + }; + + const open = (clientRect: RectGetter, query: string, commit: CommitFn) => { + // Single-dropdown invariant: tear down any open dropdown before mounting a + // new one, and bump the request token so a previous open's in-flight fetch + // cannot repopulate this dropdown. + close(); + requestId++; + dropdownState.items = []; + writeState(clientRect, query, commit); + const mounted = mount(MentionDropdown, { + target: document.body, + props: { + model: dropdownState, + // MentionDropdown reads `editorQuery` off the shared state proxy via + // this getter — Svelte 5's mount() does not expose settable prop + // accessors, so we route through the proxy (same pattern as items). + get editorQuery() { + return dropdownState.editorQuery; + }, + onSearch + } + }); + mountedDropdown = mounted as object; + exports = mounted as unknown as DropdownExports; + }; + + const update = (clientRect: RectGetter, query: string, commit: CommitFn) => { + writeState(clientRect, query, commit); + }; + + const onKeyDown = (event: KeyboardEvent) => exports?.onKeyDown(event) ?? false; + + return { open, update, close, onKeyDown }; +} + +const controller = createMentionController(); + onMount(() => { // Custom Mention node: uses personId / displayName instead of the // default id / label attribute names so the mentionSerializer can @@ -172,120 +299,32 @@ onMount(() => { ]) .run(); }, + // Thin adapter over the single mention controller. The fresh-`@` + // commit clips the typed query into the inserted displayName + // (AC-1, #380); the controller owns the dropdown lifecycle and the + // debounced/request-token search. render() { - let exports: DropdownExports | null = null; - - // Tiptap's SuggestionProps types `command` against the default - // MentionNodeAttrs (id/label). Our custom Mention extension uses - // personId/displayName, so we cast the renderProps locally. - type LooseRenderProps = { - items: unknown; - command: (props: { personId: string; displayName: string }) => void; - query: string; - clientRect?: (() => DOMRect | null) | null; + const buildFreshCommit = (loose: LooseRenderProps): CommitFn => { + const clippedQuery = loose.query.slice(0, MAX_QUERY_LENGTH); + return (item: Person) => + loose.command({ personId: item.id, displayName: clippedQuery }); }; - - // Request-token guard: every onSearch invocation bumps `requestId`; - // runSearch captures the id active when its fetch starts and discards - // the response if a newer onSearch has fired since. Without this, a - // late response can repopulate the dropdown after the user cleared - // the search input. Sara on PR #629. - let requestId = 0; - const runSearch = async (query: string) => { - const id = requestId; - try { - // Defensive client-side cap — server-side enforcement is tracked - // separately. Markus on PR #629. - const res = await fetch( - `/api/persons?review=true&q=${encodeURIComponent(query)}&size=${SEARCH_RESULT_LIMIT}` - ); - if (id !== requestId) return; - if (!res.ok) { - dropdownState.items = []; - return; - } - const body = (await res.json()) as { items?: Person[] }; - if (id !== requestId) return; - dropdownState.items = (body.items ?? []).slice(0, SEARCH_RESULT_LIMIT); - } catch { - if (id !== requestId) return; - dropdownState.items = []; - } - }; - const debouncedSearch = debounce(runSearch, SEARCH_DEBOUNCE_MS); - cancelPendingSearch = () => debouncedSearch.cancel(); - const onSearch = (query: string) => { - requestId++; - if (query.trim() === '') { - debouncedSearch.cancel(); - dropdownState.items = []; - return; - } - debouncedSearch(query); - }; - - const updateState = (renderProps: LooseRenderProps) => { - // Clip once here so both the inserted displayName and the - // dropdown's editor-mirror see the same value. The dropdown - // already clips the mirror (Nora #1 CWE-400), but without - // clipping at the command boundary an unclipped query would - // still flow through as the inserted displayName — visible - // UI divergence between "what I searched" and "what was - // inserted". Felix #3 on PR #629. - const clippedQuery = renderProps.query.slice(0, MAX_QUERY_LENGTH); - // AC-1: pass typed query as displayName, not person.displayName - dropdownState.command = (item: Person) => - renderProps.command({ - personId: item.id, - displayName: clippedQuery - }); - dropdownState.clientRect = renderProps.clientRect ?? null; - dropdownState.editorQuery = clippedQuery; - }; - return { onStart(renderProps) { const loose = renderProps as unknown as LooseRenderProps; - updateState(loose); - // MentionDropdown reads `editorQuery` off the shared state - // proxy via its `editorQuery` prop binding below — this is - // the same pattern as `model.items`. We do not pass it as a - // separate prop because Svelte 5's mount() does not expose - // settable prop accessors, so we route through the proxy. - const mounted = mount(MentionDropdown, { - target: document.body, - props: { - model: dropdownState, - get editorQuery() { - return dropdownState.editorQuery; - }, - onSearch - } - }); - mountedDropdown = mounted as object; - exports = mounted as unknown as DropdownExports; + controller.open(loose.clientRect ?? null, loose.query, buildFreshCommit(loose)); }, onUpdate(renderProps) { - updateState(renderProps as unknown as LooseRenderProps); + const loose = renderProps as unknown as LooseRenderProps; + controller.update(loose.clientRect ?? null, loose.query, buildFreshCommit(loose)); }, onKeyDown({ event }) { // Escape is handled by the suggestion plugin itself. if (event.key === 'Escape') return false; - return exports?.onKeyDown(event) ?? false; + return controller.onKeyDown(event); }, onExit() { - // Cancel any pending debounce so a closed dropdown's trailing - // runSearch cannot fire against the *next* dropdown's state. - // The hoisted `cancelPendingSearch` would be overwritten by - // the next render()'s onStart before the trailing call fires, - // so we cancel locally via the closure-scoped debouncedSearch. - // Felix #1 on PR #629. - debouncedSearch.cancel(); - if (mountedDropdown) { - unmount(mountedDropdown); - mountedDropdown = null; - exports = null; - } + controller.close(); } }; } -- 2.49.1 From 02c95b9cfc5ca629ea4da9e58eec8fd9b660d60e Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 19:33:53 +0200 Subject: [PATCH 3/9] feat(transcription): re-edit @mention via a pencil affordance (#628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hosts each mention as a Tiptap NodeView (mentionNodeView.ts) that renders the @displayName token (textContent — never innerHTML) plus a contenteditable=false pencil button in a fixed-width slot, revealed on whole-token hover and keyboard focus (instant opacity swap, no reflow). Activating the pencil (click or Enter/ Space) opens the single mention dropdown via the controller, anchored at the token and pre-filled with the stored displayName. commitRelink swaps ONLY personId in place via setNodeMarkup, sourcing the id solely from the selected Person — the stored displayName is preserved by construction (AC-3), even after the search input is edited (AC-5, the #380 AC-1 invariant). renderHTML/renderText stay for serialization + clipboard. AC-1/AC-2/AC-3/AC-5 + serializer round-trip covered by browser tests. Co-Authored-By: Claude Opus 4.8 --- .../discussion/PersonMentionEditor.svelte | 38 ++++ .../PersonMentionEditor.svelte.spec.ts | 187 ++++++++++++++++++ .../lib/shared/discussion/mentionNodeView.ts | 152 ++++++++++++++ 3 files changed, 377 insertions(+) create mode 100644 frontend/src/lib/shared/discussion/mentionNodeView.ts diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte index 98e19632..27b09457 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte @@ -9,6 +9,7 @@ import type { PersonMention } from '$lib/shared/types'; import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer'; import { debounce } from '$lib/shared/utils/debounce'; import MentionDropdown from './MentionDropdown.svelte'; +import { createMentionNodeView } from './mentionNodeView'; import { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants'; type Person = components['schemas']['Person']; @@ -190,6 +191,37 @@ function createMentionController(): MentionController { const controller = createMentionController(); +// Re-link an existing mention in place (#628 AC-3/AC-5). Captures the node's +// pos when the pencil opens and swaps ONLY personId via setNodeMarkup, so the +// stored displayName is preserved by construction — never round-tripped through +// insertContentAt (which would rebind displayName to the search query). The new +// personId comes solely from the selected Person (anti-spoof — never from the +// reflected data-person-id or the search input). +function commitRelink(pos: number): CommitFn { + return (item: Person) => { + if (!editor) return; + editor + .chain() + .focus() + .command(({ tr, state }) => { + const node = state.doc.nodeAt(pos); + if (!node || node.type.name !== 'mention') return false; + tr.setNodeMarkup(pos, undefined, { ...node.attrs, personId: item.id }); + return true; + }) + .run(); + controller.close(); + }; +} + +// Entry point handed to the mention NodeView's pencil. Opens the one dropdown +// (closing any other first — AC-6) anchored at the mention and pre-filled with +// the stored displayName. +function requestRelink(getRect: () => DOMRect | null, displayName: string, pos: number) { + if (!editor || !editor.isEditable) return; + controller.open(getRect, displayName, commitRelink(pos)); +} + onMount(() => { // Custom Mention node: uses personId / displayName instead of the // default id / label attribute names so the mentionSerializer can @@ -215,6 +247,12 @@ onMount(() => { }) } }; + }, + // #628: host the mention as a NodeView so each token carries a pencil + // re-edit affordance. renderHTML/renderText below stay for serialization + // and clipboard; this only governs the live in-editor DOM. + addNodeView() { + return createMentionNodeView(requestRelink); } }); diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 164e9b32..3b7c601c 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -771,3 +771,190 @@ describe('PersonMentionEditor — touch target', () => { expect(option.className).toContain('min-h-[44px]'); }); }); + +// ─── #628: re-edit an existing @mention via the pencil affordance ───────────── + +describe('PersonMentionEditor — #628 re-edit pencil', () => { + // A saved mention seeded into the editor: text "@Aug " + sidecar. The typed + // displayName is "Aug" (short form, #380 AC-1), distinct from the DB name. + const SAVED = { personId: 'p-aug', displayName: 'Aug' }; + + function editPencil() { + return page.getByRole('button', { name: m.person_mention_edit_label() }); + } + + async function renderSavedMention() { + const host = renderHost({ value: '@Aug ', mentionedPersons: [SAVED] }); + await expect.element(editPencil()).toBeInTheDocument(); + return host; + } + + async function pencilElement(): Promise { + return (await editPencil().element()) as HTMLElement; + } + + function clickPencil(btn: HTMLElement) { + // Native dispatch — CDP userEvent.click is unreliable for handlers attached + // imperatively inside a ProseMirror NodeView (same rationale as the option + // mousedown dispatch elsewhere in this file). + btn.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + } + + async function pickOption(name: RegExp) { + const el = (await page.getByRole('option', { name }).element()) as HTMLElement; + el.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true })); + } + + // AC-1 ---------------------------------------------------------------------- + + it('renders an edit pencil on a saved mention, labelled via aria-label', async () => { + await renderSavedMention(); + await expect.element(editPencil()).toBeInTheDocument(); + }); + + it('the pencil is keyboard-focusable (tabindex 0, focus lands on it)', async () => { + await renderSavedMention(); + const btn = await pencilElement(); + expect(btn.getAttribute('tabindex')).toBe('0'); + btn.focus(); + expect(document.activeElement).toBe(btn); + }); + + it('hides the pencil by default and reveals it on hover + focus-within (opacity swap)', async () => { + await renderSavedMention(); + const btn = await pencilElement(); + // Reveal is a class-driven instant opacity swap (no Tailwind CSS in the + // component test env — assert the mechanism structurally, as the existing + // touch-target test does for min-h-[44px]). + expect(btn.className).toContain('opacity-0'); + expect(btn.className).toContain('group-hover/mention:opacity-100'); + expect(btn.className).toContain('group-focus-within/mention:opacity-100'); + }); + + it('renders the pencil in an always-present fixed-width slot (no reflow)', async () => { + await renderSavedMention(); + const slot = document.querySelector('.mention-edit-slot') as HTMLElement | null; + expect(slot).not.toBeNull(); + expect(slot!.className).toContain('w-4'); + }); + + // AC-2 ---------------------------------------------------------------------- + + it('opens the dropdown pre-filled with the stored displayName when the pencil is clicked', async () => { + mockFetchEmpty(); + await renderSavedMention(); + clickPencil(await pencilElement()); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toHaveValue('Aug'); + }); + }); + + it('opens the dropdown when the pencil is activated by keyboard (Enter)', async () => { + mockFetchEmpty(); + await renderSavedMention(); + const btn = await pencilElement(); + btn.focus(); + btn.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true }) + ); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toHaveValue('Aug'); + }); + }); + + // AC-3 ---------------------------------------------------------------------- + + it('relinks in place: picking a different person swaps data-person-id', async () => { + mockFetchWithPersons(); // AUGUSTE (p-aug) + ANNA (p-anna) + await renderSavedMention(); + clickPencil(await pencilElement()); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + await pickOption(/Anna Schmidt/); + + await vi.waitFor(() => { + const token = document.querySelector('[data-type="mention"]') as HTMLElement; + expect(token.getAttribute('data-person-id')).toBe('p-anna'); + }); + }); + + it('relink preserves the displayed token text exactly (only personId changes)', async () => { + mockFetchWithPersons(); + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + await pickOption(/Anna Schmidt/); + + await vi.waitFor(() => { + expect(host.snapshot.mentionedPersons).toEqual([{ personId: 'p-anna', displayName: 'Aug' }]); + }); + }); + + // Serializer regression — text byte-identical, only personId swapped -------- + + it('serializer regression: re-link keeps the text byte-identical, swaps only personId', async () => { + mockFetchWithPersons(); + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + await pickOption(/Anna Schmidt/); + + await vi.waitFor(() => { + expect(host.snapshot.value).toBe('@Aug '); + expect(host.snapshot.mentionedPersons).toEqual([{ personId: 'p-anna', displayName: 'Aug' }]); + }); + }); + + // AC-5 — edited-then-picked (highest value): personId updates, text invariant + + it('AC-5: editing the search input then picking sets the new personId', async () => { + mockFetchWithPersons(); + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toBeVisible(); + }); + + await page.getByRole('searchbox').fill('Anna'); + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + await pickOption(/Anna Schmidt/); + + await vi.waitFor(() => { + expect(host.snapshot.mentionedPersons[0].personId).toBe('p-anna'); + }); + }); + + it('AC-5: editing the search input then picking keeps the ORIGINAL displayName', async () => { + mockFetchWithPersons(); + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toBeVisible(); + }); + + await page.getByRole('searchbox').fill('Anna'); + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + await pickOption(/Anna Schmidt/); + + await vi.waitFor(() => { + // The #380 AC-1 invariant wins over the edited search input. + expect(host.snapshot.mentionedPersons[0].displayName).toBe('Aug'); + expect(host.snapshot.value).toBe('@Aug '); + expect(host.snapshot.value).not.toContain('@Anna'); + }); + }); +}); diff --git a/frontend/src/lib/shared/discussion/mentionNodeView.ts b/frontend/src/lib/shared/discussion/mentionNodeView.ts new file mode 100644 index 00000000..733dc8d5 --- /dev/null +++ b/frontend/src/lib/shared/discussion/mentionNodeView.ts @@ -0,0 +1,152 @@ +import type { Editor } from '@tiptap/core'; +import type { Node as ProseMirrorNode } from '@tiptap/pm/model'; +import { m } from '$lib/paraglide/messages.js'; + +const SVG_NS = 'http://www.w3.org/2000/svg'; + +/** + * Opens the re-edit dropdown for a saved @mention. The host editor supplies + * this; the NodeView calls it when the pencil is activated. + * + * @param getRect anchor for the dropdown — the mention token's on-screen rect + * @param displayName the stored display text, used to pre-fill the search input + * @param pos the mention node's document position, captured at open time + * so the eventual re-link targets exactly this node + */ +export type RelinkRequest = ( + getRect: () => DOMRect | null, + displayName: string, + pos: number +) => void; + +type NodeViewArgs = { + node: ProseMirrorNode; + editor: Editor; + getPos: () => number | undefined; +}; + +// Static developer markup — no user data reaches the DOM here, so building the +// glyph element-by-element (never innerHTML) keeps the "user text is textContent +// only" rule honest across the whole NodeView. +function buildPencilIcon(): SVGSVGElement { + const svg = document.createElementNS(SVG_NS, 'svg'); + svg.setAttribute('viewBox', '0 0 24 24'); + svg.setAttribute('fill', 'none'); + svg.setAttribute('stroke', 'currentColor'); + svg.setAttribute('stroke-width', '2'); + svg.setAttribute('aria-hidden', 'true'); + svg.setAttribute('class', 'h-4 w-4'); + const tip = document.createElementNS(SVG_NS, 'path'); + tip.setAttribute('d', 'M12 20h9'); + tip.setAttribute('stroke-linecap', 'round'); + const body = document.createElementNS(SVG_NS, 'path'); + body.setAttribute('d', 'M16.5 3.5a2.12 2.12 0 0 1 3 3L7 19l-4 1 1-4 12.5-12.5z'); + body.setAttribute('stroke-linejoin', 'round'); + svg.append(tip, body); + return svg; +} + +/** + * Tiptap NodeView for a person @mention. Renders the `@displayName` token text + * (as `textContent` — the name is user-influenced, never `innerHTML`) plus a + * `contenteditable="false"` pencil button that opens the re-edit dropdown (#628). + * + * The pencil sits in a fixed-width slot revealed on whole-token hover and + * keyboard focus — an instant opacity swap (respects prefers-reduced-motion), + * so following words never shift and the caret model stays stable. + */ +export function createMentionNodeView(requestRelink: RelinkRequest) { + return ({ node, editor, getPos }: NodeViewArgs) => { + const dom = document.createElement('span'); + dom.className = 'mention-token group/mention'; + dom.setAttribute('data-type', 'mention'); + dom.setAttribute('data-person-id', node.attrs.personId ?? ''); + dom.setAttribute('data-display-name', node.attrs.displayName ?? ''); + + const text = document.createElement('span'); + text.className = 'underline decoration-ink/50 underline-offset-2 text-brand-navy font-medium'; + text.textContent = `@${node.attrs.displayName}`; + dom.appendChild(text); + + // Fixed-width slot — always present so revealing the pencil never reflows + // the following text (NFR no-reflow). Only opacity changes on reveal. + const slot = document.createElement('span'); + slot.className = + 'mention-edit-slot ml-0.5 inline-flex w-4 shrink-0 items-center justify-center align-middle'; + + const button = document.createElement('button'); + button.type = 'button'; + button.contentEditable = 'false'; + button.tabIndex = 0; + button.setAttribute('aria-label', m.person_mention_edit_label()); + // Visible glyph stays ~16px; the 44px tap target comes from padding pulled + // back with negative margin so the larger hit box does not reflow the line. + button.className = [ + 'mention-edit-btn', + '-mx-3 -my-2 inline-flex h-11 w-11 items-center justify-center rounded-sm text-ink-2', + 'opacity-0 transition-none', + 'group-hover/mention:opacity-100 group-focus-within/mention:opacity-100 focus:opacity-100', + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-brand-navy', + 'disabled:cursor-not-allowed' + ].join(' '); + button.appendChild(buildPencilIcon()); + slot.appendChild(button); + dom.appendChild(slot); + + const openRelink = (event: Event) => { + event.preventDefault(); + event.stopPropagation(); + // AC-7: inert when the editor is disabled — independent of the wrapper's + // pointer-events-none, on both the pointer and the keyboard path. + if (!editor.isEditable) return; + const pos = getPos(); + if (pos === undefined) return; + requestRelink(() => text.getBoundingClientRect(), node.attrs.displayName ?? '', pos); + }; + + // Prevent mousedown from moving the editor selection before the click + // fires — keeps the captured pos valid. click + Enter/Space activate. + button.addEventListener('mousedown', (event) => event.preventDefault()); + button.addEventListener('click', openRelink); + button.addEventListener('keydown', (event) => { + if (event.key === 'Enter' || event.key === ' ') openRelink(event); + }); + + const syncEditable = () => { + const editable = editor.isEditable; + button.disabled = !editable; + if (editable) { + button.removeAttribute('aria-disabled'); + button.tabIndex = 0; + } else { + button.setAttribute('aria-disabled', 'true'); + button.tabIndex = -1; + } + }; + syncEditable(); + // setEditable (and any doc change) dispatches a transaction — re-sync the + // pencil's inert state so a mid-session disable flip takes effect. + editor.on('transaction', syncEditable); + + return { + dom, + // The mention is an atom leaf — its DOM is fully owned here, so PM must + // ignore our mutations (token textContent / disabled attr) rather than + // trying to read them back into the document. + ignoreMutation: () => true, + // Pencil events are ours; everything else (caret placement on the token) + // flows to ProseMirror. + stopEvent: (event: Event) => button.contains(event.target as Node), + update(updatedNode: ProseMirrorNode) { + if (updatedNode.type.name !== 'mention') return false; + text.textContent = `@${updatedNode.attrs.displayName}`; + dom.setAttribute('data-person-id', updatedNode.attrs.personId ?? ''); + dom.setAttribute('data-display-name', updatedNode.attrs.displayName ?? ''); + return true; + }, + destroy() { + editor.off('transaction', syncEditable); + } + }; + }; +} -- 2.49.1 From 9deaaae3e88c3a80110b8e12c2523718a5d80aa5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 19:40:37 +0200 Subject: [PATCH 4/9] feat(transcription): dismiss + keyboard-operate the re-edit dropdown (#628 AC-4/AC-9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a visible × dismiss control to MentionDropdown (shared by the fresh-@ and re-edit paths) and, for the re-edit path which has no Tiptap suggestion plugin to forward keys, focuses the search input on open and handles its own keyboard: Escape dismisses (AC-4), Arrow/Enter reuse the exported selection logic so the dropdown is navigable on its own (AC-9 parity with the fresh-@ dropdown). Both close paths (Escape + ×) leave the mention node attrs + text byte-identical (AC-4) — close() never touches the document. Controller wires ondismiss=close (+refocus editor) and focusOnMount only for the re-edit open. Co-Authored-By: Claude Opus 4.8 --- .../shared/discussion/MentionDropdown.svelte | 56 +++++++++++- .../discussion/PersonMentionEditor.svelte | 22 ++++- .../PersonMentionEditor.svelte.spec.ts | 89 +++++++++++++++++++ 3 files changed, 162 insertions(+), 5 deletions(-) diff --git a/frontend/src/lib/shared/discussion/MentionDropdown.svelte b/frontend/src/lib/shared/discussion/MentionDropdown.svelte index c647f434..48ee6db7 100644 --- a/frontend/src/lib/shared/discussion/MentionDropdown.svelte +++ b/frontend/src/lib/shared/discussion/MentionDropdown.svelte @@ -2,7 +2,7 @@ import type { components } from '$lib/generated/api'; // eslint-disable-next-line boundaries/dependencies -- mention dropdown needs person date formatting; extract to shared if it becomes reusable import { formatLifeDateRange } from '$lib/person/personLifeDates'; -import { untrack } from 'svelte'; +import { onMount, untrack } from 'svelte'; import { m } from '$lib/paraglide/messages.js'; // Layered defence cap on the @mention search query length (CWE-400 // amplification). The attribute below caps direct @@ -31,17 +31,42 @@ type DropdownState = { let { model, editorQuery = '', - onSearch = () => {} + onSearch = () => {}, + ondismiss = () => {}, + focusOnMount = false }: { model: DropdownState; /** Text typed after `@` in the host editor. Mirrors into the search input * until the user takes manual ownership by typing into the input itself. */ editorQuery?: string; onSearch?: (query: string) => void; + /** Closes the dropdown without touching the document — invoked by the visible + * × dismiss control and by Escape on the re-edit path (#628 AC-4). */ + ondismiss?: () => void; + /** Re-edit (#628) opens with the search field focused; the fresh-@ path keeps + * focus in the editor so typing flows to the contenteditable. */ + focusOnMount?: boolean; } = $props(); let searchQuery = $state(untrack(() => editorQuery.slice(0, MAX_QUERY_LENGTH))); let userHasEdited = $state(false); +let searchInput: HTMLInputElement; + +onMount(() => { + if (focusOnMount) searchInput?.focus(); +}); + +// Re-edit has no Tiptap suggestion plugin to forward keys, so the search input +// handles its own navigation: Escape dismisses (and is prevented from clearing +// the native search field), Arrow/Enter reuse the exported selection logic. +function handleSearchKeydown(event: KeyboardEvent) { + if (event.key === 'Escape') { + event.preventDefault(); + ondismiss(); + return; + } + if (onKeyDown(event)) event.preventDefault(); +} // Intent-revealing alias used by both the persistent aria-live announcer and // the visible empty-state copy. Folding the duplicated rule into one $derived @@ -184,6 +209,7 @@ function selectItem(item: Person) { { userHasEdited = true; }} + onkeydown={handleSearchKeydown} onmousedown={(e) => e.stopPropagation()} /> + +

{#if model.items.length === 0} - {isQueryEmpty ? m.person_mention_search_prompt() : m.person_mention_popup_empty()} + {#if editingDisplayName && !userHasEdited} + {m.person_mention_editing_announce({ displayName: editingDisplayName })} + {:else} + {isQueryEmpty ? m.person_mention_search_prompt() : m.person_mention_popup_empty()} + {/if} {:else if model.items.length === 1} {m.person_mention_results_count_singular()} {:else} diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte index 84df6b5d..274f97ce 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte @@ -82,7 +82,7 @@ type LooseRenderProps = { // and (#628) the pencil re-edit affordance — so at most one dropdown is ever // mounted (the AC-6 single-dropdown invariant). open() closes any prior // dropdown first; render() is a thin adapter over open()/update()/close(). -type OpenOptions = { focusOnMount?: boolean }; +type OpenOptions = { focusOnMount?: boolean; editingDisplayName?: string }; type MentionController = { open: (clientRect: RectGetter, query: string, commit: CommitFn, opts?: OpenOptions) => void; @@ -183,6 +183,7 @@ function createMentionController(): MentionController { model: dropdownState, ondismiss: dismiss, focusOnMount: opts.focusOnMount ?? false, + editingDisplayName: opts.editingDisplayName ?? '', // MentionDropdown reads `editorQuery` off the shared state proxy via // this getter — Svelte 5's mount() does not expose settable prop // accessors, so we route through the proxy (same pattern as items). @@ -235,7 +236,10 @@ function commitRelink(pos: number): CommitFn { // the stored displayName. function requestRelink(getRect: () => DOMRect | null, displayName: string, pos: number) { if (!editor || !editor.isEditable) return; - controller.open(getRect, displayName, commitRelink(pos), { focusOnMount: true }); + controller.open(getRect, displayName, commitRelink(pos), { + focusOnMount: true, + editingDisplayName: displayName + }); } onMount(() => { diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 479291b8..195da335 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -1309,3 +1309,27 @@ describe('PersonMentionEditor — #628 security', () => { }); }); }); + +// ─── #628 NFR a11y: editing context announced via the existing live region ─── + +describe('PersonMentionEditor — #628 editing announce', () => { + it('re-edit open announces the editing context through the single persistent live region', async () => { + mockFetchEmpty(); + renderHost({ value: '@Aug ', mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }] }); + await vi.waitFor(() => + expect(document.querySelector('[data-type="mention"] button')).not.toBeNull() + ); + (document.querySelector('[data-type="mention"] button') as HTMLElement).dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true }) + ); + + await vi.waitFor(() => { + const liveRegions = document.querySelectorAll('[role="listbox"] [aria-live="polite"]'); + // Exactly ONE live region (no second announcer — avoids double-announce). + expect(liveRegions.length).toBe(1); + expect(liveRegions[0].textContent ?? '').toContain( + m.person_mention_editing_announce({ displayName: 'Aug' }) + ); + }); + }); +}); -- 2.49.1 From a62a333d09d2bd25a3a25311618aa6b5a5005653 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 20:08:43 +0200 Subject: [PATCH 8/9] fix(transcription): harden re-edit pencil hit-testing + disable sync (#628 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the clean-agent review of PR #717: - C1: the hidden pencil was opacity-0 only, which still hit-tests; its 44px box overhangs adjacent text, so a click in the gap between two mentions could land on the invisible button and spuriously open the dropdown (AC-8 hole). Add pointer-events-none while hidden, re-enabled with the opacity reveal on hover/focus. - C2/N1: editor.setEditable() emits "update", not a ProseMirror transaction, so the NodeView's 'transaction' listener missed a mid-session disable flip (stale aria-disabled/tabindex; the comment was wrong). Listen on 'update' instead — which also skips selection-only changes, so it fires far less often. - N2: track the node across update() so the pencil opens with the live displayName (hardening; relink only swaps personId today). Tests: structural guard that the hidden pencil is pointer-events-none + reveals, and a mid-session disable-flip test (fixture gains an onReady setDisabled hook). Co-Authored-By: Claude Opus 4.8 --- .../PersonMentionEditor.svelte.spec.ts | 51 +++++++++++++++++++ .../PersonMentionEditor.test-fixture.svelte | 15 ++++-- .../lib/shared/discussion/mentionNodeView.ts | 29 ++++++++--- 3 files changed, 85 insertions(+), 10 deletions(-) diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 195da335..ae15e31d 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -1333,3 +1333,54 @@ describe('PersonMentionEditor — #628 editing announce', () => { }); }); }); + +// ─── #628 review fixes: hidden-pencil hit-testing + mid-session disable ─────── + +describe('PersonMentionEditor — #628 review fixes', () => { + it('keeps the hidden pencil out of hit-testing (pointer-events-none until revealed)', async () => { + // opacity-0 alone still hit-tests, and the 44px button overhangs adjacent + // text — so a click in the gap between mentions would otherwise land on the + // invisible pencil and open the dropdown (AC-8). It must be pointer-events-none + // while hidden and only become clickable together with the opacity reveal. + renderHost({ value: '@Aug ', mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }] }); + await vi.waitFor(() => + expect(document.querySelector('[data-type="mention"] button')).not.toBeNull() + ); + const btn = document.querySelector('[data-type="mention"] button') as HTMLElement; + expect(btn.className).toContain('pointer-events-none'); + expect(btn.className).toContain('group-hover/mention:pointer-events-auto'); + expect(btn.className).toContain('group-focus-within/mention:pointer-events-auto'); + }); + + it('re-syncs the pencil to inert when the editor is disabled mid-session', async () => { + // editor.setEditable() emits "update", not a transaction — the NodeView must + // listen on "update" or a runtime disable flip leaves the pencil stale. + let setDisabled!: (value: boolean) => void; + render(PersonMentionEditorHost, { + initialValue: '@Aug ', + initialMentions: [{ personId: 'p-aug', displayName: 'Aug' }], + disabled: false, + onChange: () => {}, + onReady: (api: { setDisabled: (value: boolean) => void }) => { + setDisabled = api.setDisabled; + } + }); + + await vi.waitFor(() => { + const btn = document.querySelector( + '[data-type="mention"] button' + ) as HTMLButtonElement | null; + expect(btn).not.toBeNull(); + expect(btn!.disabled).toBe(false); + }); + + setDisabled(true); + + await vi.waitFor(() => { + const btn = document.querySelector('[data-type="mention"] button') as HTMLButtonElement; + expect(btn.disabled).toBe(true); + expect(btn.getAttribute('aria-disabled')).toBe('true'); + expect(btn.tabIndex).toBe(-1); + }); + }); +}); diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.test-fixture.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.test-fixture.svelte index cdc31df0..d59daa42 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.test-fixture.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.test-fixture.svelte @@ -1,5 +1,5 @@