Skip to content

Commit 174bf40

Browse files
crisbetojelbourn
authored andcommitted
fix(popover-edit): focus trap not being destroyed and potential leaks with takeUntil (#16091)
* Fixes the focus trap associated with a popover edit not being cleaned up. * Fixes potential memory leaks in the way `takeUntil` was being used. See https://blog.angularindepth.com/rxjs-avoiding-takeuntil-leaks-fb5182d047ef.
1 parent 13becda commit 174bf40

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

src/cdk-experimental/popover-edit/table-directives.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,25 +82,24 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
8282
}
8383

8484
private _listenForTableEvents(): void {
85-
const element = this.elementRef.nativeElement!;
86-
85+
const element = this.elementRef.nativeElement;
8786
const toClosest = (selector: string) =>
8887
map((event: UIEvent) => closest(event.target, selector));
8988

9089
this.ngZone.runOutsideAngular(() => {
9190
// Track mouse movement over the table to hide/show hover content.
9291
fromEvent<MouseEvent>(element, 'mouseover').pipe(
93-
takeUntil(this.destroyed),
9492
toClosest(ROW_SELECTOR),
93+
takeUntil(this.destroyed),
9594
).subscribe(this.editEventDispatcher.hovering);
9695
fromEvent<MouseEvent>(element, 'mouseleave').pipe(
97-
takeUntil(this.destroyed),
9896
mapTo(null),
97+
takeUntil(this.destroyed),
9998
).subscribe(this.editEventDispatcher.hovering);
10099
fromEvent<MouseEvent>(element, 'mousemove').pipe(
101-
takeUntil(this.destroyed),
102100
throttleTime(MOUSE_MOVE_THROTTLE_TIME_MS),
103101
toClosest(ROW_SELECTOR),
102+
takeUntil(this.destroyed),
104103
).subscribe(this.editEventDispatcher.mouseMove);
105104

106105
// Track focus within the table to hide/show/make focusable hover content.
@@ -135,9 +134,9 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
135134
).subscribe(this.editEventDispatcher.allRows);
136135

137136
fromEvent<KeyboardEvent>(element, 'keyup').pipe(
138-
takeUntil(this.destroyed),
139137
filter(event => event.key === 'Enter'),
140138
toClosest(CELL_SELECTOR),
139+
takeUntil(this.destroyed),
141140
).subscribe(this.editEventDispatcher.editing);
142141

143142
// Keydown must be used here or else key autorepeat does not work properly on some platforms.
@@ -217,6 +216,11 @@ export class CdkPopoverEdit<C> implements AfterViewInit, OnDestroy {
217216
this.destroyed.next();
218217
this.destroyed.complete();
219218

219+
if (this.focusTrap) {
220+
this.focusTrap.destroy();
221+
this.focusTrap = undefined;
222+
}
223+
220224
if (this.overlayRef) {
221225
this.overlayRef.dispose();
222226
}

0 commit comments

Comments
 (0)