From 86cc57ee7d590330282fca03acdaa9f11b6b10b7 Mon Sep 17 00:00:00 2001 From: Zach Arend Date: Tue, 28 Mar 2023 21:21:07 +0000 Subject: [PATCH] fix(multiple): fix VoiceOver confused by Select/Autocomplete's ARIA semantics For Select and Autcomplete components, fix issues where VoiceOver was confused by the ARIA semantics of the combobox. Fix multiple behaviors: - Fix VoiceOver focus ring stuck on the combobox while navigating options. - Fix VoiceOver would sometimes reading option as a TextNode and not communicating the selected state and position in set. - Fix VoiceOver "flickering" behavior where VoiceOver would display one announcement then quickly change to another annoucement. Fix the same issues for both Select and Autocomplete component. Implement fix by correcting the combobox element and also invidual options. First, move the aria-owns reference to the overlay from the child of the combobox to the parent modal of the comobobx. Having an aria-owns reference inside the combobox element seemed to confuse VoiceOver. Second, apply `aria-hidden="true"` to the ripple element and pseudo checkboxes on mat-option. These DOM nodes are only used for visual purposes, so it is most appropriate to remove them from the accessibility tree. This seemed to make VoiceOver's behavior more consistent. Fix #23202, #19798 --- src/cdk/a11y/live-announcer/live-announcer.ts | 5 +- src/dev-app/autocomplete/BUILD.bazel | 1 + .../autocomplete/autocomplete-demo.html | 7 ++ src/dev-app/autocomplete/autocomplete-demo.ts | 64 ++++++++++++++++++- src/dev-app/dialog/dialog-demo.html | 24 +++++-- src/dev-app/dialog/dialog-demo.ts | 23 +++++-- .../autocomplete/autocomplete-trigger.ts | 58 ++++++++++++++++- .../autocomplete/autocomplete.spec.ts | 12 ++-- .../testing/autocomplete-harness.ts | 7 +- src/material/core/option/option.html | 13 ++-- src/material/select/select.html | 10 --- src/material/select/select.spec.ts | 13 ---- src/material/select/select.ts | 59 +++++++++++++++++ src/material/snack-bar/snack-bar-container.ts | 6 +- .../material/autocomplete-testing.md | 2 + 15 files changed, 248 insertions(+), 56 deletions(-) diff --git a/src/cdk/a11y/live-announcer/live-announcer.ts b/src/cdk/a11y/live-announcer/live-announcer.ts index 9706aa96c0b1..ecd3b9fd7691 100644 --- a/src/cdk/a11y/live-announcer/live-announcer.ts +++ b/src/cdk/a11y/live-announcer/live-announcer.ts @@ -190,9 +190,8 @@ export class LiveAnnouncer implements OnDestroy { * pointing the `aria-owns` of all modals to the live announcer element. */ private _exposeAnnouncerToModals(id: string) { - // Note that the selector here is limited to CDK overlays at the moment in order to reduce the - // section of the DOM we need to look through. This should cover all the cases we support, but - // the selector can be expanded if it turns out to be too narrow. + // TODO(http://github.com/angular/components/issues/26853): consider de-duplicating this with + // the `SnakBarContainer` and any other usages. const modals = this._document.querySelectorAll( 'body > .cdk-overlay-container [aria-modal="true"]', ); diff --git a/src/dev-app/autocomplete/BUILD.bazel b/src/dev-app/autocomplete/BUILD.bazel index efc5063c8511..27f1e500850e 100644 --- a/src/dev-app/autocomplete/BUILD.bazel +++ b/src/dev-app/autocomplete/BUILD.bazel @@ -14,6 +14,7 @@ ng_module( "//src/material/button", "//src/material/card", "//src/material/checkbox", + "//src/material/dialog", "//src/material/form-field", "//src/material/input", "@npm//@angular/forms", diff --git a/src/dev-app/autocomplete/autocomplete-demo.html b/src/dev-app/autocomplete/autocomplete-demo.html index 080723351160..2502c1584c65 100644 --- a/src/dev-app/autocomplete/autocomplete-demo.html +++ b/src/dev-app/autocomplete/autocomplete-demo.html @@ -112,6 +112,13 @@ (ngModelChange)="filteredGroupedStates = filterStateGroups(currentGroupedState)"> + + + Autocomplete inside a Dialog + + + + diff --git a/src/dev-app/autocomplete/autocomplete-demo.ts b/src/dev-app/autocomplete/autocomplete-demo.ts index 3cfad904ea49..bb1f6ff02c68 100644 --- a/src/dev-app/autocomplete/autocomplete-demo.ts +++ b/src/dev-app/autocomplete/autocomplete-demo.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, ViewChild} from '@angular/core'; +import {Component, inject, ViewChild} from '@angular/core'; import {FormControl, NgModel, FormsModule, ReactiveFormsModule} from '@angular/forms'; import {CommonModule} from '@angular/common'; import {MatAutocompleteModule} from '@angular/material/autocomplete'; @@ -17,6 +17,7 @@ import {MatInputModule} from '@angular/material/input'; import {Observable} from 'rxjs'; import {map, startWith} from 'rxjs/operators'; import {ThemePalette} from '@angular/material/core'; +import {MatDialog, MatDialogModule, MatDialogRef} from '@angular/material/dialog'; export interface State { code: string; @@ -43,6 +44,7 @@ type DisableStateOption = 'none' | 'first-middle-last' | 'all'; MatButtonModule, MatCardModule, MatCheckboxModule, + MatDialogModule, MatInputModule, ReactiveFormsModule, ], @@ -202,4 +204,64 @@ export class AutocompleteDemo { } return false; } + + dialog = inject(MatDialog); + dialogRef: MatDialogRef | null; + + openDialog() { + this.dialogRef = this.dialog.open(AutocompleteDemoExampleDialog, {width: '400px'}); + } +} + +@Component({ + selector: 'autocomplete-demo-example-dialog', + template: ` +
+

