Skip to content
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

Merged
merged 52 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
54f3ae1
Add means to type new entries into combobox
charliepark Jun 28, 2024
b65263d
make items prop optional; without items, combobox acts as regular input
charliepark Jun 28, 2024
a8c9284
add data fetching for instances and VPCs
charliepark Jun 28, 2024
35b46bf
Update with working combobox and dynamic data loading; still a few ro…
charliepark Jun 28, 2024
fcd2d73
items should be required
charliepark Jun 28, 2024
19fd538
Merge branch 'main' into expand-combobox-flexibility
charliepark Jul 1, 2024
7a579c0
Tweak onKeyDown, as enter key shouldn't get overridden with comboboxe…
charliepark Jul 1, 2024
a5d0ecd
test refactor
charliepark Jul 1, 2024
6cd8888
Merge branch 'main' into expand-combobox-flexibility
charliepark Jul 2, 2024
39157ec
Merge branch 'main' into expand-combobox-flexibility
charliepark Jul 15, 2024
cec67a6
Make form clearable when field is empty
charliepark Jul 15, 2024
c651791
test updates
charliepark Jul 16, 2024
a660300
Merge main and hopefully don't bork firewall-rules-create and firewal…
charliepark Aug 15, 2024
7e0174e
Add more controls on Target section of firewall rules create form
charliepark Aug 16, 2024
1137eb5
update tests
charliepark Aug 19, 2024
5ab814c
Merge main and hopefully resolve issues
charliepark Aug 22, 2024
98d707a
Update firewall form a bit more
charliepark Aug 22, 2024
ce8351c
Refactoring / deduping code
charliepark Aug 22, 2024
dd2ccb0
e2e utils
charliepark Aug 22, 2024
cac1ac3
Remove duplicate section
charliepark Aug 22, 2024
deb50d1
Merge branch 'main' into expand-combobox-flexibility
charliepark Aug 23, 2024
8202ebf
Refactor duplicated code
charliepark Aug 23, 2024
1fe7c1c
More cleanup
charliepark Aug 23, 2024
307125d
Merge branch 'main' into expand-combobox-flexibility
charliepark Aug 23, 2024
e8af303
More cleanup
charliepark Aug 23, 2024
757695a
A bit more sorting
charliepark Aug 23, 2024
da547f7
Last cleanup before tests, hopefully
charliepark Aug 23, 2024
6808ba5
jk optimized the mini table
charliepark Aug 24, 2024
c93bbca
Update aria-label for Combobox
charliepark Aug 24, 2024
91a7b9e
Update tests
charliepark Aug 26, 2024
8435343
Cleanup
charliepark Aug 26, 2024
ef44da2
showNoMatchPlaceholder -> allowNewItems and change default
charliepark Aug 27, 2024
934b9c7
Merge branch 'main' into expand-combobox-flexibility
charliepark Aug 29, 2024
83e419b
Add test for 'no items match'
charliepark Aug 29, 2024
b00a78a
Refactoring post-review
charliepark Sep 4, 2024
089a056
Add prop to selectively hide 'optional' tag on FieldLabel
charliepark Sep 4, 2024
268a997
A few more spots for showOptionalTag
charliepark Sep 4, 2024
045c58a
Updated placeholder/description text
charliepark Sep 4, 2024
07ccefe
Refactoring
charliepark Sep 4, 2024
9a76cc6
more cleanup; not totally sold on microcopy on firewall rule combobox
charliepark Sep 4, 2024
c29cdf1
Revert placeholder in two spots
charliepark Sep 4, 2024
8d869ff
fix capitalization on test
charliepark Sep 5, 2024
bf7b3c5
Default to current VPC in VPC subnet form
charliepark Sep 5, 2024
107371e
Fix tests
charliepark Sep 5, 2024
cb0bdde
Merge branch 'main' into expand-combobox-flexibility
charliepark Sep 5, 2024
e48a3af
Remove VPC selection step from VPC Subnet targeting; refactor and add…
charliepark Sep 7, 2024
5e515cb
Update logic around show/hideOptionalTag
charliepark Sep 7, 2024
85178ad
More precise logic to get better TS error messages
charliepark Sep 7, 2024
b64a257
remove obsolete default
charliepark Sep 7, 2024
c613f23
Remove obviated prop
charliepark Sep 9, 2024
9e400d5
Merge branch 'main' into expand-combobox-flexibility
charliepark Sep 9, 2024
f5f2964
Update limit values to 1000
charliepark Sep 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions app/components/form/fields/ComboboxField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export type ComboboxFieldProps<
name: TName
control: Control<TFieldValues>
onChange?: (value: string | null | undefined) => void
disabled?: boolean
} & ComboboxBaseProps

export function ComboboxField<
Expand All @@ -38,22 +37,37 @@ export function ComboboxField<
label = capitalize(name),
required,
onChange,
disabled,
allowArbitraryValues,
placeholder,
// Intent is to not show both a placeholder and a description, while still having good defaults; prefer a description to a placeholder
/*
* If description is provided, use it.
* If not, but a placeholder is provided, the default description should be undefined.
* If no placeholder is provided and arbitrary values are allowed, the default description should be 'Select an option or enter a custom value'.
* If no placeholder is provided and arbitrary values are not allowed, the default description should be 'Select an option'.
*/
description = placeholder
? undefined
: allowArbitraryValues
? 'Select an option or enter a custom value'
: 'Select an option',
...props
}: ComboboxFieldProps<TFieldValues, TName>) {
const { field, fieldState } = useController({ name, control, rules: { required } })
return (
<div className="max-w-lg">
<Combobox
isDisabled={disabled}
Copy link
Collaborator

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.

Copy link
Collaborator

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.

045c58a

Copy link
Contributor Author

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.

label={label}
placeholder={placeholder}
description={description}
required={required}
selected={field.value || null}
hasError={fieldState.error !== undefined}
onChange={(value) => {
field.onChange(value)
onChange?.(value)
}}
allowArbitraryValues={allowArbitraryValues}
{...props}
/>
<ErrorMessage error={fieldState.error} label={label} />
Expand Down
3 changes: 3 additions & 0 deletions app/components/form/fields/ListboxField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type ListboxFieldProps<
onChange?: (value: string | null | undefined) => void
isLoading?: boolean
noItemsPlaceholder?: string
hideOptionalTag?: boolean
}

export function ListboxField<
Expand All @@ -54,6 +55,7 @@ export function ListboxField<
onChange,
isLoading,
noItemsPlaceholder,
hideOptionalTag,
}: ListboxFieldProps<TFieldValues, TName>) {
// TODO: recreate this logic
// validate: (v) => (required && !v ? `${name} is required` : undefined),
Expand All @@ -80,6 +82,7 @@ export function ListboxField<
hasError={fieldState.error !== undefined}
isLoading={isLoading}
buttonRef={field.ref}
hideOptionalTag={hideOptionalTag}
/>
<ErrorMessage error={fieldState.error} label={label} />
</div>
Expand Down
Loading
Loading