From a747fe9466e44743489911f982ccd08c7096852b Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 23 Oct 2024 15:36:26 -0700
Subject: [PATCH 01/13] Update how allowArbitraryValue functions in comboboxes

---
 app/ui/lib/Combobox.tsx | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx
index bc7de2ff8a..b4008903af 100644
--- a/app/ui/lib/Combobox.tsx
+++ b/app/ui/lib/Combobox.tsx
@@ -177,13 +177,21 @@ export const Combobox = ({
             </ComboboxButton>
           )}
         </div>
-        {items.length > 0 && (
+        {(items.length > 0 || allowArbitraryValues) && (
           <ComboboxOptions
             anchor="bottom start"
             // 14px gap is presumably because it's measured from inside the outline or something
             className={`ox-menu pointer-events-auto ${zIndex} relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border !outline-none border-secondary [--anchor-gap:14px] empty:hidden`}
             modal={false}
           >
+            {allowArbitraryValues && filteredItems.length === 0 && (
+              <ComboboxOption value={query} className="relative">
+                <div className="ox-menu-item is-highlighted">
+                  <span className="text-secondary">Use </span>
+                  {query}
+                </div>
+              </ComboboxOption>
+            )}
             {!allowArbitraryValues && filteredItems.length === 0 && (
               <ComboboxOption disabled value="no-matches" className="relative">
                 <div className="ox-menu-item !text-disabled">No items match</div>

From 63484a66c9bf1d89d1b36b67db3665cb95e803bd Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 23 Oct 2024 16:33:26 -0700
Subject: [PATCH 02/13] Update vertical position, to reduce peek-through of
 button on lower layer

---
 app/ui/lib/Combobox.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx
index b4008903af..50b93d5b76 100644
--- a/app/ui/lib/Combobox.tsx
+++ b/app/ui/lib/Combobox.tsx
@@ -181,7 +181,7 @@ export const Combobox = ({
           <ComboboxOptions
             anchor="bottom start"
             // 14px gap is presumably because it's measured from inside the outline or something
-            className={`ox-menu pointer-events-auto ${zIndex} relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border !outline-none border-secondary [--anchor-gap:14px] empty:hidden`}
+            className={`ox-menu pointer-events-auto ${zIndex} relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border !outline-none border-secondary [--anchor-gap:13px] empty:hidden`}
             modal={false}
           >
             {allowArbitraryValues && filteredItems.length === 0 && (

From c30b2f37d6c59d1940bc896228aeebfec7a9336d Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 23 Oct 2024 16:57:54 -0700
Subject: [PATCH 03/13] update test to work with new approach

---
 test/e2e/firewall-rules.e2e.ts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index 44f3ba7731..8ed665ed55 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -188,6 +188,8 @@ test('firewall rule form targets table', async ({ page }) => {
   // now add a subnet by entering text
   await selectOption(page, 'Target type', 'VPC subnet')
   await subnetNameField.fill('abc')
+  // tap the Enter key
+  await subnetNameField.press('Enter')
   await addButton.click()
   await expectRowVisible(targets, { Type: 'subnet', Value: 'abc' })
 

From f8ae171548f98956f47cc15cd8f572ac572e9316 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 23 Oct 2024 20:46:56 -0700
Subject: [PATCH 04/13] Update tests, though Safari is still a bit touchy

---
 test/e2e/firewall-rules.e2e.ts | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index 8ed665ed55..ac83f61caa 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -53,6 +53,7 @@ test('can create firewall rule', async ({ page }) => {
   // add host filter instance "host-filter-instance"
   await selectOption(page, 'Host type', 'Instance')
   await page.getByRole('combobox', { name: 'Instance name' }).fill('host-filter-instance')
+  await page.getByText('Use host-filter-instance').click()
   await page.getByRole('button', { name: 'Add host filter' }).click()
 
   // host is added to hosts table
@@ -167,11 +168,13 @@ test('firewall rule form targets table', async ({ page }) => {
   // add targets with overlapping names and types to test delete
 
   await targetVpcNameField.fill('abc')
+  await page.getByText('Use abc').click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'vpc', Value: 'abc' })
 
   // enter a VPC called 'mock-subnet', even if that doesn't make sense here, to test dropdown later
   await targetVpcNameField.fill('mock-subnet')
+  await page.getByText('Use mock-subnet').click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'vpc', Value: 'mock-subnet' })
 
@@ -188,8 +191,7 @@ test('firewall rule form targets table', async ({ page }) => {
   // now add a subnet by entering text
   await selectOption(page, 'Target type', 'VPC subnet')
   await subnetNameField.fill('abc')
-  // tap the Enter key
-  await subnetNameField.press('Enter')
+  await page.getByText('Use abc').click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'subnet', Value: 'abc' })
 
@@ -223,6 +225,7 @@ test('firewall rule form target validation', async ({ page }) => {
   // Enter invalid VPC name
   const vpcNameField = page.getByRole('combobox', { name: 'VPC name' }).first()
   await vpcNameField.fill('ab-')
+  await page.getByText('Use ab-').click()
   await addButton.click()
   await expect(nameError).toBeVisible()
 
@@ -248,6 +251,7 @@ test('firewall rule form target validation', async ({ page }) => {
   await expect(ipError).toBeHidden()
   await expect(nameError).toBeHidden()
   await vpcNameField.fill('abc')
+  await page.getByText('Use abc').click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'vpc', Value: 'abc' })
 
@@ -286,6 +290,7 @@ test('firewall rule form host validation', async ({ page }) => {
   // Enter invalid VPC name
   const vpcNameField = page.getByRole('combobox', { name: 'VPC name' }).nth(1)
   await vpcNameField.fill('ab-')
+  await page.getByText('Use ab-').click()
   await addButton.click()
   await expect(nameError).toBeVisible()
 
@@ -311,6 +316,7 @@ test('firewall rule form host validation', async ({ page }) => {
   await expect(ipError).toBeHidden()
   await expect(nameError).toBeHidden()
   await vpcNameField.fill('abc')
+  await page.getByText('Use abc').click()
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'vpc', Value: 'abc' })
 
@@ -352,10 +358,12 @@ test('firewall rule form hosts table', async ({ page }) => {
   // add hosts with overlapping names and types to test delete
 
   await hostFiltersVpcNameField.fill('abc')
+  await page.getByText('Use abc').click()
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'vpc', Value: 'abc' })
 
   await hostFiltersVpcNameField.fill('def')
+  await page.getByText('Use def').click()
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'vpc', Value: 'def' })
 
@@ -366,6 +374,7 @@ test('firewall rule form hosts table', async ({ page }) => {
 
   await selectOption(page, 'Host type', 'VPC subnet')
   await page.getByRole('combobox', { name: 'Subnet name' }).fill('abc')
+  await page.getByText('Use abc').click()
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'subnet', Value: 'abc' })
 
@@ -429,6 +438,7 @@ test('can update firewall rule', async ({ page }) => {
   // add host filter
   await selectOption(page, 'Host type', 'VPC subnet')
   await page.getByRole('combobox', { name: 'Subnet name' }).fill('edit-filter-subnet')
+  await page.getByText('Use edit-filter-subnet').click()
   await page.getByRole('button', { name: 'Add host filter' }).click()
 
   // new host is added to hosts table

From 71bf367c1fb68f9964793db4573a0ad8ef79d32b Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 23 Oct 2024 21:30:49 -0700
Subject: [PATCH 05/13] Select via Enter

---
 test/e2e/firewall-rules.e2e.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index ac83f61caa..de8db0af42 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -358,12 +358,12 @@ test('firewall rule form hosts table', async ({ page }) => {
   // add hosts with overlapping names and types to test delete
 
   await hostFiltersVpcNameField.fill('abc')
-  await page.getByText('Use abc').click()
+  await hostFiltersVpcNameField.press('Enter')
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'vpc', Value: 'abc' })
 
   await hostFiltersVpcNameField.fill('def')
-  await page.getByText('Use def').click()
+  await hostFiltersVpcNameField.press('Enter')
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'vpc', Value: 'def' })
 
@@ -374,7 +374,7 @@ test('firewall rule form hosts table', async ({ page }) => {
 
   await selectOption(page, 'Host type', 'VPC subnet')
   await page.getByRole('combobox', { name: 'Subnet name' }).fill('abc')
-  await page.getByText('Use abc').click()
+  await page.getByRole('combobox', { name: 'Subnet name' }).press('Enter')
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'subnet', Value: 'abc' })
 

From 5756cd35fad682da8b1771e69b34b72b6e7d2fbe Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 23 Oct 2024 21:51:01 -0700
Subject: [PATCH 06/13] Update test

---
 test/e2e/firewall-rules.e2e.ts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index de8db0af42..d5b2ef4b16 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -168,13 +168,13 @@ test('firewall rule form targets table', async ({ page }) => {
   // add targets with overlapping names and types to test delete
 
   await targetVpcNameField.fill('abc')
-  await page.getByText('Use abc').click()
+  await targetVpcNameField.press('Enter')
   await addButton.click()
   await expectRowVisible(targets, { Type: 'vpc', Value: 'abc' })
 
   // enter a VPC called 'mock-subnet', even if that doesn't make sense here, to test dropdown later
   await targetVpcNameField.fill('mock-subnet')
-  await page.getByText('Use mock-subnet').click()
+  await targetVpcNameField.press('Enter')
   await addButton.click()
   await expectRowVisible(targets, { Type: 'vpc', Value: 'mock-subnet' })
 
@@ -225,7 +225,7 @@ test('firewall rule form target validation', async ({ page }) => {
   // Enter invalid VPC name
   const vpcNameField = page.getByRole('combobox', { name: 'VPC name' }).first()
   await vpcNameField.fill('ab-')
-  await page.getByText('Use ab-').click()
+  await vpcNameField.press('Enter')
   await addButton.click()
   await expect(nameError).toBeVisible()
 
@@ -290,7 +290,7 @@ test('firewall rule form host validation', async ({ page }) => {
   // Enter invalid VPC name
   const vpcNameField = page.getByRole('combobox', { name: 'VPC name' }).nth(1)
   await vpcNameField.fill('ab-')
-  await page.getByText('Use ab-').click()
+  await vpcNameField.press('Enter')
   await addButton.click()
   await expect(nameError).toBeVisible()
 

From 1ba1b43ef54c7daa9dfec046b049a1bf53bd6285 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Thu, 31 Oct 2024 11:06:27 -0700
Subject: [PATCH 07/13] Change UX for allowArbitraryOption

---
 app/ui/lib/Combobox.tsx | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx
index 50b93d5b76..2941030148 100644
--- a/app/ui/lib/Combobox.tsx
+++ b/app/ui/lib/Combobox.tsx
@@ -97,6 +97,15 @@ export const Combobox = ({
     keys: ['selectedLabel', 'label'],
     sorter: (items) => items, // preserve original order, don't sort by match
   })
+  // If the user has typed in a value that isn't in the list,
+  // add it as an option if `allowArbitraryValues` is true
+  if (
+    allowArbitraryValues &&
+    query.length > 0 &&
+    !filteredItems.some((i) => i.selectedLabel === query)
+  ) {
+    filteredItems.push({ value: query, label: query, selectedLabel: query })
+  }
   const zIndex = usePopoverZIndex()
   const id = useId()
   return (
@@ -184,19 +193,6 @@ export const Combobox = ({
             className={`ox-menu pointer-events-auto ${zIndex} relative w-[calc(var(--input-width)+var(--button-width))] overflow-y-auto border !outline-none border-secondary [--anchor-gap:13px] empty:hidden`}
             modal={false}
           >
-            {allowArbitraryValues && filteredItems.length === 0 && (
-              <ComboboxOption value={query} className="relative">
-                <div className="ox-menu-item is-highlighted">
-                  <span className="text-secondary">Use </span>
-                  {query}
-                </div>
-              </ComboboxOption>
-            )}
-            {!allowArbitraryValues && filteredItems.length === 0 && (
-              <ComboboxOption disabled value="no-matches" className="relative">
-                <div className="ox-menu-item !text-disabled">No items match</div>
-              </ComboboxOption>
-            )}
             {filteredItems.map((item) => (
               <ComboboxOption
                 key={item.value}
@@ -210,7 +206,7 @@ export const Combobox = ({
                   // of those rules one by one. Better to rely on the shared classes.
                   <div
                     className={cn('ox-menu-item', {
-                      'is-selected': selected,
+                      'is-selected': selected && query !== item.value,
                       'is-highlighted': focus,
                     })}
                   >
@@ -219,6 +215,11 @@ export const Combobox = ({
                 )}
               </ComboboxOption>
             ))}
+            {!allowArbitraryValues && filteredItems.length === 0 && (
+              <ComboboxOption disabled value="no-matches" className="relative">
+                <div className="ox-menu-item !text-disabled">No items match</div>
+              </ComboboxOption>
+            )}
           </ComboboxOptions>
         )}
       </HCombobox>

From e9a4a31fa156ae0674da09c2f4c769ef72c9629b Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Thu, 31 Oct 2024 11:14:52 -0700
Subject: [PATCH 08/13] Update test

---
 test/e2e/firewall-rules.e2e.ts | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index d5b2ef4b16..833d80f5f3 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -53,7 +53,7 @@ test('can create firewall rule', async ({ page }) => {
   // add host filter instance "host-filter-instance"
   await selectOption(page, 'Host type', 'Instance')
   await page.getByRole('combobox', { name: 'Instance name' }).fill('host-filter-instance')
-  await page.getByText('Use host-filter-instance').click()
+  await page.getByText('host-filter-instance').click()
   await page.getByRole('button', { name: 'Add host filter' }).click()
 
   // host is added to hosts table
@@ -191,7 +191,7 @@ test('firewall rule form targets table', async ({ page }) => {
   // now add a subnet by entering text
   await selectOption(page, 'Target type', 'VPC subnet')
   await subnetNameField.fill('abc')
-  await page.getByText('Use abc').click()
+  await page.getByText('abc').click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'subnet', Value: 'abc' })
 
@@ -251,7 +251,7 @@ test('firewall rule form target validation', async ({ page }) => {
   await expect(ipError).toBeHidden()
   await expect(nameError).toBeHidden()
   await vpcNameField.fill('abc')
-  await page.getByText('Use abc').click()
+  await page.getByText('abc').click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'vpc', Value: 'abc' })
 
@@ -316,7 +316,7 @@ test('firewall rule form host validation', async ({ page }) => {
   await expect(ipError).toBeHidden()
   await expect(nameError).toBeHidden()
   await vpcNameField.fill('abc')
-  await page.getByText('Use abc').click()
+  await page.getByText('abc').click()
   await addButton.click()
   await expectRowVisible(hosts, { Type: 'vpc', Value: 'abc' })
 
@@ -438,7 +438,7 @@ test('can update firewall rule', async ({ page }) => {
   // add host filter
   await selectOption(page, 'Host type', 'VPC subnet')
   await page.getByRole('combobox', { name: 'Subnet name' }).fill('edit-filter-subnet')
-  await page.getByText('Use edit-filter-subnet').click()
+  await page.getByText('edit-filter-subnet').click()
   await page.getByRole('button', { name: 'Add host filter' }).click()
 
   // new host is added to hosts table

From 7ead0017f9b87902a29b36ecd9c6b5f3ac780201 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Thu, 31 Oct 2024 11:37:49 -0700
Subject: [PATCH 09/13] More test updating

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

diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index 833d80f5f3..7c1a49be7a 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -191,7 +191,7 @@ test('firewall rule form targets table', async ({ page }) => {
   // now add a subnet by entering text
   await selectOption(page, 'Target type', 'VPC subnet')
   await subnetNameField.fill('abc')
-  await page.getByText('abc').click()
+  await page.getByText('abc').first().click()
   await addButton.click()
   await expectRowVisible(targets, { Type: 'subnet', Value: 'abc' })
 

From cdc31c5ed2655f885afc141691341e3f752ff9ed Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 4 Nov 2024 13:12:18 -0800
Subject: [PATCH 10/13] new copy: 'Use custom value:'

---
 app/ui/lib/Combobox.tsx | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx
index 2941030148..d53b543229 100644
--- a/app/ui/lib/Combobox.tsx
+++ b/app/ui/lib/Combobox.tsx
@@ -104,7 +104,11 @@ export const Combobox = ({
     query.length > 0 &&
     !filteredItems.some((i) => i.selectedLabel === query)
   ) {
-    filteredItems.push({ value: query, label: query, selectedLabel: query })
+    filteredItems.push({
+      value: query,
+      label: `Use custom value: ${query}`,
+      selectedLabel: query,
+    })
   }
   const zIndex = usePopoverZIndex()
   const id = useId()

From 45b8585a2ba94b3226fefb23b9d6228baf22ed02 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 6 Nov 2024 13:51:39 -0600
Subject: [PATCH 11/13] gray out "custom:" in virtual item, fix query clearing
 onClose, e2e test

---
 .eslintrc.cjs                  |  2 +-
 app/ui/lib/Combobox.tsx        |  9 ++++++--
 test/e2e/firewall-rules.e2e.ts | 41 +++++++++++++++++++++++++++-------
 3 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/.eslintrc.cjs b/.eslintrc.cjs
index 511c68aa9d..f630805bc7 100644
--- a/.eslintrc.cjs
+++ b/.eslintrc.cjs
@@ -116,7 +116,7 @@ module.exports = {
       rules: {
         'playwright/expect-expect': [
           'warn',
-          { assertFunctionNames: ['expectVisible', 'expectRowVisible'] },
+          { assertFunctionNames: ['expectVisible', 'expectRowVisible', 'expectOptions'] },
         ],
       },
     },
diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx
index d53b543229..4a6292150c 100644
--- a/app/ui/lib/Combobox.tsx
+++ b/app/ui/lib/Combobox.tsx
@@ -106,7 +106,11 @@ export const Combobox = ({
   ) {
     filteredItems.push({
       value: query,
-      label: `Use custom value: ${query}`,
+      label: (
+        <>
+          <span className="text-secondary">Custom:</span> {query}
+        </>
+      ),
       selectedLabel: query,
     })
   }
@@ -119,7 +123,8 @@ export const Combobox = ({
         value={selectedItemValue}
         // fallback to '' allows clearing field to work
         onChange={(val) => onChange(val || '')}
-        onClose={() => setQuery('')}
+        // we only want to keep the query on close when arbitrary values are allowed
+        onClose={allowArbitraryValues ? undefined : () => setQuery('')}
         disabled={disabled || isLoading}
         immediate
         {...props}
diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index 7c1a49be7a..e1945c17e8 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -6,14 +6,9 @@
  * Copyright Oxide Computer Company
  */
 
-import {
-  clickRowAction,
-  expect,
-  expectRowVisible,
-  selectOption,
-  test,
-  type Locator,
-} from './utils'
+import { expect, test, type Locator, type Page } from '@playwright/test'
+
+import { clickRowAction, expectRowVisible, selectOption } from './utils'
 
 const defaultRules = ['allow-internal-inbound', 'allow-ssh', 'allow-icmp']
 
@@ -573,3 +568,33 @@ test('name conflict error on edit', async ({ page }) => {
   await page.getByRole('button', { name: 'Update rule' }).click()
   await expectRowVisible(page.getByRole('table'), { Name: 'allow-icmp2', Priority: '37' })
 })
+
+async function expectOptions(page: Page, options: string[]) {
+  const selector = page.getByRole('option')
+  await expect(selector).toHaveCount(options.length)
+  for (const option of options) {
+    await expect(page.getByRole('option', { name: option })).toBeVisible()
+  }
+}
+
+test('arbitrary values combobox', async ({ page }) => {
+  await page.goto('/projects/mock-project/vpcs/mock-vpc/firewall-rules-new')
+
+  await selectOption(page, 'Target type', 'Instance')
+  const input = page.getByRole('combobox', { name: 'Instance name' })
+
+  await input.focus()
+  await expectOptions(page, ['db1', 'you-fail', 'not-there-yet'])
+
+  await input.fill('d')
+  await expectOptions(page, ['db1', 'Custom: d'])
+
+  await input.blur()
+  await expect(page.getByRole('option')).toBeHidden()
+
+  await expect(input).toHaveValue('d')
+  await input.focus()
+
+  // same options show up after blur (there was a bug around this)
+  await expectOptions(page, ['db1', 'Custom: d'])
+})

From a79be2e66ccfb05971acbafb0af21d537963953c Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Wed, 6 Nov 2024 14:51:18 -0600
Subject: [PATCH 12/13] use key to nuke combobox on host filter value type
 change

---
 app/forms/firewall-rules-common.tsx | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx
index b0935c893b..9d644d6b54 100644
--- a/app/forms/firewall-rules-common.tsx
+++ b/app/forms/firewall-rules-common.tsx
@@ -125,6 +125,10 @@ const DynamicTypeAndValueFields = ({
       {/* In the firewall rules form, a few types get comboboxes instead of text fields */}
       {valueType === 'vpc' || valueType === 'subnet' || valueType === 'instance' ? (
         <ComboboxField
+          // key means we nuke the entire field when we change types. this fixes
+          // a bug where the firewall rule host filter combobox would keep an
+          // custom value query around when you change types
+          key={valueType}
           disabled={disabled}
           name="value"
           {...getFilterValueProps(valueType)}
@@ -468,7 +472,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
       />
 
       <div className="flex flex-col gap-3">
-        {/* We have to blow this up instead of using TextField to get better 
+        {/* We have to blow this up instead of using TextField to get better
             text styling on the label */}
         <div className="mt-2">
           <label id="portRange-label" htmlFor="portRange" className="text-sans-lg">

From bc64570a5e05180fb97ffb60df2913140c9e247e Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Thu, 7 Nov 2024 10:19:02 -0600
Subject: [PATCH 13/13] explicit useEffect for clearing query whenever value is
 cleared

---
 app/forms/firewall-rules-common.tsx |  4 ----
 app/ui/lib/Combobox.tsx             | 22 +++++++++++++++++++++-
 test/e2e/firewall-rules.e2e.ts      | 16 ++++++++++++++++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx
index 9d644d6b54..c9a6d3e9c7 100644
--- a/app/forms/firewall-rules-common.tsx
+++ b/app/forms/firewall-rules-common.tsx
@@ -125,10 +125,6 @@ const DynamicTypeAndValueFields = ({
       {/* In the firewall rules form, a few types get comboboxes instead of text fields */}
       {valueType === 'vpc' || valueType === 'subnet' || valueType === 'instance' ? (
         <ComboboxField
-          // key means we nuke the entire field when we change types. this fixes
-          // a bug where the firewall rule host filter combobox would keep an
-          // custom value query around when you change types
-          key={valueType}
           disabled={disabled}
           name="value"
           {...getFilterValueProps(valueType)}
diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx
index 4a6292150c..f9cbc7cf45 100644
--- a/app/ui/lib/Combobox.tsx
+++ b/app/ui/lib/Combobox.tsx
@@ -15,7 +15,7 @@ import {
 } from '@headlessui/react'
 import cn from 'classnames'
 import { matchSorter } from 'match-sorter'
-import { useId, useState, type ReactNode, type Ref } from 'react'
+import { useEffect, useId, useState, type ReactNode, type Ref } from 'react'
 
 import { SelectArrows6Icon } from '@oxide/design-system/icons/react'
 
@@ -97,6 +97,26 @@ export const Combobox = ({
     keys: ['selectedLabel', 'label'],
     sorter: (items) => items, // preserve original order, don't sort by match
   })
+
+  // In the arbitraryValues case, clear the query whenever the value is cleared.
+  // this is necessary, e.g., for the firewall rules form when you submit the
+  // targets subform and clear the field. Two possible changes we might want to make
+  // here if we run into issues:
+  //
+  //   1. do it all the time, not just in the arbitraryValues case
+  //   2. do it on all value changes, not just on clear
+  //
+  // Currently, I don't think there are any arbitraryValues=false cases where we
+  // set the value from outside. There is an arbitraryvalues=true case where we
+  // setValue to something other than empty string, but we don't need the
+  // sync because that setValue is done in onInputChange and we already are
+  // doing setQuery in here along with it.
+  useEffect(() => {
+    if (allowArbitraryValues && !selectedItemValue) {
+      setQuery('')
+    }
+  }, [allowArbitraryValues, selectedItemValue])
+
   // If the user has typed in a value that isn't in the list,
   // add it as an option if `allowArbitraryValues` is true
   if (
diff --git a/test/e2e/firewall-rules.e2e.ts b/test/e2e/firewall-rules.e2e.ts
index e1945c17e8..1cd693b813 100644
--- a/test/e2e/firewall-rules.e2e.ts
+++ b/test/e2e/firewall-rules.e2e.ts
@@ -580,6 +580,22 @@ async function expectOptions(page: Page, options: string[]) {
 test('arbitrary values combobox', async ({ page }) => {
   await page.goto('/projects/mock-project/vpcs/mock-vpc/firewall-rules-new')
 
+  // test for bug where we'd persist the d after add and only show 'Custom: d'
+  const vpcInput = page.getByRole('combobox', { name: 'VPC name' }).first()
+  await vpcInput.focus()
+  await expectOptions(page, ['mock-vpc'])
+
+  await vpcInput.fill('d')
+  await expectOptions(page, ['Custom: d'])
+
+  await vpcInput.blur()
+  page.getByRole('button', { name: 'Add target' }).click()
+  await expect(vpcInput).toHaveValue('')
+
+  await vpcInput.focus()
+  await expectOptions(page, ['mock-vpc']) // bug cause failure here
+
+  // test keeping query around on blur
   await selectOption(page, 'Target type', 'Instance')
   const input = page.getByRole('combobox', { name: 'Instance name' })