Skip to content

Commit

Permalink
fix(multiple): fix VoiceOver confused by Select/Autocomplete's ARIA s…
Browse files Browse the repository at this point in the history
…emantics

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 angular#23202, angular#19798
  • Loading branch information
zarend committed Apr 14, 2023
1 parent 97410fa commit 63b9472
Show file tree
Hide file tree
Showing 18 changed files with 268 additions and 51 deletions.
3 changes: 3 additions & 0 deletions src/cdk/a11y/live-announcer/live-announcer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ export class LiveAnnouncer implements OnDestroy {
* pointing the `aria-owns` of all modals to the live announcer element.
*/
private _exposeAnnouncerToModals(id: string) {
// TODO(http://github.com/angular/components/issues/26853): consider de-duplicating this with
// the `SnakBarContainer` and other usages.
//
// 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.
Expand Down
1 change: 1 addition & 0 deletions src/cdk/a11y/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
export * from './aria-describer/aria-describer';
export * from './aria-describer/aria-reference';
export * from './key-manager/activedescendant-key-manager';
export * from './key-manager/focus-key-manager';
export * from './key-manager/list-key-manager';
Expand Down
1 change: 1 addition & 0 deletions src/dev-app/autocomplete/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions src/dev-app/autocomplete/autocomplete-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@
(ngModelChange)="filteredGroupedStates = filterStateGroups(currentGroupedState)">
</mat-form-field>
</mat-card>

<mat-card>
<mat-card-subtitle>Autocomplete inside a Dialog</mat-card-subtitle>
<mat-card-content>
<button mat-button (click)="openDialog()">Open dialog</button>
</mat-card-content>
</mat-card>
</div>

<mat-autocomplete #groupedAuto="matAutocomplete">
Expand Down
64 changes: 63 additions & 1 deletion src/dev-app/autocomplete/autocomplete-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -43,6 +44,7 @@ type DisableStateOption = 'none' | 'first-middle-last' | 'all';
MatButtonModule,
MatCardModule,
MatCheckboxModule,
MatDialogModule,
MatInputModule,
ReactiveFormsModule,
],
Expand Down Expand Up @@ -202,4 +204,64 @@ export class AutocompleteDemo {
}
return false;
}

dialog = inject(MatDialog);
dialogRef: MatDialogRef<AutocompleteDemoExampleDialog> | null;

openDialog() {
this.dialogRef = this.dialog.open(AutocompleteDemoExampleDialog, {width: '400px'});
}
}

@Component({
selector: 'autocomplete-demo-example-dialog',
template: `
<form (submit)="close()">
<p>Choose a T-shirt size.</p>
<mat-form-field>
<mat-label>T-Shirt Size</mat-label>
<input matInput [matAutocomplete]="tdAuto" [(ngModel)]="currentSize" name="size">
<mat-autocomplete #tdAuto="matAutocomplete">
<mat-option *ngFor="let size of sizes" [value]="size">
{{size}}
</mat-option>
</mat-autocomplete>
</mat-form-field>
<button type="submit" mat-button>Close</button>
</form>
`,
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<AutocompleteDemoExampleDialog>) {}

currentSize = '';
sizes = ['S', 'M', 'L'];

close() {
this.dialogRef.close();
}
}
24 changes: 18 additions & 6 deletions src/dev-app/dialog/dialog-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,30 @@ <h2>Other options</h2>
<p>Last beforeClose result: {{lastBeforeCloseResult}}</p>

<ng-template let-data let-dialogRef="dialogRef">
I'm a template dialog. I've been opened {{numTemplateOpens}} times!

<p>It's Jazz!</p>
<p>Order printer ink refills.</p>

<mat-form-field>
<mat-label>How much?</mat-label>
<mat-label>How many?</mat-label>
<input matInput #howMuch>
</mat-form-field>

<mat-form-field>
<mat-label>What color?</mat-label>
<mat-select #whatColor>
<mat-option></mat-option>
<mat-option value="black">Black</mat-option>
<mat-option value="cyan">Cyan</mat-option>
<mat-option value="magenta">Magenta</mat-option>
<mat-option value="yellow">Yellow</mat-option>
</mat-select>
</mat-form-field>

<p> {{ data.message }} </p>
<button type="button" (click)="dialogRef.close(howMuch.value)" class="demo-dialog-button"
cdkFocusInitial>

<p>I'm a template dialog. I've been opened {{numTemplateOpens}} times!</p>

