Skip to content
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.

Sign up for GitHub

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

fix(overlay): infinite loop when used together with zone-patch-rxjs #13081

Merged
merged 1 commit into from
Sep 14, 2018
+30 −19
Merged

fix(overlay): infinite loop when used together with zone-patch-rxjs #13081

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 30 additions & 19 deletions src/cdk/overlay/overlay-ref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,25 +186,9 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
// Remove this overlay from keyboard dispatcher tracking.
this._keyboardDispatcher.remove(this);

// Keeping the host element in DOM the can cause scroll jank, because it still gets rendered,
// even though it's transparent and unclickable. We can't remove the host here immediately,
// because the overlay pane's content might still be animating. This stream helps us avoid
// interrupting the animation by waiting for the pane to become empty.
const subscription = this._ngZone.onStable
.asObservable()
.pipe(takeUntil(merge(this._attachments, this._detachments)))
.subscribe(() => {
// Needs a couple of checks for the pane and host, because
// they may have been removed by the time the zone stabilizes.
if (!this._pane || !this._host || this._pane.children.length === 0) {
if (this._host && this._host.parentElement) {
this._previousHostParent = this._host.parentElement;
this._previousHostParent.removeChild(this._host);
}

subscription.unsubscribe();
}
});
// Keeping the host element in DOM the can cause scroll jank, because it still gets
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: "in DOM the" -> "in the DOM"

// rendered, even though it's transparent and unclickable which is why we remove it.
this._detachContentWhenStable();

return detachmentResult;
}
Expand Down Expand Up @@ -425,6 +409,33 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
isAdd ? classList.add(cssClass) : classList.remove(cssClass);
});
}

/** Detaches the overlay content next time the zone stabilizes. */
private _detachContentWhenStable() {
// Normally we wouldn't have to explicitly run this outside the `NgZone`, however
// if the consumer is using `zone-patch-rxjs`, the `Subscription.unsubscribe` call will
// be patched to run inside the zone, which will throw us into an infinite loop.
this._ngZone.runOutsideAngular(() => {
// We can't remove the host here immediately, because the overlay pane's content
// might still be animating. This stream helps us avoid interrupting the animation
// by waiting for the pane to become empty.
const subscription = this._ngZone.onStable
.asObservable()
.pipe(takeUntil(merge(this._attachments, this._detachments)))
.subscribe(() => {
// Needs a couple of checks for the pane and host, because
// they may have been removed by the time the zone stabilizes.
if (!this._pane || !this._host || this._pane.children.length === 0) {
if (this._host && this._host.parentElement) {
this._previousHostParent = this._host.parentElement;
this._previousHostParent.removeChild(this._host);
}

subscription.unsubscribe();
}
});
});
}
}


Expand Down