Skip to content

Commit

Permalink
Fix listbox IDs; better link labels to inputs (#2492)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliepark authored Oct 9, 2024
1 parent 48f3bca commit 0a47097
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 9 deletions.
17 changes: 12 additions & 5 deletions app/ui/lib/Listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@
*/
import {
Listbox as HListbox,
Label,
ListboxButton,
ListboxOption,
ListboxOptions,
} from '@headlessui/react'
import cn from 'classnames'
import { type ReactNode, type Ref } from 'react'
import { useId, type ReactNode, type Ref } from 'react'

import { SelectArrows6Icon } from '@oxide/design-system/icons/react'

Expand Down Expand Up @@ -68,6 +67,7 @@ export const Listbox = <Value extends string = string>({
const noItems = !isLoading && items.length === 0
const isDisabled = disabled || noItems
const zIndex = usePopoverZIndex()
const id = useId()

return (
<div className={cn('relative', className)}>
Expand All @@ -83,13 +83,20 @@ export const Listbox = <Value extends string = string>({
<>
{label && (
<div className="mb-2">
<FieldLabel id={``} as="div" optional={!required && !hideOptionalTag}>
<Label>{label}</Label>
<FieldLabel
id={`${id}-label`}
htmlFor={id}
optional={!required && !hideOptionalTag}
>
{label}
</FieldLabel>
{description && <TextInputHint id={``}>{description}</TextInputHint>}
{description && (
<TextInputHint id={`${id}-help-text`}>{description}</TextInputHint>
)}
</div>
)}
<ListboxButton
id={id}
name={name}
className={cn(
`flex h-10 w-full items-center justify-between rounded border text-sans-md`,
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/floating-ip-create.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ test('can detach and attach a floating IP', async ({ page }) => {
// Now click back to floating IPs and reattach it to db1
await page.getByRole('link', { name: 'Floating IPs' }).click()
await clickRowAction(page, 'cola-float', 'Attach')
await page.getByRole('button', { name: 'Select an instance' }).click()
await page.getByLabel('Instance').click()
await page.getByRole('option', { name: 'db1' }).click()

await page.getByRole('button', { name: 'Attach' }).click()
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/instance-create.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,10 @@ test('does not attach an ephemeral IP when the checkbox is unchecked', async ({

test('attaches a floating IP; disables button when no IPs available', async ({ page }) => {
const attachFloatingIpButton = page.getByRole('button', { name: 'Attach floating IP' })
const selectFloatingIpButton = page.getByRole('button', { name: 'Select a floating ip' })
const dialog = page.getByRole('dialog')
const selectFloatingIpButton = dialog.getByRole('button', { name: 'Floating IP' })
const rootbeerFloatOption = page.getByRole('option', { name: 'rootbeer-float' })
const attachButton = page.getByRole('button', { name: 'Attach', exact: true })
const attachButton = dialog.getByRole('button', { name: 'Attach', exact: true })

const instanceName = 'with-floating-ip'
await page.goto('/projects/mock-project/instances-new')
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/instance-networking.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ test('Instance networking tab — floating IPs', async ({ page }) => {
// Select the 'rootbeer-float' option
const dialog = page.getByRole('dialog')
// TODO: this "select the option" syntax is awkward; it's working, but I suspect there's a better way
await dialog.getByRole('button', { name: 'Select a floating IP' }).click()
await dialog.getByLabel('Floating IP').click()
await page.keyboard.press('ArrowDown')
await page.keyboard.press('Enter')
// await dialog.getByRole('button', { name: 'rootbeer-float' }).click()
Expand Down

0 comments on commit 0a47097

Please sign in to comment.