Skip to content

Commit f4057b8

Browse files
siddharthkpsiddharthkp
authored
different visual styles for keyboard navigation (#1603)
1 parent a2e195b commit f4057b8

File tree

3 files changed

+82
-57
lines changed

3 files changed

+82
-57
lines changed

‎.changeset/actionlist-keyboard-a11y.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/components': patch
3+
---
4+
5+
ActionList: Improve keyboard accessibility with focus styles cross browser

‎src/ActionList2/Item.tsx‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
9494
onSelect = () => null,
9595
sx: sxProp = {},
9696
id,
97-
_PrivateItemWrapper = ({children}) => <>{children}</>,
97+
_PrivateItemWrapper,
9898
...props
9999
},
100100
forwardedRef
@@ -109,9 +109,9 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
109109
fontSize: 1,
110110
paddingY: '6px', // custom value off the scale
111111
lineHeight: TEXT_ROW_HEIGHT,
112-
marginX: listVariant === 'inset' ? 2 : 0,
113112
minHeight: 5,
114-
borderRadius: 2,
113+
marginX: listVariant === 'inset' ? 2 : 0,
114+
borderRadius: listVariant === 'inset' ? 2 : 0,
115115
transition: 'background 33.333ms linear',
116116
color: getVariantStyles(variant, disabled).color,
117117
textDecoration: 'none', // for as="a"
@@ -121,11 +121,17 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
121121
backgroundColor: `actionListItem.${variant}.hoverBg`,
122122
color: getVariantStyles(variant, disabled).hoverColor
123123
},
124-
':focus:not([aria-disabled])': {
124+
':focus:not([data-focus-visible-added])': {
125125
backgroundColor: `actionListItem.${variant}.selectedBg`,
126126
color: getVariantStyles(variant, disabled).hoverColor,
127127
outline: 'none'
128128
},
129+
'&[data-focus-visible-added]': {
130+
// we don't use :focus-visible because not all browsers (safari) have it yet
131+
outline: 'none',
132+
border: `2 solid`,
133+
boxShadow: `0 0 0 2px ${theme?.colors.accent.emphasis}`
134+
},
129135
':active:not([aria-disabled])': {
130136
backgroundColor: `actionListItem.${variant}.activeBg`,
131137
color: getVariantStyles(variant, disabled).hoverColor
@@ -147,14 +153,16 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
147153
borderColor: 'var(--divider-color, transparent)'
148154
},
149155
// show between 2 items
150-
':not(:first-of-type):not([aria-selected=true])': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
156+
':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
151157
// hide divider after dividers & group header, with higher importance!
152158
'[data-component="ActionList.Divider"] + &': {'--divider-color': 'transparent !important'},
153159
// hide border on current and previous item
154-
'&:hover:not([aria-disabled]), &:focus:not([aria-disabled])': {'--divider-color': 'transparent'},
155-
'&:hover:not([aria-disabled]) + &, &:focus:not([aria-disabled]) + &': {'--divider-color': 'transparent'},
156-
// hide border around selected item
157-
'&[aria-selected=true] + &': {'--divider-color': 'transparent'}
160+
'&:hover:not([aria-disabled]), &:focus:not([aria-disabled]), &[data-focus-visible-added]:not([aria-disabled])': {
161+
'--divider-color': 'transparent'
162+
},
163+
'&:hover:not([aria-disabled]) + &, &:focus:not([aria-disabled]) + &, &[data-focus-visible-added] + li': {
164+
'--divider-color': 'transparent'
165+
}
158166
}
159167

160168
const clickHandler = React.useCallback(
@@ -165,26 +173,40 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
165173
[onSelect, disabled]
166174
)
167175

176+
const keyPressHandler = React.useCallback(
177+
event => {
178+
if (disabled) return
179+
180+
if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) {
181+
onSelect(event)
182+
}
183+
},
184+
[onSelect, disabled]
185+
)
186+
168187
// use props.id if provided, otherwise generate one.
169188
const labelId = useSSRSafeId(id)
170189
const inlineDescriptionId = useSSRSafeId(id && `${id}--inline-description`)
171190
const blockDescriptionId = useSSRSafeId(id && `${id}--block-description`)
172191

