Skip to content

Commit 68c4980

Browse files
Revert "SelectPanel: Accessibility remediation (#2138)" (#2338)
* Revert "SelectPanel: Accessibility remediation (#2138)" This reverts commit ace38af. * add changeset * Update .changeset/stupid-knives-arrive.md Co-authored-by: Brendan Forster <[email protected]> Co-authored-by: Brendan Forster <[email protected]>
1 parent e052644 commit 68c4980

File tree

20 files changed

+2553
-2705
lines changed

20 files changed

+2553
-2705
lines changed

.changeset/stupid-knives-arrive.md

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+
Reverted SelectPanel breaking behavioral changes

docs/content/SelectPanel.mdx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,28 +37,27 @@ const items = [
3737
]
3838

3939
function DemoComponent() {
40-
const [selected, setSelected] = React.useState([items[2], items[4]])
40+
const [selected, setSelected] = React.useState([items[0], items[1]])
4141
const [filter, setFilter] = React.useState('')
4242
const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
4343
const [open, setOpen] = React.useState(false)
4444

4545
return (
4646
<SelectPanel
47-
renderAnchor={({...anchorProps}) => (
48-
<Button trailingIcon={TriangleDownIcon} {...anchorProps}>
49-
Select Labels
47+
renderAnchor={({children, 'aria-labelledby': ariaLabelledBy, ...anchorProps}) => (
48+
<Button trailingIcon={TriangleDownIcon} aria-labelledby={` ${ariaLabelledBy}`} {...anchorProps}>
49+
{children || 'Select Labels'}
5050
</Button>
5151
)}
52-
title="Select Items"
53-
inputLabel="Select Items"
52+
placeholderText="Filter Labels"
5453
open={open}
5554
onOpenChange={setOpen}
5655
items={filteredItems}
5756
selected={selected}
5857
onSelectedChange={setSelected}
5958
onFilterChange={setFilter}
6059
showItemDividers={true}
61-
overlayProps={{width: 'medium', height: 'medium'}}
60+
overlayProps={{width: 'small', height: 'xsmall'}}
6261
/>
6362
)
6463
}

docs/content/deprecated/ActionList.mdx

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,25 @@ Use [new version of ActionList](/ActionList) with composable API, design updates
1414

1515
```jsx
1616
<ActionList
17-
role="listbox"
1817
items={[
19-
{text: 'New file', role: 'option'},
20-
{text: 'Copy link', role: 'option'},
21-
{text: 'Edit file', role: 'option'},
18+
{text: 'New file'},
19+
{text: 'Copy link'},
20+
{text: 'Edit file'},
2221
ActionList.Divider,
23-
{text: 'Delete file', variant: 'danger', role: 'option'}
22+
{text: 'Delete file', variant: 'danger'}
2423
]}
2524
/>
2625
```
2726

2827
**After**
2928

3029
```jsx
31-
<ActionList role="listbox">
32-
<ActionList.Item role="option">New file</ActionList.Item>
33-
<ActionList.Item role="option">Copy link</ActionList.Item>
34-
<ActionList.Item role="option">Edit file</ActionList.Item>
30+
<ActionList>
31+
<ActionList.Item>New file</ActionList.Item>
32+
<ActionList.Item>Copy link</ActionList.Item>
33+
<ActionList.Item>Edit file</ActionList.Item>
3534
<ActionList.Divider />
36-
<ActionList.Item variant="danger" role="option">Delete file</ActionList.Item>
35+
<ActionList.Item variant="danger">Delete file</ActionList.Item>
3736
</ActionList>
3837
```
3938

@@ -47,13 +46,12 @@ import {ActionList} from '@primer/react/deprecated'
4746

4847
```jsx live deprecated
4948
<ActionList
50-
role="listbox"
5149
items={[
52-
{text: 'New file', role: 'option'},
50+
{text: 'New file'},
5351
ActionList.Divider,
54-
{text: 'Copy link', role: 'option'},
55-
{text: 'Edit file', role: 'option'},
56-
{text: 'Delete file', variant: 'danger', role: 'option'}
52+
{text: 'Copy link'},
53+
{text: 'Edit file'},
54+
{text: 'Delete file', variant: 'danger'}
5755
]}
5856
/>
5957
```
@@ -62,7 +60,6 @@ import {ActionList} from '@primer/react/deprecated'
6260

6361
```jsx live deprecated
6462
<ActionList
65-
role="listbox"
6663
groupMetadata={[
6764
{groupId: '0'},
6865
{groupId: '1', header: {title: 'Live query', variant: 'subtle'}},
@@ -71,37 +68,34 @@ import {ActionList} from '@primer/react/deprecated'
7168
{groupId: '4'}
7269
]}
7370
items={[
74-
{key: '1', leadingVisual: TypographyIcon, text: 'Rename', groupId: '0', trailingVisual: '⌘R', role: 'option'},
75-
{key: '2', leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0', role: 'option'},
76-
{key: '3', leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1', role: 'option'},
71+
{key: '1', leadingVisual: TypographyIcon, text: 'Rename', groupId: '0', trailingVisual: '⌘R'},
72+
{key: '2', leadingVisual: VersionsIcon, text: 'Duplicate', groupId: '0'},
73+
{key: '3', leadingVisual: SearchIcon, text: 'repo:github/github', groupId: '1'},
7774
{
7875
key: '4',
7976
leadingVisual: NoteIcon,
8077
text: 'Table',
8178
description: 'Information-dense table optimized for operations across teams',
8279
descriptionVariant: 'block',
83-
groupId: '2',
84-
role: 'option'
80+
groupId: '2'
8581
},
8682
{
8783
key: '5',
8884
leadingVisual: ProjectIcon,
8985
text: 'Board',
9086
description: 'Kanban-style board focused on visual states',
9187
descriptionVariant: 'block',
92-
groupId: '2',
93-
role: 'option'
88+
groupId: '2'
9489
},
9590
{
9691
key: '6',
9792
leadingVisual: FilterIcon,
9893
text: 'Save sort and filters to current view',
9994
disabled: true,
100-
groupId: '3',
101-
role: 'option'
95+
groupId: '3'
10296
},
103-
{key: '7', leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3', role: 'option'},
104-
{key: '8', leadingVisual: GearIcon, text: 'View settings', groupId: '4', trailingVisual: ArrowRightIcon, role: 'option'}
97+
{key: '7', leadingVisual: FilterIcon, text: 'Save sort and filters to new view', groupId: '3'},
98+
{key: '8', leadingVisual: GearIcon, text: 'View settings', groupId: '4', trailingVisual: ArrowRightIcon}
10599
]}
106100
/>
107101
```
@@ -110,21 +104,17 @@ import {ActionList} from '@primer/react/deprecated'
110104

111105
```jsx deprecated
112106
<ActionList
113-
role="listbox"
114107
items={[
115108
{
116109
text: 'Vanilla link',
117-
role: 'option',
118110
renderItem: props => <ActionList.Item as="a" href="/about" {...props} />
119111
},
120112
{
121113
text: 'React Router link',
122-
role: 'option',
123114
renderItem: props => <ActionList.Item as={ReactRouterLikeLink} to="/about" {...props} />
124115
},
125116
{
126117
text: 'NextJS style',
127-
role: 'option',
128118
renderItem: props => (
129119
<NextJSLikeLink href="/about">
130120
<ActionList.Item as="a" {...props} />

src/Autocomplete/AutocompleteMenu.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
153153
items.map(selectableItem => {
154154
return {
155155
...selectableItem,
156-
_legacyEnterSupport: true, //TODO: Change behaviour, the enter key should not be used here.
157156
role: 'option',
158157
id: selectableItem.id,
159158
selected: selectionVariant === 'multiple' ? selectedItemIds.includes(selectableItem.id) : undefined,
@@ -220,7 +219,6 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
220219
? [
221220
{
222221
...addNewItem,
223-
_legacyEnterSupport: true, //TODO: Change behaviour, the enter key should not be used here.
224222
leadingVisual: () => <PlusIcon />,
225223
onAction: (item: T) => {
226224
// TODO: make it possible to pass a leadingVisual when using `addNewItem`

src/FilteredActionList/FilteredActionList.tsx

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import TextInput, {TextInputProps} from '../TextInput'
55
import Box from '../Box'
66
import {ActionList} from '../deprecated/ActionList'
77
import Spinner from '../Spinner'
8+
import {useFocusZone} from '../hooks/useFocusZone'
89
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
910
import styled from 'styled-components'
1011
import {get} from '../constants'
@@ -13,7 +14,6 @@ import useScrollFlash from '../hooks/useScrollFlash'
1314
import {scrollIntoView} from '@primer/behaviors'
1415
import type {ScrollIntoViewOptions} from '@primer/behaviors'
1516
import {SxProp} from '../sx'
16-
import VisuallyHidden from '../_VisuallyHidden'
1717

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

@@ -22,7 +22,7 @@ export interface FilteredActionListProps
2222
ListPropsBase,
2323
SxProp {
2424
loading?: boolean
25-
placeholderText?: string
25+
placeholderText: string
2626
filterValue?: string
2727
onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
2828
textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
@@ -56,10 +56,10 @@ export function FilteredActionList({
5656
)
5757

5858
const scrollContainerRef = useRef<HTMLDivElement>(null)
59+
const listContainerRef = useRef<HTMLDivElement>(null)
5960
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
6061
const activeDescendantRef = useRef<HTMLElement>()
6162
const listId = useSSRSafeId()
62-
const inputDescriptionTextId = useSSRSafeId()
6363
const onInputKeyPress: KeyboardEventHandler = useCallback(
6464
event => {
6565
if (event.key === 'Enter' && activeDescendantRef.current) {
@@ -74,6 +74,28 @@ export function FilteredActionList({
7474
[activeDescendantRef]
7575
)
7676

77+
useFocusZone(
78+
{
79+
containerRef: listContainerRef,
80+
focusOutBehavior: 'wrap',
81+
focusableElementFilter: element => {
82+
return !(element instanceof HTMLInputElement)
83+
},
84+
activeDescendantFocus: inputRef,
85+
onActiveDescendantChanged: (current, previous, directlyActivated) => {
86+
activeDescendantRef.current = current
87+
88+
if (current && scrollContainerRef.current && directlyActivated) {
89+
scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
90+
}
91+
}
92+
},
93+
[
94+
// List ref isn't set while loading. Need to re-bind focus zone when it changes
95+
loading
96+
]
97+
)
98+
7799
useEffect(() => {
78100
// if items changed, we want to instantly move active descendant into view
79101
if (activeDescendantRef.current && scrollContainerRef.current) {
@@ -97,18 +119,16 @@ export function FilteredActionList({
97119
placeholder={placeholderText}
98120
aria-label={placeholderText}
99121
aria-controls={listId}
100-
aria-describedby={inputDescriptionTextId}
101122
{...textInputProps}
102123
/>
103-
<VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
104124
</StyledHeader>
105125
<Box ref={scrollContainerRef} overflow="auto">
106126
{loading ? (
107127
<Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
108128
<Spinner />
109129
</Box>
110130
) : (
111-
<ActionList items={items} {...listProps} role="listbox" id={listId} />
131+
<ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
112132
)}
113133
</Box>
114134
</Box>

0 commit comments

Comments
 (0)