From 107cab724340db170d7f329998233ff4db1c0c5b Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 18 Sep 2018 21:59:29 +0200 Subject: [PATCH] fix(overlay): incorrectly calculating centered position on a scrolled page with pushing Fixes a centered flexible overlay with pushing, on a scrolled page, not calculating the position properly. There were a couple of issues: * We were using the `top` viewport offset to calculate along the X axis, as well as `left` to calculate along Y. * We weren't accounting correctly for the scroll position. Fixes #11868. --- ...exible-connected-position-strategy.spec.ts | 34 +++++++++++++++++++ .../flexible-connected-position-strategy.ts | 17 ++++++---- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts b/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts index f9e801fdccc1..47ddf8793b54 100644 --- a/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts +++ b/src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts @@ -1704,6 +1704,40 @@ describe('FlexibleConnectedPositionStrategy', () => { expect(Math.floor(overlayRect.top)).toBe(viewportMargin); }); + it('should center flexible overlay with push on a scrolled page', () => { + const veryLargeElement = document.createElement('div'); + + originElement.style.left = '200px'; + originElement.style.top = '200px'; + + veryLargeElement.style.width = '100%'; + veryLargeElement.style.height = '2000px'; + document.body.appendChild(veryLargeElement); + window.scroll(0, 250); + + positionStrategy + .withFlexibleDimensions() + .withPush(true) + .withPositions([{ + overlayY: 'top', + overlayX: 'center', + originY: 'bottom', + originX: 'center' + }]); + + attachOverlay({positionStrategy}); + + const overlayRect = overlayRef.overlayElement.getBoundingClientRect(); + const originRect = originElement.getBoundingClientRect(); + + expect(Math.floor(overlayRect.left - overlayRect.width / 2)) + .toBe(Math.floor(originRect.left - originRect.width / 2)); + + window.scroll(0, 0); + document.body.removeChild(veryLargeElement); + }); + + }); describe('onPositionChange with scrollable view properties', () => { diff --git a/src/cdk/overlay/position/flexible-connected-position-strategy.ts b/src/cdk/overlay/position/flexible-connected-position-strategy.ts index 41b2b8f3060f..d8920ffd0fd1 100644 --- a/src/cdk/overlay/position/flexible-connected-position-strategy.ts +++ b/src/cdk/overlay/position/flexible-connected-position-strategy.ts @@ -687,10 +687,13 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy { bottom = viewport.height - origin.y + this._viewportMargin * 2; height = viewport.height - bottom + this._viewportMargin; } else { - // If neither top nor bottom, it means that the overlay - // is vertically centered on the origin point. + // If neither top nor bottom, it means that the overlay is vertically centered on the + // origin point. Note that we want the position relative to the viewport, rather than + // the page, which is why we don't use something like `viewport.bottom - origin.y` and + // `origin.y - viewport.top`. const smallestDistanceToViewportEdge = - Math.min(viewport.bottom - origin.y, origin.y - viewport.left); + Math.min(viewport.bottom - origin.y + viewport.top, origin.y); + const previousHeight = this._lastBoundingBoxSize.height; height = smallestDistanceToViewportEdge * 2; @@ -720,10 +723,12 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy { left = origin.x; width = viewport.right - origin.x; } else { - // If neither start nor end, it means that the overlay - // is horizontally centered on the origin point. + // If neither start nor end, it means that the overlay is horizontally centered on the + // origin point. Note that we want the position relative to the viewport, rather than + // the page, which is why we don't use something like `viewport.right - origin.x` and + // `origin.x - viewport.left`. const smallestDistanceToViewportEdge = - Math.min(viewport.right - origin.x, origin.x - viewport.top); + Math.min(viewport.right - origin.x + viewport.left, origin.x); const previousWidth = this._lastBoundingBoxSize.width; width = smallestDistanceToViewportEdge * 2;