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

Allow ReactElements to be used in Combobox dropdowns #2474

Merged
merged 36 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
e3b53ec
Enable ReactElements in Combobox dropdowns; some type coercion still …
charliepark Sep 24, 2024
681dfe4
Oh, vim mode
charliepark Sep 25, 2024
901a1ff
Refactor types and filtering
charliepark Sep 25, 2024
8a99093
revert unneeded change
charliepark Sep 25, 2024
bb71d2f
More precise placeholder text for image select
charliepark Sep 25, 2024
e4eebb6
tests are still doing a strange scrolling thing, but are almost there
charliepark Sep 26, 2024
14ce76b
Merge branch 'main' into allow-react-elements-in-combobox-dropdowns
charliepark Sep 26, 2024
23b8a8e
don't be clever with strings
charliepark Sep 26, 2024
9d7ac4d
Update test to filter combobox
charliepark Sep 26, 2024
3a892e0
Move getSelectedLabelFromValue to onChange event, rather than in a us…
charliepark Sep 26, 2024
d1e0bb8
Refactor toComboboxItems
charliepark Sep 26, 2024
4362dc8
More general type for toComboboxItem; fix test
charliepark Sep 26, 2024
b18a39c
Wrap a few toComboboxItems functions in useMemo hooks
charliepark Sep 27, 2024
08017be
Fix issue with non-clearing Combobox input onSubmit
charliepark Sep 27, 2024
885e6b3
Fix actually had to do with showing Value vs. Label
charliepark Sep 27, 2024
b4b385e
better const name
charliepark Sep 30, 2024
9b99506
Merge branch 'main' into allow-react-elements-in-combobox-dropdowns
charliepark Sep 30, 2024
d93817b
merge main and resolve conflicts
charliepark Oct 1, 2024
0510ddc
Use simpler name-only version of selected image in combobox dropdown
charliepark Oct 1, 2024
3154cc5
Address ComboboxInput vs. ComboboxButton strangeness
charliepark Oct 2, 2024
a1e6946
the gang does unholy things with CSS calc and Tailwind
charliepark Oct 2, 2024
783d588
A few util adjustments to match new combobox structure
charliepark Oct 2, 2024
deac415
More adjustments, around field labels
charliepark Oct 2, 2024
a9ecdb0
A few more errant tests
charliepark Oct 2, 2024
ebf81b2
update test helper
charliepark Oct 2, 2024
f320faf
Update approach to labels in Combobox to remove code smell from Playw…
charliepark Oct 2, 2024
6be65a0
nth(1) was a code smell anyway
charliepark Oct 2, 2024
749726b
Merge branch 'main' into allow-react-elements-in-combobox-dropdowns
charliepark Oct 3, 2024
a48b804
Update test
charliepark Oct 3, 2024
96d3159
refactor pass
charliepark Oct 3, 2024
f4e9dd3
Better filtering in combobox
charliepark Oct 3, 2024
f6db060
early-return some image calculations when the boot disk image selecto…
charliepark Oct 3, 2024
cc59d82
let RHF focus combobox field on error
david-crespo Oct 3, 2024
d610bc3
Update CSS to better handle error state for combobox
charliepark Oct 3, 2024
431a949
Merge branch 'main' into allow-react-elements-in-combobox-dropdowns
charliepark Oct 3, 2024
73a85de
Don't open dropdown automatically when there's an error
charliepark Oct 3, 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
17 changes: 15 additions & 2 deletions app/components/form/fields/ComboboxField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Copyright Oxide Computer Company
*/

