Skip to content

Commit 800a924

Browse files
committed
fix(overlay): flexible overlay with push not handling scroll offset and position locking
* Fixes the position of flexible overlays with pushing enabled being thrown off once the user starts scrolling. * Fixes flexible overlays with pushing not handling locked positioning. With these changes locked overlays will only be pushed when they're opened, whereas non-locked overlays will stay in the viewport, even when the user scrolls down. * Fixes a potential issue due to a couple of variables being initialized together where one is set to zero and the other one is undefined. Fixes #11365.
1 parent abc3d38 commit 800a924

File tree

4 files changed

+149
-17
lines changed

4 files changed

+149
-17
lines changed

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,105 @@ describe('FlexibleConnectedPositionStrategy', () => {
11011101
expect(Math.floor(overlayRect.top)).toBe(15);
11021102
});
11031103

1104+
it('should not mess with the left offset when pushing from the top', () => {
1105+
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
1106+
originElement.style.left = '200px';
1107+
1108+
positionStrategy.withPositions([{
1109+
originX: 'start',
1110+
originY: 'bottom',
1111+
overlayX: 'start',
1112+
overlayY: 'top'
1113+
}]);
1114+
1115+
attachOverlay({positionStrategy});
1116+
1117+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1118+
expect(Math.floor(overlayRect.left)).toBe(200);
1119+
});
1120+
1121+
it('should keep the element inside the viewport as the user is scrolling, ' +
1122+
'with position locking disabled', () => {
1123+
const veryLargeElement = document.createElement('div');
1124+
1125+
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
1126+
originElement.style.left = '200px';
1127+
1128+
veryLargeElement.style.width = '100%';
1129+
veryLargeElement.style.height = '2000px';
1130+
document.body.appendChild(veryLargeElement);
1131+
1132+
positionStrategy
1133+
.withLockedPosition(false)
1134+
.withViewportMargin(0)
1135+
.withPositions([{
1136+
overlayY: 'top',
1137+
overlayX: 'start',
1138+
originY: 'top',
1139+
originX: 'start'
1140+
}]);
1141+
1142+
attachOverlay({positionStrategy});
1143+
1144+
let overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1145+
expect(Math.floor(overlayRect.top))
1146+
.toBe(0, 'Expected overlay to be in the viewport initially.');
1147+
1148+
window.scroll(0, 100);
1149+
overlayRef.updatePosition();
1150+
zone.simulateZoneExit();
1151+
1152+
overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1153+
expect(Math.floor(overlayRect.top))
1154+
.toBe(0, 'Expected overlay to stay in the viewport after scrolling.');
1155+
1156+
window.scroll(0, 0);
1157+
document.body.removeChild(veryLargeElement);
1158+
});
1159+
1160+
it('should not continue pushing the overlay as the user scrolls, if position ' +
1161+
'locking is enabled', () => {
1162+
const veryLargeElement = document.createElement('div');
1163+
1164+
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
1165+
originElement.style.left = '200px';
1166+
1167+
veryLargeElement.style.width = '100%';
1168+
veryLargeElement.style.height = '2000px';
1169+
document.body.appendChild(veryLargeElement);
1170+
1171+
positionStrategy
1172+
.withLockedPosition()
1173+
.withViewportMargin(0)
1174+
.withPositions([{
1175+
overlayY: 'top',
1176+
overlayX: 'start',
1177+
originY: 'top',
1178+
originX: 'start'
1179+
}]);
1180+
1181+
attachOverlay({positionStrategy});
1182+
1183+
const scrollBy = 100;
1184+
let initialOverlayTop = Math.floor(overlayRef.overlayElement.getBoundingClientRect().top);
1185+
1186+
expect(initialOverlayTop).toBe(0, 'Expected overlay to be inside the viewport initially.');
1187+
1188+
window.scroll(0, scrollBy);
1189+
overlayRef.updatePosition();
1190+
zone.simulateZoneExit();
1191+
1192+
let currentOverlayTop = Math.floor(overlayRef.overlayElement.getBoundingClientRect().top);
1193+
1194+
expect(currentOverlayTop).toBeLessThan(0,
1195+
'Expected overlay to no longer be completely inside the viewport.');
1196+
expect(currentOverlayTop).toBe(initialOverlayTop - scrollBy,
1197+
'Expected overlay to maintain its previous position.');
1198+
1199+
window.scroll(0, 0);
1200+
document.body.removeChild(veryLargeElement);
1201+
});
1202+
11041203
});
11051204

