Virtualized Combobox component and keyword-search node-type picker migration - #2131
Open
mjuarros wants to merge 7 commits into
Open
Virtualized Combobox component and keyword-search node-type picker migration#2131mjuarros wants to merge 7 commits into
mjuarros wants to merge 7 commits into
Conversation
Backs large option lists (5,000-10,000+ items) with @tanstack/react-virtual so rendering stays smooth instead of freezing the UI, matching the look and keyboard/animation behavior of the existing Select component. Also supports an inner label above the value, matching SelectField's labelPlacement="inner" style, so sites migrating off SelectField don't lose that caption.
Swaps the node-type dropdowns in FilterSearchTabContent and DataExplorer over to the virtualized Combobox to fix UI freezing on schemas with large numbers of vertex/edge types. DataExplorer uses the inner label to keep the same caption the SelectField's labelPlacement="inner" showed.
The previous commit shipped a hand-rolled Combobox instead of the @base-ui/react primitives the proposal called for. This finishes that migration, then fixes what verifying it surfaced: a popup width mismatch, an arrow button that didn't close on a second click, an entrance animation twice as long as every other dropdown, and an empty-state element reserving phantom space above the list. The significant fix: the virtualizer was measuring the wrong scroll element, so it rendered nearly all 10,000 options as real DOM nodes on every open instead of ~20 - confirmed with an isolated Chromium harness (~1.75s per open before, ~30ms after). Also fixes a memoization bug in useSearchableAttributes surfaced while restoring its referential-stability guarantee, and updates the test suite's offsetHeight/offsetWidth mock to actually distinguish which element gets measured.
…bility issues The virtualized prop was missing from BaseCombobox.Root, silently bounding keyboard navigation to whatever was mounted in the DOM window instead of the full option count. Open/filter state was smeared across handlers that didn't distinguish typed input from Base UI's own programmatic syncs, causing stale display text and a popup that never closed on focus-out. The toggle button is no longer hidden from the accessibility tree: when the input already has a value, VoiceOver's Read-All treats it as content to read and skips announcing its combobox role, so the button is what re-announces it in that case. Also fixes popup sizing/animation, prop passthrough safety, filtering efficiency, and several tests that didn't assert what they claimed.
useTranslations() returned a fresh function on every render, which fed into useAttributeOptions's own useMemo dependency array and defeated it every render regardless of whether the underlying data changed. vertexOptions had the same problem one level up: a fresh array of fresh objects on every render, defeating Combobox's own filteredOptions memo on every keystroke.
…ive nanoid @base-ui/react and @tanstack/react-virtual were declared in both the workspace root and packages/graph-explorer, though only graph-explorer imports them. Also adds an override for nanoid (GHSA-2v37-7h3g-55p8), pulled in transitively via vite > postcss and confined to dev, following the existing override pattern for esbuild/dompurify/undici.
…-filter support Adds an ADR for introducing @base-ui/react and @tanstack/react-virtual alongside the existing Radix/shadcn stack, exceptions to the React Compiler guidance for the memoization and useVirtualizer cases this PR needed, the Base UI vs Radix data-attribute divergence, and the jsdom offsetHeight gotcha the test suite works around. Also updates user-facing docs to mention that the node-type/attribute pickers now support typing to filter.
mjuarros
marked this pull request as ready for review
August 18, 2026 18:58
noel-improv
reviewed
Aug 21, 2026
noel-improv
left a comment
noel-improv
left a comment
There was a problem hiding this comment.
Big PR but overall nice organization just a few comments left
| flatted@<3.4.2: ">=3.4.2" | ||
| form-data@>=4.0.0 <4.0.6: ">=4.0.6" | ||
| minimatch@<10.2.3: ">=10.2.3" | ||
| nanoid@<3.3.18: ">=3.3.18" |
There was a problem hiding this comment.
Can you confirm this override is intended as unbounded? The lockfile was already on 3.3.18, which is outside <3.3.18, but it still moves to nanoid 6.0.1 (ESM-only, engines.node: ^22 || ^24 || >=26, no require export condition) and postcss loads nanoid/non-secure from CJS. Maybe bound it to ">=3.3.18 <4", or drop it?
| // previously validated behavior (fixed across several rounds of | ||
| // manual testing). | ||
| if (reason !== "keyboard" && reason !== "none") return; | ||
| virtualizerRef.current?.scrollToIndex(index, { align: "center" }); |
There was a problem hiding this comment.
Does this need a guard for the cleared-highlight case? Base UI fires onItemHighlighted with index: -1 and reason: "keyboard" when the highlight clears, which gets past the check above, and scrollToIndex(-1) clamps to 0 — I think that scrolls the list to the top on the first keystroke. Base UI's virtualized example uses if (!item || !virtualizer) return;.
| - **A second UI primitive stack now exists, scoped to one file.** An agent choosing between `Combobox` and `Select`/`SelectField` for a new picker should pick based on scale: `Combobox` for schema-sized/unbounded lists that need type-to-filter and virtualization, `Select`/`SelectField` for small bounded enums. Do not introduce a third primitive stack for the same class of problem — extend `Combobox` instead. | ||
| - **Base UI's attribute convention differs from Radix's.** Base UI emits bare boolean data attributes (`data-open`, `data-closed`, `data-starting-style`, `data-ending-style`), not Radix's `data-state="open"`/`"closed"`. The project's `data-open:`/`data-closed:` Tailwind shorthand is scoped to the Radix convention and will not match Base UI elements — see `docs/agents/design.md`. | ||
| - **`useVirtualizer` needs a React Compiler suppression.** The call in `Combobox.tsx` carries a `// eslint-disable-next-line react-compiler/incompatible-library` comment, since the compiler can't verify the hook's internal mutation patterns are safe to auto-memoize. See `docs/agents/react.md`. | ||
| - **The trigger/input interaction pattern is VoiceOver-validated, not just ARIA-linted.** The decorative arrow button is `aria-hidden` and click-only (not keyboard-focusable); the input itself gets an explicit `onClick` handler to open the list, because a text input has no native "click" default action the way a `<button>` does, and VoiceOver's Control-Option-Space gesture needs one to trigger reliably. A grouped, AX-visible trigger button reads to VoiceOver as "stop interacting with this group" rather than "open the list" — this was found and fixed via live VoiceOver testing on macOS across several rounds, not derived from an accessibility guideline. Changing this interaction pattern needs to be re-validated with a screen reader, not just re-derived from ARIA best practices. |
There was a problem hiding this comment.
This says the arrow button is aria-hidden and click-only, but Combobox.tsx:261-272 keeps the Trigger in the accessibility tree on purpose (with a comment explaining why). Is the ADR the stale one here?
| - Use **Tailwind v4 CSS syntax** — `@theme`, `@utility`, `@custom-variant` blocks in CSS. | ||
| - Prefer **data attributes** for conditional styles. Tailwind v4 provides two forms: | ||
| - `data-open:` / `data-closed:` — shorthand variants (defined via `@custom-variant` in `index.css`) that match `[data-state="open"]` / `[data-state="closed"]`, the Radix convention. Note: this **redefines** Tailwind's native behavior, where a bare `data-open:` would match the presence of a `data-open` attribute. Also: `aria-invalid:` for form validation. | ||
| - **Base UI components don't emit `data-state`.** They set bare boolean attributes instead (`data-open`, `data-closed`, `data-starting-style`, `data-ending-style`), so the project's `data-open:`/`data-closed:` shorthand above will silently never match a Base UI element. For Base UI components (currently only `components/Combobox.tsx`), use Tailwind's native bare-attribute variants directly — `data-open:`, `data-ending-style:`, etc. — not the Radix-scoped shorthand. |
There was a problem hiding this comment.
This warns that data-open:/data-closed: never match Base UI elements, then recommends using them. index.css:5-6 redefines both globally to [data-state="open"]/[data-state="closed"], so they'd still silently fail here. Should this say data-[open]:/data-[closed]: instead? (data-ending-style:/data-starting-style: aren't redefined, so those work as written.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
SelectField renders every option as a DOM node unconditionally. With a schema of 10,000+ vertex/edge types, this froze the UI for several seconds every time the Data Explorer node-type picker, the Search Sidebar's node-type filter, or its attribute filter was opened.
This replaces those three pickers with a new Combobox component built on Base UI's accessible combobox primitives plus @tanstack/react-virtual for windowed rendering, so only the visible options (~20) are ever mounted regardless of how many types the schema has. Type-to-filter narrows the list as you type. Every other SelectField usage in the app is untouched — they're all either fixed-size enums or scoped to a single vertex/edge type's own attributes, so they were never at risk of this.
How to read
packages/graph-explorer/src/components/Combobox.tsx— the new component; start herepackages/graph-explorer/src/routes/DataExplorer/DataExplorer.tsx,packages/graph-explorer/src/modules/SearchSidebar/FilterSearchTabContent.tsx,packages/graph-explorer/src/modules/SearchSidebar/useKeywordSearch.ts— the three sites swapped from SelectField to Comboboxpackages/graph-explorer/src/core/StateProvider/displayTypeConfigs.ts— a small pre-existing memoization bug in useSearchableAttributes fixed along the way (found while restoring a referential-stability guarantee post-migration); tangential to the main changepackages/graph-explorer/src/components/Combobox.test.tsx,packages/graph-explorer/src/modules/SearchSidebar/useKeywordSearch.test.ts— tests, including a 10,000-item scale guardrailValidation
Manually validated against a synthetic 10,000-vertex-type / 10,000-edge-type dataset: the Data Explorer and Search Sidebar pickers previously froze the page for several seconds on open; they now open instantly regardless of schema size. Confirmed with an isolated real-Chromium test harness that open latency stays flat (~30ms) across 10, 500, and 10,000 items — a virtualizer-wiring bug that briefly regressed this was caught the same way and fixed before landing.
pnpm checksandpnpm testpass (219 test files, 2696 tests).Related Issues
Check List
pnpm checkspasses with no errors.pnpm testpasses with no failures.Delay combobox implementation with no text type option
New virtualized combobox with text type option
