Add virtualization via Headless UI's virtual prop

app/components/form/fields/ComboboxField.tsx

@@ -6,7 +6,6 @@
  * Copyright Oxide Computer Company
  */
 
-import { useState } from 'react'
 import {
   useController,
   type Control,
@@ -16,11 +15,7 @@ import {
   type Validate,
 } from 'react-hook-form'
 
-import {
-  Combobox,
-  getSelectedLabelFromValue,
-  type ComboboxBaseProps,
-} from '~/ui/lib/Combobox'
+import { Combobox, type ComboboxBaseProps } from '~/ui/lib/Combobox'
 import { capitalize } from '~/util/str'
 
 import { ErrorMessage } from './ErrorMessage'
@@ -69,9 +64,6 @@ export function ComboboxField<
     control,
     rules: { required, validate },
   })
-  const [selectedItemLabel, setSelectedItemLabel] = useState(
-    getSelectedLabelFromValue(items, field.value || '')
-  )
   return (
     <div className="max-w-lg">
       <Combobox
@@ -81,12 +73,10 @@ export function ComboboxField<
         items={items}
         required={required}
         selectedItemValue={field.value}
-        selectedItemLabel={selectedItemLabel}
         hasError={fieldState.error !== undefined}
         onChange={(value) => {
           field.onChange(value)
           onChange?.(value)
-          setSelectedItemLabel(getSelectedLabelFromValue(items, value))
         }}
         allowArbitraryValues={allowArbitraryValues}
         inputRef={field.ref}

app/ui/lib/Combobox.tsx

@@ -25,6 +25,22 @@ import { TextInputHint } from './TextInput'
 
 export type ComboboxItem = { value: string; label: ReactNode; selectedLabel: string }
 
+// HUI's virtual render prop only accepts one child, so we surface "No items
+// match" via a disabled sentinel option instead of a sibling element.
+const NO_MATCH_VALUE = '__combobox_no_match__'
+const NO_MATCH_ITEM: ComboboxItem = {
+  value: NO_MATCH_VALUE,
+  label: 'No items match',
+  selectedLabel: '',
+}
+
+// HUI's virtualizer needs the scroll container to have a non-zero height
+// before it'll render any items, but it renders no items until the
+// container has a height. Setting an explicit height breaks the cycle.
+// 40 matches HUI's internal estimateSize default.
+const ITEM_HEIGHT = 40
+const MAX_PANEL_HEIGHT = 280
+
 /** Convert an array of items with a `name` attribute to an array of ComboboxItems
  *  Useful when the rendered label and value are the same; in more complex cases,
  *  you may want to create a custom ComboboxItem object (see toImageComboboxItem).
@@ -36,11 +52,6 @@ export const toComboboxItems = (items?: Array<{ name: string }>): Array<Combobox
     selectedLabel: name,
   })) || []
 
-export const getSelectedLabelFromValue = (
-  items: Array<ComboboxItem>,
-  selectedValue: string
-): string => items.find((item) => item.value === selectedValue)?.selectedLabel || ''
-
 /** Simple non-generic props shared with ComboboxField */
 export type ComboboxBaseProps = {
   description?: React.ReactNode
@@ -72,7 +83,6 @@ export type ComboboxBaseProps = {
 
 type ComboboxProps = {
   selectedItemValue: string
-  selectedItemLabel: string
   hasError?: boolean
   /** Fires when the user *selects* an item from the list */
   onChange: (value: string) => void
@@ -85,7 +95,6 @@ export const Combobox = ({
   items = [],
   label,
   selectedItemValue,
-  selectedItemLabel,
   placeholder,
   required,
   hasError,
@@ -98,36 +107,23 @@ export const Combobox = ({
   hideOptionalTag,
   inputRef,
   transform,
-  ...props
 }: ComboboxProps) => {
   const [query, setQuery] = useState(selectedItemValue || '')
-  const q = query.toLowerCase().replace(/\s*/g, '')
+  const q = query.toLowerCase().replace(/\s+/g, '')
   const filteredItems = matchSorter(items, q, {
     keys: ['selectedLabel', 'label'],
     sorter: (items) => items, // preserve original order, don't sort by match
   })
 
-  // In the arbitraryValues case, clear the query whenever the value is cleared.
-  // this is necessary, e.g., for the firewall rules form when you submit the
-  // targets subform and clear the field. Two possible changes we might want to make
-  // here if we run into issues:
-  //
-  //   1. do it all the time, not just in the arbitraryValues case
-  //   2. do it on all value changes, not just on clear
-  //
-  // Currently, I don't think there are any arbitraryValues=false cases where we
-  // set the value from outside. There is an arbitraryvalues=true case where we
-  // setValue to something other than empty string, but we don't need the
-  // sync because that setValue is done in onInputChange and we already are
-  // doing setQuery in here along with it.
+  // Clear the query when the parent clears the value (e.g. firewall rules
+  // form on subform submit). Only needed for allowArbitraryValues; without
+  // it, parent-driven clears leave the input still showing the old query.
   useEffect(() => {
     if (allowArbitraryValues && !selectedItemValue) {
       setQuery('')
     }
   }, [allowArbitraryValues, selectedItemValue])
 
-  // If the user has typed in a value that isn't in the list,
-  // add it as an option if `allowArbitraryValues` is true
   if (
     allowArbitraryValues &&
     query.length > 0 &&
@@ -143,35 +139,55 @@ export const Combobox = ({
       selectedLabel: query,
     })
   }
+  const virtualOptions: ComboboxItem[] =
+    filteredItems.length === 0 && !allowArbitraryValues ? [NO_MATCH_ITEM] : filteredItems
+
+  // Arbitrary values may not be in `items`, so synthesize a stand-in.
+  const selectedItem: ComboboxItem | null = (() => {
+    if (!selectedItemValue) return null
+    const found = items.find((i) => i.value === selectedItemValue)
+    if (found) return found
+    if (allowArbitraryValues) {
+      return {
+        value: selectedItemValue,
+        label: selectedItemValue,
+        selectedLabel: selectedItemValue,
+      }
+    }
+    return null
+  })()
+
   const zIndex = usePopoverZIndex()
   const id = useId()
-  // Tracks whether the dropdown is open so the onKeyDown handler can
-  // distinguish Enter-to-select (dropdown open, let HUI handle it) from
-  // Enter-to-submit (dropdown closed, fire onEnter). We use a ref instead
-  // of HUI's `open` render prop because our handler runs before HUI's
-  // (useRender merges user props first) and the render prop can be stale.
-  // The ref stays current because onClose sets it synchronously during
-  // HUI's own keydown handler. With the stale render prop, the handler
-  // could see the combobox as closed one keydown too late, firing onEnter
-  // instead of letting HUI select — hard to notice manually but caused
-  // consistent e2e flakes in Firefox.
+  // Lets onKeyDown distinguish Enter-to-select (open) from Enter-to-submit
+  // (closed). HUI's `open` render prop is stale by one keydown in our
+  // handler ordering — caused Firefox e2e flakes.
   const isOpenRef = useRef(false)
   return (
     <HCombobox
-      // necessary, as the displayed "value" is not the same as the actual selected item's *value*
-      value={selectedItemValue}
-      // fallback to '' allows clearing field to work
-      onChange={(val) => onChange(val || '')}
+      // items are re-created each render, so compare by value field
+      by="value"
+      value={selectedItem}
+      onChange={(item) => {
+        if (!item || item.value === NO_MATCH_VALUE) {
+          onChange('')
+          return
+        }
+        onChange(item.value)
+      }}
       onClose={() => {
         isOpenRef.current = false
         if (!allowArbitraryValues) setQuery('')
       }}
       disabled={disabled || isLoading}
       immediate
-      {...props}
+      virtual={{
+        options: virtualOptions,
+        // HUI types this with the same union as `value`, so item may be null
+        disabled: (item) => item?.value === NO_MATCH_VALUE,
+      }}
     >
       {({ open }) => {
-        // Sync open state to ref on render (handles the opening side)
         if (open) isOpenRef.current = true
         return (
           <div>
@@ -201,39 +217,30 @@ export const Combobox = ({
                   : 'bg-default',
                 disabled && hasError && 'border-error-secondary!'
               )}
-              // Putting the inputRef on the div makes it so the div can be focused by RHF when there's an error.
-              // We want to focus on the div (rather than the input) so the combobox doesn't open automatically
-              // and obscure the error message.
+              // ref on the wrapper, not the input, so RHF can focus on
+              // error without opening the dropdown and hiding the message
               ref={inputRef}
-              // tabIndex=-1 is necessary to make the div focusable
               tabIndex={-1}
             >
               <ComboboxInput
                 id={`${id}-input`}
-                // If an option has been selected, display either the selected item's label or value.
-                // If no option has been selected yet, or the user has started editing the input, display the query.
-                // We are using value here, as opposed to Headless UI's displayValue, so we can normalize
-                // the value entered into the input (via the onChange event).
+                // Bypass HUI's displayValue so `transform` can normalize what
+                // the user types via onChange
                 value={
                   selectedItemValue
                     ? allowArbitraryValues
                       ? selectedItemValue
-                      : selectedItemLabel
+                      : (selectedItem?.selectedLabel ?? '')
                     : query
                 }
                 onChange={(event) => {
                   const value = transform
                     ? transform(event.target.value)
                     : event.target.value
-                  // updates the query state as the user types, in order to filter the list of items
                   setQuery(value)
-                  // if the parent component wants to know about input changes, call the callback
                   onInputChange?.(value)
                 }}
                 onKeyDown={(e) => {
-                  // When the dropdown is open, Enter should select the
-                  // highlighted option (HUI handles this). When closed,
-                  // Enter should submit the subform via onEnter.
                   if (e.key === 'Enter' && !isOpenRef.current) {
                     e.preventDefault()
                     onEnter?.(e)
@@ -264,37 +271,37 @@ export const Combobox = ({
             {(items.length > 0 || allowArbitraryValues) && (
               <ComboboxOptions
                 anchor="bottom start"
-                // 13px gap is presumably because it's measured from inside the outline or something
-                className={`ox-menu shadow-menu-inset pointer-events-auto ${zIndex} border-secondary relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border [--anchor-gap:13px] empty:hidden`}
+                className={`ox-menu shadow-menu-inset pointer-events-auto ${zIndex} border-secondary relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border [--anchor-gap:13px]`}
+                style={{
+                  height: Math.min(virtualOptions.length * ITEM_HEIGHT, MAX_PANEL_HEIGHT),
+                }}
                 modal={false}
               >
-                {filteredItems.map((item) => (
-                  <ComboboxOption
-                    key={item.value}
-                    value={item.value}
-                    className="border-secondary relative border-b last:border-0"
-                  >
-                    {({ focus, selected }) => (
-                      // This *could* be done with data-[focus] and data-[selected] instead, but
-                      // it would be a lot more verbose. those can only be used with TW classes,
-                      // not our .is-selected and .is-highlighted, so we'd have to copy the pieces
-                      // of those rules one by one. Better to rely on the shared classes.
-                      <div
-                        className={cn('ox-menu-item', {
-                          'is-selected': selected && query !== item.value,
-                          'is-highlighted': focus,
-                        })}
-                      >
-                        {item.label}
-                      </div>
-                    )}
-                  </ComboboxOption>
-                ))}
-                {!allowArbitraryValues && filteredItems.length === 0 && (
-                  <ComboboxOption disabled value="no-matches" className="relative">
-                    <div className="ox-menu-item text-disabled!">No items match</div>
-                  </ComboboxOption>
-                )}
+                {({ option }: { option: ComboboxItem }) => {
+                  const isNoMatch = option.value === NO_MATCH_VALUE
+                  return (
+                    <ComboboxOption
+                      value={option}
+                      disabled={isNoMatch}
+                      // w-full: HUI's virtualizer absolutely-positions each
+                      // option, so without an explicit width they collapse
+                      // to content width.
+                      className="border-secondary relative w-full border-b last:border-0"
+                    >
+                      {({ focus, selected }) => (
+                        <div
+                          className={cn('ox-menu-item', {
+                            'is-selected': selected && query !== option.value && !isNoMatch,
+                            'is-highlighted': focus && !isNoMatch,
+                            'text-disabled!': isNoMatch,
+                          })}
+                        >
+                          {option.label}
+                        </div>
+                      )}
+                    </ComboboxOption>
+                  )
+                }}
               </ComboboxOptions>
             )}
           </div>

test/e2e/instance-create.e2e.ts

@@ -566,10 +566,13 @@ test('attaching additional disks allows for combobox filtering', async ({ page }
 
   await attachExistingDiskButton.click()
   await selectADisk.click()
-  // several disks should be shown
+  // several disks should be shown in the visible window. the combobox
+  // virtualizes, so only the first ~20 options live in the DOM at once;
+  // aria-setsize reports the full attachable count (in the hundreds for
+  // this seeded dataset).
   await expect(page.getByRole('option', { name: 'disk-0005' })).toBeVisible()
   await expect(page.getByRole('option', { name: 'disk-0007' })).toBeVisible()
-  await expect(page.getByRole('option', { name: 'disk-0988' })).toBeVisible()
+  await expect(page.getByRole('option').first()).toHaveAttribute('aria-setsize', /\d{2,}/)
 
   // type in a string to use as a filter
   await selectADisk.fill('disk-02')