11061205
describe('with flexible dimensions', () => {

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

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {PositionStrategy} from './position-strategy';
1010
import {ElementRef} from '@angular/core';
11-
import {ViewportRuler, CdkScrollable} from '@angular/cdk/scrolling';
11+
import {ViewportRuler, CdkScrollable, ViewportScrollPosition} from '@angular/cdk/scrolling';
1212
import {
1313
ConnectedOverlayPositionChange,
1414
ConnectionPositionPair,
@@ -108,6 +108,9 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
108108
/** Selector to be used when finding the elements on which to set the transform origin. */
109109
private _transformOriginSelector: string;
110110

111+
/** Amount by which the overlay was pushed in each axis during the last time it was positioned. */
112+
private _previousPushAmount: {x: number, y: number} | null;
113+
111114
/** Observable sequence of position changes. */
112115
positionChanges: Observable<ConnectedOverlayPositionChange> =
113116
this._positionChanges.asObservable();
@@ -269,6 +272,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
269272
}
270273

271274
detach() {
275+
this._lastPosition = null;
276+
this._previousPushAmount = null;
272277
this._resizeSubscription.unsubscribe();
273278
}
274279

@@ -528,23 +533,37 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
528533
* the viewport, the top-left corner will be pushed on-screen (with overflow occuring on the
529534
* right and bottom).
530535
*
531-
* @param start The starting point from which the overlay is pushed.
532-
* @param overlay The overlay dimensions.
536+
* @param start Starting point from which the overlay is pushed.
537+
* @param overlay Dimensions of the overlay.
538+
* @param scrollPosition Current viewport scroll position.
533539
* @returns The point at which to position the overlay after pushing. This is effectively a new
534540
* originPoint.
535541
*/
536-
private _pushOverlayOnScreen(start: Point, overlay: ClientRect): Point {
542+
private _pushOverlayOnScreen(start: Point,
543+
overlay: ClientRect,
544+
scrollPosition: ViewportScrollPosition): Point {
545+
// If the position is locked and we've pushed the overlay already, reuse the previous push
546+
// amount, rather than pushing it again. If we were to continue pushing, the element would
547+
// remain in the viewport, which goes against the expectations when position locking is enabled.
548+
if (this._previousPushAmount && this._positionLocked) {
549+
return {
550+
x: start.x + this._previousPushAmount.x,
551+
y: start.y + this._previousPushAmount.y
552+
};
553+
}
554+
537555
const viewport = this._viewportRect;
538556

539-
// Determine how much the overlay goes outside the viewport on each side, which we'll use to
540-
// decide which direction to push it.
557+
// Determine how much the overlay goes outside the viewport on each
558+
// side, which we'll use to decide which direction to push it.
541559
const overflowRight = Math.max(start.x + overlay.width - viewport.right, 0);
542560
const overflowBottom = Math.max(start.y + overlay.height - viewport.bottom, 0);
543-
const overflowTop = Math.max(viewport.top - start.y, 0);
544-
const overflowLeft = Math.max(viewport.left - start.x, 0);
561+
const overflowTop = Math.max(viewport.top - scrollPosition.top - start.y, 0);
562+
const overflowLeft = Math.max(viewport.left - scrollPosition.left - start.x, 0);
545563

546-
// Amount by which to push the overlay in each direction such that it remains on-screen.
547-
let pushX, pushY = 0;
564+
// Amount by which to push the overlay in each axis such that it remains on-screen.
565+
let pushX = 0;
566+
let pushY = 0;
548567

549568
// If the overlay fits completely within the bounds of the viewport, push it from whichever
550569
// direction is goes off-screen. Otherwise, push the top-left corner such that its in the
@@ -561,6 +580,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
561580
pushY = viewport.top - start.y;
562581
}
563582

583+
this._previousPushAmount = {x: pushX, y: pushY};
584+
564585
return {
565586
x: start.x + pushX,
566587
y: start.y + pushY,
@@ -776,8 +797,9 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
776797
const styles = {} as CSSStyleDeclaration;
777798

778799
if (this._hasExactPosition()) {
779-
extendStyles(styles, this._getExactOverlayY(position, originPoint));
780-
extendStyles(styles, this._getExactOverlayX(position, originPoint));
800+
const scrollPosition = this._viewportRuler.getViewportScrollPosition();
801+
extendStyles(styles, this._getExactOverlayY(position, originPoint, scrollPosition));
802+
extendStyles(styles, this._getExactOverlayX(position, originPoint, scrollPosition));
781803
} else {
782804
styles.position = 'static';
783805
}
@@ -816,14 +838,16 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
816838
}
817839

818840
/** Gets the exact top/bottom for the overlay when not using flexible sizing or when pushing. */
819-
private _getExactOverlayY(position: ConnectedPosition, originPoint: Point) {
841+
private _getExactOverlayY(position: ConnectedPosition,
842+
originPoint: Point,
843+
scrollPosition: ViewportScrollPosition) {
820844
// Reset any existing styles. This is necessary in case the
821845
// preferred position has changed since the last `apply`.
822846
let styles = {top: null, bottom: null} as CSSStyleDeclaration;
823847
let overlayPoint = this._getOverlayPoint(originPoint, this._overlayRect, position);
824848

825849
if (this._isPushed) {
826-
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect);
850+
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect, scrollPosition);
827851
}
828852

829853
// We want to set either `top` or `bottom` based on whether the overlay wants to appear
@@ -841,14 +865,16 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
841865
}
842866

843867
/** Gets the exact left/right for the overlay when not using flexible sizing or when pushing. */
844-
private _getExactOverlayX(position: ConnectedPosition, originPoint: Point) {
868+
private _getExactOverlayX(position: ConnectedPosition,
869+
originPoint: Point,
870+
scrollPosition: ViewportScrollPosition) {
845871
// Reset any existing styles. This is necessary in case the preferred position has
846872
// changed since the last `apply`.
847873
let styles = {left: null, right: null} as CSSStyleDeclaration;
848874
let overlayPoint = this._getOverlayPoint(originPoint, this._overlayRect, position);
849875

850876
if (this._isPushed) {
851-
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect);
877+
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect, scrollPosition);
852878
}
853879

854880
// We want to set either `left` or `right` based on whether the overlay wants to appear "before"

src/cdk/scrolling/viewport-ruler.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ import {auditTime} from 'rxjs/operators';
1414
/** Time in ms to throttle the resize events by default. */
1515
export const DEFAULT_RESIZE_TIME = 20;
1616

17+
/** Object that holds the scroll position of the viewport in each direction. */
18+
export interface ViewportScrollPosition {
19+
top: number;
20+
left: number;
21+
}
22+
1723
/**
1824
* Simple utility for getting the bounds of the browser viewport.
1925
* @docs-private
@@ -82,7 +88,7 @@ export class ViewportRuler implements OnDestroy {
8288
}
8389

8490
/** Gets the (top, left) scroll position of the viewport. */
85-
getViewportScrollPosition() {
91+
getViewportScrollPosition(): ViewportScrollPosition {
8692
// While we can get a reference to the fake document
8793
// during SSR, it doesn't have getBoundingClientRect.
8894
if (!this._platform.isBrowser) {

src/lib/menu/menu-trigger.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
395395
return this._overlay.position()
396396
.flexibleConnectedTo(this._element)
397397
.withTransformOriginOn('.mat-menu-panel')
398+
.withLockedPosition()
398399
.withPositions([
399400
{originX, originY, overlayX, overlayY, offsetY},
400401
{originX: originFallbackX, originY, overlayX: overlayFallbackX, overlayY, offsetY},

0 commit comments

Comments
 (0)