192+
const ItemWrapper = _PrivateItemWrapper || React.Fragment
193+
173194
return (
174195
<Slots context={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
175196
{slots => (
176197
<LiBox
177198
ref={forwardedRef}
178199
sx={merge(styles, sxProp as SxProp)}
179200
onClick={clickHandler}
201+
onKeyPress={keyPressHandler}
180202
aria-selected={selected}
181203
aria-disabled={disabled ? true : undefined}
182-
tabIndex={disabled ? undefined : -1}
204+
tabIndex={disabled || _PrivateItemWrapper ? undefined : 0}
183205
aria-labelledby={`${labelId} ${slots.InlineDescription ? inlineDescriptionId : ''}`}
184206
aria-describedby={slots.BlockDescription ? blockDescriptionId : undefined}
185207
{...props}
186208
>
187-
<_PrivateItemWrapper>
209+
<ItemWrapper>
188210
<Selection selected={selected} disabled={disabled} />
189211
{slots.LeadingVisual}
190212
<Box
@@ -205,7 +227,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
205227
</ConditionalBox>
206228
{slots.BlockDescription}
207229
</Box>
208-
</_PrivateItemWrapper>
230+
</ItemWrapper>
209231
</LiBox>
210232
)}
211233
</Slots>

‎src/stories/ActionList2.stories.tsx‎

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,14 @@ export function SimpleListStory(): JSX.Element {
7373
return (
7474
<>
7575
<h1>Simple List</h1>
76-
<ErsatzOverlay>
77-
<ActionList>
78-
<ActionList.Item>Copy link</ActionList.Item>
79-
<ActionList.Item>Quote reply</ActionList.Item>
80-
<ActionList.Item>Edit comment</ActionList.Item>
81-
<ActionList.Divider />
82-
<ActionList.Item variant="danger">Delete file</ActionList.Item>
83-
</ActionList>
84-
</ErsatzOverlay>
76+
77+
<ActionList>
78+
<ActionList.Item>Copy link</ActionList.Item>
79+
<ActionList.Item>Quote reply</ActionList.Item>
80+
<ActionList.Item>Edit comment</ActionList.Item>
81+
<ActionList.Divider />
82+
<ActionList.Item variant="danger">Delete file</ActionList.Item>
83+
</ActionList>
8584
</>
8685
)
8786
}
@@ -91,40 +90,39 @@ export function WithIcon(): JSX.Element {
9190
return (
9291
<>
9392
<h1>With Icon</h1>
94-
<ErsatzOverlay>
95-
<ActionList>
96-
<ActionList.Item>
97-
<ActionList.LeadingVisual>
98-
<LinkIcon />
99-
</ActionList.LeadingVisual>
100-
github.com/primer
101-
</ActionList.Item>
102-
<ActionList.Item>
103-
<ActionList.LeadingVisual>
104-
<LawIcon />
105-
</ActionList.LeadingVisual>
106-
MIT License
107-
</ActionList.Item>
108-
<ActionList.Item>
109-
<ActionList.LeadingVisual>
110-
<StarIcon />
111-
</ActionList.LeadingVisual>
112-
256 stars
113-
</ActionList.Item>
114-
<ActionList.Item>
115-
<ActionList.LeadingVisual>
116-
<GitForkIcon />
117-
</ActionList.LeadingVisual>
118-
3 forks
119-
</ActionList.Item>
120-
<ActionList.Item variant="danger">
121-
<ActionList.LeadingVisual>
122-
<AlertIcon />
123-
</ActionList.LeadingVisual>
124-
4 vulnerabilities
125-
</ActionList.Item>
126-
</ActionList>
127-
</ErsatzOverlay>
93+
94+
<ActionList>
95+
<ActionList.Item>
96+
<ActionList.LeadingVisual>
97+
<LinkIcon />
98+
</ActionList.LeadingVisual>
99+
github.com/primer
100+
</ActionList.Item>
101+
<ActionList.Item>
102+
<ActionList.LeadingVisual>
103+
<LawIcon />
104+
</ActionList.LeadingVisual>
105+
MIT License
106+
</ActionList.Item>
107+
<ActionList.Item>
108+
<ActionList.LeadingVisual>
109+
<StarIcon />
110+
</ActionList.LeadingVisual>
111+
256 stars
112+
</ActionList.Item>
113+
<ActionList.Item>
114+
<ActionList.LeadingVisual>
115+
<GitForkIcon />
116+
</ActionList.LeadingVisual>
117+
3 forks
118+
</ActionList.Item>
119+
<ActionList.Item variant="danger">
120+
<ActionList.LeadingVisual>
121+
<AlertIcon />
122+
</ActionList.LeadingVisual>
123+
4 vulnerabilities
124+
</ActionList.Item>
125+
</ActionList>
128126
</>
129127
)
130128
}
@@ -637,7 +635,7 @@ export function DOMPropsStory(): JSX.Element {
637635
<h1>Simple List</h1>
638636
<ErsatzOverlay>
639637
<ActionList>
640-
<ActionList.Item id="something" onClick={event => alert(`Id is '${event.currentTarget.getAttribute('id')}'`)}>
638+
<ActionList.Item id="something" onClick={event => alert(`Id is '${event.target.id}'`)}>
641639
Has an id
642640
</ActionList.Item>
643641
</ActionList>
@@ -1185,7 +1183,7 @@ const SortableItem: React.FC<SortableItemProps> = ({option, onSelect, reorder})
11851183
sx={{
11861184
opacity: isDragging ? 0.5 : 1,
11871185
boxShadow: isOver ? theme => `0px 2px 0 0px ${theme.colors.accent.emphasis}` : undefined,
1188-
borderRadius: isOver ? 0 : undefined
1186+
borderRadius: isOver ? 0 : 2
11891187
}}
11901188
>
11911189
<ActionList.LeadingVisual>{option.icon}</ActionList.LeadingVisual>

0 commit comments

Comments
 (0)