Skip to content

Commit 2c0506c

Browse files
crisbetokara
authored andcommitted
refactor(connected-overlay-directive): avoid issues with property initialization (#3228)
Fixes #3225.
1 parent f27df86 commit 2c0506c

File tree

2 files changed

+41
-15
lines changed

2 files changed

+41
-15
lines changed

src/lib/core/overlay/overlay-directives.spec.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import {ComponentFixture, TestBed} from '@angular/core/testing';
1+
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
22
import {Component, ViewChild} from '@angular/core';
33
import {By} from '@angular/platform-browser';
4-
import {ConnectedOverlayDirective, OverlayModule} from './overlay-directives';
4+
import {ConnectedOverlayDirective, OverlayModule, OverlayOrigin} from './overlay-directives';
55
import {OverlayContainer} from './overlay-container';
66
import {ConnectedPositionStrategy} from './position/connected-position-strategy';
77
import {ConnectedOverlayPositionChange} from './position/connected-position';
@@ -18,7 +18,7 @@ describe('Overlay directives', () => {
1818
beforeEach(() => {
1919
TestBed.configureTestingModule({
2020
imports: [OverlayModule],
21-
declarations: [ConnectedOverlayDirectiveTest],
21+
declarations: [ConnectedOverlayDirectiveTest, ConnectedOverlayPropertyInitOrder],
2222
providers: [
2323
{provide: OverlayContainer, useFactory: () => {
2424
overlayContainerElement = document.createElement('div');
@@ -111,6 +111,21 @@ describe('Overlay directives', () => {
111111
'Expected overlay to have been detached.');
112112
});
113113

114+
it('should not depend on the order in which the `origin` and `open` are set', async(() => {
115+
fixture.destroy();
116+
117+
const propOrderFixture = TestBed.createComponent(ConnectedOverlayPropertyInitOrder);
118+
propOrderFixture.detectChanges();
119+
120+
const overlayDirective = propOrderFixture.componentInstance.connectedOverlayDirective;
121+
122+
expect(() => {
123+
overlayDirective.open = true;
124+
overlayDirective.origin = propOrderFixture.componentInstance.trigger;
125+
propOrderFixture.detectChanges();
126+
}).not.toThrow();
127+
}));
128+
114129
describe('inputs', () => {
115130

116131
it('should set the width', () => {
@@ -310,3 +325,14 @@ class ConnectedOverlayDirectiveTest {
310325

311326
@ViewChild(ConnectedOverlayDirective) connectedOverlayDirective: ConnectedOverlayDirective;
312327
}
328+
329+
@Component({
330+
template: `
331+
<button cdk-overlay-origin #trigger="cdkOverlayOrigin">Toggle menu</button>
332+
<ng-template cdk-connected-overlay>Menu content</ng-template>`,
333+
})
334+
class ConnectedOverlayPropertyInitOrder {
335+
@ViewChild(ConnectedOverlayDirective) connectedOverlayDirective: ConnectedOverlayDirective;
336+
@ViewChild('trigger') trigger: OverlayOrigin;
337+
}
338+

src/lib/core/overlay/overlay-directives.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {
1010
Output,
1111
ElementRef,
1212
Renderer2,
13+
OnChanges,
14+
SimpleChanges,
1315
} from '@angular/core';
1416
import {Overlay, OVERLAY_PROVIDERS} from './overlay';
1517
import {OverlayRef} from './overlay-ref';
@@ -64,10 +66,9 @@ export class OverlayOrigin {
6466
selector: '[cdk-connected-overlay], [connected-overlay], [cdkConnectedOverlay]',
6567
exportAs: 'cdkConnectedOverlay'
6668
})
67-
export class ConnectedOverlayDirective implements OnDestroy {
69+
export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
6870
private _overlayRef: OverlayRef;
6971
private _templatePortal: TemplatePortal;
70-
private _open = false;
7172
private _hasBackdrop = false;
7273
private _backdropSubscription: Subscription;
7374
private _positionSubscription: Subscription;
@@ -126,6 +127,9 @@ export class ConnectedOverlayDirective implements OnDestroy {
126127
/** Strategy to be used when handling scroll events while the overlay is open. */
127128
@Input() scrollStrategy: ScrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher);
128129

130+
/** Whether the overlay is open. */
131+
@Input() open: boolean = false;
132+
129133
/** Whether or not the overlay should attach a backdrop. */
130134
@Input()
131135
get hasBackdrop() {
@@ -136,16 +140,6 @@ export class ConnectedOverlayDirective implements OnDestroy {
136140
this._hasBackdrop = coerceBooleanProperty(value);
137141
}
138142

139-
@Input()
140-
get open() {
141-
return this._open;
142-
}
143-
144-
set open(value: boolean) {
145-
value ? this._attachOverlay() : this._detachOverlay();
146-
this._open = value;
147-
}
148-
149143
/** Event emitted when the backdrop is clicked. */
150144
@Output() backdropClick = new EventEmitter<void>();
151145

@@ -184,6 +178,12 @@ export class ConnectedOverlayDirective implements OnDestroy {
184178
this._destroyOverlay();
185179
}
186180

181+
ngOnChanges(changes: SimpleChanges) {
182+
if (changes['open']) {
183+
this.open ? this._attachOverlay() : this._detachOverlay();
184+
}
185+
}
186+
187187
/** Creates an overlay */
188188
private _createOverlay() {
189189
if (!this.positions || !this.positions.length) {

0 commit comments

Comments
 (0)