Skip to content

Commit 84da1c9

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 94adecd commit 84da1c9

File tree

2 files changed

+27
-37
lines changed

2 files changed

+27
-37
lines changed

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

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

183183
fixture.componentInstance.offsetX = 15;
184+
fixture.detectChanges();
185+
184186
fixture.componentInstance.isOpen = true;
185187
fixture.detectChanges();
186188

@@ -210,6 +212,8 @@ describe('Overlay directives', () => {
210212
fixture.detectChanges();
211213

212214
fixture.componentInstance.offsetY = 55;
215+
fixture.detectChanges();
216+
213217
fixture.componentInstance.isOpen = true;
214218
fixture.detectChanges();
215219
expect(pane.style.top)

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

Lines changed: 23 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import {
99
Input,
1010
OnDestroy,
1111
Output,
12-
ElementRef
12+
ElementRef,
13+
OnChanges,
14+
SimpleChanges,
1315
} from '@angular/core';
1416
import {Overlay, OVERLAY_PROVIDERS} from './overlay';
1517
import {OverlayRef} from './overlay-ref';
@@ -58,15 +60,12 @@ export class OverlayOrigin {
5860
selector: '[cdk-connected-overlay], [connected-overlay]',
5961
exportAs: 'cdkConnectedOverlay'
6062
})
61-
export class ConnectedOverlayDirective implements OnDestroy {
63+
export class ConnectedOverlayDirective implements OnDestroy, OnChanges {
6264
private _overlayRef: OverlayRef;
6365
private _templatePortal: TemplatePortal;
64-
private _open = false;
6566
private _hasBackdrop = false;
6667
private _backdropSubscription: Subscription;
6768
private _positionSubscription: Subscription;
68-
private _offsetX: number = 0;
69-
private _offsetY: number = 0;
7069
private _position: ConnectedPositionStrategy;
7170

7271
/** Origin for the connected overlay. */
@@ -76,30 +75,10 @@ export class ConnectedOverlayDirective implements OnDestroy {
7675
@Input() positions: ConnectionPositionPair[];
7776

7877
/** The offset in pixels for the overlay connection point on the x-axis */
79-
@Input()
80-
get offsetX(): number {
81-
return this._offsetX;
82-
}
83-
84-
set offsetX(offsetX: number) {
85-
this._offsetX = offsetX;
86-
if (this._position) {
87-
this._position.withOffsetX(offsetX);
88-
}
89-
}
78+
@Input() offsetX: number = 0;
9079

9180
/** The offset in pixels for the overlay connection point on the y-axis */
92-
@Input()
93-
get offsetY() {
94-
return this._offsetY;
95-
}
96-
97-
set offsetY(offsetY: number) {
98-
this._offsetY = offsetY;
99-
if (this._position) {
100-
this._position.withOffsetY(offsetY);
101-
}
102-
}
81+
@Input() offsetY: number = 0;
10382

10483
/** The width of the overlay panel. */
10584
@Input() width: number | string;
@@ -116,6 +95,9 @@ export class ConnectedOverlayDirective implements OnDestroy {
11695
/** The custom class to be set on the backdrop element. */
11796
@Input() backdropClass: string;
11897

98+
/** Whether the overlay is open. */
99+
@Input() open: boolean = false;
100+
119101
/** Whether or not the overlay should attach a backdrop. */
120102
@Input()
121103
get hasBackdrop() {
@@ -126,16 +108,6 @@ export class ConnectedOverlayDirective implements OnDestroy {
126108
this._hasBackdrop = coerceBooleanProperty(value);
127109
}
128110

129-
@Input()
130-
get open() {
131-
return this._open;
132-
}
133-
134-
set open(value: boolean) {
135-
value ? this._attachOverlay() : this._detachOverlay();
136-
this._open = value;
137-
}
138-
139111
/** Event emitted when the backdrop is clicked. */
140112
@Output() backdropClick = new EventEmitter<void>();
141113

@@ -172,6 +144,20 @@ export class ConnectedOverlayDirective implements OnDestroy {
172144
this._destroyOverlay();
173145
}
174146

147+
ngOnChanges(changes: SimpleChanges) {
148+
if (changes['open']) {
149+
changes['open'].currentValue ? this._attachOverlay() : this._detachOverlay();
150+
}
151+
152+
if (changes['offsetX'] && this._position) {
153+
this._position.withOffsetX(changes['offsetX'].currentValue);
154+
}
155+
156+
if (changes['offsetY'] && this._position) {
157+
this._position.withOffsetY(changes['offsetY'].currentValue);
158+
}
159+
}
160+
175161
/** Creates an overlay */
176162
private _createOverlay() {
177163
if (!this.positions || !this.positions.length) {

0 commit comments

Comments
 (0)