<button type="button" class="demo-dialog-button" cdkFocusInitial
(click)="dialogRef.close({ quantity: howMuch.value, color: whatColor.value })">
Close dialog
</button>
<button (click)="dialogRef.updateSize('500px', '500px').updatePosition({top: '25px', left: '25px'});"
Expand Down
23 changes: 19 additions & 4 deletions src/dev-app/dialog/dialog-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,38 @@ export class DialogDemo {
selector: 'demo-jazz-dialog',
template: `
<div cdkDrag cdkDragRootElement=".cdk-overlay-pane">
<p>It's Jazz!</p>
<p>Order printer ink refills.</p>
<mat-form-field>
<mat-label>How much?</mat-label>
<mat-label>How many?</mat-label>
<input matInput #howMuch>
</mat-form-field>
<mat-form-field>
<mat-label>What color?</mat-label>
<mat-select #whatColor>
<mat-option></mat-option>
<mat-option value="black">Black</mat-option>
<mat-option value="cyan">Cyan</mat-option>
<mat-option value="magenta">Magenta</mat-option>
<mat-option value="yellow">Yellow</mat-option>
</mat-select>
</mat-form-field>
<p cdkDragHandle> {{ data.message }} </p>
<button type="button" (click)="dialogRef.close(howMuch.value)">Close dialog</button>
<button type="button" class="demo-dialog-button"
(click)="dialogRef.close({ quantity: howMuch.value, color: whatColor.value })">
Close dialog
</button>
<button (click)="togglePosition()">Change dimensions</button>
<button (click)="temporarilyHide()">Hide for 2 seconds</button>
</div>
`,
encapsulation: ViewEncapsulation.None,
styles: [`.hidden-dialog { opacity: 0; }`],
standalone: true,
imports: [MatInputModule, DragDropModule],
imports: [DragDropModule, MatInputModule, MatSelectModule],
})
export class JazzDialog {
private _dimensionToggle = false;
Expand Down
1 change: 1 addition & 0 deletions src/material/autocomplete/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ng_module(
":autocomplete_scss",
] + glob(["**/*.html"]),
deps = [
"//src/cdk/a11y",
"//src/cdk/coercion",
"//src/cdk/overlay",
"//src/cdk/scrolling",
Expand Down
64 changes: 63 additions & 1 deletion src/material/autocomplete/autocomplete-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {addAriaReferencedId, removeAriaReferencedId} from '@angular/cdk/a11y';
import {
AfterViewInit,
ChangeDetectorRef,
Expand Down Expand Up @@ -244,6 +245,7 @@ export abstract class _MatAutocompleteTriggerBase
this._componentDestroyed = true;
this._destroyPanel();
this._closeKeyEventStream.complete();
this._clearFromModals();
}

/** Whether or not the autocomplete panel is open. */
Expand Down Expand Up @@ -670,6 +672,8 @@ export abstract class _MatAutocompleteTriggerBase
this.autocomplete._isOpen = this._overlayAttached = true;
this.autocomplete._setColor(this._formField?.color);

this._applyModalPanelOwnership();

// 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) {
Expand Down Expand Up @@ -858,6 +862,64 @@ export abstract class _MatAutocompleteTriggerBase
// but the behvior isn't exactly the same and it ends up breaking some internal tests.
overlayRef.outsidePointerEvents().subscribe();
}

/**
* Track what modals we have modified the `aria-owns` attribute of. When the combobox trigger is
* inside an aria-modal, we apply aria-owns to the parent modal with the `id` of the options
* panel. Track modals we have changed so we can undo the changes on destroy.
*/
private _trackedModals = new Set<Element>();

/**
* If the autocomplete trigger is inside of an `aria-modal` element, connect
* that modal to the options panel with `aria-owns`.
*
* For some browser + screen reader combinations, when navigation is inside
* of an `aria-modal` element, the screen reader treats everything outside
* of that modal as hidden or invisible.
*
* This causes a problem when the combobox trigger is _inside_ of a modal, because the
* options panel is rendered _outside_ of that modal, preventing screen reader navigation
* from reaching the panel.
*
* We can work around this issue by applying `aria-owns` to the modal with the `id` of
* the options panel. This effectively communicates to assistive technology that the
* options panel is part of the same interaction as the modal.
*
* At time of this writing, this issue is present in VoiceOver.
* See https://github.com/angular/components/issues/20694
*/
private _applyModalPanelOwnership() {
// TODO(http://github.com/angular/components/issues/26853): consider de-duplicating this with
// the `LiveAnnouncer` and any other usages.
//
// 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.
const modal = this._element.nativeElement.closest(
'body > .cdk-overlay-container [aria-modal="true"]',
);

if (!modal) {
// Most commonly, the autocomplete trigger is not inside a modal.
return;
}

const panelId = this.autocomplete.id;

addAriaReferencedId(modal, 'aria-owns', panelId);
this._trackedModals.add(modal);
}

/** Clears the references to the listbox overlay element from any modals it was added to. */
private _clearFromModals() {
for (const modal of this._trackedModals) {
const panelId = this.autocomplete.id;

removeAriaReferencedId(modal, 'aria-owns', panelId);
this._trackedModals.delete(modal);
}
}
}

@Directive({
Expand All @@ -869,7 +931,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.
Expand Down
12 changes: 6 additions & 6 deletions src/material/autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1863,26 +1863,26 @@ 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();

const panel = fixture.debugElement.query(
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(() => {
Expand Down
2 changes: 2 additions & 0 deletions src/material/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ export abstract class _MatAutocompleteBase
_classList: {[key: string]: boolean} = {};

/** Unique ID to be used by autocomplete trigger's "aria-owns" property. */
// This string needs to be Regex friendly, so that it can be used in the regex to attach this
// panel to aria modals. See `_applyModalPanelOwnership` method in autocomplete-trigger.ts.
id: string = `mat-autocomplete-${_uniqueAutocompleteIdCounter++}`;

/**
Expand Down
7 changes: 6 additions & 1 deletion src/material/autocomplete/testing/autocomplete-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> {
protected async _getPanelSelector(): Promise<string> {
return `#${await (await this.host()).getAttribute('aria-owns')}`;
}
}
Expand Down Expand Up @@ -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<string> {
return `#${await (await this.host()).getAttribute('aria-controls')}`;
}
}
Loading

0 comments on commit 63b9472

Please sign in to comment.