From 5a2e2be38f693f5ac62b89fc64cf5f2a2dcfea89 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 18 May 2026 14:26:26 -0700 Subject: [PATCH 01/13] Add virtualization via Headless UI's virtual prop --- app/ui/lib/Combobox.tsx | 170 ++++++++++++++++++-------------- test/e2e/instance-create.e2e.ts | 7 +- 2 files changed, 102 insertions(+), 75 deletions(-) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index 0abe2b54c..687e6a888 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -15,7 +15,15 @@ import { } from '@headlessui/react' import cn from 'classnames' import { matchSorter } from 'match-sorter' -import { useEffect, useId, useRef, useState, type ReactNode, type Ref } from 'react' +import { + useEffect, + useId, + useMemo, + useRef, + useState, + type ReactNode, + type Ref, +} from 'react' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' @@ -25,6 +33,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). @@ -107,27 +131,15 @@ export const Combobox = ({ 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 +155,56 @@ 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 = useMemo(() => { + 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 + }, [items, selectedItemValue, allowArbitraryValues]) + 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 ( 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 + virtual={{ + options: virtualOptions, + // HUI types this with the same union as `value`, so item may be null + disabled: (item) => item?.value === NO_MATCH_VALUE, + }} {...props} > {({ open }) => { - // Sync open state to ref on render (handles the opening side) if (open) isOpenRef.current = true return (
@@ -201,19 +234,15 @@ 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} > { - // 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 +288,37 @@ 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. -
- {item.label} -
- )} - - ))} - {!allowArbitraryValues && filteredItems.length === 0 && ( - -
No items match
- - )} + {({ option }: { option: ComboboxItem }) => { + const isNoMatch = option.value === NO_MATCH_VALUE + return ( + + {({ focus, selected }) => ( +
+ {option.label} +
+ )} + + ) + }} )}
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index e7ed4d785..e9d1f0500 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/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') From 928eaa373c0ab386a6c9f0d2ef3efbeca1e6fbd2 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 20 May 2026 14:11:02 -0700 Subject: [PATCH 02/13] refactoring --- app/components/form/fields/ComboboxField.tsx | 12 +-------- app/ui/lib/Combobox.tsx | 27 ++++---------------- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index efa055829..e2c71bd4a 100644 --- a/app/components/form/fields/ComboboxField.tsx +++ b/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 (
{ field.onChange(value) onChange?.(value) - setSelectedItemLabel(getSelectedLabelFromValue(items, value)) }} allowArbitraryValues={allowArbitraryValues} inputRef={field.ref} diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index 687e6a888..1e308bc61 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -15,15 +15,7 @@ import { } from '@headlessui/react' import cn from 'classnames' import { matchSorter } from 'match-sorter' -import { - useEffect, - useId, - useMemo, - useRef, - useState, - type ReactNode, - type Ref, -} from 'react' +import { useEffect, useId, useRef, useState, type ReactNode, type Ref } from 'react' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' @@ -60,11 +52,6 @@ export const toComboboxItems = (items?: Array<{ name: string }>): Array, - selectedValue: string -): string => items.find((item) => item.value === selectedValue)?.selectedLabel || '' - /** Simple non-generic props shared with ComboboxField */ export type ComboboxBaseProps = { description?: React.ReactNode @@ -96,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 @@ -109,7 +95,6 @@ export const Combobox = ({ items = [], label, selectedItemValue, - selectedItemLabel, placeholder, required, hasError, @@ -122,10 +107,9 @@ 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 @@ -159,7 +143,7 @@ export const Combobox = ({ filteredItems.length === 0 && !allowArbitraryValues ? [NO_MATCH_ITEM] : filteredItems // Arbitrary values may not be in `items`, so synthesize a stand-in. - const selectedItem: ComboboxItem | null = useMemo(() => { + const selectedItem: ComboboxItem | null = (() => { if (!selectedItemValue) return null const found = items.find((i) => i.value === selectedItemValue) if (found) return found @@ -171,7 +155,7 @@ export const Combobox = ({ } } return null - }, [items, selectedItemValue, allowArbitraryValues]) + })() const zIndex = usePopoverZIndex() const id = useId() @@ -202,7 +186,6 @@ export const Combobox = ({ // HUI types this with the same union as `value`, so item may be null disabled: (item) => item?.value === NO_MATCH_VALUE, }} - {...props} > {({ open }) => { if (open) isOpenRef.current = true @@ -247,7 +230,7 @@ export const Combobox = ({ selectedItemValue ? allowArbitraryValues ? selectedItemValue - : selectedItemLabel + : (selectedItem?.selectedLabel ?? '') : query } onChange={(event) => { From 726f3ec8c1cb345f553790d824c1c47052da2e95 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 21 May 2026 17:59:10 -0500 Subject: [PATCH 03/13] Bump omicron to latest main (#3224) Just make the types pass. Followup PR will add new `contact_support` field on update status page. --- OMICRON_VERSION | 2 +- app/api/__generated__/Api.ts | 242 +++++++++++++++++--------- app/api/__generated__/OMICRON_VERSION | 2 +- app/api/__generated__/msw-handlers.ts | 56 ++++++ app/api/__generated__/validate.ts | 189 ++++++++++++++------ mock-api/msw/handlers.ts | 4 + mock-api/system-update.ts | 1 + 7 files changed, 360 insertions(+), 136 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 5d15d27be..e91b1c476 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -254a0c51bc0beecb79c8a9dfccce8e7bc35b5ca4 +985304a607b16191bdc35e0ea87fa522d925b514 diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index 5edbefb34..7e88135aa 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -945,6 +945,23 @@ export type BgpImported = { switch: SwitchSlot } +/** + * Router lifetime in seconds for unnumbered BGP peers + */ +export type RouterLifetimeConfig = number + +export type RouterPeerType = + | { + /** Router lifetime in seconds for unnumbered BGP peers. */ + routerLifetime: RouterLifetimeConfig + type: 'unnumbered' + } + | { + /** IP address for numbered BGP peers. */ + ip: string + type: 'numbered' + } + /** * Define policy relating to the import and export of prefixes from a BGP peer. */ @@ -952,11 +969,11 @@ export type ImportExportPolicy = /** Do not perform any filtering. */ { type: 'no_filtering' } | { type: 'allow'; value: IpNet[] } /** - * A BGP peer configuration for an interface. Includes the set of announcements that will be advertised to the peer identified by `addr`. The `bgp_config` parameter is a reference to global BGP parameters. The `interface_name` indicates what interface the peer should be contacted on. + * A BGP peer configuration for an interface. Includes the set of announcements that will be advertised to the peer. The `bgp_config` parameter is a reference to global BGP parameters. */ export type BgpPeer = { - /** The address of the host to peer with. If not provided, this is an unnumbered BGP session that will be established over the interface specified by `interface_name`. */ - addr?: string | null + /** The address of the host to peer with, or specifying the configuration of an unnumbered BGP session. */ + addr: RouterPeerType /** Define export policy for a peer. */ allowedExport: ImportExportPolicy /** Define import policy for a peer. */ @@ -965,7 +982,7 @@ export type BgpPeer = { bgpConfig: NameOrId /** Include the provided communities in updates sent to the peer. */ communities: number[] - /** How long to to wait between TCP connection retries (seconds). */ + /** How long to wait between TCP connection retries (seconds). */ connectRetry: number /** How long to delay sending an open request after establishing a TCP session (seconds). */ delayOpen: number @@ -975,8 +992,6 @@ export type BgpPeer = { holdTime: number /** How long to hold a peer in idle before attempting a new session (seconds). */ idleHoldTime: number - /** The name of interface to peer on. This is relative to the port configuration this BGP peer configuration is a part of. For example this value could be phy0 to refer to a primary physical interface. Or it could be vlan47 to refer to a VLAN interface. */ - interfaceName: Name /** How often to send keepalive requests (seconds). */ keepalive: number /** Apply a local preference to routes received from this peer. */ @@ -989,8 +1004,6 @@ export type BgpPeer = { multiExitDiscriminator?: number | null /** Require that a peer has a specified ASN. */ remoteAsn?: number | null - /** Router lifetime in seconds for unnumbered BGP peers. */ - routerLifetime: number /** Associate a VLAN ID with a peer. */ vlanId?: number | null } @@ -3625,6 +3638,36 @@ export type PhysicalDisk = { vendor: string } +/** + * The unique identity of a physical disk provided by the manufacturer + */ +export type PhysicalDiskManufacturerIdentity = { + model: string + serial: string + vendor: string +} + +export type PhysicalDiskAdoptionRequestUuid = string + +/** + * A request to adopt a physical disk into the control plane + */ +export type PhysicalDiskAdoptionRequest = { + diskId: PhysicalDiskManufacturerIdentity + id: PhysicalDiskAdoptionRequestUuid + timeCreated: Date +} + +/** + * A single page of results + */ +export type PhysicalDiskAdoptionRequestResultsPage = { + /** list of items on this page of results */ + items: PhysicalDiskAdoptionRequest[] + /** token used to fetch the next page of results (if any) */ + nextPage?: string | null +} + /** * A single page of results */ @@ -3643,7 +3686,7 @@ export type Ping = { } /** - * Identity-related metadata that's included in nearly all public API objects + * A networking probe */ export type Probe = { /** Human-readable free-form text about a resource */ @@ -3944,7 +3987,7 @@ export type RouterRouteUpdate = { } /** - * Identity-related metadata that's included in nearly all public API objects + * A SAML identity provider */ export type SamlIdentityProvider = { /** Service provider endpoint where the response will be sent */ @@ -4411,6 +4454,8 @@ export type SledResultsPage = { nextPage?: string | null } +export type SledUuid = string + export type SnapshotState = 'creating' | 'ready' | 'faulted' | 'destroyed' /** @@ -4702,17 +4747,19 @@ export type Switch = { } /** - * Describes the kind of an switch interface. + * Indicates the kind for a switch interface. */ -export type SwitchInterfaceKind2 = +export type SwitchInterfaceKind = /** Primary interfaces are associated with physical links. There is exactly one primary interface per physical link. */ - | 'primary' - + | { type: 'primary' } /** VLAN interfaces allow physical interfaces to be multiplexed onto multiple logical links, each distinguished by a 12-bit 802.1Q Ethernet tag. */ - | 'vlan' - + | { + type: 'vlan' + /** The virtual network id (VID) that distinguishes this interface and is used for producing and consuming 802.1Q Ethernet tags. This field has a maximum value of 4095 as 802.1Q tags are twelve bits. */ + vid: number + } /** Loopback interfaces are anchors for IP addresses that are not specific to any particular port. */ - | 'loopback' + | { type: 'loopback' } /** * A switch port interface configuration for a port settings object. @@ -4723,28 +4770,13 @@ export type SwitchInterfaceConfig = { /** The name of this switch interface. */ interfaceName: Name /** The switch interface kind. */ - kind: SwitchInterfaceKind2 + kind: SwitchInterfaceKind /** The port settings object this switch interface configuration belongs to. */ portSettingsId: string /** Whether or not IPv6 is enabled on this interface. */ v6Enabled: boolean } -/** - * Indicates the kind for a switch interface. - */ -export type SwitchInterfaceKind = - /** Primary interfaces are associated with physical links. There is exactly one primary interface per physical link. */ - | { type: 'primary' } - /** VLAN interfaces allow physical interfaces to be multiplexed onto multiple logical links, each distinguished by a 12-bit 802.1Q Ethernet tag. */ - | { - type: 'vlan' - /** The virtual network id (VID) that distinguishes this interface and is used for producing and consuming 802.1Q Ethernet tags. This field has a maximum value of 4095 as 802.1Q tags are twelve bits. */ - vid: number - } - /** Loopback interfaces are anchors for IP addresses that are not specific to any particular port. */ - | { type: 'loopback' } - /** * A layer-3 switch interface configuration. When IPv6 is enabled, a link local address will be created for the interface. */ @@ -4806,7 +4838,7 @@ export type SwitchPortApplySettings = { /** * The link geometry associated with a switch port. */ -export type SwitchPortGeometry2 = +export type SwitchPortGeometry = /** The port contains a single QSFP28 link with four lanes. */ | 'qsfp28x1' @@ -4821,24 +4853,11 @@ export type SwitchPortGeometry2 = */ export type SwitchPortConfig = { /** The physical link geometry of the port. */ - geometry: SwitchPortGeometry2 + geometry: SwitchPortGeometry /** The id of the port settings object this configuration belongs to. */ portSettingsId: string } -/** - * The link geometry associated with a switch port. - */ -export type SwitchPortGeometry = - /** The port contains a single QSFP28 link with four lanes. */ - | 'qsfp28x1' - - /** The port contains two QSFP28 links each with two lanes. */ - | 'qsfp28x2' - - /** The port contains four SFP28 links each with one lane. */ - | 'sfp28x4' - /** * Physical switch port configuration. */ @@ -4847,22 +4866,6 @@ export type SwitchPortConfigCreate = { geometry: SwitchPortGeometry } -/** - * Per-port tx-eq overrides. This can be used to fine-tune the transceiver equalization settings to improve signal integrity. - */ -export type TxEqConfig2 = { - /** Main tap */ - main?: number | null - /** Post-cursor tap1 */ - post1?: number | null - /** Post-cursor tap2 */ - post2?: number | null - /** Pre-cursor tap1 */ - pre1?: number | null - /** Pre-cursor tap2 */ - pre2?: number | null -} - /** * A link configuration for a port settings object. */ @@ -4882,7 +4885,7 @@ export type SwitchPortLinkConfig = { /** The configured speed of the link. */ speed: LinkSpeed /** The tx_eq configuration for this link. */ - txEqConfig?: TxEqConfig2 | null + txEqConfig?: TxEqConfig | null } /** @@ -4923,16 +4926,6 @@ export type SwitchPortSettingsGroups = { portSettingsId: string } -/** - * A switch port VLAN interface configuration for a port settings object. - */ -export type SwitchVlanInterfaceConfig = { - /** The switch interface configuration this VLAN interface configuration belongs to. */ - interfaceConfigId: string - /** The virtual network id for this interface that is used for producing and consuming 802.1Q Ethernet tags. This field has a maximum value of 4095 as 802.1Q tags are twelve bits. */ - vlanId: number -} - /** * This structure contains all port settings information in one place. It's a convenience data structure for getting a complete view of a particular port's settings. */ @@ -4961,8 +4954,6 @@ export type SwitchPortSettings = { timeCreated: Date /** Timestamp when this resource was last modified */ timeModified: Date - /** Vlan interface settings. */ - vlanInterfaces: SwitchVlanInterfaceConfig[] } /** @@ -5063,6 +5054,7 @@ export type Units = | 'amps' | 'watts' | 'degrees_celsius' + | 'joules' /** No meaningful units, e.g. a dimensionless quanity. */ | 'none' @@ -5135,6 +5127,26 @@ export type TufRepoUploadStatus = /** The repository already existed in the data export type TufRepoUpload = { repo: TufRepo; status: TufRepoUploadStatus } +/** + * A physical disk that has not yet been adopted by the control plane + */ +export type UnadoptedPhysicalDisk = { + diskId: PhysicalDiskManufacturerIdentity + sledId: SledUuid + slot: number + variant: PhysicalDiskKind +} + +/** + * A single page of results + */ +export type UnadoptedPhysicalDiskResultsPage = { + /** list of items on this page of results */ + items: UnadoptedPhysicalDisk[] + /** token used to fetch the next page of results (if any) */ + nextPage?: string | null +} + /** * A sled that has not been added to an initialized rack yet */ @@ -5157,6 +5169,10 @@ Keys will be either: * Semver-like release version strings * "install dataset", representing the initial rack software before any updates * "unknown", which means there is no TUF repo uploaded that matches the software running on the component) */ componentsByReleaseVersion: Record + /** If true, the system has detected one or more known conditions that require Oxide support to resolve + +You should contact support to resolve these issues before proceeding with an update, or after one has completed. The checks underlying this field are not exhaustive, so this being `false` does not mean the entire system is completely healthy. */ + contactSupport: boolean /** Whether automatic update is suspended due to manual update activity After a manual support procedure that changes the system software, automatic update activity is suspended to avoid undoing the change. To resume automatic update, first upload the TUF repository matching the manually applied update, then set that as the target release. */ @@ -6662,6 +6678,16 @@ export interface AuditLogListQueryParams { startTime?: Date } +export interface PhysicalDiskDisableAdoptionPathParams { + physicalDiskAdoptionReqId: string +} + +export interface PhysicalDiskListAdoptionRequestsQueryParams { + limit?: number | null + pageToken?: string | null + sortBy?: IdSortMode +} + export interface PhysicalDiskListQueryParams { limit?: number | null pageToken?: string | null @@ -6672,6 +6698,11 @@ export interface PhysicalDiskViewPathParams { diskId: string } +export interface PhysicalDiskListUnadoptedQueryParams { + limit?: number | null + pageToken?: string | null +} + export interface NetworkingSwitchPortLldpNeighborsPathParams { port: Name rackId: string @@ -7482,7 +7513,7 @@ export class Api { * Pulled from info.version in the OpenAPI schema. Sent in the * `api-version` header on all requests. */ - apiVersion = '2026032500.0.0' + apiVersion = '2026052000.0.0' constructor({ host = '', baseParams = {}, token }: ApiConfig = {}) { this.host = host @@ -9950,6 +9981,47 @@ export class Api { ...params, }) }, + /** + * Enable adoption of a physical disk for general use + */ + physicalDiskEnableAdoption: ( + { body }: { body: PhysicalDiskManufacturerIdentity }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/hardware/disk-adoption-request`, + method: 'PUT', + body, + ...params, + }) + }, + /** + * Disable adoption of a physical disk for general use + */ + physicalDiskDisableAdoption: ( + { path }: { path: PhysicalDiskDisableAdoptionPathParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/hardware/disk-adoption-request/${path.physicalDiskAdoptionReqId}`, + method: 'DELETE', + ...params, + }) + }, + /** + * List physical disk adoption requests + */ + physicalDiskListAdoptionRequests: ( + { query = {} }: { query?: PhysicalDiskListAdoptionRequestsQueryParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/hardware/disk-adoption-requests`, + method: 'GET', + query, + ...params, + }) + }, /** * List physical disks */ @@ -9977,6 +10049,20 @@ export class Api { ...params, }) }, + /** + * List physical disks that have not yet been adopted for use + */ + physicalDiskListUnadopted: ( + { query = {} }: { query?: PhysicalDiskListUnadoptedQueryParams }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/system/hardware/disks-unadopted`, + method: 'GET', + query, + ...params, + }) + }, /** * Fetch LLDP neighbors for switch port */ diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index b8bd5a888..3648028c0 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -254a0c51bc0beecb79c8a9dfccce8e7bc35b5ca4 +985304a607b16191bdc35e0ea87fa522d925b514 diff --git a/app/api/__generated__/msw-handlers.ts b/app/api/__generated__/msw-handlers.ts index df98e9e1a..d888fea05 100644 --- a/app/api/__generated__/msw-handlers.ts +++ b/app/api/__generated__/msw-handlers.ts @@ -1053,6 +1053,24 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `PUT /v1/system/hardware/disk-adoption-request` */ + physicalDiskEnableAdoption: (params: { + body: Json + req: Request + cookies: Record + }) => Promisable> + /** `DELETE /v1/system/hardware/disk-adoption-request/:physicalDiskAdoptionReqId` */ + physicalDiskDisableAdoption: (params: { + path: Api.PhysicalDiskDisableAdoptionPathParams + req: Request + cookies: Record + }) => Promisable + /** `GET /v1/system/hardware/disk-adoption-requests` */ + physicalDiskListAdoptionRequests: (params: { + query: Api.PhysicalDiskListAdoptionRequestsQueryParams + req: Request + cookies: Record + }) => Promisable> /** `GET /v1/system/hardware/disks` */ physicalDiskList: (params: { query: Api.PhysicalDiskListQueryParams @@ -1065,6 +1083,12 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `GET /v1/system/hardware/disks-unadopted` */ + physicalDiskListUnadopted: (params: { + query: Api.PhysicalDiskListUnadoptedQueryParams + req: Request + cookies: Record + }) => Promisable> /** `GET /v1/system/hardware/rack-switch-port/:rackId/:switchSlot/:port/lldp/neighbors` */ networkingSwitchPortLldpNeighbors: (params: { path: Api.NetworkingSwitchPortLldpNeighborsPathParams @@ -2984,6 +3008,30 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/system/audit-log', handler(handlers['auditLogList'], schema.AuditLogListParams, null) ), + http.put( + '/v1/system/hardware/disk-adoption-request', + handler( + handlers['physicalDiskEnableAdoption'], + null, + schema.PhysicalDiskManufacturerIdentity + ) + ), + http.delete( + '/v1/system/hardware/disk-adoption-request/:physicalDiskAdoptionReqId', + handler( + handlers['physicalDiskDisableAdoption'], + schema.PhysicalDiskDisableAdoptionParams, + null + ) + ), + http.get( + '/v1/system/hardware/disk-adoption-requests', + handler( + handlers['physicalDiskListAdoptionRequests'], + schema.PhysicalDiskListAdoptionRequestsParams, + null + ) + ), http.get( '/v1/system/hardware/disks', handler(handlers['physicalDiskList'], schema.PhysicalDiskListParams, null) @@ -2992,6 +3040,14 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/system/hardware/disks/:diskId', handler(handlers['physicalDiskView'], schema.PhysicalDiskViewParams, null) ), + http.get( + '/v1/system/hardware/disks-unadopted', + handler( + handlers['physicalDiskListUnadopted'], + schema.PhysicalDiskListUnadoptedParams, + null + ) + ), http.get( '/v1/system/hardware/rack-switch-port/:rackId/:switchSlot/:port/lldp/neighbors', handler( diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index 0c46dc8e5..1985c8d2a 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -878,6 +878,22 @@ export const BgpImported = z.preprocess( }) ) +/** + * Router lifetime in seconds for unnumbered BGP peers + */ +export const RouterLifetimeConfig = z.preprocess( + processResponseBody, + z.number().min(0).max(9000) +) + +export const RouterPeerType = z.preprocess( + processResponseBody, + z.union([ + z.object({ routerLifetime: RouterLifetimeConfig, type: z.enum(['unnumbered']) }), + z.object({ ip: z.union([z.ipv4(), z.ipv6()]), type: z.enum(['numbered']) }), + ]) +) + /** * Define policy relating to the import and export of prefixes from a BGP peer. */ @@ -890,12 +906,12 @@ export const ImportExportPolicy = z.preprocess( ) /** - * A BGP peer configuration for an interface. Includes the set of announcements that will be advertised to the peer identified by `addr`. The `bgp_config` parameter is a reference to global BGP parameters. The `interface_name` indicates what interface the peer should be contacted on. + * A BGP peer configuration for an interface. Includes the set of announcements that will be advertised to the peer. The `bgp_config` parameter is a reference to global BGP parameters. */ export const BgpPeer = z.preprocess( processResponseBody, z.object({ - addr: z.union([z.ipv4(), z.ipv6()]).nullable().optional(), + addr: RouterPeerType, allowedExport: ImportExportPolicy, allowedImport: ImportExportPolicy, bgpConfig: NameOrId, @@ -905,14 +921,12 @@ export const BgpPeer = z.preprocess( enforceFirstAs: SafeBoolean, holdTime: z.number().min(0).max(4294967295), idleHoldTime: z.number().min(0).max(4294967295), - interfaceName: Name, keepalive: z.number().min(0).max(4294967295), localPref: z.number().min(0).max(4294967295).nullable().optional(), md5AuthKey: z.string().nullable().optional(), minTtl: z.number().min(0).max(255).nullable().optional(), multiExitDiscriminator: z.number().min(0).max(4294967295).nullable().optional(), remoteAsn: z.number().min(0).max(4294967295).nullable().optional(), - routerLifetime: z.number().min(0).max(65535), vlanId: z.number().min(0).max(65535).nullable().optional(), }) ) @@ -3331,6 +3345,39 @@ export const PhysicalDisk = z.preprocess( }) ) +/** + * The unique identity of a physical disk provided by the manufacturer + */ +export const PhysicalDiskManufacturerIdentity = z.preprocess( + processResponseBody, + z.object({ model: z.string(), serial: z.string(), vendor: z.string() }) +) + +export const PhysicalDiskAdoptionRequestUuid = z.preprocess(processResponseBody, z.uuid()) + +/** + * A request to adopt a physical disk into the control plane + */ +export const PhysicalDiskAdoptionRequest = z.preprocess( + processResponseBody, + z.object({ + diskId: PhysicalDiskManufacturerIdentity, + id: PhysicalDiskAdoptionRequestUuid, + timeCreated: z.coerce.date(), + }) +) + +/** + * A single page of results + */ +export const PhysicalDiskAdoptionRequestResultsPage = z.preprocess( + processResponseBody, + z.object({ + items: PhysicalDiskAdoptionRequest.array(), + nextPage: z.string().nullable().optional(), + }) +) + /** * A single page of results */ @@ -3344,7 +3391,7 @@ export const PingStatus = z.preprocess(processResponseBody, z.enum(['ok'])) export const Ping = z.preprocess(processResponseBody, z.object({ status: PingStatus })) /** - * Identity-related metadata that's included in nearly all public API objects + * A networking probe */ export const Probe = z.preprocess( processResponseBody, @@ -3633,7 +3680,7 @@ export const RouterRouteUpdate = z.preprocess( ) /** - * Identity-related metadata that's included in nearly all public API objects + * A SAML identity provider */ export const SamlIdentityProvider = z.preprocess( processResponseBody, @@ -4026,6 +4073,8 @@ export const SledResultsPage = z.preprocess( z.object({ items: Sled.array(), nextPage: z.string().nullable().optional() }) ) +export const SledUuid = z.preprocess(processResponseBody, z.uuid()) + export const SnapshotState = z.preprocess( processResponseBody, z.enum(['creating', 'ready', 'faulted', 'destroyed']) @@ -4277,11 +4326,15 @@ export const Switch = z.preprocess( ) /** - * Describes the kind of an switch interface. + * Indicates the kind for a switch interface. */ -export const SwitchInterfaceKind2 = z.preprocess( +export const SwitchInterfaceKind = z.preprocess( processResponseBody, - z.enum(['primary', 'vlan', 'loopback']) + z.union([ + z.object({ type: z.enum(['primary']) }), + z.object({ type: z.enum(['vlan']), vid: z.number().min(0).max(65535) }), + z.object({ type: z.enum(['loopback']) }), + ]) ) /** @@ -4292,24 +4345,12 @@ export const SwitchInterfaceConfig = z.preprocess( z.object({ id: z.uuid(), interfaceName: Name, - kind: SwitchInterfaceKind2, + kind: SwitchInterfaceKind, portSettingsId: z.uuid(), v6Enabled: SafeBoolean, }) ) -/** - * Indicates the kind for a switch interface. - */ -export const SwitchInterfaceKind = z.preprocess( - processResponseBody, - z.union([ - z.object({ type: z.enum(['primary']) }), - z.object({ type: z.enum(['vlan']), vid: z.number().min(0).max(65535) }), - z.object({ type: z.enum(['loopback']) }), - ]) -) - /** * A layer-3 switch interface configuration. When IPv6 is enabled, a link local address will be created for the interface. */ @@ -4364,7 +4405,7 @@ export const SwitchPortApplySettings = z.preprocess( /** * The link geometry associated with a switch port. */ -export const SwitchPortGeometry2 = z.preprocess( +export const SwitchPortGeometry = z.preprocess( processResponseBody, z.enum(['qsfp28x1', 'qsfp28x2', 'sfp28x4']) ) @@ -4374,15 +4415,7 @@ export const SwitchPortGeometry2 = z.preprocess( */ export const SwitchPortConfig = z.preprocess( processResponseBody, - z.object({ geometry: SwitchPortGeometry2, portSettingsId: z.uuid() }) -) - -/** - * The link geometry associated with a switch port. - */ -export const SwitchPortGeometry = z.preprocess( - processResponseBody, - z.enum(['qsfp28x1', 'qsfp28x2', 'sfp28x4']) + z.object({ geometry: SwitchPortGeometry, portSettingsId: z.uuid() }) ) /** @@ -4393,20 +4426,6 @@ export const SwitchPortConfigCreate = z.preprocess( z.object({ geometry: SwitchPortGeometry }) ) -/** - * Per-port tx-eq overrides. This can be used to fine-tune the transceiver equalization settings to improve signal integrity. - */ -export const TxEqConfig2 = z.preprocess( - processResponseBody, - z.object({ - main: z.number().min(-2147483647).max(2147483647).nullable().optional(), - post1: z.number().min(-2147483647).max(2147483647).nullable().optional(), - post2: z.number().min(-2147483647).max(2147483647).nullable().optional(), - pre1: z.number().min(-2147483647).max(2147483647).nullable().optional(), - pre2: z.number().min(-2147483647).max(2147483647).nullable().optional(), - }) -) - /** * A link configuration for a port settings object. */ @@ -4420,7 +4439,7 @@ export const SwitchPortLinkConfig = z.preprocess( mtu: z.number().min(0).max(65535), portSettingsId: z.uuid(), speed: LinkSpeed, - txEqConfig: TxEqConfig2.nullable().optional(), + txEqConfig: TxEqConfig.nullable().optional(), }) ) @@ -4455,14 +4474,6 @@ export const SwitchPortSettingsGroups = z.preprocess( z.object({ portSettingsGroupId: z.uuid(), portSettingsId: z.uuid() }) ) -/** - * A switch port VLAN interface configuration for a port settings object. - */ -export const SwitchVlanInterfaceConfig = z.preprocess( - processResponseBody, - z.object({ interfaceConfigId: z.uuid(), vlanId: z.number().min(0).max(65535) }) -) - /** * This structure contains all port settings information in one place. It's a convenience data structure for getting a complete view of a particular port's settings. */ @@ -4481,7 +4492,6 @@ export const SwitchPortSettings = z.preprocess( routes: SwitchPortRouteConfig.array(), timeCreated: z.coerce.date(), timeModified: z.coerce.date(), - vlanInterfaces: SwitchVlanInterfaceConfig.array(), }) ) @@ -4594,6 +4604,7 @@ export const Units = z.preprocess( 'amps', 'watts', 'degrees_celsius', + 'joules', ]), z.enum(['none']), z.enum(['rpm']), @@ -4665,6 +4676,30 @@ export const TufRepoUpload = z.preprocess( z.object({ repo: TufRepo, status: TufRepoUploadStatus }) ) +/** + * A physical disk that has not yet been adopted by the control plane + */ +export const UnadoptedPhysicalDisk = z.preprocess( + processResponseBody, + z.object({ + diskId: PhysicalDiskManufacturerIdentity, + sledId: SledUuid, + slot: z.number(), + variant: PhysicalDiskKind, + }) +) + +/** + * A single page of results + */ +export const UnadoptedPhysicalDiskResultsPage = z.preprocess( + processResponseBody, + z.object({ + items: UnadoptedPhysicalDisk.array(), + nextPage: z.string().nullable().optional(), + }) +) + /** * A sled that has not been added to an initialized rack yet */ @@ -4685,6 +4720,7 @@ export const UpdateStatus = z.preprocess( processResponseBody, z.object({ componentsByReleaseVersion: z.record(z.string(), z.number().min(0)), + contactSupport: SafeBoolean, suspended: SafeBoolean, targetRelease: TargetRelease.nullable(), timeLastStepPlanned: z.coerce.date(), @@ -6968,6 +7004,36 @@ export const AuditLogListParams = z.preprocess( }) ) +export const PhysicalDiskEnableAdoptionParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({}), + query: z.object({}), + }) +) + +export const PhysicalDiskDisableAdoptionParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({ + physicalDiskAdoptionReqId: z.uuid(), + }), + query: z.object({}), + }) +) + +export const PhysicalDiskListAdoptionRequestsParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({}), + query: z.object({ + limit: z.number().min(1).max(4294967295).nullable().optional(), + pageToken: z.string().nullable().optional(), + sortBy: IdSortMode.optional(), + }), + }) +) + export const PhysicalDiskListParams = z.preprocess( processResponseBody, z.object({ @@ -6990,6 +7056,17 @@ export const PhysicalDiskViewParams = z.preprocess( }) ) +export const PhysicalDiskListUnadoptedParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({}), + query: z.object({ + limit: z.number().min(1).max(4294967295).nullable().optional(), + pageToken: z.string().nullable().optional(), + }), + }) +) + export const NetworkingSwitchPortLldpNeighborsParams = z.preprocess( processResponseBody, z.object({ diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 9f00f10ca..96a0dd31e 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -2631,6 +2631,10 @@ export const handlers = makeHandlers({ networkingSwitchPortSettingsList: NotImplemented, networkingSwitchPortSettingsView: NotImplemented, networkingSwitchPortStatus: NotImplemented, + physicalDiskDisableAdoption: NotImplemented, + physicalDiskEnableAdoption: NotImplemented, + physicalDiskListAdoptionRequests: NotImplemented, + physicalDiskListUnadopted: NotImplemented, physicalDiskView: NotImplemented, probeCreate: NotImplemented, probeDelete: NotImplemented, diff --git a/mock-api/system-update.ts b/mock-api/system-update.ts index bed256a4f..5285352ef 100644 --- a/mock-api/system-update.ts +++ b/mock-api/system-update.ts @@ -36,6 +36,7 @@ export const updateStatus: Json = { '17.0.0': 12, '16.0.0': 5, }, + contact_support: false, suspended: false, target_release: { version: '17.0.0', From 7b27814ce2bc10cdc2ec6a2b6ab6f5a2a2c13f68 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 27 May 2026 11:06:19 -0700 Subject: [PATCH 04/13] e2e adjustments --- test/e2e/instance-create.e2e.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index e9d1f0500..1055757b7 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -574,6 +574,15 @@ test('attaching additional disks allows for combobox filtering', async ({ page } await expect(page.getByRole('option', { name: 'disk-0007' })).toBeVisible() await expect(page.getByRole('option').first()).toHaveAttribute('aria-setsize', /\d{2,}/) + // Pressing End jumps the active option to the last entry, which forces + // the virtualizer to mount it. disk-0988 is the last detached + // (attachable) disk in the seeded set and isn't in the DOM on first + // open — toBeHidden confirms we're genuinely virtualizing rather than + // just rendering everything. + await expect(page.getByRole('option', { name: 'disk-0988' })).toBeHidden() + await selectADisk.press('End') + await expect(page.getByRole('option', { name: 'disk-0988' })).toBeVisible() + // type in a string to use as a filter await selectADisk.fill('disk-02') // only disks with that substring should be shown @@ -583,12 +592,15 @@ test('attaching additional disks allows for combobox filtering', async ({ page } await expect(page.getByRole('option', { name: 'disk-0220' })).toBeHidden() await expect(page.getByRole('option', { name: 'disk-1000' })).toBeHidden() - // select one - await page.getByRole('option', { name: 'disk-0211' }).click() + // Filter down to a single late-in-the-list disk and select it. This + // guards against a virtualization regression where filter results past + // the initial render window can be seen but not actually clicked. + await selectADisk.fill('disk-0988') + await page.getByRole('option', { name: 'disk-0988' }).click() // now options hidden and only the selected one is visible in the button/input await expect(page.getByRole('option')).toBeHidden() - await expect(page.getByRole('combobox', { name: 'Disk name' })).toHaveValue('disk-0211') + await expect(page.getByRole('combobox', { name: 'Disk name' })).toHaveValue('disk-0988') // a random string should give a disabled option await selectADisk.click() From 01019f04c4a209a45985fd8edca3611bf7f51b2d Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Wed, 27 May 2026 11:57:29 -0700 Subject: [PATCH 05/13] refactoring --- app/ui/lib/Combobox.tsx | 32 +++++++++++++++++++------------- test/e2e/instance-create.e2e.ts | 3 +-- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index 1e308bc61..8759c8a20 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -15,7 +15,15 @@ import { } from '@headlessui/react' import cn from 'classnames' import { matchSorter } from 'match-sorter' -import { useEffect, useId, useRef, useState, type ReactNode, type Ref } from 'react' +import { + useEffect, + useId, + useMemo, + useRef, + useState, + type ReactNode, + type Ref, +} from 'react' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' @@ -33,6 +41,8 @@ const NO_MATCH_ITEM: ComboboxItem = { label: 'No items match', selectedLabel: '', } +// HUI types `disabled` with the same union as `value`, so item may be null +const isNoMatch = (item: ComboboxItem | null) => item?.value === NO_MATCH_VALUE // 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 @@ -143,7 +153,7 @@ export const Combobox = ({ filteredItems.length === 0 && !allowArbitraryValues ? [NO_MATCH_ITEM] : filteredItems // Arbitrary values may not be in `items`, so synthesize a stand-in. - const selectedItem: ComboboxItem | null = (() => { + const selectedItem = useMemo(() => { if (!selectedItemValue) return null const found = items.find((i) => i.value === selectedItemValue) if (found) return found @@ -155,7 +165,7 @@ export const Combobox = ({ } } return null - })() + }, [items, selectedItemValue, allowArbitraryValues]) const zIndex = usePopoverZIndex() const id = useId() @@ -181,11 +191,7 @@ export const Combobox = ({ }} disabled={disabled || isLoading} immediate - virtual={{ - options: virtualOptions, - // HUI types this with the same union as `value`, so item may be null - disabled: (item) => item?.value === NO_MATCH_VALUE, - }} + virtual={{ options: virtualOptions, disabled: isNoMatch }} > {({ open }) => { if (open) isOpenRef.current = true @@ -278,11 +284,11 @@ export const Combobox = ({ modal={false} > {({ option }: { option: ComboboxItem }) => { - const isNoMatch = option.value === NO_MATCH_VALUE + const noMatch = option.value === NO_MATCH_VALUE return ( (
{option.label} diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 1055757b7..20bea9ef7 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -568,8 +568,7 @@ test('attaching additional disks allows for combobox filtering', async ({ page } await selectADisk.click() // 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). + // aria-setsize reports the full attachable count. await expect(page.getByRole('option', { name: 'disk-0005' })).toBeVisible() await expect(page.getByRole('option', { name: 'disk-0007' })).toBeVisible() await expect(page.getByRole('option').first()).toHaveAttribute('aria-setsize', /\d{2,}/) From 1b6033f48d7a859e2e5914001394ebd8a2ee6f5b Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 28 May 2026 10:26:42 -0700 Subject: [PATCH 06/13] refactoring --- app/ui/lib/Combobox.tsx | 16 +++++----------- test/e2e/instance-create.e2e.ts | 8 ++++---- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index 8759c8a20..e54fde98e 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -14,7 +14,6 @@ import { Combobox as HCombobox, } from '@headlessui/react' import cn from 'classnames' -import { matchSorter } from 'match-sorter' import { useEffect, useId, @@ -45,9 +44,6 @@ const NO_MATCH_ITEM: ComboboxItem = { const isNoMatch = (item: ComboboxItem | null) => item?.value === NO_MATCH_VALUE // 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 @@ -120,10 +116,9 @@ export const Combobox = ({ }: 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 filteredItems = items.filter((item) => + item.selectedLabel.toLowerCase().replace(/\s+/g, '').includes(q) + ) // Clear the query when the parent clears the value (e.g. firewall rules // form on subform submit). Only needed for allowArbitraryValues; without @@ -173,6 +168,7 @@ export const Combobox = ({ // (closed). HUI's `open` render prop is stale by one keydown in our // handler ordering — caused Firefox e2e flakes. const isOpenRef = useRef(false) + const minHeight = Math.min(virtualOptions.length * ITEM_HEIGHT, MAX_PANEL_HEIGHT) return ( {({ option }: { option: ComboboxItem }) => { diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 20bea9ef7..e04d9ecba 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -584,11 +584,11 @@ test('attaching additional disks allows for combobox filtering', async ({ page } // type in a string to use as a filter await selectADisk.fill('disk-02') - // only disks with that substring should be shown - await expect(page.getByRole('option', { name: 'disk-0023' })).toBeVisible() - await expect(page.getByRole('option', { name: 'disk-0125' })).toBeVisible() + // only disks containing "disk-02" should be shown await expect(page.getByRole('option', { name: 'disk-0211' })).toBeVisible() - await expect(page.getByRole('option', { name: 'disk-0220' })).toBeHidden() + await expect(page.getByRole('option', { name: 'disk-0288' })).toBeVisible() + // disks without that substring are hidden + await expect(page.getByRole('option', { name: 'disk-0125' })).toBeHidden() await expect(page.getByRole('option', { name: 'disk-1000' })).toBeHidden() // Filter down to a single late-in-the-list disk and select it. This From 35d9bb38df81fb16a766d7b47c2a79610057a169 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 28 May 2026 10:54:15 -0700 Subject: [PATCH 07/13] revert to using matchSorter --- app/ui/lib/Combobox.tsx | 22 +++++++++++----------- test/e2e/instance-create.e2e.ts | 9 +++++---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index e54fde98e..885676297 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -14,6 +14,7 @@ import { Combobox as HCombobox, } from '@headlessui/react' import cn from 'classnames' +import { matchSorter } from 'match-sorter' import { useEffect, useId, @@ -34,14 +35,12 @@ export type ComboboxItem = { value: string; label: ReactNode; selectedLabel: str // 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, + value: '__combobox_no_match__', // won't match any actual values label: 'No items match', selectedLabel: '', } -// HUI types `disabled` with the same union as `value`, so item may be null -const isNoMatch = (item: ComboboxItem | null) => item?.value === NO_MATCH_VALUE +const isNoMatch = (item: ComboboxItem | null) => item === NO_MATCH_ITEM // HUI's virtualizer needs the scroll container to have a non-zero height const ITEM_HEIGHT = 40 @@ -116,9 +115,10 @@ export const Combobox = ({ }: ComboboxProps) => { const [query, setQuery] = useState(selectedItemValue || '') const q = query.toLowerCase().replace(/\s+/g, '') - const filteredItems = items.filter((item) => - item.selectedLabel.toLowerCase().replace(/\s+/g, '').includes(q) - ) + const filteredItems = matchSorter(items, q, { + keys: ['selectedLabel', 'label'], + sorter: (items) => items, // preserve original order, don't sort by match + }) // Clear the query when the parent clears the value (e.g. firewall rules // form on subform submit). Only needed for allowArbitraryValues; without @@ -146,6 +146,7 @@ export const Combobox = ({ } const virtualOptions: ComboboxItem[] = filteredItems.length === 0 && !allowArbitraryValues ? [NO_MATCH_ITEM] : filteredItems + const minHeight = Math.min(virtualOptions.length * ITEM_HEIGHT, MAX_PANEL_HEIGHT) // Arbitrary values may not be in `items`, so synthesize a stand-in. const selectedItem = useMemo(() => { @@ -168,14 +169,13 @@ export const Combobox = ({ // (closed). HUI's `open` render prop is stale by one keydown in our // handler ordering — caused Firefox e2e flakes. const isOpenRef = useRef(false) - const minHeight = Math.min(virtualOptions.length * ITEM_HEIGHT, MAX_PANEL_HEIGHT) return ( { - if (!item || item.value === NO_MATCH_VALUE) { + if (!item || item === NO_MATCH_ITEM) { onChange('') return } @@ -270,7 +270,7 @@ export const Combobox = ({ )}
- {(items.length > 0 || allowArbitraryValues) && ( + {(items.length > 0 || allowArbitraryValues) && virtualOptions.length > 0 && ( {({ option }: { option: ComboboxItem }) => { - const noMatch = option.value === NO_MATCH_VALUE + const noMatch = option === NO_MATCH_ITEM return (