diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index efa055829..83d934597 100644 --- a/app/components/form/fields/ComboboxField.tsx +++ b/app/components/form/fields/ComboboxField.tsx @@ -45,6 +45,7 @@ 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 @@ -83,11 +84,19 @@ export function ComboboxField< selectedItemValue={field.value} selectedItemLabel={selectedItemLabel} hasError={fieldState.error !== undefined} + // if user selects an item, save the value in form state onChange={(value) => { field.onChange(value) onChange?.(value) setSelectedItemLabel(getSelectedLabelFromValue(items, value)) }} + // if user edits the combobox input … + onInputChange={(value) => { + // if arbitrary values are allowed, save edited string in form state so it can be submitted + // if not allowed, clear the selected item from form state, to force intentional selection of valid options + field.onChange(allowArbitraryValues ? value : undefined) + onInputChange?.(value) + }} allowArbitraryValues={allowArbitraryValues} inputRef={field.ref} transform={transform} diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index ded6cec33..22760cdab 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' @@ -210,7 +210,7 @@ const DiskSourceField = ({ /> )} {value.type === 'image' && ( - toImageComboboxItem(i, true))} 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)) @@ -254,7 +255,7 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { const diskSizeField = useController({ control, name: 'size' }).field return ( - }) => { isLoading={snapshotsQuery.isPending} required onChange={(id) => { - const snapshot = snapshots.find((i) => i.id === id)! // if it's selected, it must be present + const snapshot = snapshots.find((i) => i.id === id) + if (!snapshot) return const snapshotSizeGiB = snapshot.size / GiB if (diskSizeField.value < snapshotSizeGiB) { diskSizeField.onChange(diskSizeNearest10(snapshotSizeGiB)) diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 13482ad3e..eb4a573f2 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -80,20 +80,20 @@ test.describe('Disk create', () => { test('from snapshot', async ({ page }) => { await page.getByRole('radio', { name: 'Snapshot' }).click() - await page.getByRole('button', { name: 'Source snapshot' }).click() + await page.getByRole('combobox', { name: 'Source snapshot' }).click() await page.getByRole('option', { name: 'delete-500' }).click() }) // max-size snapshot required a fix test('from max-size snapshot', async ({ page }) => { await page.getByRole('radio', { name: 'Snapshot' }).click() - await page.getByRole('button', { name: 'Source snapshot' }).click() + await page.getByRole('combobox', { name: 'Source snapshot' }).click() await page.getByRole('option', { name: 'snapshot-max' }).click() }) test('from image', async ({ page }) => { await page.getByRole('radio', { name: 'Image' }).click() - await page.getByRole('button', { name: 'Source image' }).click() + await page.getByRole('combobox', { name: 'Source image' }).click() await page.getByRole('option', { name: 'image-3' }).click() }) diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 52a5f0ea0..f962807cb 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -37,6 +37,16 @@ const selectAnExistingDisk = async (page: Page, name: string) => { await page.getByRole('option', { name }).click() } +/** Ensure that certain combobox options are present, others are hidden */ +const expectComboboxOptions = async (page: Page, present: string[], hidden: string[]) => { + for (const name of present) { + await expect(page.getByRole('option', { name })).toBeVisible() + } + for (const name of hidden) { + await expect(page.getByRole('option', { name })).toBeHidden() + } +} + test('can create an instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') await page.locator('text="New Instance"').click() @@ -513,18 +523,16 @@ test('attaching additional disks allows for combobox filtering', async ({ page } 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() + await expectComboboxOptions(page, ['disk-0001', 'disk-0002', 'disk-1000'], []) // 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() + await expectComboboxOptions( + page, + ['disk-0100', 'disk-0101', 'disk-0102'], + ['disk-0001', 'disk-1000'] + ) // select one await page.getByRole('option', { name: 'disk-0102' }).click() @@ -649,3 +657,59 @@ test('Validate CPU and RAM', async ({ page }) => { await expect(cpuMsg).toBeVisible() await expect(memMsg).toBeVisible() }) + +test('clears silo image selection when typing arbitrary text and blurring', async ({ + page, +}) => { + const instanceName = 'test-instance' + const validImage = 'ubuntu-22-04' + const alternateImage1 = 'ubuntu-20-04' + const alternateImage2 = 'arch-2022-06-01' + + await page.goto('/projects/mock-project/instances-new') + await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName) + + const imageSelectCombobox = page.getByRole('combobox', { name: 'Image' }) + await imageSelectCombobox.scrollIntoViewIfNeeded() + + // Ensure the combobox is visible and has the expected options + await expect(imageSelectCombobox).toHaveValue('') + await imageSelectCombobox.click() + await expectComboboxOptions(page, [validImage, alternateImage1, alternateImage2], []) + + // Filter the combobox for a particular silo image pattern + await imageSelectCombobox.fill('ubuntu') + + // Ensure that only show the options that match the filter are visible + await expectComboboxOptions(page, [validImage, alternateImage1], [alternateImage2]) + + // Select an image + await page.getByRole('option', { name: validImage }).click() + await expect(imageSelectCombobox).toHaveValue(validImage) + + // Delete four characters from the end to reveal more options + await page.keyboard.press('Backspace') + await page.keyboard.press('Backspace') + await page.keyboard.press('Backspace') + await page.keyboard.press('Backspace') + + // There should now be an invalid value in the combobox, but we should be able to see both the ubuntu options: `ubuntu-22-04` and `ubuntu-20-04` + // and we should NOT be able to see the `arch-2022-06-01` option + await expect(imageSelectCombobox).toHaveValue('ubuntu-2') + await expectComboboxOptions(page, [validImage, alternateImage1], [alternateImage2]) + + // Blur the field by clicking elsewhere; because the value is not a valid silo image, the selection should be cleared + await page.getByRole('textbox', { name: 'Name', exact: true }).click() + + // The selection should be cleared since allowArbitraryValues=false + await expect(imageSelectCombobox).toHaveValue('') + + // Re-focus and select the original option again + await imageSelectCombobox.click() + await page.getByRole('option', { name: validImage }).click() + await expect(imageSelectCombobox).toHaveValue(validImage) + + // Should be able to continue with instance creation + await page.getByRole('button', { name: 'Create instance' }).click() + await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`) +}) diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index f6c59b063..a8569f4ec 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -117,7 +117,7 @@ test('Create disk', async ({ page }) => { await expect(createForm.getByRole('textbox', { name: 'Description' })).toBeVisible() await page.getByRole('radio', { name: 'Snapshot' }).click() - await page.getByRole('button', { name: 'Source snapshot' }).click() + await page.getByRole('combobox', { name: 'Source snapshot' }).click() await page.getByRole('option', { name: 'snapshot-heavy' }).click() await createForm.getByRole('button', { name: 'Create disk' }).click()