Choose a T-shirt size.

+ + T-Shirt Size + + + + {{size}} + + + + + +
+ `, + styles: [ + ` + :host { + display: block; + padding: 20px; + } + + form { + display: flex; + flex-direction: column; + align-items: flex-start; + } + `, + ], + standalone: true, + imports: [ + CommonModule, + FormsModule, + MatAutocompleteModule, + MatButtonModule, + MatDialogModule, + MatInputModule, + ], +}) +export class AutocompleteDemoExampleDialog { + constructor(public dialogRef: MatDialogRef) {} + + currentSize = ''; + sizes = ['S', 'M', 'L']; + + close() { + this.dialogRef.close(); + } } diff --git a/src/dev-app/dialog/dialog-demo.html b/src/dev-app/dialog/dialog-demo.html index 8aa566b7d247..5a00217091ca 100644 --- a/src/dev-app/dialog/dialog-demo.html +++ b/src/dev-app/dialog/dialog-demo.html @@ -116,18 +116,30 @@

Other options

Last beforeClose result: {{lastBeforeCloseResult}}

- I'm a template dialog. I've been opened {{numTemplateOpens}} times! - -

It's Jazz!

+

Order printer ink refills.

- How much? + How many? + + What color? + + + Black + Cyan + Magenta + Yellow + + +

{{ data.message }}

- + @@ -141,7 +156,7 @@ export class DialogDemo { encapsulation: ViewEncapsulation.None, styles: [`.hidden-dialog { opacity: 0; }`], standalone: true, - imports: [MatInputModule, DragDropModule], + imports: [DragDropModule, MatInputModule, MatSelectModule], }) export class JazzDialog { private _dimensionToggle = false; diff --git a/src/material/autocomplete/autocomplete-trigger.ts b/src/material/autocomplete/autocomplete-trigger.ts index d3a39199bf2c..448465979a6a 100644 --- a/src/material/autocomplete/autocomplete-trigger.ts +++ b/src/material/autocomplete/autocomplete-trigger.ts @@ -244,6 +244,7 @@ export abstract class _MatAutocompleteTriggerBase this._componentDestroyed = true; this._destroyPanel(); this._closeKeyEventStream.complete(); + this._clearFromModals(); } /** Whether or not the autocomplete panel is open. */ @@ -670,6 +671,8 @@ export abstract class _MatAutocompleteTriggerBase this.autocomplete._isOpen = this._overlayAttached = true; this.autocomplete._setColor(this._formField?.color); + this._exposeToModals(); + // We need to do an extra `panelOpen` check in here, because the // autocomplete won't be shown if there are no options. if (this.panelOpen && wasOpen !== this.panelOpen) { @@ -858,6 +861,59 @@ export abstract class _MatAutocompleteTriggerBase // but the behvior isn't exactly the same and it ends up breaking some internal tests. overlayRef.outsidePointerEvents().subscribe(); } + + private _trackedModals = new Set(); + + /** + * Some browsers won't expose the accessibility node of the listbox overlay if there is an + * `aria-modal` and the overlay element is outside of it. This method works around the issue by + * pointing the `aria-owns` of all modals to the overlay element. + * + * While aria-owns is not required for the ARIA 1.2 `role="combobox"` interaction pattern, + * it fixes an issue with VoiceOver when the autocomplete appears inside of an `aria-model="true"` + * element (e.g. a dialog). Without this `aria-owns`, the `aria-modal` on a dialog prevents + * VoiceOver from "seeing" the autocomplete's listbox overlay for aria-activedescendant. + * Using `aria-owns` re-parents the autocomplete overlay so that it works again. + * See https://github.com/angular/components/issues/20694 + */ + private _exposeToModals() { + // TODO(http://github.com/angular/components/issues/26853): consider de-duplicating this with + // the `LiveAnnouncer` and any other usages. + const id = this.autocomplete.id; + const modals = this._document.querySelectorAll( + 'body > .cdk-overlay-container [aria-modal="true"]', + ); + + for (let i = 0; i < modals.length; i++) { + const modal = modals[i]; + const ariaOwns = modal.getAttribute('aria-owns'); + this._trackedModals.add(modal); + + if (!ariaOwns) { + modal.setAttribute('aria-owns', id); + } else if (ariaOwns.indexOf(id) === -1) { + modal.setAttribute('aria-owns', ariaOwns + ' ' + id); + } + } + } + + /** Clears the references to the listbox overlay element from any modals it was added to. */ + private _clearFromModals() { + this._trackedModals.forEach(modal => { + const ariaOwns = modal.getAttribute('aria-owns'); + + if (ariaOwns) { + const newValue = ariaOwns.replace(`${this.autocomplete.id}-panel`, '').trim(); + + if (newValue.length > 0) { + modal.setAttribute('aria-owns', newValue); + } else { + modal.removeAttribute('aria-owns'); + } + } + }); + this._trackedModals.clear(); + } } @Directive({ @@ -869,7 +925,7 @@ export abstract class _MatAutocompleteTriggerBase '[attr.aria-autocomplete]': 'autocompleteDisabled ? null : "list"', '[attr.aria-activedescendant]': '(panelOpen && activeOption) ? activeOption.id : null', '[attr.aria-expanded]': 'autocompleteDisabled ? null : panelOpen.toString()', - '[attr.aria-owns]': '(autocompleteDisabled || !panelOpen) ? null : autocomplete?.id', + '[attr.aria-controls]': '(autocompleteDisabled || !panelOpen) ? null : autocomplete?.id', '[attr.aria-haspopup]': 'autocompleteDisabled ? null : "listbox"', // Note: we use `focusin`, as opposed to `focus`, in order to open the panel // a little earlier. This avoids issues where IE delays the focusing of the input. diff --git a/src/material/autocomplete/autocomplete.spec.ts b/src/material/autocomplete/autocomplete.spec.ts index df5264503e01..94c4be351633 100644 --- a/src/material/autocomplete/autocomplete.spec.ts +++ b/src/material/autocomplete/autocomplete.spec.ts @@ -1863,7 +1863,7 @@ describe('MDC-based MatAutocomplete', () => { .toBe('false'); })); - it('should set aria-owns based on the attached autocomplete', () => { + it('should set aria-controls based on the attached autocomplete', () => { fixture.componentInstance.trigger.openPanel(); fixture.detectChanges(); @@ -1871,18 +1871,18 @@ describe('MDC-based MatAutocomplete', () => { By.css('.mat-mdc-autocomplete-panel'), )!.nativeElement; - expect(input.getAttribute('aria-owns')) - .withContext('Expected aria-owns to match attached autocomplete.') + expect(input.getAttribute('aria-controls')) + .withContext('Expected aria-controls to match attached autocomplete.') .toBe(panel.getAttribute('id')); }); - it('should not set aria-owns while the autocomplete is closed', () => { - expect(input.getAttribute('aria-owns')).toBeFalsy(); + it('should not set aria-controls while the autocomplete is closed', () => { + expect(input.getAttribute('aria-controls')).toBeFalsy(); fixture.componentInstance.trigger.openPanel(); fixture.detectChanges(); - expect(input.getAttribute('aria-owns')).toBeTruthy(); + expect(input.getAttribute('aria-controls')).toBeTruthy(); }); it('should restore focus to the input when clicking to select a value', fakeAsync(() => { diff --git a/src/material/autocomplete/testing/autocomplete-harness.ts b/src/material/autocomplete/testing/autocomplete-harness.ts index 93ff6ccf9255..a777e7c34dbc 100644 --- a/src/material/autocomplete/testing/autocomplete-harness.ts +++ b/src/material/autocomplete/testing/autocomplete-harness.ts @@ -129,7 +129,7 @@ export abstract class _MatAutocompleteHarnessBase< } /** Gets the selector that can be used to find the autocomplete trigger's panel. */ - private async _getPanelSelector(): Promise { + protected async _getPanelSelector(): Promise { return `#${await (await this.host()).getAttribute('aria-owns')}`; } } @@ -168,4 +168,9 @@ export class MatAutocompleteHarness extends _MatAutocompleteHarnessBase< return (await harness.isDisabled()) === disabled; }); } + + /** Gets the selector that can be used to find the autocomplete trigger's panel. */ + protected override async _getPanelSelector(): Promise { + return `#${await (await this.host()).getAttribute('aria-controls')}`; + } } diff --git a/src/material/core/option/option.html b/src/material/core/option/option.html index aff571f282f3..19adfc3b2ac2 100644 --- a/src/material/core/option/option.html +++ b/src/material/core/option/option.html @@ -1,5 +1,5 @@ - + @@ -7,13 +7,12 @@ + class="mat-mdc-option-pseudo-checkbox" [disabled]="disabled" state="checked" + [attr.aria-hidden]="'true'" appearance="minimal"> ({{ group.label }}) -
+
diff --git a/src/material/select/select.html b/src/material/select/select.html index 3a1c41419414..5ee5974aa442 100644 --- a/src/material/select/select.html +++ b/src/material/select/select.html @@ -1,14 +1,4 @@ -
{ expect(ariaControls).toBe(document.querySelector('.mat-mdc-select-panel')!.id); })); - it('should point the aria-owns attribute to the listbox on the trigger', fakeAsync(() => { - const trigger = select.querySelector('.mat-mdc-select-trigger')!; - expect(trigger.hasAttribute('aria-owns')).toBe(false); - - fixture.componentInstance.select.open(); - fixture.detectChanges(); - flush(); - - const ariaOwns = trigger.getAttribute('aria-owns'); - expect(ariaOwns).toBeTruthy(); - expect(ariaOwns).toBe(document.querySelector('.mat-mdc-select-panel')!.id); - })); - it('should set aria-expanded based on the select open state', fakeAsync(() => { expect(select.getAttribute('aria-expanded')).toBe('false'); diff --git a/src/material/select/select.ts b/src/material/select/select.ts index 6c518afaf31e..effa4b7423a3 100644 --- a/src/material/select/select.ts +++ b/src/material/select/select.ts @@ -33,6 +33,7 @@ import { ScrollStrategy, } from '@angular/cdk/overlay'; import {ViewportRuler} from '@angular/cdk/scrolling'; +import {DOCUMENT} from '@angular/common'; import { AfterContentInit, AfterViewInit, @@ -46,6 +47,7 @@ import { DoCheck, ElementRef, EventEmitter, + inject, Inject, InjectionToken, Input, @@ -588,6 +590,7 @@ export abstract class _MatSelectBase this._destroy.next(); this._destroy.complete(); this.stateChanges.complete(); + this._clearFromModals(); } /** Toggles the overlay panel open or closed. */ @@ -598,6 +601,8 @@ export abstract class _MatSelectBase /** Opens the overlay panel. */ open(): void { if (this._canOpen()) { + this._exposeToModals(); + this._panelOpen = true; this._keyManager.withHorizontalOrientation(null); this._highlightCorrectOption(); @@ -605,6 +610,60 @@ export abstract class _MatSelectBase } } + private _document = inject(DOCUMENT); + private _trackedModals = new Set(); + + /** + * Some browsers won't expose the accessibility node of the listbox overlay if there is an + * `aria-modal` and the overlay element is outside of it. This method works around the issue by + * pointing the `aria-owns` of all modals to the overlay element. + * + * While aria-owns is not required for the ARIA 1.2 `role="combobox"` interaction pattern, + * it fixes an issue with VoiceOver when the select appears inside of an `aria-model="true"` + * element (e.g. a dialog). Without this `aria-owns`, the `aria-modal` on a dialog prevents + * VoiceOver from "seeing" the select's listbox overlay for aria-activedescendant. + * Using `aria-owns` re-parents the select overlay so that it works again. + * See https://github.com/angular/components/issues/20694 + */ + private _exposeToModals() { + // TODO(http://github.com/angular/components/issues/26853): consider de-duplicating this with + // the `LiveAnnouncer` and any other usages. + const id = `${this.id}-panel`; + const modals = this._document.querySelectorAll( + 'body > .cdk-overlay-container [aria-modal="true"]', + ); + + for (let i = 0; i < modals.length; i++) { + const modal = modals[i]; + const ariaOwns = modal.getAttribute('aria-owns'); + this._trackedModals.add(modal); + + if (!ariaOwns) { + modal.setAttribute('aria-owns', id); + } else if (ariaOwns.indexOf(id) === -1) { + modal.setAttribute('aria-owns', ariaOwns + ' ' + id); + } + } + } + + /** Clears the references to the listbox overlay element from any modals it was added to. */ + private _clearFromModals() { + this._trackedModals.forEach(modal => { + const ariaOwns = modal.getAttribute('aria-owns'); + + if (ariaOwns) { + const newValue = ariaOwns.replace(`${this.id}-panel`, '').trim(); + + if (newValue.length > 0) { + modal.setAttribute('aria-owns', newValue); + } else { + modal.removeAttribute('aria-owns'); + } + } + }); + this._trackedModals.clear(); + } + /** Closes the overlay panel and focuses the host element. */ close(): void { if (this._panelOpen) { diff --git a/src/material/snack-bar/snack-bar-container.ts b/src/material/snack-bar/snack-bar-container.ts index 1a792f7e79d0..f726b9fe31ef 100644 --- a/src/material/snack-bar/snack-bar-container.ts +++ b/src/material/snack-bar/snack-bar-container.ts @@ -241,10 +241,8 @@ export abstract class _MatSnackBarContainerBase extends BasePortalOutlet impleme * pointing the `aria-owns` of all modals to the live element. */ private _exposeToModals() { - // TODO(crisbeto): consider de-duplicating this with the `LiveAnnouncer`. - // Note that the selector here is limited to CDK overlays at the moment in order to reduce the - // section of the DOM we need to look through. This should cover all the cases we support, but - // the selector can be expanded if it turns out to be too narrow. + // TODO(http://github.com/angular/components/issues/26853): consider de-duplicating this with the + // `LiveAnnouncer` and any other usages. const id = this._liveElementId; const modals = this._document.querySelectorAll( 'body > .cdk-overlay-container [aria-modal="true"]', diff --git a/tools/public_api_guard/material/autocomplete-testing.md b/tools/public_api_guard/material/autocomplete-testing.md index 378cbfae9e3a..48cfcb73d04b 100644 --- a/tools/public_api_guard/material/autocomplete-testing.md +++ b/tools/public_api_guard/material/autocomplete-testing.md @@ -21,6 +21,7 @@ export interface AutocompleteHarnessFilters extends BaseHarnessFilters { // @public export class MatAutocompleteHarness extends _MatAutocompleteHarnessBase { + protected _getPanelSelector(): Promise; static hostSelector: string; // (undocumented) protected _optionClass: typeof MatOptionHarness; @@ -45,6 +46,7 @@ export abstract class _MatAutocompleteHarnessBase; getOptionGroups(filters?: Omit): Promise; getOptions(filters?: Omit): Promise; + protected _getPanelSelector(): Promise; getValue(): Promise; isDisabled(): Promise; isFocused(): Promise;