Skip to content

Commit

Permalink
fix(dialog): backdrop not being removed if it doesn't have transitions (
Browse files Browse the repository at this point in the history
#1716)

* fix(dialog): backdrop not being removed if it doesn't have transitions

Fixes the dialog's backdrop not being removed if it's transition have been disabled.

Fixes #1607.
  • Loading branch information
crisbeto authored and jelbourn committed Nov 11, 2016
1 parent d3a50b3 commit accab20
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 9 deletions.
24 changes: 18 additions & 6 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ export class OverlayRef implements PortalHost {

// Add class to fade-in the backdrop after one frame.
requestAnimationFrame(() => {
this._backdropElement.classList.add('md-overlay-backdrop-showing');
if (this._backdropElement) {
this._backdropElement.classList.add('md-overlay-backdrop-showing');
}
});
}

Expand All @@ -101,18 +103,28 @@ export class OverlayRef implements PortalHost {
let backdropToDetach = this._backdropElement;

if (backdropToDetach) {
backdropToDetach.classList.remove('md-overlay-backdrop-showing');
backdropToDetach.classList.remove(this._state.backdropClass);
backdropToDetach.addEventListener('transitionend', () => {
backdropToDetach.parentNode.removeChild(backdropToDetach);
let finishDetach = () => {
// It may not be attached to anything in certain cases (e.g. unit tests).
if (backdropToDetach && backdropToDetach.parentNode) {
backdropToDetach.parentNode.removeChild(backdropToDetach);
}

// It is possible that a new portal has been attached to this overlay since we started
// removing the backdrop. If that is the case, only clear the backdrop reference if it
// is still the same instance that we started to remove.
if (this._backdropElement == backdropToDetach) {
this._backdropElement = null;
}
});
};

backdropToDetach.classList.remove('md-overlay-backdrop-showing');
backdropToDetach.classList.remove(this._state.backdropClass);
backdropToDetach.addEventListener('transitionend', finishDetach);

// If the backdrop doesn't have a transition, the `transitionend` event won't fire.
// In this case we make it unclickable and we try to remove it after a delay.
backdropToDetach.style.pointerEvents = 'none';
setTimeout(finishDetach, 500);
}
}
}
Expand Down
20 changes: 17 additions & 3 deletions src/lib/core/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ describe('Overlay', () => {
overlayRef.attach(componentPortal);

viewContainerFixture.detectChanges();
let backdrop = <HTMLElement> overlayContainerElement.querySelector('.md-overlay-backdrop');
let backdrop = overlayContainerElement.querySelector('.md-overlay-backdrop') as HTMLElement;
expect(backdrop).toBeTruthy();
expect(backdrop.classList).not.toContain('md-overlay-backdrop-showing');

Expand All @@ -188,7 +188,7 @@ describe('Overlay', () => {
overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();

let backdrop = <HTMLElement> overlayContainerElement.querySelector('.md-overlay-backdrop');
let backdrop = overlayContainerElement.querySelector('.md-overlay-backdrop') as HTMLElement;
expect(backdrop.classList).toContain('md-overlay-dark-backdrop');
});

Expand All @@ -199,10 +199,24 @@ describe('Overlay', () => {
overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();

let backdrop = <HTMLElement> overlayContainerElement.querySelector('.md-overlay-backdrop');
let backdrop = overlayContainerElement.querySelector('.md-overlay-backdrop') as HTMLElement;
expect(backdrop.classList).toContain('md-overlay-transparent-backdrop');
});

it('should disable the pointer events of a backdrop that is being removed', () => {
let overlayRef = overlay.create(config);
overlayRef.attach(componentPortal);

viewContainerFixture.detectChanges();
let backdrop = overlayContainerElement.querySelector('.md-overlay-backdrop') as HTMLElement;

expect(backdrop.style.pointerEvents).toBeFalsy();

overlayRef.detach();

expect(backdrop.style.pointerEvents).toBe('none');
});

});
});

Expand Down
2 changes: 2 additions & 0 deletions src/lib/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
flushMicrotasks,
ComponentFixture,
TestBed,
tick,
} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {NgModule, Component, Directive, ViewChild, ViewContainerRef} from '@angular/core';
Expand Down Expand Up @@ -207,6 +208,7 @@ describe('MdDialog', () => {
.not.toBe('dialog-trigger', 'Expected the focus to change when dialog was opened.');

dialogRef.close();
tick(500);
viewContainerFixture.detectChanges();
flushMicrotasks();

Expand Down

0 comments on commit accab20

Please sign in to comment.