From 7977d22d0b87dadcc38eed067fcf581319228079 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 10 Jun 2026 07:53:56 +0200 Subject: [PATCH] =?UTF-8?q?fix(picker):=20honest=20combobox=20=E2=80=94=20?= =?UTF-8?q?keyboard=20navigation,=20listbox=20roles,=20no-results=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The input declared role=combobox + aria-autocomplete=list while arrow keys did nothing (WCAG 4.1.2). Wired the useTypeahead activeIndex the same way PersonTypeahead does: ArrowUp/Down cycle, Enter/Space select, Escape closes, aria-activedescendant tracks the active option; the list is a real listbox with option roles again (the interim role downgrade is reverted). Zero hits now render a 'Keine Treffer' row instead of silently vanishing, aria-expanded matches the visible state, and the hook sets loading at setQuery so the debounce window can't read as 'no results'. DocumentMultiSelect renders the shared error state too. _uid counter replaced with $props.id(). Co-Authored-By: Claude Fable 5 --- frontend/messages/de.json | 6 ++ frontend/messages/en.json | 6 ++ frontend/messages/es.json | 6 ++ .../lib/document/DocumentMultiSelect.svelte | 4 +- .../DocumentMultiSelect.svelte.spec.ts | 23 +++++ .../document/DocumentPickerDropdown.svelte | 64 ++++++++++++-- .../DocumentPickerDropdown.svelte.spec.ts | 83 +++++++++++++++++++ .../shared/hooks/useTypeahead.svelte.test.ts | 11 +++ .../lib/shared/hooks/useTypeahead.svelte.ts | 4 +- 9 files changed, 196 insertions(+), 11 deletions(-) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index e3f4a455..63c21073 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -302,6 +302,7 @@ "comp_multiselect_remove": "Entfernen", "comp_multiselect_loading": "Suche...", "comp_typeahead_error": "Suche fehlgeschlagen. Bitte versuchen Sie es erneut.", + "comp_typeahead_no_results": "Keine Treffer", "comp_taginput_placeholder_create": "Schlagworte hinzufügen...", "comp_taginput_placeholder_filter": "Nach Schlagworten filtern...", "comp_taginput_remove": "Schlagwort entfernen", @@ -1179,6 +1180,11 @@ "journey_selector_aria_live_hint": "Bitte wähle einen Typ aus, um fortzufahren.", "journey_add_document": "Brief hinzufügen", "journey_add_interlude": "Zwischentext hinzufügen", + "journey_interlude_label": "Zwischentext", + "journey_item_pending_remove": "wird entfernt…", + "journey_publish_disabled_hint": "Titel und mindestens ein Eintrag erforderlich.", + "journey_title_aria_label": "Titel der Lesereise", + "journey_intro_aria_label": "Einleitung der Lesereise", "journey_note_add": "Notiz hinzufügen", "journey_note_remove": "Notiz entfernen", "journey_note_save_hint": "Wird gespeichert, wenn du das Feld verlässt.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 7e55ccb2..9126f19a 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -302,6 +302,7 @@ "comp_multiselect_remove": "Remove", "comp_multiselect_loading": "Searching...", "comp_typeahead_error": "Search failed. Please try again.", + "comp_typeahead_no_results": "No matches", "comp_taginput_placeholder_create": "Add tags...", "comp_taginput_placeholder_filter": "Filter by tags...", "comp_taginput_remove": "Remove tag", @@ -1179,6 +1180,11 @@ "journey_selector_aria_live_hint": "Please select a type to continue.", "journey_add_document": "Add letter", "journey_add_interlude": "Add interlude", + "journey_interlude_label": "Interlude", + "journey_item_pending_remove": "removing…", + "journey_publish_disabled_hint": "A title and at least one entry are required.", + "journey_title_aria_label": "Title of the reading journey", + "journey_intro_aria_label": "Introduction of the reading journey", "journey_note_add": "Add note", "journey_note_remove": "Remove note", "journey_note_save_hint": "Saved when you leave the field.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 7c835331..34dd3ab9 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -302,6 +302,7 @@ "comp_multiselect_remove": "Eliminar", "comp_multiselect_loading": "Buscando...", "comp_typeahead_error": "La búsqueda falló. Inténtelo de nuevo.", + "comp_typeahead_no_results": "Sin resultados", "comp_taginput_placeholder_create": "Añadir etiquetas...", "comp_taginput_placeholder_filter": "Filtrar por etiquetas...", "comp_taginput_remove": "Eliminar etiqueta", @@ -1179,6 +1180,11 @@ "journey_selector_aria_live_hint": "Por favor, selecciona un tipo para continuar.", "journey_add_document": "Añadir carta", "journey_add_interlude": "Añadir interludio", + "journey_interlude_label": "Interludio", + "journey_item_pending_remove": "eliminando…", + "journey_publish_disabled_hint": "Se requieren un título y al menos una entrada.", + "journey_title_aria_label": "Título del viaje de lectura", + "journey_intro_aria_label": "Introducción del viaje de lectura", "journey_note_add": "Añadir nota", "journey_note_remove": "Eliminar nota", "journey_note_save_hint": "Se guarda al salir del campo.", diff --git a/frontend/src/lib/document/DocumentMultiSelect.svelte b/frontend/src/lib/document/DocumentMultiSelect.svelte index 8da29855..c95b9cff 100644 --- a/frontend/src/lib/document/DocumentMultiSelect.svelte +++ b/frontend/src/lib/document/DocumentMultiSelect.svelte @@ -100,13 +100,15 @@ function removeDocument(id: string | undefined) { /> - {#if picker.isOpen && (filteredResults.length > 0 || picker.loading)} + {#if picker.isOpen && (filteredResults.length > 0 || picker.loading || picker.error)}
{#if picker.loading}
{m.comp_multiselect_loading()}
+ {:else if picker.error} + {:else} {#each filteredResults as doc (doc.id)}
new Promise((r) => setTimeout(r, 350)); @@ -124,6 +125,28 @@ describe('DocumentMultiSelect — search and select', () => { }); }); +describe('DocumentMultiSelect — search failure', () => { + it('shows an error row when the search request fails instead of looking like "no results"', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: false, + status: 500, + json: vi.fn().mockResolvedValue({ code: 'INTERNAL_ERROR' }) + }) + ); + + render(DocumentMultiSelect); + + await userEvent.fill(page.getByPlaceholder('Dokument suchen…'), 'Eug'); + await waitForDebounce(); + + const alert = page.getByRole('alert'); + await expect.element(alert).toBeInTheDocument(); + await expect.element(alert).toHaveTextContent(m.comp_typeahead_error()); + }); +}); + describe('DocumentMultiSelect — remove', () => { it('removes a chip when its × button is clicked', async () => { render(DocumentMultiSelect, { diff --git a/frontend/src/lib/document/DocumentPickerDropdown.svelte b/frontend/src/lib/document/DocumentPickerDropdown.svelte index 951f79b9..c55f6cc0 100644 --- a/frontend/src/lib/document/DocumentPickerDropdown.svelte +++ b/frontend/src/lib/document/DocumentPickerDropdown.svelte @@ -1,7 +1,3 @@ - -
picker.close()} class="relative"> @@ -53,34 +90,43 @@ function handleSelect(doc: DocumentOption) { role="combobox" autocomplete="off" aria-label={placeholder} - aria-expanded={picker.isOpen && picker.results.length > 0} + aria-expanded={picker.isOpen} aria-controls={listboxId} aria-autocomplete="list" + aria-activedescendant={activeOptionId} placeholder={placeholder} value={inputValue} oninput={handleInput} + onkeydown={handleKeydown} class="block w-full rounded border border-line bg-surface px-3 py-2 text-sm text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring" /> - {#if picker.isOpen && (picker.results.length > 0 || picker.loading || picker.error)} + {#if picker.isOpen}
    {#if picker.loading}
  • {m.comp_multiselect_loading()}
  • {:else if picker.error} + {:else if picker.results.length === 0} +
  • {m.comp_typeahead_no_results()}
  • {:else} - {#each picker.results as doc (doc.id)} + {#each picker.results as doc, i (doc.id)} {@const disabled = alreadyAddedIds.has(doc.id!)}
  • handleSelect(doc)} - onkeydown={(e) => e.key === 'Enter' && handleSelect(doc)} + onkeydown={(e) => handleOptionKeydown(e, doc)} tabindex={disabled ? -1 : 0} class={[ 'px-3 py-2 text-ink select-none', + i === picker.activeIndex ? 'bg-muted' : '', disabled ? 'cursor-default opacity-50' : 'cursor-pointer hover:bg-muted focus:bg-muted focus:outline-none' diff --git a/frontend/src/lib/document/DocumentPickerDropdown.svelte.spec.ts b/frontend/src/lib/document/DocumentPickerDropdown.svelte.spec.ts index 67ebdaef..046d5767 100644 --- a/frontend/src/lib/document/DocumentPickerDropdown.svelte.spec.ts +++ b/frontend/src/lib/document/DocumentPickerDropdown.svelte.spec.ts @@ -95,6 +95,89 @@ describe('DocumentPickerDropdown — selection', () => { }); }); +describe('DocumentPickerDropdown — keyboard navigation', () => { + it('selects the first option via ArrowDown then Enter', async () => { + const onSelect = vi.fn(); + mockSearchResponse([docFactory('d1', 'Brief von Eugenie'), docFactory('d2', 'Brief 2')]); + + render(DocumentPickerDropdown, { onSelect }); + + await userEvent.fill(page.getByRole('combobox'), 'Brief'); + await waitForDebounce(); + await userEvent.keyboard('{ArrowDown}'); + await userEvent.keyboard('{Enter}'); + + expect(onSelect).toHaveBeenCalledWith(expect.objectContaining({ id: 'd1' })); + }); + + it('does not select an aria-disabled option on Enter', async () => { + const onSelect = vi.fn(); + mockSearchResponse([docFactory('d1', 'Brief von Eugenie')]); + + render(DocumentPickerDropdown, { + alreadyAddedIds: new Set(['d1']), + onSelect + }); + + await userEvent.fill(page.getByRole('combobox'), 'Brief'); + await waitForDebounce(); + await userEvent.keyboard('{ArrowDown}'); + await userEvent.keyboard('{Enter}'); + + expect(onSelect).not.toHaveBeenCalled(); + }); + + it('closes the dropdown on Escape', async () => { + mockSearchResponse([docFactory('d1', 'Brief von Eugenie')]); + + render(DocumentPickerDropdown, { onSelect: vi.fn() }); + + await userEvent.fill(page.getByRole('combobox'), 'Brief'); + await waitForDebounce(); + await expect.element(page.getByRole('listbox')).toBeInTheDocument(); + + await userEvent.keyboard('{Escape}'); + + await expect.element(page.getByRole('listbox')).not.toBeInTheDocument(); + }); + + it('points aria-activedescendant at the active option', async () => { + mockSearchResponse([docFactory('d1', 'Brief von Eugenie'), docFactory('d2', 'Brief 2')]); + + render(DocumentPickerDropdown, { onSelect: vi.fn() }); + + const input = page.getByRole('combobox'); + await userEvent.fill(input, 'Brief'); + await waitForDebounce(); + + expect(input.element().getAttribute('aria-activedescendant')).toBeNull(); + + await userEvent.keyboard('{ArrowDown}'); + + const activeId = input.element().getAttribute('aria-activedescendant'); + expect(activeId).toMatch(/-option-0$/); + const firstOption = page + .getByText(/Brief von Eugenie/i) + .element() + .closest('li')!; + expect(firstOption.id).toBe(activeId); + expect(firstOption.getAttribute('aria-selected')).toBe('true'); + }); +}); + +describe('DocumentPickerDropdown — no results', () => { + it('shows a non-interactive no-results row when the search returns zero hits', async () => { + mockSearchResponse([]); + + render(DocumentPickerDropdown, { onSelect: vi.fn() }); + + await userEvent.fill(page.getByRole('combobox'), 'xyz'); + await waitForDebounce(); + + await expect.element(page.getByText(m.comp_typeahead_no_results())).toBeInTheDocument(); + }); +}); + describe('DocumentPickerDropdown — search failure', () => { it('shows an error message when the search request fails instead of vanishing', async () => { // 500 from /api/documents/search — must surface, not render as "no results" diff --git a/frontend/src/lib/shared/hooks/useTypeahead.svelte.test.ts b/frontend/src/lib/shared/hooks/useTypeahead.svelte.test.ts index 7cdd05e9..89794289 100644 --- a/frontend/src/lib/shared/hooks/useTypeahead.svelte.test.ts +++ b/frontend/src/lib/shared/hooks/useTypeahead.svelte.test.ts @@ -106,6 +106,17 @@ describe('createTypeahead', () => { errorSpy.mockRestore(); }); + it('sets loading immediately on setQuery so empty results read as pending, not "no results"', async () => { + const fetchUrl = vi.fn().mockResolvedValue([]); + const ta = createTypeahead({ fetchUrl, debounceMs: 300 }); + ta.setQuery('foo'); + // During the debounce window no fetch has run yet — callers must be able to + // distinguish "still searching" from "searched, zero hits". + expect(ta.loading).toBe(true); + await vi.advanceTimersByTimeAsync(300); + expect(ta.loading).toBe(false); + }); + it('setActiveIndex updates activeIndex', () => { const ta = createTypeahead({ fetchUrl: vi.fn().mockResolvedValue([]) }); expect(ta.activeIndex).toBe(-1); diff --git a/frontend/src/lib/shared/hooks/useTypeahead.svelte.ts b/frontend/src/lib/shared/hooks/useTypeahead.svelte.ts index b132da2b..4f51abbe 100644 --- a/frontend/src/lib/shared/hooks/useTypeahead.svelte.ts +++ b/frontend/src/lib/shared/hooks/useTypeahead.svelte.ts @@ -19,9 +19,11 @@ export function createTypeahead(options: Options) { function setQuery(q: string) { query = q; isOpen = true; + // Set loading before the debounce fires so callers can distinguish + // "still searching" from "searched, zero hits" during the debounce window. + loading = true; clearTimeout(debounceTimer); debounceTimer = setTimeout(async () => { - loading = true; error = false; try { results = await fetchUrl(q);