Compare commits
38 Commits
feat/issue
...
feat/issue
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9764ada854 | ||
|
|
1757b01af1 | ||
|
|
021a0c6cb3 | ||
|
|
31e7d97c30 | ||
|
|
ca0d539972 | ||
|
|
27e8c96c49 | ||
|
|
b3d49b28d7 | ||
|
|
26f1aeaa9d | ||
|
|
1081f5d263 | ||
|
|
4f2880a61a | ||
|
|
e37351f5c2 | ||
|
|
332d81975f | ||
|
|
b5455066c9 | ||
|
|
2df46b71f3 | ||
|
|
34b6a8a220 | ||
|
|
b6b9235dd8 | ||
|
|
7603c8d936 | ||
|
|
a822479535 | ||
|
|
58358e845d | ||
|
|
fcd4a41ba1 | ||
|
|
b6bf24db60 | ||
|
|
44209048a2 | ||
|
|
f67f5330ce | ||
|
|
fb658e7647 | ||
|
|
7618558895 | ||
|
|
94f63c4550 | ||
|
|
8052131576 | ||
|
|
2556e7f5c8 | ||
|
|
ecc4d1aa67 | ||
|
|
896d34cfcd | ||
|
|
a4e184d939 | ||
|
|
e1b5c1b15c | ||
|
|
5099dfa424 | ||
|
|
d9be001f1f | ||
|
|
671d05acac | ||
|
|
25afed0d65 | ||
|
|
a026d8bb05 | ||
|
|
1746cdd161 |
@@ -106,6 +106,31 @@ export default defineConfig(
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
// Forbid test fixtures (*.test-fixture.svelte) from being imported by
|
||||
// production code. Tree-shaking keeps them out of the production bundle
|
||||
// today (no route reaches them), but a lint rule makes the boundary
|
||||
// explicit so an accidental autocomplete import in a route or component
|
||||
// fails fast. Test files (*.spec.ts / *.test.ts) and the fixtures
|
||||
// themselves are exempt — see the next block. Nora #2 on PR #629
|
||||
// round 3.
|
||||
files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js', '**/*.ts'],
|
||||
ignores: ['**/*.spec.ts', '**/*.test.ts', '**/*.test-fixture.svelte'],
|
||||
rules: {
|
||||
'no-restricted-imports': [
|
||||
'error',
|
||||
{
|
||||
patterns: [
|
||||
{
|
||||
group: ['**/*.test-fixture.svelte'],
|
||||
message:
|
||||
'Test fixtures (*.test-fixture.svelte) are test-only — do not import from production code. Tracked by #637.'
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
plugins: { boundaries },
|
||||
settings: {
|
||||
|
||||
@@ -445,8 +445,12 @@
|
||||
"person_mention_load_error": "Person konnte nicht geladen werden.",
|
||||
"person_mention_loading": "Lade Person…",
|
||||
"person_mention_popup_empty": "Keine Personen gefunden",
|
||||
"person_mention_search_label": "Person suchen",
|
||||
"person_mention_search_prompt": "Namen eingeben…",
|
||||
"person_mention_btn_label": "Person verlinken",
|
||||
"person_mention_create_new": "Neue Person anlegen",
|
||||
"person_mention_results_count_singular": "1 Person gefunden",
|
||||
"person_mention_results_count_plural": "{count} Personen gefunden",
|
||||
"transcription_editor_aria_label": "Transkriptionstext",
|
||||
"person_born_name_prefix": "geb.",
|
||||
"page_title_home": "Archiv",
|
||||
|
||||
@@ -445,8 +445,12 @@
|
||||
"person_mention_load_error": "Could not load person.",
|
||||
"person_mention_loading": "Loading person…",
|
||||
"person_mention_popup_empty": "No persons found",
|
||||
"person_mention_search_label": "Search for a person",
|
||||
"person_mention_search_prompt": "Enter a name…",
|
||||
"person_mention_btn_label": "Link person",
|
||||
"person_mention_create_new": "Create new person",
|
||||
"person_mention_results_count_singular": "1 person found",
|
||||
"person_mention_results_count_plural": "{count} persons found",
|
||||
"transcription_editor_aria_label": "Transcription text",
|
||||
"person_born_name_prefix": "née",
|
||||
"page_title_home": "Archive",
|
||||
|
||||
@@ -445,8 +445,12 @@
|
||||
"person_mention_load_error": "No se pudo cargar la persona.",
|
||||
"person_mention_loading": "Cargando persona…",
|
||||
"person_mention_popup_empty": "No se encontraron personas",
|
||||
"person_mention_search_label": "Buscar persona",
|
||||
"person_mention_search_prompt": "Escribe un nombre…",
|
||||
"person_mention_btn_label": "Vincular persona",
|
||||
"person_mention_create_new": "Crear nueva persona",
|
||||
"person_mention_results_count_singular": "1 persona encontrada",
|
||||
"person_mention_results_count_plural": "{count} personas encontradas",
|
||||
"transcription_editor_aria_label": "Texto de transcripción",
|
||||
"person_born_name_prefix": "n.",
|
||||
"page_title_home": "Archivo",
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page } from 'vitest/browser';
|
||||
import TranscriptionBlockHost from './TranscriptionBlock.test-host.svelte';
|
||||
import TranscriptionBlockHost from './TranscriptionBlock.test-fixture.svelte';
|
||||
import type { ConfirmService } from '$lib/shared/services/confirm.svelte.js';
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
@@ -2,7 +2,18 @@
|
||||
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 { m } from '$lib/paraglide/messages.js';
|
||||
// Layered defence cap on the @mention search query length (CWE-400
|
||||
// amplification). The <input maxlength> attribute below caps direct
|
||||
// user edits, but the editor-mirror path (Tiptap contenteditable -> mirror
|
||||
// $effect -> searchQuery) is not covered by `maxlength` since the
|
||||
// contenteditable has no such enforcement. Clipping at the mirror keeps
|
||||
// the cap honest from both paths. Tracked server-side separately.
|
||||
// Nora #1 on PR #629. Hoisted to mentionConstants.ts so the host editor
|
||||
// (PersonMentionEditor) can clip the inserted displayName to the same cap
|
||||
// — see Felix #3 on PR #629.
|
||||
import { MAX_QUERY_LENGTH } from './mentionConstants';
|
||||
|
||||
type Person = components['schemas']['Person'];
|
||||
|
||||
@@ -17,7 +28,41 @@ type DropdownState = {
|
||||
clientRect: (() => DOMRect | null) | null;
|
||||
};
|
||||
|
||||
let { model }: { model: DropdownState } = $props();
|
||||
let {
|
||||
model,
|
||||
editorQuery = '',
|
||||
onSearch = () => {}
|
||||
}: {
|
||||
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;
|
||||
} = $props();
|
||||
|
||||
let searchQuery = $state(untrack(() => editorQuery.slice(0, MAX_QUERY_LENGTH)));
|
||||
let userHasEdited = $state(false);
|
||||
|
||||
// Mirror the editor's typed text until the user takes ownership.
|
||||
//
|
||||
// Why `$state + $effect` (not `$derived`): `searchQuery` is also written by
|
||||
// `bind:value` on the <input> below, so it needs to be a mutable `$state`.
|
||||
// A `$derived` would be read-only and would clobber direct user edits on
|
||||
// every editor keystroke. The `userHasEdited` latch pins ownership once the
|
||||
// user types into the input. Felix #1 on PR #629.
|
||||
$effect(() => {
|
||||
if (!userHasEdited) {
|
||||
searchQuery = editorQuery.slice(0, MAX_QUERY_LENGTH);
|
||||
}
|
||||
});
|
||||
|
||||
// Fire onSearch whenever the effective query changes — covers both the
|
||||
// editor mirror and direct input edits. This is the only place onSearch
|
||||
// fires; when the dropdown is unmounted, the effect is disposed and no
|
||||
// further fetches occur.
|
||||
$effect(() => {
|
||||
onSearch(searchQuery);
|
||||
});
|
||||
|
||||
// highlightedIndex must be both writable (keyboard handler mutates it) and
|
||||
// reset when `items` changes (so it never points past the end of a new list).
|
||||
@@ -112,16 +157,70 @@ function selectItem(item: Person) {
|
||||
unauthenticated users.
|
||||
-->
|
||||
<div
|
||||
class="fixed z-50 w-72 overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
|
||||
class="fixed z-50 w-72 max-w-[calc(100vw-1rem)] overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
|
||||
role="listbox"
|
||||
aria-label={m.person_mention_btn_label()}
|
||||
style:top={position.top}
|
||||
style:bottom={position.bottom}
|
||||
style:left={position.left}
|
||||
>
|
||||
<div class="border-b border-line px-3 py-2">
|
||||
<label class="sr-only" for="mention-search">{m.person_mention_search_label()}</label>
|
||||
<div class="flex items-center gap-2">
|
||||
<svg
|
||||
aria-hidden="true"
|
||||
viewBox="0 0 24 24"
|
||||
fill="none"
|
||||
stroke="currentColor"
|
||||
stroke-width="2"
|
||||
class="h-5 w-5 shrink-0 text-ink-2"
|
||||
>
|
||||
<circle cx="11" cy="11" r="7" />
|
||||
<path d="m20 20-3.5-3.5" stroke-linecap="round" />
|
||||
</svg>
|
||||
<input
|
||||
id="mention-search"
|
||||
type="search"
|
||||
data-test-search-input
|
||||
maxlength={MAX_QUERY_LENGTH}
|
||||
class="min-h-[44px] w-full bg-transparent font-sans text-base text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset"
|
||||
placeholder={m.person_mention_search_prompt()}
|
||||
bind:value={searchQuery}
|
||||
oninput={() => {
|
||||
userHasEdited = true;
|
||||
}}
|
||||
onmousedown={(e) => e.stopPropagation()}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<!--
|
||||
Persistent aria-live region — lives ABOVE the conditional branches so the
|
||||
element never unmounts when items transition between empty and populated.
|
||||
VoiceOver in particular swallows announcements from freshly-mounted live
|
||||
regions, and the previous (conditional-inside) markup silently dropped
|
||||
the "N persons found" announcement when results populated. Leonie #3 on
|
||||
PR #629 round 3.
|
||||
-->
|
||||
<p class="sr-only" aria-live="polite">
|
||||
{#if model.items.length === 0}
|
||||
{searchQuery.trim() === ''
|
||||
? m.person_mention_search_prompt()
|
||||
: m.person_mention_popup_empty()}
|
||||
{:else if model.items.length === 1}
|
||||
{m.person_mention_results_count_singular()}
|
||||
{:else}
|
||||
{m.person_mention_results_count_plural({ count: model.items.length })}
|
||||
{/if}
|
||||
</p>
|
||||
{#if model.items.length === 0}
|
||||
<!--
|
||||
Visible empty-state copy — visual-only. The persistent sr-only <p>
|
||||
above is the announcer. Leonie #3 on PR #629 round 3.
|
||||
-->
|
||||
<p class="px-3 py-2.5 font-sans text-sm text-ink-3">
|
||||
{m.person_mention_popup_empty()}
|
||||
{searchQuery.trim() === ''
|
||||
? m.person_mention_search_prompt()
|
||||
: m.person_mention_popup_empty()}
|
||||
</p>
|
||||
<!--
|
||||
Empty-state escape hatch — without it the transcriber has to close
|
||||
@@ -132,7 +231,7 @@ function selectItem(item: Person) {
|
||||
<a
|
||||
href="/persons/new"
|
||||
target="_blank"
|
||||
rel="noopener"
|
||||
rel="noopener noreferrer"
|
||||
class="flex min-h-[44px] items-center gap-2 border-t border-line px-3 py-2.5 font-sans text-sm font-medium text-brand-navy hover:bg-canvas focus:bg-canvas focus:outline-none"
|
||||
onmousedown={(e) => e.preventDefault()}
|
||||
>
|
||||
|
||||
@@ -1,22 +1,37 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page } from 'vitest/browser';
|
||||
import { page, userEvent } from 'vitest/browser';
|
||||
import { flushSync, mount, tick, unmount } from 'svelte';
|
||||
import MentionDropdown from './MentionDropdown.svelte';
|
||||
import MentionDropdownFixture from './MentionDropdown.test-fixture.svelte';
|
||||
import { m } from '$lib/paraglide/messages.js';
|
||||
import type { components } from '$lib/generated/api';
|
||||
|
||||
type Person = components['schemas']['Person'];
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
const makePerson = (id: string, name: string, overrides: Record<string, unknown> = {}) => ({
|
||||
id,
|
||||
firstName: name.split(' ')[0] ?? null,
|
||||
lastName: name.split(' ').slice(1).join(' ') || name,
|
||||
displayName: name,
|
||||
birthYear: null as number | null,
|
||||
deathYear: null as number | null,
|
||||
...overrides
|
||||
});
|
||||
const makePerson = (id: string, name: string, overrides: Partial<Person> = {}): Person => {
|
||||
const parts = name.split(' ');
|
||||
return {
|
||||
id,
|
||||
firstName: parts[0],
|
||||
lastName: parts.slice(1).join(' ') || name,
|
||||
displayName: name,
|
||||
personType: 'PERSON',
|
||||
familyMember: false,
|
||||
...overrides
|
||||
};
|
||||
};
|
||||
|
||||
const baseModel = (overrides: Record<string, unknown> = {}) => ({
|
||||
items: [] as ReturnType<typeof makePerson>[],
|
||||
type DropdownState = {
|
||||
items: Person[];
|
||||
command: (item: Person) => void;
|
||||
clientRect: (() => DOMRect | null) | null;
|
||||
};
|
||||
|
||||
const baseModel = (overrides: Partial<DropdownState> = {}): DropdownState => ({
|
||||
items: [],
|
||||
command: vi.fn(),
|
||||
clientRect: () => new DOMRect(100, 100, 0, 24),
|
||||
...overrides
|
||||
@@ -29,14 +44,32 @@ describe('MentionDropdown', () => {
|
||||
await expect.element(page.getByRole('listbox', { name: /person verlinken/i })).toBeVisible();
|
||||
});
|
||||
|
||||
it('renders the empty placeholder when items is empty', async () => {
|
||||
it('shows the "enter a name" prompt when the search field is empty', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
await expect.element(page.getByText('Keine Personen gefunden')).toBeVisible();
|
||||
// Scope to the visible empty-state <p> (text-ink-3) — the persistent
|
||||
// sr-only aria-live region above also contains the same prompt copy.
|
||||
const visibleEmptyP = document.querySelector(
|
||||
'[role="listbox"] p.text-ink-3'
|
||||
) as HTMLElement | null;
|
||||
expect(visibleEmptyP).not.toBeNull();
|
||||
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_search_prompt());
|
||||
expect(visibleEmptyP!.textContent ?? '').not.toContain(m.person_mention_popup_empty());
|
||||
});
|
||||
|
||||
it('shows "no persons found" when the search has a query but the list is empty', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'WdG' } });
|
||||
|
||||
const visibleEmptyP = document.querySelector(
|
||||
'[role="listbox"] p.text-ink-3'
|
||||
) as HTMLElement | null;
|
||||
expect(visibleEmptyP).not.toBeNull();
|
||||
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
|
||||
expect(visibleEmptyP!.textContent ?? '').not.toContain(m.person_mention_search_prompt());
|
||||
});
|
||||
|
||||
it('shows the create-new escape hatch link in the empty state', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'unknown' } });
|
||||
|
||||
const link = (await page
|
||||
.getByRole('link', { name: /neue person anlegen/i })
|
||||
@@ -44,6 +77,7 @@ describe('MentionDropdown', () => {
|
||||
expect(link.href).toContain('/persons/new');
|
||||
expect(link.target).toBe('_blank');
|
||||
expect(link.rel).toContain('noopener');
|
||||
expect(link.rel).toContain('noreferrer');
|
||||
});
|
||||
|
||||
it('renders one option per item when populated', async () => {
|
||||
@@ -104,3 +138,293 @@ describe('MentionDropdown', () => {
|
||||
expect(dropdown.style.left).toBe('123px');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Search input — Issue #380 ────────────────────────────────────────────────
|
||||
|
||||
describe('MentionDropdown — search input', () => {
|
||||
it('renders a search input pre-filled with the editorQuery prop', async () => {
|
||||
render(MentionDropdown, {
|
||||
props: { model: baseModel(), editorQuery: 'WdG' }
|
||||
});
|
||||
|
||||
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
|
||||
});
|
||||
|
||||
it('exposes a data-test-search-input attribute for E2E selectors', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const input = document.querySelector('[data-test-search-input]');
|
||||
expect(input).not.toBeNull();
|
||||
expect((input as HTMLInputElement).type).toBe('search');
|
||||
});
|
||||
|
||||
it('search input wrapper meets the 44px touch target (WCAG 2.2 AA)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const input = document.querySelector('[data-test-search-input]') as HTMLElement;
|
||||
expect(input).not.toBeNull();
|
||||
expect(input.className).toContain('min-h-[44px]');
|
||||
});
|
||||
|
||||
it('renders a persistent aria-live="polite" region (does not remount on items transition; Leonie #3 on PR #629)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const listbox = document.querySelector('[role="listbox"]');
|
||||
expect(listbox).not.toBeNull();
|
||||
const live = listbox!.querySelector('p[aria-live="polite"]');
|
||||
expect(live).not.toBeNull();
|
||||
// Empty + empty-query → "Namen eingeben…" prompt
|
||||
expect(live!.textContent ?? '').toContain(m.person_mention_search_prompt());
|
||||
});
|
||||
|
||||
it('announces the result count in the persistent live region when items populate (Leonie #3 on PR #629)', async () => {
|
||||
render(MentionDropdown, {
|
||||
props: {
|
||||
model: baseModel({
|
||||
items: [
|
||||
makePerson('p1', 'Anna Schmidt'),
|
||||
makePerson('p2', 'Bert Meier'),
|
||||
makePerson('p3', 'Carl Vogel')
|
||||
]
|
||||
})
|
||||
}
|
||||
});
|
||||
|
||||
const listbox = document.querySelector('[role="listbox"]');
|
||||
expect(listbox).not.toBeNull();
|
||||
const live = listbox!.querySelector('p[aria-live="polite"]');
|
||||
expect(live).not.toBeNull();
|
||||
// Populated → "3 Personen gefunden" (plural)
|
||||
expect(live!.textContent ?? '').toContain('3');
|
||||
});
|
||||
|
||||
it('keeps the visible empty-state copy without its own aria-live (the persistent live region announces; Leonie #3 on PR #629)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'WdG' } });
|
||||
|
||||
// Visible empty-state <p> exists with the empty-result copy ...
|
||||
const empty = document.querySelector('p.text-ink-3') as HTMLElement | null;
|
||||
expect(empty).not.toBeNull();
|
||||
expect(empty!.textContent ?? '').toContain(m.person_mention_popup_empty());
|
||||
// ... but it must NOT carry its own aria-live (the persistent sr-only
|
||||
// region above the conditional is the announcer now).
|
||||
expect(empty!.hasAttribute('aria-live')).toBe(false);
|
||||
});
|
||||
|
||||
it('renders the magnifier icon at h-5 w-5 with text-ink-2 (Leonie BLOCKER on PR #629)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const icon = document.querySelector('[data-test-search-input]')
|
||||
?.previousElementSibling as SVGElement | null;
|
||||
expect(icon).not.toBeNull();
|
||||
expect(icon!.tagName.toLowerCase()).toBe('svg');
|
||||
expect(icon!.getAttribute('class') ?? '').toContain('h-5');
|
||||
expect(icon!.getAttribute('class') ?? '').toContain('w-5');
|
||||
expect(icon!.getAttribute('class') ?? '').toContain('text-ink-2');
|
||||
});
|
||||
|
||||
it('caps the search input at maxlength=100 (CWE-400 amplification — Nora on PR #629)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
|
||||
expect(input).not.toBeNull();
|
||||
expect(input.maxLength).toBe(100);
|
||||
});
|
||||
|
||||
it('clips a long editorQuery mirror to 100 chars (CWE-400 layered — Nora #1 on PR #629)', async () => {
|
||||
const longQuery = 'A'.repeat(200);
|
||||
render(MentionDropdown, { props: { model: baseModel(), editorQuery: longQuery } });
|
||||
|
||||
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
|
||||
expect(input).not.toBeNull();
|
||||
expect(input.value.length).toBe(100);
|
||||
expect(input.value).toBe('A'.repeat(100));
|
||||
});
|
||||
|
||||
it('caps the listbox width to the viewport (320 px reflow guard — Leonie FINDING-MENTION-005)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const listbox = document.querySelector('[role="listbox"]') as HTMLElement;
|
||||
expect(listbox).not.toBeNull();
|
||||
expect(listbox.className).toContain('max-w-[calc(100vw-1rem)]');
|
||||
});
|
||||
|
||||
it('renders the @mention search input at text-base (16 px senior-audience floor — Leonie FINDING-MENTION-006)', async () => {
|
||||
render(MentionDropdown, { props: { model: baseModel() } });
|
||||
|
||||
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
|
||||
expect(input).not.toBeNull();
|
||||
expect(input.className).toContain('text-base');
|
||||
expect(input.className).not.toContain('text-sm');
|
||||
});
|
||||
|
||||
it('invokes onSearch with the current value whenever the user types', async () => {
|
||||
const onSearch = vi.fn();
|
||||
render(MentionDropdown, { props: { model: baseModel(), onSearch } });
|
||||
|
||||
await userEvent.type(page.getByRole('searchbox'), 'Walter');
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onSearch).toHaveBeenCalled();
|
||||
expect(onSearch).toHaveBeenLastCalledWith('Walter');
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps the user-edited search value when editorQuery changes after the takeover (Felix on PR #629)', async () => {
|
||||
let setEditorQuery!: (q: string) => void;
|
||||
render(MentionDropdownFixture, {
|
||||
model: baseModel(),
|
||||
initialEditorQuery: 'WdG',
|
||||
onReady: (s: (q: string) => void) => {
|
||||
setEditorQuery = s;
|
||||
}
|
||||
});
|
||||
|
||||
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
|
||||
|
||||
await page.getByRole('searchbox').fill('Walter');
|
||||
await expect.element(page.getByRole('searchbox')).toHaveValue('Walter');
|
||||
|
||||
setEditorQuery('WdGruyter');
|
||||
// Flush pending Svelte reactivity so any (non-)update from the mirror
|
||||
// $effect has landed before we assert. expect.element already polls, so
|
||||
// no fixed-timeout fallback is needed. Sara on PR #629 round 3.
|
||||
await tick();
|
||||
|
||||
await expect.element(page.getByRole('searchbox')).toHaveValue('Walter');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── ArrowDown via exported onKeyDown (Sara #3 on PR #629) ──────────────────
|
||||
//
|
||||
// In production, Tiptap intercepts ArrowDown/ArrowUp/Enter at the editor level
|
||||
// and forwards them to the dropdown via its exported onKeyDown(event) function
|
||||
// — the dropdown itself has no DOM keydown listener. This test exercises the
|
||||
// same export so a regression in highlightedIndex/selection logic is caught
|
||||
// at the unit level. The full E2E focus-chain test is deferred to a separate
|
||||
// issue (Playwright).
|
||||
//
|
||||
// These unit tests directly invoke the exported `onKeyDown` to pin its
|
||||
// behaviour in isolation. They do NOT exercise the Tiptap forwarding
|
||||
// chain (PersonMentionEditor.suggestion.render() returning { onKeyDown })
|
||||
// — that integration is covered by the 'ArrowDown moves the highlight'
|
||||
// test in PersonMentionEditor.svelte.spec.ts. Sara on PR #629 round 3.
|
||||
|
||||
describe('MentionDropdown — onKeyDown forwarding', () => {
|
||||
// flushSync ensures Svelte reactivity propagation completes before
|
||||
// asserting (uniform across all four key tests so the next reader
|
||||
// doesn't have to figure out why some are wrapped and others aren't).
|
||||
// Felix #1 suggestion on PR #629 round 3.
|
||||
|
||||
it('ArrowDown advances aria-selected to the next option in the listbox', async () => {
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const instance = mount(MentionDropdown, {
|
||||
target: container,
|
||||
props: {
|
||||
model: baseModel({
|
||||
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')]
|
||||
})
|
||||
}
|
||||
});
|
||||
try {
|
||||
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
|
||||
|
||||
// First option starts highlighted.
|
||||
const first = container.querySelector('[data-test-person-id="p1"]') as HTMLElement;
|
||||
const second = container.querySelector('[data-test-person-id="p2"]') as HTMLElement;
|
||||
expect(first.getAttribute('aria-selected')).toBe('true');
|
||||
expect(second.getAttribute('aria-selected')).toBe('false');
|
||||
|
||||
let consumed = false;
|
||||
flushSync(() => {
|
||||
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'ArrowDown' }));
|
||||
});
|
||||
expect(consumed).toBe(true);
|
||||
|
||||
expect(first.getAttribute('aria-selected')).toBe('false');
|
||||
expect(second.getAttribute('aria-selected')).toBe('true');
|
||||
} finally {
|
||||
unmount(instance);
|
||||
container.remove();
|
||||
}
|
||||
});
|
||||
|
||||
it('ArrowUp wraps from the first option to the last', async () => {
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const instance = mount(MentionDropdown, {
|
||||
target: container,
|
||||
props: {
|
||||
model: baseModel({
|
||||
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')]
|
||||
})
|
||||
}
|
||||
});
|
||||
try {
|
||||
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
|
||||
|
||||
let consumed = false;
|
||||
flushSync(() => {
|
||||
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'ArrowUp' }));
|
||||
});
|
||||
expect(consumed).toBe(true);
|
||||
|
||||
const second = container.querySelector('[data-test-person-id="p2"]') as HTMLElement;
|
||||
expect(second.getAttribute('aria-selected')).toBe('true');
|
||||
} finally {
|
||||
unmount(instance);
|
||||
container.remove();
|
||||
}
|
||||
});
|
||||
|
||||
it('Enter invokes model.command with the currently highlighted item', async () => {
|
||||
const command = vi.fn();
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const instance = mount(MentionDropdown, {
|
||||
target: container,
|
||||
props: {
|
||||
model: baseModel({
|
||||
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')],
|
||||
command
|
||||
})
|
||||
}
|
||||
});
|
||||
try {
|
||||
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
|
||||
|
||||
let consumed = false;
|
||||
flushSync(() => {
|
||||
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'Enter' }));
|
||||
});
|
||||
expect(consumed).toBe(true);
|
||||
expect(command).toHaveBeenCalledTimes(1);
|
||||
expect(command.mock.calls[0][0].id).toBe('p1');
|
||||
} finally {
|
||||
unmount(instance);
|
||||
container.remove();
|
||||
}
|
||||
});
|
||||
|
||||
it('Escape returns false so the suggestion plugin can handle it', async () => {
|
||||
const container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
const instance = mount(MentionDropdown, {
|
||||
target: container,
|
||||
props: {
|
||||
model: baseModel({ items: [makePerson('p1', 'Anna Schmidt')] })
|
||||
}
|
||||
});
|
||||
try {
|
||||
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
|
||||
let consumed = true;
|
||||
flushSync(() => {
|
||||
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
});
|
||||
expect(consumed).toBe(false);
|
||||
} finally {
|
||||
unmount(instance);
|
||||
container.remove();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
<script lang="ts">
|
||||
import { untrack } from 'svelte';
|
||||
import MentionDropdown from './MentionDropdown.svelte';
|
||||
import type { components } from '$lib/generated/api';
|
||||
|
||||
type Person = components['schemas']['Person'];
|
||||
type DropdownState = {
|
||||
items: Person[];
|
||||
command: (item: Person) => void;
|
||||
clientRect: (() => DOMRect | null) | null;
|
||||
};
|
||||
|
||||
type Props = {
|
||||
model: DropdownState;
|
||||
initialEditorQuery: string;
|
||||
/** Test hook: receives a setter for editorQuery so the test can mutate it. */
|
||||
onReady?: (setEditorQuery: (q: string) => void) => void;
|
||||
onSearch?: (q: string) => void;
|
||||
};
|
||||
|
||||
let { model, initialEditorQuery, onReady, onSearch = () => {} }: Props = $props();
|
||||
|
||||
let editorQuery = $state(untrack(() => initialEditorQuery));
|
||||
|
||||
$effect(() => {
|
||||
onReady?.((q) => {
|
||||
editorQuery = q;
|
||||
});
|
||||
});
|
||||
</script>
|
||||
|
||||
<MentionDropdown model={model} editorQuery={editorQuery} onSearch={onSearch} />
|
||||
@@ -7,7 +7,9 @@ import { m } from '$lib/paraglide/messages.js';
|
||||
import type { components } from '$lib/generated/api';
|
||||
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 { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants';
|
||||
|
||||
type Person = components['schemas']['Person'];
|
||||
|
||||
@@ -33,6 +35,13 @@ let {
|
||||
|
||||
let editorEl: HTMLDivElement;
|
||||
let editor: Editor | null = null;
|
||||
// Hoisted so onDestroy can guarantee the imperatively-mounted dropdown is
|
||||
// torn down even if Tiptap's suggestion plugin onExit didn't fire (e.g. when
|
||||
// the host component is unmounted while the dropdown is still open).
|
||||
let mountedDropdown: object | null = null;
|
||||
// Hoisted so onDestroy can cancel any pending fetch — otherwise a trailing
|
||||
// debounced search can fire after the editor is gone and pollute later tests.
|
||||
let cancelPendingSearch: (() => void) | null = null;
|
||||
|
||||
// Single reactive state object shared with MentionDropdown. Mutating these
|
||||
// fields propagates to the mounted dropdown via Svelte's $state proxy —
|
||||
@@ -42,10 +51,12 @@ let dropdownState = $state<{
|
||||
items: Person[];
|
||||
command: (item: Person) => void;
|
||||
clientRect: (() => DOMRect | null) | null;
|
||||
editorQuery: string;
|
||||
}>({
|
||||
items: [],
|
||||
command: () => {},
|
||||
clientRect: null
|
||||
clientRect: null,
|
||||
editorQuery: ''
|
||||
});
|
||||
|
||||
type DropdownExports = {
|
||||
@@ -138,16 +149,13 @@ onMount(() => {
|
||||
// Nora #5618 #3 — separate issue tracks the GET /api/persons
|
||||
// response-shape audit (PersonSummaryDTO leaks `notes`).
|
||||
// ─────────────────────────────────────────────────────────────
|
||||
items: async ({ query }: { query: string }) => {
|
||||
if (!query) return [];
|
||||
try {
|
||||
const res = await fetch(`/api/persons?q=${encodeURIComponent(query)}`);
|
||||
if (!res.ok) return [];
|
||||
return ((await res.json()) as Person[]).slice(0, 5);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
},
|
||||
// Tiptap's suggestion plugin requires an `items()` callback to keep
|
||||
// the dropdown alive, but the actual fetch is owned by `runSearch`
|
||||
// below — routed through the dropdown's search input via the
|
||||
// debounced `onSearch` channel. Returning `[]` here keeps Tiptap
|
||||
// happy without firing a duplicate per-keystroke fetch.
|
||||
// Markus #5616 / Felix / Nora / Sara on PR #629.
|
||||
items: async () => [],
|
||||
// AC-1 fix: insert the typed query as displayName, not person.displayName.
|
||||
command({ editor: ed, range, props }) {
|
||||
const p = props as unknown as { personId: string; displayName: string };
|
||||
@@ -165,7 +173,6 @@ onMount(() => {
|
||||
.run();
|
||||
},
|
||||
render() {
|
||||
let component: object | null = null;
|
||||
let exports: DropdownExports | null = null;
|
||||
|
||||
// Tiptap's SuggestionProps types `command` against the default
|
||||
@@ -178,25 +185,84 @@ onMount(() => {
|
||||
clientRect?: (() => DOMRect | null) | null;
|
||||
};
|
||||
|
||||
// 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?q=${encodeURIComponent(query)}&limit=${SEARCH_RESULT_LIMIT}`
|
||||
);
|
||||
if (id !== requestId) return;
|
||||
if (!res.ok) {
|
||||
dropdownState.items = [];
|
||||
return;
|
||||
}
|
||||
const data = (await res.json()) as Person[];
|
||||
if (id !== requestId) return;
|
||||
dropdownState.items = data.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) => {
|
||||
dropdownState.items = renderProps.items as Person[];
|
||||
// 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: renderProps.query
|
||||
displayName: clippedQuery
|
||||
});
|
||||
dropdownState.clientRect = renderProps.clientRect ?? null;
|
||||
dropdownState.editorQuery = clippedQuery;
|
||||
};
|
||||
|
||||
return {
|
||||
onStart(renderProps) {
|
||||
updateState(renderProps as unknown as LooseRenderProps);
|
||||
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 }
|
||||
props: {
|
||||
model: dropdownState,
|
||||
get editorQuery() {
|
||||
return dropdownState.editorQuery;
|
||||
},
|
||||
onSearch
|
||||
}
|
||||
});
|
||||
component = mounted as object;
|
||||
mountedDropdown = mounted as object;
|
||||
exports = mounted as unknown as DropdownExports;
|
||||
},
|
||||
onUpdate(renderProps) {
|
||||
@@ -208,9 +274,16 @@ onMount(() => {
|
||||
return exports?.onKeyDown(event) ?? false;
|
||||
},
|
||||
onExit() {
|
||||
if (component) {
|
||||
unmount(component);
|
||||
component = null;
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
@@ -253,7 +326,15 @@ onMount(() => {
|
||||
});
|
||||
|
||||
onDestroy(() => {
|
||||
cancelPendingSearch?.();
|
||||
editor?.destroy();
|
||||
// Tiptap suggestion onExit usually unmounts the dropdown, but if the host
|
||||
// component is destroyed while a suggestion is active the dropdown can
|
||||
// outlive the editor — clean it up explicitly.
|
||||
if (mountedDropdown) {
|
||||
unmount(mountedDropdown);
|
||||
mountedDropdown = null;
|
||||
}
|
||||
});
|
||||
|
||||
// Keep the data-placeholder attribute in sync with actual emptiness so the
|
||||
|
||||
@@ -8,29 +8,39 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page, userEvent } from 'vitest/browser';
|
||||
import PersonMentionEditorHost from './PersonMentionEditor.test-host.svelte';
|
||||
import PersonMentionEditorHost from './PersonMentionEditor.test-fixture.svelte';
|
||||
import type { components } from '$lib/generated/api';
|
||||
import { m } from '$lib/paraglide/messages.js';
|
||||
// Single source of truth for the debounce window — imported from the shared
|
||||
// module so the test cannot drift from production. Sara on PR #629 round 3.
|
||||
import { SEARCH_DEBOUNCE_MS } from './mentionConstants';
|
||||
|
||||
type Person = components['schemas']['Person'];
|
||||
type PersonMention = components['schemas']['PersonMention'];
|
||||
|
||||
// Slack on top of the debounce to absorb CI jitter (total 500 ms is generous).
|
||||
const POST_DEBOUNCE_SLACK_MS = 350;
|
||||
|
||||
const AUGUSTE: Person = {
|
||||
id: 'p-aug',
|
||||
firstName: 'Auguste',
|
||||
lastName: 'Raddatz',
|
||||
displayName: 'Auguste Raddatz',
|
||||
personType: 'PERSON',
|
||||
familyMember: false,
|
||||
birthYear: 1882,
|
||||
deathYear: 1944
|
||||
} as unknown as Person;
|
||||
};
|
||||
|
||||
const ANNA: Person = {
|
||||
id: 'p-anna',
|
||||
firstName: 'Anna',
|
||||
lastName: 'Schmidt',
|
||||
displayName: 'Anna Schmidt',
|
||||
personType: 'PERSON',
|
||||
familyMember: false,
|
||||
birthYear: 1860
|
||||
} as unknown as Person;
|
||||
};
|
||||
|
||||
function mockFetchWithPersons(persons: Person[] = [AUGUSTE, ANNA]) {
|
||||
vi.stubGlobal(
|
||||
@@ -125,6 +135,20 @@ describe('PersonMentionEditor — typeahead', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('appends &limit=5 to the fetch URL (defensive client-side cap, Markus on PR #629)', async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@Aug');
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('limit=5'));
|
||||
});
|
||||
});
|
||||
|
||||
it('shows life dates next to the name in the dropdown', async () => {
|
||||
mockFetchWithPersons();
|
||||
renderHost();
|
||||
@@ -142,8 +166,15 @@ describe('PersonMentionEditor — typeahead', () => {
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@xyz');
|
||||
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByText('Keine Personen gefunden')).toBeInTheDocument();
|
||||
// The visible empty-state <p> (text-ink-3) shows the copy. The persistent
|
||||
// sr-only aria-live region also contains the same copy, so we scope to the
|
||||
// visible element to avoid a multi-match resolution in expect.element.
|
||||
await vi.waitFor(() => {
|
||||
const visibleEmptyP = document.querySelector(
|
||||
'[role="listbox"] p.text-ink-3'
|
||||
) as HTMLElement | null;
|
||||
expect(visibleEmptyP).not.toBeNull();
|
||||
expect(visibleEmptyP!.textContent ?? '').toContain('Keine Personen gefunden');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -161,6 +192,248 @@ describe('PersonMentionEditor — typeahead', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── AC-2/3: search input drives the person fetch (debounced) ───────────────
|
||||
|
||||
describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => {
|
||||
it('editing the search input fires a debounced fetch with the new query', async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
// Open the dropdown so the search input is reachable.
|
||||
await userEvent.type(page.getByRole('textbox'), '@');
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByRole('searchbox')).toBeVisible();
|
||||
});
|
||||
|
||||
const fetchesBeforeSearch = fetchMock.mock.calls.length;
|
||||
|
||||
// `fill` simulates a single input event with the final value — sidesteps
|
||||
// per-keystroke timing of userEvent.type so the test can deterministically
|
||||
// assert that one input event collapses into one debounced fetch.
|
||||
await page.getByRole('searchbox').fill('Walter');
|
||||
|
||||
await vi.waitFor(
|
||||
() => {
|
||||
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('q=Walter'));
|
||||
},
|
||||
{ timeout: 1000 }
|
||||
);
|
||||
|
||||
const fetchesAfterSearch = fetchMock.mock.calls.length - fetchesBeforeSearch;
|
||||
expect(fetchesAfterSearch).toBe(1);
|
||||
});
|
||||
|
||||
it('fires exactly one /api/persons fetch when the user searches for Walter (regression guard)', async () => {
|
||||
// Regression guard: a previous version of PersonMentionEditor had a
|
||||
// duplicated `items()` callback in the Tiptap suggestion config that
|
||||
// fetched per-keystroke in addition to the debounced search-input fetch
|
||||
// (Markus & Felix round-1). To catch that regression, we must NOT
|
||||
// subtract any baseline — every fetch from render onwards counts.
|
||||
// Sara on PR #629 round 3.
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
// Open the dropdown, then drive the search input via fill() — sidesteps
|
||||
// per-keystroke timing of userEvent.type that Sara flagged round 2.
|
||||
await userEvent.type(page.getByRole('textbox'), '@');
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByRole('searchbox')).toBeVisible();
|
||||
});
|
||||
await page.getByRole('searchbox').fill('Walter');
|
||||
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
|
||||
// No baseline subtraction — count ALL /api/persons fetches since render.
|
||||
// If the legacy per-keystroke items() callback returns, typing `@` alone
|
||||
// would already produce one fetch and `fill('Walter')` another, breaking
|
||||
// this assertion.
|
||||
const personsFetches = fetchMock.mock.calls.filter(
|
||||
([url]) => typeof url === 'string' && url.startsWith('/api/persons')
|
||||
);
|
||||
expect(personsFetches.length).toBe(1);
|
||||
});
|
||||
|
||||
it('clearing the search input clears the list without firing a fetch', async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@Aug');
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByText('Auguste Raddatz')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const fetchesBeforeClear = fetchMock.mock.calls.length;
|
||||
|
||||
await userEvent.clear(page.getByRole('searchbox'));
|
||||
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
|
||||
expect(fetchMock.mock.calls.length).toBe(fetchesBeforeClear);
|
||||
await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Whitespace-only query (Elicit AC-4 ambiguity on PR #629) ───────────────
|
||||
|
||||
describe('PersonMentionEditor — whitespace-only query', () => {
|
||||
it('keeps the "Namen eingeben…" prompt and fires no fetch when @ is followed only by spaces', async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@ ');
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByRole('searchbox')).toBeVisible();
|
||||
});
|
||||
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
|
||||
// Scope to the visible empty-state <p> (text-ink-3) — the persistent
|
||||
// sr-only aria-live region above contains the same copy.
|
||||
const visibleEmptyP = document.querySelector(
|
||||
'[role="listbox"] p.text-ink-3'
|
||||
) as HTMLElement | null;
|
||||
expect(visibleEmptyP).not.toBeNull();
|
||||
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_search_prompt());
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Stale-response race (Sara on PR #629) ───────────────────────────────────
|
||||
|
||||
describe('PersonMentionEditor — stale-response race', () => {
|
||||
it('discards a stale response that resolves after the search has been cleared', async () => {
|
||||
let resolveFetch!: (v: { ok: boolean; json: () => Promise<Person[]> }) => void;
|
||||
const pendingResponse = new Promise<{ ok: boolean; json: () => Promise<Person[]> }>((r) => {
|
||||
resolveFetch = r;
|
||||
});
|
||||
const fetchMock = vi.fn().mockReturnValue(pendingResponse);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
// Open the dropdown and let the debounce fire so a fetch is in flight.
|
||||
await userEvent.type(page.getByRole('textbox'), '@Aug');
|
||||
await vi.waitFor(() => {
|
||||
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug'));
|
||||
});
|
||||
|
||||
// Clear the search input *before* the fetch resolves.
|
||||
await userEvent.clear(page.getByRole('searchbox'));
|
||||
await expect.element(page.getByRole('searchbox')).toHaveValue('');
|
||||
|
||||
// The stale fetch now resolves with persons. The dropdown must stay empty.
|
||||
resolveFetch({ ok: true, json: () => Promise.resolve([AUGUSTE]) });
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Server failure characterization (Sara #2 on PR #629) ───────────────────
|
||||
|
||||
describe('PersonMentionEditor — server failure', () => {
|
||||
it('on 500 response keeps the dropdown open with the empty-state copy (silent failure pinned; distinct error UX tracked separately)', async () => {
|
||||
const fetchMock = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ ok: false, status: 500, json: vi.fn().mockResolvedValue({}) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@Aug');
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
|
||||
// Pins current silent-failure behaviour. The day someone implements a
|
||||
// distinct error UX (toast / "Suche fehlgeschlagen" copy), this test
|
||||
// goes red and forces them to update the assertion. Scope to the
|
||||
// visible <p> (text-ink-3) — the persistent sr-only live region
|
||||
// above contains the same copy.
|
||||
const visibleEmptyP = document.querySelector(
|
||||
'[role="listbox"] p.text-ink-3'
|
||||
) as HTMLElement | null;
|
||||
expect(visibleEmptyP).not.toBeNull();
|
||||
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
|
||||
});
|
||||
|
||||
it('on a fetch reject (network failure) keeps the dropdown open with the empty-state copy', async () => {
|
||||
const fetchMock = vi.fn().mockRejectedValue(new TypeError('NetworkError'));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@Aug');
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
|
||||
const visibleEmptyP = document.querySelector(
|
||||
'[role="listbox"] p.text-ink-3'
|
||||
) as HTMLElement | null;
|
||||
expect(visibleEmptyP).not.toBeNull();
|
||||
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
|
||||
});
|
||||
});
|
||||
|
||||
// ─── onExit cancels pending debounce (Felix #1 on PR #629) ───────────────────
|
||||
|
||||
describe('PersonMentionEditor — onExit cancels pending debounce', () => {
|
||||
it('cancels the pending debounced fetch when Escape closes the dropdown before the debounce fires', async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) });
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
renderHost();
|
||||
|
||||
// Open the dropdown by typing @ + a query in the editor.
|
||||
await userEvent.type(page.getByRole('textbox'), '@A');
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByRole('searchbox')).toBeVisible();
|
||||
});
|
||||
|
||||
// Wait for any in-flight fetch from opening the dropdown to settle.
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
const fetchesBeforeEscape = fetchMock.mock.calls.length;
|
||||
|
||||
// Trigger a new debounced search (queues runSearch after 150 ms), then
|
||||
// immediately Escape *while focus is back in the editor* so Tiptap's
|
||||
// suggestion-plugin Escape handler fires onExit before the debounce.
|
||||
// Without onExit cancelling the pending debounce, runSearch executes
|
||||
// against the now-unmounted dropdown's state.
|
||||
await page.getByRole('searchbox').fill('Walter');
|
||||
// Focus the editor so the Escape lands on Tiptap's suggestion handler.
|
||||
(page.getByRole('textbox').element() as HTMLElement).focus();
|
||||
await userEvent.keyboard('{Escape}');
|
||||
|
||||
// Wait past the debounce window. If onExit did not cancel the pending
|
||||
// debounce, a fetch with q=Walter would still fire here.
|
||||
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
|
||||
|
||||
const newFetches = fetchMock.mock.calls.slice(fetchesBeforeEscape);
|
||||
const walterFetches = newFetches.filter(
|
||||
([url]) => typeof url === 'string' && url.includes('q=Walter')
|
||||
);
|
||||
expect(walterFetches.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── AC-1: search input prefilled with text typed after @ ───────────────────
|
||||
|
||||
describe('PersonMentionEditor — AC-1: search input prefill', () => {
|
||||
it('prefills the dropdown search input with the text typed after @', async () => {
|
||||
mockFetchEmpty();
|
||||
renderHost();
|
||||
|
||||
await userEvent.type(page.getByRole('textbox'), '@WdG');
|
||||
|
||||
await vi.waitFor(async () => {
|
||||
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ─── AC-1: typed text becomes displayName, not DB name ───────────────────────
|
||||
|
||||
describe('PersonMentionEditor — AC-1: typed text as displayName', () => {
|
||||
@@ -229,6 +502,36 @@ describe('PersonMentionEditor — AC-1: typed text as displayName', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('clips the inserted displayName to MAX_QUERY_LENGTH=100 chars (Felix #3 on PR #629)', async () => {
|
||||
// CWE-400 amplification: the dropdown clips its search input + mirror at
|
||||
// 100 chars (Nora #1), but the host editor was passing the unclipped
|
||||
// renderProps.query straight through to displayName — so a 105-char
|
||||
// @-suffix in the editor could insert a 105-char displayName into the
|
||||
// sidecar even though the dropdown only searched the first 100.
|
||||
mockFetchWithPersons();
|
||||
const host = renderHost();
|
||||
|
||||
// Type @ + 105 'A' chars in the contenteditable. The renderProps.query
|
||||
// fed into the command callback derives from the editor text after `@`,
|
||||
// not the dropdown's searchbox — so we must drive the editor.
|
||||
await userEvent.type(page.getByRole('textbox'), '@' + 'A'.repeat(105));
|
||||
|
||||
// The mocked /api/persons returns AUGUSTE for any query — wait for it.
|
||||
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).toHaveLength(1);
|
||||
expect(host.snapshot.mentionedPersons[0].displayName.length).toBeLessThanOrEqual(100);
|
||||
});
|
||||
});
|
||||
|
||||
it('does not duplicate the sidecar entry when the same person is selected twice', async () => {
|
||||
mockFetchWithPersons();
|
||||
const host = renderHost({
|
||||
|
||||
6
frontend/src/lib/shared/discussion/mentionConstants.ts
Normal file
6
frontend/src/lib/shared/discussion/mentionConstants.ts
Normal file
@@ -0,0 +1,6 @@
|
||||
/** Shared knobs for the @mention typeahead. Single source of truth for
|
||||
* the dropdown component and the host editor — keeps the layered length
|
||||
* cap and the debounce window consistent across both files. */
|
||||
export const MAX_QUERY_LENGTH = 100;
|
||||
export const SEARCH_DEBOUNCE_MS = 150;
|
||||
export const SEARCH_RESULT_LIMIT = 5;
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page, userEvent } from 'vitest/browser';
|
||||
import TestHost from './confirm.test-host.svelte';
|
||||
import TestHost from './confirm.test-fixture.svelte';
|
||||
import type { ConfirmService } from './confirm.svelte.js';
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
@@ -1,12 +1,25 @@
|
||||
/**
|
||||
* Returns a debounced version of fn that delays invocation until after
|
||||
* `delay` ms have elapsed since the last call.
|
||||
* `delay` ms have elapsed since the last call. The returned function
|
||||
* exposes a `cancel()` method that DROPS (does not flush) the pending
|
||||
* trailing invocation — essential when the host context (a destroyed
|
||||
* component, an unmounted editor) shouldn't fire the trailing call.
|
||||
*/
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
export function debounce<T extends (...args: any[]) => void>(fn: T, delay: number): T {
|
||||
let timer: ReturnType<typeof setTimeout>;
|
||||
return ((...args: Parameters<T>) => {
|
||||
clearTimeout(timer);
|
||||
export function debounce<T extends (...args: any[]) => void>(
|
||||
fn: T,
|
||||
delay: number
|
||||
): T & { cancel: () => void } {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const wrapped = ((...args: Parameters<T>) => {
|
||||
if (timer !== undefined) clearTimeout(timer);
|
||||
timer = setTimeout(() => fn(...args), delay);
|
||||
}) as T;
|
||||
}) as T & { cancel: () => void };
|
||||
wrapped.cancel = () => {
|
||||
if (timer !== undefined) {
|
||||
clearTimeout(timer);
|
||||
timer = undefined;
|
||||
}
|
||||
};
|
||||
return wrapped;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user