Skip to content

Commit

Permalink
fix(select): tab opening multiple select and space scrolling page (#4210
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
crisbeto authored and mmalerba committed Apr 28, 2017
1 parent 4e8d806 commit 24a762f
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 35 deletions.
24 changes: 13 additions & 11 deletions src/lib/core/testing/dispatch-events.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
import {
createFakeEvent,
createKeyboardEvent,
createMouseEvent
} from './event-objects';
import {createFakeEvent, createKeyboardEvent, createMouseEvent} from './event-objects';

/** Utility to dispatch any event on a Node. */
export function dispatchEvent(node: Node | Window, event: Event): Event {
node.dispatchEvent(event);
return event;
}

/** Shorthand to dispatch a fake event on a specified node. */
export function dispatchFakeEvent(node: Node | Window, type: string) {
node.dispatchEvent(createFakeEvent(type));
export function dispatchFakeEvent(node: Node | Window, type: string): Event {
return dispatchEvent(node, createFakeEvent(type));
}

/** Shorthand to dispatch a keyboard event with a specified key code. */
export function dispatchKeyboardEvent(node: Node, type: string, keyCode: number) {
node.dispatchEvent(createKeyboardEvent(type, keyCode));
export function dispatchKeyboardEvent(node: Node, type: string, keyCode: number): KeyboardEvent {
return dispatchEvent(node, createKeyboardEvent(type, keyCode)) as KeyboardEvent;
}

/** Shorthand to dispatch a mouse event on the specified coordinates. */
export function dispatchMouseEvent(node: Node, type: string, x = 0, y = 0) {
node.dispatchEvent(createMouseEvent(type, x, y));
export function dispatchMouseEvent(node: Node, type: string, x = 0, y = 0): MouseEvent {
return dispatchEvent(node, createMouseEvent(type, x, y)) as MouseEvent;
}
11 changes: 8 additions & 3 deletions src/lib/core/testing/event-objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ export function createKeyboardEvent(type: string, keyCode: number) {
let event = document.createEvent('KeyboardEvent') as any;
// Firefox does not support `initKeyboardEvent`, but supports `initKeyEvent`.
let initEventFn = (event.initKeyEvent || event.initKeyboardEvent).bind(event);
let originalPreventDefault = event.preventDefault;

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

// Webkit Browsers don't set the keyCode when calling the init function.
// See related bug https://bugs.webkit.org/show_bug.cgi?id=16735
Object.defineProperty(event, 'keyCode', {
get: function() { return keyCode; }
});
Object.defineProperty(event, 'keyCode', { get: () => keyCode });

// IE won't set `defaultPrevented` on synthetic events so we need to do it manually.
event.preventDefault = function() {
Object.defineProperty(event, 'defaultPrevented', { get: () => true });
return originalPreventDefault.apply(this, arguments);
};

return event;
}
Expand Down
45 changes: 44 additions & 1 deletion src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {MdSelect, MdSelectFloatPlaceholderType} from './select';
import {MdSelectDynamicMultipleError, MdSelectNonArrayValueError} from './select-errors';
import {MdOption} from '../core/option/option';
import {Dir} from '../core/rtl/dir';
import {DOWN_ARROW, UP_ARROW} from '../core/keyboard/keycodes';
import {DOWN_ARROW, UP_ARROW, ENTER, SPACE} from '../core/keyboard/keycodes';
import {
ControlValueAccessor, FormControl, FormsModule, NG_VALUE_ACCESSOR, ReactiveFormsModule
} from '@angular/forms';
Expand Down Expand Up @@ -1370,6 +1370,26 @@ describe('MdSelect', () => {
'Expected value from second option to have been set on the model.');
});

it('should open the panel when pressing the arrow keys on a closed multiple select', () => {
fixture.destroy();

const multiFixture = TestBed.createComponent(MultiSelect);
const instance = multiFixture.componentInstance;

multiFixture.detectChanges();
select = multiFixture.debugElement.query(By.css('md-select')).nativeElement;

const initialValue = instance.control.value;

expect(instance.select.panelOpen).toBe(false, 'Expected panel to be closed.');

const event = dispatchKeyboardEvent(select, 'keydown', DOWN_ARROW);

expect(instance.select.panelOpen).toBe(true, 'Expected panel to be open.');
expect(instance.control.value).toBe(initialValue, 'Expected value to stay the same.');
expect(event.defaultPrevented).toBe(true, 'Expected default to be prevented.');
});

it('should do nothing if the key manager did not change the active item', () => {
const formControl = fixture.componentInstance.control;

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

it('should not open a multiple select when tabbing through', () => {
fixture.destroy();

const multiFixture = TestBed.createComponent(MultiSelect);

multiFixture.detectChanges();
select = multiFixture.debugElement.query(By.css('md-select')).nativeElement;

expect(multiFixture.componentInstance.select.panelOpen)
.toBe(false, 'Expected panel to be closed initially.');

dispatchKeyboardEvent(select, 'keydown', TAB);

expect(multiFixture.componentInstance.select.panelOpen)
.toBe(false, 'Expected panel to stay closed.');
});

it('should prevent the default action when pressing space', () => {
let event = dispatchKeyboardEvent(select, 'keydown', SPACE);

expect(event.defaultPrevented).toBe(true);
});

});

describe('for options', () => {
Expand Down
52 changes: 32 additions & 20 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
OnInit,
} from '@angular/core';
import {MdOption, MdOptionSelectionChange} from '../core/option/option';
import {ENTER, SPACE} from '../core/keyboard/keycodes';
import {ENTER, SPACE, UP_ARROW, DOWN_ARROW} from '../core/keyboard/keycodes';
import {FocusKeyManager} from '../core/a11y/focus-key-manager';
import {Dir} from '../core/rtl/dir';
import {Observable} from 'rxjs/Observable';
Expand Down Expand Up @@ -480,27 +480,14 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
this._triggerWidth = this._getTriggerRect().width;
}

/** Ensures the panel opens if activated by the keyboard. */
/** Handles the keyboard interactions of a closed select. */
_handleKeydown(event: KeyboardEvent): void {
if (event.keyCode === ENTER || event.keyCode === SPACE) {
this.open();
} else if (!this.disabled) {
let prevActiveItem = this._keyManager.activeItem;

// Cycle though the select options even when the select is closed,
// matching the behavior of the native select element.
// TODO(crisbeto): native selects also cycle through the options with left/right arrows,
// however the key manager only supports up/down at the moment.
this._keyManager.onKeydown(event);

let currentActiveItem = this._keyManager.activeItem as MdOption;

if (this._multiple) {
if (!this.disabled) {
if (event.keyCode === ENTER || event.keyCode === SPACE) {
event.preventDefault(); // prevents the page from scrolling down when pressing space
this.open();
} else if (currentActiveItem !== prevActiveItem) {
this._clearSelection();
this._setSelectionByValue(currentActiveItem.value);
this._propagateChanges();
} else if (event.keyCode === UP_ARROW || event.keyCode === DOWN_ARROW) {
this._handleArrowKey(event);
}
}
}
Expand Down Expand Up @@ -975,6 +962,31 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
private _floatPlaceholderState(): string {
return this._isRtl() ? 'floating-rtl' : 'floating-ltr';
}

/** Handles the user pressing the arrow keys on a closed select. */
private _handleArrowKey(event: KeyboardEvent): void {
if (this._multiple) {
event.preventDefault();
this.open();
} else {
const prevActiveItem = this._keyManager.activeItem;

// Cycle though the select options even when the select is closed,
// matching the behavior of the native select element.
// TODO(crisbeto): native selects also cycle through the options with left/right arrows,
// however the key manager only supports up/down at the moment.
this._keyManager.onKeydown(event);

const currentActiveItem = this._keyManager.activeItem as MdOption;

if (currentActiveItem !== prevActiveItem) {
this._clearSelection();
this._setSelectionByValue(currentActiveItem.value);
this._propagateChanges();
}
}
}

}

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

0 comments on commit 24a762f

Please sign in to comment.