Skip to content

Commit

Permalink
fix(overlay): ensure proper stacking order when attaching (#3581)
Browse files Browse the repository at this point in the history
Fixes a case where the order of overlays can be invalid if one overlay is detached, while another is destroyed. Next time both of them are open, the one that was disposed will end up beneath the freshly-created one, no matter the order of attachment. This PR adds some logic that will ensure that all each newly-opened overlay will be at the top of the stacking order.

Fixes #3574.
  • Loading branch information
crisbeto authored and tinayuangao committed Mar 27, 2017
1 parent c29f8ca commit aa5925b
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
22 changes: 18 additions & 4 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,21 @@ export class OverlayRef implements PortalHost {
* @returns The portal attachment result.
*/
attach(portal: Portal<any>): any {
if (this._state.hasBackdrop) {
this._attachBackdrop();
}

let attachResult = this._portalHost.attach(portal);

// Update the pane element with the given state configuration.
this._updateStackingOrder();
this.updateSize();
this.updateDirection();
this.updatePosition();

// Enable pointer events for the overlay pane element.
this._togglePointerEvents(true);

if (this._state.hasBackdrop) {
this._attachBackdrop();
}

return attachResult;
}

Expand Down Expand Up @@ -153,6 +154,19 @@ export class OverlayRef implements PortalHost {
});
}

/**
* Updates the stacking order of the element, moving it to the top if necessary.
* This is required in cases where one overlay was detached, while another one,
* that should be behind it, was destroyed. The next time both of them are opened,
* the stacking will be wrong, because the detached element's pane will still be
* in its original DOM position.
*/
private _updateStackingOrder() {
if (this._pane.nextSibling) {
this._pane.parentNode.appendChild(this._pane);
}
}

/** Detaches the backdrop (if any) associated with the overlay. */
detachBackdrop(): void {
let backdropToDetach = this._backdropElement;
Expand Down
25 changes: 25 additions & 0 deletions src/lib/core/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,31 @@ describe('Overlay', () => {
expect(overlayContainerElement.textContent).toBe('');
});

it('should ensure that the most-recently-attached overlay is on top', () => {
let pizzaOverlayRef = overlay.create();
let cakeOverlayRef = overlay.create();

pizzaOverlayRef.attach(componentPortal);
cakeOverlayRef.attach(templatePortal);

expect(pizzaOverlayRef.overlayElement.nextSibling)
.toBeTruthy('Expected pizza to be on the bottom.');
expect(cakeOverlayRef.overlayElement.nextSibling)
.toBeFalsy('Expected cake to be on top.');

pizzaOverlayRef.dispose();
cakeOverlayRef.detach();

pizzaOverlayRef = overlay.create();
pizzaOverlayRef.attach(componentPortal);
cakeOverlayRef.attach(templatePortal);

expect(pizzaOverlayRef.overlayElement.nextSibling)
.toBeTruthy('Expected pizza to still be on the bottom.');
expect(cakeOverlayRef.overlayElement.nextSibling)
.toBeFalsy('Expected cake to still be on top.');
});

it('should set the direction', () => {
const state = new OverlayState();
state.direction = 'rtl';
Expand Down

0 comments on commit aa5925b

Please sign in to comment.