From 48a317d4521b1510c481cca0eadbcce617d80271 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Mon, 19 May 2025 23:42:55 -0600 Subject: [PATCH 1/6] 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/6] 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/6] 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/6] 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/6] 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 40e8f09229ddce3d3de6a2bc6d8da862dc41e986 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Thu, 24 Jul 2025 13:34:11 -0700 Subject: [PATCH 6/6] Add util function to update form fields consistently --- app/components/form/fields/ComboboxField.tsx | 6 ++++ .../form/fields/ImageSelectField.tsx | 14 ++------- app/forms/disk-create.tsx | 25 ++-------------- app/forms/disk-util.ts | 30 +++++++++++++++++++ 4 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 app/forms/disk-util.ts diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index efa055829..b3cacf7f7 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) => { + // for forms allowing arbitrary values, treat typing as a selection + field.onChange(allowArbitraryValues ? value : undefined) + onInputChange?.(value) + }} allowArbitraryValues={allowArbitraryValues} inputRef={field.ref} transform={transform} diff --git a/app/components/form/fields/ImageSelectField.tsx b/app/components/form/fields/ImageSelectField.tsx index 13bb17663..d4af49913 100644 --- a/app/components/form/fields/ImageSelectField.tsx +++ b/app/components/form/fields/ImageSelectField.tsx @@ -9,11 +9,11 @@ import { useController, type Control } from 'react-hook-form' import type { Image } from '@oxide/api' +import { adjustDiskSizeForSource } from '~/forms/disk-util' import type { InstanceCreateInput } from '~/forms/instance-create' import type { ComboboxItem } from '~/ui/lib/Combobox' import { Slash } from '~/ui/lib/Slash' -import { diskSizeNearest10 } from '~/util/math' -import { bytesToGiB, GiB } from '~/util/units' +import { bytesToGiB } from '~/util/units' import { ComboboxField } from './ComboboxField' @@ -43,15 +43,7 @@ export function BootDiskImageSelectField({ items={images.map((i) => toImageComboboxItem(i))} required onChange={(id) => { - const image = images.find((i) => i.id === id) - // the most likely scenario where image would be undefined is if the user has - // manually cleared the ComboboxField; they will need to pick a boot disk image - // in order to submit the form, so we don't need to do anything here - if (!image) return - const imageSizeGiB = image.size / GiB - if (diskSizeField.value < imageSizeGiB) { - diskSizeField.onChange(diskSizeNearest10(imageSizeGiB)) - } + adjustDiskSizeForSource(diskSizeField, id, images) }} /> ) diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 8cb8418f3..e6716699d 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -28,6 +28,7 @@ import { NameField } from '~/components/form/fields/NameField' import { RadioField } from '~/components/form/fields/RadioField' import { SideModalForm } from '~/components/form/SideModalForm' import { HL } from '~/components/HL' +import { adjustDiskSizeForSource } from '~/forms/disk-util' import { useProjectSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' import { FormDivider } from '~/ui/lib/Divider' @@ -36,7 +37,6 @@ import { Radio } from '~/ui/lib/Radio' import { RadioGroup } from '~/ui/lib/RadioGroup' import { Slash } from '~/ui/lib/Slash' import { toLocaleDateString } from '~/util/date' -import { diskSizeNearest10 } from '~/util/math' import { bytesToGiB, GiB } from '~/util/units' const blankDiskSource: DiskSource = { @@ -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,15 +218,8 @@ 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 - if (diskSizeField.value < imageSizeGiB) { - diskSizeField.onChange(diskSizeNearest10(imageSizeGiB)) - } + adjustDiskSizeForSource(diskSizeField, id, images) }} /> )} @@ -256,10 +248,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 - if (diskSizeField.value < snapshotSizeGiB) { - diskSizeField.onChange(diskSizeNearest10(snapshotSizeGiB)) - } + adjustDiskSizeForSource(diskSizeField, id, snapshots) }} /> ) diff --git a/app/forms/disk-util.ts b/app/forms/disk-util.ts new file mode 100644 index 000000000..dc0031c65 --- /dev/null +++ b/app/forms/disk-util.ts @@ -0,0 +1,30 @@ +/* + * 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 { type Image, type Snapshot } from '~/api' +import { diskSizeNearest10 } from '~/util/math' +import { GiB } from '~/util/units' + +/** + * Adjusts disk size field if current value is smaller than source size. + * Used when selecting images or snapshots to ensure disk is large enough. + */ +export function adjustDiskSizeForSource( + diskSizeField: { value: number; onChange: (value: number) => void }, + id: string | null | undefined, + items: (Image | Snapshot)[] +) { + if (!id) return + + const item = items.find((i) => i.id === id) + if (!item) return + + const sourceSizeGiB = item.size / GiB + if (diskSizeField.value < sourceSizeGiB) { + diskSizeField.onChange(diskSizeNearest10(sourceSizeGiB)) + } +}