Skip to content

Commit da0f48b

Browse files
authored
WIP: Select panel custom screen reader announcements (#4968)
* copy changes from #4927 * fix index to announce * add liveRegion.clear * separate type import * remove hard coded "labels" * don't assume live region exists * bump @primer/live-region-element to 0.7.1 * lint!!!
1 parent 928bfd2 commit da0f48b

File tree

11 files changed

+237
-73
lines changed

11 files changed

+237
-73
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/react": patch
3+
---
4+
5+
SelectPanel: Add announcements for screen readers (behind feature flag `primer_react_select_panel_with_modern_action_list`)

package-lock.json

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/react/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
"@lit-labs/react": "1.2.1",
8888
"@oddbird/popover-polyfill": "^0.3.1",
8989
"@primer/behaviors": "^1.7.2",
90-
"@primer/live-region-element": "^0.7.0",
90+
"@primer/live-region-element": "^0.7.1",
9191
"@primer/octicons-react": "^19.9.0",
9292
"@primer/primitives": "^9.0.3",
9393
"@styled-system/css": "^5.1.5",

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {SxProp} from '../sx'
2020

2121
import {isValidElementType} from 'react-is'
2222
import type {RenderItemFn} from '../deprecated/ActionList/List'
23+
import {useAnnouncements} from './useAnnouncements'
2324

2425
const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
2526

@@ -114,6 +115,7 @@ export function FilteredActionList({
114115
}, [items])
115116

116117
useScrollFlash(scrollContainerRef)
118+
useAnnouncements(items, listContainerRef, inputRef)
117119

118120
function getItemListForEachGroup(groupId: string) {
119121
const itemsInGroup = []
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Announcements for FilteredActionList (and SelectPanel) based
2+
// on https://github.com/github/multi-select-user-testing
3+
4+
import {announce} from '@primer/live-region-element'
5+
import {useEffect, useRef} from 'react'
6+
import type {FilteredActionListProps} from './FilteredActionListEntry'
7+
8+
// we add a delay so that it does not interrupt default screen reader announcement and queues after it
9+
const delayMs = 500
10+
11+
const useFirstRender = () => {
12+
const firstRender = useRef(true)
13+
useEffect(() => {
14+
firstRender.current = false
15+
}, [])
16+
return firstRender.current
17+
}
18+
19+
const getItemWithActiveDescendant = (
20+
listRef: React.RefObject<HTMLUListElement>,
21+
items: FilteredActionListProps['items'],
22+
) => {
23+
const listElement = listRef.current
24+
const activeItemElement = listElement?.querySelector('[data-is-active-descendant]')
25+
26+
if (!listElement || !activeItemElement?.textContent) return
27+
28+
const optionElements = listElement.querySelectorAll('[role="option"]')
29+
30+
const index = Array.from(optionElements).indexOf(activeItemElement)
31+
const activeItem = items[index]
32+
33+
const text = activeItem.text
34+
const selected = activeItem.selected
35+
36+
return {index, text, selected}
37+
}
38+
39+
export const useAnnouncements = (
40+
items: FilteredActionListProps['items'],
41+
listContainerRef: React.RefObject<HTMLUListElement>,
42+
inputRef: React.RefObject<HTMLInputElement>,
43+
) => {
44+
const liveRegion = document.querySelector('live-region')
45+
46+
useEffect(
47+
function announceInitialFocus() {
48+
const focusHandler = () => {
49+
// give @primer/behaviors a moment to apply active-descendant
50+
window.requestAnimationFrame(() => {
51+
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
52+
if (!activeItem) return
53+
const {index, text, selected} = activeItem
54+
55+
const announcementText = [
56+
`Focus on filter text box and list of items`,
57+
`Focused item: ${text}`,
58+
`${selected ? 'selected' : 'not selected'}`,
59+
`${index + 1} of ${items.length}`,
60+
].join(', ')
61+
announce(announcementText, {
62+
delayMs,
63+
from: liveRegion ? liveRegion : undefined, // announce will create a liveRegion if it doesn't find one
64+
})
65+
})
66+
}
67+
68+
const inputElement = inputRef.current
69+
inputElement?.addEventListener('focus', focusHandler)
70+
return () => inputElement?.removeEventListener('focus', focusHandler)
71+
},
72+
[listContainerRef, inputRef, items, liveRegion],
73+
)
74+
75+
const isFirstRender = useFirstRender()
76+
useEffect(
77+
function announceListUpdates() {
78+
if (isFirstRender) return // ignore on first render as announceInitialFocus will also announce
79+
80+
liveRegion?.clear() // clear previous announcements
81+
82+
if (items.length === 0) {
83+
announce('No matching items.', {delayMs})
84+
return
85+
}
86+
87+
// give @primer/behaviors a moment to update active-descendant
88+
window.requestAnimationFrame(() => {
89+
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
90+
if (!activeItem) return
91+
const {index, text, selected} = activeItem
92+
93+
const announcementText = [
94+
`List updated`,
95+
`Focused item: ${text}`,
96+
`${selected ? 'selected' : 'not selected'}`,
97+
`${index + 1} of ${items.length}`,
98+
].join(', ')
99+
100+
announce(announcementText, {
101+
delayMs,
102+
from: liveRegion ? liveRegion : undefined, // announce will create a liveRegion if it doesn't find one
103+
})
104+
})
105+
},
106+
[isFirstRender, items, listContainerRef, liveRegion],
107+
)
108+
}

packages/react/src/SelectPanel/SelectPanel.test.tsx

Lines changed: 91 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import {render, screen} from '@testing-library/react'
1+
import {render, screen, waitFor} from '@testing-library/react'
22
import React from 'react'
33
import {SelectPanel, type SelectPanelProps} from '../SelectPanel'
44
import type {ItemInput, GroupedListProps} from '../deprecated/ActionList/List'
55
import {userEvent} from '@testing-library/user-event'
66
import ThemeProvider from '../ThemeProvider'
77
import {FeatureFlags} from '../FeatureFlags'
8+
import {getLiveRegion} from '../utils/testing'
89

910
const renderWithFlag = (children: React.ReactNode, flag: boolean) => {
1011
return render(
@@ -336,39 +337,39 @@ for (const useModernActionList of [false, true]) {
336337
})
337338
})
338339

339-
describe('filtering', () => {
340-
function FilterableSelectPanel() {
341-
const [selected, setSelected] = React.useState<SelectPanelProps['items']>([])
342-
const [filter, setFilter] = React.useState('')
343-
const [open, setOpen] = React.useState(false)
344-
345-
const onSelectedChange = (selected: SelectPanelProps['items']) => {
346-
setSelected(selected)
347-
}
340+
function FilterableSelectPanel() {
341+
const [selected, setSelected] = React.useState<SelectPanelProps['items']>([])
342+
const [filter, setFilter] = React.useState('')
343+
const [open, setOpen] = React.useState(false)
348344

349-
return (
350-
<ThemeProvider>
351-
<SelectPanel
352-
title="test title"
353-
subtitle="test subtitle"
354-
items={items.filter(item => item.text?.includes(filter))}
355-
placeholder="Select items"
356-
placeholderText="Filter items"
357-
selected={selected}
358-
onSelectedChange={onSelectedChange}
359-
filterValue={filter}
360-
onFilterChange={value => {
361-
setFilter(value)
362-
}}
363-
open={open}
364-
onOpenChange={isOpen => {
365-
setOpen(isOpen)
366-
}}
367-
/>
368-
</ThemeProvider>
369-
)
345+
const onSelectedChange = (selected: SelectPanelProps['items']) => {
346+
setSelected(selected)
370347
}
371348

349+
return (
350+
<ThemeProvider>
351+
<SelectPanel
352+
title="test title"
353+
subtitle="test subtitle"
354+
items={items.filter(item => item.text?.includes(filter))}
355+
placeholder="Select items"
356+
placeholderText="Filter items"
357+
selected={selected}
358+
onSelectedChange={onSelectedChange}
359+
filterValue={filter}
360+
onFilterChange={value => {
361+
setFilter(value)
362+
}}
363+
open={open}
364+
onOpenChange={isOpen => {
365+
setOpen(isOpen)
366+
}}
367+
/>
368+
</ThemeProvider>
369+
)
370+
}
371+
372+
describe('filtering', () => {
372373
it('should filter the list of items when the user types into the input', async () => {
373374
const user = userEvent.setup()
374375

@@ -381,10 +382,67 @@ for (const useModernActionList of [false, true]) {
381382
await user.type(document.activeElement!, 'two')
382383
expect(screen.getAllByRole('option')).toHaveLength(1)
383384
})
385+
})
386+
387+
describe('screen reader announcements', () => {
388+
// this is only implemented with the feature flag
389+
if (!useModernActionList) return
390+
391+
it('should announce initial focused item', async () => {
392+
const user = userEvent.setup()
393+
renderWithFlag(<FilterableSelectPanel />, useModernActionList)
394+
395+
await user.click(screen.getByText('Select items'))
396+
expect(screen.getByLabelText('Filter items')).toHaveFocus()
397+
398+
// we wait because announcement is intentionally updated after a timeout to not interrupt user input
399+
await waitFor(async () => {
400+
expect(getLiveRegion().getMessage('polite')).toBe(
401+
'Focus on filter text box and list of items, Focused item: item one, not selected, 1 of 3',
402+
)
403+
})
404+
})
405+
406+
it('should announce filtered results', async () => {
407+
const user = userEvent.setup()
408+
renderWithFlag(<FilterableSelectPanel />, useModernActionList)
409+
410+
await user.click(screen.getByText('Select items'))
411+
await user.type(document.activeElement!, 'o')
412+
expect(screen.getAllByRole('option')).toHaveLength(2)
413+
414+
await waitFor(
415+
async () => {
416+
expect(getLiveRegion().getMessage('polite')).toBe(
417+
'List updated, Focused item: item one, not selected, 1 of 2',
418+
)
419+
},
420+
{timeout: 3000}, // increased timeout because we don't want the test to compare with previous announcement
421+
)
422+
423+
await user.type(document.activeElement!, 'ne') // now: one
424+
expect(screen.getAllByRole('option')).toHaveLength(1)
425+
426+
await waitFor(async () => {
427+
expect(getLiveRegion().getMessage('polite')).toBe(
428+
'List updated, Focused item: item one, not selected, 1 of 1',
429+
)
430+
})
431+
})
384432

385-
it.todo('should announce the number of results')
433+
it('should announce when no results are available', async () => {
434+
const user = userEvent.setup()
435+
renderWithFlag(<FilterableSelectPanel />, useModernActionList)
386436

387-
it.todo('should announce when no results are available')
437+
await user.click(screen.getByText('Select items'))
438+
439+
await user.type(document.activeElement!, 'zero')
440+
expect(screen.queryByRole('option')).toBeNull()
441+
442+
await waitFor(async () => {
443+
expect(getLiveRegion().getMessage('polite')).toBe('No matching items.')
444+
})
445+
})
388446
})
389447

390448
describe('with footer', () => {

packages/react/src/SelectPanel/SelectPanel.tsx

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {FocusZoneHookSettings} from '../hooks/useFocusZone'
1717
import {useId} from '../hooks/useId'
1818
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
1919
import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion'
20+
import {useFeatureFlag} from '../FeatureFlags'
2021

2122
interface SelectPanelSingleSelection {
2223
selected: ItemInput | undefined
@@ -174,6 +175,8 @@ export function SelectPanel({
174175
}
175176
}, [inputLabel, textInputProps])
176177

178+
const usingModernActionList = useFeatureFlag('primer_react_select_panel_with_modern_action_list')
179+
177180
return (
178181
<LiveRegion>
179182
<AnchoredOverlay
@@ -192,15 +195,17 @@ export function SelectPanel({
192195
focusZoneSettings={focusZoneSettings}
193196
>
194197
<LiveRegionOutlet />
195-
<Message
196-
value={
197-
filterValue === ''
198-
? 'Showing all items'
199-
: items.length <= 0
200-
? 'No matching items'
201-
: `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
202-
}
203-
/>
198+
{usingModernActionList ? null : (
199+
<Message
200+
value={
201+
filterValue === ''
202+
? 'Showing all items'
203+
: items.length <= 0
204+
? 'No matching items'
205+
: `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
206+
}
207+
/>
208+
)}
204209
<Box sx={{display: 'flex', flexDirection: 'column', height: 'inherit', maxHeight: 'inherit'}}>
205210
<Box sx={{pt: 2, px: 3}}>
206211
<Heading as="h1" id={titleId} sx={{fontSize: 1}}>

packages/react/src/live-region/__tests__/Announce.test.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,7 @@
11
import {render, screen} from '@testing-library/react'
22
import React from 'react'
3-
import type {LiveRegionElement} from '@primer/live-region-element'
43
import {Announce} from '../Announce'
5-
6-
function getLiveRegion(): LiveRegionElement {
7-
const liveRegion = document.querySelector('live-region')
8-
if (liveRegion) {
9-
return liveRegion as LiveRegionElement
10-
}
11-
throw new Error('No live-region found')
12-
}
4+
import {getLiveRegion} from '../../utils/testing'
135

146
describe('Announce', () => {
157
beforeEach(() => {

packages/react/src/live-region/__tests__/AriaAlert.test.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,7 @@
11
import {render, screen} from '@testing-library/react'
22
import React from 'react'
3-
import type {LiveRegionElement} from '@primer/live-region-element'
43
import {AriaAlert} from '../AriaAlert'
5-
6-
function getLiveRegion(): LiveRegionElement {
7-
const liveRegion = document.querySelector('live-region')
8-
if (liveRegion) {
9-
return liveRegion as LiveRegionElement
10-
}
11-
throw new Error('No live-region found')
12-
}
4+
import {getLiveRegion} from '../../utils/testing'
135

146
describe('AriaAlert', () => {
157
beforeEach(() => {

0 commit comments

Comments
 (0)