Skip to content

Commit f5408a7

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 dad0ed0 commit f5408a7

File tree

4 files changed

+220
-22
lines changed

4 files changed

+220
-22
lines changed

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

+159
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,165 @@ describe('FlexibleConnectedPositionStrategy', () => {
11341134
expect(Math.floor(overlayRect.top)).toBe(15);
11351135
});
11361136

1137+
it('should not mess with the left offset when pushing from the top', () => {
1138+
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
1139+
originElement.style.left = '200px';
1140+
1141+
positionStrategy.withPositions([{
1142+
originX: 'start',
1143+
originY: 'bottom',
1144+
overlayX: 'start',
1145+
overlayY: 'top'
1146+
}]);
1147+
1148+
attachOverlay({positionStrategy});
1149+
1150+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1151+
expect(Math.floor(overlayRect.left)).toBe(200);
1152+
});
1153+
1154+
it('should align to the trigger if the overlay is wider than the viewport, but the trigger ' +
1155+
'is still within the viewport', () => {
1156+
originElement.style.top = '200px';
1157+
originElement.style.left = '150px';
1158+
1159+
positionStrategy.withPositions([
1160+
{
1161+
originX: 'start',
1162+
originY: 'bottom',
1163+
overlayX: 'start',
1164+
overlayY: 'top'
1165+
},
1166+
{
1167+
originX: 'end',
1168+
originY: 'bottom',
1169+
overlayX: 'end',
1170+
overlayY: 'top'
1171+
}
1172+
]);
1173+
1174+
attachOverlay({
1175+
width: viewport.getViewportRect().width + 100,
1176+
positionStrategy
1177+
});
1178+
1179+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1180+
const originRect = originElement.getBoundingClientRect();
1181+
1182+
expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left));
1183+
});
1184+
1185+
it('should push into the viewport if the overlay is wider than the viewport and the trigger' +
1186+
'out of the viewport', () => {
1187+
originElement.style.top = '200px';
1188+
originElement.style.left = `-${DEFAULT_WIDTH / 2}px`;
1189+
1190+
positionStrategy.withPositions([
1191+
{
1192+
originX: 'start',
1193+
originY: 'bottom',
1194+
overlayX: 'start',
1195+
overlayY: 'top'
1196+
},
1197+
{
1198+
originX: 'end',
1199+
originY: 'bottom',
1200+
overlayX: 'end',
1201+
overlayY: 'top'
1202+
}
1203+
]);
1204+
1205+
attachOverlay({
1206+
width: viewport.getViewportRect().width + 100,
1207+
positionStrategy
1208+
});
1209+
1210+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1211+
expect(Math.floor(overlayRect.left)).toBe(0);
1212+
});
1213+
1214+
it('should keep the element inside the viewport as the user is scrolling, ' +
1215+
'with position locking disabled', () => {
1216+
const veryLargeElement = document.createElement('div');
1217+
1218+
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
1219+
originElement.style.left = '200px';
1220+
1221+
veryLargeElement.style.width = '100%';
1222+
veryLargeElement.style.height = '2000px';
1223+
document.body.appendChild(veryLargeElement);
1224+
1225+
positionStrategy
1226+
.withLockedPosition(false)
1227+
.withViewportMargin(0)
1228+
.withPositions([{
1229+
overlayY: 'top',
1230+
overlayX: 'start',
1231+
originY: 'top',
1232+
originX: 'start'
1233+
}]);
1234+
1235+
attachOverlay({positionStrategy});
1236+
1237+
let overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1238+
expect(Math.floor(overlayRect.top))
1239+
.toBe(0, 'Expected overlay to be in the viewport initially.');
1240+
1241+
window.scroll(0, 100);
1242+
overlayRef.updatePosition();
1243+
zone.simulateZoneExit();
1244+
1245+
overlayRect = overlayRef.overlayElement.getBoundingClientRect();
1246+
expect(Math.floor(overlayRect.top))
1247+
.toBe(0, 'Expected overlay to stay in the viewport after scrolling.');
1248+
1249+
window.scroll(0, 0);
1250+
document.body.removeChild(veryLargeElement);
1251+
});
1252+
1253+
it('should not continue pushing the overlay as the user scrolls, if position ' +
1254+
'locking is enabled', () => {
1255+
const veryLargeElement = document.createElement('div');
1256+
1257+
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
1258+
originElement.style.left = '200px';
1259+
1260+
veryLargeElement.style.width = '100%';
1261+
veryLargeElement.style.height = '2000px';
1262+
document.body.appendChild(veryLargeElement);
1263+
1264+
positionStrategy
1265+
.withLockedPosition()
1266+
.withViewportMargin(0)
1267+
.withPositions([{
1268+
overlayY: 'top',
1269+
overlayX: 'start',
1270+
originY: 'top',
1271+
originX: 'start'
1272+
}]);
1273+
1274+
attachOverlay({positionStrategy});
1275+
1276+
const scrollBy = 100;
1277+
let initialOverlayTop = Math.floor(overlayRef.overlayElement.getBoundingClientRect().top);
1278+
1279+
expect(initialOverlayTop).toBe(0, 'Expected overlay to be inside the viewport initially.');
1280+
1281+
window.scroll(0, scrollBy);
1282+
overlayRef.updatePosition();
1283+
zone.simulateZoneExit();
1284+
1285+
let currentOverlayTop = Math.floor(overlayRef.overlayElement.getBoundingClientRect().top);
1286+
1287+
expect(currentOverlayTop).toBeLessThan(0,
1288+
'Expected overlay to no longer be completely inside the viewport.');
1289+
expect(currentOverlayTop).toBe(initialOverlayTop - scrollBy,
1290+
'Expected overlay to maintain its previous position.');
1291+
1292+
window.scroll(0, 0);
1293+
document.body.removeChild(veryLargeElement);
1294+
});
1295+
11371296
});
11381297

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

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

