Skip to content

Commit 1a30ee2

Browse files
siddharthkpsiddharthkp
authored
Merge branch 'main' into improve-ssr-theming
2 parents 54b3590 + 6cc9260 commit 1a30ee2

File tree

10 files changed

+48
-82
lines changed

10 files changed

+48
-82
lines changed

‎.changeset/drafts-actionlist-group-roles.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+
Better aria roles for ActionList group

‎src/ActionList2/ActionListContainerContext.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
/** This context can be used by components that compose ActionList inside a Menu */
22

33
import React from 'react'
4+
import {AriaRole} from '../utils/types'
45

56
type ContextProps = {
67
container?: string
7-
listRole?: string
8+
listRole?: AriaRole
89
selectionVariant?: 'single' | 'multiple' // TODO: Remove after DropdownMenu2 deprecation
910
selectionAttribute?: 'aria-selected' | 'aria-checked'
1011
listLabelledBy?: string

‎src/ActionList2/Group.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@ export const Group: React.FC<GroupProps> = ({
4545
...props
4646
}) => {
4747
const labelId = useSSRSafeId()
48+
const {role: listRole} = React.useContext(ListContext)
4849

4950
return (
5051
<Box
5152
as="li"
53+
role="none"
5254
sx={{
5355
'&:not(:first-child)': {marginTop: 2},
5456
listStyle: 'none', // hide the ::marker inserted by browser's stylesheet
@@ -58,7 +60,12 @@ export const Group: React.FC<GroupProps> = ({
5860
>
5961
{title && <Header title={title} variant={variant} auxiliaryText={auxiliaryText} labelId={labelId} />}
6062
<GroupContext.Provider value={{selectionVariant}}>
61-
<Box as="ul" sx={{paddingInlineStart: 0}} aria-labelledby={title ? labelId : undefined} role={role}>
63+
<Box
64+
as="ul"
65+
sx={{paddingInlineStart: 0}}
66+
aria-labelledby={title ? labelId : undefined}
67+
role={role || (listRole && 'group')}
68+
>
6269
{props.children}
6370
</Box>
6471
</GroupContext.Provider>

‎src/ActionList2/Item.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
104104
): JSX.Element => {
105105
const {variant: listVariant, showDividers, selectionVariant: listSelectionVariant} = React.useContext(ListContext)
106106
const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext)
107-
const {container, afterSelect, selectionAttribute = 'aria-selected'} = React.useContext(ActionListContainerContext)
107+
const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext)
108108

109109
let selectionVariant: ListProps['selectionVariant'] | GroupProps['selectionVariant']
110110
if (typeof groupSelectionVariant !== 'undefined') selectionVariant = groupSelectionVariant
@@ -227,7 +227,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
227227
aria-labelledby={`${labelId} ${slots.InlineDescription ? inlineDescriptionId : ''}`}
228228
aria-describedby={slots.BlockDescription ? blockDescriptionId : undefined}
229229
role={role || itemRole}
230-
{...{[selectionAttribute]: selected}}
230+
{...(selectionAttribute && {[selectionAttribute]: selected})}
231231
{...props}
232232
>
233233
<ItemWrapper>

‎src/ActionList2/List.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export type ListProps = {
2424
role?: AriaRole
2525
} & SxProp
2626

27-
type ContextProps = Pick<ListProps, 'variant' | 'selectionVariant' | 'showDividers'>
27+
type ContextProps = Pick<ListProps, 'variant' | 'selectionVariant' | 'showDividers' | 'role'>
2828
export const ListContext = React.createContext<ContextProps>({})
2929

3030
const ListBox = styled.ul<SxProp>(sx)
@@ -56,7 +56,12 @@ export const List = React.forwardRef<HTMLUListElement, ListProps>(
5656
ref={forwardedRef}
5757
>
5858
<ListContext.Provider
59-
value={{variant, selectionVariant: selectionVariant || containerSelectionVariant, showDividers}}
59+
value={{
60+
variant,
61+
selectionVariant: selectionVariant || containerSelectionVariant,
62+
showDividers,
63+
role: role || listRole
64+
}}
6065
>
6166
{props.children}
6267
</ListContext.Provider>

‎src/__tests__/ActionList2.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ function SingleSelectListStory(): JSX.Element {
4242
key={index}
4343
role="option"
4444
selected={index === selectedIndex}
45+
aria-selected={index === selectedIndex}
4546
onSelect={() => setSelectedIndex(index)}
4647
disabled={project.disabled}
4748
>

‎src/stories/ActionList2/examples.stories.tsx‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,14 @@ export function Groups(): JSX.Element {
190190

191191
<p>Grouped content with labels and description. This patterns appears in pull requests to pick a reviewer.</p>
192192

193-
<ActionList selectionVariant="multiple" showDividers aria-label="Select reviewers">
194-
<ActionList.Group title="Suggestions" role="listbox">
193+
<ActionList selectionVariant="multiple" role="menu" showDividers aria-label="Select reviewers">
194+
<ActionList.Group title="Suggestions">
195195
{users.slice(0, 2).map(user => (
196196
<ActionList.Item
197197
key={user.login}
198-
role="option"
198+
role="menuitemcheckbox"
199199
selected={Boolean(assignees.find(assignee => assignee.login === user.login))}
200+
aria-checked={Boolean(assignees.find(assignee => assignee.login === user.login))}
200201
onSelect={() => toggleAssignee(user)}
201202
>
202203
<ActionList.LeadingVisual>
@@ -208,12 +209,13 @@ export function Groups(): JSX.Element {
208209
</ActionList.Item>
209210
))}
210211
</ActionList.Group>
211-
<ActionList.Group title="Everyone" role="listbox">
212+
<ActionList.Group title="Everyone">
212213
{users.slice(2).map(user => (
213214
<ActionList.Item
214-
role="option"
215+
role="menuitemcheckbox"
215216
key={user.login}
216217
selected={Boolean(assignees.find(assignee => assignee.login === user.login))}
218+
aria-checked={Boolean(assignees.find(assignee => assignee.login === user.login))}
217219
onSelect={() => toggleAssignee(user)}
218220
>
219221
<ActionList.LeadingVisual>

‎src/stories/ActionList2/fixtures.stories.tsx‎

Lines changed: 7 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,13 @@ export function DisabledStory(): JSX.Element {
213213
<>
214214
<h1>Disabled Items</h1>
215215
<ErsatzOverlay>
216-
<ActionList selectionVariant="single" showDividers role="listbox" aria-label="Select a project">
216+
<ActionList selectionVariant="single" showDividers role="menu" aria-label="Select a project">
217217
{projects.map((project, index) => (
218218
<ActionList.Item
219219
key={index}
220-
role="option"
220+
role="menuitemradio"
221221
selected={index === selectedIndex}
222+
aria-checked={index === selectedIndex}
222223
onSelect={() => setSelectedIndex(index)}
223224
disabled={index === 1}
224225
>
@@ -236,61 +237,6 @@ export function DisabledStory(): JSX.Element {
236237
}
237238
DisabledStory.storyName = 'Disabled Items'
238239

239-
export function GroupsStory(): JSX.Element {
240-
const [assignees, setAssignees] = React.useState(users.slice(0, 1))
241-
242-
const toggleAssignee = (assignee: typeof users[number]) => {
243-
const assigneeIndex = assignees.findIndex(a => a.login === assignee.login)
244-
245-
if (assigneeIndex === -1) setAssignees([...assignees, assignee])
246-
else setAssignees(assignees.filter((_, index) => index !== assigneeIndex))
247-
}
248-
249-
return (
250-
<>
251-
<h1>Groups</h1>
252-
<ErsatzOverlay>
253-
<ActionList selectionVariant="multiple" showDividers aria-label="Select reviewers">
254-
<ActionList.Group title="Suggestions" variant="filled" role="listbox">
255-
{users.slice(0, 2).map(user => (
256-
<ActionList.Item
257-
key={user.login}
258-
role="option"
259-
selected={Boolean(assignees.find(assignee => assignee.login === user.login))}
260-
onSelect={() => toggleAssignee(user)}
261-
>
262-
<ActionList.LeadingVisual>
263-
<Avatar src={`https://github.com/${user.login}.png`} />
264-
</ActionList.LeadingVisual>
265-
{user.login}
266-
<ActionList.Description>{user.name}</ActionList.Description>
267-
<ActionList.Description variant="block">Recently edited these files</ActionList.Description>
268-
</ActionList.Item>
269-
))}
270-
</ActionList.Group>
271-
<ActionList.Group title="Everyone" variant="filled" role="listbox">
272-
{users.slice(2).map(user => (
273-
<ActionList.Item
274-
role="option"
275-
key={user.login}
276-
selected={Boolean(assignees.find(assignee => assignee.login === user.login))}
277-
onSelect={() => toggleAssignee(user)}
278-
>
279-
<ActionList.LeadingVisual>
280-
<Avatar src={`https://github.com/${user.login}.png`} />
281-
</ActionList.LeadingVisual>
282-
{user.login}
283-
<ActionList.Description>{user.name}</ActionList.Description>
284-
</ActionList.Item>
285-
))}
286-
</ActionList.Group>
287-
</ActionList>
288-
</ErsatzOverlay>
289-
</>
290-
)
291-
}
292-
GroupsStory.storyName = 'Groups'
293-
294240
export function ActionsStory(): JSX.Element {
295241
return (
296242
<>
@@ -1007,20 +953,19 @@ export function MemexSortable(): JSX.Element {
1007953
<h1>Memex Sortable List</h1>
1008954
<ErsatzOverlay>
1009955
<DndProvider backend={HTML5Backend}>
1010-
<ActionList selectionVariant="multiple">
1011-
<ActionList.Group title="Visible fields (can be reordered)" role="listbox">
956+
<ActionList selectionVariant="multiple" role="menu">
957+
<ActionList.Group title="Visible fields (can be reordered)">
1012958
{visibleOptions.map(option => (
1013959
<SortableItem
1014960
key={option.text}
1015-
role="option"
961+
role="menuitemcheckbox"
1016962
option={option}
1017963
onSelect={() => toggle(option.text)}
1018964
reorder={reorder}
1019965
/>
1020966
))}
1021967
</ActionList.Group>
1022968
<ActionList.Group
1023-
role="listbox"
1024969
title="Hidden fields"
1025970
selectionVariant={
1026971
/** selectionVariant override on Group: disable selection if there are no options */
@@ -1030,7 +975,7 @@ export function MemexSortable(): JSX.Element {
1030975
{hiddenOptions.map((option, index) => (
1031976
<ActionList.Item
1032977
key={index}
1033-
role="option"
978+
role="menuitemcheckbox"
1034979
selected={option.selected}
1035980
onSelect={() => toggle(option.text)}
1036981
>

‎src/stories/ActionMenu2/examples.stories.tsx‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ export function GroupsAndDescription(): JSX.Element {
171171
Milestone
172172
</ActionMenu.Button>
173173
<ActionMenu.Overlay width="medium">
174-
<ActionList selectionVariant="single" showDividers role="none">
175-
<ActionList.Group title="Open" role="menu">
174+
<ActionList selectionVariant="single" showDividers>
175+
<ActionList.Group title="Open">
176176
{milestones
177177
.filter(milestone => !milestone.name.includes('21'))
178178
.map((milestone, index) => (
@@ -189,7 +189,7 @@ export function GroupsAndDescription(): JSX.Element {
189189
</ActionList.Item>
190190
))}
191191
</ActionList.Group>
192-
<ActionList.Group title="Closed" role="menu">
192+
<ActionList.Group title="Closed">
193193
{milestones
194194
.filter(milestone => milestone.name.includes('21'))
195195
.map((milestone, index) => (
@@ -270,7 +270,6 @@ export function MultipleSelection(): JSX.Element {
270270
<ActionList selectionVariant="multiple" showDividers>
271271
{users.map(user => (
272272
<ActionList.Item
273-
role="option"
274273
key={user.login}
275274
selected={Boolean(assignees.find(assignee => assignee.login === user.login))}
276275
onSelect={() => toggleAssignee(user)}
@@ -321,8 +320,8 @@ export function MixedSelection(): JSX.Element {
321320
{selectedOption ? `Group by ${selectedOption.text}` : 'Group items by'}
322321
</ActionMenu.Button>
323322
<ActionMenu.Overlay width="medium">
324-
<ActionList role="none">
325-
<ActionList.Group selectionVariant="single" title="Group by" role="menu">
323+
<ActionList>
324+
<ActionList.Group selectionVariant="single" title="Group by">
326325
{options.map((option, index) => (
327326
<ActionList.Item
328327
key={index}
@@ -337,9 +336,9 @@ export function MixedSelection(): JSX.Element {
337336
))}
338337
</ActionList.Group>
339338
{typeof selectedIndex === 'number' && (
340-
<ActionList.Group role="menu">
339+
<ActionList.Group>
341340
<ActionList.Divider />
342-
<ActionList.Item onSelect={() => setSelectedIndex(null)} role="menuitem">
341+
<ActionList.Item onSelect={() => setSelectedIndex(null)}>
343342
<ActionList.LeadingVisual>
344343
<XIcon />
345344
</ActionList.LeadingVisual>

‎src/utils/testing.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,8 @@ export function checkStoriesForAxeViolations(name: string) {
254254
Object.values(Stories).map(Story => {
255255
if (typeof Story !== 'function') return
256256

257-
it(`story ${(Story as StoryType).storyName} should have no axe violations`, async () => {
257+
const {storyName, name: StoryFunctionName} = Story as StoryType
258+
it(`story ${storyName || StoryFunctionName} should have no axe violations`, async () => {
258259
const {container} = HTMLRender(<Story />)
259260
const results = await axe(container)
260261
expect(results).toHaveNoViolations()

0 commit comments

Comments
 (0)