From 005ecf2317a32dc1709e10d0c25b67e191f7850d Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 21 Apr 2018 15:15:19 -0600 Subject: [PATCH] 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`. --- .../connected-position-strategy.spec.ts | 57 ------------------- .../position/connected-position-strategy.ts | 21 ------- ...exible-connected-position-strategy.spec.ts | 51 +++++++++++++++++ .../flexible-connected-position-strategy.ts | 22 +++++++ 4 files changed, 73 insertions(+), 78 deletions(-) diff --git a/src/cdk/overlay/position/connected-position-strategy.spec.ts b/src/cdk/overlay/position/connected-position-strategy.spec.ts index 0f7a9764b2c0..d39260a414b7 100644 --- a/src/cdk/overlay/position/connected-position-strategy.spec.ts +++ b/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. */ diff --git a/src/cdk/overlay/position/connected-position-strategy.ts b/src/cdk/overlay/position/connected-position-strategy.ts index b9dfbb6795cd..ca1f0124830c 100644 --- a/src/cdk/overlay/position/connected-position-strategy.ts +++ b/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'; @@ -109,7 +107,6 @@ export class ConnectedPositionStrategy implements PositionStrategy { * @docs-private */ apply(): void { - this._validatePositions(); this._positionStrategy.apply(); } @@ -119,7 +116,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(); } @@ -213,21 +209,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); - }); - } } diff --git a/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts b/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts index ee34be156e00..d443e28ceadd 100644 --- a/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts +++ b/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts @@ -1593,6 +1593,57 @@ 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', () => { + expect(() => positionStrategy.withPositions([])).toThrow(); + }); + + it('should throw when passing in something that is missing a connection point', () => { + expect(() => { + positionStrategy.withPositions([{ + originY: 'top', + overlayX: 'start', + overlayY: 'top' + } as any]); + }).toThrow(); + }); + + it('should throw when passing in something that has an invalid X position', () => { + expect(() => { + positionStrategy.withPositions([{ + originX: 'left', + originY: 'top', + overlayX: 'left', + overlayY: 'top' + } as any]); + }).toThrow(); + }); + + it('should throw when passing in something that has an invalid Y position', () => { + expect(() => { + positionStrategy.withPositions([{ + originX: 'start', + originY: 'middle', + overlayX: 'start', + overlayY: 'middle' + } as any]); + }).toThrow(); + }); + }); + }); /** Creates an absolutely positioned, display: block element with a default size. */ diff --git a/src/cdk/overlay/position/flexible-connected-position-strategy.ts b/src/cdk/overlay/position/flexible-connected-position-strategy.ts index cb58b06cea04..d9c3929f631a 100644 --- a/src/cdk/overlay/position/flexible-connected-position-strategy.ts +++ b/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'; @@ -127,6 +129,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy { throw Error('This position strategy is already attached to an overlay'); } + this._validatePositions(); + overlayRef.hostElement.classList.add('cdk-overlay-connected-position-bounding-box'); this._overlayRef = overlayRef; @@ -315,6 +319,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy { this._lastPosition = null; } + this._validatePositions(); + return this; } @@ -901,6 +907,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. */