+53-21
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,
@@ -115,6 +115,9 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
115115
/** Keeps track of the CSS classes that the position strategy has applied on the overlay panel. */
116116
private _appliedPanelClasses: string[] = [];
117117

118+
/** Amount by which the overlay was pushed in each axis during the last time it was positioned. */
119+
private _previousPushAmount: {x: number, y: number} | null;
120+
118121
/** Observable sequence of position changes. */
119122
positionChanges: Observable<ConnectedOverlayPositionChange> = Observable.create(observer => {
120123
const subscription = this._positionChanges.subscribe(observer);
@@ -155,7 +158,13 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
155158
this._boundingBox = overlayRef.hostElement;
156159
this._pane = overlayRef.overlayElement;
157160
this._resizeSubscription.unsubscribe();
158-
this._resizeSubscription = this._viewportRuler.change().subscribe(() => this.apply());
161+
this._resizeSubscription = this._viewportRuler.change().subscribe(() => {
162+
// When the window is resized, we want to trigger the next reposition as if it
163+
// was an initial render, in order for the strategy to pick a new optimal position,
164+
// otherwise position locking will cause it to stay at the old one.
165+
this._isInitialRender = true;
166+
this.apply();
167+
});
159168
}
160169

161170
/**
@@ -287,6 +296,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
287296

288297
detach() {
289298
this._clearPanelClasses();
299+
this._lastPosition = null;
300+
this._previousPushAmount = null;
290301
this._resizeSubscription.unsubscribe();
291302
}
292303

@@ -546,39 +557,55 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
546557
* the viewport, the top-left corner will be pushed on-screen (with overflow occuring on the
547558
* right and bottom).
548559
*
549-
* @param start The starting point from which the overlay is pushed.
550-
* @param overlay The overlay dimensions.
560+
* @param start Starting point from which the overlay is pushed.
561+
* @param overlay Dimensions of the overlay.
562+
* @param scrollPosition Current viewport scroll position.
551563
* @returns The point at which to position the overlay after pushing. This is effectively a new
552564
* originPoint.
553565
*/
554-
private _pushOverlayOnScreen(start: Point, overlay: ClientRect): Point {
566+
private _pushOverlayOnScreen(start: Point,
567+
overlay: ClientRect,
568+
scrollPosition: ViewportScrollPosition): Point {
569+
// If the position is locked and we've pushed the overlay already, reuse the previous push
570+
// amount, rather than pushing it again. If we were to continue pushing, the element would
571+
// remain in the viewport, which goes against the expectations when position locking is enabled.
572+
if (this._previousPushAmount && this._positionLocked) {
573+
return {
574+
x: start.x + this._previousPushAmount.x,
575+
y: start.y + this._previousPushAmount.y
576+
};
577+
}
578+
555579
const viewport = this._viewportRect;
556580

557-
// Determine how much the overlay goes outside the viewport on each side, which we'll use to
558-
// decide which direction to push it.
581+
// Determine how much the overlay goes outside the viewport on each
582+
// side, which we'll use to decide which direction to push it.
559583
const overflowRight = Math.max(start.x + overlay.width - viewport.right, 0);
560584
const overflowBottom = Math.max(start.y + overlay.height - viewport.bottom, 0);
561-
const overflowTop = Math.max(viewport.top - start.y, 0);
562-
const overflowLeft = Math.max(viewport.left - start.x, 0);
585+
const overflowTop = Math.max(viewport.top - scrollPosition.top - start.y, 0);
586+
const overflowLeft = Math.max(viewport.left - scrollPosition.left - start.x, 0);
563587

564-
// Amount by which to push the overlay in each direction such that it remains on-screen.
565-
let pushX, pushY = 0;
588+
// Amount by which to push the overlay in each axis such that it remains on-screen.
589+
let pushX = 0;
590+
let pushY = 0;
566591

567592
// If the overlay fits completely within the bounds of the viewport, push it from whichever
568593
// direction is goes off-screen. Otherwise, push the top-left corner such that its in the
569594
// viewport and allow for the trailing end of the overlay to go out of bounds.
570-
if (overlay.width <= viewport.width) {
595+
if (overlay.width < viewport.width) {
571596
pushX = overflowLeft || -overflowRight;
572597
} else {
573-
pushX = viewport.left - start.x;
598+
pushX = start.x < this._viewportMargin ? (viewport.left - scrollPosition.left) - start.x : 0;
574599
}
575600

576-
if (overlay.height <= viewport.height) {
601+
if (overlay.height < viewport.height) {
577602
pushY = overflowTop || -overflowBottom;
578603
} else {
579-
pushY = viewport.top - start.y;
604+
pushY = start.y < this._viewportMargin ? (viewport.top - scrollPosition.top) - start.y : 0;
580605
}
581606

607+
this._previousPushAmount = {x: pushX, y: pushY};
608+
582609
return {
583610
x: start.x + pushX,
584611
y: start.y + pushY,
@@ -801,8 +828,9 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
801828
const styles = {} as CSSStyleDeclaration;
802829

803830
if (this._hasExactPosition()) {
804-
extendStyles(styles, this._getExactOverlayY(position, originPoint));
805-
extendStyles(styles, this._getExactOverlayX(position, originPoint));
831+
const scrollPosition = this._viewportRuler.getViewportScrollPosition();
832+
extendStyles(styles, this._getExactOverlayY(position, originPoint, scrollPosition));
833+
extendStyles(styles, this._getExactOverlayX(position, originPoint, scrollPosition));
806834
} else {
807835
styles.position = 'static';
808836
}
@@ -841,14 +869,16 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
841869
}
842870

843871
/** Gets the exact top/bottom for the overlay when not using flexible sizing or when pushing. */
844-
private _getExactOverlayY(position: ConnectedPosition, originPoint: Point) {
872+
private _getExactOverlayY(position: ConnectedPosition,
873+
originPoint: Point,
874+
scrollPosition: ViewportScrollPosition) {
845875
// Reset any existing styles. This is necessary in case the
846876
// preferred position has changed since the last `apply`.
847877
let styles = {top: null, bottom: null} as CSSStyleDeclaration;
848878
let overlayPoint = this._getOverlayPoint(originPoint, this._overlayRect, position);
849879

850880
if (this._isPushed) {
851-
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect);
881+
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect, scrollPosition);
852882
}
853883

854884
// @breaking-change 7.0.0 Currently the `_overlayContainer` is optional in order to avoid a
@@ -878,14 +908,16 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
878908
}
879909

880910
/** Gets the exact left/right for the overlay when not using flexible sizing or when pushing. */
881-
private _getExactOverlayX(position: ConnectedPosition, originPoint: Point) {
911+
private _getExactOverlayX(position: ConnectedPosition,
912+
originPoint: Point,
913+
scrollPosition: ViewportScrollPosition) {
882914
// Reset any existing styles. This is necessary in case the preferred position has
883915
// changed since the last `apply`.
884916
let styles = {left: null, right: null} as CSSStyleDeclaration;
885917
let overlayPoint = this._getOverlayPoint(originPoint, this._overlayRect, position);
886918

887919
if (this._isPushed) {
888-
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect);
920+
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect, scrollPosition);
889921
}
890922

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

src/cdk/scrolling/viewport-ruler.ts

+7-1
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

+1
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
360360
return new OverlayConfig({
361361
positionStrategy: this._overlay.position()
362362
.flexibleConnectedTo(this._element)
363+
.withLockedPosition()
363364
.withTransformOriginOn('.mat-menu-panel'),
364365
hasBackdrop: this.menu.hasBackdrop == null ? !this.triggersSubmenu() : this.menu.hasBackdrop,
365366
backdropClass: this.menu.backdropClass || 'cdk-overlay-transparent-backdrop',

0 commit comments

Comments
 (0)