diff --git a/src/cdk/overlay/position/connected-position-strategy.spec.ts b/src/cdk/overlay/position/connected-position-strategy.spec.ts index 2ca91f9678c8..93c3630e7a9e 100644 --- a/src/cdk/overlay/position/connected-position-strategy.spec.ts +++ b/src/cdk/overlay/position/connected-position-strategy.spec.ts @@ -328,6 +328,8 @@ describe('ConnectedPositionStrategy', () => { it('should allow for the fallback positions to specify their own offsets', () => { originElement.style.bottom = '0'; + originElement.style.left = '50%'; + originElement.style.position = 'fixed'; originRect = originElement.getBoundingClientRect(); strategy = positionBuilder .connectedTo( diff --git a/src/lib/core/option/option.spec.ts b/src/lib/core/option/option.spec.ts index f03c47f8e30c..aa13e8b911a1 100644 --- a/src/lib/core/option/option.spec.ts +++ b/src/lib/core/option/option.spec.ts @@ -29,7 +29,7 @@ describe('MatOption component', () => { }); it('should show ripples by default', () => { - expect(optionInstance.disableRipple).toBe(false, 'Expected ripples to be enabled by default'); + expect(optionInstance.disableRipple).toBeFalsy('Expected ripples to be enabled by default'); expect(optionNativeElement.querySelectorAll('.mat-ripple-element').length) .toBe(0, 'Expected no ripples to show up initially'); @@ -54,20 +54,6 @@ describe('MatOption component', () => { .toBe(0, 'Expected no ripples to show up after click on a disabled option.'); }); - it('should not show ripples if the ripples are disabled using disableRipple', () => { - expect(optionNativeElement.querySelectorAll('.mat-ripple-element').length) - .toBe(0, 'Expected no ripples to show up initially'); - - optionInstance.disableRipple = true; - fixture.detectChanges(); - - dispatchFakeEvent(optionNativeElement, 'mousedown'); - dispatchFakeEvent(optionNativeElement, 'mouseup'); - - expect(optionNativeElement.querySelectorAll('.mat-ripple-element').length) - .toBe(0, 'Expected no ripples to show up after click when ripples are disabled.'); - }); - }); }); diff --git a/src/lib/core/option/option.ts b/src/lib/core/option/option.ts index 4b47c9bf5a4c..27bb5bfa7c00 100644 --- a/src/lib/core/option/option.ts +++ b/src/lib/core/option/option.ts @@ -19,6 +19,8 @@ import { Output, QueryList, ViewEncapsulation, + InjectionToken, + Inject, } from '@angular/core'; import {MatOptgroup} from './optgroup'; @@ -33,6 +35,22 @@ export class MatOptionSelectionChange { constructor(public source: MatOption, public isUserInput = false) { } } +/** + * Describes a parent component that manages a list of options. + * Contains properties that the options can inherit. + * @docs-private + */ +export interface MatOptionParentComponent { + disableRipple?: boolean; + multiple?: boolean; +} + +/** + * Injection token used to provide the parent component to options. + */ +export const MAT_OPTION_PARENT_COMPONENT = + new InjectionToken('MAT_OPTION_PARENT_COMPONENT'); + /** * Single option inside of a `` element. */ @@ -60,24 +78,13 @@ export class MatOptionSelectionChange { changeDetection: ChangeDetectionStrategy.OnPush, }) export class MatOption { - private _selected: boolean = false; - private _active: boolean = false; - private _multiple: boolean = false; - private _disableRipple: boolean = false; - - /** Whether the option is disabled. */ - private _disabled: boolean = false; - - private _id: string = `mat-option-${_uniqueIdCounter++}`; + private _selected = false; + private _active = false; + private _disabled = false; + private _id = `mat-option-${_uniqueIdCounter++}`; /** Whether the wrapping component is in multiple selection mode. */ - get multiple() { return this._multiple; } - set multiple(value: boolean) { - if (value !== this._multiple) { - this._multiple = value; - this._changeDetectorRef.markForCheck(); - } - } + get multiple() { return this._parent && this._parent.multiple; } /** The unique ID of the option. */ get id(): string { return this._id; } @@ -94,11 +101,7 @@ export class MatOption { set disabled(value: any) { this._disabled = coerceBooleanProperty(value); } /** Whether ripples for the option are disabled. */ - get disableRipple() { return this._disableRipple; } - set disableRipple(value: boolean) { - this._disableRipple = value; - this._changeDetectorRef.markForCheck(); - } + get disableRipple() { return this._parent && this._parent.disableRipple; } /** Event emitted when the option is selected or deselected. */ @Output() onSelectionChange = new EventEmitter(); @@ -106,6 +109,7 @@ export class MatOption { constructor( private _element: ElementRef, private _changeDetectorRef: ChangeDetectorRef, + @Optional() @Inject(MAT_OPTION_PARENT_COMPONENT) private _parent: MatOptionParentComponent, @Optional() public readonly group: MatOptgroup) {} /** diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 0be5121e9be9..d840b3198d46 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -248,19 +248,18 @@ describe('MatSelect', () => { expect(fixture.componentInstance.select.panelOpen).toBe(false); }); - it('should set the width of the overlay if there is no placeholder', async(() => { + it('should set the width of the overlay if there is no placeholder', fakeAsync(() => { let noPlaceholder = TestBed.createComponent(BasicSelectNoPlaceholder); noPlaceholder.detectChanges(); trigger = noPlaceholder.debugElement.query(By.css('.mat-select-trigger')).nativeElement; - noPlaceholder.whenStable().then(() => { - trigger.click(); - noPlaceholder.detectChanges(); + trigger.click(); + noPlaceholder.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; - expect(parseInt(pane.style.minWidth as string)).toBeGreaterThan(0); - }); + const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; + expect(parseInt(pane.style.minWidth as string)).toBeGreaterThan(0); })); it('should close the panel when tabbing out', fakeAsync(() => { @@ -348,6 +347,22 @@ describe('MatSelect', () => { expect(options.every(option => option.disableRipple === true)) .toBeTruthy('Expected all options to have disableRipple set to true.'); }); + + it('should not show ripples if they were disabled', fakeAsync(() => { + fixture.componentInstance.disableRipple = true; + fixture.detectChanges(); + + trigger.click(); + fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); + + const option = overlayContainerElement.querySelector('mat-option')!; + + dispatchFakeEvent(option, 'mousedown'); + dispatchFakeEvent(option, 'mouseup'); + + expect(option.querySelectorAll('.mat-ripple-element').length).toBe(0); + })); }); describe('selection logic', () => { @@ -548,7 +563,7 @@ describe('MatSelect', () => { expect(fixture.componentInstance.select.selected).toBeUndefined(); }); - it('should not select options inside a disabled group', async(() => { + it('should not select options inside a disabled group', () => { fixture.destroy(); const groupFixture = TestBed.createComponent(SelectWithGroups); @@ -565,7 +580,7 @@ describe('MatSelect', () => { expect(groupFixture.componentInstance.select.panelOpen).toBe(true); expect(options[0].classList).not.toContain('mat-selected'); expect(groupFixture.componentInstance.select.selected).toBeUndefined(); - })); + }); }); @@ -1944,14 +1959,10 @@ describe('MatSelect', () => { expect(select.getAttribute('aria-labelledby')).toBe('myLabelId'); }); - it('should not have aria-labelledby in the DOM if it`s not specified', async(() => { - + it('should not have aria-labelledby in the DOM if it`s not specified', () => { fixture.detectChanges(); - - fixture.whenStable().then(() => { - expect(select.hasAttribute('aria-labelledby')).toBeFalsy(); - }); - })); + expect(select.hasAttribute('aria-labelledby')).toBeFalsy(); + }); it('should set the tabindex of the select to 0 by default', () => { expect(select.getAttribute('tabindex')).toEqual('0'); @@ -2434,7 +2445,7 @@ describe('MatSelect', () => { }); describe('special cases', () => { - it('should handle nesting in an ngIf', async(() => { + it('should handle nesting in an ngIf', fakeAsync(() => { const fixture = TestBed.createComponent(NgIfSelect); fixture.detectChanges(); @@ -2444,41 +2455,42 @@ describe('MatSelect', () => { const trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; trigger.style.width = '300px'; - fixture.whenStable().then(() => { - fixture.detectChanges(); - const value = fixture.debugElement.query(By.css('.mat-select-value')); - expect(value.nativeElement.textContent) - .toContain('Pizza', `Expected trigger to be populated by the control's initial value.`); + fixture.detectChanges(); + tick(); - trigger.click(); - fixture.detectChanges(); + trigger.click(); + fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; - expect(pane.style.minWidth).toEqual('300px'); + const value = fixture.debugElement.query(By.css('.mat-select-value')); + expect(value.nativeElement.textContent) + .toContain('Pizza', `Expected trigger to be populated by the control's initial value.`); - expect(fixture.componentInstance.select.panelOpen).toBe(true); - expect(overlayContainerElement.textContent).toContain('Steak'); - expect(overlayContainerElement.textContent).toContain('Pizza'); - expect(overlayContainerElement.textContent).toContain('Tacos'); - }); + const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; + expect(pane.style.minWidth).toEqual('300px'); + + expect(fixture.componentInstance.select.panelOpen).toBe(true); + expect(overlayContainerElement.textContent).toContain('Steak'); + expect(overlayContainerElement.textContent).toContain('Pizza'); + expect(overlayContainerElement.textContent).toContain('Tacos'); })); - it('should not crash the browser when a sibling throws an error on init', async(() => { + it('should not crash the browser when a sibling throws an error on init', () => { // Note that this test can be considered successful if the error being thrown didn't // end up crashing the testing setup altogether. expect(() => { TestBed.createComponent(SelectWithErrorSibling).detectChanges(); }).toThrowError(new RegExp('Oh no!', 'g')); - })); + }); - it('should not throw when trying to access the selected value on init', async(() => { + it('should not throw when trying to access the selected value on init', () => { expect(() => { TestBed.createComponent(SelectEarlyAccessSibling).detectChanges(); }).not.toThrow(); - })); + }); it('should not throw selection model-related errors in addition to the errors from ngModel', - async(() => { + () => { const fixture = TestBed.createComponent(InvalidSelectInForm); // The first change detection run will throw the "ngModel is missing a name" error. @@ -2486,7 +2498,7 @@ describe('MatSelect', () => { // The second run shouldn't throw selection-model related errors. expect(() => fixture.detectChanges()).not.toThrow(); - })); + }); it('should not throw when the triggerValue is accessed when there is no selected value', () => { const fixture = TestBed.createComponent(BasicSelect); @@ -2655,13 +2667,13 @@ describe('MatSelect', () => { let testInstance: MultiSelect; let trigger: HTMLElement; - beforeEach(async(() => { + beforeEach(() => { fixture = TestBed.createComponent(MultiSelect); testInstance = fixture.componentInstance; fixture.detectChanges(); trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; - })); + }); it('should be able to select multiple values', () => { trigger.click(); @@ -2695,28 +2707,25 @@ describe('MatSelect', () => { expect(testInstance.control.value).toEqual([]); }); - it('should update the label', async(() => { + it('should update the label', fakeAsync(() => { trigger.click(); fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - fixture.whenStable().then(() => { - const options = overlayContainerElement.querySelectorAll('mat-option') as - NodeListOf; + const options = overlayContainerElement.querySelectorAll('mat-option') as + NodeListOf; - options[0].click(); - options[2].click(); - options[5].click(); - fixture.detectChanges(); + options[0].click(); + options[2].click(); + options[5].click(); + fixture.detectChanges(); - expect(trigger.textContent).toContain('Steak, Tacos, Eggs'); + expect(trigger.textContent).toContain('Steak, Tacos, Eggs'); - options[2].click(); - fixture.detectChanges(); + options[2].click(); + fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(trigger.textContent).toContain('Steak, Eggs'); - }); - }); + expect(trigger.textContent).toContain('Steak, Eggs'); })); it('should be able to set the selected value by taking an array', () => { @@ -2771,41 +2780,39 @@ describe('MatSelect', () => { expect(testInstance.select.panelOpen).toBe(true); }); - it('should sort the selected options based on their order in the panel', async(() => { + it('should sort the selected options based on their order in the panel', fakeAsync(() => { trigger.click(); fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - fixture.whenStable().then(() => { - const options = overlayContainerElement.querySelectorAll('mat-option') as - NodeListOf; + const options = overlayContainerElement.querySelectorAll('mat-option') as + NodeListOf; - options[2].click(); - options[0].click(); - options[1].click(); - fixture.detectChanges(); + options[2].click(); + options[0].click(); + options[1].click(); + fixture.detectChanges(); - expect(trigger.textContent).toContain('Steak, Pizza, Tacos'); - expect(fixture.componentInstance.control.value).toEqual(['steak-0', 'pizza-1', 'tacos-2']); - }); + expect(trigger.textContent).toContain('Steak, Pizza, Tacos'); + expect(fixture.componentInstance.control.value).toEqual(['steak-0', 'pizza-1', 'tacos-2']); })); - it('should sort the selected options in reverse in rtl', async(() => { + it('should sort the selected options in reverse in rtl', fakeAsync(() => { dir.value = 'rtl'; trigger.click(); fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - fixture.whenStable().then(() => { - const options = overlayContainerElement.querySelectorAll('mat-option') as - NodeListOf; + const options = overlayContainerElement.querySelectorAll('mat-option') as + NodeListOf; - options[2].click(); - options[0].click(); - options[1].click(); - fixture.detectChanges(); + options[2].click(); + options[0].click(); + options[1].click(); + fixture.detectChanges(); - expect(trigger.textContent).toContain('Tacos, Pizza, Steak'); - expect(fixture.componentInstance.control.value).toEqual(['steak-0', 'pizza-1', 'tacos-2']); - }); + expect(trigger.textContent).toContain('Tacos, Pizza, Steak'); + expect(fixture.componentInstance.control.value).toEqual(['steak-0', 'pizza-1', 'tacos-2']); })); it('should sort the values, that get set via the model, based on the panel order', () => { @@ -2829,34 +2836,31 @@ describe('MatSelect', () => { expect(trigger.textContent).toContain('Tacos, Pizza, Steak'); }); - it('should throw an exception when trying to set a non-array value', async(() => { + it('should throw an exception when trying to set a non-array value', () => { expect(() => { testInstance.control.setValue('not-an-array'); }).toThrowError(wrappedErrorMessage(getMatSelectNonArrayValueError())); - })); + }); - it('should throw an exception when trying to change multiple mode after init', async(() => { + it('should throw an exception when trying to change multiple mode after init', () => { expect(() => { testInstance.select.multiple = false; }).toThrowError(wrappedErrorMessage(getMatSelectDynamicMultipleError())); - })); + }); - it('should pass the `multiple` value to all of the option instances', async(() => { + it('should pass the `multiple` value to all of the option instances', fakeAsync(() => { trigger.click(); fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - fixture.whenStable().then(() => { - expect(testInstance.options.toArray().every(option => option.multiple)).toBe(true, - 'Expected `multiple` to have been added to initial set of options.'); + expect(testInstance.options.toArray().every(option => !!option.multiple)).toBe(true, + 'Expected `multiple` to have been added to initial set of options.'); - testInstance.foods.push({ value: 'cake-8', viewValue: 'Cake' }); - fixture.detectChanges(); + testInstance.foods.push({ value: 'cake-8', viewValue: 'Cake' }); + fixture.detectChanges(); - fixture.whenStable().then(() => { - expect(testInstance.options.toArray().every(option => option.multiple)).toBe(true, - 'Expected `multiple` to have been set on dynamically-added option.'); - }); - }); + expect(testInstance.options.toArray().every(option => !!option.multiple)).toBe(true, + 'Expected `multiple` to have been set on dynamically-added option.'); })); }); @@ -2864,10 +2868,10 @@ describe('MatSelect', () => { describe('theming', () => { let fixture: ComponentFixture; - beforeEach(async(() => { + beforeEach(() => { fixture = TestBed.createComponent(BasicSelectWithTheming); fixture.detectChanges(); - })); + }); it('should transfer the theme to the select panel', () => { fixture.componentInstance.theme = 'warn'; @@ -2887,7 +2891,7 @@ describe('MatSelect', () => { let formField: HTMLElement; let options: NodeListOf; - beforeEach(async(() => { + beforeEach(fakeAsync(() => { fixture = TestBed.createComponent(ResetValuesSelect); fixture.detectChanges(); trigger = fixture.debugElement.query(By.css('.mat-select-trigger')).nativeElement; @@ -2895,12 +2899,12 @@ describe('MatSelect', () => { trigger.click(); fixture.detectChanges(); + tick(SELECT_OPEN_ANIMATION); - fixture.whenStable().then(() => { - options = overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; - options[0].click(); - fixture.detectChanges(); - }); + options = overlayContainerElement.querySelectorAll('mat-option') as NodeListOf; + options[0].click(); + fixture.detectChanges(); + tick(SELECT_CLOSE_ANIMATION); })); it('should reset when an option with an undefined value is selected', fakeAsync(() => { diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index 88008d035466..8ad70fc9d2dd 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -64,6 +64,7 @@ import { MatOptionSelectionChange, mixinDisabled, mixinTabIndex, + MAT_OPTION_PARENT_COMPONENT, } from '@angular/material/core'; import {MatFormField, MatFormFieldControl} from '@angular/material/form-field'; import {Observable} from 'rxjs/Observable'; @@ -187,7 +188,10 @@ export class MatSelectTrigger {} transformPanel, fadeInContent ], - providers: [{provide: MatFormFieldControl, useExisting: MatSelect}], + providers: [ + {provide: MatFormFieldControl, useExisting: MatSelect}, + {provide: MAT_OPTION_PARENT_COMPONENT, useExisting: MatSelect} + ], }) export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, OnChanges, OnDestroy, OnInit, DoCheck, ControlValueAccessor, CanDisable, HasTabIndex, @@ -371,7 +375,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, get disableRipple(): boolean { return this._disableRipple; } set disableRipple(value: boolean) { this._disableRipple = coerceBooleanProperty(value); - this._setOptionDisableRipple(); } private _disableRipple: boolean = false; @@ -804,8 +807,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, }); this._setOptionIds(); - this._setOptionMultiple(); - this._setOptionDisableRipple(); } /** Invoked when an option is clicked. */ @@ -873,25 +874,6 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit, this._optionIds = this.options.map(option => option.id).join(' '); } - /** - * Sets the `multiple` property on each option. The promise is necessary - * in order to avoid Angular errors when modifying the property after init. - */ - private _setOptionMultiple() { - if (this.multiple) { - Promise.resolve(null).then(() => { - this.options.forEach(option => option.multiple = this.multiple); - }); - } - } - - /** Sets the `disableRipple` property on each option. */ - private _setOptionDisableRipple() { - if (this.options) { - this.options.forEach(option => option.disableRipple = this.disableRipple); - } - } - /** * Highlights the selected item. If no option is selected, it will highlight * the first item instead.