Skip to content

ActionList: a11y fixes #1958

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 5 commits into from
Mar 15, 2022
Merged

ActionList: a11y fixes #1958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
de32f66
role=none without a parent role is invalid
siddharthkp Mar 14, 2022
9e4ef53
Add example with Heading
siddharthkp Mar 14, 2022
d35bcf3
Add focus styles for high contrast mode
siddharthkp Mar 14, 2022
cfa94c4
Create actionlist-a11y-fixes.md
siddharthkp Mar 14, 2022
09e0bf9
Merge branch 'main' into siddharth/actionlist-a11y
siddharthkp Mar 15, 2022
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
6 changes: 6 additions & 0 deletions .changeset/actionlist-a11y-fixes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@primer/react": patch
---

ActionList: Add focus styles for Windows high contrast mode
ActionList: Fix incorrect role for ActionList.Group outside ActionMenu
Comment on lines +5 to +6
Copy link
Contributor

@colebemis colebemis Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI you can add multiple changesets in one PR

2 changes: 1 addition & 1 deletion src/ActionList/Group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const Group: React.FC<ActionListGroupProps> = ({
return (
<Box
as="li"
role="none"
role={listRole ? 'none' : undefined}
sx={{
'&:not(:first-child)': {marginTop: 2},
listStyle: 'none', // hide the ::marker inserted by browser's stylesheet
Expand Down
8 changes: 8 additions & 0 deletions src/ActionList/Item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
}
},

'@media (forced-colors: active)': {
':focus': {
// we set color to be transparent and let the high contrast rules
// decide what color with contrast should that be corrected to
outline: 'solid 1px transparent !important'
Copy link
Contributor

@colebemis colebemis Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there documentation about this browser behavior anywhere?

Copy link
Contributor

@pksjce pksjce Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also what happens with safari with this media tag?

Copy link
Member Author

@siddharthkp siddharthkp Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there documentation about this browser behavior anywhere?

Yep! Here's the color adjustment spec and windows blog post with transparent tip. I got this trick from primer/css action-list-item.scss

Also what happens with safari with this media tag?

Safari doesn't support it, so it skips it. (Not sure if there's a way to make this work with windows high contrast + safari)

}
},

/** Divider styles */
'[data-component="ActionList.Item--DividerContainer"]': {
position: 'relative'
Expand Down
13 changes: 10 additions & 3 deletions src/stories/ActionList/examples.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
XIcon
} from '@primer/octicons-react'

import {ThemeProvider} from '../..'
import {Heading, ThemeProvider} from '../..'
import {ActionList} from '../../ActionList'
import BaseStyles from '../../BaseStyles'
import Avatar from '../../Avatar'
Expand Down Expand Up @@ -49,9 +49,16 @@ export function WithLinks(): JSX.Element {
<>
<h1>With Links</h1>

<p>This pattern can be seen in the repository sidebar, containing a list of links</p>
<p>This pattern can be seen in the repository sidebar, containing a list of links. </p>
<p>
The heading &quot;Details&quot; is outside the ActionList and needs to have an <code>id</code> which is passed
to ActionList with <code>aria-labelledby</code>.
</p>

<ActionList>
<Heading as="h1" id="list-heading" sx={{fontSize: 1}}>
Details
</Heading>
<ActionList aria-labelledby="list-heading">
<ActionList.LinkItem href="https://github.com/primer/react#readme">
<ActionList.LeadingVisual>
<BookIcon />
Expand Down