-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(chips): Add basic accessibility support. #9650
Conversation
807fc46
to
2dcc43c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with JAWS, here's what I found:
- In IE11 and Firefox, once you add a new chip you get into a weird state where keyboard input (arrow keys, typing into the input) do nothing. You have to tab and then tab back to make it work again.
- In Edge, nothing is announced. However, the website for JAWS says not to use it with Edge yet, so the ball is probably in their court.
- Everything is great in Chrome
- In IE11, when I add a new chip, JAWS announces
"{{$chip}}"
literally.
ctrl.wrapperId = '_md-chips-wrapper-' + ctrl.$mdUtil.nextUid(); | ||
|
||
// Setup a watcher which manages the role and aria-owns attributes | ||
ctrl.$scope.$watch('$mdChipsCtrl.items.length', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$watchCollection
is generally better than $watch
for array length because it's possible to add and remove the same number of items in one cycle.
ctrl.$scope.$watch('$mdChipsCtrl.items.length', function() { | ||
if (ctrl.items && ctrl.items.length) { | ||
// Dynamically add the listbox role | ||
wrapper.attr('role', 'listbox'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment like "Add the listbox on every change because it must be removed when there are no items."
tabindex="{{$mdChipsCtrl.ariaTabIndex == $index ? 0 : -1}}"\ | ||
id="{{$mdChipsCtrl.contentIdFor($index)}}"\ | ||
role="option"\ | ||
aria-live="assertive"\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if aria-live
is a good idea here. Generally you only want one live region for the whole app.
@marcysutton do you have any thoughts on this?
@@ -5,7 +5,7 @@ <h2 class="md-title">Use a custom chip template.</h2> | |||
|
|||
<form name="fruitForm"> | |||
<md-chips ng-model="ctrl.roFruitNames" name="fruitName" readonly="ctrl.readonly" | |||
md-removable="ctrl.removable" md-max-chips="5"> | |||
md-removable="ctrl.removable" md-max-chips="5" placeholder="Enter a fruit..."> | |||
<md-chip-template> | |||
<strong>{{$chip}}</strong> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In JAWS w/ IE11, upon adding a new chip, the screen reader announces "Left brace left brace dollar chip right brace right brace". Is the template being added to the dom before being linked?
IE11 and JAWS is a known issue that may be fixed by using ng-bind: angular/angular.js#15079 It works in Chrome and what AT? You should try Safari and Voiceover. NVDA is typically used with Firefox, and Edge is best supported with Narrator. Mobile Safari and Voiceover would be good to check as well. For Firefox and JAWS, I'll have to take a closer look when I'm in front of my computer. So many AT's, it can be tough I know. It is a good idea to limit use of aria-live to specific announcer elements to avoid conflicting messages. Too many similar messages will also get swallowed in JAWS, so we often use two for each politeness level. You can see this implemented in ngA11y. https://github.com/dequelabs/ngA11y/blob/master/src/nga11yannounce.js |
@topherfangio ping to finish. |
b2d6596
to
1217121
Compare
184467c
to
f0d883b
Compare
@jelbourn After discussion with team and @marcysutton, we have added a The latest updates include this change as well as all of the related documentation. Can you review this change and the new docs? Also, I have tested in again JAWS but only have 40 minute mode so it was pretty basic. Can you please re-test and see if you have any additional issues? I still want to add a few test and hopefully remove one TODO, but this is pretty close to final. |
@jelbourn - ping for review. |
@topherfangio can you rebase? |
Previously, chips had no, or very broken, support for accessibility. Make many updates to fix broken functionality and add required features. - Remove existing `aria-hidden` attributes. - Dynamically apply required `role` and `aria-owns` attributes. - Ensure `tabindex` is set properly on all elements. Particularly, you can now tab to, and navigate through, a set of chips in readonly mode. - Provide new `input-aria-label` option to identify the inputs. - Provide new `container-hint` option used when the element is in readonly mode. - Provide new `md-chip-append-delay` option which controls the delay (in ms) before a user can add a new chip (fixes screen readers). - Fix issue with `delete-hint` never being read by screen readers because it was outside of the chip. - Fix keyboard navigation to properly wrap when moving both left or right. - Fix keyboard navigation when in readonly mode. - Fix issue where the wrong chip may be focused because the old chip was still in the DOM. - Fix issue where `onAdd` callback passed incorrect `$index` (it was 1-based instead of 0-based). Additionally update the Chips docs with new Accessibility info and a few other minor changes to the docs/demos. - Remove the "WORK IN PROGRESS" notice. - Fix mispelling of "maximum" on first chips demo. - Fix Contact Chips demo filtering. - Fix Contact Chips demo images to use new/faster service. BREAKING CHANGES **MAJOR** In order to make the chips fully accessible and work properly across a wide variety of screen readers, we have added a 300ms delay after appending a chip before we re-focus the input. This is necessary because some screen readers change operational modes when the enter key is pressed inside of an input and this causes the screen reader to move into "navigation" mode and no longer apply keystrokes to the input. Additionally, this ensures that the newly added chip is properly announced to the screen readers. You **may** alter this value with the new `md-chip-append-delay` attribute, however using a value less than `300` can cause issues on JAWS and NVDA and will make your application inaccessible to those users. Note 1: This issue only seems to affect chips appended when using the `enter` key. If you override the `md-separator-keys` and disable the `enter` key (and enable something like `,` or `;`), you may be able to reduce this delay to `0` and achieve past functionality. Note 2: This issue does not appear to affect VoiceOver or ChromeVox, so if you are only targeting those users, you may be able to reduce the delay to `0`. **MINOR** We consider the following to be minor breaking changes since we never expected these attributes/elements to be utilized by developers. Nonetheless, we want to ensure that they are documented. - The `role` of the `.md-chip-content` has been modified from `button` to `option` so that it works with the new `listbox` role of it's parent. If you rely on this role being `button`, you should update your code accordingly. - The delete hint on removable chips now resides inside of the `div.md-chip-content` rather than the parent `md-chip` element where it could not be read by screen readers. If you interact with this element (in CSS or JS) please update your selectors/code to account for the new DOM hierarchy. Fixes angular#9391. Fixes angular#9556. Fixes angular#8897. Fixes angular#8867. Fixes angular#9649.
f0d883b
to
7de90c0
Compare
Fixes angular#9391. Fixes angular#9556. Fixes angular#8897. Fixes angular#8867. Fixes angular#9649.
Previously, chips had no, or very broken, support for accessibility.
Make many updates to fix broken functionality and add required features.
aria-hidden
attributes.role
andaria-owns
attributes.tabindex
is set properly on all elements. Particularly,you can now tab to, and navigate through, a set of chips in
readonly mode.
input-aria-label
option to identify the inputs.container-hint
option used when the element is inreadonly mode.
md-chip-append-delay
option which controls the delay(in ms) before a user can add a new chip (fixes screen readers).
delete-hint
never being read by screen readersbecause it was outside of the chip.
or right.
chip was still in the DOM.
onAdd
callback passed incorrect$index
(itwas 1-based instead of 0-based).
Additionally update the Chips docs with new Accessibility info and
a few other minor changes to the docs/demos.
BREAKING CHANGES
MAJOR
In order to make the chips fully accessible and work properly across
a wide variety of screen readers, we have added a 300ms delay after
appending a chip before we re-focus the input.
This is necessary because some screen readers change operational modes
when the enter key is pressed inside of an input and this causes the
screen reader to move into "navigation" mode and no longer apply
keystrokes to the input.
Additionally, this ensures that the newly added chip is properly
announced to the screen readers.
You may alter this value with the new
md-chip-append-delay
attribute, however using a value less than
300
can cause issueson JAWS and NVDA and will make your application inaccessible to those
users.
Note 1: This issue only seems to affect chips appended when using the
enter
key. If you override themd-separator-keys
and disable theenter
key (and enable something like,
or;
), you may be ableto reduce this delay to
0
and achieve past functionality.Note 2: This issue does not appear to affect VoiceOver or ChromeVox,
so if you are only targeting those users, you may be able to reduce
the delay to
0
.MINOR
We consider the following to be minor breaking changes since we never
expected these attributes/elements to be utilized by developers.
Nonetheless, we want to ensure that they are documented.
The
role
of the.md-chip-content
has been modified frombutton
to
option
so that it works with the newlistbox
role of it'sparent.
If you rely on this role being
button
, you should update your codeaccordingly.
The delete hint on removable chips now resides inside of the
div.md-chip-content
rather than the parentmd-chip
element whereit could not be read by screen readers.
If you interact with this element (in CSS or JS) please update your
selectors/code to account for the new DOM hierarchy.
Fixes #9391. Fixes #9556. Fixes #8897. Fixes #8867. Fixes #9649.