Skip to content

feat(overlay): add support for swappable position strategies #12306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher'
import {OverlayConfig} from './overlay-config';
import {coerceCssPixelValue, coerceArray} from '@angular/cdk/coercion';
import {OverlayReference} from './overlay-reference';
import {PositionStrategy} from './position/position-strategy';


/** An object where all of its properties cannot be written. */
Expand All @@ -31,12 +32,14 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
private _backdropClick: Subject<MouseEvent> = new Subject();
private _attachments = new Subject<void>();
private _detachments = new Subject<void>();
private _positionStrategy: PositionStrategy | undefined;

/**
* Reference to the parent of the `_host` at the time it was detached. Used to restore
* the `_host` to its original position in the DOM when it gets re-attached.
*/
private _previousHostParent: HTMLElement;

private _keydownEventsObservable: Observable<KeyboardEvent> = Observable.create(observer => {
const subscription = this._keydownEvents.subscribe(observer);
this._keydownEventSubscriptions++;
Expand Down Expand Up @@ -65,6 +68,8 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
if (_config.scrollStrategy) {
_config.scrollStrategy.attach(this);
}

this._positionStrategy = _config.positionStrategy;
}

/** The overlay's HTML element */
Expand Down Expand Up @@ -100,8 +105,8 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
attach(portal: Portal<any>): any {
let attachResult = this._portalOutlet.attach(portal);

if (this._config.positionStrategy) {
this._config.positionStrategy.attach(this);
if (this._positionStrategy) {
this._positionStrategy.attach(this);
}

// Update the pane element with the given configuration.
Expand Down Expand Up @@ -166,8 +171,8 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
// pointer events therefore. Depends on the position strategy and the applied pane boundaries.
this._togglePointerEvents(false);

if (this._config.positionStrategy && this._config.positionStrategy.detach) {
this._config.positionStrategy.detach();
if (this._positionStrategy && this._positionStrategy.detach) {
this._positionStrategy.detach();
}

if (this._config.scrollStrategy) {
Expand Down Expand Up @@ -213,8 +218,8 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
dispose(): void {
const isAttached = this.hasAttached();

if (this._config.positionStrategy) {
this._config.positionStrategy.dispose();
if (this._positionStrategy) {
this._positionStrategy.dispose();
}

if (this._config.scrollStrategy) {
Expand Down Expand Up @@ -274,8 +279,26 @@ export class OverlayRef implements PortalOutlet, OverlayReference {

/** Updates the position of the overlay based on the position strategy. */
updatePosition() {
if (this._config.positionStrategy) {
this._config.positionStrategy.apply();
if (this._positionStrategy) {
this._positionStrategy.apply();
}
}

/** Switches to a new position strategy and updates the overlay position. */
updatePositionStrategy(strategy: PositionStrategy) {
if (strategy === this._positionStrategy) {
return;
}

if (this._positionStrategy) {
this._positionStrategy.dispose();
}

this._positionStrategy = strategy;

if (this.hasAttached()) {
strategy.attach(this);
this.updatePosition();
}
}

Expand Down
64 changes: 64 additions & 0 deletions src/cdk/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,70 @@ describe('Overlay', () => {
expect(config.positionStrategy.apply).not.toHaveBeenCalled();
}));

it('should be able to swap position strategies', fakeAsync(() => {
const firstStrategy = new FakePositionStrategy();
const secondStrategy = new FakePositionStrategy();

[firstStrategy, secondStrategy].forEach(strategy => {
spyOn(strategy, 'attach');
spyOn(strategy, 'apply');
spyOn(strategy, 'dispose');
});

config.positionStrategy = firstStrategy;

const overlayRef = overlay.create(config);
overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();
zone.simulateZoneExit();
tick();

expect(firstStrategy.attach).toHaveBeenCalledTimes(1);
expect(firstStrategy.apply).toHaveBeenCalledTimes(1);

expect(secondStrategy.attach).not.toHaveBeenCalled();
expect(secondStrategy.apply).not.toHaveBeenCalled();

overlayRef.updatePositionStrategy(secondStrategy);
viewContainerFixture.detectChanges();
tick();

expect(firstStrategy.attach).toHaveBeenCalledTimes(1);
expect(firstStrategy.apply).toHaveBeenCalledTimes(1);
expect(firstStrategy.dispose).toHaveBeenCalledTimes(1);

expect(secondStrategy.attach).toHaveBeenCalledTimes(1);
expect(secondStrategy.apply).toHaveBeenCalledTimes(1);
}));

it('should not do anything when trying to swap a strategy with itself', fakeAsync(() => {
const strategy = new FakePositionStrategy();

spyOn(strategy, 'attach');
spyOn(strategy, 'apply');
spyOn(strategy, 'dispose');

config.positionStrategy = strategy;

const overlayRef = overlay.create(config);
overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();
zone.simulateZoneExit();
tick();

expect(strategy.attach).toHaveBeenCalledTimes(1);
expect(strategy.apply).toHaveBeenCalledTimes(1);
expect(strategy.dispose).not.toHaveBeenCalled();

overlayRef.updatePositionStrategy(strategy);
viewContainerFixture.detectChanges();
tick();

expect(strategy.attach).toHaveBeenCalledTimes(1);
expect(strategy.apply).toHaveBeenCalledTimes(1);
expect(strategy.dispose).not.toHaveBeenCalled();
}));

});

describe('size', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,46 @@ describe('FlexibleConnectedPositionStrategy', () => {
document.body.removeChild(originElement);
});

it('should clean up after itself when disposed', () => {
const origin = document.createElement('div');
const positionStrategy = overlay.position()
.flexibleConnectedTo(origin)
.withPositions([{
overlayX: 'start',
overlayY: 'top',
originX: 'start',
originY: 'bottom'
}]);

// Needs to be in the DOM for IE not to throw an "Unspecified error".
document.body.appendChild(origin);
attachOverlay({positionStrategy});

const boundingBox = overlayRef.hostElement;
const pane = overlayRef.overlayElement;

positionStrategy.dispose();

expect(boundingBox.style.top).toBeFalsy();
expect(boundingBox.style.bottom).toBeFalsy();
expect(boundingBox.style.left).toBeFalsy();
expect(boundingBox.style.right).toBeFalsy();
expect(boundingBox.style.width).toBeFalsy();
expect(boundingBox.style.height).toBeFalsy();
expect(boundingBox.style.alignItems).toBeFalsy();
expect(boundingBox.style.justifyContent).toBeFalsy();
expect(boundingBox.classList).not.toContain('cdk-overlay-connected-position-bounding-box');

expect(pane.style.top).toBeFalsy();
expect(pane.style.bottom).toBeFalsy();
expect(pane.style.left).toBeFalsy();
expect(pane.style.right).toBeFalsy();
expect(pane.style.position).toBeFalsy();

overlayRef.dispose();
document.body.removeChild(origin);
});

describe('without flexible dimensions and pushing', () => {
const ORIGIN_HEIGHT = DEFAULT_HEIGHT;
const ORIGIN_WIDTH = DEFAULT_WIDTH;
Expand Down
45 changes: 38 additions & 7 deletions src/cdk/overlay/position/flexible-connected-position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import {OverlayContainer} from '../overlay-container';
// TODO: refactor clipping detection into a separate thing (part of scrolling module)
// TODO: doesn't handle both flexible width and height when it has to scroll along both axis.

/** Class to be added to the overlay bounding box. */
const boundingBoxClass = 'cdk-overlay-connected-position-bounding-box';

/**
* A strategy for positioning overlays. Using this strategy, an overlay is given an
* implicit position relative some origin element. The relative position is defined in terms of
Expand All @@ -38,7 +41,7 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
private _overlayRef: OverlayReference;

/** Whether we're performing the very first positioning of the overlay. */
private _isInitialRender = true;
private _isInitialRender: boolean;

/** Last size used for the bounding box. Used to avoid resizing the overlay after open. */
private _lastBoundingBoxSize = {width: 0, height: 0};
Expand Down Expand Up @@ -152,11 +155,14 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {

this._validatePositions();

overlayRef.hostElement.classList.add('cdk-overlay-connected-position-bounding-box');
overlayRef.hostElement.classList.add(boundingBoxClass);

this._overlayRef = overlayRef;
this._boundingBox = overlayRef.hostElement;
this._pane = overlayRef.overlayElement;
this._isDisposed = false;
this._isInitialRender = true;
this._lastPosition = null;
this._resizeSubscription.unsubscribe();
this._resizeSubscription = this._viewportRuler.change().subscribe(() => {
// When the window is resized, we want to trigger the next reposition as if it
Expand Down Expand Up @@ -303,12 +309,37 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {

/** Cleanup after the element gets destroyed. */
dispose() {
if (!this._isDisposed) {
this.detach();
this._boundingBox = null;
this._positionChanges.complete();
this._isDisposed = true;
if (this._isDisposed) {
return;
}

// We can't use `_resetBoundingBoxStyles` here, because it resets
// some properties to zero, rather than removing them.
if (this._boundingBox) {
extendStyles(this._boundingBox.style, {
top: '',
left: '',
right: '',
bottom: '',
height: '',
width: '',
alignItems: '',
justifyContent: '',
} as CSSStyleDeclaration);
}

if (this._pane) {
this._resetOverlayElementStyles();
}

if (this._overlayRef) {
this._overlayRef.hostElement.classList.remove(boundingBoxClass);
}

this.detach();
this._positionChanges.complete();
this._overlayRef = this._boundingBox = null!;
this._isDisposed = true;
}

/**
Expand Down
21 changes: 21 additions & 0 deletions src/cdk/overlay/position/global-position-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,27 @@ describe('GlobalPositonStrategy', () => {
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
expect(parentStyle.justifyContent).toBe('flex-start');
});

it('should clean up after itself when it has been disposed', () => {
const positionStrategy = overlay.position().global().top('10px').left('40px');

attachOverlay({positionStrategy});

const elementStyle = overlayRef.overlayElement.style;
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;

positionStrategy.dispose();

expect(elementStyle.marginTop).toBeFalsy();
expect(elementStyle.marginLeft).toBeFalsy();
expect(elementStyle.marginBottom).toBeFalsy();
expect(elementStyle.marginBottom).toBeFalsy();
expect(elementStyle.position).toBeFalsy();

expect(parentStyle.justifyContent).toBeFalsy();
expect(parentStyle.alignItems).toBeFalsy();
});

});


Expand Down
29 changes: 24 additions & 5 deletions src/cdk/overlay/position/global-position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import {PositionStrategy} from './position-strategy';
import {OverlayReference} from '../overlay-reference';

/** Class to be added to the overlay pane wrapper. */
const wrapperClass = 'cdk-global-overlay-wrapper';

/**
* A strategy for positioning overlays. Using this strategy, an overlay is given an
Expand All @@ -28,6 +30,7 @@ export class GlobalPositionStrategy implements PositionStrategy {
private _justifyContent: string = '';
private _width: string = '';
private _height: string = '';
private _isDisposed: boolean;

attach(overlayRef: OverlayReference): void {
const config = overlayRef.getConfig();
Expand All @@ -42,7 +45,8 @@ export class GlobalPositionStrategy implements PositionStrategy {
overlayRef.updateSize({height: this._height});
}

overlayRef.hostElement.classList.add('cdk-global-overlay-wrapper');
overlayRef.hostElement.classList.add(wrapperClass);
this._isDisposed = false;
}

/**
Expand Down Expand Up @@ -153,7 +157,7 @@ export class GlobalPositionStrategy implements PositionStrategy {
// Since the overlay ref applies the strategy asynchronously, it could
// have been disposed before it ends up being applied. If that is the
// case, we shouldn't do anything.
if (!this._overlayRef.hasAttached()) {
if (!this._overlayRef || !this._overlayRef.hasAttached()) {
return;
}

Expand All @@ -170,7 +174,7 @@ export class GlobalPositionStrategy implements PositionStrategy {
if (config.width === '100%') {
parentStyles.justifyContent = 'flex-start';
} else if (this._justifyContent === 'center') {
parentStyles.justifyContent = 'center';
parentStyles.justifyContent = 'center';
} else if (this._overlayRef.getConfig().direction === 'rtl') {
// In RTL the browser will invert `flex-start` and `flex-end` automatically, but we
// don't want that because our positioning is explicitly `left` and `right`, hence
Expand All @@ -189,8 +193,23 @@ export class GlobalPositionStrategy implements PositionStrategy {
}

/**
* Noop implemented as a part of the PositionStrategy interface.
* Cleans up the DOM changes from the position strategy.
* @docs-private
*/
dispose(): void { }
dispose(): void {
if (this._isDisposed || !this._overlayRef) {
return;
}

const styles = this._overlayRef.overlayElement.style;
const parent = this._overlayRef.hostElement;
const parentStyles = parent.style;

parent.classList.remove(wrapperClass);
parentStyles.justifyContent = parentStyles.alignItems = styles.marginTop =
styles.marginBottom = styles.marginLeft = styles.marginRight = styles.position = '';

this._overlayRef = null!;
this._isDisposed = true;
}
}