bug(persons): person selection dropdown is visually clipped / cut off #343

Closed
opened 2026-04-26 19:56:36 +02:00 by marcel · 10 comments
Owner

Bug

The person dropdown (PersonTypeahead / PersonMultiSelect) is clipped by a parent container — list items below the fold are hidden and unreachable.

Steps to reproduce

  1. Open a form with a person selector (e.g. document edit or new document).
  2. Click the person field to open the dropdown.
  3. Observe: the dropdown list is cut off behind a parent container boundary.

Expected behavior

The dropdown overflows its container (via overflow: visible, absolute/fixed positioning, or a portal) so all options are visible and scrollable.

Acceptance criteria

  • Given the person dropdown is open, then all list items are visible and scrollable without clipping.
  • Fix works on desktop (≥ 1024 px) and tablet (768–1023 px).
  • No regression in other dropdowns (TagInput, etc.).
## Bug The person dropdown (PersonTypeahead / PersonMultiSelect) is clipped by a parent container — list items below the fold are hidden and unreachable. ## Steps to reproduce 1. Open a form with a person selector (e.g. document edit or new document). 2. Click the person field to open the dropdown. 3. Observe: the dropdown list is cut off behind a parent container boundary. ## Expected behavior The dropdown overflows its container (via `overflow: visible`, absolute/fixed positioning, or a portal) so all options are visible and scrollable. ## Acceptance criteria - Given the person dropdown is open, then all list items are visible and scrollable without clipping. - Fix works on desktop (≥ 1024 px) and tablet (768–1023 px). - No regression in other dropdowns (TagInput, etc.).
marcel added this to the Demo Day — family get-together milestone 2026-04-26 19:56:36 +02:00
marcel added the P1-highbugpersonui labels 2026-04-26 19:56:56 +02:00
Author
Owner

👨‍💻 Markus Keller — Application Architect

Observations

  • The root cause is a split-brain positioning strategy: PersonTypeahead uses position: absolute on a div.relative wrapper (CSS stacking context inside the card), while PersonMultiSelect has already solved this correctly — it uses fixed positioning computed via getBoundingClientRect(). The problem is that PersonTypeahead's dropdown sits inside the .rounded-sm.border.border-line.bg-surface.p-6 card container in WhoWhenSection.svelte, and anything that establishes a stacking context (even just shadow-sm combined with z- on a sibling) can clip it.

  • The fix is a pure CSS/positioning concern, not an architectural issue. No new abstraction, no portal library. The PersonMultiSelect approach (fixed + getBoundingClientRect) already exists in the codebase and works — PersonTypeahead just needs to adopt the same pattern.

  • The ConversationFilterBar.svelte defensively patches this with relative z-30 on each column wrapper and relative z-10 on the lower grid — a workaround, not a fix. This kind of scattered compensation code is the signal that the component itself needs to be corrected.

Recommendations

  • Align PersonTypeahead to use the same fixed-position approach as PersonMultiSelect: bind a ref to the input element, call getBoundingClientRect() on open, and set style="position:fixed;top:…;left:…;width:…" on the dropdown div. Add svelte:window listeners for scroll and resize (same as PersonMultiSelect).
  • Once PersonTypeahead is fixed, remove the z-30/z-10 z-index workarounds from ConversationFilterBar.svelte — they become dead code.
  • The acceptance criterion "no regression in other dropdowns (TagInput, etc.)" is important: TagInput uses position: absolute within a relative wrapper inside the chip row's inner div.relative.min-w-[120px]. That inner relative is correctly scoped below any card-level clipping context, so TagInput is likely unaffected — but verify visually.
## 👨‍💻 Markus Keller — Application Architect ### Observations - The root cause is a split-brain positioning strategy: `PersonTypeahead` uses `position: absolute` on a `div.relative` wrapper (CSS stacking context inside the card), while `PersonMultiSelect` has already solved this correctly — it uses `fixed` positioning computed via `getBoundingClientRect()`. The problem is that `PersonTypeahead`'s dropdown sits inside the `.rounded-sm.border.border-line.bg-surface.p-6` card container in `WhoWhenSection.svelte`, and anything that establishes a stacking context (even just `shadow-sm` combined with `z-` on a sibling) can clip it. - The fix is a pure CSS/positioning concern, not an architectural issue. No new abstraction, no portal library. The `PersonMultiSelect` approach (`fixed` + `getBoundingClientRect`) already exists in the codebase and works — `PersonTypeahead` just needs to adopt the same pattern. - The `ConversationFilterBar.svelte` defensively patches this with `relative z-30` on each column wrapper and `relative z-10` on the lower grid — a workaround, not a fix. This kind of scattered compensation code is the signal that the component itself needs to be corrected. ### Recommendations - Align `PersonTypeahead` to use the same fixed-position approach as `PersonMultiSelect`: bind a ref to the input element, call `getBoundingClientRect()` on open, and set `style="position:fixed;top:…;left:…;width:…"` on the dropdown `div`. Add `svelte:window` listeners for scroll and resize (same as `PersonMultiSelect`). - Once `PersonTypeahead` is fixed, remove the `z-30`/`z-10` z-index workarounds from `ConversationFilterBar.svelte` — they become dead code. - The acceptance criterion "no regression in other dropdowns (`TagInput`, etc.)" is important: `TagInput` uses `position: absolute` within a `relative` wrapper inside the chip row's inner `div.relative.min-w-[120px]`. That inner `relative` is correctly scoped below any card-level clipping context, so `TagInput` is likely unaffected — but verify visually.
Author
Owner

