From 0d666a51d98af23db30b14dcfe236c16a082a0a9 Mon Sep 17 00:00:00 2001 From: Vanessa Schmitt Date: Tue, 26 Mar 2019 13:36:26 -0700 Subject: [PATCH] fix(option): Remove aria-selected='false' in single-selection mode Remove the aria-selected attribute from unselected options in single selection mode. The multi-selection behavior is unchanged. Motivation: the screen reader NVDA announces 'not selected' on any element that has aria-selected='false', which is disruptive when a user is navigating through a long list of options. The W3 aria best practices example only uses aria-selected='true for single selection', false is implicit: https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html --- src/lib/core/option/option.ts | 12 ++++++- src/lib/select/select.spec.ts | 51 ++++++++++++++++++++++------ tools/public_api_guard/lib/core.d.ts | 1 + 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/lib/core/option/option.ts b/src/lib/core/option/option.ts index 7955684b2f7b..18f2441366c7 100644 --- a/src/lib/core/option/option.ts +++ b/src/lib/core/option/option.ts @@ -72,7 +72,7 @@ export const MAT_OPTION_PARENT_COMPONENT = '[class.mat-option-multiple]': 'multiple', '[class.mat-active]': 'active', '[id]': 'id', - '[attr.aria-selected]': 'selected.toString()', + '[attr.aria-selected]': '_getAriaSelected()', '[attr.aria-disabled]': 'disabled.toString()', '[class.mat-option-disabled]': 'disabled', '(click)': '_selectViaInteraction()', @@ -220,6 +220,16 @@ export class MatOption implements AfterViewChecked, OnDestroy { } } + /** + * 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'; diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 5aebeef226e3..d2e924fe102e 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -863,7 +863,7 @@ describe('MatSelect', () => { describe('for options', () => { let fixture: ComponentFixture; let trigger: HTMLElement; - let options: NodeListOf; + let options: Array; beforeEach(fakeAsync(() => { fixture = TestBed.createComponent(BasicSelect); @@ -872,8 +872,7 @@ describe('MatSelect', () => { trigger.click(); fixture.detectChanges(); - options = - overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; + options = Array.from(overlayContainerElement.querySelectorAll('mat-option')); })); it('should set the role of mat-option to option', fakeAsync(() => { @@ -882,10 +881,9 @@ describe('MatSelect', () => { expect(options[2].getAttribute('role')).toEqual('option'); })); - it('should set aria-selected on each option', fakeAsync(() => { - expect(options[0].getAttribute('aria-selected')).toEqual('false'); - expect(options[1].getAttribute('aria-selected')).toEqual('false'); - expect(options[2].getAttribute('aria-selected')).toEqual('false'); + it('should set aria-selected on each option for single select', fakeAsync(() => { + expect(options.every(option => !option.hasAttribute('aria-selected'))).toBe(true, + 'Expected all unselected single-select options not to have aria-selected set.'); options[1].click(); fixture.detectChanges(); @@ -894,11 +892,44 @@ describe('MatSelect', () => { fixture.detectChanges(); flush(); - expect(options[0].getAttribute('aria-selected')).toEqual('false'); - expect(options[1].getAttribute('aria-selected')).toEqual('true'); - expect(options[2].getAttribute('aria-selected')).toEqual('false'); + expect(options[1].getAttribute('aria-selected')).toEqual('true', + 'Expected selected single-select option to have aria-selected="true".'); + options.splice(1, 1); + expect(options.every(option => !option.hasAttribute('aria-selected'))).toBe(true, + 'Expected all unselected single-select options not to have aria-selected set.'); })); + it('should set aria-selected on each option for multi-select', fakeAsync(() => { + fixture.destroy(); + + const multiFixture = TestBed.createComponent(MultiSelect); + multiFixture.detectChanges(); + + trigger = multiFixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; + trigger.click(); + multiFixture.detectChanges(); + + options = Array.from(overlayContainerElement.querySelectorAll('mat-option')); + + expect(options.every(option => option.hasAttribute('aria-selected') && + option.getAttribute('aria-selected') === 'false')).toBe(true, + 'Expected all unselected multi-select options to have aria-selected="false".'); + + options[1].click(); + multiFixture.detectChanges(); + + trigger.click(); + multiFixture.detectChanges(); + flush(); + + expect(options[1].getAttribute('aria-selected')).toEqual('true', + 'Expected selected multi-select option to have aria-selected="true".'); + options.splice(1, 1); + expect(options.every(option => option.hasAttribute('aria-selected') && + option.getAttribute('aria-selected') === 'false')).toBe(true, + 'Expected all unselected multi-select options to have aria-selected="false".'); + })); + it('should set the tabindex of each option according to disabled state', fakeAsync(() => { expect(options[0].getAttribute('tabindex')).toEqual('0'); expect(options[1].getAttribute('tabindex')).toEqual('0'); diff --git a/tools/public_api_guard/lib/core.d.ts b/tools/public_api_guard/lib/core.d.ts index 44b2a292f0fa..631d05f83adf 100644 --- a/tools/public_api_guard/lib/core.d.ts +++ b/tools/public_api_guard/lib/core.d.ts @@ -244,6 +244,7 @@ export declare class MatOption implements AfterViewChecked, OnDestroy { value: any; readonly viewValue: string; constructor(_element: ElementRef, _changeDetectorRef: ChangeDetectorRef, _parent: MatOptionParentComponent, group: MatOptgroup); + _getAriaSelected(): boolean | null; _getHostElement(): HTMLElement; _getTabIndex(): string; _handleKeydown(event: KeyboardEvent): void;