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.

Author: crisbeto

src/cdk/overlay/overlay-directives.ts

@@ -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();
   }
 }

src/cdk/overlay/position/connected-position-strategy.spec.ts

@@ -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';

src/cdk/overlay/position/connected-position-strategy.ts

@@ -108,6 +108,7 @@ export class ConnectedPositionStrategy implements PositionStrategy {
   dispose() {
     this._applied = false;
     this._resizeSubscription.unsubscribe();
+    this._onPositionChange.complete();
   }
 
   /** @docs-private */

src/lib/menu/menu-trigger.ts

@@ -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();
   }