Skip to content

Commit

Permalink
fix(overlay): validate that ConnectedPositionStrategy positions are p…
Browse files Browse the repository at this point in the history
…assed 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.
  • Loading branch information
crisbeto committed Jan 18, 2018
1 parent 56e2566 commit 11f154d
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 3 deletions.
54 changes: 54 additions & 0 deletions src/cdk/overlay/position/connected-position-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
23 changes: 20 additions & 3 deletions src/cdk/overlay/position/connected-position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
OverlayConnectionPosition,
ConnectedOverlayPositionChange,
ScrollingVisibility,
validateHorizontalPosition,
validateVerticalPosition,
} from './connected-position';
import {Subject} from 'rxjs/Subject';
import {Subscription} from 'rxjs/Subscription';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -428,14 +433,26 @@ 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.');
}

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. */
Expand Down
26 changes: 26 additions & 0 deletions src/cdk/overlay/position/connected-position.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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".`);
}
}

0 comments on commit 11f154d

Please sign in to comment.