diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx new file mode 100644 index 0000000000..923baca235 --- /dev/null +++ b/app/components/form/fields/ComboboxField.tsx @@ -0,0 +1,62 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import { + useController, + type Control, + type FieldPath, + type FieldValues, +} from 'react-hook-form' + +import { Combobox, type ComboboxBaseProps } from '~/ui/lib/Combobox' +import { capitalize } from '~/util/str' + +import { ErrorMessage } from './ErrorMessage' + +export type ComboboxFieldProps< + TFieldValues extends FieldValues, + TName extends FieldPath, +> = { + name: TName + control: Control + onChange?: (value: string | null | undefined) => void + disabled?: boolean +} & ComboboxBaseProps + +export function ComboboxField< + TFieldValues extends FieldValues, + TName extends FieldPath, + // TODO: constrain TValue to extend string +>({ + control, + name, + label = capitalize(name), + required, + onChange, + disabled, + ...props +}: ComboboxFieldProps) { + const { field, fieldState } = useController({ name, control, rules: { required } }) + return ( +
+ { + field.onChange(value) + onChange?.(value) + }} + {...props} + /> + +
+ ) +} diff --git a/app/components/form/fields/ImageSelectField.tsx b/app/components/form/fields/ImageSelectField.tsx index ea4aa0be4b..4f11e6f3fb 100644 --- a/app/components/form/fields/ImageSelectField.tsx +++ b/app/components/form/fields/ImageSelectField.tsx @@ -32,6 +32,8 @@ export function BootDiskImageSelectField({ }: ImageSelectFieldProps) { const diskSizeField = useController({ control, name: 'bootDiskSize' }).field return ( + // This should be migrated to a `ComboboxField` (and with a `toComboboxItem`), once + // we have a combobox that supports more elaborate labels (beyond just strings). , -> = Omit, 'items'> & { +> = Omit, 'items'> & { vpcNameField: FieldPath } @@ -47,7 +47,7 @@ export function SubnetListbox< ).data?.items || [] return ( - ({ value: name, label: name }))} disabled={!vpcExists} diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index 931346f574..7895c81b91 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -7,7 +7,7 @@ */ import { useApiQuery, type ApiError } from '@oxide/api' -import { ListboxField } from '~/components/form/fields/ListboxField' +import { ComboboxField } from '~/components/form/fields/ComboboxField' import { SideModalForm } from '~/components/form/SideModalForm' import { useForm, useProjectSelector } from '~/hooks' @@ -33,14 +33,11 @@ export function AttachDiskSideModalForm({ loading, submitError = null, }: AttachDiskProps) { - const projectSelector = useProjectSelector() + const { project } = useProjectSelector() - // TODO: loading state? because this fires when the modal opens and not when - // they focus the combobox, it will almost always be done by the time they - // click in - // TODO: error handling + const { data } = useApiQuery('diskList', { query: { project, limit: 1000 } }) const detachedDisks = - useApiQuery('diskList', { query: projectSelector }).data?.items.filter( + data?.items.filter( (d) => d.state.state === 'detached' && !diskNamesToExclude.includes(d.name) ) || [] @@ -51,14 +48,15 @@ export function AttachDiskSideModalForm({ form={form} formType="create" resourceName="disk" - title="Attach Disk" + title="Attach disk" onSubmit={onSubmit} loading={loading} submitError={submitError} onDismiss={onDismiss} > - ({ value: name, label: name }))} required diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 70bb34370b..cf20bd6efe 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -37,6 +37,7 @@ import { import { AccordionItem } from '~/components/AccordionItem' import { DocsPopover } from '~/components/DocsPopover' import { CheckboxField } from '~/components/form/fields/CheckboxField' +import { ComboboxField } from '~/components/form/fields/ComboboxField' import { DescriptionField } from '~/components/form/fields/DescriptionField' import { DiskSizeField } from '~/components/form/fields/DiskSizeField' import { @@ -45,7 +46,6 @@ import { } from '~/components/form/fields/DisksTableField' import { FileField } from '~/components/form/fields/FileField' import { BootDiskImageSelectField as ImageSelectField } from '~/components/form/fields/ImageSelectField' -import { ListboxField } from '~/components/form/fields/ListboxField' import { NameField } from '~/components/form/fields/NameField' import { NetworkInterfaceField } from '~/components/form/fields/NetworkInterfaceField' import { NumberField } from '~/components/form/fields/NumberField' @@ -550,13 +550,14 @@ export function CreateInstanceForm() { /> ) : ( - )} diff --git a/app/forms/network-interface-create.tsx b/app/forms/network-interface-create.tsx index 0b1d8e4218..756da6c0ee 100644 --- a/app/forms/network-interface-create.tsx +++ b/app/forms/network-interface-create.tsx @@ -9,8 +9,8 @@ import { useMemo } from 'react' import { useApiQuery, type ApiError, type InstanceNetworkInterfaceCreate } from '@oxide/api' +import { ComboboxField } from '~/components/form/fields/ComboboxField' import { DescriptionField } from '~/components/form/fields/DescriptionField' -import { ListboxField } from '~/components/form/fields/ListboxField' import { NameField } from '~/components/form/fields/NameField' import { SubnetListbox } from '~/components/form/fields/SubnetListbox' import { TextField } from '~/components/form/fields/TextField' @@ -65,7 +65,7 @@ export function CreateNetworkInterfaceForm({ - ({ label: name, value: name }))} diff --git a/app/forms/snapshot-create.tsx b/app/forms/snapshot-create.tsx index dfb9e111b7..501ffb9632 100644 --- a/app/forms/snapshot-create.tsx +++ b/app/forms/snapshot-create.tsx @@ -16,8 +16,8 @@ import { type SnapshotCreate, } from '@oxide/api' +import { ComboboxField } from '~/components/form/fields/ComboboxField' import { DescriptionField } from '~/components/form/fields/DescriptionField' -import { ListboxField } from '~/components/form/fields/ListboxField' import { NameField } from '~/components/form/fields/NameField' import { SideModalForm } from '~/components/form/SideModalForm' import { useForm, useProjectSelector } from '~/hooks' @@ -73,7 +73,13 @@ export function CreateSnapshotSideModalForm() { > - + ) } diff --git a/app/pages/system/SiloImagesPage.tsx b/app/pages/system/SiloImagesPage.tsx index 48aa227dc0..61cd292129 100644 --- a/app/pages/system/SiloImagesPage.tsx +++ b/app/pages/system/SiloImagesPage.tsx @@ -20,6 +20,7 @@ import { import { Images16Icon, Images24Icon } from '@oxide/design-system/icons/react' import { DocsPopover } from '~/components/DocsPopover' +import { ComboboxField } from '~/components/form/fields/ComboboxField' import { toListboxItem } from '~/components/form/fields/ImageSelectField' import { ListboxField } from '~/components/form/fields/ListboxField' import { useForm } from '~/hooks' @@ -167,7 +168,7 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
- - void }) { content="Users in the selected silo will be able to allocate IPs from this pool." /> - void }) { content="Users in this silo will be able to allocate IPs from the selected pool." /> - void +} & ComboboxBaseProps + +export const Combobox = ({ + description, + items, + selected, + label, + placeholder, + tooltipText, + required, + hasError, + isDisabled, + isLoading, + onChange, +}: ComboboxProps) => { + const [query, setQuery] = useState(selected || '') + + const q = query.toLowerCase() + const filteredItems = matchSorter(items, q, { + keys: ['value'], + sorter: (items) => items, // preserve original order, don't sort by match + }) + + const zIndex = usePopoverZIndex() + + return ( + <> + onChange(val || '')} + onClose={() => setQuery('')} + defaultValue={selected} + disabled={isDisabled || isLoading} + > + {label && ( + // TODO: FieldLabel needs a real ID +
+ + + + {description && {description}} +
+ )} + + (selected ? selected : query)} + onChange={(event) => setQuery(event.target.value)} + placeholder={placeholder} + disabled={isDisabled || isLoading} + className={cn( + `w-full rounded !border-none px-3 py-[0.5rem] !outline-none text-sans-md text-default placeholder:text-quaternary`, + isDisabled + ? 'cursor-not-allowed text-disabled bg-disabled !border-default' + : 'bg-default', + hasError && 'focus-error' + )} + /> +
+ +
+ + + {filteredItems.length === 0 && ( + +
No items match
+ + )} + {filteredItems.map((item) => ( + { + onChange(item.label) + setQuery(item.label) + }} + > + {({ 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} +
+ )} + + ))} + + + + ) +} diff --git a/app/ui/lib/Listbox.tsx b/app/ui/lib/Listbox.tsx index 8350d92601..40ada6fbb7 100644 --- a/app/ui/lib/Listbox.tsx +++ b/app/ui/lib/Listbox.tsx @@ -140,14 +140,12 @@ export const Listbox = ({ value={item.value} className="relative border-b border-secondary last:border-0" > - {({ active, selected }) => ( - // TODO: redo active styling with `data-active` somehow + {({ focus, selected }) => (
{item.label}
diff --git a/app/ui/styles/components/menu-list.css b/app/ui/styles/components/menu-list.css index 35c28f87a5..4241dec636 100644 --- a/app/ui/styles/components/menu-list.css +++ b/app/ui/styles/components/menu-list.css @@ -30,6 +30,7 @@ @apply border-0 text-accent bg-accent-secondary hover:bg-accent-secondary-hover; } +/* beautiful ring */ .ox-menu-item.is-selected:after { content: ''; @apply absolute bottom-0 left-0 right-0 top-0 block rounded border border-accent; diff --git a/mock-api/disk.ts b/mock-api/disk.ts index c58ff18e0e..ba5c5ca665 100644 --- a/mock-api/disk.ts +++ b/mock-api/disk.ts @@ -146,9 +146,9 @@ export const disks: Json[] = [ size: 12 * GiB, block_size: 2048, }, - // put a ton of disks in project 2 so we can use it to test pagination - ...new Array(55).fill(0).map((_, i) => { - const numStr = (i + 1).toString().padStart(2, '0') + // put a ton of disks in project 2 so we can use it to test comboboxes + ...new Array(1010).fill(0).map((_, i) => { + const numStr = (i + 1).toString().padStart(4, '0') return { id: '9747d936-795d-4d76-8ee0-15561f4cbb' + numStr, name: 'disk-' + numStr, diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts index a86c86640f..67b910cdff 100644 --- a/test/e2e/images.e2e.ts +++ b/test/e2e/images.e2e.ts @@ -24,7 +24,7 @@ test('can promote an image from silo', async ({ page }) => { await expectNotVisible(page, ['role=cell[name="image-1"]']) // Listboxes are visible - await expect(page.locator(`text="Filter images by project"`)).toBeVisible() + await expect(page.getByPlaceholder('Filter images by project')).toBeVisible() await expect(page.locator(`text="Select an image"`)).toBeVisible() // Notice is visible diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index bdeb21b8f2..2796d69b08 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -269,7 +269,8 @@ test('create instance with a different existing disk', async ({ page }) => { await page.goto('/projects/mock-project/instances-new') await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName) await page.getByRole('tab', { name: 'Existing disks' }).click() - await page.getByLabel('Disk', { exact: true }).click() + // verify combobox text entry + await page.getByPlaceholder('Select a disk').fill('disk-') await page.getByRole('option', { name: 'disk-4' }).click() await page.getByRole('button', { name: 'Create instance' }).click() await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`) @@ -282,7 +283,7 @@ test('start with an existing disk, but then switch to a silo image', async ({ pa await page.goto('/projects/mock-project/instances-new') await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName) await page.getByRole('tab', { name: 'Existing disks' }).click() - await page.getByLabel('Disk', { exact: true }).click() + await page.getByPlaceholder('Select a disk').fill('disk-') await page.getByRole('option', { name: 'disk-7' }).click() await page.getByRole('tab', { name: 'Silo images' }).click() await page.getByRole('button', { name: 'Create instance' }).click() @@ -297,13 +298,13 @@ test('additional disks do not list committed disks as available', async ({ page const attachExistingDiskButton = page.getByRole('button', { name: 'Attach existing disk', }) - const selectAnOption = page.getByRole('button', { name: 'Select an option' }) + const selectADisk = page.getByPlaceholder('Select a disk') const disk2 = page.getByRole('option', { name: 'disk-2' }) const disk3 = page.getByRole('option', { name: 'disk-3' }) const disk4 = page.getByRole('option', { name: 'disk-4' }) await attachExistingDiskButton.click() - await selectAnOption.click() + await selectADisk.click() // disk-2 is already attached, so should not be visible in the list await expect(disk2).toBeHidden() // disk-3, though, should be present @@ -315,7 +316,7 @@ test('additional disks do not list committed disks as available', async ({ page await page.getByRole('button', { name: 'Attach disk' }).click() await attachExistingDiskButton.click() - await selectAnOption.click() + await selectADisk.click() // disk-2 should still be hidden await expect(disk2).toBeHidden() // now disk-3 should be hidden as well @@ -428,3 +429,40 @@ test('attach a floating IP section has Empty version when no floating IPs exist page.getByText('Create a floating IP to attach it to this instance') ).toBeVisible() }) + +test('attaching additional disks allows for combobox filtering', async ({ page }) => { + await page.goto('/projects/other-project/instances-new') + + const attachExistingDiskButton = page.getByRole('button', { + name: 'Attach existing disk', + }) + const selectADisk = page.getByPlaceholder('Select a disk') + + await attachExistingDiskButton.click() + await selectADisk.click() + // several disks should be shown + await expect(page.getByRole('option', { name: 'disk-0001' })).toBeVisible() + await expect(page.getByRole('option', { name: 'disk-0002' })).toBeVisible() + await expect(page.getByRole('option', { name: 'disk-1000' })).toBeVisible() + + // type in a string to use as a filter + await selectADisk.fill('disk-010') + // only disks with that substring should be shown + await expect(page.getByRole('option', { name: 'disk-0100' })).toBeVisible() + await expect(page.getByRole('option', { name: 'disk-0101' })).toBeVisible() + await expect(page.getByRole('option', { name: 'disk-0102' })).toBeVisible() + await expect(page.getByRole('option', { name: 'disk-0001' })).toBeHidden() + await expect(page.getByRole('option', { name: 'disk-1000' })).toBeHidden() + + // select one + await page.getByRole('option', { name: 'disk-0102' }).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('button', { name: 'disk-0102' })).toBeVisible() + + // a random string should give a disabled option + await selectADisk.click() + await selectADisk.fill('asdf') + await expect(page.getByRole('option', { name: 'No items match' })).toBeVisible() +}) diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index 373ebbb5ad..95981b277b 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -63,7 +63,7 @@ test('Attach disk', async ({ page }) => { await expectVisible(page, ['role=option[name="disk-3"]', 'role=option[name="disk-4"]']) await page.click('role=option[name="disk-3"]') - await page.click('role=button[name="Attach Disk"]') + await page.click('role=button[name="Attach disk"]') await expectVisible(page, ['role=cell[name="disk-3"]']) }) diff --git a/test/e2e/ip-pools.e2e.ts b/test/e2e/ip-pools.e2e.ts index 532f6f0c72..32becca80f 100644 --- a/test/e2e/ip-pools.e2e.ts +++ b/test/e2e/ip-pools.e2e.ts @@ -100,8 +100,8 @@ test('IP pool link silo', async ({ page }) => { await page.getByRole('button', { name: 'Link silo' }).click() await expect(modal).toBeVisible() - // select silo in listbox and click link - await page.getByRole('button', { name: 'Select silo' }).click() + // select silo in combobox and click link + await page.getByPlaceholder('Select silo').fill('m') await page.getByRole('option', { name: 'myriad' }).click() await modal.getByRole('button', { name: 'Link' }).click() diff --git a/test/e2e/pagination.e2e.ts b/test/e2e/pagination.e2e.ts index 49e1cce0d5..2013675b36 100644 --- a/test/e2e/pagination.e2e.ts +++ b/test/e2e/pagination.e2e.ts @@ -10,7 +10,7 @@ import { expect, test } from '@playwright/test' import { expectRowVisible } from './utils' test('pagination', async ({ page }) => { - await page.goto('/projects/other-project/disks') + await page.goto('/projects/mock-project/snapshots') const table = page.getByRole('table') const rows = page.getByRole('row') @@ -18,24 +18,19 @@ test('pagination', async ({ page }) => { const prevButton = page.getByRole('button', { name: 'prev', exact: true }) await expect(rows).toHaveCount(26) - await expectRowVisible(table, { name: 'disk-01' }) - await expectRowVisible(table, { name: 'disk-25' }) + await expectRowVisible(table, { name: 'snapshot-1' }) + await expectRowVisible(table, { name: 'disk-1-snapshot-24' }) await nextButton.click() - await expect(rows).toHaveCount(26) - await expectRowVisible(table, { name: 'disk-26' }) - await expectRowVisible(table, { name: 'disk-50' }) + await expect(rows).toHaveCount(7) + await expectRowVisible(table, { name: 'disk-1-snapshot-25' }) + await expectRowVisible(table, { name: 'disk-1-snapshot-30' }) await prevButton.click() await expect(rows).toHaveCount(26) - await expectRowVisible(table, { name: 'disk-01' }) - await expectRowVisible(table, { name: 'disk-25' }) + await expectRowVisible(table, { name: 'snapshot-1' }) + await expectRowVisible(table, { name: 'disk-1-snapshot-24' }) await nextButton.click() - await nextButton.click() - await expect(rows).toHaveCount(6) - await expectRowVisible(table, { name: 'disk-51' }) - await expectRowVisible(table, { name: 'disk-55' }) - await expect(nextButton).toBeDisabled() // no more pages }) diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts index 0a2504ecee..cbdb893d6c 100644 --- a/test/e2e/silos.e2e.ts +++ b/test/e2e/silos.e2e.ts @@ -242,8 +242,8 @@ test('Silo IP pools link pool', async ({ page }) => { await page.getByRole('button', { name: 'Link pool' }).click() await expect(modal).toBeVisible() - // select silo in listbox and click link - await page.getByRole('button', { name: 'Select pool' }).click() + // select silo in combobox and click link + await page.getByPlaceholder('Select pool').fill('ip-pool') await page.getByRole('option', { name: 'ip-pool-3' }).click() await modal.getByRole('button', { name: 'Link' }).click()