Skip to content

Commit

Permalink
Revert "fix(overlay): flexible overlay with push not handling scroll …
Browse files Browse the repository at this point in the history 
…offset and position locking (#11421)" (#11622)
  • Loading branch information
@josephperrott
josephperrott authored Jun 1, 2018
1 parent 55aaa03 commit 9f6aa84
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 213 deletions.
159 changes: 0 additions & 159 deletions src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1101,165 +1101,6 @@ describe('FlexibleConnectedPositionStrategy', () => {
expect(Math.floor(overlayRect.top)).toBe(15);
});

it('should not mess with the left offset when pushing from the top', () => {
originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
originElement.style.left = '200px';

positionStrategy.withPositions([{
originX: 'start',
originY: 'bottom',
overlayX: 'start',
overlayY: 'top'
}]);

attachOverlay({positionStrategy});

const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.left)).toBe(200);
});

it('should align to the trigger if the overlay is wider than the viewport, but the trigger ' +
'is still within the viewport', () => {
originElement.style.top = '200px';
originElement.style.left = '200px';

positionStrategy.withPositions([
{
originX: 'start',
originY: 'bottom',
overlayX: 'start',
overlayY: 'top'
},
{
originX: 'end',
originY: 'bottom',
overlayX: 'end',
overlayY: 'top'
}
]);

attachOverlay({
width: viewport.getViewportRect().width + 100,
positionStrategy
});

const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
const originRect = originElement.getBoundingClientRect();

expect(Math.floor(overlayRect.left)).toBe(Math.floor(originRect.left));
});

it('should push into the viewport if the overlay is wider than the viewport and the trigger' +
'out of the viewport', () => {
originElement.style.top = '200px';
originElement.style.left = `-${DEFAULT_WIDTH / 2}px`;

positionStrategy.withPositions([
{
originX: 'start',
originY: 'bottom',
overlayX: 'start',
overlayY: 'top'
},
{
originX: 'end',
originY: 'bottom',
overlayX: 'end',
overlayY: 'top'
}
]);

attachOverlay({
width: viewport.getViewportRect().width + 100,
positionStrategy
});

const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.left)).toBe(0);
});

it('should keep the element inside the viewport as the user is scrolling, ' +
'with position locking disabled', () => {
const veryLargeElement = document.createElement('div');

originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
originElement.style.left = '200px';

veryLargeElement.style.width = '100%';
veryLargeElement.style.height = '2000px';
document.body.appendChild(veryLargeElement);

positionStrategy
.withLockedPosition(false)
.withViewportMargin(0)
.withPositions([{
overlayY: 'top',
overlayX: 'start',
originY: 'top',
originX: 'start'
}]);

attachOverlay({positionStrategy});

let overlayRect = overlayRef.overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top))
.toBe(0, 'Expected overlay to be in the viewport initially.');

window.scroll(0, 100);
overlayRef.updatePosition();
zone.simulateZoneExit();

overlayRect = overlayRef.overlayElement.getBoundingClientRect();
expect(Math.floor(overlayRect.top))
.toBe(0, 'Expected overlay to stay in the viewport after scrolling.');

window.scroll(0, 0);
document.body.removeChild(veryLargeElement);
});

it('should not continue pushing the overlay as the user scrolls, if position ' +
'locking is enabled', () => {
const veryLargeElement = document.createElement('div');

originElement.style.top = `${-OVERLAY_HEIGHT * 2}px`;
originElement.style.left = '200px';

veryLargeElement.style.width = '100%';
veryLargeElement.style.height = '2000px';
document.body.appendChild(veryLargeElement);

positionStrategy
.withLockedPosition()
.withViewportMargin(0)
.withPositions([{
overlayY: 'top',
overlayX: 'start',
originY: 'top',
originX: 'start'
}]);

attachOverlay({positionStrategy});

const scrollBy = 100;
let initialOverlayTop = Math.floor(overlayRef.overlayElement.getBoundingClientRect().top);

expect(initialOverlayTop).toBe(0, 'Expected overlay to be inside the viewport initially.');

window.scroll(0, scrollBy);
overlayRef.updatePosition();
zone.simulateZoneExit();

let currentOverlayTop = Math.floor(overlayRef.overlayElement.getBoundingClientRect().top);

expect(currentOverlayTop).toBeLessThan(0,
'Expected overlay to no longer be completely inside the viewport.');
expect(currentOverlayTop).toBe(initialOverlayTop - scrollBy,
'Expected overlay to maintain its previous position.');

window.scroll(0, 0);
document.body.removeChild(veryLargeElement);
});

});