💻 Felix Brandt — Fullstack Developer

Observations

  • PersonTypeahead (line 144–167): The dropdown uses class="absolute top-full left-0 z-50 …". The outer wrapper <div class="relative"> is the positioning ancestor. When any ancestor element in the DOM tree creates a new stacking context (via overflow, transform, will-change, or even combined position+z-index), the absolute dropdown is clipped to that ancestor's bounds. This is the confirmed root cause.

  • PersonMultiSelect (lines 21–25 + 107–111): Already solved the same problem correctly. It computes dropdownStyle via getBoundingClientRect() and uses position:fixed, escaping any CSS stacking context. It also registers svelte:window onscroll and onresize handlers to keep the position current.

  • Inconsistency: Two components in the same component library use different positioning strategies for the same visual pattern. The PersonMultiSelect pattern is correct — apply it to PersonTypeahead.

  • ConversationFilterBar.svelte: The relative z-30 defensive wrapper on each PersonTypeahead column is a symptom of the bug, not a fix. It partially works for the filter bar's own stacking context but fails in other parent containers.

  • Code smell: PersonMultiSelect uses bare let state (let results, let showDropdown, let loading, let debounceTimer) alongside Svelte 5 runes. The debounce timer is a raw setTimeout with manual cleanup — no $effect cleanup. This is a minor technical debt but separate from this bug.

