From ac9e95db4ceda7bd9eddd84fd06617ca4556e3e9 Mon Sep 17 00:00:00 2001 From: Zach Arend Date: Wed, 5 Oct 2022 18:25:26 +0000 Subject: [PATCH] fix(material/core): don't remove aria-selected from deselected options For mat-option, set `aria-selected="false"` on deselected options that are selectable. Conforms with [WAI ARIA Listbox authoring practices guide]( https://www.w3.org/WAI/ARIA/apg/patterns/listbox/), which says to always include aria-selected attribute on options that are selectable. Fix issue where voiceover reads every option as "selected" (#25736). Fix #25736 --- src/material/core/option/option.ts | 23 ++++++++++++----------- src/material/legacy-core/option/option.ts | 15 ++++++++++++++- src/material/legacy-select/select.spec.ts | 8 ++++---- src/material/select/select.spec.ts | 8 ++++---- tools/public_api_guard/material/core.md | 1 - 5 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/material/core/option/option.ts b/src/material/core/option/option.ts index 6a9fe92f2fce..53f4e89599a5 100644 --- a/src/material/core/option/option.ts +++ b/src/material/core/option/option.ts @@ -195,16 +195,6 @@ export class _MatOptionBase implements FocusableOption, AfterViewChecke } } - /** - * Gets the `aria-selected` value for the option. We explicitly omit the `aria-selected` - * attribute from single-selection, unselected options. Including the `aria-selected="false"` - * attributes adds a significant amount of noise to screen-reader users without providing useful - * information. - */ - _getAriaSelected(): boolean | null { - return this.selected || (this.multiple ? false : null); - } - /** Returns the correct tabindex for the option depending on disabled state. */ _getTabIndex(): string { return this.disabled ? '-1' : '0'; @@ -255,7 +245,18 @@ export class _MatOptionBase implements FocusableOption, AfterViewChecke '[class.mat-mdc-option-active]': 'active', '[class.mdc-list-item--disabled]': 'disabled', '[id]': 'id', - '[attr.aria-selected]': '_getAriaSelected()', + // The aria-selected attribute applied to the option conforms to WAI ARIA best practices for + // listbox interaction pattern. + // + // From [WAI ARIA Listbox authoring practices guide]( + // https://www.w3.org/WAI/ARIA/apg/patterns/listbox/): + // "If any options are selected, each selected option has either aria-selected or aria-checked + // set to true. All options that are selectable but not selected have either aria-selected or + // aria-checked set to false." + // + // Set `aria-selected="false"` on not-selected listbox options that are selectable to fix + // VoiceOver reading every option as "selected" (#21491). + '[attr.aria-selected]': 'selected', '[attr.aria-disabled]': 'disabled.toString()', '(click)': '_selectViaInteraction()', '(keydown)': '_handleKeydown($event)', diff --git a/src/material/legacy-core/option/option.ts b/src/material/legacy-core/option/option.ts index 696af7fcf96f..984981b2d218 100644 --- a/src/material/legacy-core/option/option.ts +++ b/src/material/legacy-core/option/option.ts @@ -27,6 +27,19 @@ import {MatLegacyOptgroup} from './optgroup'; * Single option inside of a `` element. * @deprecated Use `MatOption` from `@angular/material/core` instead. See https://material.angular.io/guide/mdc-migration for information about migrating. * @breaking-change 17.0.0 + * + * The aria-selected attribute applied to the option conforms to WAI ARIA best practices for listbox + * interaction patterns. + * + * From [WAI ARIA Listbox authoring practices guide]( + * https://www.w3.org/WAI/ARIA/apg/patterns/listbox/): + * "If any options are selected, each selected option has either aria-selected or aria-checked + * set to true. All options that are selectable but not selected have either aria-selected or + * aria-checked set to false." + * + * Set `aria-selected="false"` on not-selected listbox options that are selectable to fix + * VoiceOver reading every option as "selected" (#25736). Also fixes chromevox not announcing + * options as selectable. */ @Component({ selector: 'mat-option', @@ -38,7 +51,7 @@ import {MatLegacyOptgroup} from './optgroup'; '[class.mat-option-multiple]': 'multiple', '[class.mat-active]': 'active', '[id]': 'id', - '[attr.aria-selected]': '_getAriaSelected()', + '[attr.aria-selected]': 'selected', '[attr.aria-disabled]': 'disabled.toString()', '[class.mat-option-disabled]': 'disabled', '(click)': '_selectViaInteraction()', diff --git a/src/material/legacy-select/select.spec.ts b/src/material/legacy-select/select.spec.ts index 79235a112ab0..839f14508533 100644 --- a/src/material/legacy-select/select.spec.ts +++ b/src/material/legacy-select/select.spec.ts @@ -1135,9 +1135,9 @@ describe('MatSelect', () => { })); it('should set aria-selected on each option for single select', fakeAsync(() => { - expect(options.every(option => !option.hasAttribute('aria-selected'))) + expect(options.every(option => option.getAttribute('aria-selected') === 'false')) .withContext( - 'Expected all unselected single-select options not to have ' + 'aria-selected set.', + 'Expected all unselected single-select options to have aria-selected="false" set.', ) .toBe(true); @@ -1152,9 +1152,9 @@ describe('MatSelect', () => { .withContext('Expected selected single-select option to have aria-selected="true".') .toEqual('true'); options.splice(1, 1); - expect(options.every(option => !option.hasAttribute('aria-selected'))) + expect(options.every(option => option.getAttribute('aria-selected') === 'false')) .withContext( - 'Expected all unselected single-select options not to have ' + 'aria-selected set.', + 'Expected all unselected single-select options to have aria-selected="false" set.', ) .toBe(true); })); diff --git a/src/material/select/select.spec.ts b/src/material/select/select.spec.ts index 76bd740012b9..74cde96c3ec5 100644 --- a/src/material/select/select.spec.ts +++ b/src/material/select/select.spec.ts @@ -1168,9 +1168,9 @@ describe('MDC-based MatSelect', () => { })); it('should set aria-selected on each option for single select', fakeAsync(() => { - expect(options.every(option => !option.hasAttribute('aria-selected'))) + expect(options.every(option => option.getAttribute('aria-selected') === 'false')) .withContext( - 'Expected all unselected single-select options not to have ' + 'aria-selected set.', + 'Expected all unselected single-select options to have aria-selected="false" set.', ) .toBe(true); @@ -1187,9 +1187,9 @@ describe('MDC-based MatSelect', () => { ) .toEqual('true'); options.splice(1, 1); - expect(options.every(option => !option.hasAttribute('aria-selected'))) + expect(options.every(option => option.getAttribute('aria-selected') === 'false')) .withContext( - 'Expected all unselected single-select options not to have ' + 'aria-selected set.', + 'Expected all unselected single-select options to have aria-selected="false" set.', ) .toBe(true); })); diff --git a/tools/public_api_guard/material/core.md b/tools/public_api_guard/material/core.md index 3ff072128465..68a6847288c6 100644 --- a/tools/public_api_guard/material/core.md +++ b/tools/public_api_guard/material/core.md @@ -280,7 +280,6 @@ export class _MatOptionBase implements FocusableOption, AfterViewChecke set disabled(value: BooleanInput); get disableRipple(): boolean; focus(_origin?: FocusOrigin, options?: FocusOptions): void; - _getAriaSelected(): boolean | null; _getHostElement(): HTMLElement; getLabel(): string; _getTabIndex(): string;