feat(transcription): re-edit @mention via a pencil affordance (#628)
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 <noreply@anthropic.com>
This commit is contained in:
Marcel
@@ -9,6 +9,7 @@ import type { PersonMention } from '$lib/shared/types';
|
|||||||
import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer';
|
import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer';
|
||||||
import { debounce } from '$lib/shared/utils/debounce';
|
import { debounce } from '$lib/shared/utils/debounce';
|
||||||
import MentionDropdown from './MentionDropdown.svelte';
|
import MentionDropdown from './MentionDropdown.svelte';
|
||||||
|
import { createMentionNodeView } from './mentionNodeView';
|
||||||
import { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants';
|
import { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants';
|
||||||
|
|
||||||
type Person = components['schemas']['Person'];
|
type Person = components['schemas']['Person'];
|
||||||
@@ -190,6 +191,37 @@ function createMentionController(): MentionController {
|
|||||||
|
|
||||||
const controller = createMentionController();
|
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(() => {
|
onMount(() => {
|
||||||
// Custom Mention node: uses personId / displayName instead of the
|
// Custom Mention node: uses personId / displayName instead of the
|
||||||
// default id / label attribute names so the mentionSerializer can
|
// 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);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -771,3 +771,190 @@ describe('PersonMentionEditor — touch target', () => {
|
|||||||
expect(option.className).toContain('min-h-[44px]');
|
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<HTMLElement> {
|
||||||
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
152
frontend/src/lib/shared/discussion/mentionNodeView.ts
Normal file
152
frontend/src/lib/shared/discussion/mentionNodeView.ts
Normal file
@@ -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);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
};
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user