fix(overlay): onPositionChange stream not being completed (#8562)

Completes the `onPositionChange` stream when the position strategy is disposed. Also cleans up a few cases that were unsubscribing from it explicitly.
This commit is contained in:
Kristiyan Kostadinov
2018-02-01 20:23:51 +01:00
committed by tinayuangao
parent 1417387b4f
commit 7fe651b88a
4 changed files with 19 additions and 7 deletions
+1 -4
View File
@@ -98,7 +98,6 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
private _templatePortal: TemplatePortal;
private _hasBackdrop = false;
private _backdropSubscription = Subscription.EMPTY;
private _positionSubscription = Subscription.EMPTY;
private _offsetX: number = 0;
private _offsetY: number = 0;
private _position: ConnectedPositionStrategy;
@@ -369,8 +368,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
);
}
this._positionSubscription = strategy.onPositionChange
.subscribe(pos => this.positionChange.emit(pos));
strategy.onPositionChange.subscribe(pos => this.positionChange.emit(pos));
return strategy;
}
@@ -419,6 +417,5 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
}
this._backdropSubscription.unsubscribe();
this._positionSubscription.unsubscribe();
}
}
@@ -407,6 +407,22 @@ describe('ConnectedPositionStrategy', () => {
subscription.unsubscribe();
});
it('should complete the onPositionChange stream on dispose', () => {
strategy = positionBuilder.connectedTo(
fakeElementRef,
{originX: 'end', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'});
const completeHandler = jasmine.createSpy('complete handler');
strategy.onPositionChange.subscribe(undefined, undefined, completeHandler);
strategy.attach(fakeOverlayRef(overlayElement));
strategy.apply();
strategy.dispose();
expect(completeHandler).toHaveBeenCalled();
});
it('should pick the fallback position that shows the largest area of the element', () => {
originElement.style.top = '200px';
originElement.style.right = '25px';
@@ -108,6 +108,7 @@ export class ConnectedPositionStrategy implements PositionStrategy {
dispose() {
this._applied = false;
this._resizeSubscription.unsubscribe();
this._onPositionChange.complete();
}
/** @docs-private */
+1 -3
View File
@@ -87,7 +87,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
private _overlayRef: OverlayRef | null = null;
private _menuOpen: boolean = false;
private _closeSubscription = Subscription.EMPTY;
private _positionSubscription = Subscription.EMPTY;
private _hoverSubscription = Subscription.EMPTY;
// Tracking input type is necessary so it's possible to only auto-focus
@@ -353,7 +352,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
* correct, even if a fallback position is used for the overlay.
*/
private _subscribeToPositions(position: ConnectedPositionStrategy): void {
this._positionSubscription = position.onPositionChange.subscribe(change => {
position.onPositionChange.subscribe(change => {
const posX: MenuPositionX = change.connectionPair.overlayX === 'start' ? 'after' : 'before';
const posY: MenuPositionY = change.connectionPair.overlayY === 'top' ? 'below' : 'above';
@@ -408,7 +407,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
/** Cleans up the active subscriptions. */
private _cleanUpSubscriptions(): void {
this._closeSubscription.unsubscribe();
this._positionSubscription.unsubscribe();
this._hoverSubscription.unsubscribe();
}