Skip to content
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.

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

Already on GitHub? Sign in to your account

fix(aria): improve WAI-ARIA compliance #396

Merged
merged 1 commit into from Mar 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions .all-contributorsrc
Expand Up @@ -712,6 +712,15 @@
"code",
"test"
]
},
{
"login": "1Copenut",
"name": "Trevor Pierce",
"avatar_url": "https://avatars0.githubusercontent.com/u/934879?v=4",
"profile": "https://github.com/1Copenut",
"contributions": [
"review"
]
}
]
}
12 changes: 10 additions & 2 deletions README.md
Expand Up @@ -19,7 +19,7 @@ autocomplete/dropdown/select/combobox components</p>
[![downloads][downloads-badge]][npmcharts] [![version][version-badge]][package]
[![MIT License][license-badge]][license]

[![All Contributors](https://img.shields.io/badge/all_contributors-71-orange.svg?style=flat-square)](#contributors)
[![All Contributors](https://img.shields.io/badge/all_contributors-72-orange.svg?style=flat-square)](#contributors)
[![PRs Welcome][prs-badge]][prs] [![Chat][chat-badge]][chat]
[![Code of Conduct][coc-badge]][coc]

Expand Down Expand Up @@ -611,6 +611,14 @@ This method should be applied to the element which contains your list of items.
Typically, this will be a `<div>` or a `<ul>` that surrounds a `map` expression.
This handles the proper ARIA roles and attributes.

Optional properties:

* `aria-label`: By default the menu will add an `aria-labelledby` that refers
to the `<label>` rendered with `getLabelProps`. However, if you provide
`aria-label` to give a more specific label that describes the options
available, then `aria-labelledby` will not be provided and screen readers
can use your `aria-label` instead.

```jsx
<ul {...getMenuProps()}>
{!isOpen
Expand Down Expand Up @@ -1000,7 +1008,7 @@ Thanks goes to these people ([emoji key][emojis]):
| [<img src="https://avatars2.githubusercontent.com/u/1556430?v=4" width="100px;"/><br /><sub><b>Pete Redmond</b></sub>](https://httpete.com)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Ahttpete-ire "Bug reports") | [<img src="https://avatars2.githubusercontent.com/u/1706342?v=4" width="100px;"/><br /><sub><b>Nick Lavin</b></sub>](https://github.com/Zashy)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3AZashy "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=Zashy "Code") [鈿狅笍](https://github.com/paypal/downshift/commits?author=Zashy "Tests") | [<img src="https://avatars2.githubusercontent.com/u/17031?v=4" width="100px;"/><br /><sub><b>James Long</b></sub>](http://jlongster.com)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Ajlongster "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=jlongster "Code") | [<img src="https://avatars0.githubusercontent.com/u/1505907?v=4" width="100px;"/><br /><sub><b>Michael Ball</b></sub>](http://michaelball.co)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Acycomachead "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=cycomachead "Code") [鈿狅笍](https://github.com/paypal/downshift/commits?author=cycomachead "Tests") | [<img src="https://avatars0.githubusercontent.com/u/8990614?v=4" width="100px;"/><br /><sub><b>CAVALEIRO Julien</b></sub>](https://github.com/Julienng)<br />[馃挕](#example-Julienng "Examples") | [<img src="https://avatars1.githubusercontent.com/u/3421067?v=4" width="100px;"/><br /><sub><b>Kim Gr枚nqvist</b></sub>](http://www.kimgronqvist.se)<br />[馃捇](https://github.com/paypal/downshift/commits?author=kimgronqvist "Code") [鈿狅笍](https://github.com/paypal/downshift/commits?author=kimgronqvist "Tests") | [<img src="https://avatars2.githubusercontent.com/u/3675602?v=4" width="100px;"/><br /><sub><b>Sijie</b></sub>](http://sijietian.com)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Atiansijie "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=tiansijie "Code") |
| [<img src="https://avatars0.githubusercontent.com/u/410792?v=4" width="100px;"/><br /><sub><b>Dony Sukardi</b></sub>](http://dsds.io)<br />[馃挕](#example-donysukardi "Examples") [馃挰](#question-donysukardi "Answering Questions") [馃捇](https://github.com/paypal/downshift/commits?author=donysukardi "Code") [鈿狅笍](https://github.com/paypal/downshift/commits?author=donysukardi "Tests") | [<img src="https://avatars1.githubusercontent.com/u/2755722?v=4" width="100px;"/><br /><sub><b>Dillon Mulroy</b></sub>](https://dillonmulroy.com)<br />[馃摉](https://github.com/paypal/downshift/commits?author=dmmulroy "Documentation") | [<img src="https://avatars3.githubusercontent.com/u/12440573?v=4" width="100px;"/><br /><sub><b>Curtis Tate Wilkinson</b></sub>](https://twitter.com/curtytate)<br />[馃捇](https://github.com/paypal/downshift/commits?author=curtiswilkinson "Code") | [<img src="https://avatars3.githubusercontent.com/u/383212?v=4" width="100px;"/><br /><sub><b>Brice BERNARD</b></sub>](https://github.com/brikou)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Abrikou "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=brikou "Code") | [<img src="https://avatars3.githubusercontent.com/u/14304503?v=4" width="100px;"/><br /><sub><b>Tony Xu</b></sub>](https://github.com/xutopia)<br />[馃捇](https://github.com/paypal/downshift/commits?author=xutopia "Code") | [<img src="https://avatars1.githubusercontent.com/u/14035529?v=4" width="100px;"/><br /><sub><b>Anthony Ng</b></sub>](http://anthonyng.me)<br />[馃摉](https://github.com/paypal/downshift/commits?author=newyork-anthonyng "Documentation") | [<img src="https://avatars2.githubusercontent.com/u/11996139?v=4" width="100px;"/><br /><sub><b>S S</b></sub>](https://github.com/notruth)<br />[馃挰](#question-notruth "Answering Questions") [馃捇](https://github.com/paypal/downshift/commits?author=notruth "Code") [馃摉](https://github.com/paypal/downshift/commits?author=notruth "Documentation") [馃](#ideas-notruth "Ideas, Planning, & Feedback") [鈿狅笍](https://github.com/paypal/downshift/commits?author=notruth "Tests") |
| [<img src="https://avatars0.githubusercontent.com/u/29493001?v=4" width="100px;"/><br /><sub><b>Austin Tackaberry</b></sub>](http://austintackaberry.co)<br />[馃挰](#question-austintackaberry "Answering Questions") [馃捇](https://github.com/paypal/downshift/commits?author=austintackaberry "Code") [馃摉](https://github.com/paypal/downshift/commits?author=austintackaberry "Documentation") [馃悰](https://github.com/paypal/downshift/issues?q=author%3Aaustintackaberry "Bug reports") [馃挕](#example-austintackaberry "Examples") [馃](#ideas-austintackaberry "Ideas, Planning, & Feedback") [馃憖](#review-austintackaberry "Reviewed Pull Requests") [鈿狅笍](https://github.com/paypal/downshift/commits?author=austintackaberry "Tests") | [<img src="https://avatars3.githubusercontent.com/u/4168055?v=4" width="100px;"/><br /><sub><b>Jean Duthon</b></sub>](https://github.com/jduthon)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Ajduthon "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=jduthon "Code") | [<img src="https://avatars3.githubusercontent.com/u/3889580?v=4" width="100px;"/><br /><sub><b>Anton Telesh</b></sub>](http://antontelesh.github.io)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3AAntontelesh "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=Antontelesh "Code") | [<img src="https://avatars3.githubusercontent.com/u/1060669?v=4" width="100px;"/><br /><sub><b>Eric Edem</b></sub>](https://github.com/ericedem)<br />[馃捇](https://github.com/paypal/downshift/commits?author=ericedem "Code") [馃摉](https://github.com/paypal/downshift/commits?author=ericedem "Documentation") [馃](#ideas-ericedem "Ideas, Planning, & Feedback") [鈿狅笍](https://github.com/paypal/downshift/commits?author=ericedem "Tests") | [<img src="https://avatars3.githubusercontent.com/u/3409645?v=4" width="100px;"/><br /><sub><b>Austin Wood</b></sub>](https://github.com/indiesquidge)<br />[馃挰](#question-indiesquidge "Answering Questions") [馃摉](https://github.com/paypal/downshift/commits?author=indiesquidge "Documentation") [馃憖](#review-indiesquidge "Reviewed Pull Requests") | [<img src="https://avatars3.githubusercontent.com/u/14275790?v=4" width="100px;"/><br /><sub><b>Mark Murray</b></sub>](https://github.com/mmmurray)<br />[馃殗](#infra-mmmurray "Infrastructure (Hosting, Build-Tools, etc)") | [<img src="https://avatars0.githubusercontent.com/u/1862172?v=4" width="100px;"/><br /><sub><b>Gianmarco</b></sub>](https://github.com/gsimone)<br />[馃悰](https://github.com/paypal/downshift/issues?q=author%3Agsimone "Bug reports") [馃捇](https://github.com/paypal/downshift/commits?author=gsimone "Code") |
| [<img src="https://avatars2.githubusercontent.com/u/179534?v=4" width="100px;"/><br /><sub><b>stereobooster</b></sub>](https://github.com/stereobooster)<br />[馃捇](https://github.com/paypal/downshift/commits?author=stereobooster "Code") [鈿狅笍](https://github.com/paypal/downshift/commits?author=stereobooster "Tests") |
| [<img src="https://avatars2.githubusercontent.com/u/179534?v=4" width="100px;"/><br /><sub><b>stereobooster</b></sub>](https://github.com/stereobooster)<br />[馃捇](https://github.com/paypal/downshift/commits?author=stereobooster "Code") [鈿狅笍](https://github.com/paypal/downshift/commits?author=stereobooster "Tests") | [<img src="https://avatars0.githubusercontent.com/u/934879?v=4" width="100px;"/><br /><sub><b>Trevor Pierce</b></sub>](https://github.com/1Copenut)<br />[馃憖](#review-1Copenut "Reviewed Pull Requests") |
Copy link
Member Author

Choose a reason for hiding this comment

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

Here's what this looks like (scroll down): https://github.com/paypal/downshift/blob/7f66c4e143a9e0b5155d973469d3c1f953be4f87/README.md

You ok with this @1Copenut? Just want to make sure we give you the proper accolades for your contributions :)

Choose a reason for hiding this comment

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

@kentcdodds Yes, this is great. I added one more comment about an attribute I missed on the first pass, but everything else looks 馃挴.


<!-- ALL-CONTRIBUTORS-LIST:END -->

Expand Down
Expand Up @@ -5,22 +5,21 @@ exports[`renders with React Native components 1`] = `
aria-expanded={false}
aria-haspopup="listbox"
aria-labelledby="downshift-0-label"
aria-owns="downshift-0-menu"
aria-owns={null}
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't worry about this. This is react-native and it's actually just the value of the props. When it's rendered the prop wont actually be there at all.

role="combobox"
>
<TextInput
allowFontScaling={true}
aria-activedescendant={null}
aria-autocomplete="list"
aria-controls="downshift-0-menu"
aria-controls={null}
Copy link
Member Author

Choose a reason for hiding this comment

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

Same story here.

aria-expanded={false}

Choose a reason for hiding this comment

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

@kentcdodds I'm sorry, I missed this one on first go-round. The aria-expanded attribute isn't needed here, where you've defined it on the container DIV. It throws an error in axe-core, but removing it from the input, line 642 in Downshift.js, suppresses the error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect! Thanks! I'll just push that directly to the next branch. Hopefully this will all be released in the next week or two.

aria-labelledby="downshift-0-label"
autoComplete="off"
id="downshift-0-input"
onBlur={[Function]}
onChangeText={[Function]}
onKeyDown={[Function]}
role="listbox"
value=""
/>
<View>
Expand Down
8 changes: 0 additions & 8 deletions src/__tests__/__snapshots__/downshift.aria.js.snap
Expand Up @@ -5,7 +5,6 @@ exports[`basic snapshot 1`] = `
aria-expanded="false"
aria-haspopup="listbox"
aria-labelledby="downshift-0-label"
aria-owns="downshift-0-menu"
role="combobox"
>
<label
Expand All @@ -17,17 +16,14 @@ exports[`basic snapshot 1`] = `
</label>
<input
aria-autocomplete="list"
aria-controls="downshift-0-menu"
aria-expanded="false"
aria-labelledby="downshift-0-label"
autocomplete="off"
data-testid="input"
id="downshift-0-input"
role="listbox"
value="item"
/>
<button
aria-expanded="false"
aria-haspopup="true"
aria-label="open menu"
data-testid="button"
Expand Down Expand Up @@ -58,7 +54,6 @@ exports[`can override the ids 1`] = `
aria-expanded="false"
aria-haspopup="listbox"
aria-labelledby="custom-label-id"
aria-owns="custom-menu-id"
role="combobox"
>
<label
Expand All @@ -70,17 +65,14 @@ exports[`can override the ids 1`] = `
</label>
<input
aria-autocomplete="list"
aria-controls="custom-menu-id"
aria-expanded="false"
aria-labelledby="custom-label-id"
autocomplete="off"
data-testid="input"
id="custom-input-id"
role="listbox"
value=""
/>
<button
aria-expanded="false"
aria-haspopup="true"
aria-label="open menu"
data-testid="button"
Expand Down
Expand Up @@ -5,7 +5,6 @@ exports[`getItemProps defaults the index when no index is given 1`] = `
aria-expanded="false"
aria-haspopup="listbox"
aria-labelledby="downshift-1-label"
aria-owns="downshift-1-menu"
role="combobox"
>
<span
Expand Down
57 changes: 33 additions & 24 deletions src/__tests__/downshift.aria.js
Expand Up @@ -24,34 +24,43 @@ test('can override the ids', () => {
expect(container.firstChild).toMatchSnapshot()
})

function defaultRenderFn({
getInputProps,
getToggleButtonProps,
getLabelProps,
getMenuProps,
getItemProps,
}) {
return (
<div>
<label data-testid="label" {...getLabelProps()}>
label
</label>
<input data-testid="input" {...getInputProps()} />
<button data-testid="button" {...getToggleButtonProps()} />
<ul data-testid="menu" {...getMenuProps()}>
<li data-testid="item-0" {...getItemProps({item: 'item', index: 0})}>
item
</li>
</ul>
</div>
)
}
test('if aria-label is provided to the menu then aria-labelledby is not applied to the label', () => {
const customLabel = 'custom menu label'
const {menu} = renderDownshift({
menuProps: {'aria-label': customLabel},
})
expect(menu.getAttribute('aria-labelledby')).toBeNull()
expect(menu.getAttribute('aria-label')).toBe(customLabel)
})

function renderDownshift({renderFn, props, menuProps} = {}) {
function defaultRenderFn({
getInputProps,
getToggleButtonProps,
getLabelProps,
getMenuProps,
getItemProps,
}) {
return (
<div>
<label data-testid="label" {...getLabelProps()}>
label
</label>
<input data-testid="input" {...getInputProps()} />
<button data-testid="button" {...getToggleButtonProps()} />
<ul data-testid="menu" {...getMenuProps(menuProps)}>
<li data-testid="item-0" {...getItemProps({item: 'item', index: 0})}>
item
</li>
</ul>
</div>
)
}

function renderDownshift({renderFn = defaultRenderFn, props} = {}) {
let renderArg
const renderSpy = jest.fn(controllerArg => {
renderArg = controllerArg
return renderFn(controllerArg)
return renderFn || defaultRenderFn(controllerArg)
})
const utils = renderToDOM(<Downshift {...props} render={renderSpy} />)
return {
Expand Down
11 changes: 5 additions & 6 deletions src/downshift.js
Expand Up @@ -485,12 +485,13 @@ class Downshift extends Component {
this.getRootProps.called = true
this.getRootProps.refKey = refKey
this.getRootProps.suppressRefError = suppressRefError
const {isOpen} = this.getState()
return {
[refKey]: this.rootRef,
role: 'combobox',
'aria-expanded': this.getState().isOpen,
'aria-expanded': isOpen,
'aria-haspopup': 'listbox',
'aria-owns': this.menuId,
'aria-owns': isOpen ? this.menuId : null,
'aria-labelledby': this.labelId,
...rest,
}
Expand Down Expand Up @@ -558,7 +559,6 @@ class Downshift extends Component {
type: 'button',
role: 'button',
'aria-label': isOpen ? 'close menu' : 'open menu',
'aria-expanded': isOpen,
'aria-haspopup': true,
'data-toggle': true,
...eventHandlers,
Expand Down Expand Up @@ -638,14 +638,13 @@ class Downshift extends Component {
onBlur: composeEventHandlers(onBlur, this.input_handleBlur),
}
return {
role: 'listbox',
'aria-autocomplete': 'list',
'aria-expanded': isOpen,
'aria-activedescendant':
isOpen && typeof highlightedIndex === 'number' && highlightedIndex >= 0
? this.getItemId(highlightedIndex)
: null,
'aria-controls': this.menuId,
'aria-controls': isOpen ? this.menuId : null,
'aria-labelledby': this.labelId,
autoComplete: 'off',
value: inputValue,
Expand Down Expand Up @@ -692,7 +691,7 @@ class Downshift extends Component {
getMenuProps = props => {
return {
role: 'listbox',
'aria-labelledby': this.labelId,
'aria-labelledby': props && props['aria-label'] ? null : this.labelId,
id: this.menuId,
...props,
}
Expand Down