From 45be44999ff4ba37bf7e62400b4c816bac9381e0 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 1 Jul 2024 14:13:07 -0400
Subject: [PATCH 01/17] Remove picked-by-default image and disk from
 instance-create

---
 app/forms/instance-create.tsx | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 9fba26874..25c360e4e 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -221,9 +221,6 @@ export function CreateInstanceForm() {
   const defaultValues: InstanceCreateInput = {
     ...baseDefaultValues,
     bootDiskSourceType: defaultSource,
-    siloImageSource: siloImages?.[0]?.id || '',
-    projectImageSource: projectImages?.[0]?.id || '',
-    diskSource: disks?.[0]?.value || '',
     sshPublicKeys: allKeys,
     bootDiskSize: nearest10(defaultImage?.size / GiB),
     externalIps: [{ type: 'ephemeral', pool: defaultPool }],

From a287eed6f34a3a03e286f0fac211eadaa4515173 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 1 Jul 2024 14:28:34 -0400
Subject: [PATCH 02/17] Add helper to pick project image in test

---
 test/e2e/instance-create.e2e.ts | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 2796d69b0..c5830b74d 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -7,7 +7,20 @@
  */
 import { floatingIp, images } from '@oxide/api-mocks'
 
-import { expect, expectNotVisible, expectRowVisible, expectVisible, test } from './utils'
+import {
+  expect,
+  expectNotVisible,
+  expectRowVisible,
+  expectVisible,
+  test,
+  type Page,
+} from './utils'
+
+const pickAProjectImage = async (page: Page, index = 0) => {
+  await page.getByRole('tab', { name: 'Project images' }).click()
+  await page.getByRole('button', { name: 'Image' }).click()
+  await page.getByRole('option', { name: images[index].name }).click()
+}
 
 test('can create an instance', async ({ page }) => {
   await page.goto('/projects/mock-project/instances')
@@ -36,9 +49,7 @@ test('can create an instance', async ({ page }) => {
   await diskSizeInput.fill('20')
 
   // pick a project image just to show we can
-  await page.getByRole('tab', { name: 'Project images' }).click()
-  await page.getByRole('button', { name: 'Image' }).click()
-  await page.getByRole('option', { name: images[2].name }).click()
+  await pickAProjectImage(page, 2)
 
   // should be hidden in accordion
   await expectNotVisible(page, [
@@ -144,9 +155,7 @@ test('can create an instance with custom hardware', async ({ page }) => {
   await page.keyboard.press('Tab')
 
   // pick a project image just to show we can
-  await page.getByRole('tab', { name: 'Project images' }).click()
-  await page.getByRole('button', { name: 'Image' }).click()
-  await page.getByRole('option', { name: images[2].name }).click()
+  await pickAProjectImage(page, 2)
   // the disk size should bot have been changed from what was entered earlier
   await expect(diskSizeInput).toHaveValue('20')
 
@@ -182,16 +191,13 @@ test('automatically updates disk size when larger image selected', async ({ page
   await page.keyboard.press('Tab')
 
   // pick a disk image that's smaller than 5GiB (the first project image works [4GiB])
-  await page.getByRole('tab', { name: 'Project images' }).click()
-  await page.getByRole('button', { name: 'Image' }).click()
-  await page.getByRole('option', { name: images[0].name }).click()
+  await pickAProjectImage(page, 0)
 
   // test that it still says 5, as that's larger than the given image
   await expect(diskSizeInput).toHaveValue('5')
 
   // pick a disk image that's larger than 5GiB (the third project image works [6GiB])
-  await page.getByRole('button', { name: 'Image' }).click()
-  await page.getByRole('option', { name: images[2].name }).click()
+  await pickAProjectImage(page, 2)
 
   // test that it has been automatically increased to next-largest incremement of 10
   await expect(diskSizeInput).toHaveValue('10')

From cee7bf581f1ef8ef370aceff8018f3861e4b29bf Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 1 Jul 2024 16:13:01 -0400
Subject: [PATCH 03/17] Revert existing disks back to ListboxField

---
 app/forms/instance-create.tsx   | 38 ++++++++++++++++++---------------
 test/e2e/instance-create.e2e.ts |  2 ++
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 25c360e4e..dddc6dd23 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -37,7 +37,6 @@ 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 {
@@ -46,6 +45,7 @@ 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'
@@ -547,7 +547,7 @@ export function CreateInstanceForm() {
                 />
               </div>
             ) : (
-              <ComboboxField
+              <ListboxField
                 label="Disk"
                 name="diskSource"
                 description="Existing disks that are not attached to an instance"
@@ -771,21 +771,25 @@ const AdvancedAccordion = ({
                 <MiniTable.HeadCell className="w-12" />
               </MiniTable.Header>
               <MiniTable.Body>
-                {attachedFloatingIpsData.map((item, index) => (
-                  <MiniTable.Row
-                    tabIndex={0}
-                    aria-rowindex={index + 1}
-                    aria-label={`Name: ${item.name}, IP: ${item.ip}`}
-                    key={item.name}
-                  >
-                    <MiniTable.Cell>{item.name}</MiniTable.Cell>
-                    <MiniTable.Cell>{item.ip}</MiniTable.Cell>
-                    <MiniTable.RemoveCell
-                      onClick={() => detachFloatingIp(item.name)}
-                      label={`remove floating IP ${item.name}`}
-                    />
-                  </MiniTable.Row>
-                ))}
+                {attachedFloatingIpsData.map((item, index) =>
+                  item ? (
+                    <MiniTable.Row
+                      tabIndex={0}
+                      aria-rowindex={index + 1}
+                      aria-label={`Name: ${item.name}, IP: ${item.ip}`}
+                      key={item.name}
+                    >
+                      <MiniTable.Cell>{item.name}</MiniTable.Cell>
+                      <MiniTable.Cell>{item.ip}</MiniTable.Cell>
+                      <MiniTable.RemoveCell
+                        onClick={() => detachFloatingIp(item.name)}
+                        label={`remove floating IP ${item.name}`}
+                      />
+                    </MiniTable.Row>
+                  ) : (
+                    <></>
+                  )
+                )}
               </MiniTable.Body>
             </MiniTable.Table>
           )}
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index c5830b74d..4193eddf1 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -115,6 +115,7 @@ test('can create an instance', async ({ page }) => {
 test('duplicate instance name produces visible error', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.fill('input[name=name]', 'db1')
+  await pickAProjectImage(page, 0)
   await page.locator('button:has-text("Create instance")').click()
   await expect(page.getByText('Instance name already exists')).toBeVisible()
 })
@@ -217,6 +218,7 @@ test('automatically updates disk size when larger image selected', async ({ page
 test('with disk name already taken', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.fill('input[name=name]', 'my-instance')
+  await pickAProjectImage(page, 0)
   await page.fill('input[name=bootDiskName]', 'disk-1')
 
   await page.getByRole('button', { name: 'Create instance' }).click()

From c9a4f165f1cfed7388442707dd78fbcb11093ea4 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 1 Jul 2024 18:46:54 -0400
Subject: [PATCH 04/17] fix tests

---
 test/e2e/instance-create.e2e.ts | 38 ++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 4193eddf1..35a8d9655 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -16,12 +16,24 @@ import {
   type Page,
 } from './utils'
 
-const pickAProjectImage = async (page: Page, index = 0) => {
+const pickASiloImage = async (page: Page, name: string) => {
+  await page.getByRole('tab', { name: 'Silo images' }).click()
+  await page.getByRole('button', { name: 'Select an image' }).click()
+  await page.getByRole('option', { name }).click()
+}
+
+const pickAProjectImage = async (page: Page, index: number) => {
   await page.getByRole('tab', { name: 'Project images' }).click()
   await page.getByRole('button', { name: 'Image' }).click()
   await page.getByRole('option', { name: images[index].name }).click()
 }
 
+const pickAnExistingDisk = async (page: Page, diskName: string) => {
+  await page.getByRole('tab', { name: 'Existing disks' }).click()
+  await page.getByRole('button', { name: 'Select a disk' }).click()
+  await page.getByRole('option', { name: diskName }).click()
+}
+
 test('can create an instance', async ({ page }) => {
   await page.goto('/projects/mock-project/instances')
   await page.locator('text="New Instance"').click()
@@ -256,44 +268,38 @@ test('add ssh key from instance create form', async ({ page }) => {
 test('shows object not found error on no default pool', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-default-pool')
+  await pickAProjectImage(page, 0)
   await page.getByRole('button', { name: 'Create instance' }).click()
-
-  await expect(page.getByText('Not found: default IP pool')).toBeVisible()
+  await expect(page.getByText('Not found: default IP pool for current silo')).toBeVisible()
 })
 
 test('create instance with existing disk', async ({ page }) => {
   const instanceName = 'my-existing-disk-instance'
   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 pickAnExistingDisk(page, 'disk-3')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`)
   await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB'])
   await expectRowVisible(page.getByRole('table'), { Disk: 'disk-3' })
 })
 
-test('create instance with a different existing disk', async ({ page }) => {
+test('create instance with a silo image', async ({ page }) => {
   const instanceName = 'my-existing-disk-2'
   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()
-  // verify combobox text entry
-  await page.getByPlaceholder('Select a disk').fill('disk-')
-  await page.getByRole('option', { name: 'disk-4' }).click()
+  await pickASiloImage(page, 'ubuntu-22-04')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`)
-  await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB'])
-  await expectRowVisible(page.getByRole('table'), { Disk: 'disk-4' })
+  await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=10 GiB'])
 })
 
 test('start with an existing disk, but then switch to a silo image', async ({ page }) => {
   const instanceName = 'silo-image-instance'
   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.getByPlaceholder('Select a disk').fill('disk-')
-  await page.getByRole('option', { name: 'disk-7' }).click()
-  await page.getByRole('tab', { name: 'Silo images' }).click()
+  await pickAnExistingDisk(page, 'disk-7')
+  await pickASiloImage(page, 'ubuntu-22-04')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`)
   await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB'])
@@ -360,6 +366,7 @@ test('maintains selected values even when changing tabs', async ({ page }) => {
 test('does not attach an ephemeral IP when the checkbox is unchecked', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-ephemeral-ip')
+  await pickAProjectImage(page, 0)
   await page.getByRole('button', { name: 'Networking' }).click()
   await page
     .getByRole('checkbox', { name: 'Allocate and attach an ephemeral IP address' })
@@ -378,6 +385,7 @@ test('attaches a floating IP; disables button when no IPs available', async ({ p
   const instanceName = 'with-floating-ip'
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
+  await pickAProjectImage(page, 0)
   await page.getByRole('button', { name: 'Networking' }).click()
 
   await attachFloatingIpButton.click()

From 322a93457b72193927429f6e049304eea586d308 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 1 Jul 2024 18:49:50 -0400
Subject: [PATCH 05/17] make 'select a/an' consistent for placeholders

---
 app/components/AttachEphemeralIpModal.tsx          | 2 +-
 app/components/AttachFloatingIpModal.tsx           | 2 +-
 app/forms/floating-ip-create.tsx                   | 2 +-
 app/forms/instance-create.tsx                      | 4 ++--
 app/pages/project/floating-ips/FloatingIpsPage.tsx | 2 +-
 app/pages/system/SiloImagesPage.tsx                | 2 +-
 app/pages/system/networking/IpPoolPage.tsx         | 2 +-
 app/pages/system/silos/SiloIpPoolsTab.tsx          | 2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/components/AttachEphemeralIpModal.tsx b/app/components/AttachEphemeralIpModal.tsx
index d16fbcbf6..73b71f6c2 100644
--- a/app/components/AttachEphemeralIpModal.tsx
+++ b/app/components/AttachEphemeralIpModal.tsx
@@ -50,7 +50,7 @@ export const AttachEphemeralIpModal = ({ onDismiss }: { onDismiss: () => void })
               label="IP pool"
               placeholder={
                 siloPools?.items && siloPools.items.length > 0
-                  ? 'Select pool'
+                  ? 'Select a pool'
                   : 'No pools available'
               }
               items={
diff --git a/app/components/AttachFloatingIpModal.tsx b/app/components/AttachFloatingIpModal.tsx
index 1430a72ea..eaedd2dbd 100644
--- a/app/components/AttachFloatingIpModal.tsx
+++ b/app/components/AttachFloatingIpModal.tsx
@@ -71,7 +71,7 @@ export const AttachFloatingIpModal = ({
               control={form.control}
               name="floatingIp"
               label="Floating IP"
-              placeholder="Select floating IP"
+              placeholder="Select a floating IP"
               items={floatingIps.map((ip) => ({
                 value: ip.id,
                 label: <FloatingIpLabel fip={ip} />,
diff --git a/app/forms/floating-ip-create.tsx b/app/forms/floating-ip-create.tsx
index af80f3bb2..6bc8fc8cb 100644
--- a/app/forms/floating-ip-create.tsx
+++ b/app/forms/floating-ip-create.tsx
@@ -111,7 +111,7 @@ export function CreateFloatingIpSideModalForm() {
             items={(allPools?.items || []).map((p) => toListboxItem(p))}
             label="IP pool"
             control={form.control}
-            placeholder="Select pool"
+            placeholder="Select a pool"
           />
         </AccordionItem>
       </Accordion.Root>
diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index dddc6dd23..4a633eded 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -729,7 +729,7 @@ const AdvancedAccordion = ({
             <Listbox
               name="pools"
               label="IP pool for ephemeral IP"
-              placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
+              placeholder={defaultPool ? `${defaultPool} (default)` : 'Select a pool'}
               selected={`${siloPools.find((pool) => pool.name === selectedPool)?.name}`}
               items={
                 siloPools.map((pool) => ({
@@ -838,7 +838,7 @@ const AdvancedAccordion = ({
                       )
                     }}
                     required
-                    placeholder="Select floating IP"
+                    placeholder="Select a floating IP"
                     selected={selectedFloatingIp?.name || ''}
                   />
                 </form>
diff --git a/app/pages/project/floating-ips/FloatingIpsPage.tsx b/app/pages/project/floating-ips/FloatingIpsPage.tsx
index 19714cdc7..d5e4547cf 100644
--- a/app/pages/project/floating-ips/FloatingIpsPage.tsx
+++ b/app/pages/project/floating-ips/FloatingIpsPage.tsx
@@ -290,7 +290,7 @@ const AttachFloatingIpModal = ({
                 form.setValue('instanceId', e)
               }}
               required
-              placeholder="Select instance"
+              placeholder="Select an instance"
               selected={form.watch('instanceId')}
             />
           </form>
diff --git a/app/pages/system/SiloImagesPage.tsx b/app/pages/system/SiloImagesPage.tsx
index 61cd29212..fa7fa3467 100644
--- a/app/pages/system/SiloImagesPage.tsx
+++ b/app/pages/system/SiloImagesPage.tsx
@@ -169,7 +169,7 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => {
         <Modal.Section>
           <form autoComplete="off" onSubmit={handleSubmit(onSubmit)} className="space-y-4">
             <ComboboxField
-              placeholder="Filter images by project"
+              placeholder="Select a project"
               name="project"
               label="Project"
               items={projectItems}
diff --git a/app/pages/system/networking/IpPoolPage.tsx b/app/pages/system/networking/IpPoolPage.tsx
index db7095783..1c77e114d 100644
--- a/app/pages/system/networking/IpPoolPage.tsx
+++ b/app/pages/system/networking/IpPoolPage.tsx
@@ -369,7 +369,7 @@ function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) {
             />
 
             <ComboboxField
-              placeholder="Select silo"
+              placeholder="Select a silo"
               name="silo"
               label="Silo"
               items={unlinkedSiloItems}
diff --git a/app/pages/system/silos/SiloIpPoolsTab.tsx b/app/pages/system/silos/SiloIpPoolsTab.tsx
index ea2da0839..ba2a9d665 100644
--- a/app/pages/system/silos/SiloIpPoolsTab.tsx
+++ b/app/pages/system/silos/SiloIpPoolsTab.tsx
@@ -236,7 +236,7 @@ function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) {
             />
 
             <ComboboxField
-              placeholder="Select pool"
+              placeholder="Select a pool"
               name="pool"
               label="IP pool"
               items={unlinkedPoolItems}

From e929fab872afd94480075b7ff6e06f19b15705c8 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 1 Jul 2024 18:51:16 -0400
Subject: [PATCH 06/17] Add placeholder on snapshot-create

---
 app/forms/snapshot-create.tsx | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/forms/snapshot-create.tsx b/app/forms/snapshot-create.tsx
index 501ffb963..008025142 100644
--- a/app/forms/snapshot-create.tsx
+++ b/app/forms/snapshot-create.tsx
@@ -76,6 +76,7 @@ export function CreateSnapshotSideModalForm() {
       <ComboboxField
         label="Disk"
         name="disk"
+        placeholder="Select a disk"
         items={diskItems}
         required
         control={form.control}

From c9187e0c3ca1b33c5961341bd9faee7a4c6d19f9 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 09:14:57 -0400
Subject: [PATCH 07/17] Update tests' placeholder strings

---
 test/e2e/floating-ip-create.e2e.ts  | 2 +-
 test/e2e/instance-create.e2e.ts     | 2 +-
 test/e2e/instance-networking.e2e.ts | 2 +-
 test/e2e/ip-pools.e2e.ts            | 2 +-
 test/e2e/silos.e2e.ts               | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/e2e/floating-ip-create.e2e.ts b/test/e2e/floating-ip-create.e2e.ts
index 0f847df0a..62ba9d2d8 100644
--- a/test/e2e/floating-ip-create.e2e.ts
+++ b/test/e2e/floating-ip-create.e2e.ts
@@ -83,7 +83,7 @@ test('can detach and attach a floating IP', async ({ page }) => {
   // Now click back to floating IPs and reattach it to db1
   await page.getByRole('link', { name: 'Floating IPs' }).click()
   await clickRowAction(page, 'cola-float', 'Attach')
-  await page.getByRole('button', { name: 'Select instance' }).click()
+  await page.getByRole('button', { name: 'Select an instance' }).click()
   await page.getByRole('option', { name: 'db1' }).click()
 
   await page.getByRole('button', { name: 'Attach' }).click()
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 35a8d9655..316dfa6b6 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -378,7 +378,7 @@ test('does not attach an ephemeral IP when the checkbox is unchecked', async ({
 
 test('attaches a floating IP; disables button when no IPs available', async ({ page }) => {
   const attachFloatingIpButton = page.getByRole('button', { name: 'Attach floating IP' })
-  const selectFloatingIpButton = page.getByRole('button', { name: 'Select floating ip' })
+  const selectFloatingIpButton = page.getByRole('button', { name: 'Select a floating ip' })
   const rootbeerFloatOption = page.getByRole('option', { name: 'rootbeer-float' })
   const attachButton = page.getByRole('button', { name: 'Attach', exact: true })
 
diff --git a/test/e2e/instance-networking.e2e.ts b/test/e2e/instance-networking.e2e.ts
index 93045d287..39aeecc3d 100644
--- a/test/e2e/instance-networking.e2e.ts
+++ b/test/e2e/instance-networking.e2e.ts
@@ -129,7 +129,7 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
   // Select the 'rootbeer-float' option
   const dialog = page.getByRole('dialog')
   // TODO: this "select the option" syntax is awkward; it's working, but I suspect there's a better way
-  await dialog.getByRole('button', { name: 'Select floating IP' }).click()
+  await dialog.getByRole('button', { name: 'Select a floating IP' }).click()
   await page.keyboard.press('ArrowDown')
   await page.keyboard.press('Enter')
   // await dialog.getByRole('button', { name: 'rootbeer-float' }).click()
diff --git a/test/e2e/ip-pools.e2e.ts b/test/e2e/ip-pools.e2e.ts
index 32becca80..947079489 100644
--- a/test/e2e/ip-pools.e2e.ts
+++ b/test/e2e/ip-pools.e2e.ts
@@ -101,7 +101,7 @@ test('IP pool link silo', async ({ page }) => {
   await expect(modal).toBeVisible()
 
   // select silo in combobox and click link
-  await page.getByPlaceholder('Select silo').fill('m')
+  await page.getByPlaceholder('Select a silo').fill('m')
   await page.getByRole('option', { name: 'myriad' }).click()
   await modal.getByRole('button', { name: 'Link' }).click()
 
diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts
index cbdb893d6..43b1154c8 100644
--- a/test/e2e/silos.e2e.ts
+++ b/test/e2e/silos.e2e.ts
@@ -243,7 +243,7 @@ test('Silo IP pools link pool', async ({ page }) => {
   await expect(modal).toBeVisible()
 
   // select silo in combobox and click link
-  await page.getByPlaceholder('Select pool').fill('ip-pool')
+  await page.getByPlaceholder('Select a pool').fill('ip-pool')
   await page.getByRole('option', { name: 'ip-pool-3' }).click()
   await modal.getByRole('button', { name: 'Link' }).click()
 

From b1c77550eb40b0799d13085cbead988110adfbca Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 09:47:56 -0400
Subject: [PATCH 08/17] one more string update for tests

---
 test/e2e/images.e2e.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts
index 67b910cdf..0c34aae0b 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.getByPlaceholder('Filter images by project')).toBeVisible()
+  await expect(page.getByPlaceholder('Select a project')).toBeVisible()
   await expect(page.locator(`text="Select an image"`)).toBeVisible()
 
   // Notice is visible

From c490c31831ea4800e82b43dd3b236b285278a37d Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 10:13:51 -0400
Subject: [PATCH 09/17] refactor util function name

---
 test/e2e/instance-create.e2e.ts | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 316dfa6b6..8d297c72a 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -16,19 +16,19 @@ import {
   type Page,
 } from './utils'
 
-const pickASiloImage = async (page: Page, name: string) => {
+const selectASiloImage = async (page: Page, name: string) => {
   await page.getByRole('tab', { name: 'Silo images' }).click()
   await page.getByRole('button', { name: 'Select an image' }).click()
   await page.getByRole('option', { name }).click()
 }
 
-const pickAProjectImage = async (page: Page, index: number) => {
+const selectAProjectImage = async (page: Page, index: number) => {
   await page.getByRole('tab', { name: 'Project images' }).click()
   await page.getByRole('button', { name: 'Image' }).click()
   await page.getByRole('option', { name: images[index].name }).click()
 }
 
-const pickAnExistingDisk = async (page: Page, diskName: string) => {
+const selectAnExistingDisk = async (page: Page, diskName: string) => {
   await page.getByRole('tab', { name: 'Existing disks' }).click()
   await page.getByRole('button', { name: 'Select a disk' }).click()
   await page.getByRole('option', { name: diskName }).click()
@@ -61,7 +61,7 @@ test('can create an instance', async ({ page }) => {
   await diskSizeInput.fill('20')
 
   // pick a project image just to show we can
-  await pickAProjectImage(page, 2)
+  await selectAProjectImage(page, 2)
 
   // should be hidden in accordion
   await expectNotVisible(page, [
@@ -127,7 +127,7 @@ test('can create an instance', async ({ page }) => {
 test('duplicate instance name produces visible error', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.fill('input[name=name]', 'db1')
-  await pickAProjectImage(page, 0)
+  await selectAProjectImage(page, 0)
   await page.locator('button:has-text("Create instance")').click()
   await expect(page.getByText('Instance name already exists')).toBeVisible()
 })
@@ -168,7 +168,7 @@ test('can create an instance with custom hardware', async ({ page }) => {
   await page.keyboard.press('Tab')
 
   // pick a project image just to show we can
-  await pickAProjectImage(page, 2)
+  await selectAProjectImage(page, 2)
   // the disk size should bot have been changed from what was entered earlier
   await expect(diskSizeInput).toHaveValue('20')
 
@@ -204,13 +204,13 @@ test('automatically updates disk size when larger image selected', async ({ page
   await page.keyboard.press('Tab')
 
   // pick a disk image that's smaller than 5GiB (the first project image works [4GiB])
-  await pickAProjectImage(page, 0)
+  await selectAProjectImage(page, 0)
 
   // test that it still says 5, as that's larger than the given image
   await expect(diskSizeInput).toHaveValue('5')
 
   // pick a disk image that's larger than 5GiB (the third project image works [6GiB])
-  await pickAProjectImage(page, 2)
+  await selectAProjectImage(page, 2)
 
   // test that it has been automatically increased to next-largest incremement of 10
   await expect(diskSizeInput).toHaveValue('10')
@@ -230,7 +230,7 @@ test('automatically updates disk size when larger image selected', async ({ page
 test('with disk name already taken', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.fill('input[name=name]', 'my-instance')
-  await pickAProjectImage(page, 0)
+  await selectAProjectImage(page, 0)
   await page.fill('input[name=bootDiskName]', 'disk-1')
 
   await page.getByRole('button', { name: 'Create instance' }).click()
@@ -268,7 +268,7 @@ test('add ssh key from instance create form', async ({ page }) => {
 test('shows object not found error on no default pool', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-default-pool')
-  await pickAProjectImage(page, 0)
+  await selectAProjectImage(page, 0)
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page.getByText('Not found: default IP pool for current silo')).toBeVisible()
 })
@@ -277,7 +277,7 @@ test('create instance with existing disk', async ({ page }) => {
   const instanceName = 'my-existing-disk-instance'
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
-  await pickAnExistingDisk(page, 'disk-3')
+  await selectAnExistingDisk(page, 'disk-3')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`)
   await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB'])
@@ -288,7 +288,7 @@ test('create instance with a silo image', async ({ page }) => {
   const instanceName = 'my-existing-disk-2'
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
-  await pickASiloImage(page, 'ubuntu-22-04')
+  await selectASiloImage(page, 'ubuntu-22-04')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`)
   await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=10 GiB'])
@@ -298,8 +298,8 @@ test('start with an existing disk, but then switch to a silo image', async ({ pa
   const instanceName = 'silo-image-instance'
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
-  await pickAnExistingDisk(page, 'disk-7')
-  await pickASiloImage(page, 'ubuntu-22-04')
+  await selectAnExistingDisk(page, 'disk-7')
+  await selectASiloImage(page, 'ubuntu-22-04')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`)
   await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB'])
@@ -366,7 +366,7 @@ test('maintains selected values even when changing tabs', async ({ page }) => {
 test('does not attach an ephemeral IP when the checkbox is unchecked', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-ephemeral-ip')
-  await pickAProjectImage(page, 0)
+  await selectAProjectImage(page, 0)
   await page.getByRole('button', { name: 'Networking' }).click()
   await page
     .getByRole('checkbox', { name: 'Allocate and attach an ephemeral IP address' })
@@ -385,7 +385,7 @@ test('attaches a floating IP; disables button when no IPs available', async ({ p
   const instanceName = 'with-floating-ip'
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
-  await pickAProjectImage(page, 0)
+  await selectAProjectImage(page, 0)
   await page.getByRole('button', { name: 'Networking' }).click()
 
   await attachFloatingIpButton.click()

From 115aeec29c12efd6ff244f23c3c5314d2718313d Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 11:03:04 -0400
Subject: [PATCH 10/17] small refactor for consistency

---
 test/e2e/instance-create.e2e.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 8d297c72a..975db0af4 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -28,10 +28,10 @@ const selectAProjectImage = async (page: Page, index: number) => {
   await page.getByRole('option', { name: images[index].name }).click()
 }
 
-const selectAnExistingDisk = async (page: Page, diskName: string) => {
+const selectAnExistingDisk = async (page: Page, name: string) => {
   await page.getByRole('tab', { name: 'Existing disks' }).click()
   await page.getByRole('button', { name: 'Select a disk' }).click()
-  await page.getByRole('option', { name: diskName }).click()
+  await page.getByRole('option', { name }).click()
 }
 
 test('can create an instance', async ({ page }) => {

From c80be051fbaf7a2df65f040b37b7405545d8e96f Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 11:52:44 -0400
Subject: [PATCH 11/17] Revert NIC form to use Listboxes instead of Comboboxes

---
 app/components/form/fields/ListboxField.tsx  |  3 ++
 app/components/form/fields/SubnetListbox.tsx |  8 +++--
 app/forms/instance-create.tsx                | 34 +++++++++-----------
 app/forms/network-interface-create.tsx       |  5 +--
 test/e2e/network-interface-create.e2e.ts     |  2 +-
 5 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/app/components/form/fields/ListboxField.tsx b/app/components/form/fields/ListboxField.tsx
index 20f3da2b5..6ffefbea6 100644
--- a/app/components/form/fields/ListboxField.tsx
+++ b/app/components/form/fields/ListboxField.tsx
@@ -34,6 +34,7 @@ export type ListboxFieldProps<
   items: ListboxItem[]
   onChange?: (value: string | null | undefined) => void
   isLoading?: boolean
+  noItemsPlaceholder?: string
 }
 
 export function ListboxField<
@@ -52,6 +53,7 @@ export function ListboxField<
   control,
   onChange,
   isLoading,
+  noItemsPlaceholder,
 }: ListboxFieldProps<TFieldValues, TName>) {
   // TODO: recreate this logic
   //   validate: (v) => (required && !v ? `${name} is required` : undefined),
@@ -64,6 +66,7 @@ export function ListboxField<
         tooltipText={tooltipText}
         required={required}
         placeholder={placeholder}
+        noItemsPlaceholder={noItemsPlaceholder}
         selected={field.value || null}
         items={items}
         onChange={(value) => {
diff --git a/app/components/form/fields/SubnetListbox.tsx b/app/components/form/fields/SubnetListbox.tsx
index 398dfde7b..fd1d65656 100644
--- a/app/components/form/fields/SubnetListbox.tsx
+++ b/app/components/form/fields/SubnetListbox.tsx
@@ -11,12 +11,12 @@ import { useApiQuery } from '@oxide/api'
 
 import { useProjectSelector } from '~/hooks'
 
-import { ComboboxField, type ComboboxFieldProps } from './ComboboxField'
+import { ListboxField, type ListboxFieldProps } from './ListboxField'
 
 type SubnetListboxProps<
   TFieldValues extends FieldValues,
   TName extends FieldPath<TFieldValues>,
-> = Omit<ComboboxFieldProps<TFieldValues, TName>, 'items'> & {
+> = Omit<ListboxFieldProps<TFieldValues, TName>, 'items'> & {
   vpcNameField: FieldPath<TFieldValues>
 }
 
@@ -47,11 +47,13 @@ export function SubnetListbox<
     ).data?.items || []
 
   return (
-    <ComboboxField
+    <ListboxField
       {...fieldProps}
       items={subnets.map(({ name }) => ({ value: name, label: name }))}
       disabled={!vpcExists}
       control={control}
+      placeholder="Select a subnet"
+      noItemsPlaceholder="Select a VPC to see subnets"
     />
   )
 }
diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 4a633eded..4324ce620 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -771,25 +771,21 @@ const AdvancedAccordion = ({
                 <MiniTable.HeadCell className="w-12" />
               </MiniTable.Header>
               <MiniTable.Body>
-                {attachedFloatingIpsData.map((item, index) =>
-                  item ? (
-                    <MiniTable.Row
-                      tabIndex={0}
-                      aria-rowindex={index + 1}
-                      aria-label={`Name: ${item.name}, IP: ${item.ip}`}
-                      key={item.name}
-                    >
-                      <MiniTable.Cell>{item.name}</MiniTable.Cell>
-                      <MiniTable.Cell>{item.ip}</MiniTable.Cell>
-                      <MiniTable.RemoveCell
-                        onClick={() => detachFloatingIp(item.name)}
-                        label={`remove floating IP ${item.name}`}
-                      />
-                    </MiniTable.Row>
-                  ) : (
-                    <></>
-                  )
-                )}
+                {attachedFloatingIpsData.map((item, index) => (
+                  <MiniTable.Row
+                    tabIndex={0}
+                    aria-rowindex={index + 1}
+                    aria-label={`Name: ${item.name}, IP: ${item.ip}`}
+                    key={item.name}
+                  >
+                    <MiniTable.Cell>{item.name}</MiniTable.Cell>
+                    <MiniTable.Cell>{item.ip}</MiniTable.Cell>
+                    <MiniTable.RemoveCell
+                      onClick={() => detachFloatingIp(item.name)}
+                      label={`remove floating IP ${item.name}`}
+                    />
+                  </MiniTable.Row>
+                ))}
               </MiniTable.Body>
             </MiniTable.Table>
           )}
diff --git a/app/forms/network-interface-create.tsx b/app/forms/network-interface-create.tsx
index 756da6c0e..c3e452d32 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,12 +65,13 @@ export function CreateNetworkInterfaceForm({
       <DescriptionField name="description" control={form.control} />
       <FormDivider />
 
-      <ComboboxField
+      <ListboxField
         name="vpcName"
         label="VPC"
         items={vpcs.map(({ name }) => ({ label: name, value: name }))}
         required
         control={form.control}
+        placeholder="Select a VPC"
       />
       <SubnetListbox
         name="subnetName"
diff --git a/test/e2e/network-interface-create.e2e.ts b/test/e2e/network-interface-create.e2e.ts
index 4ff5a4723..db6890faf 100644
--- a/test/e2e/network-interface-create.e2e.ts
+++ b/test/e2e/network-interface-create.e2e.ts
@@ -20,7 +20,7 @@ test('can create a NIC with a specified IP address', async ({ page }) => {
 
   // fill out the form
   await page.getByLabel('Name').fill('nic-1')
-  await page.getByRole('button', { name: 'VPC' }).click()
+  await page.getByRole('button', { name: 'VPC' }).first().click()
   await page.getByRole('option', { name: 'mock-vpc' }).click()
   await page.getByRole('button', { name: 'Subnet' }).click()
   await page.getByRole('option', { name: 'mock-subnet' }).click()

From 92f92169bc6691d2bf0f75f54016e0920aa99324 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 12:10:45 -0400
Subject: [PATCH 12/17] conditional noItems placeholder for clearer messaging

---
 app/components/form/fields/SubnetListbox.tsx | 10 ++++++++--
 app/forms/network-interface-create.tsx       |  3 +++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/app/components/form/fields/SubnetListbox.tsx b/app/components/form/fields/SubnetListbox.tsx
index fd1d65656..66989d7bb 100644
--- a/app/components/form/fields/SubnetListbox.tsx
+++ b/app/components/form/fields/SubnetListbox.tsx
@@ -18,6 +18,7 @@ type SubnetListboxProps<
   TName extends FieldPath<TFieldValues>,
 > = Omit<ListboxFieldProps<TFieldValues, TName>, 'items'> & {
   vpcNameField: FieldPath<TFieldValues>
+  noItemsPlaceholder?: string
 }
 
 /**
@@ -30,7 +31,12 @@ type SubnetListboxProps<
 export function SubnetListbox<
   TFieldValues extends FieldValues,
   TName extends FieldPath<TFieldValues>,
->({ vpcNameField, control, ...fieldProps }: SubnetListboxProps<TFieldValues, TName>) {
+>({
+  vpcNameField,
+  control,
+  noItemsPlaceholder,
+  ...fieldProps
+}: SubnetListboxProps<TFieldValues, TName>) {
   const projectSelector = useProjectSelector()
 
   const [vpcName] = useWatch({ control, name: [vpcNameField] })
@@ -53,7 +59,7 @@ export function SubnetListbox<
       disabled={!vpcExists}
       control={control}
       placeholder="Select a subnet"
-      noItemsPlaceholder="Select a VPC to see subnets"
+      noItemsPlaceholder={noItemsPlaceholder}
     />
   )
 }
diff --git a/app/forms/network-interface-create.tsx b/app/forms/network-interface-create.tsx
index c3e452d32..2c96395bf 100644
--- a/app/forms/network-interface-create.tsx
+++ b/app/forms/network-interface-create.tsx
@@ -79,6 +79,9 @@ export function CreateNetworkInterfaceForm({
         vpcNameField="vpcName"
         required
         control={form.control}
+        noItemsPlaceholder={
+          form.watch('vpcName') ? 'No subnets found' : 'Select a VPC to see subnets'
+        }
       />
       <TextField
         name="ip"

From 49dbca7de9229991477b1277bfa929b8062c8097 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 12:31:47 -0400
Subject: [PATCH 13/17] Update test/e2e/network-interface-create.e2e.ts

Co-authored-by: David Crespo <david-crespo@users.noreply.github.com>
---
 test/e2e/network-interface-create.e2e.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/e2e/network-interface-create.e2e.ts b/test/e2e/network-interface-create.e2e.ts
index db6890faf..d51a32e3c 100644
--- a/test/e2e/network-interface-create.e2e.ts
+++ b/test/e2e/network-interface-create.e2e.ts
@@ -20,7 +20,7 @@ test('can create a NIC with a specified IP address', async ({ page }) => {
 
   // fill out the form
   await page.getByLabel('Name').fill('nic-1')
-  await page.getByRole('button', { name: 'VPC' }).first().click()
+  await page.getByLabel('VPC', { exact: true }).click()
   await page.getByRole('option', { name: 'mock-vpc' }).click()
   await page.getByRole('button', { name: 'Subnet' }).click()
   await page.getByRole('option', { name: 'mock-subnet' }).click()

From 3908ff8b455c161810c9cbe54ab46e737f052e1a Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 12:39:39 -0400
Subject: [PATCH 14/17] refactor test to use string in place of index

---
 test/e2e/instance-create.e2e.ts | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 975db0af4..adab4d1de 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -22,10 +22,10 @@ const selectASiloImage = async (page: Page, name: string) => {
   await page.getByRole('option', { name }).click()
 }
 
-const selectAProjectImage = async (page: Page, index: number) => {
+const selectAProjectImage = async (page: Page, name: string) => {
   await page.getByRole('tab', { name: 'Project images' }).click()
-  await page.getByRole('button', { name: 'Image' }).click()
-  await page.getByRole('option', { name: images[index].name }).click()
+  await page.getByRole('button', { name: 'Select an image' }).click()
+  await page.getByRole('option', { name }).click()
 }
 
 const selectAnExistingDisk = async (page: Page, name: string) => {
@@ -61,7 +61,7 @@ test('can create an instance', async ({ page }) => {
   await diskSizeInput.fill('20')
 
   // pick a project image just to show we can
-  await selectAProjectImage(page, 2)
+  await selectAProjectImage(page, 'image-3')
 
   // should be hidden in accordion
   await expectNotVisible(page, [
@@ -127,7 +127,7 @@ test('can create an instance', async ({ page }) => {
 test('duplicate instance name produces visible error', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.fill('input[name=name]', 'db1')
-  await selectAProjectImage(page, 0)
+  await selectAProjectImage(page, 'image-1')
   await page.locator('button:has-text("Create instance")').click()
   await expect(page.getByText('Instance name already exists')).toBeVisible()
 })
@@ -168,7 +168,7 @@ test('can create an instance with custom hardware', async ({ page }) => {
   await page.keyboard.press('Tab')
 
   // pick a project image just to show we can
-  await selectAProjectImage(page, 2)
+  await selectAProjectImage(page, 'image-3')
   // the disk size should bot have been changed from what was entered earlier
   await expect(diskSizeInput).toHaveValue('20')
 
@@ -204,13 +204,13 @@ test('automatically updates disk size when larger image selected', async ({ page
   await page.keyboard.press('Tab')
 
   // pick a disk image that's smaller than 5GiB (the first project image works [4GiB])
-  await selectAProjectImage(page, 0)
+  await selectAProjectImage(page, 'image-1')
 
   // test that it still says 5, as that's larger than the given image
   await expect(diskSizeInput).toHaveValue('5')
 
   // pick a disk image that's larger than 5GiB (the third project image works [6GiB])
-  await selectAProjectImage(page, 2)
+  await selectAProjectImage(page, 'image-3')
 
   // test that it has been automatically increased to next-largest incremement of 10
   await expect(diskSizeInput).toHaveValue('10')
@@ -230,7 +230,7 @@ test('automatically updates disk size when larger image selected', async ({ page
 test('with disk name already taken', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.fill('input[name=name]', 'my-instance')
-  await selectAProjectImage(page, 0)
+  await selectAProjectImage(page, 'image-1')
   await page.fill('input[name=bootDiskName]', 'disk-1')
 
   await page.getByRole('button', { name: 'Create instance' }).click()
@@ -268,7 +268,7 @@ test('add ssh key from instance create form', async ({ page }) => {
 test('shows object not found error on no default pool', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-default-pool')
-  await selectAProjectImage(page, 0)
+  await selectAProjectImage(page, 'image-1')
   await page.getByRole('button', { name: 'Create instance' }).click()
   await expect(page.getByText('Not found: default IP pool for current silo')).toBeVisible()
 })
@@ -366,7 +366,7 @@ test('maintains selected values even when changing tabs', async ({ page }) => {
 test('does not attach an ephemeral IP when the checkbox is unchecked', async ({ page }) => {
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-ephemeral-ip')
-  await selectAProjectImage(page, 0)
+  await selectAProjectImage(page, 'image-1')
   await page.getByRole('button', { name: 'Networking' }).click()
   await page
     .getByRole('checkbox', { name: 'Allocate and attach an ephemeral IP address' })
@@ -385,7 +385,7 @@ test('attaches a floating IP; disables button when no IPs available', async ({ p
   const instanceName = 'with-floating-ip'
   await page.goto('/projects/mock-project/instances-new')
   await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName)
-  await selectAProjectImage(page, 0)
+  await selectAProjectImage(page, 'image-1')
   await page.getByRole('button', { name: 'Networking' }).click()
 
   await attachFloatingIpButton.click()

From c77dfb06a5a71a79d8e56146f6f9eba867acf0f5 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 12:54:07 -0400
Subject: [PATCH 15/17] Update test; shift dynamic placeholder copy into
 SubnetListbox

---
 app/components/form/fields/SubnetListbox.tsx | 10 ++--------
 app/forms/network-interface-create.tsx       |  3 ---
 test/e2e/z-index.e2e.ts                      |  2 +-
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/app/components/form/fields/SubnetListbox.tsx b/app/components/form/fields/SubnetListbox.tsx
index 66989d7bb..77931f63b 100644
--- a/app/components/form/fields/SubnetListbox.tsx
+++ b/app/components/form/fields/SubnetListbox.tsx
@@ -18,7 +18,6 @@ type SubnetListboxProps<
   TName extends FieldPath<TFieldValues>,
 > = Omit<ListboxFieldProps<TFieldValues, TName>, 'items'> & {
   vpcNameField: FieldPath<TFieldValues>
-  noItemsPlaceholder?: string
 }
 
 /**
@@ -31,12 +30,7 @@ type SubnetListboxProps<
 export function SubnetListbox<
   TFieldValues extends FieldValues,
   TName extends FieldPath<TFieldValues>,
->({
-  vpcNameField,
-  control,
-  noItemsPlaceholder,
-  ...fieldProps
-}: SubnetListboxProps<TFieldValues, TName>) {
+>({ vpcNameField, control, ...fieldProps }: SubnetListboxProps<TFieldValues, TName>) {
   const projectSelector = useProjectSelector()
 
   const [vpcName] = useWatch({ control, name: [vpcNameField] })
@@ -59,7 +53,7 @@ export function SubnetListbox<
       disabled={!vpcExists}
       control={control}
       placeholder="Select a subnet"
-      noItemsPlaceholder={noItemsPlaceholder}
+      noItemsPlaceholder={vpcName ? 'No subnets found' : 'Select a VPC to see subnets'}
     />
   )
 }
diff --git a/app/forms/network-interface-create.tsx b/app/forms/network-interface-create.tsx
index 2c96395bf..c3e452d32 100644
--- a/app/forms/network-interface-create.tsx
+++ b/app/forms/network-interface-create.tsx
@@ -79,9 +79,6 @@ export function CreateNetworkInterfaceForm({
         vpcNameField="vpcName"
         required
         control={form.control}
-        noItemsPlaceholder={
-          form.watch('vpcName') ? 'No subnets found' : 'Select a VPC to see subnets'
-        }
       />
       <TextField
         name="ip"
diff --git a/test/e2e/z-index.e2e.ts b/test/e2e/z-index.e2e.ts
index 9b7ce9fd5..6379685bc 100644
--- a/test/e2e/z-index.e2e.ts
+++ b/test/e2e/z-index.e2e.ts
@@ -55,7 +55,7 @@ test('Dropdown content in SidebarModal shows on screen', async ({ page }) => {
 
   // select the VPC and subnet via the dropdowns. The fact that the options are
   // clickable means they are not obscured due to having a too-low z-index
-  await page.getByRole('button', { name: 'VPC' }).click()
+  await page.getByLabel('VPC', { exact: true }).click()
   await page.getByRole('option', { name: 'mock-vpc' }).click()
   await page.getByRole('button', { name: 'Subnet' }).click()
   await page.getByRole('option', { name: 'mock-subnet' }).click()

From f4338e10a7a6a960dade8d43610d78e46204dedc Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 13:12:03 -0400
Subject: [PATCH 16/17] missed one last test

---
 test/e2e/network-interface-create.e2e.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/e2e/network-interface-create.e2e.ts b/test/e2e/network-interface-create.e2e.ts
index d51a32e3c..c735553fe 100644
--- a/test/e2e/network-interface-create.e2e.ts
+++ b/test/e2e/network-interface-create.e2e.ts
@@ -47,7 +47,7 @@ test('can create a NIC with a blank IP address', async ({ page }) => {
 
   // fill out the form
   await page.getByLabel('Name').fill('nic-2')
-  await page.getByRole('button', { name: 'VPC' }).click()
+  await page.getByLabel('VPC', { exact: true }).click()
   await page.getByRole('option', { name: 'mock-vpc' }).click()
   await page.getByRole('button', { name: 'Subnet' }).click()
   await page.getByRole('option', { name: 'mock-subnet' }).click()

From 0f25455fa743a43f053cb27641104aa73d87a4ad Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 2 Jul 2024 13:33:57 -0400
Subject: [PATCH 17/17] And a bit more test refactoring

---
 test/e2e/instance-create.e2e.ts     |  9 ++++-----
 test/e2e/instance-networking.e2e.ts | 26 +++++++++++++-------------
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index adab4d1de..5ccf320d3 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -5,7 +5,7 @@
  *
  * Copyright Oxide Computer Company
  */
-import { floatingIp, images } from '@oxide/api-mocks'
+import { floatingIp } from '@oxide/api-mocks'
 
 import {
   expect,
@@ -18,13 +18,13 @@ import {
 
 const selectASiloImage = async (page: Page, name: string) => {
   await page.getByRole('tab', { name: 'Silo images' }).click()
-  await page.getByRole('button', { name: 'Select an image' }).click()
+  await page.getByLabel('Image', { exact: true }).click()
   await page.getByRole('option', { name }).click()
 }
 
 const selectAProjectImage = async (page: Page, name: string) => {
   await page.getByRole('tab', { name: 'Project images' }).click()
-  await page.getByRole('button', { name: 'Select an image' }).click()
+  await page.getByLabel('Image', { exact: true }).click()
   await page.getByRole('option', { name }).click()
 }
 
@@ -216,8 +216,7 @@ test('automatically updates disk size when larger image selected', async ({ page
   await expect(diskSizeInput).toHaveValue('10')
 
   // pick another image, just to verify that the diskSizeInput stays as it was
-  await page.getByRole('button', { name: 'Image' }).click()
-  await page.getByRole('option', { name: images[1].name }).click()
+  await selectAProjectImage(page, 'image-2')
   await expect(diskSizeInput).toHaveValue('10')
 
   const submitButton = page.getByRole('button', { name: 'Create instance' })
diff --git a/test/e2e/instance-networking.e2e.ts b/test/e2e/instance-networking.e2e.ts
index 39aeecc3d..2ae4fb4ef 100644
--- a/test/e2e/instance-networking.e2e.ts
+++ b/test/e2e/instance-networking.e2e.ts
@@ -17,7 +17,7 @@ test('Instance networking tab — NIC table', async ({ page }) => {
   await expect(page.getByRole('link', { name: '123.4.56.0' })).toBeVisible()
 
   // Instance networking tab
-  await page.click('role=tab[name="Networking"]')
+  await page.getByRole('tab', { name: 'Networking' }).click()
 
   const nicTable = page.getByRole('table', { name: 'Network interfaces' })
 
@@ -44,20 +44,20 @@ test('Instance networking tab — NIC table', async ({ page }) => {
   // TODO: modal title is not getting hooked up, IDs are wrong
   await expectVisible(page, [
     'role=heading[name="Add network interface"]',
-    'role=textbox[name="Name"]',
     'role=textbox[name="Description"]',
-    'role=button[name*="VPC"]', // listbox
-    'role=button[name*="Subnet"]', // listbox
     'role=textbox[name="IP Address"]',
   ])
 
-  await page.fill('role=textbox[name="Name"]', 'nic-2')
-  await page.click('role=button[name*="VPC"]')
-  await page.click('role=option[name="mock-vpc"]')
-  await page.click('role=button[name*="Subnet"]')
-  await page.click('role=option[name="mock-subnet"]')
-  await page.click('role=dialog >> role=button[name="Add network interface"]')
-  await expectVisible(page, ['role=cell[name="nic-2"]'])
+  await page.getByRole('textbox', { name: 'Name' }).fill('nic-2')
+  await page.getByLabel('VPC', { exact: true }).click()
+  await page.getByRole('option', { name: 'mock-vpc' }).click()
+  await page.getByLabel('Subnet').click()
+  await page.getByRole('option', { name: 'mock-subnet' }).click()
+  await page
+    .getByRole('dialog')
+    .getByRole('button', { name: 'Add network interface' })
+    .click()
+  await expect(page.getByRole('cell', { name: 'nic-2' })).toBeVisible()
 
   // Make this interface primary
   await clickRowAction(page, 'nic-2', 'Make primary')
@@ -66,8 +66,8 @@ test('Instance networking tab — NIC table', async ({ page }) => {
 
   // Make an edit to the network interface
   await clickRowAction(page, 'nic-2', 'Edit')
-  await page.fill('role=textbox[name="Name"]', 'nic-3')
-  await page.click('role=button[name="Update network interface"]')
+  await page.getByRole('textbox', { name: 'Name' }).fill('nic-3')
+  await page.getByRole('button', { name: 'Update network interface' }).click()
   await expect(page.getByRole('cell', { name: 'nic-2' })).toBeHidden()
   const nic3 = page.getByRole('cell', { name: 'nic-3' })
   await expect(nic3).toBeVisible()