Skip to content

Commit f2a565b

Browse files
committed
fix(overlay): re-use last preferred position when re-applying to open connected overlay
Currently when updating the position of an open connected overlay (e.g. when the user is scrolling) we go through the same process for determining the preferred position as when the overlay was attached. This means that the preferred position could change, causing the overlay to jump. With these changes the overlay will determine its preferred position when it's attached and the re-use the same position until it's detached. This is a prerequisite to moving the scroll clipping logic out of the `ConnectedPositionStrategy` and into the scroll strategies. This PR is a resubmit of #5471.
1 parent 9673f63 commit f2a565b

File tree

2 files changed

+37
-4
lines changed

2 files changed

+37
-4
lines changed

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,28 @@ describe('ConnectedPositionStrategy', () => {
438438
expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left));
439439
});
440440

441+
it('should re-use the preferred position when re-applying while the overlay is open', () => {
442+
positionBuilder = new OverlayPositionBuilder(viewportRuler);
443+
strategy = positionBuilder.connectedTo(
444+
fakeElementRef,
445+
{originX: 'end', originY: 'center'},
446+
{overlayX: 'start', overlayY: 'center'})
447+
.withFallbackPosition(
448+
{originX: 'start', originY: 'bottom'},
449+
{overlayX: 'end', overlayY: 'top'});
450+
451+
const recalcSpy = spyOn(strategy, 'recalculateLastPosition');
452+
453+
strategy.attach(fakeOverlayRef(overlayElement));
454+
strategy.apply();
455+
456+
expect(recalcSpy).not.toHaveBeenCalled();
457+
458+
strategy.apply();
459+
460+
expect(recalcSpy).toHaveBeenCalled();
461+
});
462+
441463
/**
442464
* Run all tests for connecting the overlay to the origin such that first preferred
443465
* position does not go off-screen. We do this because there are several cases where we

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,10 @@ export class ConnectedPositionStrategy implements PositionStrategy {
6868
/** The last position to have been calculated as the best fit position. */
6969
private _lastConnectedPosition: ConnectionPositionPair;
7070

71-
_onPositionChange:
72-
Subject<ConnectedOverlayPositionChange> = new Subject<ConnectedOverlayPositionChange>();
71+
/** Whether the position strategy is currently applied. */
72+
private _applied = false;
73+
74+
_onPositionChange = new Subject<ConnectedOverlayPositionChange>();
7375

7476
/** Emits an event when the connection point changes. */
7577
get onPositionChange(): Observable<ConnectedOverlayPositionChange> {
@@ -99,22 +101,31 @@ export class ConnectedPositionStrategy implements PositionStrategy {
99101

100102
/** Performs any cleanup after the element is destroyed. */
101103
dispose() {
104+
this._applied = false;
102105
this._resizeSubscription.unsubscribe();
103106
}
104107

105108
/** @docs-private */
106109
detach() {
110+
this._applied = false;
107111
this._resizeSubscription.unsubscribe();
108112
}
109113

110114
/**
111115
* Updates the position of the overlay element, using whichever preferred position relative
112116
* to the origin fits on-screen.
113117
* @docs-private
114-
*
115-
* @returns Resolves when the styles have been applied.
116118
*/
117119
apply(): void {
120+
// If the position has been applied already (e.g. when the overlay was opened), re-use the
121+
// old position, in order to prevent the overlay from jumping around.
122+
if (this._applied && this._lastConnectedPosition) {
123+
this.recalculateLastPosition();
124+
return;
125+
}
126+
127+
this._applied = true;
128+
118129
// We need the bounding rects for the origin and the overlay to determine how to position
119130
// the overlay relative to the origin.
120131
const element = this._pane;

0 commit comments

Comments
 (0)