fix(aria): improve WAI-ARIA compliance

Big thanks to @1Copenut for leaving feedback in #285
downshift-js · Mar 23, 2018 · 7f66c4e · 7f66c4e
1 parent 1dcf82c
commit 7f66c4e
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 44 deletions.
diff --git a/.all-contributorsrc b/.all-contributorsrc
@@ -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"
+      ]
     }
   ]
 }
diff --git a/README.md b/README.md
@@ -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]
 
@@ -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
@@ -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") |
 
 <!-- ALL-CONTRIBUTORS-LIST:END -->
 

diff --git a/other/react-native/__tests__/__snapshots__/render-tests.js.snap b/other/react-native/__tests__/__snapshots__/render-tests.js.snap
@@ -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}
   role="combobox"
 >
   <TextInput
     allowFontScaling={true}
     aria-activedescendant={null}
     aria-autocomplete="list"
-    aria-controls="downshift-0-menu"
+    aria-controls={null}
     aria-expanded={false}
     aria-labelledby="downshift-0-label"
     autoComplete="off"
     id="downshift-0-input"
     onBlur={[Function]}
     onChangeText={[Function]}
     onKeyDown={[Function]}
-    role="listbox"
     value=""
   />
   <View>

diff --git a/src/__tests__/__snapshots__/downshift.aria.js.snap b/src/__tests__/__snapshots__/downshift.aria.js.snap
@@ -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
@@ -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"
@@ -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
@@ -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"

diff --git a/src/__tests__/__snapshots__/downshift.get-item-props.js.snap b/src/__tests__/__snapshots__/downshift.get-item-props.js.snap
@@ -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

diff --git a/src/__tests__/downshift.aria.js b/src/__tests__/downshift.aria.js
@@ -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 {

diff --git a/src/downshift.js b/src/downshift.js
@@ -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,
     }
@@ -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,
@@ -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,
@@ -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,
     }