From e6a5dd16eb6c063df261e7f21b7c767e51f0a83a Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 19 Jan 2018 21:14:43 +0100 Subject: [PATCH] fix(overlay): validate that ConnectedPositionStrategy positions are passed in correctly at runtime Since we have some cases where we pass in positions verbatim to the connected position strategy (the connected position directive), we could end up in a situation where the consumer passed in a set of invalid positions that don't break necessarily, but don't work correctly either. These changes add some sanity checks at runtime to make debugging easier. --- .../connected-position-strategy.spec.ts | 54 +++++++++++++++++++ .../position/connected-position-strategy.ts | 25 +++++++-- .../overlay/position/connected-position.ts | 26 +++++++++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/cdk/overlay/position/connected-position-strategy.spec.ts b/src/cdk/overlay/position/connected-position-strategy.spec.ts index 4a77926d5b99..d4f8de5214fd 100644 --- a/src/cdk/overlay/position/connected-position-strategy.spec.ts +++ b/src/cdk/overlay/position/connected-position-strategy.spec.ts @@ -777,6 +777,60 @@ describe('ConnectedPositionStrategy', () => { }); + describe('validations', () => { + let overlayElement: HTMLElement; + let originElement: HTMLElement; + let strategy: ConnectedPositionStrategy; + + beforeEach(() => { + overlayElement = createPositionedBlockElement(); + overlayContainerElement.appendChild(overlayElement); + originElement = createBlockElement(); + + strategy = positionBuilder.connectedTo( + new ElementRef(originElement), + {originX: 'start', originY: 'bottom'}, + {overlayX: 'start', overlayY: 'top'}); + strategy.attach(fakeOverlayRef(overlayElement)); + }); + + afterEach(() => { + strategy.dispose(); + }); + + it('should throw when attaching without any positions', () => { + strategy.withPositions([]); + expect(() => strategy.apply()).toThrow(); + }); + + it('should throw when passing in something that is missing a connection point', () => { + strategy.withPositions([{originY: 'top', overlayX: 'start', overlayY: 'top'} as any]); + expect(() => strategy.apply()).toThrow(); + }); + + it('should throw when passing in something that has an invalid X position', () => { + strategy.withPositions([{ + originX: 'left', + originY: 'top', + overlayX: 'left', + overlayY: 'top' + } as any]); + + expect(() => strategy.apply()).toThrow(); + }); + + it('should throw when passing in something that has an invalid Y position', () => { + strategy.withPositions([{ + originX: 'start', + originY: 'middle', + overlayX: 'start', + overlayY: 'middle' + } as any]); + + expect(() => strategy.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 6dd2d337e129..1aee633cb775 100644 --- a/src/cdk/overlay/position/connected-position-strategy.ts +++ b/src/cdk/overlay/position/connected-position-strategy.ts @@ -15,6 +15,8 @@ import { OverlayConnectionPosition, ConnectedOverlayPositionChange, ScrollingVisibility, + validateHorizontalPosition, + validateVerticalPosition, } from './connected-position'; import {Subject} from 'rxjs/Subject'; import {Subscription} from 'rxjs/Subscription'; @@ -130,6 +132,7 @@ export class ConnectedPositionStrategy implements PositionStrategy { return; } + this._validatePositions(); this._applied = true; // We need the bounding rects for the origin and the overlay to determine how to position @@ -183,6 +186,8 @@ export class ConnectedPositionStrategy implements PositionStrategy { return; } + this._validatePositions(); + const originRect = this._origin.getBoundingClientRect(); const overlayRect = this._pane.getBoundingClientRect(); const viewportSize = this._viewportRuler.getViewportSize(); @@ -428,14 +433,28 @@ export class ConnectedPositionStrategy implements PositionStrategy { this._onPositionChange.next(positionChange); } - /** - * Subtracts the amount that an element is overflowing on an axis from it's length. - */ + /** Subtracts the amount that an element is overflowing on an axis from it's length. */ private _subtractOverflows(length: number, ...overflows: number[]): number { return overflows.reduce((currentValue: number, currentOverflow: number) => { return currentValue - Math.max(currentOverflow, 0); }, length); } + + /** 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. + 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. */ diff --git a/src/cdk/overlay/position/connected-position.ts b/src/cdk/overlay/position/connected-position.ts index 0b8d8a9c6173..9dd364e7c236 100644 --- a/src/cdk/overlay/position/connected-position.ts +++ b/src/cdk/overlay/position/connected-position.ts @@ -90,3 +90,29 @@ export class ConnectedOverlayPositionChange { /** @docs-private */ @Optional() public scrollableViewProperties: ScrollingVisibility) {} } + +/** + * Validates whether a vertical position property matches the expected values. + * @param property Name of the property being validated. + * @param value Value of the property being validated. + * @docs-private + */ +export function validateVerticalPosition(property: string, value: VerticalConnectionPos) { + if (value !== 'top' && value !== 'bottom' && value !== 'center') { + throw Error(`ConnectedPosition: Invalid ${property} "${value}". ` + + `Expected "top", "bottom" or "center".`); + } +} + +/** + * Validates whether a horizontal position property matches the expected values. + * @param property Name of the property being validated. + * @param value Value of the property being validated. + * @docs-private + */ +export function validateHorizontalPosition(property: string, value: HorizontalConnectionPos) { + if (value !== 'start' && value !== 'end' && value !== 'center') { + throw Error(`ConnectedPosition: Invalid ${property} "${value}". ` + + `Expected "start", "end" or "center".`); + } +}