From 02c95b9cfc5ca629ea4da9e58eec8fd9b660d60e Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 19:33:53 +0200 Subject: [PATCH] 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); + } + }; + }; +}