Skip to content

Commit

Permalink
Merge 0008445 into a79aeee
Browse files Browse the repository at this point in the history
  • Loading branch information
siddharthkp committed Sep 12, 2024
2 parents a79aeee + 0008445 commit 9592e0f
Show file tree
Hide file tree
Showing 9 changed files with 223 additions and 69 deletions.
5 changes: 5 additions & 0 deletions .changeset/small-melons-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

SelectPanel: Add announcements for screen readers (behind feature flag `primer_react_select_panel_with_modern_action_list`)
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {SxProp} from '../sx'
import {isValidElementType} from 'react-is'
import type {RenderItemFn} from '../deprecated/ActionList/List'
import {CircleSlashIcon} from '@primer/octicons-react'
import {useAnnouncements} from './useAnnouncements'

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

Expand Down Expand Up @@ -115,6 +116,7 @@ export function FilteredActionList({
}, [items])

useScrollFlash(scrollContainerRef)
useAnnouncements(items, listContainerRef, inputRef)

function getItemListForEachGroup(groupId: string) {
const itemsInGroup = []
Expand Down
97 changes: 97 additions & 0 deletions packages/react/src/FilteredActionList/useAnnouncements.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Announcements for FilteredActionList (and SelectPanel) based
// on https://github.com/github/multi-select-user-testing

import {announce} from '@primer/live-region-element'
import {useEffect, useRef} from 'react'
import type {FilteredActionListProps} from './FilteredActionListEntry'

// we add a delay so that it does not interrupt default screen reader announcement and queues after it
const delayMs = 500

const useFirstRender = () => {
const firstRender = useRef(true)
useEffect(() => {
firstRender.current = false
}, [])
return firstRender.current
}

const getItemWithActiveDescendant = (
listRef: React.RefObject<HTMLUListElement>,
items: FilteredActionListProps['items'],
) => {
const listElement = listRef.current
const activeItemElement = listElement?.querySelector('[data-is-active-descendant]')

if (!listElement || !activeItemElement?.textContent) return

const optionElements = listElement.querySelectorAll('[role="option"]')

const index = Array.from(optionElements).indexOf(activeItemElement)
const activeItem = items[index]

const text = activeItem.text
const selected = activeItem.selected

return {index, text, selected}
}

export const useAnnouncements = (
items: FilteredActionListProps['items'],
listContainerRef: React.RefObject<HTMLUListElement>,
inputRef: React.RefObject<HTMLInputElement>,
) => {
useEffect(
function announceInitialFocus() {
const focusHandler = () => {
// give @primer/behaviors a moment to apply active-descendant
window.requestAnimationFrame(() => {
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
if (!activeItem) return
const {index, text, selected} = activeItem

const announcementText = [
`Focus on filter text box and list of labels`,
`Focused item: ${text}`,
`${selected ? 'selected' : 'not selected'}`,
`${index + 1} of ${items.length}`,
].join(', ')
announce(announcementText, {delayMs})
})
}

const inputElement = inputRef.current
inputElement?.addEventListener('focus', focusHandler)
return () => inputElement?.removeEventListener('focus', focusHandler)
},
[listContainerRef, inputRef, items],
)

const isFirstRender = useFirstRender()
useEffect(
function announceListUpdates() {
if (isFirstRender) return // ignore on first render as announceInitialFocus will also announce

if (items.length === 0) {
announce('No matching items.', {delayMs})
return
}

// give @primer/behaviors a moment to update active-descendant
window.requestAnimationFrame(() => {
const activeItem = getItemWithActiveDescendant(listContainerRef, items)
if (!activeItem) return
const {index, text, selected} = activeItem

const announcementText = [
`List updated`,
`Focused item: ${text}`,
`${selected ? 'selected' : 'not selected'}`,
`${index} of ${items.length}`,
].join(', ')
announce(announcementText, {delayMs})
})
},
[listContainerRef, inputRef, items, isFirstRender],
)
}
125 changes: 92 additions & 33 deletions packages/react/src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {render, screen} from '@testing-library/react'
import {render, screen, waitFor} from '@testing-library/react'
import React from 'react'
import {SelectPanel, type SelectPanelProps} from '../SelectPanel'
import type {ItemInput, GroupedListProps} from '../deprecated/ActionList/List'
import {userEvent} from '@testing-library/user-event'
import ThemeProvider from '../ThemeProvider'
import {FeatureFlags} from '../FeatureFlags'
import {getLiveRegion} from '../utils/testing'

const renderWithFlag = (children: React.ReactNode, flag: boolean) => {
return render(
Expand Down Expand Up @@ -336,39 +337,39 @@ for (const useModernActionList of [false, true]) {
})
})

