refactor(overlay): move connected position assertions into FlexibleConnectedPositionStrategy

Since the `ConnectedPositionStrategy` proxies everything to `FlexibleConnectedPositionStrategy`, these changes move the assertions that verify that the config looks correctly into the `FlexibleConnectedPositionStrategy`.

Author: crisbeto

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

@@ -26,7 +26,6 @@ const DEFAULT_WIDTH = 60;
 describe('ConnectedPositionStrategy', () => {
   let overlay: Overlay;
   let overlayContainer: OverlayContainer;
-  let overlayContainerElement: HTMLElement;
   let zone: MockNgZone;
   let overlayRef: OverlayRef;
 
@@ -39,7 +38,6 @@ describe('ConnectedPositionStrategy', () => {
     inject([Overlay, OverlayContainer], (o: Overlay, oc: OverlayContainer) => {
       overlay = o;
       overlayContainer = oc;
-      overlayContainerElement = oc.getContainerElement();
     })();
   });
 
@@ -775,61 +773,6 @@ describe('ConnectedPositionStrategy', () => {
 
   });
 
-  describe('validations', () => {
-    let overlayElement: HTMLElement;
-    let originElement: HTMLElement;
-    let positionStrategy: ConnectedPositionStrategy;
-
-    beforeEach(() => {
-      overlayElement = createPositionedBlockElement();
-      overlayContainerElement.appendChild(overlayElement);
-      originElement = createBlockElement();
-
-      positionStrategy = overlay.position().connectedTo(
-          new ElementRef(originElement),
-          {originX: 'start', originY: 'bottom'},
-          {overlayX: 'start', overlayY: 'top'});
-
-      attachOverlay(positionStrategy);
-    });
-
-    afterEach(() => {
-      positionStrategy.dispose();
-    });
-
-    it('should throw when attaching without any positions', () => {
-      positionStrategy.withPositions([]);
-      expect(() => positionStrategy.apply()).toThrow();
-    });
-
-    it('should throw when passing in something that is missing a connection point', () => {
-      positionStrategy.withPositions([{originY: 'top', overlayX: 'start', overlayY: 'top'} as any]);
-      expect(() => positionStrategy.apply()).toThrow();
-    });
-
-    it('should throw when passing in something that has an invalid X position', () => {
-      positionStrategy.withPositions([{
-        originX: 'left',
-        originY: 'top',
-        overlayX: 'left',
-        overlayY: 'top'
-      } as any]);
-
-      expect(() => positionStrategy.apply()).toThrow();
-    });
-
-    it('should throw when passing in something that has an invalid Y position', () => {
-      positionStrategy.withPositions([{
-        originX: 'start',
-        originY: 'middle',
-        overlayX: 'start',
-        overlayY: 'middle'
-      } as any]);
-
-      expect(() => positionStrategy.apply()).toThrow();
-    });
-  });
-
 });
 
 /** Creates an absolutely positioned, display: block element with a default size. */

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

@@ -16,8 +16,6 @@ import {
   ConnectionPositionPair,
   OriginConnectionPosition,
   OverlayConnectionPosition,
-  validateHorizontalPosition,
-  validateVerticalPosition,
 } from './connected-position';
 import {FlexibleConnectedPositionStrategy} from './flexible-connected-position-strategy';
 import {PositionStrategy} from './position-strategy';
@@ -110,7 +108,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
    * @docs-private
    */
   apply(): void {
-    this._validatePositions();
     this._positionStrategy.apply();
   }
 
@@ -120,7 +117,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
    * allows one to re-align the panel without changing the orientation of the panel.
    */
   recalculateLastPosition(): void {
-    this._validatePositions();
     this._positionStrategy.reapplyLastPosition();
   }
 
@@ -214,21 +210,4 @@ export class ConnectedPositionStrategy implements PositionStrategy {
     this._positionStrategy.setOrigin(origin);
     return this;
   }
