Skip to content

Commit e6a5dd1

Browse files
committed
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.
1 parent 56e2566 commit e6a5dd1

File tree

3 files changed

+102
-3
lines changed

3 files changed

+102
-3
lines changed

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,60 @@ describe('ConnectedPositionStrategy', () => {
777777

778778
});
779779

780+
describe('validations', () => {
781+
let overlayElement: HTMLElement;
782+
let originElement: HTMLElement;
783+
let strategy: ConnectedPositionStrategy;
784+
785+
beforeEach(() => {
786+
overlayElement = createPositionedBlockElement();
787+
overlayContainerElement.appendChild(overlayElement);
788+
originElement = createBlockElement();
789+
790+
strategy = positionBuilder.connectedTo(
791+
new ElementRef(originElement),
792+
{originX: 'start', originY: 'bottom'},
793+
{overlayX: 'start', overlayY: 'top'});
794+
strategy.attach(fakeOverlayRef(overlayElement));
795+
});
796+
797+
afterEach(() => {
798+
strategy.dispose();
799+
});
800+
801+
it('should throw when attaching without any positions', () => {
802+
strategy.withPositions([]);
803+
expect(() => strategy.apply()).toThrow();
804+
});
805+
806+
it('should throw when passing in something that is missing a connection point', () => {
807+
strategy.withPositions([{originY: 'top', overlayX: 'start', overlayY: 'top'} as any]);
808+
expect(() => strategy.apply()).toThrow();
809+
});
810+
811+
it('should throw when passing in something that has an invalid X position', () => {
812+
strategy.withPositions([{
813+
originX: 'left',
814+
originY: 'top',
815+
overlayX: 'left',
816+
overlayY: 'top'
817+
} as any]);
818+
819+
expect(() => strategy.apply()).toThrow();
820+
});
821+
822+
it('should throw when passing in something that has an invalid Y position', () => {
823+
strategy.withPositions([{
824+
originX: 'start',
825+
originY: 'middle',
826+
overlayX: 'start',
827+
overlayY: 'middle'
828+
} as any]);
829+
830+
expect(() => strategy.apply()).toThrow();
831+
});
832+
});
833+
780834
});
781835

782836
/** Creates an absolutely positioned, display: block element with a default size. */

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
OverlayConnectionPosition,
1616
ConnectedOverlayPositionChange,
1717
ScrollingVisibility,
18+
validateHorizontalPosition,
19+
validateVerticalPosition,
1820
} from './connected-position';
1921
import {Subject} from 'rxjs/Subject';
2022
import {Subscription} from 'rxjs/Subscription';
@@ -130,6 +132,7 @@ export class ConnectedPositionStrategy implements PositionStrategy {
130132
return;
131133
}
132134

135+
this._validatePositions();
133136
this._applied = true;
134137

135138
// 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 {
183186
return;
184187
}
185188

189+
this._validatePositions();
190+
186191
const originRect = this._origin.getBoundingClientRect();
187192
const overlayRect = this._pane.getBoundingClientRect();
188193
const viewportSize = this._viewportRuler.getViewportSize();
@@ -428,14 +433,28 @@ export class ConnectedPositionStrategy implements PositionStrategy {
428433
this._onPositionChange.next(positionChange);
429434
}
430435

431-
/**
432-
* Subtracts the amount that an element is overflowing on an axis from it's length.
433-
*/
436+
/** Subtracts the amount that an element is overflowing on an axis from it's length. */
434437
private _subtractOverflows(length: number, ...overflows: number[]): number {
435438
return overflows.reduce((currentValue: number, currentOverflow: number) => {
436439
return currentValue - Math.max(currentOverflow, 0);
437440
}, length);
438441
}
442+
443+
/** Validates that the current position match the expected values. */
444+
private _validatePositions(): void {
445+
if (!this._preferredPositions.length) {
446+
throw Error('ConnectedPositionStrategy: At least one position is required.');
447+
}
448+
449+
// TODO(crisbeto): remove these once Angular's template type
450+
// checking is advanced enough to catch these cases.
451+
this._preferredPositions.forEach(pair => {
452+
validateHorizontalPosition('originX', pair.originX);
453+
validateVerticalPosition('originY', pair.originY);
454+
validateHorizontalPosition('overlayX', pair.overlayX);
455+
validateVerticalPosition('overlayY', pair.overlayY);
456+
});
457+
}
439458
}
440459

441460
/** A simple (x, y) coordinate. */

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,29 @@ export class ConnectedOverlayPositionChange {
9090
/** @docs-private */
9191
@Optional() public scrollableViewProperties: ScrollingVisibility) {}
9292
}
93+
94+
/**
95+
* Validates whether a vertical position property matches the expected values.
96+
* @param property Name of the property being validated.
97+
* @param value Value of the property being validated.
98+
* @docs-private
99+
*/
100+
export function validateVerticalPosition(property: string, value: VerticalConnectionPos) {
101+
if (value !== 'top' && value !== 'bottom' && value !== 'center') {
102+
throw Error(`ConnectedPosition: Invalid ${property} "${value}". ` +
103+
`Expected "top", "bottom" or "center".`);
104+
}
105+
}
106+
107+
/**
108+
* Validates whether a horizontal position property matches the expected values.
109+
* @param property Name of the property being validated.
110+
* @param value Value of the property being validated.
111+
* @docs-private
112+
*/
113+
export function validateHorizontalPosition(property: string, value: HorizontalConnectionPos) {
114+
if (value !== 'start' && value !== 'end' && value !== 'center') {
115+
throw Error(`ConnectedPosition: Invalid ${property} "${value}". ` +
116+
`Expected "start", "end" or "center".`);
117+
}
118+
}

0 commit comments

Comments
 (0)