import { useState } from 'react'
import {
useController,
type Control,
Expand All @@ -15,7 +16,11 @@ import {
type Validate,
} from 'react-hook-form'

import { Combobox, type ComboboxBaseProps } from '~/ui/lib/Combobox'
import {
Combobox,
getSelectedLabelFromValue,
type ComboboxBaseProps,
} from '~/ui/lib/Combobox'
import { capitalize } from '~/util/str'

import { ErrorMessage } from './ErrorMessage'
Expand Down Expand Up @@ -54,6 +59,7 @@ export function ComboboxField<
: allowArbitraryValues
? 'Select an option or enter a custom value'
: 'Select an option',
items,
validate,
...props
}: ComboboxFieldProps<TFieldValues, TName>) {
Expand All @@ -62,20 +68,27 @@ export function ComboboxField<
control,
rules: { required, validate },
})
const [selectedItemLabel, setSelectedItemLabel] = useState(
getSelectedLabelFromValue(items, field.value || '')
)
return (
<div className="max-w-lg">
<Combobox
label={label}
placeholder={placeholder}
description={description}
items={items}
required={required}
selected={field.value || null}
selectedItemValue={field.value}
selectedItemLabel={selectedItemLabel}
hasError={fieldState.error !== undefined}
onChange={(value) => {
field.onChange(value)
onChange?.(value)
setSelectedItemLabel(getSelectedLabelFromValue(items, value))
}}
allowArbitraryValues={allowArbitraryValues}
inputRef={field.ref}
{...props}
/>
<ErrorMessage error={fieldState.error} label={label} />
Expand Down
48 changes: 22 additions & 26 deletions app/components/form/fields/ImageSelectField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import { useController, type Control } from 'react-hook-form'
import type { Image } from '@oxide/api'

import type { InstanceCreateInput } from '~/forms/instance-create'
import type { ListboxItem } from '~/ui/lib/Listbox'
import type { ComboboxItem } from '~/ui/lib/Combobox'
import { Slash } from '~/ui/lib/Slash'
import { nearest10 } from '~/util/math'
import { bytesToGiB, GiB } from '~/util/units'

import { ListboxField } from './ListboxField'
import { ComboboxField } from './ComboboxField'

type ImageSelectFieldProps = {
images: Image[]
Expand All @@ -32,18 +32,22 @@ export function BootDiskImageSelectField({
}: ImageSelectFieldProps) {
const diskSizeField = useController({ control, name: 'bootDiskSize' }).field
return (
// This should be migrated to a `ComboboxField` (and with a `toComboboxItem`), once
// we have a combobox that supports more elaborate labels (beyond just strings).
<ListboxField
<ComboboxField
disabled={disabled}
control={control}
name={name}
label="Image"
placeholder="Select an image"
items={images.map((i) => toListboxItem(i))}
placeholder={
name === 'siloImageSource' ? 'Select a silo image' : 'Select a project image'
}
items={images.map((i) => toImageComboboxItem(i))}
required
onChange={(id) => {
const image = images.find((i) => i.id === id)! // if it's selected, it must be present
const image = images.find((i) => i.id === id)
// the most likely scenario where image would be undefined is if the user has
// manually cleared the ComboboxField; they will need to pick a boot disk image
// in order to submit the form, so we don't need to do anything here
if (!image) return
const imageSizeGiB = image.size / GiB
if (diskSizeField.value < imageSizeGiB) {
diskSizeField.onChange(nearest10(imageSizeGiB))
Expand All @@ -53,24 +57,18 @@ export function BootDiskImageSelectField({
)
}

export function toListboxItem(i: Image, includeProjectSiloIndicator = false): ListboxItem {
const { name, os, projectId, size, version } = i
const formattedSize = `${bytesToGiB(size, 1)} GiB`

// filter out any undefined metadata and create a comma-separated list
// for the selected listbox item (shown in selectedLabel)
const condensedImageMetadata = [os, version, formattedSize].filter((i) => !!i).join(', ')
const metadataForSelectedLabel = condensedImageMetadata.length
? ` (${condensedImageMetadata})`
: ''
export function toImageComboboxItem(
image: Image,
includeProjectSiloIndicator = false
): ComboboxItem {
const { id, name, os, projectId, size, version } = image

// for metadata showing in the dropdown's options, include the project / silo indicator if requested
const projectSiloIndicator = includeProjectSiloIndicator
? `${projectId ? 'Project' : 'Silo'} image`
: null
// filter out undefined metadata here, as well, and create a `<Slash />`-separated list
// for the listbox item (shown for each item in the dropdown)
const metadataForLabel = [os, version, formattedSize, projectSiloIndicator]
// filter out undefined metadata and create a `<Slash />`-separated list for each comboboxitem
const itemMetadata = [os, version, `${bytesToGiB(size, 1)} GiB`, projectSiloIndicator]
.filter((i) => !!i)
.map((i, index) => (
<span key={`${i}`}>
Expand All @@ -79,14 +77,12 @@ export function toListboxItem(i: Image, includeProjectSiloIndicator = false): Li
</span>
))
return {
value: i.id,
selectedLabel: `${name}${metadataForSelectedLabel}`,
value: id,
selectedLabel: name,
label: (
<>
<div>{name}</div>
<div className="text-tertiary selected:text-accent-secondary">
{metadataForLabel}
</div>
<div className="text-tertiary selected:text-accent-secondary">{itemMetadata}</div>
</>
),
}
Expand Down
17 changes: 12 additions & 5 deletions app/forms/disk-attach.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
*
* Copyright Oxide Computer Company
*/
import { useMemo } from 'react'
import { useForm } from 'react-hook-form'

import { useApiQuery, type ApiError } from '@oxide/api'

import { ComboboxField } from '~/components/form/fields/ComboboxField'
import { SideModalForm } from '~/components/form/SideModalForm'
import { useProjectSelector } from '~/hooks/use-params'
import { toComboboxItems } from '~/ui/lib/Combobox'
import { ALL_ISH } from '~/util/consts'

const defaultValues = { name: '' }
Expand Down Expand Up @@ -41,10 +43,15 @@ export function AttachDiskSideModalForm({
const { data } = useApiQuery('diskList', {
query: { project, limit: ALL_ISH },
})
const detachedDisks =
data?.items.filter(
(d) => d.state.state === 'detached' && !diskNamesToExclude.includes(d.name)
) || []
const detachedDisks = useMemo(
() =>
toComboboxItems(
data?.items.filter(
(d) => d.state.state === 'detached' && !diskNamesToExclude.includes(d.name)
)
),
[data, diskNamesToExclude]
)

const form = useForm({ defaultValues })

Expand All @@ -63,7 +70,7 @@ export function AttachDiskSideModalForm({
label="Disk name"
placeholder="Select a disk"
name="name"
items={detachedDisks.map(({ name }) => ({ value: name, label: name }))}
items={detachedDisks}
required
control={form.control}
/>
Expand Down
4 changes: 2 additions & 2 deletions app/forms/disk-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {

import { DescriptionField } from '~/components/form/fields/DescriptionField'
import { DiskSizeField } from '~/components/form/fields/DiskSizeField'
import { toListboxItem } from '~/components/form/fields/ImageSelectField'
import { toImageComboboxItem } from '~/components/form/fields/ImageSelectField'
import { ListboxField } from '~/components/form/fields/ListboxField'
import { NameField } from '~/components/form/fields/NameField'
import { RadioField } from '~/components/form/fields/RadioField'
Expand Down Expand Up @@ -210,7 +210,7 @@ const DiskSourceField = ({
label="Source image"
placeholder="Select an image"
isLoading={areImagesLoading}
items={images.map((i) => toListboxItem(i, true))}
items={images.map((i) => toImageComboboxItem(i, true))}
required
onChange={(id) => {
const image = images.find((i) => i.id === id)! // if it's selected, it must be present
Expand Down
15 changes: 7 additions & 8 deletions app/forms/firewall-rules-common.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { RadioField } from '~/components/form/fields/RadioField'
import { TextField, TextFieldInner } from '~/components/form/fields/TextField'
import { useVpcSelector } from '~/hooks/use-params'
import { Badge } from '~/ui/lib/Badge'
import { toComboboxItems, type ComboboxItem } from '~/ui/lib/Combobox'
import { FormDivider } from '~/ui/lib/Divider'
import { Message } from '~/ui/lib/Message'
import * as MiniTable from '~/ui/lib/MiniTable'
Expand Down Expand Up @@ -99,7 +100,7 @@ const DynamicTypeAndValueFields = ({
sectionType: 'target' | 'host'
control: Control<TargetAndHostFormValues>
valueType: TargetAndHostFilterType
items: Array<{ value: string; label: string }>
items: Array<ComboboxItem>
disabled?: boolean
onInputChange?: (value: string) => void
onTypeChange: () => void
Expand Down Expand Up @@ -204,8 +205,8 @@ const TypeAndValueTable = ({ sectionType, items }: TypeAndValueTableProps) => (
</MiniTable.Table>
)

// Given an array of committed items (VPCs, Subnets, Instances) and
// a list of all items, return the items that are available
/** Given an array of *committed* items (VPCs, Subnets, Instances) and a list of *all* items,
* return the items that are available */
const availableItems = (
committedItems: Array<VpcFirewallRuleTarget | VpcFirewallRuleHostFilter>,
itemType: 'vpc' | 'subnet' | 'instance',
Expand All @@ -214,13 +215,11 @@ const availableItems = (
if (!items) return []
return (
items
.map((item) => item.name)
// remove any items that match the committed items on both type and value
.filter(
(name) =>
({ name }) =>
!committedItems.filter((ci) => ci.type === itemType && ci.value === name).length
)
.map((name) => ({ label: name, value: name }))
)
}

Expand Down Expand Up @@ -434,7 +433,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
sectionType="target"
control={targetForm.control}
valueType={targetType}
items={targetItems[targetType]}
items={toComboboxItems(targetItems[targetType])}
// HACK: reset the whole subform, keeping type (because we just set
// it). most importantly, this resets isSubmitted so the form can go
// back to validating on submit instead of change
Expand Down Expand Up @@ -546,7 +545,7 @@ export const CommonFields = ({ control, nameTaken, error }: CommonFieldsProps) =
sectionType="host"
control={hostForm.control}
valueType={hostType}
items={hostFilterItems[hostType]}
items={toComboboxItems(hostFilterItems[hostType])}
// HACK: reset the whole subform, keeping type (because we just set
// it). most importantly, this resets isSubmitted so the form can go
// back to validating on submit instead of change
Expand Down
6 changes: 2 additions & 4 deletions app/forms/instance-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import { addToast } from '~/stores/toast'
import { Badge } from '~/ui/lib/Badge'
import { Button } from '~/ui/lib/Button'
import { Checkbox } from '~/ui/lib/Checkbox'
import { toComboboxItems } from '~/ui/lib/Combobox'
import { FormDivider } from '~/ui/lib/Divider'
import { EmptyMessage } from '~/ui/lib/EmptyMessage'
import { Listbox } from '~/ui/lib/Listbox'
Expand Down Expand Up @@ -197,10 +198,7 @@ export function CreateInstanceForm() {
const allDisks = usePrefetchedApiQuery('diskList', {
query: { project, limit: ALL_ISH },
}).data.items
const disks = useMemo(
() => allDisks.filter(diskCan.attach).map(({ name }) => ({ value: name, label: name })),
[allDisks]
)
const disks = useMemo(() => toComboboxItems(allDisks.filter(diskCan.attach)), [allDisks])

const { data: sshKeys } = usePrefetchedApiQuery('currentUserSshKeyList', {})
const allKeys = useMemo(() => sshKeys.items.map((key) => key.id), [sshKeys])
Expand Down
11 changes: 5 additions & 6 deletions app/forms/snapshot-create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*
* Copyright Oxide Computer Company
*/
import { useMemo } from 'react'
import { useForm } from 'react-hook-form'
import { useNavigate } from 'react-router-dom'

Expand All @@ -23,18 +24,15 @@ import { NameField } from '~/components/form/fields/NameField'
import { SideModalForm } from '~/components/form/SideModalForm'
import { useProjectSelector } from '~/hooks/use-params'
import { addToast } from '~/stores/toast'
import { toComboboxItems } from '~/ui/lib/Combobox'
import { ALL_ISH } from '~/util/consts'
import { pb } from '~/util/path-builder'

const useSnapshotDiskItems = (projectSelector: PP.Project) => {
const { data: disks } = useApiQuery('diskList', {
query: { ...projectSelector, limit: ALL_ISH },
})
return (
disks?.items
.filter(diskCan.snapshot)
.map((disk) => ({ value: disk.name, label: disk.name })) || []
)
return disks?.items.filter(diskCan.snapshot)
}

const defaultValues: SnapshotCreate = {
Expand All @@ -49,6 +47,7 @@ export function CreateSnapshotSideModalForm() {
const navigate = useNavigate()

const diskItems = useSnapshotDiskItems(projectSelector)
const diskItemsForCombobox = useMemo(() => toComboboxItems(diskItems), [diskItems])

const onDismiss = () => navigate(pb.snapshots(projectSelector))

Expand Down Expand Up @@ -79,7 +78,7 @@ export function CreateSnapshotSideModalForm() {
label="Disk"
name="disk"
placeholder="Select a disk"
items={diskItems}
items={diskItemsForCombobox}
required
control={form.control}
/>
Expand Down
6 changes: 1 addition & 5 deletions app/forms/vpc-router-route-common.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,18 @@ import type { UseFormReturn } from 'react-hook-form'

import {
usePrefetchedApiQuery,
type Instance,
type RouteDestination,
type RouterRouteCreate,
type RouterRouteUpdate,
type RouteTarget,
type VpcSubnet,
} from '~/api'
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 { TextField } from '~/components/form/fields/TextField'
import { useVpcRouterSelector } from '~/hooks/use-params'
import { toComboboxItems } from '~/ui/lib/Combobox'
import { Message } from '~/ui/lib/Message'
import { validateIp, validateIpNet } from '~/util/ip'

Expand Down Expand Up @@ -94,9 +93,6 @@ const targetValueDescription: Record<RouteTarget['type'], string | undefined> =
const toListboxItems = (mapping: Record<string, string>) =>
Object.entries(mapping).map(([value, label]) => ({ value, label }))

const toComboboxItems = (items: Array<Instance | VpcSubnet>) =>
items.map(({ name }) => ({ value: name, label: name }))

type RouteFormFieldsProps = {
form: UseFormReturn<RouteFormValues>
disabled?: boolean
Expand Down
Loading
Loading