fix(select): use combobox pattern for accessibility

[crisbeto]

Applies some of our recent learnings on how to handle the accessibility of a custom select to mat-select. Includes switching the trigger to be a combobox and the panel to a listbox.

Fixes #11083.

[jelbourn]

LGTM

[jelbourn]

So this has ~100 failing targets in Google from about 20 projects. I haven't looked too deeply, but it seems like a lot of people wrote selectors in their tests like mat-select[aria-label="..."]. I might see seeing about fixing up some of these with a harness and seeing how it goes.

[crisbeto]

Something to keep in mind about those selectors: people were probably trying to target a select with a specific placeholder. The select currently in master forwards its placeholder value to aria-label, unless aria-label is explicitly overwritten.

[andrewseguin]

Can we do the same thing as input and temporarily put that information into another attribute, then switch tests over to it? E.g. data-label

[crisbeto]

Have we confirmed that it's due to the selector? If that's the case, it's easy to add a similar attribute.

[andrewseguin]

I'm seeing violations that the role=combobox doesn't contain a textbox child.

https://dequeuniversity.com/rules/axe/3.5/aria-required-children?application=axeAPI

Does that apply here? Is this check incorrect in this case?

[andrewseguin]

Another set of failing accessibility errors gave me this:

Errors:
Error: AX_ARIA_04 (ARIA state and property values must be valid) failed on the following elements (1 - 4 of 4):
#mat-select-0
#mat-select-2
#mat-select-4
#mat-select-6
See https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_aria_04 for more information.

Error: AX_ARIA_02 (ARIA attributes which refer to other elements by ID should refer to elements which exist in the DOM) failed on the following elements (1 - 4 of 4):
#mat-select-0
#mat-select-2
#mat-select-4
#mat-select-6
See https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules#ax_aria_02 for more information.

[jelbourn]

The second one we should be able to fix in the PR, I think.

For the first one, the check is kind of problematic itself because we're following the draft ARIA 1.2 guidance on this. While this isn't the official ARIA spec today, it actually works better in most screen-readers than the active 1.1 spec.

[andrewseguin]

If it works better, we can disable the check, open an issue internally, and link to it with a TODO?

[crisbeto]

I've pushed a fix for the second error, but we can't do much about the first one.

[andrewseguin]

Here's a fun use case that causes an ExpressionChangedAfterChecked - a mat-select with a placeholder, inside an ng-container with ngIf

crisbeto#15

[andrewseguin]

I think three failures came up for accessibility:

1. role=combobox doesn't contain a textbox child
2. ARIA state and property values must be valid
3. ARIA attributes which refer to other elements by ID should refer to elements which exist in the DOM

(1) should be handled by disabling the check since the test's ax check is just out of date for now
(2) is still giving errors
(3) seems to be gone

Is there anything we can do about (2)?

[crisbeto]

Does it say which elements 2 and 3 are failing on? I pushed some changes last week that should've removed references to elements that aren't in the DOM yet.

[crisbeto]

Updated to fix the issue with aria-haspopup and the "changed after checked" error.

[drc-nloftsgard]

I am seeing a critical violation 'Certain ARIA roles must contain particular children' (https://dequeuniversity.com/rules/axe/3.5/aria-required-children?application=AxeChrome) on https://material.angular.io/components/select/overview when analyzing with Axe Core. I thought this PR was intended to fix that issue?

[crisbeto]

If I remember correctly, the only error right now is that the combobox doesn't contain an input, but I believe that's permitted by a newer version of the ARIA spec. Before we made these changes, we spent some time testing different prototypes against all of the popular screen readers and consulting with some of Google's accessibility experts.

[jelbourn]

Yeah, we're a little ahead of axe here. See dequelabs/axe-core#2505

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}