-
-  /** Validates that the current position match the expected values. */
-  private _validatePositions(): void {
-    if (!this._preferredPositions.length) {
-      throw Error('ConnectedPositionStrategy: At least one position is required.');
-    }
-
-    // TODO(crisbeto): remove these once Angular's template type
-    // checking is advanced enough to catch these cases.
-    // TODO(crisbeto): port these checks into the flexible positioning.
-    this._preferredPositions.forEach(pair => {
-      validateHorizontalPosition('originX', pair.originX);
-      validateVerticalPosition('originY', pair.originY);
-      validateHorizontalPosition('overlayX', pair.overlayX);
-      validateVerticalPosition('overlayY', pair.overlayY);
-    });
-  }
 }

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

@@ -1588,6 +1588,53 @@ describe('FlexibleConnectedPositionStrategy', () => {
 
   });
 
+  describe('validations', () => {
+    let originElement: HTMLElement;
+    let positionStrategy: FlexibleConnectedPositionStrategy;
+
+    beforeEach(() => {
+      originElement = createPositionedBlockElement();
+      document.body.appendChild(originElement);
+      positionStrategy = overlay.position().flexibleConnectedTo(new ElementRef(originElement));
+    });
+
+    afterEach(() => {
+      positionStrategy.dispose();
+    });
+
+    it('should throw when attaching without any positions', () => {
+      positionStrategy.withPositions([]);
+      expect(() => attachOverlay({positionStrategy})).toThrow();
+    });
+
+    it('should throw when passing in something that is missing a connection point', () => {
+      positionStrategy.withPositions([{originY: 'top', overlayX: 'start', overlayY: 'top'} as any]);
+      expect(() => attachOverlay({positionStrategy})).toThrow();
+    });
+
+    it('should throw when passing in something that has an invalid X position', () => {
+      positionStrategy.withPositions([{
+        originX: 'left',
+        originY: 'top',
+        overlayX: 'left',
+        overlayY: 'top'
+      } as any]);
+
+      expect(() => attachOverlay({positionStrategy})).toThrow();
+    });
+
+    it('should throw when passing in something that has an invalid Y position', () => {
+      positionStrategy.withPositions([{
+        originX: 'start',
+        originY: 'middle',
+        overlayX: 'start',
+        overlayY: 'middle'
+      } as any]);
+
+      expect(() => attachOverlay({positionStrategy})).toThrow();
+    });
+  });
+
 });
 
 /** Creates an absolutely positioned, display: block element with a default size. */

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

@@ -13,6 +13,8 @@ import {
   ConnectedOverlayPositionChange,
   ConnectionPositionPair,
   ScrollingVisibility,
+  validateHorizontalPosition,
+  validateVerticalPosition,
 } from './connected-position';
 import {Observable, Subscription, Subject} from 'rxjs';
 import {OverlayRef} from '../overlay-ref';
@@ -159,6 +161,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
       return;
     }
 
+    this._validatePositions();
+
     // If the position has been applied already (e.g. when the overlay was opened) and the
     // consumer opted into locking in the position, re-use the old position, in order to
     // prevent the overlay from jumping around.
@@ -284,6 +288,7 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
    */
   reapplyLastPosition(): void {
     if (!this._isDisposed) {
+      this._validatePositions();
       this._originRect = this._origin.getBoundingClientRect();
       this._overlayRect = this._pane.getBoundingClientRect();
       this._viewportRect = this._getNarrowedViewportRect();
@@ -912,6 +917,22 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
 
     return position.offsetY == null ? this._offsetY : position.offsetY;
   }
+
+  /** Validates that the current position match the expected values. */
+  private _validatePositions(): void {
+    if (!this._preferredPositions.length) {
+      throw Error('FlexibleConnectedPositionStrategy: At least one position is required.');
+    }
+
+    // TODO(crisbeto): remove these once Angular's template type
+    // checking is advanced enough to catch these cases.
+    this._preferredPositions.forEach(pair => {
+      validateHorizontalPosition('originX', pair.originX);
+      validateVerticalPosition('originY', pair.originY);
+      validateHorizontalPosition('overlayX', pair.overlayX);
+      validateVerticalPosition('overlayY', pair.overlayY);
+    });
+  }
 }
 
 /** A simple (x, y) coordinate. */