Skip to content

Commit

Permalink
fix(list): don't handle events when modifier key is pressed (#14313)
Browse files Browse the repository at this point in the history
Doesn't `preventDefault` and handle ENTER/SPACE/HOME/END keyboard events when a modifier is pressed, in order to avoid clashing with native OS shortcuts.
  • Loading branch information
crisbeto authored and jelbourn committed Dec 3, 2018
1 parent b00b3bb commit 0c7ce7a
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 7 deletions.
48 changes: 48 additions & 0 deletions src/lib/list/selection-list.spec.ts
Expand Up @@ -196,6 +196,26 @@ describe('MatSelectionList without forms', () => {
expect(ENTER_EVENT.defaultPrevented).toBe(true);
});

it('should not be able to toggle an item when pressing a modifier key', () => {
const testListItem = listOptions[1].nativeElement as HTMLElement;
const selectList =
selectionList.injector.get<MatSelectionList>(MatSelectionList).selectedOptions;

expect(selectList.selected.length).toBe(0);

[ENTER, SPACE].forEach(key => {
const event = createKeyboardEvent('keydown', key, testListItem);
Object.defineProperty(event, 'ctrlKey', { get: () => true });

dispatchFakeEvent(testListItem, 'focus');
selectionList.componentInstance._keydown(event);
fixture.detectChanges();
expect(event.defaultPrevented).toBe(false);
});

expect(selectList.selected.length).toBe(0);
});

it('should not be able to toggle a disabled option using SPACE', () => {
const testListItem = listOptions[1].nativeElement as HTMLElement;
const selectionModel = selectionList.componentInstance.selectedOptions;
Expand Down Expand Up @@ -332,6 +352,20 @@ describe('MatSelectionList without forms', () => {
expect(event.defaultPrevented).toBe(true);
});

it('should not change focus when pressing HOME with a modifier key', () => {
const manager = selectionList.componentInstance._keyManager;
expect(manager.activeItemIndex).toBe(-1);

const event = createKeyboardEvent('keydown', HOME);
Object.defineProperty(event, 'shiftKey', { get: () => true });

dispatchEvent(selectionList.nativeElement, event);
fixture.detectChanges();

expect(manager.activeItemIndex).toBe(-1);
expect(event.defaultPrevented).toBe(false);
});

it('should focus the last item when pressing END', () => {
const manager = selectionList.componentInstance._keyManager;
expect(manager.activeItemIndex).toBe(-1);
Expand All @@ -343,6 +377,20 @@ describe('MatSelectionList without forms', () => {
expect(event.defaultPrevented).toBe(true);
});

it('should not change focus when pressing END with a modifier key', () => {
const manager = selectionList.componentInstance._keyManager;
expect(manager.activeItemIndex).toBe(-1);

const event = createKeyboardEvent('keydown', END);
Object.defineProperty(event, 'shiftKey', { get: () => true });

dispatchEvent(selectionList.nativeElement, event);
fixture.detectChanges();

expect(manager.activeItemIndex).toBe(-1);
expect(event.defaultPrevented).toBe(false);
});

it('should select all items using ctrl + a', () => {
const event = createKeyboardEvent('keydown', A, selectionList.nativeElement);
Object.defineProperty(event, 'ctrlKey', {get: () => true});
Expand Down
28 changes: 21 additions & 7 deletions src/lib/list/selection-list.ts
Expand Up @@ -9,7 +9,16 @@
import {FocusableOption, FocusKeyManager} from '@angular/cdk/a11y';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {SelectionModel} from '@angular/cdk/collections';
import {SPACE, ENTER, HOME, END, UP_ARROW, DOWN_ARROW, A} from '@angular/cdk/keycodes';
import {
SPACE,
ENTER,
HOME,
END,
UP_ARROW,
DOWN_ARROW,
A,
hasModifierKey,
} from '@angular/cdk/keycodes';
import {
AfterContentInit,
Attribute,
Expand Down Expand Up @@ -417,21 +426,26 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
const keyCode = event.keyCode;
const manager = this._keyManager;
const previousFocusIndex = manager.activeItemIndex;
const hasModifier = hasModifierKey(event);

switch (keyCode) {
case SPACE:
case ENTER:
this._toggleFocusedOption();
// Always prevent space from scrolling the page since the list has focus
event.preventDefault();
if (!hasModifier) {
this._toggleFocusedOption();
// Always prevent space from scrolling the page since the list has focus
event.preventDefault();
}
break;
case HOME:
case END:
keyCode === HOME ? manager.setFirstItemActive() : manager.setLastItemActive();
event.preventDefault();
if (!hasModifier) {
keyCode === HOME ? manager.setFirstItemActive() : manager.setLastItemActive();
event.preventDefault();
}
break;
case A:
if (event.ctrlKey) {
if (hasModifierKey(event, 'ctrlKey')) {
this.options.find(option => !option.selected) ? this.selectAll() : this.deselectAll();
event.preventDefault();
}
Expand Down

0 comments on commit 0c7ce7a

Please sign in to comment.