Skip to content

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

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
Jun 6, 2017
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
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