Skip to content

ActionList: Add keyboard accessibility #1603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 15, 2021
Merged

ActionList: Add keyboard accessibility #1603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f96d2ab
different visual styles for keyboard navigation
siddharthkp Nov 11, 2021
5500790
Merge branch 'main' into siddharth/actionlist-keyboard
siddharthkp Nov 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/actionlist-keyboard-a11y.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/components': patch
---

ActionList: Improve keyboard accessibility with focus styles cross browser
46 changes: 34 additions & 12 deletions src/ActionList2/Item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
onSelect = () => null,
sx: sxProp = {},
id,
_PrivateItemWrapper = ({children}) => <>{children}</>,
_PrivateItemWrapper,
...props
},
forwardedRef
Expand All @@ -109,9 +109,9 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
fontSize: 1,
paddingY: '6px', // custom value off the scale
lineHeight: TEXT_ROW_HEIGHT,
marginX: listVariant === 'inset' ? 2 : 0,
minHeight: 5,
borderRadius: 2,
marginX: listVariant === 'inset' ? 2 : 0,
borderRadius: listVariant === 'inset' ? 2 : 0,
transition: 'background 33.333ms linear',
color: getVariantStyles(variant, disabled).color,
textDecoration: 'none', // for as="a"
Expand All @@ -121,11 +121,17 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
backgroundColor: `actionListItem.${variant}.hoverBg`,
color: getVariantStyles(variant, disabled).hoverColor
},
':focus:not([aria-disabled])': {
':focus:not([data-focus-visible-added])': {
backgroundColor: `actionListItem.${variant}.selectedBg`,
color: getVariantStyles(variant, disabled).hoverColor,
outline: 'none'
},
'&[data-focus-visible-added]': {
// we don't use :focus-visible because not all browsers (safari) have it yet
outline: 'none',
border: `2 solid`,
boxShadow: `0 0 0 2px ${theme?.colors.accent.emphasis}`
},
':active:not([aria-disabled])': {
backgroundColor: `actionListItem.${variant}.activeBg`,
color: getVariantStyles(variant, disabled).hoverColor
Expand All @@ -147,14 +153,16 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
borderColor: 'var(--divider-color, transparent)'
},
// show between 2 items
':not(:first-of-type):not([aria-selected=true])': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
':not(:first-of-type)': {'--divider-color': theme?.colors.actionListItem.inlineDivider},
// hide divider after dividers & group header, with higher importance!
'[data-component="ActionList.Divider"] + &': {'--divider-color': 'transparent !important'},
// hide border on current and previous item
'&:hover:not([aria-disabled]), &:focus:not([aria-disabled])': {'--divider-color': 'transparent'},
'&:hover:not([aria-disabled]) + &, &:focus:not([aria-disabled]) + &': {'--divider-color': 'transparent'},
// hide border around selected item
'&[aria-selected=true] + &': {'--divider-color': 'transparent'}
'&:hover:not([aria-disabled]), &:focus:not([aria-disabled]), &[data-focus-visible-added]:not([aria-disabled])': {
'--divider-color': 'transparent'
},
'&:hover:not([aria-disabled]) + &, &:focus:not([aria-disabled]) + &, &[data-focus-visible-added] + li': {
'--divider-color': 'transparent'
}
}

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

const keyPressHandler = React.useCallback(
event => {
if (disabled) return

if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) {
onSelect(event)
}
},
[onSelect, disabled]
)

// use props.id if provided, otherwise generate one.
const labelId = useSSRSafeId(id)
const inlineDescriptionId = useSSRSafeId(id && `${id}--inline-description`)
const blockDescriptionId = useSSRSafeId(id && `${id}--block-description`)

const ItemWrapper = _PrivateItemWrapper || React.Fragment

