Skip to content

Commit ff240db

Browse files
david-crespodavid-crespo
authored
Make tooltips accessible to screen readers via persistent aria-describedby (#3333)
While testing #3332 with VO in Safari I noticed that there is no way to get a screen reader to read a tooltip on a button. It appears this is because they only appear in the a11y tree when the tooltip is open (it appears visually but not to the screen reader for some reason). So instead we add a permanent describedby to the button. It works! I considered having it reuse the tooltip div as the `sr-only` description when the tooltip is closed to avoid having two copies of the description in the DOM, but basically it seems like Floating UI wants you to do conditional rendering. In order to keep the div around and _not_ conditionally render it, we would instead end up doing kinda gnarly stuff like this: ```tsx <div ref={open ? refs.setFloating : undefined} className={open ? 'ox-tooltip …' : 'sr-only'} aria-hidden={open ? undefined : true} > {content} </div> ``` It can work, but it requires a more complicated integration with Floating UI lifecycle stuff. Seems fine to duplicate the div instead and keep the tooltip rendering on the simple happy path. - https://floating-ui.com/docs/react#anchoring - https://floating-ui.com/docs/usefloating#whileelementsmounted
1 parent dfa45c5 commit ff240db

10 files changed

Lines changed: 107 additions & 30 deletions

‎app/ui/lib/Tooltip.tsx‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
type Placement,
2525
} from '@floating-ui/react'
2626
import cn from 'classnames'
27-
import { Children, cloneElement, useRef, useState, type ReactElement } from 'react'
27+
import { Children, cloneElement, useId, useRef, useState, type ReactElement } from 'react'
2828

2929
import { usePopoverZIndex } from './SideModal'
3030

