From 054ea4d1ef63b9a3dd370ba019b1498d4aa0082c Mon Sep 17 00:00:00 2001 From: Philip Pham Date: Mon, 21 Aug 2017 13:53:39 -0700 Subject: [PATCH] feat(select): implement compareWith for custom comparison (#4540) Fixes #2250, fixes #2785. --- src/demo-app/select/select-demo.html | 29 +++++++ src/demo-app/select/select-demo.ts | 15 ++++ src/lib/select/select-errors.ts | 13 ++- src/lib/select/select.spec.ts | 122 ++++++++++++++++++++++++++- src/lib/select/select.ts | 57 +++++++++++-- 5 files changed, 222 insertions(+), 14 deletions(-) diff --git a/src/demo-app/select/select-demo.html b/src/demo-app/select/select-demo.html index e5444b1b66e..8fad21eb8a7 100644 --- a/src/demo-app/select/select-demo.html +++ b/src/demo-app/select/select-demo.html @@ -96,6 +96,35 @@ Space above cards: + + + compareWith + + + + {{ drink.viewValue }} + + +

Value: {{ currentDrinkObject | json }}

+

Touched: {{ drinkObjectControl.touched }}

+

Dirty: {{ drinkObjectControl.dirty }}

+

Status: {{ drinkObjectControl.control?.status }}

+

Comparison Mode: {{ compareByValue ? 'VALUE' : 'REFERENCE' }}

+ + + + + + + +
formControl diff --git a/src/demo-app/select/select-demo.ts b/src/demo-app/select/select-demo.ts index 9a35b612e1c..48490643dbd 100644 --- a/src/demo-app/select/select-demo.ts +++ b/src/demo-app/select/select-demo.ts @@ -10,11 +10,13 @@ import {MdSelectChange} from '@angular/material'; }) export class SelectDemo { drinksRequired = false; + drinkObjectRequired = false; pokemonRequired = false; drinksDisabled = false; pokemonDisabled = false; showSelect = false; currentDrink: string; + currentDrinkObject: {}|undefined = {value: 'tea-5', viewValue: 'Tea'}; currentPokemon: string[]; currentPokemonFromGroup: string; currentDigimon: string; @@ -24,6 +26,7 @@ export class SelectDemo { topHeightCtrl = new FormControl(0); drinksTheme = 'primary'; pokemonTheme = 'primary'; + compareByValue = true; foods = [ {value: null, viewValue: 'None'}, @@ -111,4 +114,16 @@ export class SelectDemo { setPokemonValue() { this.currentPokemon = ['eevee-4', 'psyduck-6']; } + + reassignDrinkByCopy() { + this.currentDrinkObject = {...this.currentDrinkObject}; + } + + compareDrinkObjectsByValue(d1: {value: string}, d2: {value: string}) { + return d1 && d2 && d1.value === d2.value; + } + + compareByReference(o1: any, o2: any) { + return o1 === o2; + } } diff --git a/src/lib/select/select-errors.ts b/src/lib/select/select-errors.ts index 97a07b3db88..f0578131858 100644 --- a/src/lib/select/select-errors.ts +++ b/src/lib/select/select-errors.ts @@ -7,8 +7,8 @@ */ /** - * Returns an exception to be thrown when attempting to change a s - * elect's `multiple` option after initialization. + * Returns an exception to be thrown when attempting to change a select's `multiple` option + * after initialization. * @docs-private */ export function getMdSelectDynamicMultipleError(): Error { @@ -24,3 +24,12 @@ export function getMdSelectDynamicMultipleError(): Error { export function getMdSelectNonArrayValueError(): Error { return Error('Cannot assign truthy non-array value to select in `multiple` mode.'); } + +/** + * Returns an exception to be thrown when assigning a non-function value to the comparator + * used to determine if a value corresponds to an option. Note that whether the function + * actually takes two values and returns a boolean is not checked. + */ +export function getMdSelectNonFunctionValueError(): Error { + return Error('Cannot assign a non-function value to `compareWith`.'); +} diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index bbcbf60cafb..5c12c95b730 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -28,12 +28,17 @@ import {Subject} from 'rxjs/Subject'; import {map} from 'rxjs/operator/map'; import {MdSelectModule} from './index'; import {MdSelect} from './select'; -import {getMdSelectDynamicMultipleError, getMdSelectNonArrayValueError} from './select-errors'; +import { + getMdSelectDynamicMultipleError, + getMdSelectNonArrayValueError, + getMdSelectNonFunctionValueError +} from './select-errors'; import {MdOption} from '../core/option/option'; import { FloatPlaceholderType, MD_PLACEHOLDER_GLOBAL_OPTIONS } from '../core/placeholder/placeholder-options'; +import {extendObject} from '../core/util/object-extend'; describe('MdSelect', () => { @@ -73,7 +78,10 @@ describe('MdSelect', () => { BasicSelectWithoutFormsPreselected, BasicSelectWithoutFormsMultiple, SelectInsideFormGroup, - SelectWithCustomTrigger + SelectWithCustomTrigger, + FalsyValueSelect, + SelectInsideFormGroup, + NgModelCompareWithSelect, ], providers: [ {provide: OverlayContainer, useFactory: () => { @@ -2714,8 +2722,78 @@ describe('MdSelect', () => { }); -}); + describe('compareWith behavior', () => { + let fixture: ComponentFixture; + let instance: NgModelCompareWithSelect; + beforeEach(async(() => { + fixture = TestBed.createComponent(NgModelCompareWithSelect); + instance = fixture.componentInstance; + fixture.detectChanges(); + })); + + describe('when comparing by value', () => { + + it('should have a selection', () => { + const selectedOption = instance.select.selected as MdOption; + expect(selectedOption.value.value).toEqual('pizza-1'); + }); + + it('should update when making a new selection', async(() => { + instance.options.last._selectViaInteraction(); + fixture.detectChanges(); + fixture.whenStable().then(() => { + const selectedOption = instance.select.selected as MdOption; + expect(instance.selectedFood.value).toEqual('tacos-2'); + expect(selectedOption.value.value).toEqual('tacos-2'); + }); + })); + + }); + + describe('when comparing by reference', () => { + beforeEach(async(() => { + spyOn(instance, 'compareByReference').and.callThrough(); + instance.useCompareByReference(); + fixture.detectChanges(); + })); + + it('should use the comparator', () => { + expect(instance.compareByReference).toHaveBeenCalled(); + }); + + it('should initialize with no selection despite having a value', () => { + expect(instance.selectedFood.value).toBe('pizza-1'); + expect(instance.select.selected).toBeUndefined(); + }); + + it('should not update the selection if value is copied on change', async(() => { + instance.options.first._selectViaInteraction(); + fixture.detectChanges(); + fixture.whenStable().then(() => { + expect(instance.selectedFood.value).toEqual('steak-0'); + expect(instance.select.selected).toBeUndefined(); + }); + })); + + }); + + describe('when using a non-function comparator', () => { + beforeEach(() => { + instance.useNullComparator(); + }); + + it('should throw an error', () => { + expect(() => { + fixture.detectChanges(); + }).toThrowError(wrappedErrorMessage(getMdSelectNonFunctionValueError())); + }); + + }); + + }); + +}); @Component({ selector: 'basic-select', @@ -3250,6 +3328,7 @@ class BasicSelectWithoutFormsMultiple { @ViewChild(MdSelect) select: MdSelect; } + @Component({ selector: 'select-with-custom-trigger', template: ` @@ -3270,3 +3349,40 @@ class SelectWithCustomTrigger { ]; control = new FormControl(); } + + +@Component({ + selector: 'ng-model-compare-with', + template: ` + + {{ food.viewValue }} + + ` +}) +class NgModelCompareWithSelect { + foods: ({value: string, viewValue: string})[] = [ + { value: 'steak-0', viewValue: 'Steak' }, + { value: 'pizza-1', viewValue: 'Pizza' }, + { value: 'tacos-2', viewValue: 'Tacos' }, + ]; + selectedFood: {value: string, viewValue: string} = { value: 'pizza-1', viewValue: 'Pizza' }; + comparator: ((f1: any, f2: any) => boolean)|null = this.compareByValue; + + @ViewChild(MdSelect) select: MdSelect; + @ViewChildren(MdOption) options: QueryList; + + useCompareByValue() { this.comparator = this.compareByValue; } + + useCompareByReference() { this.comparator = this.compareByReference; } + + useNullComparator() { this.comparator = null; } + + compareByValue(f1: any, f2: any) { return f1 && f2 && f1.value === f2.value; } + + compareByReference(f1: any, f2: any) { return f1 === f2; } + + setFoodByCopy(newValue: {value: string, viewValue: string}) { + this.selectedFood = extendObject({}, newValue); + } +} diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 58b1ff7ece8..c3132d61145 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -29,6 +29,7 @@ import { ViewChild, ViewEncapsulation, Directive, + isDevMode, } from '@angular/core'; import {ControlValueAccessor, FormGroupDirective, NgControl, NgForm} from '@angular/forms'; import {DOWN_ARROW, END, ENTER, HOME, SPACE, UP_ARROW} from '@angular/cdk/keycodes'; @@ -51,7 +52,11 @@ import {Observable} from 'rxjs/Observable'; import {Subscription} from 'rxjs/Subscription'; import {fadeInContent, transformPanel, transformPlaceholder} from './select-animations'; import {SelectionModel} from '../core/selection/selection'; -import {getMdSelectDynamicMultipleError, getMdSelectNonArrayValueError} from './select-errors'; +import { + getMdSelectDynamicMultipleError, + getMdSelectNonArrayValueError, + getMdSelectNonFunctionValueError +} from './select-errors'; import {CanColor, mixinColor} from '../core/common-behaviors/color'; import {CanDisable, mixinDisabled} from '../core/common-behaviors/disabled'; import {MdOptgroup, MdOption, MdOptionSelectionChange} from '../core/option/index'; @@ -220,6 +225,9 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On /** Whether the component is in multiple selection mode. */ private _multiple: boolean = false; + /** Comparison function to specify which option is displayed. Defaults to object equality. */ + private _compareWith = (o1: any, o2: any) => o1 === o2; + /** Deals with the selection logic. */ _selectionModel: SelectionModel; @@ -337,6 +345,24 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On this._multiple = coerceBooleanProperty(value); } + /** + * A function to compare the option values with the selected values. The first argument + * is a value from an option. The second is a value from the selection. A boolean + * should be returned. + */ + @Input() + get compareWith() { return this._compareWith; } + set compareWith(fn: (o1: any, o2: any) => boolean) { + if (typeof fn !== 'function') { + throw getMdSelectNonFunctionValueError(); + } + this._compareWith = fn; + if (this._selectionModel) { + // A different comparator means the selection could change. + this._initializeSelection(); + } + } + /** Whether to float the placeholder text. */ @Input() get floatPlaceholder(): FloatPlaceholderType { return this._floatPlaceholder; } @@ -434,12 +460,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On this._changeSubscription = startWith.call(this.options.changes, null).subscribe(() => { this._resetOptions(); - - // Defer setting the value in order to avoid the "Expression - // has changed after it was checked" errors from Angular. - Promise.resolve().then(() => { - this._setSelectionByValue(this._control ? this._control.value : this._value); - }); + this._initializeSelection(); }); } @@ -670,6 +691,14 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On scrollContainer!.scrollTop = this._scrollTop; } + private _initializeSelection(): void { + // Defer setting the value in order to avoid the "Expression + // has changed after it was checked" errors from Angular. + Promise.resolve().then(() => { + this._setSelectionByValue(this._control ? this._control.value : this._value); + }); + } + /** * Sets the selected option based on a value. If no option can be * found with the designated value, the select trigger is cleared. @@ -710,8 +739,17 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On * @returns Option that has the corresponding value. */ private _selectValue(value: any, isUserInput = false): MdOption | undefined { - let correspondingOption = this.options.find(option => { - return option.value != null && option.value === value; + const correspondingOption = this.options.find((option: MdOption) => { + try { + // Treat null as a special reset value. + return option.value != null && this._compareWith(option.value, value); + } catch (error) { + if (isDevMode()) { + // Notify developers of errors in their comparator. + console.warn(error); + } + return false; + } }); if (correspondingOption) { @@ -722,6 +760,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On return correspondingOption; } + /** * Clears the select trigger and deselects every option in the list. * @param skip Option that should not be deselected.