Skip to content

Commit

Permalink
fix(cdk/overlay): detach overlay when portal is destroyed from the ou…
Browse files Browse the repository at this point in the history
…tside (#25212)

Fixes that some of the overlay elements were left behind when the portal is destroyed without going through the overlay API. We need to handle this case, because in many cases users don't have access to the `OverlayRef` (e.g. `MatDialog`) so that they can call `dispose` themselves.

Fixes #25163.
  • Loading branch information
crisbeto committed Jul 20, 2022
1 parent 4342387 commit 627daf9
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 1 deletion.
20 changes: 20 additions & 0 deletions src/cdk/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,26 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
}

this._outsideClickDispatcher.add(this);

// TODO(crisbeto): the null check is here, because the portal outlet returns `any`.
// We should be guaranteed for the result to be `ComponentRef | EmbeddedViewRef`, but
// `instanceof EmbeddedViewRef` doesn't appear to work at the moment.
if (typeof attachResult?.onDestroy === 'function') {
// In most cases we control the portal and we know when it is being detached so that
// we can finish the disposal process. The exception is if the user passes in a custom
// `ViewContainerRef` that isn't destroyed through the overlay API. Note that we use
// `detach` here instead of `dispose`, because we don't know if the user intends to
// reattach the overlay at a later point. It also has the advantage of waiting for animations.
attachResult.onDestroy(() => {
if (this.hasAttached()) {
// We have to delay the `detach` call, because detaching immediately prevents
// other destroy hooks from running. This is likely a framework bug similar to
// https://github.com/angular/angular/issues/46119
this._ngZone.runOutsideAngular(() => Promise.resolve().then(() => this.detach()));
}
});
}

return attachResult;
}

Expand Down
39 changes: 38 additions & 1 deletion src/cdk/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import {waitForAsync, fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
import {
waitForAsync,
fakeAsync,
tick,
ComponentFixture,
TestBed,
flush,
} from '@angular/core/testing';
import {
Component,
ViewChild,
Expand Down Expand Up @@ -463,6 +470,36 @@ describe('Overlay', () => {
expect(() => overlayRef.removePanelClass([''])).not.toThrow();
});

it('should detach a component-based overlay when the view is destroyed', fakeAsync(() => {
const overlayRef = overlay.create();
const paneElement = overlayRef.overlayElement;

overlayRef.attach(componentPortal);
viewContainerFixture.detectChanges();

expect(paneElement.childNodes.length).not.toBe(0);

viewContainerFixture.destroy();
flush();

expect(paneElement.childNodes.length).toBe(0);
}));

it('should detach a template-based overlay when the view is destroyed', fakeAsync(() => {
const overlayRef = overlay.create();
const paneElement = overlayRef.overlayElement;

overlayRef.attach(templatePortal);
viewContainerFixture.detectChanges();

expect(paneElement.childNodes.length).not.toBe(0);

viewContainerFixture.destroy();
flush();

expect(paneElement.childNodes.length).toBe(0);
}));

describe('positioning', () => {
let config: OverlayConfig;

Expand Down

0 comments on commit 627daf9

Please sign in to comment.