Skip to content

Don't preventDefault on Enter in Combobox if unhandled - #3417

Closed
fakemonster wants to merge 1 commit into
mainfrom
fix-combobox-enter-defaulting
Closed

fakemonster wants to merge 1 commit into
mainfrom
fix-combobox-enter-defaulting

Conversation

@fakemonster

Copy link
Copy Markdown
Contributor

Previously, by unconditionally preventing default on Enter presses in the Combobox, it was impossible to submit a form by hitting enter while focused in a (closed) Combobox. A user would reasonably expect that pressing Enter while it was closed, much like a regular text field, would submit the form they were in.

In practice this means a keyboard user is going to navigate to the combobox, find their selection, and hit enter twice: once to close the selection and again to submit the form.

Previously, by unconditionally preventing default on Enter presses in
the Combobox, it was impossible to submit a form by hitting enter while
focused in a (closed) Combobox. A user would reasonably expect that
pressing Enter while it was closed, much like a regular text field,
would submit the form they were in.

In practice this means a keyboard user is going to navigate to the
combobox, find their selection, and hit enter twice: once to close the
selection and again to submit the form.
@vercel

vercel Bot commented Oct 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
console Ready Ready Preview Oct 20, 2026 12:19am UTC

Request Review

@fakemonster

Copy link
Copy Markdown
Contributor Author

@charliepark this is what i was getting at in response to your comment here; propagating enter presses from the combobox is a more general change (if fairly simple)

david-crespo added a commit that referenced this pull request Oct 30, 2026
…3422)

The diff is unreadable unless you [hide whitespace
changes](https://github.com/oxidecomputer/console/pull/3422/changes?w=1)
because it adds a wrapper to existing code.

This is an alternative to #3417 that deletes the sad logic inside
Combobox instead of fixing it. We get rid of `onEnter`, and instead of
having to keep track of whether the list is open with `isOpenRef` and
decide when to `preventDefault`, Combobox never calls `preventDefault`
(unless it's done for it by Headless UI when selecting an option).

But in subforms we still need to capture those enter keypresses before
they hit the parent form, so we wrap subforms in a `Subform` component
with an `onSubmit` prop. The advantage is we no longer have to remember
to do this for every field we add to a subform (and we have messed this
up before).

This approach fixes the same problem cases as #3417, all of which are
noops on `main`. Enter in a closed combobox now:

- Adds the protocol filter from the ICMP type combobox, like Enter in
the ICMP code field next to it
- Applies the alert class filter (#3393)
- Submits standalone forms from their pickers, like link silo, link
pool, attach disk, create snapshot, and instance create's boot disk and
image pickers

`Subform.browser.spec.tsx` tests Enter in a combobox, text input, and
button inside a subform, and in a combobox outside a subform. In the e2e
tests, Enter adds a firewall rule target, port and ICMP type, a webhook
secret, and a transit IP. Enter in a closed combobox applies the alert
class filter, attaches a disk, and creates an instance from the image
picker (these tests fail on main).
@david-crespo

Copy link
Copy Markdown
Collaborator

Superseded by #3422

This branch was successfully deployed

1 active deployment
Preview — ff9276fb Deployed Oct 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants