From ca8273fb75e346a79d113cb4b2766987eff6045c Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 25 Feb 2018 19:33:40 +0100 Subject: [PATCH] fix(selection-list): improve accessibility of selection list * Since the selection list is still a `list` that contains some content, there should be a way for screenreader users, to navigate through the options (even if disabled). With this change, if the list is disabled, it's still possible to walk through the options. * Adds a new functionality to the `ListKeyManager` that allows the developer to control the items that can't be focused. `skipPredicate`. (e.g. for the selection list we want to make sure that users can navigate using the arrow keys to disabled items as well) * Testing: Fixes that by default all fake events bubble up the DOM. This works in most of the cases, but it's wrong to always bubble events. e.g. `focus` doesn't bubble. Fixes #9995 --- .../a11y/key-manager/list-key-manager.spec.ts | 30 ++++++++++++++++- src/cdk/a11y/key-manager/list-key-manager.ts | 24 ++++++++++++-- src/cdk/testing/event-objects.ts | 2 +- src/lib/list/_list-theme.scss | 11 +------ src/lib/list/list-option.html | 3 +- src/lib/list/list.scss | 15 +++------ src/lib/list/selection-list.spec.ts | 15 +++------ src/lib/list/selection-list.ts | 32 +++++++++++-------- 8 files changed, 81 insertions(+), 51 deletions(-) diff --git a/src/cdk/a11y/key-manager/list-key-manager.spec.ts b/src/cdk/a11y/key-manager/list-key-manager.spec.ts index 303c151a200d..0434bb3ec1f6 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.spec.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.spec.ts @@ -11,8 +11,11 @@ import {Subject} from 'rxjs/Subject'; class FakeFocusable { - constructor(private _label = '') { } + /** Whether the item is disabled or not. */ disabled = false; + /** Test property that can be used to test the `skipPredicate` functionality. */ + skipItem = false; + constructor(private _label = '') {} focus(_focusOrigin?: FocusOrigin) {} getLabel() { return this._label; } } @@ -480,6 +483,31 @@ describe('Key managers', () => { }); }); + describe('skip predicate', () => { + + it('should skip disabled items by default', () => { + itemList.items[1].disabled = true; + + expect(keyManager.activeItemIndex).toBe(0); + + keyManager.onKeydown(fakeKeyEvents.downArrow); + + expect(keyManager.activeItemIndex).toBe(2); + }); + + it('should be able to skip items with a custom predicate', () => { + keyManager.skipPredicate(item => item.skipItem); + + itemList.items[1].skipItem = true; + + expect(keyManager.activeItemIndex).toBe(0); + + keyManager.onKeydown(fakeKeyEvents.downArrow); + + expect(keyManager.activeItemIndex).toBe(2); + }); + }); + describe('typeahead mode', () => { const debounceInterval = 300; diff --git a/src/cdk/a11y/key-manager/list-key-manager.ts b/src/cdk/a11y/key-manager/list-key-manager.ts index c3178d4e0450..0fec6ca67345 100644 --- a/src/cdk/a11y/key-manager/list-key-manager.ts +++ b/src/cdk/a11y/key-manager/list-key-manager.ts @@ -47,6 +47,12 @@ export class ListKeyManager { private _vertical = true; private _horizontal: 'ltr' | 'rtl' | null; + /** + * Predicate function that can be used to check whether an item should be skipped + * by the key manager. By default, disabled items are skipped. + */ + private _skipPredicateFn = (item: T) => item.disabled; + // Buffer for the letters that the user has pressed when the typeahead option is turned on. private _pressedLetters: string[] = []; @@ -72,6 +78,16 @@ export class ListKeyManager { /** Stream that emits whenever the active item of the list manager changes. */ change = new Subject(); + /** + * Sets the predicate function that determines which items should be skipped by the + * list key manager. + * @param predicate Function that determines whether the given item should be skipped. + */ + skipPredicate(predicate: (item: T) => boolean): this { + this._skipPredicateFn = predicate; + return this; + } + /** * Turns on wrapping mode, which ensures that the active item will wrap to * the other end of list when there are no more items in the given direction. @@ -128,7 +144,9 @@ export class ListKeyManager { const index = (this._activeItemIndex + i) % items.length; const item = items[index]; - if (!item.disabled && item.getLabel!().toUpperCase().trim().indexOf(inputString) === 0) { + if (!this._skipPredicateFn(item) && + item.getLabel!().toUpperCase().trim().indexOf(inputString) === 0) { + this.setActiveItem(index); break; } @@ -274,7 +292,7 @@ export class ListKeyManager { const index = (this._activeItemIndex + (delta * i) + items.length) % items.length; const item = items[index]; - if (!item.disabled) { + if (!this._skipPredicateFn(item)) { this.setActiveItem(index); return; } @@ -301,7 +319,7 @@ export class ListKeyManager { return; } - while (items[index].disabled) { + while (this._skipPredicateFn(items[index])) { index += fallbackDelta; if (!items[index]) { diff --git a/src/cdk/testing/event-objects.ts b/src/cdk/testing/event-objects.ts index 9ead01e767cf..b205a3b198ad 100644 --- a/src/cdk/testing/event-objects.ts +++ b/src/cdk/testing/event-objects.ts @@ -74,7 +74,7 @@ export function createKeyboardEvent(type: string, keyCode: number, target?: Elem } /** Creates a fake event object with any desired event type. */ -export function createFakeEvent(type: string, canBubble = true, cancelable = true) { +export function createFakeEvent(type: string, canBubble = false, cancelable = true) { const event = document.createEvent('Event'); event.initEvent(type, canBubble, cancelable); return event; diff --git a/src/lib/list/_list-theme.scss b/src/lib/list/_list-theme.scss index 1319da507d42..09f44509c4e3 100644 --- a/src/lib/list/_list-theme.scss +++ b/src/lib/list/_list-theme.scss @@ -26,17 +26,8 @@ background-color: mat-color($background, disabled-list-option); } + .mat-list-option, .mat-nav-list .mat-list-item { - outline: none; - - &:hover, &.mat-list-item-focus { - background: mat-color($background, 'hover'); - } - } - - .mat-list-option { - outline: none; - &:hover, &.mat-list-item-focus { background: mat-color($background, 'hover'); } diff --git a/src/lib/list/list-option.html b/src/lib/list/list-option.html index db84f6f66dd6..4e97f99bb1c2 100644 --- a/src/lib/list/list-option.html +++ b/src/lib/list/list-option.html @@ -1,6 +1,5 @@
+ [class.mat-list-item-content-reverse]="checkboxPosition == 'after'">
{ }); it('should focus next item when press DOWN ARROW', () => { - let testListItem = listOptions[2].nativeElement as HTMLElement; - let DOWN_EVENT: KeyboardEvent = - createKeyboardEvent('keydown', DOWN_ARROW, testListItem); - let manager = selectionList.componentInstance._keyManager; + const manager = selectionList.componentInstance._keyManager; dispatchFakeEvent(listOptions[2].nativeElement, 'focus'); expect(manager.activeItemIndex).toEqual(2); - selectionList.componentInstance._keydown(DOWN_EVENT); - + selectionList.componentInstance._keydown(createKeyboardEvent('keydown', DOWN_ARROW)); fixture.detectChanges(); expect(manager.activeItemIndex).toEqual(3); }); - it('should focus the first non-disabled item when pressing HOME', () => { + it('should be able to focus the first item when pressing HOME', () => { const manager = selectionList.componentInstance._keyManager; expect(manager.activeItemIndex).toBe(-1); const event = dispatchKeyboardEvent(selectionList.nativeElement, 'keydown', HOME); fixture.detectChanges(); - // Note that the first item is disabled so we expect the second one to be focused. - expect(manager.activeItemIndex).toBe(1); + expect(manager.activeItemIndex).toBe(0); expect(event.defaultPrevented).toBe(true); }); @@ -425,7 +420,7 @@ describe('MatSelectionList without forms', () => { fixture.detectChanges(); expect(selectionList.componentInstance.tabIndex) - .toBe(-1, 'Expected the tabIndex to be set to "-1" if selection list is disabled.'); + .toBe(3, 'Expected the tabIndex to be still set to "3".'); }); }); diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index 9fd7742042bb..9126ede48821 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -32,12 +32,10 @@ import { import { CanDisable, CanDisableRipple, - HasTabIndex, MatLine, MatLineSetter, mixinDisabled, mixinDisableRipple, - mixinTabIndex, } from '@angular/material/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import {Subscription} from 'rxjs/Subscription'; @@ -45,8 +43,7 @@ import {Subscription} from 'rxjs/Subscription'; /** @docs-private */ export class MatSelectionListBase {} -export const _MatSelectionListMixinBase = - mixinTabIndex(mixinDisableRipple(mixinDisabled(MatSelectionListBase))); +export const _MatSelectionListMixinBase = mixinDisableRipple(mixinDisabled(MatSelectionListBase)); /** @docs-private */ export class MatListOptionBase {} @@ -294,7 +291,7 @@ export class MatListOption extends _MatListOptionMixinBase changeDetection: ChangeDetectionStrategy.OnPush }) export class MatSelectionList extends _MatSelectionListMixinBase implements FocusableOption, - CanDisable, CanDisableRipple, HasTabIndex, AfterContentInit, ControlValueAccessor, OnDestroy { + CanDisable, CanDisableRipple, AfterContentInit, ControlValueAccessor, OnDestroy { /** The FocusKeyManager which handles focus. */ _keyManager: FocusKeyManager; @@ -306,6 +303,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu @Output() readonly selectionChange: EventEmitter = new EventEmitter(); + /** Tabindex of the selection list. */ + @Input() tabIndex: number = 0; + /** The currently selected options. */ selectedOptions: SelectionModel = new SelectionModel(true); @@ -327,7 +327,12 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu } ngAfterContentInit(): void { - this._keyManager = new FocusKeyManager(this.options).withWrap().withTypeAhead(); + this._keyManager = new FocusKeyManager(this.options) + .withWrap() + .withTypeAhead() + // Allow disabled items to be focusable. For accessibility reasons, there must be a way for + // screenreader users, that allows reading the different options of the list. + .skipPredicate(() => false); if (this._tempValues) { this._setOptionsFromValues(this._tempValues); @@ -354,7 +359,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu this._modelChanges.unsubscribe(); } - /** Focus the selection-list. */ + /** Focuses the last active list option. */ focus() { this._element.nativeElement.focus(); } @@ -392,16 +397,15 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu /** Passes relevant key presses to our key manager. */ _keydown(event: KeyboardEvent) { - if (this.disabled) { - return; - } - switch (event.keyCode) { case SPACE: case ENTER: - this._toggleSelectOnFocusedOption(); - // Always prevent space from scrolling the page since the list has focus - event.preventDefault(); + if (!this.disabled) { + this._toggleSelectOnFocusedOption(); + + // Always prevent space from scrolling the page since the list has focus + event.preventDefault(); + } break; case HOME: case END: