Skip to content

Commit 24a762f

Browse files
crisbetommalerba
authored andcommitted
fix(select): tab opening multiple select and space scrolling page (#4210)
* fix(select): tab opening multiple select and space scrolling page * Fixes pressing tab (or any other key, except arrows) opening `md-select` in `multiple` mode. * Fixes the page getting scrolled down when opening a select by pressing space. * Adds missing test coverage for opening a multiple select with the arrow keys. * fix: defaultPrevented not working on IE * chore: cleaner dispatching of fake events
1 parent 4e8d806 commit 24a762f

File tree

4 files changed

+97
-35
lines changed

4 files changed

+97
-35
lines changed
Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
1-
import {
2-
createFakeEvent,
3-
createKeyboardEvent,
4-
createMouseEvent
5-
} from './event-objects';
1+
import {createFakeEvent, createKeyboardEvent, createMouseEvent} from './event-objects';
2+
3+
/** Utility to dispatch any event on a Node. */
4+
export function dispatchEvent(node: Node | Window, event: Event): Event {
5+
node.dispatchEvent(event);
6+
return event;
7+
}
68

79
/** Shorthand to dispatch a fake event on a specified node. */
8-
export function dispatchFakeEvent(node: Node | Window, type: string) {
9-
node.dispatchEvent(createFakeEvent(type));
10+
export function dispatchFakeEvent(node: Node | Window, type: string): Event {
11+
return dispatchEvent(node, createFakeEvent(type));
1012
}
1113

1214
/** Shorthand to dispatch a keyboard event with a specified key code. */
13-
export function dispatchKeyboardEvent(node: Node, type: string, keyCode: number) {
14-
node.dispatchEvent(createKeyboardEvent(type, keyCode));
15+
export function dispatchKeyboardEvent(node: Node, type: string, keyCode: number): KeyboardEvent {
16+
return dispatchEvent(node, createKeyboardEvent(type, keyCode)) as KeyboardEvent;
1517
}
1618

1719
/** Shorthand to dispatch a mouse event on the specified coordinates. */
18-
export function dispatchMouseEvent(node: Node, type: string, x = 0, y = 0) {
19-
node.dispatchEvent(createMouseEvent(type, x, y));
20+
export function dispatchMouseEvent(node: Node, type: string, x = 0, y = 0): MouseEvent {
21+
return dispatchEvent(node, createMouseEvent(type, x, y)) as MouseEvent;
2022
}

src/lib/core/testing/event-objects.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,19 @@ export function createKeyboardEvent(type: string, keyCode: number) {
2626
let event = document.createEvent('KeyboardEvent') as any;
2727
// Firefox does not support `initKeyboardEvent`, but supports `initKeyEvent`.
2828
let initEventFn = (event.initKeyEvent || event.initKeyboardEvent).bind(event);
29+
let originalPreventDefault = event.preventDefault;
2930

3031
initEventFn(type, true, true, window, 0, 0, 0, 0, 0, keyCode);
3132

3233
// Webkit Browsers don't set the keyCode when calling the init function.
3334
// See related bug https://bugs.webkit.org/show_bug.cgi?id=16735
34-
Object.defineProperty(event, 'keyCode', {
35-
get: function() { return keyCode; }
36-
});
35+
Object.defineProperty(event, 'keyCode', { get: () => keyCode });
36+
37+
// IE won't set `defaultPrevented` on synthetic events so we need to do it manually.
38+
event.preventDefault = function() {
39+
Object.defineProperty(event, 'defaultPrevented', { get: () => true });
40+
return originalPreventDefault.apply(this, arguments);
41+
};
3742

3843
return event;
3944
}

src/lib/select/select.spec.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {MdSelect, MdSelectFloatPlaceholderType} from './select';
1616
import {MdSelectDynamicMultipleError, MdSelectNonArrayValueError} from './select-errors';
1717
import {MdOption} from '../core/option/option';
1818
import {Dir} from '../core/rtl/dir';
19-
import {DOWN_ARROW, UP_ARROW} from '../core/keyboard/keycodes';
19+
import {DOWN_ARROW, UP_ARROW, ENTER, SPACE} from '../core/keyboard/keycodes';
2020
import {
2121
ControlValueAccessor, FormControl, FormsModule, NG_VALUE_ACCESSOR, ReactiveFormsModule
2222
} from '@angular/forms';
@@ -1370,6 +1370,26 @@ describe('MdSelect', () => {
13701370
'Expected value from second option to have been set on the model.');
13711371
});
13721372

1373+
it('should open the panel when pressing the arrow keys on a closed multiple select', () => {
1374+
fixture.destroy();
1375+
1376+
const multiFixture = TestBed.createComponent(MultiSelect);
1377+
const instance = multiFixture.componentInstance;
1378+
1379+
multiFixture.detectChanges();
1380+
select = multiFixture.debugElement.query(By.css('md-select')).nativeElement;
1381+
1382+
const initialValue = instance.control.value;
1383+
1384+
expect(instance.select.panelOpen).toBe(false, 'Expected panel to be closed.');
1385+
1386+
const event = dispatchKeyboardEvent(select, 'keydown', DOWN_ARROW);
1387+
1388+
expect(instance.select.panelOpen).toBe(true, 'Expected panel to be open.');
1389+
expect(instance.control.value).toBe(initialValue, 'Expected value to stay the same.');
1390+
expect(event.defaultPrevented).toBe(true, 'Expected default to be prevented.');
1391+
});
1392+
13731393
it('should do nothing if the key manager did not change the active item', () => {
13741394
const formControl = fixture.componentInstance.control;
13751395

@@ -1418,6 +1438,29 @@ describe('MdSelect', () => {
14181438
expect(lastOption.selected).toBe(true, 'Expected last option to stay selected.');
14191439
});
14201440

1441+
it('should not open a multiple select when tabbing through', () => {
1442+
fixture.destroy();
1443+
1444+
const multiFixture = TestBed.createComponent(MultiSelect);
1445+
1446+
multiFixture.detectChanges();
1447+
select = multiFixture.debugElement.query(By.css('md-select')).nativeElement;
1448+
1449+
expect(multiFixture.componentInstance.select.panelOpen)
1450+
.toBe(false, 'Expected panel to be closed initially.');
1451+
1452+
dispatchKeyboardEvent(select, 'keydown', TAB);
1453+
1454+
expect(multiFixture.componentInstance.select.panelOpen)
1455+
.toBe(false, 'Expected panel to stay closed.');
1456+
});
1457+
1458+
it('should prevent the default action when pressing space', () => {
1459+
let event = dispatchKeyboardEvent(select, 'keydown', SPACE);
1460+
1461+
expect(event.defaultPrevented).toBe(true);
1462+
});
1463+
14211464
});
14221465

14231466
describe('for options', () => {

src/lib/select/select.ts

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
OnInit,
1919
} from '@angular/core';
2020
import {MdOption, MdOptionSelectionChange} from '../core/option/option';
21-
import {ENTER, SPACE} from '../core/keyboard/keycodes';
21+
import {ENTER, SPACE, UP_ARROW, DOWN_ARROW} from '../core/keyboard/keycodes';
2222
import {FocusKeyManager} from '../core/a11y/focus-key-manager';
2323
import {Dir} from '../core/rtl/dir';
2424
import {Observable} from 'rxjs/Observable';
@@ -480,27 +480,14 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
480480
this._triggerWidth = this._getTriggerRect().width;
481481
}
482482

483-
/** Ensures the panel opens if activated by the keyboard. */
483+
/** Handles the keyboard interactions of a closed select. */
484484
_handleKeydown(event: KeyboardEvent): void {
485-
if (event.keyCode === ENTER || event.keyCode === SPACE) {
486-
this.open();
487-
} else if (!this.disabled) {
488-
let prevActiveItem = this._keyManager.activeItem;
489-
490-
// Cycle though the select options even when the select is closed,
491-
// matching the behavior of the native select element.
492-
// TODO(crisbeto): native selects also cycle through the options with left/right arrows,
493-
// however the key manager only supports up/down at the moment.
494-
this._keyManager.onKeydown(event);
495-
496-
let currentActiveItem = this._keyManager.activeItem as MdOption;
497-
498-
if (this._multiple) {
485+
if (!this.disabled) {
486+
if (event.keyCode === ENTER || event.keyCode === SPACE) {
487+
event.preventDefault(); // prevents the page from scrolling down when pressing space
499488
this.open();
500-
} else if (currentActiveItem !== prevActiveItem) {
501-
this._clearSelection();
502-
this._setSelectionByValue(currentActiveItem.value);
503-
this._propagateChanges();
489+
} else if (event.keyCode === UP_ARROW || event.keyCode === DOWN_ARROW) {
490+
this._handleArrowKey(event);
504491
}
505492
}
506493
}
@@ -975,6 +962,31 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
975962
private _floatPlaceholderState(): string {
976963
return this._isRtl() ? 'floating-rtl' : 'floating-ltr';
977964
}
965+
966+
/** Handles the user pressing the arrow keys on a closed select. */
967+
private _handleArrowKey(event: KeyboardEvent): void {
968+
if (this._multiple) {
969+
event.preventDefault();
970+
this.open();
971+
} else {
972+
const prevActiveItem = this._keyManager.activeItem;
973+
974+
// Cycle though the select options even when the select is closed,
975+
// matching the behavior of the native select element.
976+
// TODO(crisbeto): native selects also cycle through the options with left/right arrows,
977+
// however the key manager only supports up/down at the moment.
978+
this._keyManager.onKeydown(event);
979+
980+
const currentActiveItem = this._keyManager.activeItem as MdOption;
981+
982+
if (currentActiveItem !== prevActiveItem) {
983+
this._clearSelection();
984+
this._setSelectionByValue(currentActiveItem.value);
985+
this._propagateChanges();
986+
}
987+
}
988+
}
989+
978990
}
979991

980992
/** Clamps a value n between min and max values. */

0 commit comments

Comments
 (0)