describe('with flexible dimensions', () => {
Expand Down
66 changes: 20 additions & 46 deletions src/cdk/overlay/position/flexible-connected-position-strategy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {PositionStrategy} from './position-strategy';
import {ElementRef} from '@angular/core';
import {ViewportRuler, CdkScrollable, ViewportScrollPosition} from '@angular/cdk/scrolling';
import {ViewportRuler, CdkScrollable} from '@angular/cdk/scrolling';
import {
ConnectedOverlayPositionChange,
ConnectionPositionPair,
Expand Down Expand Up @@ -111,9 +111,6 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
/** Amount of subscribers to the `positionChanges` stream. */
private _positionChangeSubscriptions = 0;

/** Amount by which the overlay was pushed in each axis during the last time it was positioned. */
private _previousPushAmount: {x: number, y: number} | null;

/** Observable sequence of position changes. */
positionChanges: Observable<ConnectedOverlayPositionChange> = Observable.create(observer => {
const subscription = this._positionChanges.subscribe(observer);
Expand Down Expand Up @@ -282,8 +279,6 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
}

detach() {
this._lastPosition = null;
this._previousPushAmount = null;
this._resizeSubscription.unsubscribe();
}

Expand Down Expand Up @@ -543,55 +538,39 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
* the viewport, the top-left corner will be pushed on-screen (with overflow occuring on the
* right and bottom).
*
* @param start Starting point from which the overlay is pushed.
* @param overlay Dimensions of the overlay.
* @param scrollPosition Current viewport scroll position.
* @param start The starting point from which the overlay is pushed.
* @param overlay The overlay dimensions.
* @returns The point at which to position the overlay after pushing. This is effectively a new
* originPoint.
*/
private _pushOverlayOnScreen(start: Point,
overlay: ClientRect,
scrollPosition: ViewportScrollPosition): Point {
// If the position is locked and we've pushed the overlay already, reuse the previous push
// amount, rather than pushing it again. If we were to continue pushing, the element would
// remain in the viewport, which goes against the expectations when position locking is enabled.
if (this._previousPushAmount && this._positionLocked) {
return {
x: start.x + this._previousPushAmount.x,
y: start.y + this._previousPushAmount.y
};
}

private _pushOverlayOnScreen(start: Point, overlay: ClientRect): Point {
const viewport = this._viewportRect;

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

// Amount by which to push the overlay in each axis such that it remains on-screen.
let pushX = 0;
let pushY = 0;
// Amount by which to push the overlay in each direction such that it remains on-screen.
let pushX, pushY = 0;

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

if (overlay.height < viewport.height) {
if (overlay.height <= viewport.height) {
pushY = overflowTop || -overflowBottom;
} else {
pushY = start.y < this._viewportMargin ? (viewport.top - scrollPosition.top) - start.y : 0;
pushY = viewport.top - start.y;
}

this._previousPushAmount = {x: pushX, y: pushY};

return {
x: start.x + pushX,
y: start.y + pushY,
Expand Down Expand Up @@ -810,9 +789,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
const styles = {} as CSSStyleDeclaration;

if (this._hasExactPosition()) {
const scrollPosition = this._viewportRuler.getViewportScrollPosition();
extendStyles(styles, this._getExactOverlayY(position, originPoint, scrollPosition));
extendStyles(styles, this._getExactOverlayX(position, originPoint, scrollPosition));
extendStyles(styles, this._getExactOverlayY(position, originPoint));
extendStyles(styles, this._getExactOverlayX(position, originPoint));
} else {
styles.position = 'static';
}
Expand Down Expand Up @@ -851,16 +829,14 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
}

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

if (this._isPushed) {
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect, scrollPosition);
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect);
}

// We want to set either `top` or `bottom` based on whether the overlay wants to appear
Expand All @@ -878,16 +854,14 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
}

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

if (this._isPushed) {
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect, scrollPosition);
overlayPoint = this._pushOverlayOnScreen(overlayPoint, this._overlayRect);
}

// We want to set either `left` or `right` based on whether the overlay wants to appear "before"
Expand Down
8 changes: 1 addition & 7 deletions src/cdk/scrolling/viewport-ruler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ import {auditTime} from 'rxjs/operators';
/** Time in ms to throttle the resize events by default. */
export const DEFAULT_RESIZE_TIME = 20;

/** Object that holds the scroll position of the viewport in each direction. */
export interface ViewportScrollPosition {
top: number;
left: number;
}

/**
* Simple utility for getting the bounds of the browser viewport.
* @docs-private
Expand Down Expand Up @@ -88,7 +82,7 @@ export class ViewportRuler implements OnDestroy {
}

/** Gets the (top, left) scroll position of the viewport. */
getViewportScrollPosition(): ViewportScrollPosition {
getViewportScrollPosition() {
// While we can get a reference to the fake document
// during SSR, it doesn't have getBoundingClientRect.
if (!this._platform.isBrowser) {
Expand Down
1 change: 0 additions & 1 deletion src/lib/menu/menu-trigger.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
return this._overlay.position()
.flexibleConnectedTo(this._element)
.withTransformOriginOn('.mat-menu-panel')
.withLockedPosition()
.withPositions([
{originX, originY, overlayX, overlayY, offsetY},
{originX: originFallbackX, originY, overlayX: overlayFallbackX, overlayY, offsetY},
Expand Down

0 comments on commit 9f6aa84

Please sign in to comment.