From 1d8e9af7aa65843a1098121945f247bd96f8aeab Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Fri, 21 Sep 2018 19:05:04 +0200
Subject: [PATCH] fix(overlay): connected overlay directive inputs not updating
 position strategy (#13066)

Ensures that all of the `@Input` changes get propagated to the underlying position strategy. Up until now we did it on a case-by-case basis which meant that a lot of them weren't, because we never added an extra call inside the `ngOnChanges`.
---
 src/cdk/overlay/overlay-directives.ts | 78 ++++++++++++---------------
 1 file changed, 33 insertions(+), 45 deletions(-)

diff --git a/src/cdk/overlay/overlay-directives.ts b/src/cdk/overlay/overlay-directives.ts
index 901fa42b8ff..7252f8493a5 100644
--- a/src/cdk/overlay/overlay-directives.ts
+++ b/src/cdk/overlay/overlay-directives.ts
@@ -128,7 +128,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     this._offsetX = offsetX;
 
     if (this._position) {
-      this._setPositions(this._position);
+      this._updatePositionStrategy(this._position);
     }
   }
 
@@ -139,7 +139,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     this._offsetY = offsetY;
 
     if (this._position) {
-      this._setPositions(this._position);
+      this._updatePositionStrategy(this._position);
     }
   }
 
@@ -240,20 +240,10 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
 
   ngOnChanges(changes: SimpleChanges) {
     if (this._position) {
-      if (changes['positions']) {
-        this._position.withPositions(this.positions);
-      }
+      this._updatePositionStrategy(this._position);
 
-      if (changes['lockPosition']) {
-        this._position.withLockedPosition(this.lockPosition);
-      }
-
-      if (changes['origin']) {
-        this._position.setOrigin(this.origin.elementRef);
-
-        if (this.open) {
-          this._position.apply();
-        }
+      if (changes['origin'] && this.open) {
+        this._position.apply();
       }
     }
 
@@ -269,6 +259,14 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     }
 
     this._overlayRef = this._overlay.create(this._buildConfig());
+
+    this._overlayRef.keydownEvents().subscribe((event: KeyboardEvent) => {
+      this.overlayKeydown.next(event);
+
+      if (event.keyCode === ESCAPE) {
+        this._detachOverlay();
+      }
+    });
   }
 
   /** Builds the overlay config based on the directive's inputs */
@@ -308,51 +306,41 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     return overlayConfig;
   }
 
-  /** Returns the position strategy of the overlay to be set on the overlay config */
-  private _createPositionStrategy(): FlexibleConnectedPositionStrategy {
-    const strategy = this._overlay.position()
-      .flexibleConnectedTo(this.origin.elementRef)
+  /** Updates the state of a position strategy, based on the values of the directive inputs. */
+  private _updatePositionStrategy(positionStrategy: FlexibleConnectedPositionStrategy) {
+    const positions: ConnectedPosition[] = this.positions.map(currentPosition => ({
+      originX: currentPosition.originX,
+      originY: currentPosition.originY,
+      overlayX: currentPosition.overlayX,
+      overlayY: currentPosition.overlayY,
+      offsetX: currentPosition.offsetX || this.offsetX,
+      offsetY: currentPosition.offsetY || this.offsetY
+    }));
+
+    return positionStrategy
+      .setOrigin(this.origin.elementRef)
+      .withPositions(positions)
       .withFlexibleDimensions(this.flexibleDimensions)
       .withPush(this.push)
       .withGrowAfterOpen(this.growAfterOpen)
       .withViewportMargin(this.viewportMargin)
       .withLockedPosition(this.lockPosition);
+  }
 
-    this._setPositions(strategy);
+  /** Returns the position strategy of the overlay to be set on the overlay config */
+  private _createPositionStrategy(): FlexibleConnectedPositionStrategy {
+    const strategy = this._overlay.position().flexibleConnectedTo(this.origin.elementRef);
+
+    this._updatePositionStrategy(strategy);
     strategy.positionChanges.subscribe(p => this.positionChange.emit(p));
 
     return strategy;
   }
 
-  /**
-   * Sets the primary and fallback positions of a positions strategy,
-   * based on the current directive inputs.
-   */
-  private _setPositions(positionStrategy: FlexibleConnectedPositionStrategy) {
-    const positions: ConnectedPosition[] = this.positions.map(pos => ({
-      originX: pos.originX,
-      originY: pos.originY,
-      overlayX: pos.overlayX,
-      overlayY: pos.overlayY,
-      offsetX: pos.offsetX || this.offsetX,
-      offsetY: pos.offsetY || this.offsetY
-    }));
-
-    positionStrategy.withPositions(positions);
-  }
-
   /** Attaches the overlay and subscribes to backdrop clicks if backdrop exists */
   private _attachOverlay() {
     if (!this._overlayRef) {
       this._createOverlay();
-
-      this._overlayRef!.keydownEvents().subscribe((event: KeyboardEvent) => {
-        this.overlayKeydown.next(event);
-
-        if (event.keyCode === ESCAPE) {
-          this._detachOverlay();
-        }
-      });
     } else {
       // Update the overlay size, in case the directive's inputs have changed
       this._overlayRef.updateSize({