Skip to content

Commit 5d0e3d7

Browse files
committed
refactor(connected-overlay-directive): avoid issues with property initialization
* Refactors the `ConnectedOverlayDirective` not to depend on the order in which the `open` and `origin` properties are set. * Moves the `open`, `offsetX` and `offsetY` properties from using setters to `ngOnChanges` since the latter tends to output less JS. Fixes #3225.
1 parent 09c8404 commit 5d0e3d7

File tree

2 files changed

+16
-12
lines changed

2 files changed

+16
-12
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ describe('Overlay directives', () => {
194194
fixture.detectChanges();
195195

196196
fixture.componentInstance.offsetX = 15;
197+
fixture.detectChanges();
198+
197199
fixture.componentInstance.isOpen = true;
198200
fixture.detectChanges();
199201

@@ -223,6 +225,8 @@ describe('Overlay directives', () => {
223225
fixture.detectChanges();
224226

225227
fixture.componentInstance.offsetY = 55;
228+
fixture.detectChanges();
229+
226230
fixture.componentInstance.isOpen = true;
227231
fixture.detectChanges();
228232
expect(pane.style.top)

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';
@@ -63,10 +65,9 @@ export class OverlayOrigin {
6365
selector: '[cdk-connected-overlay], [connected-overlay], [cdkConnectedOverlay]',
6466
exportAs: 'cdkConnectedOverlay'
6567
})
66-
export class ConnectedOverlayDirective implements OnDestroy {
68+
export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
6769
private _overlayRef: OverlayRef;
6870
private _templatePortal: TemplatePortal;
69-
private _open = false;
7071
private _hasBackdrop = false;
7172
private _backdropSubscription: Subscription;
7273
private _positionSubscription: Subscription;
@@ -125,6 +126,9 @@ export class ConnectedOverlayDirective implements OnDestroy {
125126
/** Strategy to be used when handling scroll events while the overlay is open. */
126127
@Input() scrollStrategy: ScrollStrategy = new RepositionScrollStrategy(this._scrollDispatcher);
127128

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

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

@@ -183,6 +177,12 @@ export class ConnectedOverlayDirective implements OnDestroy {
183177
this._destroyOverlay();
184178
}
185179

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

0 commit comments

Comments
 (0)