Skip to content

Commit 2046b7a

Browse files
crisbetoandrewseguin
authored andcommitted
refactor(overlay): move connected position assertions into FlexibleConnectedPositionStrategy (#10464)
Since the `ConnectedPositionStrategy` proxies everything to `FlexibleConnectedPositionStrategy`, these changes move the assertions that verify that the config looks correctly into the `FlexibleConnectedPositionStrategy`.
1 parent 6df3709 commit 2046b7a

File tree

4 files changed

+73
-78
lines changed

4 files changed

+73
-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
@@ -26,7 +26,6 @@ const DEFAULT_WIDTH = 60;
2626
describe('ConnectedPositionStrategy', () => {
2727
let overlay: Overlay;
2828
let overlayContainer: OverlayContainer;
29-
let overlayContainerElement: HTMLElement;
3029
let zone: MockNgZone;
3130
let overlayRef: OverlayRef;
3231

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

@@ -775,61 +773,6 @@ describe('ConnectedPositionStrategy', () => {
775773

776774
});
777775

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

835778
/** 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
@@ -16,8 +16,6 @@ import {
1616
ConnectionPositionPair,
1717
OriginConnectionPosition,
1818
OverlayConnectionPosition,
19-
validateHorizontalPosition,
20-
validateVerticalPosition,
2119
} from './connected-position';
2220
import {FlexibleConnectedPositionStrategy} from './flexible-connected-position-strategy';
2321
import {PositionStrategy} from './position-strategy';
@@ -109,7 +107,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
109107
* @docs-private
110108
*/
111109
apply(): void {
112-
this._validatePositions();
113110
this._positionStrategy.apply();
114111
}
115112

@@ -119,7 +116,6 @@ export class ConnectedPositionStrategy implements PositionStrategy {
119116
* allows one to re-align the panel without changing the orientation of the panel.
120117
*/
121118
recalculateLastPosition(): void {
122-
this._validatePositions();
123119
this._positionStrategy.reapplyLastPosition();
124120
}
125121

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

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1593,6 +1593,57 @@ describe('FlexibleConnectedPositionStrategy', () => {
15931593

15941594
});
15951595

1596+
describe('validations', () => {
1597+
let originElement: HTMLElement;
1598+
let positionStrategy: FlexibleConnectedPositionStrategy;
1599+
1600+
beforeEach(() => {
1601+
originElement = createPositionedBlockElement();
1602+
document.body.appendChild(originElement);
1603+
positionStrategy = overlay.position().flexibleConnectedTo(new ElementRef(originElement));
1604+
});
1605+
1606+
afterEach(() => {
1607+
positionStrategy.dispose();
1608+
});
1609+
1610+
it('should throw when attaching without any positions', () => {
1611+
expect(() => positionStrategy.withPositions([])).toThrow();
1612+
});
1613+
1614+
it('should throw when passing in something that is missing a connection point', () => {
1615+
expect(() => {
1616+
positionStrategy.withPositions([{
1617+
originY: 'top',
1618+
overlayX: 'start',
1619+
overlayY: 'top'
1620+
} as any]);
1621+
}).toThrow();
1622+
});
1623+
1624+
it('should throw when passing in something that has an invalid X position', () => {
1625+
expect(() => {
1626+
positionStrategy.withPositions([{
1627+
originX: 'left',
1628+
originY: 'top',
1629+
overlayX: 'left',
1630+
overlayY: 'top'
1631+
} as any]);
1632+
}).toThrow();
1633+
});
1634+
1635+
it('should throw when passing in something that has an invalid Y position', () => {
1636+
expect(() => {
1637+
positionStrategy.withPositions([{
1638+
originX: 'start',
1639+
originY: 'middle',
1640+
overlayX: 'start',
1641+
overlayY: 'middle'
1642+
} as any]);
1643+
}).toThrow();
1644+
});
1645+
});
1646+
15961647
});
15971648

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

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

Lines changed: 22 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';
@@ -127,6 +129,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
127129
throw Error('This position strategy is already attached to an overlay');
128130
}
129131

132+
this._validatePositions();
133+
130134
overlayRef.hostElement.classList.add('cdk-overlay-connected-position-bounding-box');
131135

132136
this._overlayRef = overlayRef;
@@ -315,6 +319,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
315319
this._lastPosition = null;
316320
}
317321

322+
this._validatePositions();
323+
318324
return this;
319325
}
320326

@@ -901,6 +907,22 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
901907

902908
return position.offsetY == null ? this._offsetY : position.offsetY;
903909
}
910+
911+
/** Validates that the current position match the expected values. */
912+
private _validatePositions(): void {
913+
if (!this._preferredPositions.length) {
914+
throw Error('FlexibleConnectedPositionStrategy: At least one position is required.');
915+
}
916+
917+
// TODO(crisbeto): remove these once Angular's template type
918+
// checking is advanced enough to catch these cases.
919+
this._preferredPositions.forEach(pair => {
920+
validateHorizontalPosition('originX', pair.originX);
921+
validateVerticalPosition('originY', pair.originY);
922+
validateHorizontalPosition('overlayX', pair.overlayX);
923+
validateVerticalPosition('overlayY', pair.overlayY);
924+
});
925+
}
904926
}
905927

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

0 commit comments

Comments
 (0)