fix(aria): improve WAI-ARIA compliance #396
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8 tasks
kentcdodds
force-pushed
the
pr/aria-improvements-2
branch
from
a711862
to
7f66c4e
Compare
kentcdodds
commented
Mar 23, 2018
| @@ -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} | |||
There was a problem hiding this comment.
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} |
kentcdodds
commented
Mar 23, 2018
| @@ -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") | | |||
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
@kentcdodds Yes, this is great. I added one more comment about an attribute I missed on the first pass, but everything else looks 馃挴.
1Copenut
reviewed
Mar 23, 2018
| role="combobox" | ||
| > | ||
| <TextInput | ||
| allowFontScaling={true} | ||
| aria-activedescendant={null} | ||
| aria-autocomplete="list" | ||
| aria-controls="downshift-0-menu" | ||
| aria-controls={null} | ||
| aria-expanded={false} |
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
Perfect! Thanks! I'll just push that directly to the next branch. Hopefully this will all be released in the next week or two.
kentcdodds
pushed a commit
that referenced
this pull request
Mar 23, 2018
kentcdodds
pushed a commit
that referenced
this pull request
Jun 11, 2018
fix(TypeScript): Change typings (#373) BREAKING CHANGE: TypeScript typings are much more complete now. fix(exports): removed statics assignments to default export (#390) fix(build): to facilitate better tree-shaking, this is necessary feat(a11y): Improve a11y and add getMenuProps (#285) We no longer will validate your `htmlFor` and `id` attributes on labels and inputs. Instead we provide props you can pass if you really want to set those. You can still set those values yourself on the elements, but we wont validate them for you. Closes #283 fix(TypeScript): add missing types for setItemCount and unsetItemCount actions (#393) fix(TS): add missing types for setState and getMenuProps (#394) fix(aria): improve WAI-ARIA compliance (#396) Big thanks to @1Copenut for leaving feedback in #285 fix(aria): remove unnecessary aria-expanded from the input feat(flow): add flow definitions (#399) * [#23] Flow definitions * removed factory function * Fix code review notes fix(TypeScript): fix several type definitions (#407) * Fixed existing types and added missing types * Added Partial to state Closes #406 fix(TS): Change StateChangeOptions to only extend DownshiftState as a Partial (#424) * let prettier run * Add contributor * StateChangeOptions now extends only a Partial of DownshiftState fix(flow): more precise types (#402) * [#23] more precise types * update defintions
kentcdodds
pushed a commit
that referenced
this pull request
Jun 11, 2018
fix(TypeScript): Change typings (#373) BREAKING CHANGE: TypeScript typings are much more complete now. fix(exports): removed statics assignments to default export (#390) fix(build): to facilitate better tree-shaking, this is necessary feat(a11y): Improve a11y and add getMenuProps (#285) We no longer will validate your `htmlFor` and `id` attributes on labels and inputs. Instead we provide props you can pass if you really want to set those. You can still set those values yourself on the elements, but we wont validate them for you. Closes #283 fix(TypeScript): add missing types for setItemCount and unsetItemCount actions (#393) fix(TS): add missing types for setState and getMenuProps (#394) fix(aria): improve WAI-ARIA compliance (#396) Big thanks to @1Copenut for leaving feedback in #285 fix(aria): remove unnecessary aria-expanded from the input feat(flow): add flow definitions (#399) * [#23] Flow definitions * removed factory function * Fix code review notes fix(TypeScript): fix several type definitions (#407) * Fixed existing types and added missing types * Added Partial to state Closes #406 fix(TS): Change StateChangeOptions to only extend DownshiftState as a Partial (#424) * let prettier run * Add contributor * StateChangeOptions now extends only a Partial of DownshiftState fix(flow): more precise types (#402) * [#23] more precise types * update defintions
kentcdodds
pushed a commit
that referenced
this pull request
Jun 11, 2018
fix(TypeScript): Change typings (#373) BREAKING CHANGE: TypeScript typings are much more complete now. fix(exports): removed statics assignments to default export (#390) fix(build): to facilitate better tree-shaking, this is necessary feat(a11y): Improve a11y and add getMenuProps (#285) We no longer will validate your `htmlFor` and `id` attributes on labels and inputs. Instead we provide props you can pass if you really want to set those. You can still set those values yourself on the elements, but we wont validate them for you. Closes #283 fix(TypeScript): add missing types for setItemCount and unsetItemCount actions (#393) fix(TS): add missing types for setState and getMenuProps (#394) fix(aria): improve WAI-ARIA compliance (#396) Big thanks to @1Copenut for leaving feedback in #285 fix(aria): remove unnecessary aria-expanded from the input feat(flow): add flow definitions (#399) * [#23] Flow definitions * removed factory function * Fix code review notes fix(TypeScript): fix several type definitions (#407) * Fixed existing types and added missing types * Added Partial to state Closes #406 fix(TS): Change StateChangeOptions to only extend DownshiftState as a Partial (#424) * let prettier run * Add contributor * StateChangeOptions now extends only a Partial of DownshiftState fix(flow): more precise types (#402) * [#23] more precise types * update defintions
kentcdodds
pushed a commit
that referenced
this pull request
Jun 14, 2018
* feat(2.0.0): squashed commits of everything! fix(TypeScript): Change typings (#373) BREAKING CHANGE: TypeScript typings are much more complete now. fix(exports): removed statics assignments to default export (#390) fix(build): to facilitate better tree-shaking, this is necessary feat(a11y): Improve a11y and add getMenuProps (#285) We no longer will validate your `htmlFor` and `id` attributes on labels and inputs. Instead we provide props you can pass if you really want to set those. You can still set those values yourself on the elements, but we wont validate them for you. Closes #283 fix(TypeScript): add missing types for setItemCount and unsetItemCount actions (#393) fix(TS): add missing types for setState and getMenuProps (#394) fix(aria): improve WAI-ARIA compliance (#396) Big thanks to @1Copenut for leaving feedback in #285 fix(aria): remove unnecessary aria-expanded from the input feat(flow): add flow definitions (#399) * [#23] Flow definitions * removed factory function * Fix code review notes fix(TypeScript): fix several type definitions (#407) * Fixed existing types and added missing types * Added Partial to state Closes #406 fix(TS): Change StateChangeOptions to only extend DownshiftState as a Partial (#424) * let prettier run * Add contributor * StateChangeOptions now extends only a Partial of DownshiftState fix(flow): more precise types (#402) * [#23] more precise types * update defintions * feat(breakingChanges): remove the breakingChanges prop Now we'll go with the more expected behavior for controlling selectedItem. BREAKING CHANGE: When controlling `selectedItem`, the inputValue will be set to the `defaultInputValue` rather than `itemToString`. * fix(menu): don't reset downshift if clicking within the menu Closes #287 * fix: remove support for prop called `render`. Only support `children` This is done to bring downshift into conformance with the community convention of using the children prop (for example, the React Team chose `children` with the official context API). We'd prefer to reduce cognitive overhead by not having two ways to do the same thing. Closes #433 BREAKING CHANGE: If you use the `render` prop, rename it to `children`. * fix(preventDownshiftDefault): accept the property via nativeEvent Closes #404 * fix(react-native): custom onPress handlers will now be properly called * docs: add spectrum badge * fix(TS): remove breakingChanges prop * fix(native): Support both onChange and onChangeText props (#462) This supports both props that <TextInput /> accepts for supporting text input changed events. Additionally, this moves onChange related tests to their own module. BREAKING CHANGE: If you use the `render` prop, rename it to `children`.
Rendez
pushed a commit
to Rendez/downshift
that referenced
this pull request
Sep 30, 2018
* feat(2.0.0): squashed commits of everything! fix(TypeScript): Change typings (downshift-js#373) BREAKING CHANGE: TypeScript typings are much more complete now. fix(exports): removed statics assignments to default export (downshift-js#390) fix(build): to facilitate better tree-shaking, this is necessary feat(a11y): Improve a11y and add getMenuProps (downshift-js#285) We no longer will validate your `htmlFor` and `id` attributes on labels and inputs. Instead we provide props you can pass if you really want to set those. You can still set those values yourself on the elements, but we wont validate them for you. Closes downshift-js#283 fix(TypeScript): add missing types for setItemCount and unsetItemCount actions (downshift-js#393) fix(TS): add missing types for setState and getMenuProps (downshift-js#394) fix(aria): improve WAI-ARIA compliance (downshift-js#396) Big thanks to @1Copenut for leaving feedback in downshift-js#285 fix(aria): remove unnecessary aria-expanded from the input feat(flow): add flow definitions (downshift-js#399) * [downshift-js#23] Flow definitions * removed factory function * Fix code review notes fix(TypeScript): fix several type definitions (downshift-js#407) * Fixed existing types and added missing types * Added Partial to state Closes downshift-js#406 fix(TS): Change StateChangeOptions to only extend DownshiftState as a Partial (downshift-js#424) * let prettier run * Add contributor * StateChangeOptions now extends only a Partial of DownshiftState fix(flow): more precise types (downshift-js#402) * [downshift-js#23] more precise types * update defintions * feat(breakingChanges): remove the breakingChanges prop Now we'll go with the more expected behavior for controlling selectedItem. BREAKING CHANGE: When controlling `selectedItem`, the inputValue will be set to the `defaultInputValue` rather than `itemToString`. * fix(menu): don't reset downshift if clicking within the menu Closes downshift-js#287 * fix: remove support for prop called `render`. Only support `children` This is done to bring downshift into conformance with the community convention of using the children prop (for example, the React Team chose `children` with the official context API). We'd prefer to reduce cognitive overhead by not having two ways to do the same thing. Closes downshift-js#433 BREAKING CHANGE: If you use the `render` prop, rename it to `children`. * fix(preventDownshiftDefault): accept the property via nativeEvent Closes downshift-js#404 * fix(react-native): custom onPress handlers will now be properly called * docs: add spectrum badge * fix(TS): remove breakingChanges prop * fix(native): Support both onChange and onChangeText props (downshift-js#462) This supports both props that <TextInput /> accepts for supporting text input changed events. Additionally, this moves onChange related tests to their own module. BREAKING CHANGE: If you use the `render` prop, rename it to `children`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What: Make some changes to the aria attributes
Why: Improve our compliance to WAI-ARIA, thanks to feedback from @1Copenut in #285 馃憦
How: Followed the feedback to edit the aria attributes and added a test for some logic around
aria-labelforgetMenuPropsChecklist: