Skip to content

Commit 7bd3930

Browse files
committed
refactor(select): simplify option sync and more robust unit tests
* Currently the select sets the `multiple` and `disableRipple` values on its options manually. In order to avoid "changed after checked" errors we wrap the calls in promises, however this also forces us to have `async` tests which can time out if the browser is out of focus. These changes add a provider that allows for the options to take the value directly from the select. * Refactors some unit tests that have been timing out in Firefox to run in the `fakeAsync` zone instead of the `async` one.
1 parent 9673f63 commit 7bd3930

File tree

4 files changed

+118
-158
lines changed

4 files changed

+118
-158
lines changed

src/lib/core/option/option.spec.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('MatOption component', () => {
2929
});
3030

3131
it('should show ripples by default', () => {
32-
expect(optionInstance.disableRipple).toBe(false, 'Expected ripples to be enabled by default');
32+
expect(optionInstance.disableRipple).toBeFalsy('Expected ripples to be enabled by default');
3333
expect(optionNativeElement.querySelectorAll('.mat-ripple-element').length)
3434
.toBe(0, 'Expected no ripples to show up initially');
3535

@@ -54,20 +54,6 @@ describe('MatOption component', () => {
5454
.toBe(0, 'Expected no ripples to show up after click on a disabled option.');
5555
});
5656

57-
it('should not show ripples if the ripples are disabled using disableRipple', () => {
58-
expect(optionNativeElement.querySelectorAll('.mat-ripple-element').length)
59-
.toBe(0, 'Expected no ripples to show up initially');
60-
61-
optionInstance.disableRipple = true;
62-
fixture.detectChanges();
63-
64-
dispatchFakeEvent(optionNativeElement, 'mousedown');
65-
dispatchFakeEvent(optionNativeElement, 'mouseup');
66-
67-
expect(optionNativeElement.querySelectorAll('.mat-ripple-element').length)
68-
.toBe(0, 'Expected no ripples to show up after click when ripples are disabled.');
69-
});
70-
7157
});
7258

7359
});

src/lib/core/option/option.ts

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import {
1919
Output,
2020
QueryList,
2121
ViewEncapsulation,
22+
InjectionToken,
23+
Inject,
2224
} from '@angular/core';
2325
import {MatOptgroup} from './optgroup';
2426

@@ -33,6 +35,22 @@ export class MatOptionSelectionChange {
3335
constructor(public source: MatOption, public isUserInput = false) { }
3436
}
3537

38+
/**
39+
* Describes a parent form control that manages a list options.
40+
* Contains properties that the options can inherit.
41+
* @docs-private
42+
*/
43+
export interface MatOptionParentControl {
44+
disableRipple?: boolean;
45+
multiple?: boolean;
46+
}
47+
48+
/**
49+
* Injection token used to provide the parent control to options.
50+
*/
51+
export const MAT_OPTION_PARENT_CONTROL =
52+
new InjectionToken<MatOptionParentControl>('MAT_OPTION_PARENT_CONTROL');
53+
3654
/**
3755
* Single option inside of a `<mat-select>` element.
3856
*/
@@ -60,24 +78,13 @@ export class MatOptionSelectionChange {
6078
changeDetection: ChangeDetectionStrategy.OnPush,
6179
})
6280
export class MatOption {
63-
private _selected: boolean = false;
64-
private _active: boolean = false;
65-
private _multiple: boolean = false;
66-
private _disableRipple: boolean = false;
67-
68-
/** Whether the option is disabled. */
69-
private _disabled: boolean = false;
70-
71-
private _id: string = `mat-option-${_uniqueIdCounter++}`;
81+
private _selected = false;
82+
private _active = false;
83+
private _disabled = false;
84+
private _id = `mat-option-${_uniqueIdCounter++}`;
7285

7386
/** Whether the wrapping component is in multiple selection mode. */
74-
get multiple() { return this._multiple; }
75-
set multiple(value: boolean) {
76-
if (value !== this._multiple) {
77-
this._multiple = value;
78-
this._changeDetectorRef.markForCheck();
79-
}
80-
}
87+
get multiple() { return this._parent && this._parent.multiple; }
8188

8289
/** The unique ID of the option. */
8390
get id(): string { return this._id; }
@@ -94,18 +101,15 @@ export class MatOption {
94101
set disabled(value: any) { this._disabled = coerceBooleanProperty(value); }
95102

96103
/** Whether ripples for the option are disabled. */
97-
get disableRipple() { return this._disableRipple; }
98-
set disableRipple(value: boolean) {
99-
this._disableRipple = value;
100-
this._changeDetectorRef.markForCheck();
101-
}
104+
get disableRipple() { return this._parent && this._parent.disableRipple; }
102105

103106
/** Event emitted when the option is selected or deselected. */
104107
@Output() onSelectionChange = new EventEmitter<MatOptionSelectionChange>();
105108

106109
constructor(
107110
private _element: ElementRef,
108111
private _changeDetectorRef: ChangeDetectorRef,
112+
@Optional() @Inject(MAT_OPTION_PARENT_CONTROL) private _parent: MatOptionParentControl,
109113
@Optional() public readonly group: MatOptgroup) {}
110114

111115
/**

0 commit comments

Comments
 (0)