diff --git a/app/components/form/fields/ComboboxField.tsx b/app/components/form/fields/ComboboxField.tsx index ce79af7e4..a49561b7d 100644 --- a/app/components/form/fields/ComboboxField.tsx +++ b/app/components/form/fields/ComboboxField.tsx @@ -6,6 +6,7 @@ * Copyright Oxide Computer Company */ +import { useState } from 'react' import { useController, type Control, @@ -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' @@ -54,6 +59,7 @@ export function ComboboxField< : allowArbitraryValues ? 'Select an option or enter a custom value' : 'Select an option', + items, validate, ...props }: ComboboxFieldProps) { @@ -62,20 +68,27 @@ export function ComboboxField< control, rules: { required, validate }, }) + const [selectedItemLabel, setSelectedItemLabel] = useState( + getSelectedLabelFromValue(items, field.value || '') + ) return (
{ field.onChange(value) onChange?.(value) + setSelectedItemLabel(getSelectedLabelFromValue(items, value)) }} allowArbitraryValues={allowArbitraryValues} + inputRef={field.ref} {...props} /> diff --git a/app/components/form/fields/ImageSelectField.tsx b/app/components/form/fields/ImageSelectField.tsx index 4f11e6f3f..2254c0cd0 100644 --- a/app/components/form/fields/ImageSelectField.tsx +++ b/app/components/form/fields/ImageSelectField.tsx @@ -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[] @@ -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). - 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)) @@ -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 ``-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 ``-separated list for each comboboxitem + const itemMetadata = [os, version, `${bytesToGiB(size, 1)} GiB`, projectSiloIndicator] .filter((i) => !!i) .map((i, index) => ( @@ -79,14 +77,12 @@ export function toListboxItem(i: Image, includeProjectSiloIndicator = false): Li )) return { - value: i.id, - selectedLabel: `${name}${metadataForSelectedLabel}`, + value: id, + selectedLabel: name, label: ( <>
{name}
-
- {metadataForLabel} -
+
{itemMetadata}
), } diff --git a/app/forms/disk-attach.tsx b/app/forms/disk-attach.tsx index 02f76203e..ccfccdc7f 100644 --- a/app/forms/disk-attach.tsx +++ b/app/forms/disk-attach.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useMemo } from 'react' import { useForm } from 'react-hook-form' import { useApiQuery, type ApiError } from '@oxide/api' @@ -12,6 +13,7 @@ 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: '' } @@ -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 }) @@ -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} /> diff --git a/app/forms/disk-create.tsx b/app/forms/disk-create.tsx index 8f50a6d23..c3671a67f 100644 --- a/app/forms/disk-create.tsx +++ b/app/forms/disk-create.tsx @@ -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' @@ -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 diff --git a/app/forms/firewall-rules-common.tsx b/app/forms/firewall-rules-common.tsx index abb0992d3..b0935c893 100644 --- a/app/forms/firewall-rules-common.tsx +++ b/app/forms/firewall-rules-common.tsx @@ -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' @@ -99,7 +100,7 @@ const DynamicTypeAndValueFields = ({ sectionType: 'target' | 'host' control: Control valueType: TargetAndHostFilterType - items: Array<{ value: string; label: string }> + items: Array disabled?: boolean onInputChange?: (value: string) => void onTypeChange: () => void @@ -204,8 +205,8 @@ const TypeAndValueTable = ({ sectionType, items }: TypeAndValueTableProps) => ( ) -// 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, itemType: 'vpc' | 'subnet' | 'instance', @@ -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 })) ) } @@ -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 @@ -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 diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index 44cfd90d2..35f91958c 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -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' @@ -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]) diff --git a/app/forms/snapshot-create.tsx b/app/forms/snapshot-create.tsx index 656f9aebb..930cec238 100644 --- a/app/forms/snapshot-create.tsx +++ b/app/forms/snapshot-create.tsx @@ -5,6 +5,7 @@ * * Copyright Oxide Computer Company */ +import { useMemo } from 'react' import { useForm } from 'react-hook-form' import { useNavigate } from 'react-router-dom' @@ -23,6 +24,7 @@ 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' @@ -30,11 +32,7 @@ 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 = { @@ -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)) @@ -79,7 +78,7 @@ export function CreateSnapshotSideModalForm() { label="Disk" name="disk" placeholder="Select a disk" - items={diskItems} + items={diskItemsForCombobox} required control={form.control} /> diff --git a/app/forms/vpc-router-route-common.tsx b/app/forms/vpc-router-route-common.tsx index 7215113be..186667bef 100644 --- a/app/forms/vpc-router-route-common.tsx +++ b/app/forms/vpc-router-route-common.tsx @@ -10,12 +10,10 @@ 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' @@ -23,6 +21,7 @@ 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' @@ -94,9 +93,6 @@ const targetValueDescription: Record = const toListboxItems = (mapping: Record) => Object.entries(mapping).map(([value, label]) => ({ value, label })) -const toComboboxItems = (items: Array) => - items.map(({ name }) => ({ value: name, label: name })) - type RouteFormFieldsProps = { form: UseFormReturn disabled?: boolean diff --git a/app/pages/system/SiloImagesPage.tsx b/app/pages/system/SiloImagesPage.tsx index 45bc82f54..6c27a1cf5 100644 --- a/app/pages/system/SiloImagesPage.tsx +++ b/app/pages/system/SiloImagesPage.tsx @@ -21,7 +21,7 @@ import { Images16Icon, Images24Icon } from '@oxide/design-system/icons/react' import { DocsPopover } from '~/components/DocsPopover' import { ComboboxField } from '~/components/form/fields/ComboboxField' -import { toListboxItem } from '~/components/form/fields/ImageSelectField' +import { toImageComboboxItem } from '~/components/form/fields/ImageSelectField' import { ListboxField } from '~/components/form/fields/ListboxField' import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' @@ -30,6 +30,7 @@ import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { PAGE_SIZE, useQueryTable } from '~/table/QueryTable' import { Button } from '~/ui/lib/Button' +import { toComboboxItems } from '~/ui/lib/Combobox' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { Message } from '~/ui/lib/Message' import { Modal } from '~/ui/lib/Modal' @@ -140,10 +141,7 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => { }) const projects = useApiQuery('projectList', {}) - const projectItems = useMemo( - () => (projects.data?.items || []).map(({ name }) => ({ value: name, label: name })), - [projects.data] - ) + const projectItems = useMemo(() => toComboboxItems(projects.data?.items), [projects.data]) const selectedProject = watch('project') // can only fetch images if a project is selected @@ -153,7 +151,7 @@ const PromoteImageModal = ({ onDismiss }: { onDismiss: () => void }) => { { enabled: !!selectedProject } ) const imageItems = useMemo( - () => (images.data?.items || []).map((i) => toListboxItem(i)), + () => (images.data?.items || []).map((i) => toImageComboboxItem(i)), [images.data] ) @@ -242,10 +240,7 @@ const DemoteImageModal = ({ } const projects = useApiQuery('projectList', {}) - const projectItems = useMemo( - () => (projects.data?.items || []).map(({ name }) => ({ value: name, label: name })), - [projects.data] - ) + const projectItems = useMemo(() => toComboboxItems(projects.data?.items), [projects.data]) return ( diff --git a/app/pages/system/networking/IpPoolPage.tsx b/app/pages/system/networking/IpPoolPage.tsx index 2db3b07bc..7461dc41c 100644 --- a/app/pages/system/networking/IpPoolPage.tsx +++ b/app/pages/system/networking/IpPoolPage.tsx @@ -39,6 +39,7 @@ import { LinkCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { PAGE_SIZE, useQueryTable } from '~/table/QueryTable' +import { toComboboxItems } from '~/ui/lib/Combobox' import { CreateButton, CreateLink } from '~/ui/lib/CreateButton' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { Message } from '~/ui/lib/Message' @@ -387,9 +388,7 @@ function LinkSiloModal({ onDismiss }: { onDismiss: () => void }) { const unlinkedSiloItems = useMemo( () => allSilos.data && linkedSiloIds - ? allSilos.data.items - .filter((s) => !linkedSiloIds.has(s.id)) - .map((s) => ({ value: s.name, label: s.name })) + ? toComboboxItems(allSilos.data.items.filter((s) => !linkedSiloIds.has(s.id))) : [], [allSilos, linkedSiloIds] ) diff --git a/app/pages/system/silos/SiloIpPoolsTab.tsx b/app/pages/system/silos/SiloIpPoolsTab.tsx index a03f59f1c..81aef8d4c 100644 --- a/app/pages/system/silos/SiloIpPoolsTab.tsx +++ b/app/pages/system/silos/SiloIpPoolsTab.tsx @@ -23,6 +23,7 @@ import { makeLinkCell } from '~/table/cells/LinkCell' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { useQueryTable } from '~/table/QueryTable' +import { toComboboxItems } from '~/ui/lib/Combobox' import { CreateButton } from '~/ui/lib/CreateButton' import { EmptyMessage } from '~/ui/lib/EmptyMessage' import { Message } from '~/ui/lib/Message' @@ -213,9 +214,7 @@ function LinkPoolModal({ onDismiss }: { onDismiss: () => void }) { const unlinkedPoolItems = useMemo( () => allPools.data && linkedPoolIds - ? allPools.data.items - .filter((p) => !linkedPoolIds.has(p.id)) - .map((p) => ({ value: p.name, label: p.name })) + ? toComboboxItems(allPools.data.items.filter((p) => !linkedPoolIds.has(p.id))) : [], [allPools, linkedPoolIds] ) diff --git a/app/ui/lib/Combobox.tsx b/app/ui/lib/Combobox.tsx index f5fa9484b..bc7de2ff8 100644 --- a/app/ui/lib/Combobox.tsx +++ b/app/ui/lib/Combobox.tsx @@ -12,11 +12,10 @@ import { ComboboxOption, ComboboxOptions, Combobox as HCombobox, - Label, } from '@headlessui/react' import cn from 'classnames' import { matchSorter } from 'match-sorter' -import { useState } from 'react' +import { useId, useState, type ReactNode, type Ref } from 'react' import { SelectArrows6Icon } from '@oxide/design-system/icons/react' @@ -24,16 +23,33 @@ import { FieldLabel } from './FieldLabel' import { usePopoverZIndex } from './SideModal' import { TextInputHint } from './TextInput' +export type ComboboxItem = { value: string; label: ReactNode; selectedLabel: string } + +/** Convert an array of items with a `name` attribute to an array of ComboboxItems + * Useful when the rendered label and value are the same; in more complex cases, + * you may want to create a custom ComboboxItem object (see toImageComboboxItem). + */ +export const toComboboxItems = (items?: Array<{ name: string }>): Array => + items?.map(({ name }) => ({ + value: name, + label: name, + selectedLabel: name, + })) || [] + +export const getSelectedLabelFromValue = ( + items: Array, + selectedValue: string +): string => items.find((item) => item.value === selectedValue)?.selectedLabel || '' + /** Simple non-generic props shared with ComboboxField */ export type ComboboxBaseProps = { description?: React.ReactNode disabled?: boolean isLoading?: boolean - items: Array<{ label: string; value: string }> + items: Array label: string placeholder?: string required?: boolean - ariaLabel?: string hideOptionalTag?: boolean /** * Pass in `allowArbitraryValues` as `true` when the user should be able to @@ -49,16 +65,20 @@ export type ComboboxBaseProps = { } type ComboboxProps = { - selected: string | null + selectedItemValue: string + selectedItemLabel: string hasError?: boolean onChange: (value: string) => void + /** Necessary if you want RHF to be able to focus it on error */ + inputRef?: Ref } & ComboboxBaseProps export const Combobox = ({ description, items = [], - selected, label, + selectedItemValue, + selectedItemLabel, placeholder, required, hasError, @@ -67,62 +87,81 @@ export const Combobox = ({ onChange, onInputChange, allowArbitraryValues = false, - ariaLabel, hideOptionalTag, + inputRef, + ...props }: ComboboxProps) => { - const [query, setQuery] = useState(selected || '') - - const q = query.toLowerCase() + const [query, setQuery] = useState(selectedItemValue || '') + const q = query.toLowerCase().replace(/\s*/g, '') const filteredItems = matchSorter(items, q, { - keys: ['value'], + keys: ['selectedLabel', 'label'], sorter: (items) => items, // preserve original order, don't sort by match }) - const zIndex = usePopoverZIndex() - + const id = useId() return ( <> onChange(val || '')} onClose={() => setQuery('')} - defaultValue={selected} disabled={disabled || isLoading} + immediate + {...props} > {label && ( // TODO: FieldLabel needs a real ID
- - + + {label} - {description && {description}} + {description && ( + {description} + )}
)} - (selected ? selected : query)} + id={`${id}-input`} + // displayValue controls what's displayed in the input field. + // selectedItemValue is displayed when the user can type in a new value. + // Otherwise, use the provided selectedItemLabel + displayValue={() => + allowArbitraryValues ? selectedItemValue : selectedItemLabel + } onChange={(event) => { + // updates the query state as the user types, in order to filter the list of items setQuery(event.target.value) + // if the parent component wants to know about input changes, call the callback onInputChange?.(event.target.value) }} placeholder={placeholder} disabled={disabled || isLoading} className={cn( - `w-full rounded !border-none px-3 py-[0.5rem] !outline-none text-sans-md text-default placeholder:text-quaternary`, + `h-10 w-full rounded !border-none px-3 py-[0.5rem] !outline-none text-sans-md text-default placeholder:text-quaternary`, disabled ? 'cursor-not-allowed text-disabled bg-disabled !border-default' : 'bg-default', @@ -130,16 +169,19 @@ export const Combobox = ({ )} /> {items.length > 0 && ( -
+ -
+
)} - +
{items.length > 0 && ( {!allowArbitraryValues && filteredItems.length === 0 && ( @@ -149,13 +191,9 @@ export const Combobox = ({ )} {filteredItems.map((item) => ( { - onChange(item.label) - setQuery(item.label) - }} > {({ focus, selected }) => ( // This *could* be done with data-[focus] and data-[selected] instead, but diff --git a/test/e2e/images.e2e.ts b/test/e2e/images.e2e.ts index 0c15a4281..c15f91fc8 100644 --- a/test/e2e/images.e2e.ts +++ b/test/e2e/images.e2e.ts @@ -14,6 +14,7 @@ import { expectNotVisible, expectVisible, getPageAsUser, + selectOption, } from './utils' test('can promote an image from silo', async ({ page }) => { @@ -25,27 +26,27 @@ test('can promote an image from silo', async ({ page }) => { // Listboxes are visible await expect(page.getByPlaceholder('Select a project')).toBeVisible() - await expect(page.locator(`text="Select an image"`)).toBeVisible() + // have to use a locator here because the disabled button needs to be handled differently + await expect(page.locator(`text="Select an image"`)).toBeDisabled() // Notice is visible await expect(page.getByText('visible to all projects')).toBeVisible() // Select a project - await page.locator('role=button[name*="Project"]').click() - await page.locator('role=option[name="other-project"]').click() + await selectOption(page, 'Project', 'other-project') - // Should have no items - // and buttons should be disabled - await expect(page.locator(`text="No items"`)).toBeVisible() - await expect(page.locator('role=button[name*="Image"]')).toBeDisabled() + // Should have no items and dropdown should be disabled + await expect(page.locator(`text="No items"`)).toBeDisabled() // Select the other project - await page.locator('role=button[name*="Project"]').click() - await page.locator('role=option[name="mock-project"]').click() + // this blurring should not be necessary, but it's blocking the test otherwise + await page.getByRole('combobox', { name: 'Project' }).blur() + await page.getByRole('combobox', { name: 'Project' }).click() + await page.getByRole('option', { name: 'mock-project' }).click() // Select an image in that project const imageListbox = page.locator('role=button[name*="Image"]') - await expect(imageListbox).toBeEnabled({ timeout: 5000 }) + await expect(imageListbox).toBeEnabled() await imageListbox.click() await page.locator('role=option >> text="image-1"').click() await page.locator('role=button[name="Promote"]').click() @@ -102,17 +103,13 @@ test('can demote an image from silo', async ({ page }) => { await expect(page.getByText('Demoting: arch-2022-06-01')).toBeVisible() // Cannot demote without first selecting a project - await page.locator('role=button[name="Demote"]').click() + await page.getByRole('button', { name: 'Demote' }).click() await expect( page.getByRole('dialog', { name: 'Demote' }).getByText('Project is required') ).toBeVisible() - // Select an project to demote it - const imageListbox = page.locator('role=button[name*="Project"]') - await expect(imageListbox).toBeEnabled({ timeout: 5000 }) - await imageListbox.click() - await page.locator('role=option >> text="mock-project"').click() - await page.locator('role=button[name="Demote"]').click() + await selectOption(page, 'Project', 'mock-project') + await page.getByRole('button', { name: 'Demote' }).click() // Promote image and check it was successful await expectVisible(page, ['text="arch-2022-06-01 has been demoted"']) diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 90c6e8dd1..35b3f4a01 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -12,19 +12,20 @@ import { expectNotVisible, expectRowVisible, expectVisible, + selectOption, test, type Page, } from './utils' const selectASiloImage = async (page: Page, name: string) => { await page.getByRole('tab', { name: 'Silo images' }).click() - await page.getByLabel('Image', { exact: true }).click() + await page.getByPlaceholder('Select a silo image', { exact: true }).click() await page.getByRole('option', { name }).click() } const selectAProjectImage = async (page: Page, name: string) => { await page.getByRole('tab', { name: 'Project images' }).click() - await page.getByLabel('Image', { exact: true }).click() + await page.getByPlaceholder('Select a project image', { exact: true }).click() await page.getByRole('option', { name }).click() } @@ -356,21 +357,24 @@ test('additional disks do not list committed disks as available', async ({ page test('maintains selected values even when changing tabs', async ({ page }) => { const instanceName = 'arch-based-instance' + const arch = 'arch-2022-06-01' await page.goto('/projects/mock-project/instances-new') await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName) - await page.getByRole('button', { name: 'Image' }).click() - // select the arch option - await page.getByRole('option', { name: 'arch-2022-06-01' }).click() + const imageSelectCombobox = page.getByRole('combobox', { name: 'Image' }) + // Filter the combobox for a particular silo image + await imageSelectCombobox.fill('arch') + // select the image + await page.getByRole('option', { name: arch }).click() // expect to find name of the image on page - await expect(page.getByText('arch-2022-06-01')).toBeVisible() + await expect(imageSelectCombobox).toHaveValue(arch) // change to a different tab await page.getByRole('tab', { name: 'Existing disks' }).click() // the image should no longer be visible - await expect(page.getByText('arch-2022-06-01')).toBeHidden() + await expect(imageSelectCombobox).toBeHidden() // change back to the tab with the image await page.getByRole('tab', { name: 'Silo images' }).click() // arch should still be selected - await expect(page.getByText('arch-2022-06-01')).toBeVisible() + await expect(imageSelectCombobox).toHaveValue(arch) await page.getByRole('button', { name: 'Create instance' }).click() await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`) await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB']) @@ -491,7 +495,7 @@ test('attaching additional disks allows for combobox filtering', async ({ page } // now options hidden and only the selected one is visible in the button/input await expect(page.getByRole('option')).toBeHidden() - await expect(page.getByRole('button', { name: 'disk-0102' })).toBeVisible() + await expect(page.getByRole('combobox', { name: 'Disk name' })).toHaveValue('disk-0102') // a random string should give a disabled option await selectADisk.click() @@ -518,8 +522,7 @@ test('create instance with additional disks', async ({ page }) => { // Attach an existing disk await page.getByRole('button', { name: 'Attach existing disk' }).click() - await page.getByRole('button', { name: 'Disk name' }).click() - await page.getByRole('option', { name: 'disk-3' }).click() + await selectOption(page, 'Disk name', 'disk-3') await page.getByRole('button', { name: 'Attach disk' }).click() await expectRowVisible(disksTable, { Name: 'disk-3', Type: 'attach', Size: '—' }) diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index c0c0cfeaf..9e5471619 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -86,7 +86,7 @@ test('Attach disk', async ({ page }) => { await page.getByRole('button', { name: 'Attach disk' }).click() await expectVisible(page, ['role=dialog >> text="Disk name is required"']) - await page.click('role=button[name*="Disk name"]') + await page.getByRole('combobox', { name: 'Disk name' }).click() // disk-1 is already attached, so should not be visible in the list await expectNotVisible(page, ['role=option[name="disk-1"]']) await expectVisible(page, ['role=option[name="disk-3"]', 'role=option[name="disk-4"]']) diff --git a/test/e2e/instance-serial.e2e.ts b/test/e2e/instance-serial.e2e.ts index 450dcfdd0..417ad1044 100644 --- a/test/e2e/instance-serial.e2e.ts +++ b/test/e2e/instance-serial.e2e.ts @@ -11,7 +11,7 @@ test('serial console can connect while starting', async ({ page }) => { // create an instance await page.goto('/projects/mock-project/instances-new') await page.getByRole('textbox', { name: 'Name', exact: true }).fill('abc') - await page.getByLabel('Image', { exact: true }).click() + await page.getByPlaceholder('Select a silo image').click() await page.getByRole('option', { name: 'ubuntu-22-04' }).click() await page.getByRole('button', { name: 'Create instance' }).click() diff --git a/test/e2e/utils.ts b/test/e2e/utils.ts index 3d779c312..e74c39e0a 100644 --- a/test/e2e/utils.ts +++ b/test/e2e/utils.ts @@ -142,18 +142,18 @@ export async function clickRowAction(page: Page, rowText: string, actionName: st /** * Select an option from a dropdown - * buttonLocator can either be the drodown's label text or a more elaborate Locator - * optionLocator can either be the drodown's label text or a more elaborate Locator + * labelLocator can either be the dropdown's label text or a more elaborate Locator + * optionLocator can either be the dropdown's option text or a more elaborate Locator * */ export async function selectOption( page: Page, - buttonLocator: string | Locator, + labelLocator: string | Locator, optionLocator: string | Locator ) { - if (typeof buttonLocator === 'string') { - await page.getByRole('button', { name: buttonLocator }).click() + if (typeof labelLocator === 'string') { + await page.getByLabel(labelLocator, { exact: true }).click() } else { - await buttonLocator.click() + await labelLocator.click() } if (typeof optionLocator === 'string') { await page.getByRole('option', { name: optionLocator, exact: true }).click()