return (
<Slots context={{variant, disabled, inlineDescriptionId, blockDescriptionId}}>
{slots => (
<LiBox
ref={forwardedRef}
sx={merge(styles, sxProp as SxProp)}
onClick={clickHandler}
onKeyPress={keyPressHandler}
aria-selected={selected}
aria-disabled={disabled ? true : undefined}
tabIndex={disabled ? undefined : -1}
tabIndex={disabled || _PrivateItemWrapper ? undefined : 0}
aria-labelledby={`${labelId} ${slots.InlineDescription ? inlineDescriptionId : ''}`}
aria-describedby={slots.BlockDescription ? blockDescriptionId : undefined}
{...props}
>
<_PrivateItemWrapper>
<ItemWrapper>
<Selection selected={selected} disabled={disabled} />
{slots.LeadingVisual}
<Box
Expand All @@ -205,7 +227,7 @@ export const Item = React.forwardRef<HTMLLIElement, ItemProps>(
</ConditionalBox>
{slots.BlockDescription}
</Box>
</_PrivateItemWrapper>
</ItemWrapper>
</LiBox>
)}
</Slots>
Expand Down
88 changes: 43 additions & 45 deletions src/stories/ActionList2.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,14 @@ export function SimpleListStory(): JSX.Element {
return (
<>
<h1>Simple List</h1>
<ErsatzOverlay>
<ActionList>
<ActionList.Item>Copy link</ActionList.Item>
<ActionList.Item>Quote reply</ActionList.Item>
<ActionList.Item>Edit comment</ActionList.Item>
<ActionList.Divider />
<ActionList.Item variant="danger">Delete file</ActionList.Item>
</ActionList>
</ErsatzOverlay>

<ActionList>
<ActionList.Item>Copy link</ActionList.Item>
<ActionList.Item>Quote reply</ActionList.Item>
<ActionList.Item>Edit comment</ActionList.Item>
<ActionList.Divider />
<ActionList.Item variant="danger">Delete file</ActionList.Item>
</ActionList>
</>
)
}
Expand All @@ -91,40 +90,39 @@ export function WithIcon(): JSX.Element {
return (
<>
<h1>With Icon</h1>
<ErsatzOverlay>
<ActionList>
<ActionList.Item>
<ActionList.LeadingVisual>
<LinkIcon />
</ActionList.LeadingVisual>
github.com/primer
</ActionList.Item>
<ActionList.Item>
<ActionList.LeadingVisual>
<LawIcon />
</ActionList.LeadingVisual>
MIT License
</ActionList.Item>
<ActionList.Item>
<ActionList.LeadingVisual>
<StarIcon />
</ActionList.LeadingVisual>
256 stars
</ActionList.Item>
<ActionList.Item>
<ActionList.LeadingVisual>
<GitForkIcon />
</ActionList.LeadingVisual>
3 forks
</ActionList.Item>
<ActionList.Item variant="danger">
<ActionList.LeadingVisual>
<AlertIcon />
</ActionList.LeadingVisual>
4 vulnerabilities
</ActionList.Item>
</ActionList>
</ErsatzOverlay>

<ActionList>
<ActionList.Item>
<ActionList.LeadingVisual>
<LinkIcon />
</ActionList.LeadingVisual>
github.com/primer
</ActionList.Item>
<ActionList.Item>
<ActionList.LeadingVisual>
<LawIcon />
</ActionList.LeadingVisual>
MIT License
</ActionList.Item>
<ActionList.Item>
<ActionList.LeadingVisual>
<StarIcon />
</ActionList.LeadingVisual>
256 stars
</ActionList.Item>
<ActionList.Item>
<ActionList.LeadingVisual>
<GitForkIcon />
</ActionList.LeadingVisual>
3 forks
</ActionList.Item>
<ActionList.Item variant="danger">
<ActionList.LeadingVisual>
<AlertIcon />
</ActionList.LeadingVisual>
4 vulnerabilities
</ActionList.Item>
</ActionList>
</>
)
}
Expand Down Expand Up @@ -637,7 +635,7 @@ export function DOMPropsStory(): JSX.Element {
<h1>Simple List</h1>
<ErsatzOverlay>
<ActionList>
<ActionList.Item id="something" onClick={event => alert(`Id is '${event.currentTarget.getAttribute('id')}'`)}>
<ActionList.Item id="something" onClick={event => alert(`Id is '${event.target.id}'`)}>
Has an id
</ActionList.Item>
</ActionList>
Expand Down Expand Up @@ -1185,7 +1183,7 @@ const SortableItem: React.FC<SortableItemProps> = ({option, onSelect, reorder})
sx={{
opacity: isDragging ? 0.5 : 1,
boxShadow: isOver ? theme => `0px 2px 0 0px ${theme.colors.accent.emphasis}` : undefined,
borderRadius: isOver ? 0 : undefined
borderRadius: isOver ? 0 : 2
}}
>
<ActionList.LeadingVisual>{option.icon}</ActionList.LeadingVisual>
Expand Down