Skip to content

Commit

Permalink
fix(selection-list): improve accessibility of selection list (#10137)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
devversion authored and tinayuangao committed Feb 28, 2018
1 parent e42f0bc commit 51fce51
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 51 deletions.
30 changes: 29 additions & 1 deletion src/cdk/a11y/key-manager/list-key-manager.spec.ts
Expand Up @@ -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; }
}
Expand Down Expand Up @@ -502,6 +505,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;

Expand Down
24 changes: 21 additions & 3 deletions src/cdk/a11y/key-manager/list-key-manager.ts
Expand Up @@ -47,6 +47,12 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
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[] = [];

Expand All @@ -72,6 +78,16 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
/** Stream that emits whenever the active item of the list manager changes. */
change = new Subject<number>();

/**
* 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.
Expand Down Expand Up @@ -128,7 +144,9 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
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;
}
Expand Down Expand Up @@ -282,7 +300,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
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;
}
Expand All @@ -309,7 +327,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
return;
}

while (items[index].disabled) {
while (this._skipPredicateFn(items[index])) {
index += fallbackDelta;

if (!items[index]) {
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/testing/event-objects.ts
Expand Up @@ -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;
Expand Down
11 changes: 1 addition & 10 deletions src/lib/list/_list-theme.scss
Expand Up @@ -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');
}
Expand Down
3 changes: 1 addition & 2 deletions src/lib/list/list-option.html
@@ -1,6 +1,5 @@
<div class="mat-list-item-content"
[class.mat-list-item-content-reverse]="checkboxPosition == 'after'"
[class.mat-list-item-disabled]="disabled">
[class.mat-list-item-content-reverse]="checkboxPosition == 'after'">

<div mat-ripple
class="mat-list-item-ripple"
Expand Down
15 changes: 5 additions & 10 deletions src/lib/list/list.scss
Expand Up @@ -240,24 +240,19 @@ $mat-list-item-inset-divider-offset: 72px;
}
}


.mat-nav-list {
a {
text-decoration: none;
color: inherit;
}

.mat-list-item-content {
.mat-list-item {
cursor: pointer;

&:hover, &.mat-list-item-focus {
outline: none;
}
outline: none;
}
}

.mat-list-option {
&:not(.mat-list-item-disabled) {
cursor: pointer;
}
.mat-list-option:not(.mat-list-item-disabled) {
cursor: pointer;
outline: none;
}
15 changes: 5 additions & 10 deletions src/lib/list/selection-list.spec.ts
Expand Up @@ -251,30 +251,25 @@ describe('MatSelectionList without forms', () => {
});

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);
});

Expand Down Expand Up @@ -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".');
});
});

Expand Down
32 changes: 18 additions & 14 deletions src/lib/list/selection-list.ts
Expand Up @@ -32,21 +32,18 @@ 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';


/** @docs-private */
export class MatSelectionListBase {}
export const _MatSelectionListMixinBase =
mixinTabIndex(mixinDisableRipple(mixinDisabled(MatSelectionListBase)));
export const _MatSelectionListMixinBase = mixinDisableRipple(mixinDisabled(MatSelectionListBase));

/** @docs-private */
export class MatListOptionBase {}
Expand Down Expand Up @@ -299,7 +296,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<MatListOption>;
Expand All @@ -311,6 +308,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
@Output() readonly selectionChange: EventEmitter<MatSelectionListChange> =
new EventEmitter<MatSelectionListChange>();

/** Tabindex of the selection list. */
@Input() tabIndex: number = 0;

/** The currently selected options. */
selectedOptions: SelectionModel<MatListOption> = new SelectionModel<MatListOption>(true);

Expand All @@ -332,7 +332,12 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
}

ngAfterContentInit(): void {
this._keyManager = new FocusKeyManager<MatListOption>(this.options).withWrap().withTypeAhead();
this._keyManager = new FocusKeyManager<MatListOption>(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);
Expand All @@ -359,7 +364,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();
}
Expand Down Expand Up @@ -397,16 +402,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:
Expand Down

0 comments on commit 51fce51

Please sign in to comment.