@@ -36,7 +36,10 @@ export interface TooltipProps {
3636
// the corner at (0,0).
3737

3838
/** The target the tooltip hovers near; can not be a raw string. */
39-
children?: ReactElement<{ ref?: React.Ref<HTMLButtonElement> }>
39+
children?: ReactElement<{
40+
ref?: React.Ref<HTMLButtonElement>
41+
'aria-describedby'?: string
42+
}>
4043
/** The text to appear on hover/focus */
4144
content?: string | React.ReactNode
4245
/**
@@ -78,9 +81,20 @@ export const Tooltip = ({ delay = 250, children, content, placement }: TooltipPr
7881
useRole(context, { role: 'tooltip' }),
7982
])
8083

84+
const descriptionId = useId()
85+
8186
const onlyChild = Children.only(children)!
87+
const describedBy = content
88+
? `${onlyChild.props['aria-describedby'] ?? ''} ${descriptionId}`.trim()
89+
: onlyChild.props['aria-describedby']
8290
const child = cloneElement(onlyChild, {
8391
...getReferenceProps(),
92+
// Point at a visually hidden copy of the content that is always rendered,
93+
// overriding the aria-describedby that useRole sets only while the tooltip
94+
// is open. Open state requires hover or DOM focus, and a screen reader
95+
// cursor is neither, so a description that only exists while open is
96+
// unreachable for exactly the users who need it.
97+
'aria-describedby': describedBy,
8498
// merge with whatever ref is already on the button
8599
ref: useMergeRefs([refs.setReference, onlyChild.props.ref]),
86100
})
@@ -93,6 +107,15 @@ export const Tooltip = ({ delay = 250, children, content, placement }: TooltipPr
93107
<>
94108
{child}
95109
<FloatingPortal>
110+
{/* Keep the persistent description separate from the conditionally
111+
rendered tooltip. Floating UI's autoUpdate lifecycle assumes the
112+
floating element unmounts while closed. aria-hidden keeps this copy
113+
out of the reading order, but description computation still follows
114+
the aria-describedby reference into hidden nodes.
115+
https://floating-ui.com/docs/react#anchoring */}
116+
<div id={descriptionId} className="sr-only" aria-hidden>
117+
{content}
118+
</div>
96119
{open && (
97120
<div
98121
ref={refs.setFloating}

‎test/e2e/external-subnets.e2e.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ test('cannot delete an attached external subnet', async ({ page }) => {
201201
const deleteButton = page.getByRole('menuitem', { name: 'Delete' })
202202
await expect(deleteButton).toBeDisabled()
203203
await deleteButton.hover()
204-
await expect(page.getByText('must be detached')).toBeVisible()
204+
await expect(page.getByRole('tooltip').getByText('must be detached')).toBeVisible()
205205
})
206206

207207
test('can detach and reattach an external subnet from the list page', async ({ page }) => {

‎test/e2e/fleet-access.e2e.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,9 @@ test('Cross-silo user shows UUID with tooltip', async ({ page }) => {
168168
await expect(userCell).toBeVisible()
169169
await userCell.getByRole('button', { name: 'Tip' }).hover()
170170
await expect(
171-
page.getByText("Can't resolve name because user is not in your silo")
171+
page
172+
.getByRole('tooltip')
173+
.getByText("Can't resolve name because user is not in your silo")
172174
).toBeVisible()
173175

174176
// dismiss the first tooltip before checking the group's
@@ -179,7 +181,9 @@ test('Cross-silo user shows UUID with tooltip', async ({ page }) => {
179181
await expect(groupCell).toBeVisible()
180182
await groupCell.getByRole('button', { name: 'Tip' }).hover()
181183
await expect(
182-
page.getByText("Can't resolve name because group is not in your silo")
184+
page
185+
.getByRole('tooltip')
186+
.getByText("Can't resolve name because group is not in your silo")
183187
).toBeVisible()
184188
})
185189

‎test/e2e/instance-create.e2e.ts‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,12 @@ test('ephemeral pool selection tracks network interface IP version', async ({ pa
155155

156156
// Verify disabled v4 checkbox shows tooltip
157157
await v4Checkbox.hover()
158-
await expect(page.getByText('Add an IPv4 network interface')).toBeVisible()
159-
await expect(page.getByText('to attach an ephemeral IPv4 address')).toBeVisible()
158+
await expect(
159+
page.getByRole('tooltip').getByText('Add an IPv4 network interface')
160+
).toBeVisible()
161+
await expect(
162+
page.getByRole('tooltip').getByText('to attach an ephemeral IPv4 address')
163+
).toBeVisible()
160164

161165
// Change to IPv4-only NIC - v6 checkbox should become disabled and unchecked
162166
await selectOption(page, page.getByRole('button', { name: 'IPv6', exact: true }), 'IPv4')
@@ -169,8 +173,12 @@ test('ephemeral pool selection tracks network interface IP version', async ({ pa
169173

170174
// Verify disabled v6 checkbox shows tooltip
171175
await v6Checkbox.hover()
172-
await expect(page.getByText('Add an IPv6 network interface')).toBeVisible()
173-
await expect(page.getByText('to attach an ephemeral IPv6 address')).toBeVisible()
176+
await expect(
177+
page.getByRole('tooltip').getByText('Add an IPv6 network interface')
178+
).toBeVisible()
179+
await expect(
180+
page.getByRole('tooltip').getByText('to attach an ephemeral IPv6 address')
181+
).toBeVisible()
174182
})
175183

176184
test('duplicate instance name produces visible error', async ({ page }) => {
@@ -1073,13 +1081,21 @@ test('ephemeral IP checkbox disabled when no NICs configured', async ({ page })
10731081

10741082
// Verify tooltip shows disabled reason for IPv4
10751083
await v4Checkbox.hover()
1076-
await expect(page.getByText('Add an IPv4 network interface')).toBeVisible()
1077-
await expect(page.getByText('to attach an ephemeral IPv4 address')).toBeVisible()
1084+
await expect(
1085+
page.getByRole('tooltip').getByText('Add an IPv4 network interface')
1086+
).toBeVisible()
1087+
await expect(
1088+
page.getByRole('tooltip').getByText('to attach an ephemeral IPv4 address')
1089+
).toBeVisible()
10781090

10791091
// Verify tooltip shows disabled reason for IPv6
10801092
await v6Checkbox.hover()
1081-
await expect(page.getByText('Add an IPv6 network interface')).toBeVisible()
1082-
await expect(page.getByText('to attach an ephemeral IPv6 address')).toBeVisible()
1093+
await expect(
1094+
page.getByRole('tooltip').getByText('Add an IPv6 network interface')
1095+
).toBeVisible()
1096+
await expect(
1097+
page.getByRole('tooltip').getByText('to attach an ephemeral IPv6 address')
1098+
).toBeVisible()
10831099

10841100
// Select "Custom" radio → verify ephemeral IP checkboxes are still disabled and unchecked
10851101
await customRadio.click()
@@ -1092,8 +1108,12 @@ test('ephemeral IP checkbox disabled when no NICs configured', async ({ page })
10921108

10931109
// Verify tooltip still shows disabled reason when in Custom mode with no NICs
10941110
await v4Checkbox.hover()
1095-
await expect(page.getByText('Add an IPv4 network interface')).toBeVisible()
1096-
await expect(page.getByText('to attach an ephemeral IPv4 address')).toBeVisible()
1111+
await expect(
1112+
page.getByRole('tooltip').getByText('Add an IPv4 network interface')
1113+
).toBeVisible()
1114+
await expect(
1115+
page.getByRole('tooltip').getByText('to attach an ephemeral IPv4 address')
1116+
).toBeVisible()
10971117

10981118
// Click "Add network interface" button to open modal
10991119
await page.getByRole('button', { name: 'Add network interface' }).click()
@@ -1265,8 +1285,10 @@ test('floating IPs are filtered by NIC IP version', async ({ page }) => {
12651285

12661286
// Verify the disabled reason tooltip
12671287
await attachFloatingIpButton.hover()
1268-
await expect(page.getByText('A network interface is required')).toBeVisible()
1269-
await expect(page.getByText('to attach a floating IP')).toBeVisible()
1288+
await expect(
1289+
page.getByRole('tooltip').getByText('A network interface is required')
1290+
).toBeVisible()
1291+
await expect(page.getByRole('tooltip').getByText('to attach a floating IP')).toBeVisible()
12701292
})
12711293

12721294
test('can create instance with read-only boot disk', async ({ page }) => {

‎test/e2e/instance-disks.e2e.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ test('Disk detail side modal', async ({ page }) => {
3131
test('Disabled actions', async ({ page }) => {
3232
await page.goto('/projects/mock-project/instances/db1')
3333

34+
// the disabled reason is exposed to screen readers as an accessible
35+
// description even with no hover or focus to pop the tooltip
36+
await expect(
37+
page.getByRole('button', { name: 'Attach existing disk' })
38+
).toHaveAccessibleDescription('Instance must be stopped to attach a disk')
39+
await expect(
40+
page.getByRole('button', { name: 'Create disk' })
41+
).toHaveAccessibleDescription('Instance must be stopped to create and attach a disk')
42+
3443
const bootDiskRow = page.getByRole('row', { name: 'disk-1', exact: false })
3544
await expect(bootDiskRow).toBeVisible()
3645

@@ -44,13 +53,15 @@ test('Disabled actions', async ({ page }) => {
4453
await expect(unsetButton).toBeDisabled()
4554
await page.getByRole('menuitem', { name: 'Unset' }).hover()
4655
await expect(
47-
page.getByText('Instance must be stopped before boot disk can be changed')
56+
page
57+
.getByRole('tooltip')
58+
.getByText('Instance must be stopped before boot disk can be changed')
4859
).toBeVisible()
4960

5061
await expect(detachButton).toBeDisabled()
5162
await detachButton.hover()
5263
await expect(
53-
page.getByText('Boot disk must be unset before it can be detached')
64+
page.getByRole('tooltip').getByText('Boot disk must be unset before it can be detached')
5465
).toBeVisible()
5566
await page.keyboard.press('Escape') // close menu
5667

@@ -63,13 +74,17 @@ test('Disabled actions', async ({ page }) => {
6374
await expect(setButton).toBeDisabled()
6475
await setButton.hover()
6576
await expect(
66-
page.getByText('Instance must be stopped before boot disk can be changed')
77+
page
78+
.getByRole('tooltip')
79+
.getByText('Instance must be stopped before boot disk can be changed')
6780
).toBeVisible()
6881

6982
await expect(detachButton).toBeDisabled()
7083
await detachButton.hover()
7184
await expect(
72-
page.getByText('Instance must be stopped before disk can be detached')
85+
page
86+
.getByRole('tooltip')
87+
.getByText('Instance must be stopped before disk can be detached')
7388
).toBeVisible()
7489
await page.keyboard.press('Escape') // close menu
7590

‎test/e2e/instance-networking.e2e.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ test('Instance networking tab — NIC table', async ({ page }) => {
110110
const deleteButton = page.getByRole('menuitem', { name: 'Delete' })
111111
await expect(deleteButton).toBeDisabled()
112112
await deleteButton.hover()
113-
await expect(page.getByText('The primary interface can’t')).toBeVisible()
113+
await expect(
114+
page.getByRole('tooltip').getByText('The primary interface can’t')
115+
).toBeVisible()
114116

115117
// close the menu for nic-3, without the next line fails in FF and Safari (but not Chrome)
116118
await clickRowActions(page, 'nic-3')

‎test/e2e/instance.e2e.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ test('shows external IPs on the instances table', async ({ page }) => {
5454
).toHaveCount(1)
5555
// hovering the +1 reveals the other external IP in a tooltip
5656
await table.getByRole('row', { name: 'db1' }).getByText('+1').hover()
57-
await expect(page.getByText('Other external IPs')).toBeVisible()
58-
await expect(page.getByText('123.4.56.0')).toBeVisible()
57+
const tooltip = page.getByRole('tooltip')
58+
await expect(tooltip.getByText('Other external IPs')).toBeVisible()
59+
await expect(tooltip.getByText('123.4.56.0')).toBeVisible()
5960

6061
// not-there-yet has three ephemeral IPs, so it shows the first plus a +2 overflow
6162
await expect(
@@ -77,7 +78,7 @@ test('can start a failed instance', async ({ page }) => {
7778
await clickRowActions(page, 'db1')
7879
await page.getByRole('menuitem', { name: 'Start' }).hover()
7980
await expect(
80-
page.getByText('Only stopped or failed instances can be started')
81+
page.getByRole('tooltip').getByText('Only stopped or failed instances can be started')
8182
).toBeVisible()
8283
await page.keyboard.press('Escape') // get out of the menu
8384

‎test/e2e/ip-pool-silo-config.e2e.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ test.describe('IP pool configuration: myriad silo (v4-only default)', () => {
5252
await expect(v6Checkbox).not.toBeChecked()
5353
await expect(v6Checkbox).toBeDisabled()
5454
await v6Checkbox.hover()
55-
await expect(page.getByText('No IPv6 pools available')).toBeVisible()
55+
await expect(
56+
page.getByRole('tooltip').getByText('No IPv6 pools available')
57+
).toBeVisible()
5658

5759
// IPv4 pool dropdown should be visible with default pool preselected
5860
const v4PoolDropdown = page.getByLabel('IPv4 pool')
@@ -230,7 +232,9 @@ test.describe('IP pool configuration: thrax silo (v6-only default)', () => {
230232
await expect(v4Checkbox).not.toBeChecked()
231233
await expect(v4Checkbox).toBeDisabled()
232234
await v4Checkbox.hover()
233-
await expect(page.getByText('No IPv4 pools available')).toBeVisible()
235+
await expect(
236+
page.getByRole('tooltip').getByText('No IPv4 pools available')
237+
).toBeVisible()
234238
await expect(v6Checkbox).toBeVisible()
235239
await expect(v6Checkbox).toBeChecked()
236240

‎test/e2e/system-update.e2e.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ test('Set target release', async ({ page }) => {
5454
const disabledItem = page.getByRole('menuitem', { name: 'Set as target release' })
5555
await expect(disabledItem).toBeDisabled()
5656
await disabledItem.hover()
57-
await expect(page.getByText('Already set as target')).toBeVisible()
57+
await expect(page.getByRole('tooltip').getByText('Already set as target')).toBeVisible()
5858
await page.keyboard.press('Escape')
5959

6060
// Upgrade to 18.0.0
@@ -87,7 +87,9 @@ test('Set target release', async ({ page }) => {
8787
const setTargetItem = page.getByRole('menuitem', { name: 'Set as target release' })
8888
await expect(setTargetItem).toBeDisabled()
8989
await setTargetItem.hover()
90-
await expect(page.getByText('Cannot set older release as target')).toBeVisible()
90+
await expect(
91+
page.getByRole('tooltip').getByText('Cannot set older release as target')
92+
).toBeVisible()
9193
})
9294

9395
test('Cannot downgrade to older release', async ({ page }) => {
@@ -104,7 +106,9 @@ test('Cannot downgrade to older release', async ({ page }) => {
104106
const setTargetItem = page.getByRole('menuitem', { name: 'Set as target release' })
105107
await expect(setTargetItem).toBeDisabled()
106108
await setTargetItem.hover()
107-
await expect(page.getByText('Cannot set older release as target')).toBeVisible()
109+
await expect(
110+
page.getByRole('tooltip').getByText('Cannot set older release as target')
111+
).toBeVisible()
108112

109113
// Verify the target release has NOT changed - still 17.0.0
110114
await expect(page.getByLabel('Properties table')).toContainText('17.0.0')

‎vitest.browser.config.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export default defineConfig({
3737
resolve: { tsconfigPaths: true },
3838
test: {
3939
attachmentsDir: 'test-results/vitest/attachments',
40+
// Concurrent Playwright contexts make focus-sensitive Firefox interactions flaky.
41+
fileParallelism: false,
4042
include: ['app/**/*.browser.spec.{ts,tsx}'],
4143
name: 'browser',
4244
setupFiles: ['test/browser/setup.ts'],

0 commit comments

Comments
 (0)