diff --git a/e2e/interpretation.spec.ts b/e2e/interpretation.spec.ts index e7a52cd56..f39d3647f 100644 --- a/e2e/interpretation.spec.ts +++ b/e2e/interpretation.spec.ts @@ -148,20 +148,22 @@ test.describe('Entry Filtering', () => { test('should expose customer/project/user filters defaulting to "all"', async ({ page }) => { // The five relation filters (customer, project, team, user, activity) are - // searchable comboboxes that each default to their "Alle" (value 0) entry. + // searchable comboboxes. Each defaults to its "Alle" (value 0) entry, which in + // the unified control shows as no value chip — an empty search field. const filters = page.locator('form.filter-bar .searchable-select'); expect(await filters.count()).toBeGreaterThanOrEqual(5); - await expect(filters.first().locator('.searchable-select-value')).toHaveText('Alle'); + await expect(filters.first().locator('.tag-list .tag')).toHaveCount(0); }); test('should keep the page functional after changing a filter', async ({ page }) => { - // Open the first relation combobox (customer) and pick a real option past the - // leading "Alle" entry — only when the seed data provides one. - await page.locator('form.filter-bar .searchable-select-trigger').first().click(); - await expect(page.locator('.combobox-input').first()).toBeVisible(); - const realOption = page.locator('.combobox-content .combobox-item').nth(1); - if (await realOption.isVisible()) { - await realOption.click(); + // Type into the first relation combobox (customer) to open + filter the list, + // then pick a real option — only when the seed data provides a match. Typing + // filters out the leading "Alle" entry, so the first result is a real option. + const firstCombo = page.locator('form.filter-bar .searchable-select').first(); + await firstCombo.locator('.combobox-input').pressSequentially('e', { delay: 30 }); + const option = firstCombo.locator('.combobox-item').first(); + if (await option.isVisible().catch(() => false)) { + await option.click(); } else { await page.keyboard.press('Escape'); } diff --git a/frontend/src/components/SearchableSelect.test.tsx b/frontend/src/components/SearchableSelect.test.tsx index de8f3107b..147129c68 100644 --- a/frontend/src/components/SearchableSelect.test.tsx +++ b/frontend/src/components/SearchableSelect.test.tsx @@ -50,12 +50,13 @@ describe('SearchableSelect (jsdom)', () => { await waitFor(() => expect(onChange).toHaveBeenCalledWith(8)) }) - it('single: shows the current value and the "all" entry clears it to 0', async () => { + it('single: shows the current value as a chip and the "all" entry clears it to 0', async () => { const onChange = vi.fn() render(() => ) - // The chosen value is shown in the (compact) control. - expect(screen.getByRole('button', { name: 'Customer' })).toHaveTextContent('Apollo') + // Single and multi share the chip layout, so the chosen value shows as its chip + // (the input beside it stays a bare search box). + expect(screen.getByRole('listitem')).toHaveTextContent('Apollo') fireEvent.click(screen.getByRole('button', { name: 'Customer' })) await waitFor(() => expect(screen.getAllByRole('option')).toHaveLength(4)) @@ -63,12 +64,13 @@ describe('SearchableSelect (jsdom)', () => { await waitFor(() => expect(onChange).toHaveBeenCalledWith(0)) }) - it('single: with no selection and no allLabel, shows the "none" placeholder', () => { + it('single: with no selection, shows an empty search control and no "all" entry', () => { render(() => ) - // No "all" option requested → the compact control falls back to the generic - // "none" placeholder rather than a blank control. + // Nothing chosen → no value chip, just the search input + ▾ trigger; and with no + // allLabel requested, the "all" entry is not offered. expect(screen.getByRole('button', { name: 'Customer' })).toBeInTheDocument() + expect(screen.queryByRole('listitem')).not.toBeInTheDocument() expect(screen.queryByText('All')).not.toBeInTheDocument() }) diff --git a/frontend/src/components/SearchableSelect.tsx b/frontend/src/components/SearchableSelect.tsx index f1113056c..9d9974b18 100644 --- a/frontend/src/components/SearchableSelect.tsx +++ b/frontend/src/components/SearchableSelect.tsx @@ -1,9 +1,8 @@ import { Combobox } from '@ark-ui/solid/combobox' import { createMemo, createSignal, For, type JSX, Show } from 'solid-js' -import { Portal } from 'solid-js/web' import type { NamedOption } from '../api/queries' -import { ComboboxContent, ComboSearchIcon, comboCollection, type ComboItem } from '../lib/comboboxParts' +import { ComboboxContent, comboCollection, type ComboItem } from '../lib/comboboxParts' import { m } from '../paraglide/messages.js' // The "all / none" pseudo-option (value 0) for an optional single select. A @@ -12,18 +11,14 @@ import { m } from '../paraglide/messages.js' const ALL_VALUE = '__all__' /** - * A searchable relation combobox for ordinary FORMS (not the admin grid), - * built on the same Ark UI Combobox primitive + combobox CSS as the inline-grid - * editor ({@link ChipSelect}), so it looks identical to the worklog table's cell - * editor — but with a plain controlled form API (value / onChange) instead of - * the grid's field/commit/cancel contract. + * A searchable relation combobox for ordinary FORMS (not the admin grid). * - * Two layouts share all the logic: - * - SINGLE: a compact control showing the chosen label + a ▾; the search field - * lives at the top of the (body-portalled) popup. An optional "all" (value 0) - * entry clears the selection. - * - MULTI: removable chips + a leading magnifier + the search input, all in the - * control; the popup lists the options. + * Single and multi share ONE layout so they read the same (a maintainer request): + * the editable input IS the search field (WAI-ARIA 1.2 combobox), the chosen + * value(s) sit as chips beside it, and a trailing ▾ opens the list. Single differs + * only in that its chip is not removable (pick another option, or the "all" entry, + * to change it) and selecting closes the popup. Opening always shows ALL options — + * the current selection highlights, it never pre-filters the list. * * Boundary: Ark deals in string[]; relation ids are carried verbatim as the * option's string value and converted back to numbers only in onChange. @@ -71,42 +66,40 @@ export function SearchableSelect(props: { }) const collection = createMemo(() => comboCollection(filteredItems())) - // Single-select control display: the chosen label, or the "all" label when - // nothing is chosen (value 0), falling back to a generic "none" placeholder. - const singleDisplay = (): string => { - const first = selectedValues()[0] - if (first !== undefined) { - return labelOf(first) - } - - return props.allLabel ?? m.app_select_none() - } - const removeValue = (value: string): void => { props.onChange(selectedValues().filter((current) => current !== value).map(Number)) } - const searchInput = (): JSX.Element => ( - { inputEl = element }} - class="combobox-input" - aria-label={props.label} - placeholder={isMulti() && selectedValues().length === 0 ? m.app_type_to_add() : m.app_type_to_search()} - /> - ) + // The placeholder is only an invitation to search — shown while nothing is chosen. + // Once a value is picked its chip carries the display, so the input reads as a + // bare search box. + const placeholder = (): string => { + if (selectedValues().length > 0) { + return '' + } + + return isMulti() ? m.app_type_to_add() : m.app_type_to_search() + } return (
{props.label}
- {/* Multi: a leading magnifier in the control. Single's search is in the popup. */} - - -
    - - {(value) => ( -
  • - {labelOf(value)} + {/* Announce selection changes — Ark's own live region only narrates the + highlighted option during navigation. */} + + + {/* The chosen value(s) as chips — multi: removable; single: one, non-removable. + Outside Root so they share the field's wrapping flex row with the + (display:contents) input + ▾. */} +
      + + {(value) => ( +
    • + {labelOf(value)} + -
      • - )} -       -
    - - - {/* Announce selection changes — Ark's own live region only narrates the - highlighted option during navigation. */} - + +
    • + )} +     +
setInputValue(details.inputValue)} + onInputValueChange={(details) => { + // Ark echoes a single-select's chosen label back into the input; suppress that + // so the input stays a bare (empty) search box and the value shows only as its + // chip. That also means reopening never pre-filters by the current selection + // (WAI-ARIA APG: the selection highlights, it does not filter). A real query — + // which differs from the chosen label — passes through untouched, so typing + // (even the keystroke that opens the list) filters normally. + const isSelectionEcho = !isMulti() && selectedValues().some((value) => labelOf(value) === details.inputValue) + setInputValue(isSelectionEcho ? '' : details.inputValue) + }} onOpenChange={(details) => { - // Single's search input lives in the popup — focus it once the popup - // mounts so typing starts immediately. Clear a stale filter on close. if (details.open) { requestAnimationFrame(() => { if (inputEl?.isConnected) inputEl.focus() }) } else { + // Drop a typed query on close so the next open starts fresh with all options. setInputValue('') } }} @@ -158,34 +153,27 @@ export function SearchableSelect(props: { closeOnSelect={!isMulti()} allowCustomValue={false} inputBehavior="autohighlight" - positioning={{ sameWidth: true, gutter: 2, flip: true, fitViewport: true }} + positioning={{ strategy: 'fixed', sameWidth: true, gutter: 2, flip: true, fitViewport: true }} > - {/* Multi: search input next to the chips. Single: a value + ▾ trigger. */} - - {singleDisplay()} - - - } - > - {searchInput()} - ▾ - + { inputEl = element }} + class="combobox-input" + aria-label={props.label} + aria-required={props.required === true ? 'true' : undefined} + placeholder={placeholder()} + /> + ▾ - {/* Body-portal so the popup floats above cards/dialogs and is never clipped. */} - - - - - + {/* Rendered inline (NOT body-portalled) with a fixed positioning strategy. The + popup stays in this field's DOM subtree, so when the field sits in a modal + dialog its focus trap can't inert the popup (a body-portalled popup is a + sibling of the dialog → inert → dead, the Billing-modal bug); `fixed` still + lets it break out of any card/table `overflow` and clamp to the viewport. */} + + +
diff --git a/frontend/src/lib/chipSelect.tsx b/frontend/src/lib/chipSelect.tsx index c4a02878b..0b781ad0e 100644 --- a/frontend/src/lib/chipSelect.tsx +++ b/frontend/src/lib/chipSelect.tsx @@ -313,7 +313,7 @@ export function ChipSelect(props: { focus-out logic so opening it doesn't save/close the row. */} - + diff --git a/frontend/src/lib/comboboxParts.tsx b/frontend/src/lib/comboboxParts.tsx index 7b4fe9b44..42c3c1256 100644 --- a/frontend/src/lib/comboboxParts.tsx +++ b/frontend/src/lib/comboboxParts.tsx @@ -26,21 +26,26 @@ export const ComboSearchIcon = (): JSX.Element => ( /** * The dropdown body shared by both comboboxes: an optional sticky search row at the - * top (single-select only — multi keeps its search inline in the control), the - * option list, and the empty state. Rendered inside the caller's `Combobox.Root` → - * `Portal` → `Combobox.Positioner`, so Ark's context resolves normally. + * top, the option list, and the empty state. Rendered inside the caller's + * `Combobox.Root` → `Portal` → `Combobox.Positioner`, so Ark's context resolves + * normally. + * + * `popupSearch` decides where the search lives, decoupled from `multiple`: the grid + * cell editor ({@link ChipSelect}) puts a single-select's search HERE (a narrow + * column can't host an inline field), while the form control ({@link SearchableSelect}) + * always searches inline and leaves this off. */ export function ComboboxContent(props: { items: ComboItem[] - multiple: boolean - searchInput: () => JSX.Element + popupSearch?: boolean + searchInput?: () => JSX.Element }): JSX.Element { return ( - +
- {props.searchInput()} + {props.searchInput?.()}
diff --git a/frontend/src/pages/Auswertung.test.tsx b/frontend/src/pages/Auswertung.test.tsx index b602bac4f..b9c23b1ac 100644 --- a/frontend/src/pages/Auswertung.test.tsx +++ b/frontend/src/pages/Auswertung.test.tsx @@ -63,8 +63,9 @@ describe('Auswertung', () => { await waitFor(() => expect(getByRole('rowheader', { name: 'ACME' })).toBeInTheDocument()) getJson.mockClear() - // Change a filter so the query key actually changes, then submit. - const ticket = container.querySelector('.filter-grid input[type=text]') as HTMLInputElement + // Change a filter so the query key actually changes, then submit. Exclude the + // OptionSelect search inputs (.combobox-input), which now precede the ticket field. + const ticket = container.querySelector('.filter-grid input[type=text]:not(.combobox-input)') as HTMLInputElement fireEvent.input(ticket, { target: { value: 'ABC-1' } }) fireEvent.click(getByRole('button', { name: 'Refresh' })) diff --git a/frontend/src/pages/WorklogSync.test.tsx b/frontend/src/pages/WorklogSync.test.tsx index 8eca36d82..acd70f1bc 100644 --- a/frontend/src/pages/WorklogSync.test.tsx +++ b/frontend/src/pages/WorklogSync.test.tsx @@ -204,7 +204,9 @@ describe('WorklogSync', () => { fireEvent.click(screen.getByRole('button', { name: 'Users' })) fireEvent.input(screen.getByRole('combobox', { name: 'Users' }), { target: { value: 'ali' } }) fireEvent.click(await screen.findByRole('option', { name: 'alice' })) - await waitFor(() => expect(screen.getByRole('listitem')).toHaveTextContent('alice')) + // Single-select fields (the trigger ticket system) now also show their value as a + // chip, so several listitems can be present — assert the alice chip is among them. + await waitFor(() => expect(screen.getAllByRole('listitem').some((li) => li.textContent?.includes('alice'))).toBe(true)) fireEvent.click(screen.getByRole('button', { name: 'Trigger a run' })) await waitFor(() => expect(createSyncRun).toHaveBeenCalledTimes(1)) diff --git a/frontend/src/styles/app.css b/frontend/src/styles/app.css index 6ca2a0c22..ccc39759a 100644 --- a/frontend/src/styles/app.css +++ b/frontend/src/styles/app.css @@ -778,6 +778,18 @@ main:has(.tracking) { outline-offset: -1px; } +/* Native markup visually; no markup change needed. */ + .stack-form .field-check { + align-items: center; + display: grid; + gap: 0.3rem 1.2rem; + grid-template-columns: 18em minmax(0, 22em); + } + + .stack-form .field-check > span { + grid-column: 1; + text-align: right; + } + + .stack-form .field-check > input { + grid-column: 2; + justify-self: start; + } + + /* Help-icon rows keep the compact inline pattern (checkbox + its ⓘ side by side), + so they opt out of the two-column grid. */ + .stack-form .field-check-row .field-check { + display: flex; + grid-template-columns: none; + } } /* ---- Settings: grouped preference cards ---- */ @@ -2575,40 +2628,6 @@ kbd { opacity: 0.6; } -/* SINGLE: the current value fills the row, a trailing ▾ opens the popup; the - whole control is the click target. */ -.searchable-select-trigger { - align-items: center; - background: none; - border: 0; - color: var(--color-text); - cursor: pointer; - display: flex; - flex: 1; - font: inherit; - gap: 0.3rem; - justify-content: space-between; - min-height: 40px; - padding: 0; - text-align: start; -} - -.searchable-select.is-disabled .searchable-select-trigger { - cursor: not-allowed; -} - -.searchable-select-value { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -} - -.combobox-trigger-glyph { - color: var(--color-accent); - flex: none; - font-size: 0.8rem; -} - /* MULTI: the cell IS the search field — surface fill + accent border + focus ring, with the magnifier, chips and input on one (wrapping) row. */ .data-table td[data-inline-editing] .chip-select.inline-tags { @@ -2681,6 +2700,14 @@ kbd { padding: 0 0.15rem; } +/* The FORM control's ▾ matches the native