diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index efa055829..b31696eac 100644 --- a/app/components/form/fields/ComboboxField.tsx +++ b/app/components/form/fields/ComboboxField.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ -import { useState } from 'react' +import { useCallback, useMemo } from 'react' import { useController, type Control, @@ -25,7 +25,7 @@ import { capitalize } from '~/util/str' import { ErrorMessage } from './ErrorMessage' -export type ComboboxFieldProps< +type ComboboxFieldProps< TFieldValues extends FieldValues, TName extends FieldPath, > = { @@ -45,20 +45,10 @@ export function ComboboxField< label = capitalize(name), required, onChange, + onInputChange, allowArbitraryValues, placeholder, - // Intent is to not show both a placeholder and a description, while still having good defaults; prefer a description to a placeholder - /* - * If description is provided, use it. - * If not, but a placeholder is provided, the default description should be undefined. - * If no placeholder is provided and arbitrary values are allowed, the default description should be 'Select an option or enter a custom value'. - * If no placeholder is provided and arbitrary values are not allowed, the default description should be 'Select an option'. - */ - description = placeholder - ? undefined - : allowArbitraryValues - ? 'Select an option or enter a custom value' - : 'Select an option', + description, items, transform, validate, @@ -69,25 +59,62 @@ export function ComboboxField< control, rules: { required, validate }, }) - const [selectedItemLabel, setSelectedItemLabel] = useState( - getSelectedLabelFromValue(items, field.value || '') + + // Memoize description computation to avoid recalculating on every render + const memoizedDescription = useMemo(() => { + // Intent is to not show both a placeholder and a description, while still having good defaults; prefer a description to a placeholder + /* + * If description is provided, use it. + * If not, but a placeholder is provided, the default description should be undefined. + * If no placeholder is provided and arbitrary values are allowed, the default description should be 'Select an option or enter a custom value'. + * If no placeholder is provided and arbitrary values are not allowed, the default description should be 'Select an option'. + */ + return description !== undefined + ? description + : placeholder + ? undefined + : allowArbitraryValues + ? 'Select an option or enter a custom value' + : 'Select an option' + }, [description, placeholder, allowArbitraryValues]) + + // Memoize the selected label calculation to avoid recalculation when items/value haven't changed + const selectedItemLabel = useMemo( + () => getSelectedLabelFromValue(items, field.value || ''), + [items, field.value] + ) + + // Memoize onChange callback to prevent unnecessary re-renders + const handleChange = useCallback( + (value: string) => { + field.onChange(value) + onChange?.(value) + }, + [field, onChange] + ) + + // Memoize onInputChange callback to prevent unnecessary re-renders + const handleInputChange = useCallback( + (value: string) => { + // If the user edits the input field, if arbitrary values are allowed, set the field's value; otherwise, clear the selected value + field.onChange(allowArbitraryValues ? value : undefined) + onInputChange?.(value) + }, + [field, allowArbitraryValues, onInputChange] ) return (
{ - field.onChange(value) - onChange?.(value) - setSelectedItemLabel(getSelectedLabelFromValue(items, value)) - }} + onChange={handleChange} + onInputChange={handleInputChange} allowArbitraryValues={allowArbitraryValues} inputRef={field.ref} transform={transform} diff --git a/app/components/form/fields/ImageSelectField.tsx b/app/components/form/fields/ImageSelectField.tsx index 13bb17663..c84bc8a5a 100644 --- a/app/components/form/fields/ImageSelectField.tsx +++ b/app/components/form/fields/ImageSelectField.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useCallback, useMemo } from 'react' import { useController, type Control } from 'react-hook-form' import type { Image } from '@oxide/api' @@ -31,6 +32,29 @@ export function BootDiskImageSelectField({ name, }: ImageSelectFieldProps) { const diskSizeField = useController({ control, name: 'bootDiskSize' }).field + + // Memoize the expensive toImageComboboxItem mapping to avoid recalculating on every render + const imageComboboxItems = useMemo( + () => images.map((i) => toImageComboboxItem(i)), + [images] + ) + + // Memoize onChange callback to prevent function recreation on every render + const handleChange = useCallback( + (id: string) => { + const image = images.find((i) => i.id === id) + // the most likely scenario where image would be undefined is if the user has + // manually cleared the ComboboxField; they will need to pick a boot disk image + // in order to submit the form, so we don't need to do anything here + if (!image) return + const imageSizeGiB = image.size / GiB + if (diskSizeField.value < imageSizeGiB) { + diskSizeField.onChange(diskSizeNearest10(imageSizeGiB)) + } + }, + [images, diskSizeField] + ) + return ( toImageComboboxItem(i))} + items={imageComboboxItems} required - onChange={(id) => { - const image = images.find((i) => i.id === id) - // the most likely scenario where image would be undefined is if the user has - // manually cleared the ComboboxField; they will need to pick a boot disk image - // in order to submit the form, so we don't need to do anything here - if (!image) return - const imageSizeGiB = image.size / GiB - if (diskSizeField.value < imageSizeGiB) { - diskSizeField.onChange(diskSizeNearest10(imageSizeGiB)) - } - }} + onChange={(id) => handleChange(id || '')} /> ) } +// Memoize expensive bytesToGiB calculations +const sizeCache = new Map() + +function getCachedSizeGiB(size: number): string { + if (!sizeCache.has(size)) { + sizeCache.set(size, `${bytesToGiB(size, 1)} GiB`) + } + return sizeCache.get(size)! +} + export function toImageComboboxItem( image: Image, includeProjectSiloIndicator = false ): ComboboxItem { const { id, name, os, projectId, size, version } = image - // for metadata showing in the dropdown's options, include the project / silo indicator if requested const projectSiloIndicator = includeProjectSiloIndicator ? `${projectId ? 'Project' : 'Silo'} image` : null - // filter out undefined metadata and create a ``-separated list for each comboboxitem - const itemMetadata = [os, version, `${bytesToGiB(size, 1)} GiB`, projectSiloIndicator] - .filter((i) => !!i) - .map((i, index) => ( - - {index > 0 ? : ''} - {i} - - )) + + // Build metadata array more efficiently + const metadata = [] + if (os) metadata.push(os) + if (version) metadata.push(version) + if (size) metadata.push(getCachedSizeGiB(size)) + if (projectSiloIndicator) metadata.push(projectSiloIndicator) + + const itemMetadata = metadata.map((item, index) => ( + + {index > 0 && } + {item} + + )) + return { value: id, selectedLabel: name, diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index ded6cec33..0a5522e39 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -20,10 +20,10 @@ import { type Image, } from '@oxide/api' +import { ComboboxField } from '~/components/form/fields/ComboboxField' import { DescriptionField } from '~/components/form/fields/DescriptionField' import { DiskSizeField } from '~/components/form/fields/DiskSizeField' import { toImageComboboxItem } from '~/components/form/fields/ImageSelectField' -import { ListboxField } from '~/components/form/fields/ListboxField' import { NameField } from '~/components/form/fields/NameField' import { RadioField } from '~/components/form/fields/RadioField' import { SideModalForm } from '~/components/form/SideModalForm' @@ -85,32 +85,66 @@ export function CreateDiskSideModalForm({ const projectImages = useApiQuery('imageList', { query: { project } }) const siloImages = useApiQuery('imageList', {}) + // Memoize real images array to prevent recreation on every render // put project images first because if there are any, there probably aren't // very many and they're probably relevant - const images = useMemo( + const realImages = useMemo( () => [...(projectImages.data?.items || []), ...(siloImages.data?.items || [])], - [projectImages.data, siloImages.data] + [projectImages.data?.items, siloImages.data?.items] ) + + // Memoize mock images array (only create once) + const mockImages = useMemo((): Image[] => { + // TODO: REMOVE THIS AFTER STRESS TESTING IS DONE + // Generate 1000 mock items for stress testing + return Array.from({ length: 1000 }, (_, i) => ({ + id: `mock-image-${i}`, + name: `Mock Image ${i.toString().padStart(4, '0')}`, + size: 1073741824, // 1GB + version: '1.0.0', + description: `This is mock image ${i} for stress testing the combobox`, + digest: { + type: 'sha256', + value: '0'.repeat(64), + }, + blockSize: 512, + timeCreated: new Date(), + timeModified: new Date(), + os: 'linux', + })) + }, []) + + // Memoize combined images array + const images = useMemo(() => [...realImages, ...mockImages], [realImages, mockImages]) const areImagesLoading = projectImages.isPending || siloImages.isPending const snapshotsQuery = useApiQuery('snapshotList', { query: { project } }) - const snapshots = snapshotsQuery.data?.items || [] + const snapshots = useMemo( + () => snapshotsQuery.data?.items || [], + [snapshotsQuery.data?.items] + ) - // validate disk source size - const diskSource = form.watch('diskSource').type + // Use useController for targeted watching instead of form.watch() to reduce re-renders + const diskSourceController = useController({ control: form.control, name: 'diskSource' }) + const diskSource = diskSourceController.field.value.type - let validateSizeGiB: number | undefined = undefined - if (diskSource === 'snapshot') { - const selectedSnapshotId = form.watch('diskSource.snapshotId') - const selectedSnapshotSize = snapshots.find( - (snapshot) => snapshot.id === selectedSnapshotId - )?.size - validateSizeGiB = selectedSnapshotSize ? bytesToGiB(selectedSnapshotSize) : undefined - } else if (diskSource === 'image') { - const selectedImageId = form.watch('diskSource.imageId') - const selectedImageSize = images.find((image) => image.id === selectedImageId)?.size - validateSizeGiB = selectedImageSize ? bytesToGiB(selectedImageSize) : undefined - } + // Memoize size validation to avoid expensive lookups on every render + const validateSizeGiB = useMemo(() => { + if (diskSource === 'snapshot') { + const selectedSnapshotId = diskSourceController.field.value.snapshotId + if (!selectedSnapshotId) return undefined + const selectedSnapshotSize = snapshots.find( + (snapshot) => snapshot.id === selectedSnapshotId + )?.size + return selectedSnapshotSize ? bytesToGiB(selectedSnapshotSize) : undefined + } else if (diskSource === 'image') { + const selectedImageId = diskSourceController.field.value.imageId + if (!selectedImageId) return undefined + const selectedImageSize = images.find((image) => image.id === selectedImageId)?.size + return selectedImageSize ? bytesToGiB(selectedImageSize) : undefined + } + return undefined + }, [diskSource, diskSourceController.field.value, snapshots, images]) return ( images.map((i) => toImageComboboxItem(i, true)), + [images] + ) + return ( <>
@@ -210,16 +250,17 @@ const DiskSourceField = ({ /> )} {value.type === 'image' && ( - toImageComboboxItem(i, true))} + items={imageComboboxItems} required onChange={(id) => { - const image = images.find((i) => i.id === id)! // if it's selected, it must be present + const image = images.find((i) => i.id === id) + if (!image) return const imageSizeGiB = image.size / GiB if (diskSizeField.value < imageSizeGiB) { diskSizeField.onChange(diskSizeNearest10(imageSizeGiB)) @@ -250,16 +291,16 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { const { project } = useProjectSelector() const snapshotsQuery = useApiQuery('snapshotList', { query: { project } }) - const snapshots = snapshotsQuery.data?.items || [] + const snapshots = useMemo( + () => snapshotsQuery.data?.items || [], + [snapshotsQuery.data?.items] + ) const diskSizeField = useController({ control, name: 'size' }).field - return ( - { + // Memoize the expensive snapshot ComboboxItem mapping to avoid recalculating on every render + const snapshotComboboxItems = useMemo( + () => + snapshots.map((i) => { const formattedSize = filesize(i.size, { base: 2, output: 'object' }) return { value: i.id, @@ -275,7 +316,17 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { ), } - })} + }), + [snapshots] + ) + + return ( + { diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index d146c0912..8b04571be 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -13,9 +13,19 @@ import { ComboboxOptions, Combobox as HCombobox, } from '@headlessui/react' -import cn from 'classnames' import { matchSorter } from 'match-sorter' -import { useEffect, useId, useState, type ReactNode, type Ref } from 'react' +import { + memo, + useCallback, + useEffect, + useId, + useMemo, + useState, + type ReactNode, + type Ref, +} from 'react' +import { FixedSizeList as List } from 'react-window' +import { useDebounce } from 'use-debounce' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' @@ -25,6 +35,63 @@ import { TextInputHint } from './TextInput' export type ComboboxItem = { value: string; label: ReactNode; selectedLabel: string } +// Memoized component for individual options - only re-renders when focus/selected actually changes for THIS item +const MemoizedComboboxOptionContent = memo(function ComboboxOptionContent({ + item, + focus, + selected, +}: { + item: ComboboxItem + focus: boolean + selected: boolean +}) { + return ( +
+ {item.label} +
+ ) +}) + +type VirtualizedItemProps = { + index: number + style: React.CSSProperties + data: { + items: ComboboxItem[] + selectedValue: string + onSelect: (value: string) => void + query: string + } +} + +// Virtualized item component for react-window +const VirtualizedItem = memo(function VirtualizedItem({ + index, + style, + data, +}: VirtualizedItemProps) { + const item = data.items[index] + const isSelected = item.value === data.selectedValue + + return ( +
+ data.onSelect(item.value)} + > + {({ focus, selected }) => ( + + )} + +
+ ) +}) + +// === UTILITY FUNCTIONS === /** 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,10 +103,23 @@ export const toComboboxItems = (items?: Array<{ name: string }>): Array, Map>() + export const getSelectedLabelFromValue = ( items: Array, selectedValue: string -): string => items.find((item) => item.value === selectedValue)?.selectedLabel || '' +): string => { + if (!labelLookupCache.has(items)) { + // Create lookup map for this items array + const lookupMap = new Map() + items.forEach((item) => lookupMap.set(item.value, item.selectedLabel)) + labelLookupCache.set(items, lookupMap) + } + + const lookupMap = labelLookupCache.get(items)! + return lookupMap.get(selectedValue) || '' +} /** Simple non-generic props shared with ComboboxField */ export type ComboboxBaseProps = { @@ -80,6 +160,7 @@ type ComboboxProps = { inputRef?: Ref } & ComboboxBaseProps +// === MAIN COMPONENT === export const Combobox = ({ description, items = [], @@ -100,12 +181,77 @@ export const Combobox = ({ transform, ...props }: ComboboxProps) => { - const [query, setQuery] = useState(selectedItemValue || '') - 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 - }) + const [query, setQuery] = useState(() => + selectedItemValue ? (allowArbitraryValues ? selectedItemValue : selectedItemLabel) : '' + ) + const [itemHeight, setItemHeight] = useState(null) // Will be measured from actual DOM + + // Debounce the query for filtering to reduce expensive operations while typing + const [debouncedQuery] = useDebounce(query, 150) + + // Memoize query processing to avoid recalculation on every render + const normalizedQuery = useMemo( + () => debouncedQuery.toLowerCase().replace(/\s*/g, ''), + [debouncedQuery] + ) + + // Memoize filtered items to avoid re-filtering on every render + const filteredItems = useMemo(() => { + return matchSorter(items, normalizedQuery, { + keys: ['selectedLabel', 'label'], + sorter: (items) => items, // preserve original order, don't sort by match + }) + }, [items, normalizedQuery]) + + // Memoize custom arbitrary value label to avoid recreation on every render + // Use immediate query (not debounced) so custom option appears immediately + const customValueItem = useMemo(() => { + if ( + !allowArbitraryValues || + query.length === 0 || + filteredItems.some((i) => i.selectedLabel === query) + ) { + return null + } + return { + value: query, + label: ( + <> + Custom: {query} + + ), + selectedLabel: query, + } + }, [allowArbitraryValues, query, filteredItems]) + + // Final items list with custom value if applicable + const finalFilteredItems = useMemo(() => { + const itemsWithCustomValue = customValueItem + ? [...filteredItems, customValueItem] + : filteredItems + return itemsWithCustomValue + }, [filteredItems, customValueItem]) + + // Limit items to 1 during measurement phase, all items after + const itemsToRender = useMemo(() => { + if (itemHeight === null) { + return finalFilteredItems.slice(0, 1) // Only first item for measurement + } + return finalFilteredItems // All items after measurement + }, [finalFilteredItems, itemHeight]) + + // Callback ref to measure item height immediately when element is available + const measureCallbackRef = useCallback( + (element: HTMLElement | null) => { + if (element && itemHeight === null) { + const height = element.getBoundingClientRect().height + if (height > 0) { + setItemHeight(Math.ceil(height)) + } + } + }, + [itemHeight] + ) // 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 @@ -126,33 +272,72 @@ export const Combobox = ({ } }, [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 && - !filteredItems.some((i) => i.selectedLabel === query) - ) { - filteredItems.push({ - value: query, - label: ( - <> - Custom: {query} - - ), - selectedLabel: query, - }) - } + // Memoize callbacks to prevent unnecessary re-renders; fallback to '' allows clearing field to work + const handleChange = useCallback((val: string | null) => onChange(val || ''), [onChange]) + const handleClose = useCallback(() => setQuery(''), []) + // We only want to keep the query on close when arbitrary values are allowed + const onCloseHandler = allowArbitraryValues ? undefined : handleClose + + // Memoize input onChange callback to prevent function recreation on every render + const handleInputChange = useCallback( + (event: React.ChangeEvent) => { + 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) + }, + [transform, onInputChange] + ) + + // Memoize onKeyDown callback to prevent function recreation on every render + const handleKeyDown = useCallback( + (e: React.KeyboardEvent, open: boolean) => { + // If the caller is using onEnter to override enter behavior, preventDefault + // in order to prevent the containing form from being submitted too. We don't + // need to do this when the combobox is open because that enter keypress is + // already handled internally (selects the highlighted item). So we only do + // this when the combobox is closed. + if (e.key === 'Enter' && onEnter && !open) { + e.preventDefault() + onEnter(e) + } + }, + [onEnter] + ) + + // Memoize itemData for virtual list to prevent unnecessary re-renders + const virtualItemData = useMemo( + () => ({ + items: finalFilteredItems, + selectedValue: selectedItemValue || '', + onSelect: handleChange, + query, + }), + [finalFilteredItems, selectedItemValue, handleChange, query] + ) + const zIndex = usePopoverZIndex() const id = useId() + + // Memoize input value computation to avoid recalculation on every render + // 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). + const inputValue = useMemo(() => { + return selectedItemValue + ? allowArbitraryValues + ? selectedItemValue + : selectedItemLabel || '' + : query || '' + }, [selectedItemValue, allowArbitraryValues, selectedItemLabel, query]) + return ( onChange(val || '')} - // we only want to keep the query on close when arbitrary values are allowed - onClose={allowArbitraryValues ? undefined : () => setQuery('')} + value={selectedItemValue || ''} + onChange={handleChange} + onClose={onCloseHandler} disabled={disabled || isLoading} immediate {...props} @@ -160,7 +345,6 @@ export const Combobox = ({ {({ open }) => (
{label && ( - // TODO: FieldLabel needs a real ID
)}
{ - 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) => { - // If the caller is using onEnter to override enter behavior, preventDefault - // in order to prevent the containing form from being submitted too. We don't - // need to do this when the combobox is open because that enter keypress is - // already handled internally (selects the highlighted item). So we only do - // this when the combobox is closed. - if (e.key === 'Enter' && onEnter && !open) { - e.preventDefault() - onEnter(e) - } - }} + value={inputValue} + onChange={handleInputChange} + onKeyDown={(e) => handleKeyDown(e, open)} placeholder={placeholder} disabled={disabled || isLoading} - className={cn( - `h-10 w-full rounded !border-none px-3 py-2 !outline-none text-sans-md text-raise placeholder:text-tertiary`, - disabled - ? 'cursor-not-allowed text-disabled bg-disabled !border-default' - : 'bg-default', - hasError && 'focus-error' - )} + className={`h-10 w-full rounded !border-none px-3 py-2 !outline-none text-sans-md text-raise placeholder:text-tertiary ${disabled ? 'cursor-not-allowed text-disabled bg-disabled !border-default' : 'bg-default'} ${hasError ? 'focus-error' : ''}`} /> {items.length > 0 && ( @@ -248,36 +396,46 @@ export const Combobox = ({ {(items.length > 0 || allowArbitraryValues) && ( - {filteredItems.map((item) => ( - - {({ 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. -
0 ? ( + itemHeight === null || finalFilteredItems.length <= 100 ? ( + // Regular rendering: 1 item during measurement, all items for small lists + itemsToRender.map((item) => ( + - {item.label} -
- )} - - ))} - {!allowArbitraryValues && filteredItems.length === 0 && ( - -
No items match
- + {({ focus, selected }) => ( + + )} + + )) + ) : ( + // Virtualized rendering for large lists (after measurement) + + {VirtualizedItem} + + ) + ) : ( + !allowArbitraryValues && ( + +
No items match
+ + ) )} )} diff --git a/app/ui/lib/FileInput.tsx b/app/ui/lib/FileInput.tsx index 38302be29..022f4a31b 100644 --- a/app/ui/lib/FileInput.tsx +++ b/app/ui/lib/FileInput.tsx @@ -36,6 +36,7 @@ export function FileInput({ onChange, error, ref, + id, ...inputProps }: FileInputProps) { const inputRef = useRef(null) @@ -63,8 +64,9 @@ export function FileInput({ } return ( -