From 48a317d4521b1510c481cca0eadbcce617d80271 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Mon, 19 May 2025 23:42:55 -0600 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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 {