Skip to content

Conversation

@charliepark
Copy link
Contributor

This fixes a regression introduced in earlier combobox work, where hitting the Enter key in a subform (like the Firewall Rules create form) would try to submit the entire form, rather than just work within the smaller subform.

The current UX in this PR uses a ref and — upon the user hitting enter — moves the focus to the "add" button, which the user can use to add that selection to the subform. I played with having enter just submit the subform, but it felt a little surprising — a little "fast" — to have enter both select the item from the dropdown and add it to the subform in one fell swoop.

In adding the ref, I saw that it would be more efficient to move the relevant MiniTable.ClearAndAddButtons into the DynamicTypeAndValueFields (now re-titled as TargetAndHostFilterSubform), so I moved a bunch of duplicated code for targets and host filters into that subform.

Closes #2534

@vercel
Copy link

vercel bot commented Nov 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Nov 12, 2024 9:05pm

@david-crespo
Copy link
Collaborator

Nice refactor.

it felt a little surprising — a little "fast" — to have enter both select the item from the dropdown and add it to the subform in one fell swoop.

I agree with this: it shouldn't select and submit in one keypress, but I think we can accomplish that without focusing the add button. What I pictured is: when the combobox options are open, enter selects the highlighted one without submitting the subform. At this point, focus is still in the input, but with the options popover closed. Pressing enter a second time will submit the subform.

Might be kind of annoying to implement, though — you have to figure out how to handle enter differently depending on whether the options are open or not.

} = usePrefetchedApiQuery('vpcSubnetList', { query: { project, vpc } })

const subform = useForm({ defaultValues: targetAndHostDefaultValues })
const field = useController({ name: `${sectionType}s`, control }).field
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely nuts that TS can do this.

@charliepark
Copy link
Contributor Author

I recommend hiding whitespace changes if looking at the code after the last commit: https://github.com/oxidecomputer/console/pull/2540/files?diff=split&w=1

Copy link
Collaborator

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo!

@david-crespo david-crespo enabled auto-merge (squash) November 12, 2024 21:16
@david-crespo david-crespo merged commit e239a8e into main Nov 12, 2024
8 checks passed
@david-crespo david-crespo deleted the enter-in-combobox branch November 12, 2024 21:20
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Nov 14, 2024
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Nov 14, 2024
oxidecomputer/console@9ef82ba...48fc079

* [48fc079a](oxidecomputer/console@48fc079a)
oxidecomputer/console#2542
* [f3d38103](oxidecomputer/console@f3d38103)
oxidecomputer/console#2545
* [eba6626d](oxidecomputer/console@eba6626d)
bump API for moved system timeseries endpoints
* [6052fdbf](oxidecomputer/console@6052fdbf)
oxidecomputer/console#2549
* [b886e4b8](oxidecomputer/console@b886e4b8)
oxidecomputer/console#2550
* [7303d9d7](oxidecomputer/console@7303d9d7)
oxidecomputer/console#2551
* [e239a8e2](oxidecomputer/console@e239a8e2)
oxidecomputer/console#2540
* [31520a2e](oxidecomputer/console@31520a2e)
move Breadcrumbs component definition into TopBar.tsx
* [ad02edec](oxidecomputer/console@ad02edec)
minor: extract UserMenu component for readability

Co-authored-by: iliana etaoin <iliana@oxide.computer>
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.

Enter key in comboboxes in firewall rules should not submit full form

3 participants