Skip to content

Commit 1f14b04

Browse files
committed
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`.
1 parent bcff93e commit 1f14b04

File tree

4 files changed

+68
-78
lines changed

4 files changed

+68
-78
lines changed

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

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ const DEFAULT_WIDTH = 60;
2727
describe('ConnectedPositionStrategy', () => {
2828
let overlay: Overlay;
2929
let overlayContainer: OverlayContainer;
30-
let overlayContainerElement: HTMLElement;
3130
let zone: MockNgZone;
3231
let overlayRef: OverlayRef;
3332

@@ -40,7 +39,6 @@ describe('ConnectedPositionStrategy', () => {
4039
inject([Overlay, OverlayContainer], (o: Overlay, oc: OverlayContainer) => {
4140
overlay = o;
4241
overlayContainer = oc;
43-
overlayContainerElement = oc.getContainerElement();
4442
})();
4543
});
4644

@@ -776,61 +774,6 @@ describe('ConnectedPositionStrategy', () => {
776774

777775
});
778776

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

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

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

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ import {
1515
OriginConnectionPosition,
1616
OverlayConnectionPosition,
1717
ConnectedOverlayPositionChange,
18-
validateHorizontalPosition,
19-
validateVerticalPosition,
2018
} from './connected-position';
2119
import {Observable} from 'rxjs';
2220
import {CdkScrollable} from '@angular/cdk/scrolling';
@@ -112,7 +110,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
112110
* @docs-private
113111
*/
114112
apply(): void {
115-
this._validatePositions();
116113
this._positionStrategy.apply();
117114
}
118115

@@ -122,7 +119,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
122119
* allows one to re-align the panel without changing the orientation of the panel.
123120
*/
124121
recalculateLastPosition(): void {
125-
this._validatePositions();
126122
this._positionStrategy.reapplyLastPosition();
127123
}
128124

@@ -216,21 +212,4 @@ export class ConnectedPositionStrategy implements PositionStrategy {
216212
this._positionStrategy.setOrigin(origin);
217213
return this;
218214
}
219-
220-
/** Validates that the current position match the expected values. */
221-
private _validatePositions(): void {
222-
if (!this._preferredPositions.length) {
223-
throw Error('ConnectedPositionStrategy: At least one position is required.');
224-
}
225-
226-
// TODO(crisbeto): remove these once Angular's template type
227-
// checking is advanced enough to catch these cases.
228-
// TODO(crisbeto): port these checks into the flexible positioning.
229-
this._preferredPositions.forEach(pair => {
230-
validateHorizontalPosition('originX', pair.originX);
231-
validateVerticalPosition('originY', pair.originY);
232-
validateHorizontalPosition('overlayX', pair.overlayX);
233-
validateVerticalPosition('overlayY', pair.overlayY);
234-
});
235-
}
236215
}

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,6 +1561,53 @@ describe('FlexibleConnectedPositionStrategy', () => {
15611561

15621562
});
15631563

1564+
describe('validations', () => {
1565+
let originElement: HTMLElement;
1566+
let positionStrategy: FlexibleConnectedPositionStrategy;
1567+
1568+
beforeEach(() => {
1569+
originElement = createPositionedBlockElement();
1570+
document.body.appendChild(originElement);
1571+
positionStrategy = overlay.position().flexibleConnectedTo(new ElementRef(originElement));
1572+
});
1573+
1574+
afterEach(() => {
1575+
positionStrategy.dispose();
1576+
});
1577+
1578+
it('should throw when attaching without any positions', () => {
1579+
positionStrategy.withPositions([]);
1580+
expect(() => attachOverlay({positionStrategy})).toThrow();
1581+
});
1582+
1583+
it('should throw when passing in something that is missing a connection point', () => {
1584+
positionStrategy.withPositions([{originY: 'top', overlayX: 'start', overlayY: 'top'} as any]);
1585+
expect(() => attachOverlay({positionStrategy})).toThrow();
1586+
});
1587+
1588+
it('should throw when passing in something that has an invalid X position', () => {
1589+
positionStrategy.withPositions([{
1590+
originX: 'left',
1591+
originY: 'top',
1592+
overlayX: 'left',
1593+
overlayY: 'top'
1594+
} as any]);
1595+
1596+
expect(() => attachOverlay({positionStrategy})).toThrow();
1597+
});
1598+
1599+
it('should throw when passing in something that has an invalid Y position', () => {
1600+
positionStrategy.withPositions([{
1601+
originX: 'start',
1602+
originY: 'middle',
1603+
overlayX: 'start',
1604+
overlayY: 'middle'
1605+
} as any]);
1606+
1607+
expect(() => attachOverlay({positionStrategy})).toThrow();
1608+
});
1609+
});
1610+
15641611
});
15651612

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

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
ConnectedOverlayPositionChange,
1414
ConnectionPositionPair,
1515
ScrollingVisibility,
16+
validateHorizontalPosition,
17+
validateVerticalPosition,
1618
} from './connected-position';
1719
import {Observable, Subscription, Subject} from 'rxjs';
1820
import {OverlayRef} from '../overlay-ref';
@@ -158,6 +160,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
158160
return;
159161
}
160162

163+
this._validatePositions();
164+
161165
// If the position has been applied already (e.g. when the overlay was opened) and the
162166
// consumer opted into locking in the position, re-use the old position, in order to
163167
// prevent the overlay from jumping around.
@@ -283,6 +287,7 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
283287
*/
284288
reapplyLastPosition(): void {
285289
if (!this._isDisposed) {
290+
this._validatePositions();
286291
this._originRect = this._origin.getBoundingClientRect();
287292
this._overlayRect = this._pane.getBoundingClientRect();
288293
this._viewportRect = this._getNarrowedViewportRect();
@@ -908,6 +913,22 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
908913

909914
return position.offsetY == null ? this._offsetY : position.offsetY;
910915
}
916+
917+
/** Validates that the current position match the expected values. */
918+
private _validatePositions(): void {
919+
if (!this._preferredPositions.length) {
920+
throw Error('FlexibleConnectedPositionStrategy: At least one position is required.');
921+
}
922+
923+
// TODO(crisbeto): remove these once Angular's template type
924+
// checking is advanced enough to catch these cases.
925+
this._preferredPositions.forEach(pair => {
926+
validateHorizontalPosition('originX', pair.originX);
927+
validateVerticalPosition('originY', pair.originY);
928+
validateHorizontalPosition('overlayX', pair.overlayX);
929+
validateVerticalPosition('overlayY', pair.overlayY);
930+
});
931+
}
911932
}
912933

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

0 commit comments

Comments
 (0)