From bb61970c79e81d035f6c8cf7f4cae82f90783195 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 13 Jun 2017 20:17:19 +0200 Subject: [PATCH 1/2] fix(select): consider value changes via arrow keys on closed select as user actions Currently, the user changing the `md-select` value using the arrow keys on a closed select won't mark the interaction as `isUserInput` in the option's selection event. These change s correct the behavior. Fixes #5084. --- src/lib/select/select.spec.ts | 12 ++++++++++++ src/lib/select/select.ts | 12 ++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 2b7d90e41654..8b87bb7851db 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -29,6 +29,7 @@ import { FloatPlaceholderType, MD_PLACEHOLDER_GLOBAL_OPTIONS } from '../core/placeholder/placeholder-options'; +import 'rxjs/add/operator/map'; describe('MdSelect', () => { @@ -1676,6 +1677,17 @@ describe('MdSelect', () => { expect(event.defaultPrevented).toBe(true); }); + it('should consider the selection as a result of a user action when closed', () => { + const option = fixture.componentInstance.options.first; + const spy = jasmine.createSpy('option selection spy'); + const subscription = option.onSelectionChange.map(e => e.isUserInput).subscribe(spy); + + dispatchKeyboardEvent(select, 'keydown', DOWN_ARROW); + expect(spy).toHaveBeenCalledWith(true); + + subscription.unsubscribe(); + }); + }); describe('for options', () => { diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 84a70f1e6a45..8b1b514eb6bb 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -566,7 +566,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On * Sets the selected option based on a value. If no option can be * found with the designated value, the select trigger is cleared. */ - private _setSelectionByValue(value: any | any[]): void { + private _setSelectionByValue(value: any | any[], isUserInput = false): void { const isArray = Array.isArray(value); if (this.multiple && value && !isArray) { @@ -576,10 +576,10 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On this._clearSelection(); if (isArray) { - value.forEach((currentValue: any) => this._selectValue(currentValue)); + value.forEach((currentValue: any) => this._selectValue(currentValue, isUserInput)); this._sortValues(); } else { - this._selectValue(value); + this._selectValue(value, isUserInput); } this._setValueWidth(); @@ -595,14 +595,14 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On * Finds and selects and option based on its value. * @returns Option that has the corresponding value. */ - private _selectValue(value: any): MdOption { + private _selectValue(value: any, isUserInput = false): MdOption { let optionsArray = this.options.toArray(); let correspondingOption = optionsArray.find(option => { return option.value != null && option.value === value; }); if (correspondingOption) { - correspondingOption.select(); + isUserInput ? correspondingOption._selectViaInteraction() : correspondingOption.select(); this._selectionModel.select(correspondingOption); this._keyManager.setActiveItem(optionsArray.indexOf(correspondingOption)); } @@ -1027,7 +1027,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On if (currentActiveItem !== prevActiveItem) { this._clearSelection(); - this._setSelectionByValue(currentActiveItem.value); + this._setSelectionByValue(currentActiveItem.value, true); this._propagateChanges(); } } From a10362943978813280925ee6479f98ec0f8fdf5d Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 13 Jun 2017 22:45:00 +0200 Subject: [PATCH 2/2] chore: use alternate rx symbol --- src/lib/select/select.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 8b87bb7851db..2811bb1c9854 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -29,7 +29,7 @@ import { FloatPlaceholderType, MD_PLACEHOLDER_GLOBAL_OPTIONS } from '../core/placeholder/placeholder-options'; -import 'rxjs/add/operator/map'; +import {map} from 'rxjs/operator/map'; describe('MdSelect', () => { @@ -1680,7 +1680,7 @@ describe('MdSelect', () => { it('should consider the selection as a result of a user action when closed', () => { const option = fixture.componentInstance.options.first; const spy = jasmine.createSpy('option selection spy'); - const subscription = option.onSelectionChange.map(e => e.isUserInput).subscribe(spy); + const subscription = map.call(option.onSelectionChange, e => e.isUserInput).subscribe(spy); dispatchKeyboardEvent(select, 'keydown', DOWN_ARROW); expect(spy).toHaveBeenCalledWith(true);