-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expand combobox flexibility #2296
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…s the way it is for normal inputs
This is coming along, but man it feels like there's a lot of code duplication in this main file. Also, we'll eventually need to do something similar re: comboboxes in the router routes forms, and possibly other places. Would love to come up with an extracted component that we could just drop in place. |
@@ -83,7 +85,13 @@ export const Listbox = <Value extends string = string>({ | |||
<> | |||
{label && ( | |||
<div className="mb-2"> | |||
<FieldLabel id={``} as="div" tip={tooltipText} optional={!required}> | |||
<FieldLabel | |||
id={``} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't introduce this here, so we can fix it later: #2417
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LET'S JAM
...props | ||
}: ComboboxFieldProps<TFieldValues, TName>) { | ||
const { field, fieldState } = useController({ name, control, rules: { required } }) | ||
return ( | ||
<div className="max-w-lg"> | ||
<Combobox | ||
isDisabled={disabled} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this right? Seems like the mismatch between disabled
and isDisabled
may require this to be passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I think I see what you did: you changed the prop on ComboboxField
to isDisabled
and let it get passed through as part of the props spread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I realized that there was an existing isDisabled
prop and it made sense to just use that.
oxidecomputer/console@7712765...ce242e7 * [ce242e77](oxidecomputer/console@ce242e77) bump API to latest, only doc comment changes * [3fda42e3](oxidecomputer/console@3fda42e3) oxidecomputer/console#2446 * [a5c23616](oxidecomputer/console@a5c23616) oxidecomputer/console#2436 * [ca272336](oxidecomputer/console@ca272336) oxidecomputer/console#2445 * [22582e3c](oxidecomputer/console@22582e3c) oxidecomputer/console#2444 * [f9f0d157](oxidecomputer/console@f9f0d157) oxidecomputer/console#2443 * [be84c196](oxidecomputer/console@be84c196) oxidecomputer/console#2441 * [53e1da78](oxidecomputer/console@53e1da78) chore: just skip the flaky test in safari * [3a66ca77](oxidecomputer/console@3a66ca77) bump a couple of dev deps I missed * [ac67a4cf](oxidecomputer/console@ac67a4cf) bump omicron version to latest * [63c604b4](oxidecomputer/console@63c604b4) oxidecomputer/console#2440 * [b4e2626b](oxidecomputer/console@b4e2626b) oxidecomputer/console#2439 * [cdecf4e6](oxidecomputer/console@cdecf4e6) oxidecomputer/console#2435 * [01d71c07](oxidecomputer/console@01d71c07) oxidecomputer/console#2432 * [353b98d6](oxidecomputer/console@353b98d6) oxidecomputer/console#2404 * [49d6d7d8](oxidecomputer/console@49d6d7d8) oxidecomputer/console#2433 * [9c532ce9](oxidecomputer/console@9c532ce9) chore: fix react duplicate key warning on vpcs page * [1892fc14](oxidecomputer/console@1892fc14) npm audit fix, bump msw * [efc43be0](oxidecomputer/console@efc43be0) oxidecomputer/console#2422 * [f2cc79ee](oxidecomputer/console@f2cc79ee) oxidecomputer/console#2418 * [a0c29a53](oxidecomputer/console@a0c29a53) oxidecomputer/console#2427 * [a8fcdab9](oxidecomputer/console@a8fcdab9) oxidecomputer/console#2426 * [8ecb36ad](oxidecomputer/console@8ecb36ad) oxidecomputer/console#2425 * [93bcef9d](oxidecomputer/console@93bcef9d) oxidecomputer/console#2296 * [af42e704](oxidecomputer/console@af42e704) oxidecomputer/console#2421 * [4126f000](oxidecomputer/console@4126f000) oxidecomputer/console#2420 * [880cb8c4](oxidecomputer/console@880cb8c4) oxidecomputer/console#2415 * [c2909e7a](oxidecomputer/console@c2909e7a) oxidecomputer/console#2414 * [12fc862b](oxidecomputer/console@12fc862b) oxidecomputer/console#2412 * [169edeed](oxidecomputer/console@169edeed) remove unreachable sign in button in user dropdown * [f042ad4b](oxidecomputer/console@f042ad4b) oxidecomputer/console#2409 * [c648c44c](oxidecomputer/console@c648c44c) oxidecomputer/console#2408 * [e8175d30](oxidecomputer/console@e8175d30) oxidecomputer/console#2405 * [bdb55b86](oxidecomputer/console@bdb55b86) chore: bump ts client generator version in api-diff * [681355cd](oxidecomputer/console@681355cd) oxidecomputer/console#2396 * [286b8d28](oxidecomputer/console@286b8d28) oxidecomputer/console#2401 * [b2373359](oxidecomputer/console@b2373359) oxidecomputer/console#2400 * [267b9359](oxidecomputer/console@267b9359) oxidecomputer/console#2399
oxidecomputer/console@7712765...ce242e7 * [ce242e77](oxidecomputer/console@ce242e77) bump API to latest, only doc comment changes * [3fda42e3](oxidecomputer/console@3fda42e3) oxidecomputer/console#2446 * [a5c23616](oxidecomputer/console@a5c23616) oxidecomputer/console#2436 * [ca272336](oxidecomputer/console@ca272336) oxidecomputer/console#2445 * [22582e3c](oxidecomputer/console@22582e3c) oxidecomputer/console#2444 * [f9f0d157](oxidecomputer/console@f9f0d157) oxidecomputer/console#2443 * [be84c196](oxidecomputer/console@be84c196) oxidecomputer/console#2441 * [53e1da78](oxidecomputer/console@53e1da78) chore: just skip the flaky test in safari * [3a66ca77](oxidecomputer/console@3a66ca77) bump a couple of dev deps I missed * [ac67a4cf](oxidecomputer/console@ac67a4cf) bump omicron version to latest * [63c604b4](oxidecomputer/console@63c604b4) oxidecomputer/console#2440 * [b4e2626b](oxidecomputer/console@b4e2626b) oxidecomputer/console#2439 * [cdecf4e6](oxidecomputer/console@cdecf4e6) oxidecomputer/console#2435 * [01d71c07](oxidecomputer/console@01d71c07) oxidecomputer/console#2432 * [353b98d6](oxidecomputer/console@353b98d6) oxidecomputer/console#2404 * [49d6d7d8](oxidecomputer/console@49d6d7d8) oxidecomputer/console#2433 * [9c532ce9](oxidecomputer/console@9c532ce9) chore: fix react duplicate key warning on vpcs page * [1892fc14](oxidecomputer/console@1892fc14) npm audit fix, bump msw * [efc43be0](oxidecomputer/console@efc43be0) oxidecomputer/console#2422 * [f2cc79ee](oxidecomputer/console@f2cc79ee) oxidecomputer/console#2418 * [a0c29a53](oxidecomputer/console@a0c29a53) oxidecomputer/console#2427 * [a8fcdab9](oxidecomputer/console@a8fcdab9) oxidecomputer/console#2426 * [8ecb36ad](oxidecomputer/console@8ecb36ad) oxidecomputer/console#2425 * [93bcef9d](oxidecomputer/console@93bcef9d) oxidecomputer/console#2296 * [af42e704](oxidecomputer/console@af42e704) oxidecomputer/console#2421 * [4126f000](oxidecomputer/console@4126f000) oxidecomputer/console#2420 * [880cb8c4](oxidecomputer/console@880cb8c4) oxidecomputer/console#2415 * [c2909e7a](oxidecomputer/console@c2909e7a) oxidecomputer/console#2414 * [12fc862b](oxidecomputer/console@12fc862b) oxidecomputer/console#2412 * [169edeed](oxidecomputer/console@169edeed) remove unreachable sign in button in user dropdown * [f042ad4b](oxidecomputer/console@f042ad4b) oxidecomputer/console#2409 * [c648c44c](oxidecomputer/console@c648c44c) oxidecomputer/console#2408 * [e8175d30](oxidecomputer/console@e8175d30) oxidecomputer/console#2405 * [bdb55b86](oxidecomputer/console@bdb55b86) chore: bump ts client generator version in api-diff * [681355cd](oxidecomputer/console@681355cd) oxidecomputer/console#2396 * [286b8d28](oxidecomputer/console@286b8d28) oxidecomputer/console#2401 * [b2373359](oxidecomputer/console@b2373359) oxidecomputer/console#2400 * [267b9359](oxidecomputer/console@267b9359) oxidecomputer/console#2399
Fixes #2065
This is an evolution of the combobox.
Currently, a combobox provides a dropdown list of items with a text input that allows the user to filter the items in the list.
If the user enters in a non-matching string, they just get the message that there isn't a match.
There are times, though, where the user needs to be able to enter in arbitrary text into the combobox, with the intent being "if this matches item(s) in the list, show me the matching item(s), but if not, give me the option to create a new item. We see this, for example, in the VPC Firewall Rules form, where the user might be selecting from a set of existing VPCs (or subnets, or instances), or might need to create a reference to a not-yet-existing VPC (etc.).
Here, for example, we see that
mock-
matches an option …… but when we extend the search string a bit and it doesn't match any options, we see that it doesn't show the "no items match" text, and instead acts like a normal text input box …
… and the user can then add that new VPC to the form:
This behavioral change (enable addition of new option) is handled by the
allowArbitraryValues
prop.This change in Combobox behavior is then applied in this PR to the firewall rules form. In a previous PR we pulled the common code from the firewall-rules-create and firewall-rules-edit forms into a new file, firewall-rules-common. This PR updates that code, moving from the existing text fields (where the user would have to know the instance name, or VPC name, or VPC subnet name), so they can select from a dropdown. This usecase is also where we see the need for the user to add a new item that isn't currently in the list of dropdown items (via that prop mentioned above,
allowArbitraryValues
).