From 48a317d4521b1510c481cca0eadbcce617d80271 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Mon, 19 May 2025 23:42:55 -0600 Subject: [PATCH 1/9] Replace Listbox in Create Disk side modal with Combobox --- app/forms/disk-create.tsx | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index ded6cec33..7eae56bc6 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' @@ -171,6 +171,7 @@ const DiskSourceField = ({ field: { value, onChange }, } = useController({ control, name: 'diskSource' }) const diskSizeField = useController({ control, name: 'size' }).field + const diskImageIdField = useController({ control, name: 'diskSource.imageId' }).field return ( <> @@ -210,14 +211,17 @@ const DiskSourceField = ({ /> )} {value.type === 'image' && ( - toImageComboboxItem(i, true))} required + onInputChange={() => { + diskImageIdField.onChange() + }} onChange={(id) => { const image = images.find((i) => i.id === id)! // if it's selected, it must be present const imageSizeGiB = image.size / GiB @@ -252,13 +256,17 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { const snapshots = snapshotsQuery.data?.items || [] const diskSizeField = useController({ control, name: 'size' }).field + const diskSnapshotIdField = useController({ + control, + name: 'diskSource.snapshotId', + }).field return ( - { const formattedSize = filesize(i.size, { base: 2, output: 'object' }) return { @@ -278,6 +286,9 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { })} isLoading={snapshotsQuery.isPending} required + onInputChange={() => { + diskSnapshotIdField.onChange() + }} onChange={(id) => { const snapshot = snapshots.find((i) => i.id === id)! // if it's selected, it must be present const snapshotSizeGiB = snapshot.size / GiB From b3a0a7d3044270a48fd96328e3cda2dfc67b3f18 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Tue, 20 May 2025 09:51:13 -0600 Subject: [PATCH 2/9] Update e2e tests to account for the comboboxes --- test/e2e/disks.e2e.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 13482ad3e..f3895e54a 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.getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }).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.getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }).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.getByPlaceholder('Select an image or enter an image name', { exact: true }).click() await page.getByRole('option', { name: 'image-3' }).click() }) From a01a795dea9ff2dfaafcabc3fbe9d8b9f5dafe06 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Wed, 21 May 2025 18:09:45 -0600 Subject: [PATCH 3/9] Fix issues raised by CI pipeline --- test/e2e/disks.e2e.ts | 12 +++++++++--- test/e2e/instance-disks.e2e.ts | 4 +++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index f3895e54a..ffbae0cde 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -80,20 +80,26 @@ test.describe('Disk create', () => { test('from snapshot', async ({ page }) => { await page.getByRole('radio', { name: 'Snapshot' }).click() - await page.getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }).click() + await page + .getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }) + .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.getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }).click() + await page + .getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }) + .click() await page.getByRole('option', { name: 'snapshot-max' }).click() }) test('from image', async ({ page }) => { await page.getByRole('radio', { name: 'Image' }).click() - await page.getByPlaceholder('Select an image or enter an image name', { exact: true }).click() + await page + .getByPlaceholder('Select an image or enter an image name', { exact: true }) + .click() await page.getByRole('option', { name: 'image-3' }).click() }) diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index f6c59b063..f5a3bef04 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -117,7 +117,9 @@ 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 + .getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }) + .click() await page.getByRole('option', { name: 'snapshot-heavy' }).click() await createForm.getByRole('button', { name: 'Create disk' }).click() From 7f6b651ae362265dcc37b7e89d9925a45d8e332d Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Fri, 23 May 2025 14:28:30 -0600 Subject: [PATCH 4/9] Fix issues stated in code review --- test/e2e/disks.e2e.ts | 12 +++--------- test/e2e/instance-disks.e2e.ts | 4 +--- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index ffbae0cde..eb4a573f2 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -80,26 +80,20 @@ test.describe('Disk create', () => { test('from snapshot', async ({ page }) => { await page.getByRole('radio', { name: 'Snapshot' }).click() - await page - .getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }) - .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 - .getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }) - .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 - .getByPlaceholder('Select an image or enter an image name', { exact: true }) - .click() + await page.getByRole('combobox', { name: 'Source image' }).click() await page.getByRole('option', { name: 'image-3' }).click() }) diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index f5a3bef04..a8569f4ec 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -117,9 +117,7 @@ test('Create disk', async ({ page }) => { await expect(createForm.getByRole('textbox', { name: 'Description' })).toBeVisible() await page.getByRole('radio', { name: 'Snapshot' }).click() - await page - .getByPlaceholder('Select a snapshot or enter a snapshot name', { exact: true }) - .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() From 19621a5f6d8f246d1755b4127e2bd35cfaeadf4c Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Thu, 29 May 2025 14:37:52 -0600 Subject: [PATCH 5/9] Revert changes in placeholders --- app/forms/disk-create.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 7eae56bc6..8cb8418f3 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -215,7 +215,7 @@ const DiskSourceField = ({ control={control} name="diskSource.imageId" label="Source image" - placeholder="Select an image or enter an image name" + placeholder="Select an image" isLoading={areImagesLoading} items={images.map((i) => toImageComboboxItem(i, true))} required @@ -266,7 +266,7 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { control={control} name="diskSource.snapshotId" label="Source snapshot" - placeholder="Select a snapshot or enter a snapshot name" + placeholder="Select a snapshot" items={snapshots.map((i) => { const formattedSize = filesize(i.size, { base: 2, output: 'object' }) return { From 5c7b0eee2d3ac00e035f7b60e71eb6daf3013998 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 22 Jul 2025 15:49:29 -0700 Subject: [PATCH 6/9] refactor combobox input editing and add tests --- app/components/form/fields/ComboboxField.tsx | 6 ++ app/forms/disk-create.tsx | 11 --- app/ui/lib/Combobox.tsx | 2 +- app/ui/lib/FileInput.tsx | 4 +- test/e2e/instance-create.e2e.ts | 80 ++++++++++++++++++-- 5 files changed, 82 insertions(+), 21 deletions(-) diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index efa055829..ce122b748 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 @@ -88,6 +89,11 @@ export function ComboboxField< onChange?.(value) setSelectedItemLabel(getSelectedLabelFromValue(items, value)) }} + onInputChange={(value) => { + // 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) + }} allowArbitraryValues={allowArbitraryValues} inputRef={field.ref} transform={transform} diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 8cb8418f3..13e909b13 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -171,7 +171,6 @@ const DiskSourceField = ({ field: { value, onChange }, } = useController({ control, name: 'diskSource' }) const diskSizeField = useController({ control, name: 'size' }).field - const diskImageIdField = useController({ control, name: 'diskSource.imageId' }).field return ( <> @@ -219,9 +218,6 @@ const DiskSourceField = ({ isLoading={areImagesLoading} items={images.map((i) => toImageComboboxItem(i, true))} required - onInputChange={() => { - diskImageIdField.onChange() - }} onChange={(id) => { const image = images.find((i) => i.id === id)! // if it's selected, it must be present const imageSizeGiB = image.size / GiB @@ -256,10 +252,6 @@ const SnapshotSelectField = ({ control }: { control: Control }) => { const snapshots = snapshotsQuery.data?.items || [] const diskSizeField = useController({ control, name: 'size' }).field - const diskSnapshotIdField = useController({ - control, - name: 'diskSource.snapshotId', - }).field return ( }) => { })} isLoading={snapshotsQuery.isPending} required - onInputChange={() => { - diskSnapshotIdField.onChange() - }} onChange={(id) => { const snapshot = snapshots.find((i) => i.id === id)! // if it's selected, it must be present const snapshotSizeGiB = snapshot.size / GiB diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index d146c0912..dcd11fdc7 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -265,7 +265,7 @@ export const Combobox = ({ // of those rules one by one. Better to rely on the shared classes.
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 ( -