Recommendations

  • Add let inputEl: HTMLInputElement binding and let dropdownStyle = $state('') to PersonTypeahead (it doesn't have them yet).
  • Extract updateDropdownPosition() from PersonMultiSelect and replicate in PersonTypeahead — or better, extract it to a shared utility function computeFixedDropdownStyle(el: HTMLElement): string in $lib/utils/dom.ts.
  • Replace the absolute top-full left-0 classes on the dropdown div with style={dropdownStyle} and remove the positioning classes entirely.
  • Add <svelte:window onscroll={updateDropdownPosition} onresize={updateDropdownPosition} /> to PersonTypeahead.
  • Write a failing Playwright test first: open the person typeahead in document edit, verify the dropdown is visible (not clipped), then implement.
  • After the fix, delete the relative z-30 workarounds in ConversationFilterBar.svelte.
## 💻 Felix Brandt — Fullstack Developer ### Observations - **`PersonTypeahead`** (line 144–167): The dropdown uses `class="absolute top-full left-0 z-50 …"`. The outer wrapper `<div class="relative">` is the positioning ancestor. When any ancestor element in the DOM tree creates a new stacking context (via `overflow`, `transform`, `will-change`, or even combined `position`+`z-index`), the `absolute` dropdown is clipped to that ancestor's bounds. This is the confirmed root cause. - **`PersonMultiSelect`** (lines 21–25 + 107–111): Already solved the same problem correctly. It computes `dropdownStyle` via `getBoundingClientRect()` and uses `position:fixed`, escaping any CSS stacking context. It also registers `svelte:window onscroll` and `onresize` handlers to keep the position current. - **Inconsistency**: Two components in the same component library use different positioning strategies for the same visual pattern. The `PersonMultiSelect` pattern is correct — apply it to `PersonTypeahead`. - **`ConversationFilterBar.svelte`**: The `relative z-30` defensive wrapper on each `PersonTypeahead` column is a symptom of the bug, not a fix. It partially works for the filter bar's own stacking context but fails in other parent containers. - **Code smell**: `PersonMultiSelect` uses bare `let` state (`let results`, `let showDropdown`, `let loading`, `let debounceTimer`) alongside Svelte 5 runes. The debounce timer is a raw `setTimeout` with manual cleanup — no `$effect` cleanup. This is a minor technical debt but separate from this bug. ### Recommendations - Add `let inputEl: HTMLInputElement` binding and `let dropdownStyle = $state('')` to `PersonTypeahead` (it doesn't have them yet). - Extract `updateDropdownPosition()` from `PersonMultiSelect` and replicate in `PersonTypeahead` — or better, extract it to a shared utility function `computeFixedDropdownStyle(el: HTMLElement): string` in `$lib/utils/dom.ts`. - Replace the `absolute top-full left-0` classes on the dropdown `div` with `style={dropdownStyle}` and remove the positioning classes entirely. - Add `<svelte:window onscroll={updateDropdownPosition} onresize={updateDropdownPosition} />` to `PersonTypeahead`. - Write a failing Playwright test first: open the person typeahead in document edit, verify the dropdown is visible (not clipped), then implement. - After the fix, delete the `relative z-30` workarounds in `ConversationFilterBar.svelte`.
Author
Owner

🔒 Nora "NullX" Steiner — Security Engineer

Observations

  • This is a purely presentational bug. There are no new attack surfaces introduced by fixing it.
  • The fixed-position approach (copying PersonMultiSelect) renders the dropdown in a layer that escapes the card boundary — no DOM mutation, no innerHTML, no eval. The getBoundingClientRect() values are read-only geometry — no XSS vector.
  • PersonTypeahead already uses role="button" and tabindex="0" on each dropdown item. After the positioning fix, verify that keyboard Tab navigation and Enter selection still work correctly (they should be unaffected by the CSS change).
  • The fetch calls inside PersonTypeahead go to /api/persons?q=… with user-typed input passed as a URL-encoded query param — this is correct. The backend is responsible for parameterized query handling.
  • One smell: the dropdown items use role="button" but are <div> elements. This is not a security issue, but it is an accessibility gap (see UX persona).

Recommendations

  • No security-specific changes required for this fix. The PersonMultiSelect pattern to copy is already safe.
  • After the fix, confirm the clickOutside action still dismisses the dropdown correctly — a fixed-positioned dropdown that intercepts pointerdown events improperly could break the dismiss behavior, but the existing clickOutside action checks node.contains(event.target) which works regardless of CSS positioning.
  • Add a regression note: if a future refactor switches from fixed back to absolute, the clip bug returns silently. Consider a comment in the component explaining why fixed positioning is intentional.
## 🔒 Nora "NullX" Steiner — Security Engineer ### Observations - This is a purely presentational bug. There are no new attack surfaces introduced by fixing it. - The `fixed`-position approach (copying `PersonMultiSelect`) renders the dropdown in a layer that escapes the card boundary — no DOM mutation, no innerHTML, no eval. The `getBoundingClientRect()` values are read-only geometry — no XSS vector. - `PersonTypeahead` already uses `role="button"` and `tabindex="0"` on each dropdown item. After the positioning fix, verify that keyboard Tab navigation and `Enter` selection still work correctly (they should be unaffected by the CSS change). - The `fetch` calls inside `PersonTypeahead` go to `/api/persons?q=…` with user-typed input passed as a URL-encoded query param — this is correct. The backend is responsible for parameterized query handling. - One smell: the dropdown items use `role="button"` but are `<div>` elements. This is not a security issue, but it is an accessibility gap (see UX persona). ### Recommendations - No security-specific changes required for this fix. The `PersonMultiSelect` pattern to copy is already safe. - After the fix, confirm the `clickOutside` action still dismisses the dropdown correctly — a `fixed`-positioned dropdown that intercepts `pointerdown` events improperly could break the dismiss behavior, but the existing `clickOutside` action checks `node.contains(event.target)` which works regardless of CSS positioning. - Add a regression note: if a future refactor switches from `fixed` back to `absolute`, the clip bug returns silently. Consider a comment in the component explaining why `fixed` positioning is intentional.
Author
Owner

🧪 Sara Holt — QA Engineer

Observations

  • There are currently no Playwright E2E tests that verify the person dropdown renders unclipped. This bug was caught manually — it won't be caught automatically on regression.
  • The acceptance criteria are clear and testable: "all list items are visible and scrollable without clipping" on desktop (≥1024px) and tablet (768–1023px).
  • PersonTypeahead is used in six files: SearchFilterBar, PersonMergePanel, CorrespondenzHero, CorrespondenzPersonBar, ConversationFilterBar, and WhoWhenSection (document edit/new). The fix must be verified at each call site, at minimum at the two most critical ones (document edit form and the conversation filter bar).
  • PersonMultiSelect already uses fixed positioning — so that component is not affected and does not need regression tests for this specific bug. But it does lack keyboard interaction tests.

Recommendations

  • Write a Playwright test before the fix (red phase): in document edit (/documents/{id}/edit), click the sender typeahead, type a known name, and assert that the first suggestion li element is visible (not occluded by a sibling). Use page.locator('.person-dropdown-item').first().isVisible() or equivalent.
  • Add a second test at the 768px viewport width (tablet breakpoint) since the issue explicitly requires tablet coverage.
  • After the fix, keep both tests in the suite permanently. This is exactly the class of regression test that prevents a future CSS refactor from silently re-introducing the clip.
  • Test the clickOutside dismiss behavior after the fix: clicking outside the dropdown while it is in fixed position should still close it. This is a behavior edge case that fixed-positioned elements sometimes break.
  • Do not disable or skip the tests if the fix causes them to pass prematurely — confirm the red→green cycle by checking git history.

Open Decisions

  • Should the Playwright tests for the dropdown visibility live in an existing E2E spec file, or does the person selector warrant its own person-typeahead.spec.ts? This is a workflow question for the contributor.
## 🧪 Sara Holt — QA Engineer ### Observations - There are currently no Playwright E2E tests that verify the person dropdown renders unclipped. This bug was caught manually — it won't be caught automatically on regression. - The acceptance criteria are clear and testable: "all list items are visible and scrollable without clipping" on desktop (≥1024px) and tablet (768–1023px). - `PersonTypeahead` is used in six files: `SearchFilterBar`, `PersonMergePanel`, `CorrespondenzHero`, `CorrespondenzPersonBar`, `ConversationFilterBar`, and `WhoWhenSection` (document edit/new). The fix must be verified at each call site, at minimum at the two most critical ones (document edit form and the conversation filter bar). - `PersonMultiSelect` already uses `fixed` positioning — so that component is not affected and does not need regression tests for this specific bug. But it does lack keyboard interaction tests. ### Recommendations - Write a Playwright test **before** the fix (red phase): in document edit (`/documents/{id}/edit`), click the sender typeahead, type a known name, and assert that the first suggestion `li` element is `visible` (not occluded by a sibling). Use `page.locator('.person-dropdown-item').first().isVisible()` or equivalent. - Add a second test at the 768px viewport width (tablet breakpoint) since the issue explicitly requires tablet coverage. - After the fix, keep both tests in the suite permanently. This is exactly the class of regression test that prevents a future CSS refactor from silently re-introducing the clip. - Test the `clickOutside` dismiss behavior after the fix: clicking outside the dropdown while it is in `fixed` position should still close it. This is a behavior edge case that `fixed`-positioned elements sometimes break. - Do not disable or skip the tests if the fix causes them to pass prematurely — confirm the red→green cycle by checking git history. ### Open Decisions - Should the Playwright tests for the dropdown visibility live in an existing E2E spec file, or does the person selector warrant its own `person-typeahead.spec.ts`? This is a workflow question for the contributor.
Author
Owner

🎨 Leonie Voss — UX Design Lead

Observations

  • The clipping bug means that on document edit, the person picker's dropdown list is unreachable for users who cannot mouse outside the card boundary. For the 60+ transcribers using a laptop or tablet, this is a Critical blocker — they cannot select a sender without the dropdown being visible.

  • The dropdown items in PersonTypeahead use role="button" on <div> elements. The correct ARIA pattern for a combobox dropdown is role="listbox" on the container and role="option" on each item. TagInput already uses role="listbox" and role="option"PersonTypeahead should be aligned to the same pattern. PersonMultiSelect also uses <div role="button"> — both should be corrected.

  • There is no keyboard navigation (ArrowDown/ArrowUp) in PersonTypeahead. TagInput has full arrow-key navigation implemented. After fixing the visual bug, the keyboard experience remains incomplete. For the transcriber audience (often 60+, possibly using keyboard navigation), this is a High accessibility gap.

  • The PersonMultiSelect remove button has aria-label={m.comp_multiselect_remove()} — good. But the remove button class="ml-0.5 text-ink/50 hover:text-red-500 focus:outline-none" removes the focus ring entirely (focus:outline-none with no replacement). This fails WCAG 2.2 Focus Appearance.

  • Touch target on each dropdown item: py-2 gives ~8px top+bottom padding on text-sm (~20px line height) = ~36px total. WCAG 2.2 requires 24×24px minimum target area; 44px is the recommended threshold. At 36px this passes the minimum but misses the preferred threshold. For the 60+ audience, recommend py-3 (48px total).

Recommendations

  • P0 (this issue): Fix the positioning so the dropdown is visible. Copy PersonMultiSelect's fixed + getBoundingClientRect() pattern.
  • P1 (this PR): Change PersonTypeahead dropdown markup from div[role="button"] to a ul[role="listbox"] + li[role="option"] structure, matching TagInput. Add aria-expanded on the input and aria-activedescendant pointing to the active option.
  • P1 (this PR): Add ArrowDown/ArrowUp keyboard navigation to PersonTypeahead, matching TagInput's existing handleKeydown pattern.
  • P2 (follow-up): Replace focus:outline-none on the remove button in PersonMultiSelect with focus-visible:ring-2 focus-visible:ring-focus-ring. Same fix needed on the same button in TagInput.
  • P2 (follow-up): Increase dropdown item padding to py-3 in both PersonTypeahead and PersonMultiSelect for 48px touch targets.

Open Decisions

  • The PersonTypeahead input does not announce the dropdown open/close state to screen readers. Should this issue's scope include adding aria-expanded and aria-controls/aria-owns for a full combobox ARIA pattern, or is that a separate accessibility issue?
## 🎨 Leonie Voss — UX Design Lead ### Observations - The clipping bug means that on document edit, the person picker's dropdown list is unreachable for users who cannot mouse outside the card boundary. For the 60+ transcribers using a laptop or tablet, this is a **Critical** blocker — they cannot select a sender without the dropdown being visible. - The dropdown items in `PersonTypeahead` use `role="button"` on `<div>` elements. The correct ARIA pattern for a combobox dropdown is `role="listbox"` on the container and `role="option"` on each item. `TagInput` already uses `role="listbox"` and `role="option"` — `PersonTypeahead` should be aligned to the same pattern. `PersonMultiSelect` also uses `<div role="button">` — both should be corrected. - There is no keyboard navigation (ArrowDown/ArrowUp) in `PersonTypeahead`. `TagInput` has full arrow-key navigation implemented. After fixing the visual bug, the keyboard experience remains incomplete. For the transcriber audience (often 60+, possibly using keyboard navigation), this is a **High** accessibility gap. - The `PersonMultiSelect` remove button has `aria-label={m.comp_multiselect_remove()}` — good. But the remove button `class="ml-0.5 text-ink/50 hover:text-red-500 focus:outline-none"` removes the focus ring entirely (`focus:outline-none` with no replacement). This fails WCAG 2.2 Focus Appearance. - Touch target on each dropdown item: `py-2` gives ~8px top+bottom padding on `text-sm` (~20px line height) = ~36px total. WCAG 2.2 requires 24×24px minimum target area; 44px is the recommended threshold. At 36px this passes the minimum but misses the preferred threshold. For the 60+ audience, recommend `py-3` (48px total). ### Recommendations - **P0 (this issue)**: Fix the positioning so the dropdown is visible. Copy `PersonMultiSelect`'s `fixed` + `getBoundingClientRect()` pattern. - **P1 (this PR)**: Change `PersonTypeahead` dropdown markup from `div[role="button"]` to a `ul[role="listbox"]` + `li[role="option"]` structure, matching `TagInput`. Add `aria-expanded` on the input and `aria-activedescendant` pointing to the active option. - **P1 (this PR)**: Add ArrowDown/ArrowUp keyboard navigation to `PersonTypeahead`, matching `TagInput`'s existing `handleKeydown` pattern. - **P2 (follow-up)**: Replace `focus:outline-none` on the remove button in `PersonMultiSelect` with `focus-visible:ring-2 focus-visible:ring-focus-ring`. Same fix needed on the same button in `TagInput`. - **P2 (follow-up)**: Increase dropdown item padding to `py-3` in both `PersonTypeahead` and `PersonMultiSelect` for 48px touch targets. ### Open Decisions - The `PersonTypeahead` input does not announce the dropdown open/close state to screen readers. Should this issue's scope include adding `aria-expanded` and `aria-controls`/`aria-owns` for a full combobox ARIA pattern, or is that a separate accessibility issue?
Author
Owner

⚙️ Tobias Wendt — DevOps & Platform

Observations

  • This is a frontend CSS/JS positioning fix with zero infrastructure impact. No new containers, services, or build steps are required.
  • The fix involves only two component files (PersonTypeahead.svelte and optionally PersonMultiSelect.svelte alignment). The SvelteKit build pipeline handles this without changes.
  • If a Playwright E2E test is added (as Sara recommends), it will run in the existing CI pipeline. Check that the CI Playwright job runs against the correct viewport sizes — the default Chromium viewport in Playwright is 1280×720, which covers the desktop breakpoint. The tablet breakpoint (768px) requires an explicit page.setViewportSize({ width: 768, height: 1024 }) in the test.
  • No Flyway migration, no backend change, no Docker Compose change needed.

Recommendations

  • Confirm the Gitea Actions CI workflow runs npm run test:e2e as part of the merge check — if it does not, add it so the new regression tests actually gate the PR.
  • The fix does not change the OpenAPI spec or any API contract, so npm run generate:api is not needed.
  • After the fix ships, no deployment steps beyond the normal build+deploy are required.
## ⚙️ Tobias Wendt — DevOps & Platform ### Observations - This is a frontend CSS/JS positioning fix with zero infrastructure impact. No new containers, services, or build steps are required. - The fix involves only two component files (`PersonTypeahead.svelte` and optionally `PersonMultiSelect.svelte` alignment). The SvelteKit build pipeline handles this without changes. - If a Playwright E2E test is added (as Sara recommends), it will run in the existing CI pipeline. Check that the CI Playwright job runs against the correct viewport sizes — the default Chromium viewport in Playwright is 1280×720, which covers the desktop breakpoint. The tablet breakpoint (768px) requires an explicit `page.setViewportSize({ width: 768, height: 1024 })` in the test. - No Flyway migration, no backend change, no Docker Compose change needed. ### Recommendations - Confirm the Gitea Actions CI workflow runs `npm run test:e2e` as part of the merge check — if it does not, add it so the new regression tests actually gate the PR. - The fix does not change the OpenAPI spec or any API contract, so `npm run generate:api` is not needed. - After the fix ships, no deployment steps beyond the normal build+deploy are required.
Author
Owner

📋 Elicit — Requirements Engineer

Observations

  • Issue quality: The issue is clear, reproducible, and P1-high / bug-labeled — appropriate for the severity. The acceptance criteria cover the visible fix and the no-regression requirement. Good structure.

  • AC gap — ARIA/keyboard: The acceptance criteria only specify "all list items are visible and scrollable." There is no criterion for keyboard accessibility (ArrowDown/ArrowUp navigation, Enter to select, Escape to close). Given that PersonTypeahead lacks these capabilities entirely (while TagInput has them), and the audience includes 60+ users who may rely on keyboard navigation, this is a missing acceptance criterion for a component that is also a de-facto form control.

  • AC gap — tablet definition: "Tablet (768–1023px)" is specified but the tested entry point is not — the issue says "document edit or new document" but does not specify that the filter bar (Briefwechsel), the merge panel, and the search bar must also pass. Either scope the fix to all usages or explicitly call out which pages are in scope.

  • Scope creep risk: The UX reviewer identified 5 additional improvements (ARIA roles, keyboard nav, focus rings, touch targets). These are real gaps but are NOT in scope for this bug fix. They should be filed as separate issues to prevent this fix from growing into a full component overhaul.

Recommendations

  • Add to AC: "Given the dropdown is open, then the user can navigate items with ArrowDown/ArrowUp keys and select with Enter." This is the minimum keyboard contract expected of a combobox.
  • Add to AC: "The fix applies at all usage sites: document edit sender field, document edit receiver field, conversation filter sender/receiver, and document search filter bar."
  • File a follow-up issue titled enhancement(persons): PersonTypeahead keyboard navigation and ARIA combobox pattern with the ARIA and arrow-key requirements, linking it as "relates to #343."
  • File a second follow-up enhancement(a11y): remove button focus ring missing in PersonMultiSelect and TagInput for the focus ring gap.

Open Decisions

  • Should the keyboard navigation gap in PersonTypeahead be fixed in this PR (since the dropdown is already being reworked) or deferred to the follow-up issue? The cost is low if the developer is already touching the dropdown markup.
## 📋 Elicit — Requirements Engineer ### Observations - **Issue quality**: The issue is clear, reproducible, and P1-high / bug-labeled — appropriate for the severity. The acceptance criteria cover the visible fix and the no-regression requirement. Good structure. - **AC gap — ARIA/keyboard**: The acceptance criteria only specify "all list items are visible and scrollable." There is no criterion for keyboard accessibility (ArrowDown/ArrowUp navigation, Enter to select, Escape to close). Given that `PersonTypeahead` lacks these capabilities entirely (while `TagInput` has them), and the audience includes 60+ users who may rely on keyboard navigation, this is a missing acceptance criterion for a component that is also a de-facto form control. - **AC gap — tablet definition**: "Tablet (768–1023px)" is specified but the tested entry point is not — the issue says "document edit or new document" but does not specify that the filter bar (Briefwechsel), the merge panel, and the search bar must also pass. Either scope the fix to all usages or explicitly call out which pages are in scope. - **Scope creep risk**: The UX reviewer identified 5 additional improvements (ARIA roles, keyboard nav, focus rings, touch targets). These are real gaps but are NOT in scope for this bug fix. They should be filed as separate issues to prevent this fix from growing into a full component overhaul. ### Recommendations - Add to AC: "Given the dropdown is open, then the user can navigate items with ArrowDown/ArrowUp keys and select with Enter." This is the minimum keyboard contract expected of a combobox. - Add to AC: "The fix applies at all usage sites: document edit sender field, document edit receiver field, conversation filter sender/receiver, and document search filter bar." - File a follow-up issue titled `enhancement(persons): PersonTypeahead keyboard navigation and ARIA combobox pattern` with the ARIA and arrow-key requirements, linking it as "relates to #343." - File a second follow-up `enhancement(a11y): remove button focus ring missing in PersonMultiSelect and TagInput` for the focus ring gap. ### Open Decisions - Should the keyboard navigation gap in `PersonTypeahead` be fixed in this PR (since the dropdown is already being reworked) or deferred to the follow-up issue? The cost is low if the developer is already touching the dropdown markup.
Author
Owner

🗳️ Decision Queue

Consolidated from persona reviews. One decision per contributor needed.


Test Organization

From Sara (QA): Should the Playwright regression tests for dropdown visibility live inside an existing E2E spec file, or in a dedicated person-typeahead.spec.ts?

Context: There are six usage sites for PersonTypeahead. A dedicated spec file keeps person-component coverage together and makes it easy to add keyboard/ARIA tests later. An existing file avoids file proliferation. Either works; the choice only matters for maintainability.


Scope: Keyboard Navigation in This PR

From Leonie (UX) and Elicit (RE): Should ArrowDown/ArrowUp keyboard navigation and the role="listbox" / role="option" ARIA correction be included in this PR, or deferred to a follow-up issue?

Context: The developer is already touching the dropdown markup for the positioning fix. Adding keyboard navigation now has low marginal cost. However, it adds scope and test surface to what is filed as a P1 bug fix. TagInput already has the reference implementation (handleKeydown with ArrowDown/Up/Enter/Backspace), so the pattern is available to copy.


Full Combobox ARIA Pattern

From Leonie (UX): Should this PR add aria-expanded, aria-controls, and aria-activedescendant to complete a proper WCAG combobox pattern, or is that a separate issue?

Context: Without aria-expanded, screen readers don't announce when the suggestion list opens. This is a High accessibility gap for the target audience. But it is a more involved change (requires IDs on the listbox element, dynamic aria-activedescendant updates on keyboard navigation) and may belong in the follow-up ARIA issue.

## 🗳️ Decision Queue Consolidated from persona reviews. One decision per contributor needed. --- ### Test Organization **From Sara (QA):** Should the Playwright regression tests for dropdown visibility live inside an existing E2E spec file, or in a dedicated `person-typeahead.spec.ts`? _Context_: There are six usage sites for `PersonTypeahead`. A dedicated spec file keeps person-component coverage together and makes it easy to add keyboard/ARIA tests later. An existing file avoids file proliferation. Either works; the choice only matters for maintainability. --- ### Scope: Keyboard Navigation in This PR **From Leonie (UX) and Elicit (RE):** Should ArrowDown/ArrowUp keyboard navigation and the `role="listbox"` / `role="option"` ARIA correction be included in this PR, or deferred to a follow-up issue? _Context_: The developer is already touching the dropdown markup for the positioning fix. Adding keyboard navigation now has low marginal cost. However, it adds scope and test surface to what is filed as a P1 bug fix. `TagInput` already has the reference implementation (`handleKeydown` with ArrowDown/Up/Enter/Backspace), so the pattern is available to copy. --- ### Full Combobox ARIA Pattern **From Leonie (UX):** Should this PR add `aria-expanded`, `aria-controls`, and `aria-activedescendant` to complete a proper WCAG combobox pattern, or is that a separate issue? _Context_: Without `aria-expanded`, screen readers don't announce when the suggestion list opens. This is a **High** accessibility gap for the target audience. But it is a more involved change (requires IDs on the listbox element, dynamic `aria-activedescendant` updates on keyboard navigation) and may belong in the follow-up ARIA issue.
Author
Owner

Chose whatever test pattern is more maintainable
Keyboard navigation is fixed here
Full ARIA pattern

Chose whatever test pattern is more maintainable Keyboard navigation is fixed here Full ARIA pattern
Author
Owner

Implementation complete — branch feat/issue-343-person-dropdown-clipping

What was done

Root cause fixed — PersonTypeahead now uses fixed positioning

The dropdown was using position: absolute inside a .relative wrapper, which clipped whenever any ancestor established a new stacking context (via overflow, transform, shadow-sm + z-index, etc.). Adopted the same pattern already used by PersonMultiSelect:

  • Added bind:this={inputEl} to the text input
  • Added updateDropdownPosition() computing position:fixed;top:…;left:…;width:… via getBoundingClientRect()
  • Added <svelte:window onscroll={...} onresize={...} /> to keep position current
  • Replaced class="absolute top-full left-0 z-50 ..." with style={dropdownStyle} on the dropdown container

Full ARIA combobox pattern added

  • Input: role="combobox", aria-expanded, aria-haspopup="listbox", aria-controls, aria-activedescendant
  • Dropdown container: role="listbox" (was a bare div)
  • Items: role="option" with aria-selected (was role="button")

Keyboard navigation added (matching TagInput's existing pattern)

  • ArrowDown / ArrowUp — cycle through options with wrap-around
  • Enter — select the highlighted option
  • Escape — close without selecting

Dead code removed from ConversationFilterBar.svelte

The relative z-30 wrappers around each PersonTypeahead column and the relative z-10 on the lower grid were workarounds for this bug. Both removed.

Commits

  • 69b940c2 fix(persons): fix PersonTypeahead dropdown clipping with fixed positioning

Tests

29 unit tests — all green (PersonTypeahead.svelte.spec.ts):

  • Rendering, search, selection, clearing, correspondent mode, click-outside (unchanged — all pass)
  • ARIA roles: role="listbox", role="option", aria-expanded, aria-controls, aria-haspopup
  • Keyboard navigation: ArrowDown, ArrowDown×2, ArrowUp wrap, Enter select, Escape close, aria-activedescendant

E2E regression tests added (frontend/e2e/person-typeahead.spec.ts):

  • Desktop (1280×720): dropdown visible, positioned below input
  • Tablet (768×1024): dropdown visible and within viewport
  • Keyboard: ArrowDown activates first option, Escape closes, aria-expanded reflects state
  • Fixed-position click-outside dismiss works correctly
## Implementation complete — branch `feat/issue-343-person-dropdown-clipping` ### What was done **Root cause fixed — `PersonTypeahead` now uses fixed positioning** The dropdown was using `position: absolute` inside a `.relative` wrapper, which clipped whenever any ancestor established a new stacking context (via `overflow`, `transform`, `shadow-sm` + `z-index`, etc.). Adopted the same pattern already used by `PersonMultiSelect`: - Added `bind:this={inputEl}` to the text input - Added `updateDropdownPosition()` computing `position:fixed;top:…;left:…;width:…` via `getBoundingClientRect()` - Added `<svelte:window onscroll={...} onresize={...} />` to keep position current - Replaced `class="absolute top-full left-0 z-50 ..."` with `style={dropdownStyle}` on the dropdown container **Full ARIA combobox pattern added** - Input: `role="combobox"`, `aria-expanded`, `aria-haspopup="listbox"`, `aria-controls`, `aria-activedescendant` - Dropdown container: `role="listbox"` (was a bare `div`) - Items: `role="option"` with `aria-selected` (was `role="button"`) **Keyboard navigation added** (matching `TagInput`'s existing pattern) - `ArrowDown` / `ArrowUp` — cycle through options with wrap-around - `Enter` — select the highlighted option - `Escape` — close without selecting **Dead code removed from `ConversationFilterBar.svelte`** The `relative z-30` wrappers around each `PersonTypeahead` column and the `relative z-10` on the lower grid were workarounds for this bug. Both removed. ### Commits - `69b940c2` fix(persons): fix PersonTypeahead dropdown clipping with fixed positioning ### Tests **29 unit tests — all green** (`PersonTypeahead.svelte.spec.ts`): - Rendering, search, selection, clearing, correspondent mode, click-outside (unchanged — all pass) - ARIA roles: `role="listbox"`, `role="option"`, `aria-expanded`, `aria-controls`, `aria-haspopup` - Keyboard navigation: ArrowDown, ArrowDown×2, ArrowUp wrap, Enter select, Escape close, `aria-activedescendant` **E2E regression tests added** (`frontend/e2e/person-typeahead.spec.ts`): - Desktop (1280×720): dropdown visible, positioned below input - Tablet (768×1024): dropdown visible and within viewport - Keyboard: ArrowDown activates first option, Escape closes, `aria-expanded` reflects state - Fixed-position click-outside dismiss works correctly
Sign in to join this conversation.
No Label P1-high bug person ui
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#343