Skip to content

Commit ad70e87

Browse files
authored
refactor(multiple): replace fromEvent usages (#30201)
Replaces all of our usages of the rxjs `fromEvent` with direct event listeners going through the renderer. This has a few benefits: * In most cases it made our simpler and easier to follow. * By going through the renderer, other tooling can hook into it (e.g. the tracing service). * It reduces our reliance on rxjs. I also ended up cleaning up the fragile testing setup in `cdk/menu` which would've broken any time we introduce a new `inject` call.
1 parent 8921d07 commit ad70e87

21 files changed

+362
-420
lines changed

src/cdk-experimental/column-resize/column-resize.ts

+21-21
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import {
1515
Input,
1616
NgZone,
1717
OnDestroy,
18+
Renderer2,
1819
} from '@angular/core';
1920
import {_IdGenerator} from '@angular/cdk/a11y';
20-
import {fromEvent, merge, Subject} from 'rxjs';
21-
import {filter, map, mapTo, pairwise, startWith, take, takeUntil} from 'rxjs/operators';
21+
import {merge, Subject} from 'rxjs';
22+
import {mapTo, pairwise, startWith, take, takeUntil} from 'rxjs/operators';
2223

2324
import {_closest} from '@angular/cdk-experimental/popover-edit';
2425

@@ -44,6 +45,8 @@ export const COLUMN_RESIZE_OPTIONS = new InjectionToken<ColumnResizeOptions>(
4445
*/
4546
@Directive()
4647
export abstract class ColumnResize implements AfterViewInit, OnDestroy {
48+
private _renderer = inject(Renderer2);
49+
private _eventCleanups: (() => void)[] | undefined;
4750
protected readonly destroyed = new Subject<void>();
4851

4952
/* Publicly accessible interface for triggering and being notified of resizes. */
@@ -78,6 +81,7 @@ export abstract class ColumnResize implements AfterViewInit, OnDestroy {
7881
}
7982

8083
ngOnDestroy() {
84+
this._eventCleanups?.forEach(cleanup => cleanup());
8185
this.destroyed.next();
8286
this.destroyed.complete();
8387
}
@@ -99,25 +103,21 @@ export abstract class ColumnResize implements AfterViewInit, OnDestroy {
99103

100104
private _listenForRowHoverEvents() {
101105
this.ngZone.runOutsideAngular(() => {
102-
const element = this.elementRef.nativeElement!;
103-
104-
fromEvent<MouseEvent>(element, 'mouseover')
105-
.pipe(
106-
map(event => _closest(event.target, HEADER_CELL_SELECTOR)),
107-
takeUntil(this.destroyed),
108-
)
109-
.subscribe(this.eventDispatcher.headerCellHovered);
110-
fromEvent<MouseEvent>(element, 'mouseleave')
111-
.pipe(
112-
filter(
113-
event =>
114-
!!event.relatedTarget &&
115-
!(event.relatedTarget as Element).matches(RESIZE_OVERLAY_SELECTOR),
116-
),
117-
mapTo(null),
118-
takeUntil(this.destroyed),
119-
)
120-
.subscribe(this.eventDispatcher.headerCellHovered);
106+
const element = this.elementRef.nativeElement;
107+
108+
this._eventCleanups = [
109+
this._renderer.listen(element, 'mouseover', (event: MouseEvent) => {
110+
this.eventDispatcher.headerCellHovered.next(_closest(event.target, HEADER_CELL_SELECTOR));
111+
}),
112+
this._renderer.listen(element, 'mouseleave', (event: MouseEvent) => {
113+
if (
114+
event.relatedTarget &&
115+
!(event.relatedTarget as Element).matches(RESIZE_OVERLAY_SELECTOR)
116+
) {
117+
this.eventDispatcher.headerCellHovered.next(null);
118+
}
119+
}),
120+
];
121121
});
122122
}
123123

src/cdk-experimental/column-resize/overlay-handle.ts

+28-8
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,20 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {AfterViewInit, Directive, ElementRef, OnDestroy, NgZone} from '@angular/core';
9+
import {
10+
AfterViewInit,
11+
Directive,
12+
ElementRef,
13+
OnDestroy,
14+
NgZone,
15+
Renderer2,
16+
inject,
17+
} from '@angular/core';
1018
import {coerceCssPixelValue} from '@angular/cdk/coercion';
1119
import {Directionality} from '@angular/cdk/bidi';
1220
import {ESCAPE} from '@angular/cdk/keycodes';
1321
import {CdkColumnDef, _CoalescedStyleScheduler} from '@angular/cdk/table';
14-
import {fromEvent, Subject, merge} from 'rxjs';
22+
import {Subject, merge, Observable} from 'rxjs';
1523
import {
1624
distinctUntilChanged,
1725
filter,
@@ -37,6 +45,7 @@ import {ResizeRef} from './resize-ref';
3745
*/
3846
@Directive()
3947
export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
48+
private _renderer = inject(Renderer2);
4049
protected readonly destroyed = new Subject<void>();
4150

4251
protected abstract readonly columnDef: CdkColumnDef;
@@ -62,11 +71,11 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
6271

6372
private _listenForMouseEvents() {
6473
this.ngZone.runOutsideAngular(() => {
65-
fromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mouseenter')
74+
this._observableFromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mouseenter')
6675
.pipe(mapTo(this.resizeRef.origin.nativeElement!), takeUntil(this.destroyed))
6776
.subscribe(cell => this.eventDispatcher.headerCellHovered.next(cell));
6877

69-
fromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mouseleave')
78+
this._observableFromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mouseleave')
7079
.pipe(
7180
map(
7281
event =>
@@ -76,7 +85,7 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
7685
)
7786
.subscribe(cell => this.eventDispatcher.headerCellHovered.next(cell));
7887

79-
fromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mousedown')
88+
this._observableFromEvent<MouseEvent>(this.elementRef.nativeElement!, 'mousedown')
8089
.pipe(takeUntil(this.destroyed))
8190
.subscribe(mousedownEvent => {
8291
this._dragStarted(mousedownEvent);
@@ -90,9 +99,9 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
9099
return;
91100
}
92101

93-
const mouseup = fromEvent<MouseEvent>(this.document, 'mouseup');
94-
const mousemove = fromEvent<MouseEvent>(this.document, 'mousemove');
95-
const escape = fromEvent<KeyboardEvent>(this.document, 'keyup').pipe(
102+
const mouseup = this._observableFromEvent<MouseEvent>(this.document, 'mouseup');
103+
const mousemove = this._observableFromEvent<MouseEvent>(this.document, 'mousemove');
104+
const escape = this._observableFromEvent<KeyboardEvent>(this.document, 'keyup').pipe(
96105
filter(event => event.keyCode === ESCAPE),
97106
);
98107

@@ -233,4 +242,15 @@ export abstract class ResizeOverlayHandle implements AfterViewInit, OnDestroy {
233242
}
234243
});
235244
}
245+
246+
private _observableFromEvent<T extends Event>(element: Element | Document, name: string) {
247+
return new Observable<T>(subscriber => {
248+
const handler = (event: T) => subscriber.next(event);
249+
const cleanup = this._renderer.listen(element, name, handler);
250+
return () => {
251+
cleanup();
252+
subscriber.complete();
253+
};
254+
});
255+
}
236256
}

src/cdk/listbox/listbox.ts

+15-19
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ import {
4242
OnDestroy,
4343
Output,
4444
QueryList,
45+
Renderer2,
4546
signal,
4647
} from '@angular/core';
4748
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
48-
import {defer, fromEvent, merge, Observable, Subject} from 'rxjs';
49+
import {defer, merge, Observable, Subject} from 'rxjs';
4950
import {filter, map, startWith, switchMap, takeUntil} from 'rxjs/operators';
5051

5152
/**
@@ -256,6 +257,8 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
256257
],
257258
})
258259
export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, ControlValueAccessor {
260+
private _cleanupWindowBlur: (() => void) | undefined;
261+
259262
/** The id of the option's host element. */
260263
@Input()
261264
get id() {
@@ -439,7 +442,16 @@ export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, Con
439442

440443
constructor() {
441444
if (this._isBrowser) {
442-
this._setPreviousActiveOptionAsActiveOptionOnWindowBlur();
445+
const renderer = inject(Renderer2);
446+
447+
this._cleanupWindowBlur = this.ngZone.runOutsideAngular(() => {
448+
return renderer.listen('window', 'blur', () => {
449+
if (this.element.contains(document.activeElement) && this._previousActiveOption) {
450+
this._setActiveOption(this._previousActiveOption);
451+
this._previousActiveOption = null;
452+
}
453+
});
454+
});
443455
}
444456
}
445457

@@ -465,6 +477,7 @@ export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, Con
465477
}
466478

467479
ngOnDestroy() {
480+
this._cleanupWindowBlur?.();
468481
this.listKeyManager?.destroy();
469482
this.destroyed.next();
470483
this.destroyed.complete();
@@ -1035,23 +1048,6 @@ export class CdkListbox<T = unknown> implements AfterContentInit, OnDestroy, Con
10351048
const index = this.options.toArray().indexOf(this._lastTriggered!);
10361049
return index === -1 ? null : index;
10371050
}
1038-
1039-
/**
1040-
* Set previous active option as active option on window blur.
1041-
* This ensures that the `activeOption` matches the actual focused element when the user returns to the document.
1042-
*/
1043-
private _setPreviousActiveOptionAsActiveOptionOnWindowBlur() {
1044-
this.ngZone.runOutsideAngular(() => {
1045-
fromEvent(window, 'blur')
1046-
.pipe(takeUntil(this.destroyed))
1047-
.subscribe(() => {
1048-
if (this.element.contains(document.activeElement) && this._previousActiveOption) {
1049-
this._setActiveOption(this._previousActiveOption);
1050-
this._previousActiveOption = null;
1051-
}
1052-
});
1053-
});
1054-
}
10551051
}
10561052

10571053
/** Change event that is fired whenever the value of the listbox changes. */

src/cdk/menu/menu-aim.ts

+23-12
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,16 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {Directive, inject, Injectable, InjectionToken, NgZone, OnDestroy} from '@angular/core';
10-
import {fromEvent, Subject} from 'rxjs';
11-
import {filter, takeUntil} from 'rxjs/operators';
9+
import {
10+
Directive,
11+
inject,
12+
Injectable,
13+
InjectionToken,
14+
NgZone,
15+
OnDestroy,
16+
RendererFactory2,
17+
} from '@angular/core';
18+
import {Subject} from 'rxjs';
1219
import {FocusableElement, PointerFocusTracker} from './pointer-focus-tracker';
1320
import {Menu} from './menu-interface';
1421
import {throwMissingMenuReference, throwMissingPointerFocusTracker} from './menu-errors';
@@ -102,8 +109,9 @@ function isWithinSubmenu(submenuPoints: DOMRect, m: number, b: number) {
102109
*/
103110
@Injectable()
104111
export class TargetMenuAim implements MenuAim, OnDestroy {
105-
/** The Angular zone. */
106112
private readonly _ngZone = inject(NgZone);
113+
private readonly _renderer = inject(RendererFactory2).createRenderer(null, null);
114+
private _cleanupMousemove: (() => void) | undefined;
107115

108116
/** The last NUM_POINTS mouse move events. */
109117
private readonly _points: Point[] = [];
@@ -121,6 +129,7 @@ export class TargetMenuAim implements MenuAim, OnDestroy {
121129
private readonly _destroyed: Subject<void> = new Subject();
122130

123131
ngOnDestroy() {
132+
this._cleanupMousemove?.();
124133
this._destroyed.next();
125134
this._destroyed.complete();
126135
}
@@ -231,18 +240,20 @@ export class TargetMenuAim implements MenuAim, OnDestroy {
231240

232241
/** Subscribe to the root menus mouse move events and update the tracked mouse points. */
233242
private _subscribeToMouseMoves() {
234-
this._ngZone.runOutsideAngular(() => {
235-
fromEvent<MouseEvent>(this._menu.nativeElement, 'mousemove')
236-
.pipe(
237-
filter((_: MouseEvent, index: number) => index % MOUSE_MOVE_SAMPLE_FREQUENCY === 0),
238-
takeUntil(this._destroyed),
239-
)
240-
.subscribe((event: MouseEvent) => {
243+
this._cleanupMousemove?.();
244+
245+
this._cleanupMousemove = this._ngZone.runOutsideAngular(() => {
246+
let eventIndex = 0;
247+
248+
return this._renderer.listen(this._menu.nativeElement, 'mousemove', (event: MouseEvent) => {
249+
if (eventIndex % MOUSE_MOVE_SAMPLE_FREQUENCY === 0) {
241250
this._points.push({x: event.clientX, y: event.clientY});
242251
if (this._points.length > NUM_POINTS) {
243252
this._points.shift();
244253
}
245-
});
254+
}
255+
eventIndex++;
256+
});
246257
});
247258
}
248259
}

src/cdk/menu/menu-base.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
NgZone,
1818
OnDestroy,
1919
QueryList,
20+
Renderer2,
2021
computed,
2122
inject,
2223
signal,
@@ -51,12 +52,12 @@ export abstract class CdkMenuBase
5152
extends CdkMenuGroup
5253
implements Menu, AfterContentInit, OnDestroy
5354
{
55+
protected ngZone = inject(NgZone);
56+
private _renderer = inject(Renderer2);
57+
5458
/** The menu's native DOM host element. */
5559
readonly nativeElement: HTMLElement = inject(ElementRef).nativeElement;
5660

57-
/** The Angular zone. */
58-
protected ngZone = inject(NgZone);
59-
6061
/** The stack of menus this menu belongs to. */
6162
readonly menuStack: MenuStack = inject(MENU_STACK);
6263

@@ -225,7 +226,7 @@ export abstract class CdkMenuBase
225226
private _setUpPointerTracker() {
226227
if (this.menuAim) {
227228
this.ngZone.runOutsideAngular(() => {
228-
this.pointerTracker = new PointerFocusTracker(this.items);
229+
this.pointerTracker = new PointerFocusTracker(this._renderer, this.items);
229230
});
230231
this.menuAim.initialize(this, this.pointerTracker!);
231232
}

src/cdk/menu/menu-item-checkbox.spec.ts

+8-18
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,16 @@
1-
import {Component, ElementRef} from '@angular/core';
2-
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
1+
import {Component} from '@angular/core';
2+
import {ComponentFixture, TestBed} from '@angular/core/testing';
33
import {By} from '@angular/platform-browser';
44
import {CdkMenuModule} from './menu-module';
55
import {CdkMenuItemCheckbox} from './menu-item-checkbox';
6-
import {CDK_MENU} from './menu-interface';
7-
import {CdkMenu} from './menu';
8-
import {MENU_STACK, MenuStack} from './menu-stack';
96

107
describe('MenuItemCheckbox', () => {
118
let fixture: ComponentFixture<SingleCheckboxButton>;
129
let checkbox: CdkMenuItemCheckbox;
1310
let checkboxElement: HTMLButtonElement;
1411

15-
beforeEach(waitForAsync(() => {
16-
TestBed.configureTestingModule({
17-
imports: [CdkMenuModule, SingleCheckboxButton],
18-
providers: [
19-
{provide: CDK_MENU, useClass: CdkMenu},
20-
{provide: MENU_STACK, useClass: MenuStack},
21-
// View engine can't figure out the ElementRef to inject so we need to provide a fake
22-
{provide: ElementRef, useValue: new ElementRef<null>(null)},
23-
],
24-
});
25-
}));
26-
2712
beforeEach(() => {
13+
TestBed.configureTestingModule({imports: [CdkMenuModule, SingleCheckboxButton]});
2814
fixture = TestBed.createComponent(SingleCheckboxButton);
2915
fixture.detectChanges();
3016

@@ -99,7 +85,11 @@ describe('MenuItemCheckbox', () => {
9985
});
10086

10187
@Component({
102-
template: `<button cdkMenuItemCheckbox>Click me!</button>`,
88+
template: `
89+
<div cdkMenu>
90+
<button cdkMenuItemCheckbox>Click me!</button>
91+
</div>
92+
`,
10393
imports: [CdkMenuModule],
10494
})
10595
class SingleCheckboxButton {}

0 commit comments

Comments
 (0)