feat: one shared searchable, scrollable multi-select component (#1954)

[larryro]

Summary

Resolves #1954. Introduces a single reusable MultiSelect primitive and migrates the previously-divergent multi-select sites onto it.

New component

• services/platform/app/components/ui/forms/multi-select.tsx — a popover/combobox multi-select built on Radix Popover, mirroring the existing SearchableSelect:
  • Search input (toggle via searchable, default on).
  • Scrollable option list (max-h + overflow-y-auto) — handles large lists (~1000) via search + scroll.
  • Checkbox rows; toggling keeps the popover open (multi-pick).
  • Selected-chip display in the default trigger, each chip removable.
  • Keyboard accessible: ArrowUp/Down to move highlight, Enter to toggle, Home/End, Escape to close; role="listbox" + aria-multiselectable, options with role="option"/aria-selected, search input wired with aria-activedescendant.
  • Skeleton-aware (masks the default trigger under <Skeletonize loading>).
• Companion multi-select.test.tsx (24 tests) and multi-select.stories.tsx.

Migrations

• Document/project/agent team tags — team-multi-select.tsx is now a thin wrapper over MultiSelect (public API unchanged; all four call sites keep working).
• Team create — Members — team-member-checklist.tsx now renders MultiSelect.
• Organigram delegation — organigram-panel.tsx AgentChecklist now uses MultiSelect.
• Agent tools — the integration- and workflow-binding sections in tool-selector.tsx now use MultiSelect.

i18n

• Added settings.agents.form.bindIntegrationsPlaceholder / bindWorkflowsPlaceholder to en / de / fr. Other strings reuse existing common.search.* keys.

Verification

• tsc --noEmit — clean.
• oxlint --type-aware on changed files — clean.
• oxfmt --check — clean.
• knip — clean.
• UI tests (vitest --config vitest.ui.config.ts) for multi-select, team-multi-select, team-create-dialog, document-team-tags, document-upload, project-sharing, organigram — all pass.
• i18n parity/usage tests — pass.

[coderabbitai]

Warning

Review limit reached

@larryro, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 26 minutes and 56 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a3b798c6-fa75-4051-8f36-7a2be09901a7

📥 Commits

Reviewing files that changed from the base of the PR and between 6b90e78 and f1f9cb3.

📒 Files selected for processing (10)

• services/platform/app/components/ui/forms/multi-select.stories.tsx
• services/platform/app/components/ui/forms/multi-select.test.tsx
• services/platform/app/components/ui/forms/multi-select.tsx
• services/platform/app/features/agents/components/tool-selector.tsx
• services/platform/app/features/agents/organigram/organigram-panel.tsx
• services/platform/app/features/documents/components/team-multi-select.tsx
• services/platform/app/features/settings/teams/components/team-member-checklist.tsx
• services/platform/messages/de.json
• services/platform/messages/en.json
• services/platform/messages/fr.json

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch tale/xs7e16gngnycyf4qftksy59z658976ms

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[larryro]

Desk Review — #1954 shared searchable multi-select (PR #2104)

Verdict: NOT READY — CHANGES REQUIRED (one blocking regression in the new MultiSelect's disabled handling).

CI & tests

• CI: all green. gh pr checks shows every check passing (Lint, Format, Knip, Opengrep/OSS, Migrations, Analyze JS+Py, all Build jobs, CodeRabbit, and all 16 Playwright platform shards + web/docs).
• Tests run locally: cd services/platform && bunx vitest --run --config vitest.ui.config.ts multi-select team-multi-select team-member-checklist team-create-dialog document-team-tags organigram tool-selector → 5 files, 64 tests passed.

The component is a clean, faithful mirror of the sibling SearchableSelect; the 4 consumer migrations preserve their original value/onChange wiring (verified against base 6b90e785); i18n keys (bindIntegrationsPlaceholder/bindWorkflowsPlaceholder) are present and genuinely translated in en/de/fr. One real regression blocks merge.

---

🔴 BLOCKING

1. disabled is non-functional on the default trigger — a regression from the components this PR replaces.
services/platform/app/components/ui/forms/multi-select.tsx — the default trigger is a <div role="combobox"> (lines ~296–356) inside <PopoverPrimitive.Trigger asChild>. When disabled is true the code only sets tabIndex=-1, aria-disabled, and an onKeyDown early-return — so keyboard is blocked but mouse is not:

• The div has no onClick guard; Radix Popover.Trigger unconditionally attaches onOpenToggle (fires on any click — it honors only a native disabled attribute, which a <div> cannot carry).
• The disabled className (line ~315) is only cursor-not-allowed opacity-50 — no pointer-events-none.
• Neither handleOpenChange (~223) nor handleToggle (~232) checks disabled.

Result: a disabled MultiSelect can be opened by mouse click, and its options then toggle freely (option onClick → handleToggle → onValueChange, line ~519), mutating the value.

This is a regression, confirmed against base 6b90e785:

• Old team-multi-select.tsx honored disabled: onClick={() => !disabled && ...} and pointer-events-none cursor-not-allowed opacity-50 when disabled.
• The sibling searchable-select.tsx uses a native <button disabled={disabled}> (clicks suppressed by the browser).
• The old organigram checklist used native <Checkbox disabled>.

User-visible impact:

• organigram-panel.tsx passes disabled={!canEdit || isSaving}. A user without edit permission can open the "Reports to" / "Delegates to" pickers and stage delegation edits into the draft; edits are also not blocked during an in-flight save.
• team-multi-select.tsx passes disabled for document/project sharing — that disabled affordance is now bypassable.

Required fix: make the default trigger honor disabled — add disabled && 'pointer-events-none' to the trigger className AND guard handleOpenChange/handleToggle against disabled (so neither mouse open nor option toggle mutates state). Then add a test in multi-select.test.tsx asserting that a disabled MultiSelect does not open on trigger click and does not emit onValueChange when an option is clicked (the existing test only asserts aria-disabled).

---

🟡 Non-blocking (recommended — fix while in here)

2. Reopen highlights a stale row. multi-select.tsx resets highlightedIndex only via the [search] effect (~211–213); handleOpenChange sets search='' on close, which is a no-op when search was already empty, so after arrow-navigating without typing, the popover reopens scrolled to / highlighting the previously-highlighted row instead of the top. Reset highlightedIndex on open. (UX only — no wrong selection.)

3. Transient dangling aria-activedescendant. The search input emits optionId(highlightedIndex) guarded only by filteredOptions.length > 0 (~403–405), not highlightedIndex < filteredOptions.length. A keystroke that narrows the list below the current highlight points the active-descendant at a non-rendered id for one render (self-heals next tick; Enter safely no-ops). Clamp the index used for rendering. (a11y noise only.)

4. Duplicate role="combobox". Both the trigger div (~296) and the search input (~395) carry role="combobox" and both reference the same listbox when open — an invalid/ambiguous ARIA composition. Give the search input role="textbox"/searchbox, or drop the combobox role from one element.

5. searchable={false} has no keyboard path. Keyboard nav is wired only to the search input, and onOpenAutoFocus only focuses it when searchable. With searchable=false the listbox div has no onKeyDown/tabIndex, so ArrowUp/Down/Enter/Home/End do nothing — contradicting the PR's accessibility claim. No production consumer uses searchable=false today, so latent.

6. Stale value entries persist. selectedOptions drops values with no matching option, but handleToggle rebuilds from value (~179–185, 235), so a selected id whose option was removed (deleted team/member/integration) stays invisible yet gets re-saved on the next unrelated toggle. Pre-existing pattern across similar pickers; consider reconciling value against options.

7. Test/claim gaps. The new diff-based handleChange in team-member-checklist.tsx (~58–68) that bridges MultiSelect's "complete next selection" into single onToggleMember calls is untested (its only dialog test mocks the component out). The PR description says "24 tests" but multi-select.test.tsx has 21.

---

NOT READY — CHANGES REQUIRED

1. In multi-select.tsx, make the default trigger honor disabled: add pointer-events-none to the trigger className when disabled and guard handleOpenChange/handleToggle so a disabled MultiSelect cannot be opened by mouse click or toggle options/emit onValueChange. Add a multi-select.test.tsx case proving a disabled MultiSelect does not open on click and emits no onValueChange. (This restores behavior the replaced team-multi-select/organigram controls had, and that the sibling SearchableSelect still has, and closes the permission-gate bypass on the organigram delegation pickers.)

[larryro]

Desk Review — PR #2104 "feat: one shared searchable, scrollable multi-select component" (#1954)

Verdict (one line): Solid, well-structured change that genuinely resolves the issue; CI is green and all tests pass — but the shared primitive ships one half-implemented branch (searchable={false} is keyboard-inaccessible), which must be fixed before merge.

CI state

gh pr checks on tale/xs7e16gngnycyf4qftksy59z658976ms: 46 passed, 5 skipped, 0 failed, 0 pending. Includes Lint, Format, Knip, Opengrep, Analyze (JS/TS + Python), all Build jobs, and Playwright (docs + 16 platform shards) — all green. CodeRabbit: review completed.

Tests (run locally, once, by the reviewer)

• Setup: bun install (2049 packages), Node v24.17.0, bun 1.3.10.
• cd services/platform && bunx vitest --run --config vitest.ui.config.ts multi-select → 2 files, 29 tests passed.
• bunx vitest --run --config vitest.ui.config.ts team-multi-select team-create-dialog document-team-tags document-upload project-sharing organigram tool-selector team-member-checklist → 5 files, 54 tests passed.

What was reviewed

New primitive services/platform/app/components/ui/forms/multi-select.tsx (554 lines) + test + stories, and the four migrations: team-multi-select.tsx, team-member-checklist.tsx, organigram-panel.tsx, tool-selector.tsx, plus en/de/fr i18n. Two-round review: 5 parallel breadth subagents (correctness, a11y, migrations/regression, tests/i18n, robustness/elegance), then independent re-verification of every candidate blocker.

Strengths (confirmed)

• Migrations preserve behavior, no regressions. Organigram keeps reportsTo→parentSlugs / delegatesTo→directReports; team-member-checklist correctly diffs the full next-list back into single onToggleMember calls (exactly one flip per toggle); team-multi-select keeps disabled-when-empty + org-wide placeholder; tool-selector keeps the platform-tools CheckboxGroup and empty/loading states untouched. The "selected value not in options is dropped from chips" behavior is identical to origin/main — not a regression.
• i18n parity verified: the two new keys (bindIntegrationsPlaceholder, bindWorkflowsPlaceholder) exist in all three locales and are referenced; no other newly-used key is missing from any locale.
• Core behavior is well-tested (29 tests): open/close, search filter, toggle add/remove, chips + removal, removeChipLabel, disabled trigger, disabled options, Enter-on-highlight, multi-select-stays-open.
• Robustness is sound: aria-activedescendant is bounds-checked (highlightedIndex < filteredOptions.length), findNextEnabledIndex wraps safely with an all-disabled guard, no leaked listeners, skeleton masking mirrors the sibling SearchableSelect.

BLOCKING finding (confirmed in Round 2)

1. searchable={false} produces a keyboard-inaccessible listbox — multi-select.tsx.

• handleSearchKeyDown (the only keyboard handler) is attached exclusively to the search input (line 403). The listbox <div role="listbox"> (lines 419–450) and the option rows (role="option", lines 517–524) have no onKeyDown and no tabIndex.
• onOpenAutoFocus (lines 382–389) moves focus to the search input only when searchable; with searchable={false} there is no preventDefault, so focus lands on the Radix Content container, which has no key handler.
• Net effect: when searchable={false}, ArrowUp/Down/Home/End/Enter do nothing and options can only be reached/toggled with a mouse — directly contradicting issue Feature: One shared searchable, scrollable multi-select component #1954's "Accessible (keyboard + screen reader)" acceptance criterion.
• This is a public, documented prop (defaults to true) and is exercised by a Storybook story (multi-select.stories.tsx:150), so it is a sanctioned configuration of the shared primitive — not dead code. It happens to be used by zero production call sites today (all four migrations use the default searchable=true, which IS keyboard-accessible), so nothing currently shipping is broken — but a future adopter that sets searchable={false} silently gets an inaccessible control. For the one component whose mandate is to be the accessible shared primitive, this half-implemented branch should be closed before merge.
• Fix: when searchable={false}, make the option list keyboard-operable — e.g. focus the listbox in onOpenAutoFocus (drop the if (searchable) guard / add an else), give the listbox tabIndex={0} + roving aria-activedescendant, and attach the same Arrow/Home/End/Enter handling to it (reuse handleSearchKeyDown). Then add a test asserting keyboard navigation + toggle works with searchable={false}.

Non-blocking (do NOT block merge; worth addressing)

• A11y nit — double role="combobox" (trigger div line 302 + search input line 400). The sibling SearchableSelect declares role="combobox" only on its input; two simultaneous comboboxes pointing at one listbox is slightly ambiguous for AT. Consider dropping it from the trigger div.
• Disabled option highlights on mouse hover (onMouseEnter → setHighlightedIndex, line 525/436) without the disabled guard that keyboard navigation applies; cosmetic only (toggle is still correctly blocked).
• Test gaps for secondary features: ArrowUp / Home / End / wrap-around, modal, custom filterFn, controlled open, and footer interactivity are untested. Core paths are covered; these are lower-risk but would round out the suite — and the searchable={false} keyboard test above is required as part of the blocking fix.

---

NOT READY — CHANGES REQUIRED

1. In services/platform/app/components/ui/forms/multi-select.tsx, fix the searchable={false} branch so the option list is keyboard-operable (focus + ArrowUp/Down/Home/End/Enter + aria-activedescendant on the listbox when the search input is absent), and add a UI test in multi-select.test.tsx asserting keyboard navigation and toggle work with searchable={false}.

[larryro]

Desk review — #1954 shared searchable multi-select (PR #2104)

Reviewed the new MultiSelect primitive plus all four migration sites on branch tale/xs7e16gngnycyf4qftksy59z658976ms. Two-round review (8 breadth agents + targeted confirmation). CI is fully green and the UI suite passes locally (bunx vitest --run --config vitest.ui.config.ts multi-select → 33/33). The consolidation is clean and well-built — four hand-rolled comboboxes collapse into one documented, keyboard-operable primitive, i18n is complete across en/de/fr, no dead code.

There is one blocking functional gap plus a few recommended follow-ups.

🚫 Blocking — modal not passed at the dialog-hosted sites (mouse-wheel scroll broken on large lists)

MultiSelect defaults modal={false} and portals its popover to document.body. Inside a Radix modal Dialog (our Dialog/FormDialog render DialogPrimitive.Root with no modal prop → Radix default modal=true), the dialog's RemoveScroll lock swallows wheel events over the portaled popover — exactly the failure your own modal prop doc (multi-select.tsx:103-110) describes. None of the dialog-hosted call sites pass it:

• team-member-checklist.tsx → used in team-create-dialog and team-edit-dialog (the issue's headline "~1000 members" scenario).
• team-multi-select.tsx → used in document-upload-dialog and document-team-tags-dialog.

Result: in those dialogs the option list cannot be mouse-wheel scrolled (keyboard arrows + search still work), which partially defeats the "Handles large lists (~1000) via search + scroll" acceptance criterion in the very dialog that motivated the issue. tool-selector.tsx (page route) and organigram-panel.tsx (inline side panel) are not in dialogs and are unaffected.

Fix: pass modal (true) to MultiSelect in team-member-checklist.tsx and team-multi-select.tsx.

Recommended (non-blocking)

1. tool-selector.tsx — <fieldset disabled> doesn't disable the binding MultiSelects. The integration/workflow MultiSelects are div-based (role="combobox"/role="option"), so a native <fieldset disabled> (line 224) doesn't reach them; their own disabled prop is never passed. Latent today (the only call site in tools.tsx never sets disabled), but the prop is silently ineffective — thread disabled through IntegrationBindingsSection/WorkflowBindingsSection into each MultiSelect.
2. Highlight reset can land on a disabled first option. The setHighlightedIndex(0) resets on open (line 229) and on search change (line 212) don't skip a disabled filteredOptions[0], so the highlight + aria-activedescendant can point at a disabled row until the user arrows. Enter is correctly blocked; self-heals on ArrowDown. Use findNextEnabledIndex(filteredOptions, -1, 1) for those resets.
3. Double role="combobox" (trigger at line 301 and the search input at line 403, both aria-controls the same listbox). Consider role="searchbox" for the input so a single combobox owns the popup.
4. Test gaps in non-trivial new logic: (a) filter-the-list-then-Enter (the stale-highlightedIndex path — traced correct, but uncovered); (b) ArrowDown/Up traversal over a mid-list disabled option; (c) team-member-checklist.tsx's handleChange set-diff adapter has no test at all.

Verdict

NOT READY — pass modal to the two dialog-hosted MultiSelect wrappers (item above). Everything else is advisory.

[larryro]

Desk Review — PR #2104 · feat: one shared searchable, scrollable multi-select component (#1954)

Verdict: READY TO MERGE.

CI state

All required checks green. gh pr checks reports every job as pass (Analyze JS/TS + Python, Build ×9, Lint, Format, Knip, Opengrep, Migrations, UI, Browser, and all 16 Playwright platform shards + docs). The only non-pass jobs are Web container test, Smoke test (fork PR), and Validate images (fork PR), all reported as skipping — these are fork-PR/conditional jobs that do not apply to this branch, not failures. No check is failing or pending.

Tests run (by me, this review)

• Command: bunx vitest --run --config vitest.ui.config.ts --pool=forks multi-select team-multi-select team-member-checklist organigram tool-selector
  → 45 passed (3 files). (Sandbox needs --pool=forks; the default threads pool can't spawn workers here — unrelated to the code.)
• Command: bunx vitest --run --config vitest.ui.config.ts --pool=forks multi-select team-create-dialog document-team-tags document-upload project-sharing organigram
  → 81 passed (6 files).
  The new multi-select.test.tsx plus all migrated call-site suites pass.

What the change does

Introduces one MultiSelect primitive (services/platform/app/components/ui/forms/multi-select.tsx, Radix Popover + search + scrollable list + checkbox rows + removable chips + full keyboard/ARIA), and migrates all four sites named in the issue onto it: document/project/agent team tags (team-multi-select.tsx, public API unchanged), team-create Members (team-member-checklist.tsx), organigram delegation (organigram-panel.tsx), and agent integration/workflow bindings (tool-selector.tsx). Adds bindIntegrationsPlaceholder / bindWorkflowsPlaceholder to en/de/fr.

Review findings (two rounds, fan-out of 6 review agents + independent confirmation)

Correctness, robustness, migrations, keyboard/a11y, tests, and elegance/i18n were each reviewed and the candidate findings independently re-verified. No blocking issues confirmed. Specifically verified correct:

• Selection logic — handleToggle add/remove, complete-next-list contract, chip order preservation, disabled blocked on every path (trigger open, option click, Enter, chip remove), controlled/uncontrolled open state.
• Keyboard/ARIA — findNextEnabledIndex wrap-around + all-disabled → -1 guarded by callers; Home/End seeding (-1/len) lands on first/last enabled; aria-activedescendant bounded by highlightedIndex < filteredOptions.length; Escape close handled by Radix.
• Migrations — team-member-checklist diff-to-single-toggle is correct (no spurious/missed/double toggles); team-multi-select preserves disabled-when-empty + org-wide chip + public API; tool-selector preserves isEmpty/skeleton/value wiring. Confirmed the old binding sections had no max/limit/counter, so nothing was dropped.
• i18n — both new keys present in all three locales, referenced in tool-selector.tsx, no orphaned keys, no new hardcoded user-facing strings.

Non-blocking notes (optional polish, not required for merge)

1. defaultFilterFn, findNextEnabledIndex, and CONTENT_CLASSES are near-duplicates of the same code in the sibling searchable-select.tsx; could be extracted to a shared module to avoid future drift.
2. team-multi-select.tsx passes modal unconditionally, including for full-page (non-dialog) usages (agent settings page, project sharing). modal adds a body scroll-lock while open; the prop doc frames it as "required when inside a modal Dialog." Harmless (it works everywhere), but slightly over-applied.
3. Test gaps for low-risk branches: ArrowUp wrap-around, controlled-open full cycle, and the SkeletonBox masking wrapper aren't exercised. Code paths were verified correct by reading; adding cases would be nice-to-have.

READY TO MERGE.