Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(autocomplete): improve implementation of aria-activedescendant #11743

Merged
merged 1 commit into from
Aug 16, 2019

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Jun 12, 2019

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

ChromeVox announces the following when the user changes to the Alaska option

'Alaska, Status'

When it should announce something like

'Alaska, List item, 2 of 50. ... List box with 8 items.'

Issue Number:
Fixes #11742

What is the new behavior?

  • allow screen readers to do more and us to do less
    • remove extra calls to announce the item that is visually focused
    • remove tests for these extra live announcements
    • give every option an id for use with aria-activedescendant
    • use the selected class for styling and finding the active option
  • implement recommendations from a11y guides
    • add the clear button to the tab order
    • change input type to text
    • always define a name attribute
  • when the popup isn't expanded
    • aria-owns and aria-activedescendant shouldn't be defined
  • when the autocomplete is disabled
    • aria-autocomplete and aria-role shouldn't be defined
    • aria-haspopup should be false
  • add md-autocomplete-suggestion class for styling instead of using li
  • add md-autoselect to the dialog demo for help w/ manual testing
  • remove overly verbose aria-describedby from basic demo
  • mark md-icons in md-item-templates of autocomplete demos as hidden

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

  • Verified on VoiceOver for macOS
  • Verified on ChromeVox on Chrome OS
  • Verified on NVDA for Windows

Related resources

Live demo:
https://angularjs-material.firebaseapp.com/demo/autocomplete

@Splaktar Splaktar added this to the 1.1.20 milestone Jun 12, 2019
@Splaktar Splaktar self-assigned this Jun 12, 2019
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Jun 12, 2019
@Splaktar Splaktar added a11y This issue is related to accessibility P2: required Issues that must be fixed. type: bug needs: unit tests This PR needs unit tests to cover the changes being proposed g3: reported The issue was reported by an internal or external product team. g3: sync labels Jun 12, 2019
@Splaktar Splaktar force-pushed the autocomplete-improveA11y branch 4 times, most recently from a2152a9 to 8876826 Compare June 20, 2019 08:27
@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed needs: unit tests This PR needs unit tests to cover the changes being proposed labels Jun 20, 2019
@Splaktar Splaktar assigned jelbourn and andrewseguin and unassigned jelbourn Jun 20, 2019
@Splaktar Splaktar added pr: presubmit-failures needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved labels Jul 11, 2019
@Splaktar
Copy link
Member Author

There are some e2e failures in g3 due to certain DOM lookups no longer returning results. It may be due to the change to #selected_option.

There are also other unit test failures that need to be investigated.

I can probably mitigate these issues once they are more clearly defined and communicated.

@Splaktar Splaktar force-pushed the autocomplete-improveA11y branch from 8876826 to 6e1c20e Compare July 24, 2019 05:21
@Splaktar
Copy link
Member Author

Rebased.

@Splaktar
Copy link
Member Author

Splaktar commented Aug 1, 2019

I've redeployed these changes to https://angularjs-material.firebaseapp.com/demo/autocomplete. I had recently deployed some md-select a11y changes to that site for ChromeVox testing.

@Splaktar Splaktar force-pushed the autocomplete-improveA11y branch from 6e1c20e to 264182d Compare August 1, 2019 03:15
@Splaktar
Copy link
Member Author

Splaktar commented Aug 1, 2019

Received feedback on this based on JAWS testing that scrolling of the options list should wrap. Opened #11766 to track that issue separate from this PR.

@Splaktar
Copy link
Member Author

Splaktar commented Aug 1, 2019

Received feedback on this based on JAWS testing that the default Escape key behavior did not align with expectations. Noted that we do have the md-escape-options API to change this behavior. Opened #11767 to consider changing the default Escape handling to align with the WAI-ARIA Authoring Practices.

I've updated all of the demos to use md-escape-options="clear".

- allow screen readers to do more and us to do less
  - remove extra calls to announce the item that is visually focused
  - remove tests for these extra live announcements
  - give every option an id for use with `aria-activedescendant`
  - use the `selected` class for styling and finding the active option
- implement recommendations from a11y guides
  - add the clear button to the tab order
  - change input type to `text`
  - always define a `name` attribute
- when the popup isn't expanded
  - `aria-owns` and `aria-activedescendant` shouldn't be defined
- when the autocomplete is disabled
  - `aria-autocomplete` and `aria-role` shouldn't be defined
  - `aria-haspopup` should be false
- add md-autocomplete-suggestion class for styling instead of using `li`
- add `md-autoselect` to the dialog demo for help w/ manual testing
- remove overly verbose `aria-describedby` from basic demo
- mark `md-icons` in `md-item-templates` of autocomplete demos as hidden
- update demos to use `md-escape-options="clear"` for better a11y

Fixes #11742
@Splaktar
Copy link
Member Author

Splaktar commented Aug 8, 2019

It looks like the remaining presubmit issues can be handled within g3, then this will be able to get merged.

@andrewseguin andrewseguin removed the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Aug 12, 2019
@Splaktar Splaktar assigned mmalerba and unassigned andrewseguin Aug 13, 2019
@Splaktar Splaktar requested a review from jelbourn August 15, 2019 05:52
@Splaktar Splaktar modified the milestones: 1.1.20, 1.1.21 Aug 15, 2019
@mmalerba mmalerba merged commit 8c159aa into master Aug 16, 2019
@mmalerba mmalerba deleted the autocomplete-improveA11y branch August 16, 2019 23:17
jelbourn pushed a commit that referenced this pull request Jan 5, 2022
- allow screen readers to do more and us to do less
  - remove extra calls to announce the item that is visually focused
  - remove tests for these extra live announcements
  - give every option an id for use with `aria-activedescendant`
  - use the `selected` class for styling and finding the active option
- implement recommendations from a11y guides
  - add the clear button to the tab order
  - change input type to `text`
  - always define a `name` attribute
- when the popup isn't expanded
  - `aria-owns` and `aria-activedescendant` shouldn't be defined
- when the autocomplete is disabled
  - `aria-autocomplete` and `aria-role` shouldn't be defined
  - `aria-haspopup` should be false
- add md-autocomplete-suggestion class for styling instead of using `li`
- add `md-autoselect` to the dialog demo for help w/ manual testing
- remove overly verbose `aria-describedby` from basic demo
- mark `md-icons` in `md-item-templates` of autocomplete demos as hidden
- update demos to use `md-escape-options="clear"` for better a11y

Fixes #11742

(cherry picked from commit 8c159aa)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ g3: reported The issue was reported by an internal or external product team. P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autocomplete: results are not voiced with index information
6 participants