Skip to content

Commit

Permalink
fix(overlay): emit attach and detach at appropriate times (#4880)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
crisbeto authored and andrewseguin committed Jun 6, 2017
1 parent c8b1e20 commit e98372e
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 4 deletions.
12 changes: 9 additions & 3 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}

Expand All @@ -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;
}

/**
Expand All @@ -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();
}

/**
Expand Down
48 changes: 47 additions & 1 deletion src/lib/core/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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;

Expand Down Expand Up @@ -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 { }


Expand Down

0 comments on commit e98372e

Please sign in to comment.