Skip to content

Commit b363eae

Browse files
committed
fix(cdk/overlay): avoid issues with overlapping backdrop removals (#30474)
Prior to #30179 the overlay ref was able to handle multiple backdrops being removed at the same time, because we didn't need to retain any state about them, aside from the DOM node. After the switch to the renderer that's no longer the case, because we also retain the event cleanup functions. This is a problem, because they can end up overriding each other and causing events to be dropped incorrectly. These changes move the backdrop-related logic to a new `BackdropRef` class to make the removal process easier to manage, clean up the `overlay-ref.ts` a bit and resolve the issue. Fixes #30426. (cherry picked from commit 6434841)
1 parent 27a2131 commit b363eae

File tree

2 files changed

+71
-92
lines changed

2 files changed

+71
-92
lines changed

src/cdk/overlay/backdrop-ref.ts

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import {NgZone, Renderer2} from '@angular/core';
10+
11+
/** Encapsulates the logic for attaching and detaching a backdrop. */
12+
export class BackdropRef {
13+
readonly element: HTMLElement;
14+
private _cleanupClick: (() => void) | undefined;
15+
private _cleanupTransitionEnd: (() => void) | undefined;
16+
private _fallbackTimeout: ReturnType<typeof setTimeout> | undefined;
17+
18+
constructor(
19+
document: Document,
20+
private _renderer: Renderer2,
21+
private _ngZone: NgZone,
22+
onClick: (event: MouseEvent) => void,
23+
) {
24+
this.element = document.createElement('div');
25+
this.element.classList.add('cdk-overlay-backdrop');
26+
this._cleanupClick = _renderer.listen(this.element, 'click', onClick);
27+
}
28+
29+
detach() {
30+
this._ngZone.runOutsideAngular(() => {
31+
const element = this.element;
32+
clearTimeout(this._fallbackTimeout);
33+
this._cleanupTransitionEnd?.();
34+
this._cleanupTransitionEnd = this._renderer.listen(element, 'transitionend', this.dispose);
35+
this._fallbackTimeout = setTimeout(this.dispose, 500);
36+
37+
// If the backdrop doesn't have a transition, the `transitionend` event won't fire.
38+
// In this case we make it unclickable and we try to remove it after a delay.
39+
element.style.pointerEvents = 'none';
40+
element.classList.remove('cdk-overlay-backdrop-showing');
41+
});
42+
}
43+
44+
dispose = () => {
45+
clearTimeout(this._fallbackTimeout);
46+
this._cleanupClick?.();
47+
this._cleanupTransitionEnd?.();
48+
this._cleanupClick = this._cleanupTransitionEnd = this._fallbackTimeout = undefined;
49+
this.element.remove();
50+
};
51+
}

src/cdk/overlay/overlay-ref.ts

+20-92
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {OverlayConfig} from './overlay-config';
2828
import {coerceCssPixelValue, coerceArray} from '@angular/cdk/coercion';
2929
import {PositionStrategy} from './position/position-strategy';
3030
import {ScrollStrategy} from './scroll';
31+
import {BackdropRef} from './backdrop-ref';
3132

3233
/** An object where all of its properties cannot be written. */
3334
export type ImmutableObject<T> = {
@@ -39,16 +40,13 @@ export type ImmutableObject<T> = {
3940
* Used to manipulate or dispose of said overlay.
4041
*/
4142
export class OverlayRef implements PortalOutlet {
42-
private _backdropElement: HTMLElement | null = null;
43-
private _backdropTimeout: ReturnType<typeof setTimeout> | undefined;
4443
private readonly _backdropClick = new Subject<MouseEvent>();
4544
private readonly _attachments = new Subject<void>();
4645
private readonly _detachments = new Subject<void>();
4746
private _positionStrategy: PositionStrategy | undefined;
4847
private _scrollStrategy: ScrollStrategy | undefined;
4948
private _locationChanges: SubscriptionLike = Subscription.EMPTY;
50-
private _cleanupBackdropClick: (() => void) | undefined;
51-
private _cleanupBackdropTransitionEnd: (() => void) | undefined;
49+
private _backdropRef: BackdropRef | null = null;
5250

5351
/**
5452
* Reference to the parent of the `_host` at the time it was detached. Used to restore
@@ -110,7 +108,7 @@ export class OverlayRef implements PortalOutlet {
110108

111109
/** The overlay's backdrop HTML element. */
112110
get backdropElement(): HTMLElement | null {
113-
return this._backdropElement;
111+
return this._backdropRef?.element || null;
114112
}
115113

116114
/**
@@ -265,7 +263,7 @@ export class OverlayRef implements PortalOutlet {
265263
}
266264

267265
this._disposeScrollStrategy();
268-
this._disposeBackdrop(this._backdropElement);
266+
this._backdropRef?.dispose();
269267
this._locationChanges.unsubscribe();
270268
this._keyboardDispatcher.remove(this);
271269
this._portalOutlet.dispose();
@@ -276,8 +274,7 @@ export class OverlayRef implements PortalOutlet {
276274
this._outsideClickDispatcher.remove(this);
277275
this._host?.remove();
278276
this._afterNextRenderRef?.destroy();
279-
280-
this._previousHostParent = this._pane = this._host = null!;
277+
this._previousHostParent = this._pane = this._host = this._backdropRef = null!;
281278

282279
if (isAttached) {
283280
this._detachments.next();
@@ -432,41 +429,30 @@ export class OverlayRef implements PortalOutlet {
432429
private _attachBackdrop() {
433430
const showingClass = 'cdk-overlay-backdrop-showing';
434431

435-
this._backdropElement = this._document.createElement('div');
436-
this._backdropElement.classList.add('cdk-overlay-backdrop');
432+
this._backdropRef?.dispose();
433+
this._backdropRef = new BackdropRef(this._document, this._renderer, this._ngZone, event => {
434+
this._backdropClick.next(event);
435+
});
437436

438437
if (this._animationsDisabled) {
439-
this._backdropElement.classList.add('cdk-overlay-backdrop-noop-animation');
438+
this._backdropRef.element.classList.add('cdk-overlay-backdrop-noop-animation');
440439
}
441440

442441
if (this._config.backdropClass) {
443-
this._toggleClasses(this._backdropElement, this._config.backdropClass, true);
442+
this._toggleClasses(this._backdropRef.element, this._config.backdropClass, true);
444443
}
445444

446445
// Insert the backdrop before the pane in the DOM order,
447446
// in order to handle stacked overlays properly.
448-
this._host.parentElement!.insertBefore(this._backdropElement, this._host);
449-
450-
// Forward backdrop clicks such that the consumer of the overlay can perform whatever
451-
// action desired when such a click occurs (usually closing the overlay).
452-
this._cleanupBackdropClick?.();
453-
this._cleanupBackdropClick = this._renderer.listen(
454-
this._backdropElement,
455-
'click',
456-
(event: MouseEvent) => this._backdropClick.next(event),
457-
);
447+
this._host.parentElement!.insertBefore(this._backdropRef.element, this._host);
458448

459449
// Add class to fade-in the backdrop after one frame.
460450
if (!this._animationsDisabled && typeof requestAnimationFrame !== 'undefined') {
461451
this._ngZone.runOutsideAngular(() => {
462-
requestAnimationFrame(() => {
463-
if (this._backdropElement) {
464-
this._backdropElement.classList.add(showingClass);
465-
}
466-
});
452+
requestAnimationFrame(() => this._backdropRef?.element.classList.add(showingClass));
467453
});
468454
} else {
469-
this._backdropElement.classList.add(showingClass);
455+
this._backdropRef.element.classList.add(showingClass);
470456
}
471457
}
472458

@@ -485,42 +471,12 @@ export class OverlayRef implements PortalOutlet {
485471

486472
/** Detaches the backdrop (if any) associated with the overlay. */
487473
detachBackdrop(): void {
488-
const backdropToDetach = this._backdropElement;
489-
490-
if (!backdropToDetach) {
491-
return;
492-
}
493-
494474
if (this._animationsDisabled) {
495-
this._disposeBackdrop(backdropToDetach);
496-
return;
475+
this._backdropRef?.dispose();
476+
this._backdropRef = null;
477+
} else {
478+
this._backdropRef?.detach();
497479
}
498-
499-
backdropToDetach.classList.remove('cdk-overlay-backdrop-showing');
500-
501-
this._ngZone.runOutsideAngular(() => {
502-
this._cleanupBackdropTransitionEnd?.();
503-
this._cleanupBackdropTransitionEnd = this._renderer.listen(
504-
backdropToDetach,
505-
'transitionend',
506-
(event: TransitionEvent) => {
507-
this._disposeBackdrop(event.target as HTMLElement | null);
508-
},
509-
);
510-
});
511-
512-
// If the backdrop doesn't have a transition, the `transitionend` event won't fire.
513-
// In this case we make it unclickable and we try to remove it after a delay.
514-
backdropToDetach.style.pointerEvents = 'none';
515-
516-
// Run this outside the Angular zone because there's nothing that Angular cares about.
517-
// If it were to run inside the Angular zone, every test that used Overlay would have to be
518-
// either async or fakeAsync.
519-
this._backdropTimeout = this._ngZone.runOutsideAngular(() =>
520-
setTimeout(() => {
521-
this._disposeBackdrop(backdropToDetach);
522-
}, 500),
523-
);
524480
}
525481

526482
/** Toggles a single CSS class or an array of classes on an element. */
@@ -565,36 +521,8 @@ export class OverlayRef implements PortalOutlet {
565521
/** Disposes of a scroll strategy. */
566522
private _disposeScrollStrategy() {
567523
const scrollStrategy = this._scrollStrategy;
568-
569-
if (scrollStrategy) {
570-
scrollStrategy.disable();
571-
572-
if (scrollStrategy.detach) {
573-
scrollStrategy.detach();
574-
}
575-
}
576-
}
577-
578-
/** Removes a backdrop element from the DOM. */
579-
private _disposeBackdrop(backdrop: HTMLElement | null) {
580-
this._cleanupBackdropClick?.();
581-
this._cleanupBackdropTransitionEnd?.();
582-
583-
if (backdrop) {
584-
backdrop.remove();
585-
586-
// It is possible that a new portal has been attached to this overlay since we started
587-
// removing the backdrop. If that is the case, only clear the backdrop reference if it
588-
// is still the same instance that we started to remove.
589-
if (this._backdropElement === backdrop) {
590-
this._backdropElement = null;
591-
}
592-
}
593-
594-
if (this._backdropTimeout) {
595-
clearTimeout(this._backdropTimeout);
596-
this._backdropTimeout = undefined;
597-
}
524+
scrollStrategy?.disable();
525+
scrollStrategy?.detach?.();
598526
}
599527
}
600528

0 commit comments

Comments
 (0)