describe('filtering', () => {
function FilterableSelectPanel() {
const [selected, setSelected] = React.useState<SelectPanelProps['items']>([])
const [filter, setFilter] = React.useState('')
const [open, setOpen] = React.useState(false)

const onSelectedChange = (selected: SelectPanelProps['items']) => {
setSelected(selected)
}
function FilterableSelectPanel() {
const [selected, setSelected] = React.useState<SelectPanelProps['items']>([])
const [filter, setFilter] = React.useState('')
const [open, setOpen] = React.useState(false)

return (
<ThemeProvider>
<SelectPanel
title="test title"
subtitle="test subtitle"
items={items.filter(item => item.text?.includes(filter))}
placeholder="Select items"
placeholderText="Filter items"
selected={selected}
onSelectedChange={onSelectedChange}
filterValue={filter}
onFilterChange={value => {
setFilter(value)
}}
open={open}
onOpenChange={isOpen => {
setOpen(isOpen)
}}
/>
</ThemeProvider>
)
const onSelectedChange = (selected: SelectPanelProps['items']) => {
setSelected(selected)
}

return (
<ThemeProvider>
<SelectPanel
title="test title"
subtitle="test subtitle"
items={items.filter(item => item.text?.includes(filter))}
placeholder="Select items"
placeholderText="Filter items"
selected={selected}
onSelectedChange={onSelectedChange}
filterValue={filter}
onFilterChange={value => {
setFilter(value)
}}
open={open}
onOpenChange={isOpen => {
setOpen(isOpen)
}}
/>
</ThemeProvider>
)
}

describe('filtering', () => {
it('should filter the list of items when the user types into the input', async () => {
const user = userEvent.setup()

Expand All @@ -381,10 +382,68 @@ for (const useModernActionList of [false, true]) {
await user.type(document.activeElement!, 'two')
expect(screen.getAllByRole('option')).toHaveLength(1)
})
})

describe('screen reader announcements', () => {
// this is only implemented with the feature flag
if (!useModernActionList) return

it('should announce initial focused item', async () => {
const user = userEvent.setup()
renderWithFlag(<FilterableSelectPanel />, useModernActionList)

await user.click(screen.getByText('Select items'))
expect(screen.getByLabelText('Filter items')).toHaveFocus()

// we wait because announcement is intentionally updated after a timeout to not interrupt user input
await waitFor(async () => {
expect(getLiveRegion().getMessage('polite')).toBe(
'Focus on filter text box and list of labels, Focused item: item one, not selected, 1 of 3',
)
})
})

it('should announce filtered results', async () => {
const user = userEvent.setup()
renderWithFlag(<FilterableSelectPanel />, useModernActionList)

await user.click(screen.getByText('Select items'))
await user.type(document.activeElement!, 'o')
expect(screen.getAllByRole('option')).toHaveLength(2)

await waitFor(
async () => {
expect(getLiveRegion().getMessage('polite')).toBe(
'List updated, Focused item: item one, not selected, 1 of 2',
)
},
{timeout: 3000}, // increased timeout because we don't want the test to compare with previous announcement
)

await user.type(document.activeElement!, 'ne') // now: one
expect(screen.getAllByRole('option')).toHaveLength(1)

await waitFor(async () => {
expect(getLiveRegion().getMessage('polite')).toBe(
'List updated, Focused item: item one, not selected, 1 of 1',
)
})
})

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

it.todo('should announce when no results are available')
await user.click(screen.getByText('Select items'))

await user.type(document.activeElement!, 'zero')
expect(screen.queryByRole('option')).toBeNull()
expect(screen.getByText('No matches')).toBeVisible()

await waitFor(async () => {
expect(getLiveRegion().getMessage('polite')).toBe('No matching items.')
})
})
})

describe('with footer', () => {
Expand Down
24 changes: 15 additions & 9 deletions packages/react/src/SelectPanel/SelectPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {FocusZoneHookSettings} from '../hooks/useFocusZone'
import {useId} from '../hooks/useId'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion'
import {useFeatureFlag} from '../FeatureFlags'

interface SelectPanelSingleSelection {
selected: ItemInput | undefined
Expand Down Expand Up @@ -174,6 +175,8 @@ export function SelectPanel({
}
}, [inputLabel, textInputProps])

const usingModernActionList = useFeatureFlag('primer_react_select_panel_with_modern_action_list')

return (
<LiveRegion>
<AnchoredOverlay
Expand All @@ -192,15 +195,18 @@ export function SelectPanel({
focusZoneSettings={focusZoneSettings}
>
<LiveRegionOutlet />
<Message
value={
filterValue === ''
? 'Showing all items'
: items.length <= 0
? 'No matching items'
: `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
}
/>
{usingModernActionList ? null : (
<Message
value={
filterValue === ''
? 'Showing all items'
: items.length <= 0
? 'No matching items'
: `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`
}
/>
)}

<Box sx={{display: 'flex', flexDirection: 'column', height: 'inherit', maxHeight: 'inherit'}}>
<Box sx={{pt: 2, px: 3}}>
<Heading as="h1" id={titleId} sx={{fontSize: 1}}>
Expand Down
10 changes: 1 addition & 9 deletions packages/react/src/live-region/__tests__/Announce.test.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
import {render, screen} from '@testing-library/react'
import React from 'react'
import type {LiveRegionElement} from '@primer/live-region-element'
import {Announce} from '../Announce'

function getLiveRegion(): LiveRegionElement {
const liveRegion = document.querySelector('live-region')
if (liveRegion) {
return liveRegion as LiveRegionElement
}
throw new Error('No live-region found')
}
import {getLiveRegion} from '../../utils/testing'

describe('Announce', () => {
beforeEach(() => {
Expand Down
10 changes: 1 addition & 9 deletions packages/react/src/live-region/__tests__/AriaAlert.test.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
import {render, screen} from '@testing-library/react'
import React from 'react'
import type {LiveRegionElement} from '@primer/live-region-element'
import {AriaAlert} from '../AriaAlert'

function getLiveRegion(): LiveRegionElement {
const liveRegion = document.querySelector('live-region')
if (liveRegion) {
return liveRegion as LiveRegionElement
}
throw new Error('No live-region found')
}
import {getLiveRegion} from '../../utils/testing'

describe('AriaAlert', () => {
beforeEach(() => {
Expand Down
10 changes: 1 addition & 9 deletions packages/react/src/live-region/__tests__/AriaStatus.test.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
import {render, screen} from '@testing-library/react'
import React from 'react'
import type {LiveRegionElement} from '@primer/live-region-element'
import {AriaStatus} from '../AriaStatus'
import {userEvent} from '@testing-library/user-event'

function getLiveRegion(): LiveRegionElement {
const liveRegion = document.querySelector('live-region')
if (liveRegion) {
return liveRegion as LiveRegionElement
}
throw new Error('No live-region found')
}
import {getLiveRegion} from '../../utils/testing'

describe('AriaStatus', () => {
beforeEach(() => {
Expand Down
9 changes: 9 additions & 0 deletions packages/react/src/utils/testing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import axe from 'axe-core'
import customRules from '@github/axe-github'
import {ThemeProvider} from '..'
import {default as defaultTheme} from '../theme'
import type {LiveRegionElement} from '@primer/live-region-element'

type ComputedStyles = Record<string, string | Record<string, string>>

Expand Down Expand Up @@ -270,3 +271,11 @@ export function checkStoriesForAxeViolations(name: string, storyDir?: string) {
})
})
}

export function getLiveRegion(): LiveRegionElement {
const liveRegion = document.querySelector('live-region')
if (liveRegion) {
return liveRegion as LiveRegionElement
}
throw new Error('No live-region found')
}

0 comments on commit 9592e0f

Please sign in to comment.