fix(overlay): emit attach and detach at appropriate times (#4880)

* Emits the `attachments` when everything has been attached.
* Emits the `detachments` when everything has been detached.
* Completes the `attachments` observable before the `detachments`.

Fixes #4871.

Author: crisbeto

src/lib/core/overlay/overlay-ref.ts

@@ -44,7 +44,6 @@ export class OverlayRef implements PortalHost {
     this.updateSize();
     this.updateDirection();
     this.updatePosition();
-    this._attachments.next();
     this._scrollStrategy.enable();
 
     // Enable pointer events for the overlay pane element.
@@ -58,6 +57,9 @@ export class OverlayRef implements PortalHost {
       this._pane.classList.add(this._state.panelClass);
     }
 
+    // Only emit the `attachments` event once all other setup is done.
+    this._attachments.next();
+
     return attachResult;
   }
 
@@ -73,9 +75,13 @@ export class OverlayRef implements PortalHost {
     // pointer events therefore. Depends on the position strategy and the applied pane boundaries.
     this._togglePointerEvents(false);
     this._scrollStrategy.disable();
+
+    let detachmentResult = this._portalHost.detach();
+
+    // Only emit after everything is detached.
     this._detachments.next();
 
-    return this._portalHost.detach();
+    return detachmentResult;
   }
 
   /**
@@ -93,9 +99,9 @@ export class OverlayRef implements PortalHost {
 
     this.detachBackdrop();
     this._portalHost.dispose();
+    this._attachments.complete();
     this._detachments.next();
     this._detachments.complete();
-    this._attachments.complete();
   }
 
   /**

src/lib/core/overlay/overlay.spec.ts

@@ -147,6 +147,24 @@ describe('Overlay', () => {
     expect(spy).toHaveBeenCalled();
   });
 
+  it('should emit the attachment event after everything is added to the DOM', () => {
+    let state = new OverlayState();
+
+    state.hasBackdrop = true;
+
+    let overlayRef = overlay.create(state);
+
+    overlayRef.attachments().subscribe(() => {
+      expect(overlayContainerElement.querySelector('pizza'))
+          .toBeTruthy('Expected the overlay to have been attached.');
+
+      expect(overlayContainerElement.querySelector('.cdk-overlay-backdrop'))
+          .toBeTruthy('Expected the backdrop to have been attached.');
+    });
+
+    overlayRef.attach(componentPortal);
+  });
+
   it('should emit when an overlay is detached', () => {
     let overlayRef = overlay.create();
     let spy = jasmine.createSpy('detachments spy');
@@ -158,6 +176,18 @@ describe('Overlay', () => {
     expect(spy).toHaveBeenCalled();
   });
 
+  it('should emit the detachment event after the overlay is removed from the DOM', () => {
+    let overlayRef = overlay.create();
+
+    overlayRef.detachments().subscribe(() => {
+      expect(overlayContainerElement.querySelector('pizza'))
+          .toBeFalsy('Expected the overlay to have been detached.');
+    });
+
+    overlayRef.attach(componentPortal);
+    overlayRef.detach();
+  });
+
   it('should emit and complete the observables when an overlay is disposed', () => {
     let overlayRef = overlay.create();
     let disposeSpy = jasmine.createSpy('dispose spy');
@@ -175,6 +205,19 @@ describe('Overlay', () => {
     expect(detachCompleteSpy).toHaveBeenCalled();
   });
 
+  it('should complete the attachment observable before the detachment one', () => {
+    let overlayRef = overlay.create();
+    let callbackOrder = [];
+
+    overlayRef.attachments().subscribe(null, null, () => callbackOrder.push('attach'));
+    overlayRef.detachments().subscribe(null, null, () => callbackOrder.push('detach'));
+
+    overlayRef.attach(componentPortal);
+    overlayRef.dispose();
+
+    expect(callbackOrder).toEqual(['attach', 'detach']);
+  });
+
   describe('positioning', () => {
     let state: OverlayState;
 
@@ -421,7 +464,10 @@ describe('OverlayContainer theming', () => {
 });
 
 /** Simple component for testing ComponentPortal. */
-@Component({template: '<p>Pizza</p>'})
+@Component({
+  selector: 'pizza',
+  template: '<p>Pizza</p>'
+})
 class PizzaMsg { }