Don't preventDefault on Enter in Combobox if unhandled

app/forms/firewall-rules-common.tsx

@@ -494,6 +494,7 @@ const ProtocolFilters = ({ control }: { control: Control<FirewallRuleValues> })
                 description="Leave blank to match any type"
                 placeholder=""
                 allowArbitraryValues
+                onEnter={submitProtocol}
                 items={icmpTypeItems[selectedProtocolType]}
                 validate={(value) => {
                   const result = parseIcmpType(value)

app/ui/lib/Combobox.browser.spec.tsx

@@ -41,20 +41,26 @@ function ComboboxHarness({
   comboboxItems = items,
   initialValue = '',
   allowArbitraryValues = false,
+  onEnter,
 }: {
   comboboxItems?: ComboboxItem[]
   initialValue?: string
   allowArbitraryValues?: boolean
+  onEnter?: () => void
 }) {
   const [selectedItemValue, setSelectedItemValue] = useState(initialValue)
   const [parentEscapes, setParentEscapes] = useState(0)
+  const [submits, setSubmits] = useState(0)
 
   return (
     // The parent key handler models the modal form's Escape listener.
     // oxlint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
     <form
       aria-label="Parent form"
-      onSubmit={(event) => event.preventDefault()}
+      onSubmit={(event) => {
+        event.preventDefault()
+        setSubmits((count) => count + 1)
+      }}
       onKeyDown={(event) => {
         if (event.key === 'Escape') setParentEscapes((count) => count + 1)
       }}
@@ -70,9 +76,12 @@ function ComboboxHarness({
         onChange={setSelectedItemValue}
         onInputChange={allowArbitraryValues ? setSelectedItemValue : undefined}
         allowArbitraryValues={allowArbitraryValues}
+        onEnter={onEnter}
       />
+      <button type="submit">Submit</button>
       <output>Selected: {selectedItemValue || '(empty)'}</output>
       <output>Parent escapes: {parentEscapes}</output>
+      <output>Submits: {submits}</output>
     </form>
   )
 }
@@ -197,6 +206,36 @@ test('Enter commits the highlighted option instead of the arbitrary query', asyn
   await expect.element(screen.getByText('Selected: mock-vpc')).toBeVisible()
 })
 
+test('Enter selects while open, then submits the form once closed', async () => {
+  const screen = await render(<ComboboxHarness />)
+  const combobox = screen.getByRole('combobox', { name: 'Disk name' })
+
+  await commands.clickComboboxButton('Disk name')
+  await expect.element(combobox).toHaveAttribute('aria-expanded', 'true')
+  await commands.pressComboboxKey('Disk name', 'Home')
+  await commands.pressComboboxKey('Disk name', 'Enter')
+  await expect.element(screen.getByText('Selected: disk-3')).toBeVisible()
+  await expect.element(screen.getByText('Submits: 0')).toBeVisible()
+
+  await commands.pressComboboxKey('Disk name', 'Enter')
+  await expect.element(screen.getByText('Submits: 1')).toBeVisible()
+})
+
+test('onEnter replaces form submission when closed', async () => {
+  const onEnter = vi.fn()
+  const screen = await render(<ComboboxHarness onEnter={onEnter} />)
+
+  await commands.clickComboboxButton('Disk name')
+  await commands.pressComboboxKey('Disk name', 'Home')
+  await commands.pressComboboxKey('Disk name', 'Enter')
+  await expect.element(screen.getByText('Selected: disk-3')).toBeVisible()
+  expect(onEnter).not.toHaveBeenCalled()
+
+  await commands.pressComboboxKey('Disk name', 'Enter')
+  await expect.poll(() => onEnter.mock.calls.length).toBe(1)
+  await expect.element(screen.getByText('Submits: 0')).toBeVisible()
+})
+
 test('Escape closes the options before propagating to the parent', async () => {
   const screen = await render(<ComboboxHarness />)
   const combobox = screen.getByRole('combobox', { name: 'Disk name' })

app/ui/lib/Combobox.tsx

@@ -73,7 +73,11 @@ export type ComboboxBaseProps = {
    * For example, if a form value should be set when the user types, use `onInputChange`.
    */
   onInputChange?: (value: string) => void
-  /** Fires whenever the Enter key is pressed while Combobox input has focus */
+  /**
+   * Fires whenever the Enter key is pressed while Combobox input has focus and
+   * the dropdown is closed. Additionally, it prevents default, so when set it
+   * will disable form submission on Enter press.
+   */
   onEnter?: (event: React.KeyboardEvent<HTMLInputElement>) => void
   /**
    * Optional function to transform the value entered into the input as the user types.
@@ -278,11 +282,12 @@ export const Combobox = ({
                 }}
                 onKeyDown={(e) => {
                   // When the dropdown is open, Enter should select the
-                  // highlighted option (HUI handles this). When closed,
-                  // Enter should submit the subform via onEnter.
-                  if (e.key === 'Enter' && !isOpenRef.current) {
+                  // highlighted option (HUI handles this). When closed, Enter
+                  // should either fire the onEnter handler or, implicitly,
+                  // submit the form it's within.
+                  if (e.key === 'Enter' && !isOpenRef.current && onEnter) {
                     e.preventDefault()
-                    onEnter?.(e)
+                    onEnter(e)
                   }
                 }}
                 placeholder={placeholder}

test/e2e/firewall-rules.e2e.ts

@@ -639,3 +639,22 @@ test('can add ICMPv4 and ICMPv6 protocol filters', async ({ page }) => {
   await page.getByRole('option', { name: 'ICMPv4', exact: true }).click()
   await expect(page.getByRole('combobox', { name: 'ICMPv4 type' })).toHaveValue('')
 })
+
+test('enter in the ICMP type field adds a protocol filter', async ({ page }) => {
+  await page.goto('/projects/mock-project/vpcs/default/firewall-rules-new')
+
+  await page.getByRole('button', { name: 'Protocol filters' }).click()
+  await page.getByRole('option', { name: 'ICMPv4', exact: true }).click()
+
+  const v4Type = page.getByRole('combobox', { name: 'ICMPv4 type' })
+  await fillAndSelectComboboxOption(v4Type, page, '8', '8 - Echo Request')
+  await v4Type.press('Enter')
+
+  await expectRowVisible(page.getByRole('table', { name: 'Protocol filters' }), {
+    Protocol: 'ICMPv4',
+    Type: '8',
+  })
+
+  // the form itself should not have been submitted
+  await expect(page.getByRole('dialog', { name: 'Add firewall rule' })).toBeVisible()
+})