Skip to content

Commit 9ad4855

Browse files
authored
refactor(multiple): use renderer for manually-bound events with options (#30271)
This is a second attempt at landing the changes from #30195. I've removed some of the riskier changes. Switches all manually-bound event handlers that were passing options to go through the renderer.
1 parent 961ff25 commit 9ad4855

File tree

10 files changed

+325
-222
lines changed

10 files changed

+325
-222
lines changed

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

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ import {
1919
TemplateRef,
2020
ViewContainerRef,
2121
inject,
22+
Renderer2,
23+
ListenerOptions,
2224
} from '@angular/core';
23-
import {fromEvent, fromEventPattern, merge, Subject} from 'rxjs';
25+
import {merge, Observable, Subject} from 'rxjs';
2426
import {
2527
debounceTime,
2628
filter,
@@ -44,6 +46,7 @@ import {
4446
} from './focus-escape-notifier';
4547
import {closest} from './polyfill';
4648
import {EditRef} from './edit-ref';
49+
import {_bindEventWithOptions} from '@angular/cdk/platform';
4750

4851
/**
4952
* Describes the number of columns before and after the originating cell that the
@@ -73,6 +76,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
7376
inject<EditEventDispatcher<EditRef<unknown>>>(EditEventDispatcher);
7477
protected readonly focusDispatcher = inject(FocusDispatcher);
7578
protected readonly ngZone = inject(NgZone);
79+
private readonly _renderer = inject(Renderer2);
7680

7781
protected readonly destroyed = new Subject<void>();
7882

@@ -94,20 +98,37 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
9498
this._rendered.complete();
9599
}
96100

101+
private _observableFromEvent<T extends Event>(
102+
element: Element,
103+
name: string,
104+
options?: ListenerOptions,
105+
) {
106+
return new Observable<T>(subscriber => {
107+
const handler = (event: T) => subscriber.next(event);
108+
const cleanup = options
109+
? _bindEventWithOptions(this._renderer, element, name, handler, options)
110+
: this._renderer.listen(element, name, handler, options);
111+
return () => {
112+
cleanup();
113+
subscriber.complete();
114+
};
115+
});
116+
}
117+
97118
private _listenForTableEvents(): void {
98119
const element = this.elementRef.nativeElement;
99120
const toClosest = (selector: string) =>
100121
map((event: UIEvent) => closest(event.target, selector));
101122

102123
this.ngZone.runOutsideAngular(() => {
103124
// Track mouse movement over the table to hide/show hover content.
104-
fromEvent<MouseEvent>(element, 'mouseover')
125+
this._observableFromEvent<MouseEvent>(element, 'mouseover')
105126
.pipe(toClosest(ROW_SELECTOR), takeUntil(this.destroyed))
106127
.subscribe(this.editEventDispatcher.hovering);
107-
fromEvent<MouseEvent>(element, 'mouseleave')
128+
this._observableFromEvent<MouseEvent>(element, 'mouseleave')
108129
.pipe(mapTo(null), takeUntil(this.destroyed))
109130
.subscribe(this.editEventDispatcher.hovering);
110-
fromEvent<MouseEvent>(element, 'mousemove')
131+
this._observableFromEvent<MouseEvent>(element, 'mousemove')
111132
.pipe(
112133
throttleTime(MOUSE_MOVE_THROTTLE_TIME_MS),
113134
toClosest(ROW_SELECTOR),
@@ -116,19 +137,15 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
116137
.subscribe(this.editEventDispatcher.mouseMove);
117138

118139
// Track focus within the table to hide/show/make focusable hover content.
119-
fromEventPattern<FocusEvent>(
120-
handler => element.addEventListener('focus', handler, true),
121-
handler => element.removeEventListener('focus', handler, true),
122-
)
140+
this._observableFromEvent<FocusEvent>(element, 'focus', {capture: true})
123141
.pipe(toClosest(ROW_SELECTOR), share(), takeUntil(this.destroyed))
124142
.subscribe(this.editEventDispatcher.focused);
125143

126144
merge(
127-
fromEventPattern<FocusEvent>(
128-
handler => element.addEventListener('blur', handler, true),
129-
handler => element.removeEventListener('blur', handler, true),
145+
this._observableFromEvent(element, 'blur', {capture: true}),
146+
this._observableFromEvent<KeyboardEvent>(element, 'keydown').pipe(
147+
filter(event => event.key === 'Escape'),
130148
),
131-
fromEvent<KeyboardEvent>(element, 'keydown').pipe(filter(event => event.key === 'Escape')),
132149
)
133150
.pipe(mapTo(null), share(), takeUntil(this.destroyed))
134151
.subscribe(this.editEventDispatcher.focused);
@@ -150,7 +167,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
150167
)
151168
.subscribe(this.editEventDispatcher.allRows);
152169

153-
fromEvent<KeyboardEvent>(element, 'keydown')
170+
this._observableFromEvent<KeyboardEvent>(element, 'keydown')
154171
.pipe(
155172
filter(event => event.key === 'Enter'),
156173
toClosest(CELL_SELECTOR),
@@ -159,7 +176,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
159176
.subscribe(this.editEventDispatcher.editing);
160177

161178
// Keydown must be used here or else key auto-repeat does not work properly on some platforms.
162-
fromEvent<KeyboardEvent>(element, 'keydown')
179+
this._observableFromEvent<KeyboardEvent>(element, 'keydown')
163180
.pipe(takeUntil(this.destroyed))
164181
.subscribe(this.focusDispatcher.keyObserver);
165182
});

src/cdk/a11y/input-modality/input-modality-detector.ts

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,15 @@
77
*/
88

99
import {ALT, CONTROL, MAC_META, META, SHIFT} from '@angular/cdk/keycodes';
10-
import {Injectable, InjectionToken, OnDestroy, NgZone, inject} from '@angular/core';
11-
import {normalizePassiveListenerOptions, Platform, _getEventTarget} from '@angular/cdk/platform';
10+
import {
11+
Injectable,
12+
InjectionToken,
13+
OnDestroy,
14+
NgZone,
15+
inject,
16+
RendererFactory2,
17+
} from '@angular/core';
18+
import {Platform, _bindEventWithOptions, _getEventTarget} from '@angular/cdk/platform';
1219
import {DOCUMENT} from '@angular/common';
1320
import {BehaviorSubject, Observable} from 'rxjs';
1421
import {distinctUntilChanged, skip} from 'rxjs/operators';
@@ -69,10 +76,10 @@ export const TOUCH_BUFFER_MS = 650;
6976
* Event listener options that enable capturing and also mark the listener as passive if the browser
7077
* supports it.
7178
*/
72-
const modalityEventListenerOptions = normalizePassiveListenerOptions({
79+
const modalityEventListenerOptions = {
7380
passive: true,
7481
capture: true,
75-
});
82+
};
7683

7784
/**
7885
* Service that detects the user's input modality.
@@ -91,6 +98,7 @@ const modalityEventListenerOptions = normalizePassiveListenerOptions({
9198
@Injectable({providedIn: 'root'})
9299
export class InputModalityDetector implements OnDestroy {
93100
private readonly _platform = inject(Platform);
101+
private readonly _listenerCleanups: (() => void)[] | undefined;
94102

95103
/** Emits whenever an input modality is detected. */
96104
readonly modalityDetected: Observable<InputModality>;
@@ -193,21 +201,38 @@ export class InputModalityDetector implements OnDestroy {
193201
// If we're not in a browser, this service should do nothing, as there's no relevant input
194202
// modality to detect.
195203
if (this._platform.isBrowser) {
196-
ngZone.runOutsideAngular(() => {
197-
document.addEventListener('keydown', this._onKeydown, modalityEventListenerOptions);
198-
document.addEventListener('mousedown', this._onMousedown, modalityEventListenerOptions);
199-
document.addEventListener('touchstart', this._onTouchstart, modalityEventListenerOptions);
204+
const renderer = inject(RendererFactory2).createRenderer(null, null);
205+
206+
this._listenerCleanups = ngZone.runOutsideAngular(() => {
207+
return [
208+
_bindEventWithOptions(
209+
renderer,
210+
document,
211+
'keydown',
212+
this._onKeydown,
213+
modalityEventListenerOptions,
214+
),
215+
_bindEventWithOptions(
216+
renderer,
217+
document,
218+
'mousedown',
219+
this._onMousedown,
220+
modalityEventListenerOptions,
221+
),
222+
_bindEventWithOptions(
223+
renderer,
224+
document,
225+
'touchstart',
226+
this._onTouchstart,
227+
modalityEventListenerOptions,
228+
),
229+
];
200230
});
201231
}
202232
}
203233

204234
ngOnDestroy() {
205235
this._modality.complete();
206-
207-
if (this._platform.isBrowser) {
208-
document.removeEventListener('keydown', this._onKeydown, modalityEventListenerOptions);
209-
document.removeEventListener('mousedown', this._onMousedown, modalityEventListenerOptions);
210-
document.removeEventListener('touchstart', this._onTouchstart, modalityEventListenerOptions);
211-
}
236+
this._listenerCleanups?.forEach(cleanup => cleanup());
212237
}
213238
}

src/cdk/drag-drop/drag-drop-registry.ts

Lines changed: 56 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,33 @@ import {
1010
ChangeDetectionStrategy,
1111
Component,
1212
Injectable,
13+
ListenerOptions,
1314
NgZone,
1415
OnDestroy,
16+
RendererFactory2,
1517
ViewEncapsulation,
1618
WritableSignal,
1719
inject,
1820
signal,
1921
} from '@angular/core';
2022
import {DOCUMENT} from '@angular/common';
21-
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
23+
import {_bindEventWithOptions} from '@angular/cdk/platform';
2224
import {_CdkPrivateStyleLoader} from '@angular/cdk/private';
2325
import {Observable, Observer, Subject, merge} from 'rxjs';
2426
import type {DropListRef} from './drop-list-ref';
2527
import type {DragRef} from './drag-ref';
2628
import type {CdkDrag} from './directives/drag';
2729

30+
/** Event options that can be used to bind a capturing event. */
31+
const capturingEventOptions = {
32+
capture: true,
33+
};
34+
2835
/** Event options that can be used to bind an active, capturing event. */
29-
const activeCapturingEventOptions = normalizePassiveListenerOptions({
36+
const activeCapturingEventOptions = {
3037
passive: false,
3138
capture: true,
32-
});
39+
};
3340

3441
/**
3542
* Component used to load the drag&drop reset styles.
@@ -55,6 +62,8 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
5562
private _ngZone = inject(NgZone);
5663
private _document = inject(DOCUMENT);
5764
private _styleLoader = inject(_CdkPrivateStyleLoader);
65+
private _renderer = inject(RendererFactory2).createRenderer(null, null);
66+
private _cleanupDocumentTouchmove: (() => void) | undefined;
5867

5968
/** Registered drop container instances. */
6069
private _dropInstances = new Set<DropListRef>();
@@ -66,13 +75,7 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
6675
private _activeDragInstances: WritableSignal<DragRef[]> = signal([]);
6776

6877
/** Keeps track of the event listeners that we've bound to the `document`. */
69-
private _globalListeners = new Map<
70-
string,
71-
{
72-
handler: (event: Event) => void;
73-
options?: AddEventListenerOptions | boolean;
74-
}
75-
>();
78+
private _globalListeners: (() => void)[] | undefined;
7679

7780
/**
7881
* Predicate function to check if an item is being dragged. Moved out into a property,
@@ -127,7 +130,10 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
127130
this._ngZone.runOutsideAngular(() => {
128131
// The event handler has to be explicitly active,
129132
// because newer browsers make it passive by default.
130-
this._document.addEventListener(
133+
this._cleanupDocumentTouchmove?.();
134+
this._cleanupDocumentTouchmove = _bindEventWithOptions(
135+
this._renderer,
136+
this._document,
131137
'touchmove',
132138
this._persistentTouchmoveListener,
133139
activeCapturingEventOptions,
@@ -147,11 +153,7 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
147153
this.stopDragging(drag);
148154

149155
if (this._dragInstances.size === 0) {
150-
this._document.removeEventListener(
151-
'touchmove',
152-
this._persistentTouchmoveListener,
153-
activeCapturingEventOptions,
154-
);
156+
this._cleanupDocumentTouchmove?.();
155157
}
156158
}
157159

@@ -174,47 +176,43 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
174176
// passive ones for `mousemove` and `touchmove`. The events need to be active, because we
175177
// use `preventDefault` to prevent the page from scrolling while the user is dragging.
176178
const isTouchEvent = event.type.startsWith('touch');
177-
const endEventHandler = {
178-
handler: (e: Event) => this.pointerUp.next(e as TouchEvent | MouseEvent),
179-
options: true,
180-
};
179+
const endEventHandler = (e: Event) => this.pointerUp.next(e as TouchEvent | MouseEvent);
181180

182-
if (isTouchEvent) {
183-
this._globalListeners.set('touchend', endEventHandler);
184-
this._globalListeners.set('touchcancel', endEventHandler);
185-
} else {
186-
this._globalListeners.set('mouseup', endEventHandler);
187-
}
181+
const toBind: [name: string, handler: (event: Event) => void, options: ListenerOptions][] = [
182+
// Use capturing so that we pick up scroll changes in any scrollable nodes that aren't
183+
// the document. See https://github.com/angular/components/issues/17144.
184+
['scroll', (e: Event) => this.scroll.next(e), capturingEventOptions],
188185

189-
this._globalListeners
190-
.set('scroll', {
191-
handler: (e: Event) => this.scroll.next(e),
192-
// Use capturing so that we pick up scroll changes in any scrollable nodes that aren't
193-
// the document. See https://github.com/angular/components/issues/17144.
194-
options: true,
195-
})
196186
// Preventing the default action on `mousemove` isn't enough to disable text selection
197187
// on Safari so we need to prevent the selection event as well. Alternatively this can
198188
// be done by setting `user-select: none` on the `body`, however it has causes a style
199189
// recalculation which can be expensive on pages with a lot of elements.
200-
.set('selectstart', {
201-
handler: this._preventDefaultWhileDragging,
202-
options: activeCapturingEventOptions,
203-
});
190+
['selectstart', this._preventDefaultWhileDragging, activeCapturingEventOptions],
191+
];
192+
193+
if (isTouchEvent) {
194+
toBind.push(
195+
['touchend', endEventHandler, capturingEventOptions],
196+
['touchcancel', endEventHandler, capturingEventOptions],
197+
);
198+
} else {
199+
toBind.push(['mouseup', endEventHandler, capturingEventOptions]);
200+
}
204201

205202
// We don't have to bind a move event for touch drag sequences, because
206203
// we already have a persistent global one bound from `registerDragItem`.
207204
if (!isTouchEvent) {
208-
this._globalListeners.set('mousemove', {
209-
handler: (e: Event) => this.pointerMove.next(e as MouseEvent),
210-
options: activeCapturingEventOptions,
211-
});
205+
toBind.push([
206+
'mousemove',
207+
(e: Event) => this.pointerMove.next(e as MouseEvent),
208+
activeCapturingEventOptions,
209+
]);
212210
}
213211

214212
this._ngZone.runOutsideAngular(() => {
215-
this._globalListeners.forEach((config, name) => {
216-
this._document.addEventListener(name, config.handler, config.options);
217-
});
213+
this._globalListeners = toBind.map(([name, handler, options]) =>
214+
_bindEventWithOptions(this._renderer, this._document, name, handler, options),
215+
);
218216
});
219217
}
220218
}
@@ -257,17 +255,20 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
257255
streams.push(
258256
new Observable((observer: Observer<Event>) => {
259257
return this._ngZone.runOutsideAngular(() => {
260-
const eventOptions = true;
261-
const callback = (event: Event) => {
262-
if (this._activeDragInstances().length) {
263-
observer.next(event);
264-
}
265-
};
266-
267-
(shadowRoot as ShadowRoot).addEventListener('scroll', callback, eventOptions);
258+
const cleanup = _bindEventWithOptions(
259+
this._renderer,
260+
shadowRoot as ShadowRoot,
261+
'scroll',
262+
(event: Event) => {
263+
if (this._activeDragInstances().length) {
264+
observer.next(event);
265+
}
266+
},
267+
capturingEventOptions,
268+
);
268269

269270
return () => {
270-
(shadowRoot as ShadowRoot).removeEventListener('scroll', callback, eventOptions);
271+
cleanup();
271272
};
272273
});
273274
}),
@@ -338,10 +339,7 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
338339

339340
/** Clears out the global event listeners from the `document`. */
340341
private _clearGlobalListeners() {
341-
this._globalListeners.forEach((config, name) => {
342-
this._document.removeEventListener(name, config.handler, config.options);
343-
});
344-
345-
this._globalListeners.clear();
342+
this._globalListeners?.forEach(cleanup => cleanup());
343+
this._globalListeners = undefined;
346344
}
347345
}

0 commit comments

Comments
 (0)