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 3 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
74 changes: 65 additions & 9 deletions app/forms/firewall-rules-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,32 @@ import {
useApiQueryClient,
usePrefetchedApiQuery,
type ApiError,
type Instance,
type Vpc,
type VpcFirewallRule,
type VpcFirewallRuleHostFilter,
type VpcFirewallRuleTarget,
type VpcFirewallRuleUpdate,
} from '@oxide/api'

import { CheckboxField } from '~/components/form/fields/CheckboxField'
import { ComboboxField } from '~/components/form/fields/ComboboxField'
import { DescriptionField } from '~/components/form/fields/DescriptionField'
import { ListboxField } from '~/components/form/fields/ListboxField'
import { NameField } from '~/components/form/fields/NameField'
import { NumberField } from '~/components/form/fields/NumberField'
import { RadioField } from '~/components/form/fields/RadioField'
import { TextField, TextFieldInner } from '~/components/form/fields/TextField'
import { SideModalForm } from '~/components/form/SideModalForm'
import { getVpcSelector, useForm, useVpcSelector } from '~/hooks'
import {
getProjectSelector,
getVpcSelector,
useForm,
useProjectSelector,
useVpcSelector,
} from '~/hooks'
import { addToast } from '~/stores/toast'
import { PAGE_SIZE } from '~/table/QueryTable'
import { Badge } from '~/ui/lib/Badge'
import { Button } from '~/ui/lib/Button'
import { FormDivider } from '~/ui/lib/Divider'
Expand Down Expand Up @@ -123,6 +133,8 @@ const targetDefaultValues: TargetFormValues = {
type CommonFieldsProps = {
error: ApiError | null
control: Control<FirewallRuleValues>
instances: Array<Instance>
vpcs: Array<Vpc>
}

function getFilterValueProps(hostType: VpcFirewallRuleHostFilter['type']) {
Expand Down Expand Up @@ -174,7 +186,7 @@ const DocsLinkMessage = () => (
/>
)

export const CommonFields = ({ error, control }: CommonFieldsProps) => {
export const CommonFields = ({ error, control, instances, vpcs }: CommonFieldsProps) => {
const portRangeForm = useForm({ defaultValues: portRangeDefaultValues })
const ports = useController({ name: 'ports', control }).field
const submitPortRange = portRangeForm.handleSubmit(({ portRange }) => {
Expand Down Expand Up @@ -210,6 +222,22 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => {
targetForm.reset()
})

const hostFilterItems = {
vpc: vpcs.map((v) => ({ value: v.name, label: v.name })),
// todo: this should be a list of subnets in the selected VPC,
// so we'll need to have the user first specify a VPC and then load these in
subnet: [
{ value: 'subnet-1', label: 'subnet-one' },
{ value: 'subnet-2', label: 'subnet-two' },
{ value: 'subnet-3', label: 'subnet-three' },
{ value: 'subnet-4', label: 'subnet-four' },
{ value: 'subnet-5', label: 'subnet-five' },
],
instance: instances.map((i) => ({ value: i.name, label: i.name })),
ip: [],
ip_net: [],
}

return (
<>
<DocsLinkMessage />
Expand Down Expand Up @@ -482,7 +510,7 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => {
So we should probably have the label on this field change when the
host type changes. Also need to confirm that it's just an IP and
not a block. */}
<TextField
<ComboboxField
name="value"
{...getFilterValueProps(hostForm.watch('type'))}
required
Expand All @@ -493,6 +521,11 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => {
submitHost(e)
}
}}
onInputChange={(value) => {
hostForm.setValue('value', value)
}}
items={hostFilterItems[hostForm.watch('type')]}
showNoMatchPlaceholder={false}
// TODO: validate here, but it's complicated because it's conditional
// on which type is selected
/>
Expand Down Expand Up @@ -560,13 +593,22 @@ export const CommonFields = ({ error, control }: CommonFieldsProps) => {
}

CreateFirewallRuleForm.loader = async ({ params }: LoaderFunctionArgs) => {
await apiQueryClient.prefetchQuery('vpcFirewallRulesView', {
query: getVpcSelector(params),
})
const { project } = getProjectSelector(params)
await Promise.all([
apiQueryClient.prefetchQuery('vpcFirewallRulesView', {
query: getVpcSelector(params),
}),
apiQueryClient.prefetchQuery('instanceList', {
query: { project, limit: PAGE_SIZE },
}),
apiQueryClient.prefetchQuery('vpcList', { query: { project, limit: PAGE_SIZE } }),
])

return null
}

export function CreateFirewallRuleForm() {
const { project } = useProjectSelector()
const vpcSelector = useVpcSelector()
const queryClient = useApiQueryClient()

Expand All @@ -581,10 +623,19 @@ export function CreateFirewallRuleForm() {
},
})

const { data } = usePrefetchedApiQuery('vpcFirewallRulesView', {
const { data: vpcFirewallRules } = usePrefetchedApiQuery('vpcFirewallRulesView', {
query: vpcSelector,
})
const existingRules = useMemo(() => R.sortBy(data.rules, (r) => r.priority), [data])
const existingRules = useMemo(
() => R.sortBy(vpcFirewallRules.rules, (r) => r.priority),
[vpcFirewallRules]
)
const { data: instances } = usePrefetchedApiQuery('instanceList', {
query: { project, limit: PAGE_SIZE },
})
const { data: vpcs } = usePrefetchedApiQuery('vpcList', {
query: { project, limit: PAGE_SIZE },
})
charliepark marked this conversation as resolved.
Show resolved Hide resolved

const form = useForm({ defaultValues })

Expand Down Expand Up @@ -612,7 +663,12 @@ export function CreateFirewallRuleForm() {
submitError={updateRules.error}
submitLabel="Add rule"
>
<CommonFields error={updateRules.error} control={form.control} />
<CommonFields
error={updateRules.error}
control={form.control}
instances={instances.items}
vpcs={vpcs.items}
/>
</SideModalForm>
)
}
105 changes: 61 additions & 44 deletions app/ui/lib/Combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,17 @@ export type ComboboxBaseProps = {
description?: React.ReactNode
isDisabled?: boolean
isLoading?: boolean
items: ComboboxItem[]
// without items, the combobox acts as a regular text input
items?: ComboboxItem[]
charliepark marked this conversation as resolved.
Show resolved Hide resolved
label: string
placeholder?: string
required?: boolean
tooltipText?: string
onInputChange?: (value: string) => void
charliepark marked this conversation as resolved.
Show resolved Hide resolved
onKeyDown?: (event: React.KeyboardEvent<HTMLInputElement>) => void
// set to false in situations where the user should be able to type in new values
// to decide: should we even give a placeholder / warning?
showNoMatchPlaceholder?: boolean
}

type ComboboxProps = {
Expand All @@ -46,7 +52,7 @@ type ComboboxProps = {

export const Combobox = ({
description,
items,
items = [],
selected,
label,
placeholder,
Expand All @@ -56,6 +62,9 @@ export const Combobox = ({
isDisabled,
isLoading,
onChange,
onInputChange,
onKeyDown,
showNoMatchPlaceholder = true,
}: ComboboxProps) => {
const [query, setQuery] = useState(selected || '')

Expand Down Expand Up @@ -102,7 +111,11 @@ export const Combobox = ({
<ComboboxInput
aria-label="Select a disk"
charliepark marked this conversation as resolved.
Show resolved Hide resolved
displayValue={() => (selected ? selected : query)}
onChange={(event) => setQuery(event.target.value)}
onChange={(event) => {
setQuery(event.target.value)
onInputChange?.(event.target.value)
}}
onKeyDown={onKeyDown}
placeholder={placeholder}
disabled={isDisabled || isLoading}
className={cn(
Expand All @@ -113,48 +126,52 @@ export const Combobox = ({
hasError && 'focus-error'
)}
/>
<div className="flex items-center border-l px-3 border-secondary" aria-hidden>
<SelectArrows6Icon title="Select" className="w-2 text-tertiary" />
</div>
</ComboboxButton>
<ComboboxOptions
anchor="bottom start"
// 14px gap is presumably because it's measured from inside the outline or something
className={`ox-menu pointer-events-auto ${zIndex} relative w-[var(--button-width)] overflow-y-auto border !outline-none border-secondary [--anchor-gap:14px] empty:hidden`}
modal={false}
>
{filteredItems.length === 0 && (
<ComboboxOption disabled value="no-matches" className="relative">
<div className="ox-menu-item !text-disabled">No items match</div>
</ComboboxOption>
{items.length > 0 && (
<div className="flex items-center border-l px-3 border-secondary" aria-hidden>
<SelectArrows6Icon title="Select" className="w-2 text-tertiary" />
</div>
)}
{filteredItems.map((item) => (
<ComboboxOption
key={item.label}
value={item.label}
className="relative border-b border-secondary last:border-0"
onSelect={() => {
onChange(item.label)
setQuery(item.label)
}}
>
{({ focus, selected }) => (
// This *could* be done with data-[focus] and data-[selected] instead, but
// it would be a lot more verbose. those can only be used with TW classes,
// not our .is-selected and .is-highlighted, so we'd have to copy the pieces
// of those rules one by one. Better to rely on the shared classes.
<div
className={cn('ox-menu-item', {
'is-selected': selected,
'is-highlighted': focus,
})}
>
{item.label}
</div>
)}
</ComboboxOption>
))}
</ComboboxOptions>
</ComboboxButton>
{items.length > 0 && (
<ComboboxOptions
anchor="bottom start"
// 14px gap is presumably because it's measured from inside the outline or something
className={`ox-menu pointer-events-auto ${zIndex} relative w-[var(--button-width)] overflow-y-auto border !outline-none border-secondary [--anchor-gap:14px] empty:hidden`}
modal={false}
>
{showNoMatchPlaceholder && filteredItems.length === 0 && (
<ComboboxOption disabled value="no-matches" className="relative">
<div className="ox-menu-item !text-disabled">No items match</div>
</ComboboxOption>
)}
{filteredItems.map((item) => (
<ComboboxOption
key={item.label}
value={item.label}
className="relative border-b border-secondary last:border-0"
onSelect={() => {
onChange(item.label)
setQuery(item.label)
}}
>
{({ focus, selected }) => (
// This *could* be done with data-[focus] and data-[selected] instead, but
// it would be a lot more verbose. those can only be used with TW classes,
// not our .is-selected and .is-highlighted, so we'd have to copy the pieces
// of those rules one by one. Better to rely on the shared classes.
<div
className={cn('ox-menu-item', {
'is-selected': selected,
'is-highlighted': focus,
})}
>
{item.label}
</div>
)}
</ComboboxOption>
))}
</ComboboxOptions>
)}
</HCombobox>
</>
)
Expand Down
Loading