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", diff --git a/frontend/src/lib/shared/discussion/MentionDropdown.svelte b/frontend/src/lib/shared/discussion/MentionDropdown.svelte index c647f434..c1f42a68 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,47 @@ type DropdownState = { let { model, editorQuery = '', - onSearch = () => {} + onSearch = () => {}, + ondismiss = () => {}, + focusOnMount = false, + editingDisplayName = '' }: { 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; + /** When set (re-edit, #628), the persistent live region announces the editing + * context for this mention. Routed through the SAME aria-live region as the + * result count — never a second live region (avoids the double-announce bug). */ + editingDisplayName?: string; } = $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 +214,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 39261606..274f97ce 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']; @@ -63,6 +64,184 @@ 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 OpenOptions = { focusOnMount?: boolean; editingDisplayName?: string }; + +type MentionController = { + open: (clientRect: RectGetter, query: string, commit: CommitFn, opts?: OpenOptions) => 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); + }; + + // Close the dropdown without touching the document and return focus to the + // editor — wired to the dropdown's × control and the re-edit Escape path. + const dismiss = () => { + close(); + editor?.commands.focus(); + }; + + const open = ( + clientRect: RectGetter, + query: string, + commit: CommitFn, + opts: OpenOptions = {} + ) => { + // 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, + 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). + 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(); + +// 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), { + focusOnMount: true, + editingDisplayName: displayName + }); +} + onMount(() => { // Custom Mention node: uses personId / displayName instead of the // default id / label attribute names so the mentionSerializer can @@ -88,6 +267,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); } }); @@ -172,120 +357,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(); } }; } diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 164e9b32..ae15e31d 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -771,3 +771,616 @@ 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'); + }); + }); + + // AC-4 — dismiss leaves the node byte-identical on BOTH close paths --------- + + function dismissButton() { + return page.getByRole('button', { name: m.person_mention_dismiss_label() }); + } + + it('AC-4: Escape closes the re-edit dropdown leaving the node byte-identical', async () => { + mockFetchWithPersons(); + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toBeVisible(); + }); + + const search = (await page.getByRole('searchbox').element()) as HTMLElement; + search.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }) + ); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('listbox')).not.toBeInTheDocument(); + }); + expect(host.snapshot.value).toBe('@Aug '); + expect(host.snapshot.mentionedPersons).toEqual([{ personId: 'p-aug', displayName: 'Aug' }]); + }); + + it('AC-4: the visible dismiss control closes the re-edit dropdown leaving the node byte-identical', async () => { + mockFetchWithPersons(); + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + await expect.element(dismissButton()).toBeVisible(); + + const dismissEl = (await dismissButton().element()) as HTMLElement; + dismissEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('listbox')).not.toBeInTheDocument(); + }); + expect(host.snapshot.value).toBe('@Aug '); + expect(host.snapshot.mentionedPersons).toEqual([{ personId: 'p-aug', displayName: 'Aug' }]); + }); + + // AC-9 parity — the re-edit dropdown is keyboard-operable on its own -------- + + it('re-edit open focuses the search input so keyboard users land in the field', async () => { + mockFetchEmpty(); + await renderSavedMention(); + clickPencil(await pencilElement()); + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toBeVisible(); + }); + + const search = (await page.getByRole('searchbox').element()) as HTMLElement; + await vi.waitFor(() => { + expect(document.activeElement).toBe(search); + }); + }); + + it('re-edit dropdown is keyboard-navigable: ArrowDown then Enter relinks', async () => { + mockFetchWithPersons(); // [AUGUSTE (p-aug, highlighted), ANNA (p-anna)] + const host = await renderSavedMention(); + clickPencil(await pencilElement()); + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + + const search = (await page.getByRole('searchbox').element()) as HTMLElement; + search.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true, cancelable: true }) + ); + search.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true }) + ); + + await vi.waitFor(() => { + expect(host.snapshot.mentionedPersons[0].personId).toBe('p-anna'); + expect(host.snapshot.mentionedPersons[0].displayName).toBe('Aug'); + }); + }); + + // The × control is shared with the fresh-@ dropdown ------------------------ + + it('the fresh-@ dropdown also exposes the shared dismiss control', async () => { + mockFetchWithPersons(); + renderHost(); + await userEvent.type(page.getByRole('textbox'), '@Aug'); + await expect.element(dismissButton()).toBeVisible(); + }); +}); + +// ─── #628 AC-6: at most one mention dropdown open at a time ─────────────────── + +describe('PersonMentionEditor — #628 AC-6 single-dropdown invariant', () => { + const TWO_MENTIONS = [ + { personId: 'p-aug', displayName: 'Aug' }, + { personId: 'p-bert', displayName: 'Bert' } + ]; + + function mentionButtons(): HTMLButtonElement[] { + return Array.from( + document.querySelectorAll('[data-type="mention"] button') + ) as HTMLButtonElement[]; + } + + function click(el: HTMLElement) { + el.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + } + + function listboxCount() { + return document.querySelectorAll('[role="listbox"]').length; + } + + it('pencil → pencil: opening a second mention pencil closes the first', async () => { + mockFetchEmpty(); + renderHost({ value: '@Aug @Bert ', mentionedPersons: TWO_MENTIONS }); + await vi.waitFor(() => expect(mentionButtons().length).toBe(2)); + const [pencilA, pencilB] = mentionButtons(); + + click(pencilA); + await vi.waitFor(() => expect(listboxCount()).toBe(1)); + + click(pencilB); + await vi.waitFor(async () => { + expect(listboxCount()).toBe(1); + await expect.element(page.getByRole('searchbox')).toHaveValue('Bert'); + }); + }); + + it('fresh-@ → pencil: activating a pencil closes the open fresh-@ dropdown', async () => { + mockFetchEmpty(); + renderHost({ value: '@Aug ', mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }] }); + await vi.waitFor(() => expect(mentionButtons().length).toBe(1)); + + // Open a fresh-@ dropdown (prefilled with the typed query "x"). + await userEvent.type(page.getByRole('textbox'), '@x'); + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toHaveValue('x'); + }); + + // Now activate the saved mention's pencil — the fresh dropdown must give way. + click(mentionButtons()[0]); + await vi.waitFor(async () => { + expect(listboxCount()).toBe(1); + await expect.element(page.getByRole('searchbox')).toHaveValue('Aug'); + }); + }); + + it('pencil → fresh-@: typing @ in the editor closes the open re-edit dropdown', async () => { + mockFetchEmpty(); + renderHost({ value: '@Aug ', mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }] }); + await vi.waitFor(() => expect(mentionButtons().length).toBe(1)); + + click(mentionButtons()[0]); + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toHaveValue('Aug'); + }); + + // Typing @ in the editor starts a fresh suggestion — the re-edit dropdown + // must give way to it (single owner, both orderings). + await userEvent.type(page.getByRole('textbox'), '@y'); + await vi.waitFor(async () => { + expect(listboxCount()).toBe(1); + await expect.element(page.getByRole('searchbox')).toHaveValue('y'); + }); + }); + + it('regression: fresh-@ still inserts the typed text as displayName (#380 AC-1 intact)', async () => { + mockFetchWithPersons(); + const host = renderHost(); + await userEvent.type(page.getByRole('textbox'), '@Aug'); + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Auguste Raddatz/ })).toBeVisible(); + }); + const option = (await page + .getByRole('option', { name: /Auguste Raddatz/ }) + .element()) as HTMLElement; + option.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true })); + + await vi.waitFor(() => { + expect(host.snapshot.mentionedPersons).toEqual([{ personId: 'p-aug', displayName: 'Aug' }]); + }); + }); + + it('discards a stale fetch from a superseded open (open A → open B → A resolves)', async () => { + // A's fetch hangs; B's resolves empty. When A finally resolves with a + // person, the request-token guard must discard it so B's dropdown is not + // repopulated. Deterministic — no sleeps. + let resolveA!: (v: { ok: boolean; json: () => Promise<{ items: Person[] }> }) => void; + const pendingA = new Promise<{ ok: boolean; json: () => Promise<{ items: Person[] }> }>((r) => { + resolveA = r; + }); + let call = 0; + const fetchMock = vi.fn().mockImplementation(() => { + call += 1; + if (call === 1) return pendingA; + return Promise.resolve({ ok: true, json: () => Promise.resolve({ items: [] }) }); + }); + vi.stubGlobal('fetch', fetchMock); + + renderHost({ value: '@Aug @Bert ', mentionedPersons: TWO_MENTIONS }); + await vi.waitFor(() => expect(mentionButtons().length).toBe(2)); + const [pencilA, pencilB] = mentionButtons(); + + click(pencilA); + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('q=Aug')); + }); + + click(pencilB); + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('q=Bert')); + }); + + // A's stale response arrives last — it must NOT repopulate B's dropdown. + resolveA({ ok: true, json: () => Promise.resolve({ items: [AUGUSTE] }) }); + await tick(); + await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument(); + }); +}); + +// ─── #628 AC-7: pencil is inert when the editor is disabled (WCAG 2.1.1) ────── + +describe('PersonMentionEditor — #628 AC-7 disabled editor', () => { + function mentionButton(): HTMLButtonElement | null { + return document.querySelector('[data-type="mention"] button'); + } + + it('renders the pencil disabled, aria-disabled and out of tab order when disabled', async () => { + renderHost({ + value: '@Aug ', + mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }], + disabled: true + }); + await vi.waitFor(() => { + const btn = mentionButton(); + expect(btn).not.toBeNull(); + expect(btn!.disabled).toBe(true); + expect(btn!.getAttribute('aria-disabled')).toBe('true'); + expect(btn!.tabIndex).toBe(-1); + }); + }); + + it('activating the disabled pencil (keyboard or pointer) mounts no dropdown', async () => { + mockFetchWithPersons(); + renderHost({ + value: '@Aug ', + mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }], + disabled: true + }); + await vi.waitFor(() => expect(mentionButton()).not.toBeNull()); + const btn = mentionButton()!; + + btn.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, cancelable: true }) + ); + btn.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + // Give any (incorrectly) scheduled open a chance to mount before asserting. + await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS)); + + expect(document.querySelector('[role="listbox"]')).toBeNull(); + }); +}); + +// ─── #628 AC-8: no pencil / dropdown where there is no mention under the caret ─ + +describe('PersonMentionEditor — #628 AC-8 no mention under caret', () => { + it('plain text shows no edit pencil and no dropdown', async () => { + renderHost({ value: 'Nur Text ohne Erwaehnung', mentionedPersons: [] }); + await expect.element(page.getByRole('textbox')).toBeInTheDocument(); + expect(document.querySelector('[data-type="mention"]')).toBeNull(); + expect(document.querySelectorAll('[data-type="mention"] button').length).toBe(0); + expect(document.querySelector('[role="listbox"]')).toBeNull(); + }); + + it('two adjacent mentions each keep their own pencil and auto-open nothing', async () => { + renderHost({ + value: '@Aug@Bert ', + mentionedPersons: [ + { personId: 'p-aug', displayName: 'Aug' }, + { personId: 'p-bert', displayName: 'Bert' } + ] + }); + await vi.waitFor(() => + expect(document.querySelectorAll('[data-type="mention"]').length).toBe(2) + ); + // One pencil per token (no spurious pencil for the gap between them) ... + expect(document.querySelectorAll('[data-type="mention"] button').length).toBe(2); + // ... and nothing opens just from caret position. + expect(document.querySelector('[role="listbox"]')).toBeNull(); + }); + + it('a mention at the document start still renders its pencil', async () => { + renderHost({ value: '@Aug ', mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }] }); + await vi.waitFor(() => + expect(document.querySelector('[data-type="mention"] button')).not.toBeNull() + ); + }); +}); + +// ─── #628 security: query clip + personId provenance ────────────────────────── + +describe('PersonMentionEditor — #628 security', () => { + const OVERSIZED = 'A'.repeat(150); + + function mentionButton(): HTMLButtonElement { + return document.querySelector('[data-type="mention"] button') as HTMLButtonElement; + } + + it('clips an oversized stored displayName to MAX_QUERY_LENGTH in the search input, node text untouched', async () => { + mockFetchWithPersons(); + const host = renderHost({ + value: `@${OVERSIZED} `, + mentionedPersons: [{ personId: 'p-aug', displayName: OVERSIZED }] + }); + await vi.waitFor(() => expect(mentionButton()).not.toBeNull()); + mentionButton().dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('searchbox')).toBeVisible(); + }); + const search = (await page.getByRole('searchbox').element()) as HTMLInputElement; + expect(search.value.length).toBe(100); + // The preserved node text must NOT be truncated — only the search query is. + expect(host.snapshot.mentionedPersons[0].displayName.length).toBe(150); + }); + + it('re-link derives personId solely from the selected Person, never the DOM/search text', async () => { + mockFetchWithPersons(); // AUGUSTE (p-aug) + ANNA (p-anna) + const host = renderHost({ + value: `@${OVERSIZED} `, + mentionedPersons: [{ personId: 'p-aug', displayName: OVERSIZED }] + }); + await vi.waitFor(() => expect(mentionButton()).not.toBeNull()); + mentionButton().dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Anna Schmidt/ })).toBeVisible(); + }); + const anna = (await page + .getByRole('option', { name: /Anna Schmidt/ }) + .element()) as HTMLElement; + anna.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true })); + + await vi.waitFor(() => { + // id comes from the picked Person (p-anna), not the reflected p-aug + // nor the clipped search text; the long displayName stays untouched. + expect(host.snapshot.mentionedPersons[0].personId).toBe('p-anna'); + expect(host.snapshot.mentionedPersons[0].displayName.length).toBe(150); + }); + }); +}); + +// ─── #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' }) + ); + }); + }); +}); + +// ─── #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 @@