Skip to content

Commit 84f6fb1

Browse files
crisbetoandrewseguin
authored andcommitted
fix(overlay): clear last calculated position when new set of positions is provided
In the `FlexibleConnectedPositionStrategy` once we apply a position, we save a reference to it in order to be able to reuse it if the consumer wants to recalculate the overlay dimensions, however that cached position isn't being cleared. This means that if consumer were to provide a new set of positions and tried to re-apply the last calculated position, the cached position may no longer be a part of the list of preferred positions. These changes will clear the last position if it's not a part of the preferred positions anymore. Relates to #10457.
1 parent 89ea485 commit 84f6fb1

5 files changed

+51
-4
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {FlexibleConnectedPositionStrategy} from './flexible-connected-position-s
3131
* a point on the origin element that is connected to a point on the overlay element. For example,
3232
* a basic dropdown is connecting the bottom-left corner of the origin to the top-left corner
3333
* of the overlay.
34-
* @deprecated
34+
* @deprecated Use `FlexibleConnectedPositionStrategy` instead.
3535
* @deletion-target 7.0.0
3636
*/
3737
export class ConnectedPositionStrategy implements PositionStrategy {

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,45 @@ describe('FlexibleConnectedPositionStrategy', () => {
633633
expect(recalcSpy).toHaveBeenCalled();
634634
});
635635

636+
it('should not retain the last preferred position when overriding the positions', () => {
637+
originElement.style.top = '100px';
638+
originElement.style.left = '100px';
639+
640+
const originRect = originElement.getBoundingClientRect();
641+
642+
positionStrategy.withPositions([{
643+
originX: 'start',
644+
originY: 'top',
645+
overlayX: 'start',
646+
overlayY: 'top',
647+
offsetX: 10,
648+
offsetY: 20
649+
}]);
650+
651+
attachOverlay({positionStrategy});
652+
653+
let overlayRect = overlayRef.overlayElement.getBoundingClientRect();
654+
655+
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top) + 20);
656+
expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left) + 10);
657+
658+
positionStrategy.withPositions([{
659+
originX: 'start',
660+
originY: 'top',
661+
overlayX: 'start',
662+
overlayY: 'top',
663+
offsetX: 20,
664+
offsetY: 40
665+
}]);
666+
667+
positionStrategy.reapplyLastPosition();
668+
669+
overlayRect = overlayRef.overlayElement.getBoundingClientRect();
670+
671+
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.top) + 40);
672+
expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left) + 20);
673+
});
674+
636675
/**
637676
* Run all tests for connecting the overlay to the origin such that first preferred
638677
* position does not go off-screen. We do this because there are several cases where we

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
9595
private _boundingBox: HTMLElement | null;
9696

9797
/** The last position to have been calculated as the best fit position. */
98-
private _lastPosition: ConnectedPosition;
98+
private _lastPosition: ConnectedPosition | null;
9999

100100
/** Subject that emits whenever the position changes. */
101101
private _positionChanges = new Subject<ConnectedOverlayPositionChange>();
@@ -305,6 +305,13 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
305305
*/
306306
withPositions(positions: ConnectedPosition[]): this {
307307
this._preferredPositions = positions;
308+
309+
// If the last calculated position object isn't part of the positions anymore, clear
310+
// it in order to avoid it being picked up if the consumer tries to re-apply.
311+
if (positions.indexOf(this._lastPosition!) === -1) {
312+
this._lastPosition = null;
313+
}
314+
308315
return this;
309316
}
310317

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ export class OverlayPositionBuilder {
3434
* @param elementRef
3535
* @param originPos
3636
* @param overlayPos
37+
* @deprecated Use `flexibleConnectedTo` instead.
38+
* @deletion-target 7.0.0
3739
*/
3840
connectedTo(
3941
elementRef: ElementRef,

src/lib/select/select.html

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,10 @@
1515
<div class="mat-select-arrow-wrapper"><div class="mat-select-arrow"></div></div>
1616
</div>
1717

18-
<!-- TODO(crisbeto): re-enable locked position after flexible positioning gets in. -->
1918
<ng-template
2019
cdk-connected-overlay
21-
cdkConnectedOverlayHasBackdrop
2220
cdkConnectedOverlayLockPosition
21+
cdkConnectedOverlayHasBackdrop
2322
cdkConnectedOverlayBackdropClass="cdk-overlay-transparent-backdrop"
2423
[cdkConnectedOverlayScrollStrategy]="_scrollStrategy"
2524
[cdkConnectedOverlayOrigin]="origin"

0 commit comments

Comments
 (0)