Skip to content

Commit

Permalink
refactor(multiple): use renderer for manually-bound events with optio…
Browse files Browse the repository at this point in the history 
…ns (#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.
  • Loading branch information
@crisbeto
crisbeto authored Jan 22, 2025
1 parent 961ff25 commit 9ad4855
Show file tree
Hide file tree
Showing 10 changed files with 325 additions and 222 deletions.
45 changes: 31 additions & 14 deletions src/cdk-experimental/popover-edit/table-directives.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ import {
TemplateRef,
ViewContainerRef,
inject,
Renderer2,
ListenerOptions,
} from '@angular/core';
import {fromEvent, fromEventPattern, merge, Subject} from 'rxjs';
import {merge, Observable, Subject} from 'rxjs';
import {
debounceTime,
filter,
Expand All @@ -44,6 +46,7 @@ import {
} from './focus-escape-notifier';
import {closest} from './polyfill';
import {EditRef} from './edit-ref';
import {_bindEventWithOptions} from '@angular/cdk/platform';

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

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

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

private _observableFromEvent<T extends Event>(
element: Element,
name: string,
options?: ListenerOptions,
) {
return new Observable<T>(subscriber => {
const handler = (event: T) => subscriber.next(event);
const cleanup = options
? _bindEventWithOptions(this._renderer, element, name, handler, options)
: this._renderer.listen(element, name, handler, options);
return () => {
cleanup();
subscriber.complete();
};
});
}

private _listenForTableEvents(): void {
const element = this.elementRef.nativeElement;
const toClosest = (selector: string) =>
map((event: UIEvent) => closest(event.target, selector));

this.ngZone.runOutsideAngular(() => {
// Track mouse movement over the table to hide/show hover content.
fromEvent<MouseEvent>(element, 'mouseover')
this._observableFromEvent<MouseEvent>(element, 'mouseover')
.pipe(toClosest(ROW_SELECTOR), takeUntil(this.destroyed))
.subscribe(this.editEventDispatcher.hovering);
fromEvent<MouseEvent>(element, 'mouseleave')
this._observableFromEvent<MouseEvent>(element, 'mouseleave')
.pipe(mapTo(null), takeUntil(this.destroyed))
.subscribe(this.editEventDispatcher.hovering);
fromEvent<MouseEvent>(element, 'mousemove')
this._observableFromEvent<MouseEvent>(element, 'mousemove')
.pipe(
throttleTime(MOUSE_MOVE_THROTTLE_TIME_MS),
toClosest(ROW_SELECTOR),
Expand All @@ -116,19 +137,15 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
.subscribe(this.editEventDispatcher.mouseMove);

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

merge(
fromEventPattern<FocusEvent>(
handler => element.addEventListener('blur', handler, true),
handler => element.removeEventListener('blur', handler, true),
this._observableFromEvent(element, 'blur', {capture: true}),
this._observableFromEvent<KeyboardEvent>(element, 'keydown').pipe(
filter(event => event.key === 'Escape'),
),
fromEvent<KeyboardEvent>(element, 'keydown').pipe(filter(event => event.key === 'Escape')),
)
.pipe(mapTo(null), share(), takeUntil(this.destroyed))
.subscribe(this.editEventDispatcher.focused);
Expand All @@ -150,7 +167,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
)
.subscribe(this.editEventDispatcher.allRows);

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

// Keydown must be used here or else key auto-repeat does not work properly on some platforms.
fromEvent<KeyboardEvent>(element, 'keydown')
this._observableFromEvent<KeyboardEvent>(element, 'keydown')
.pipe(takeUntil(this.destroyed))
.subscribe(this.focusDispatcher.keyObserver);
});
Expand Down
53 changes: 39 additions & 14 deletions src/cdk/a11y/input-modality/input-modality-detector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@
*/

import {ALT, CONTROL, MAC_META, META, SHIFT} from '@angular/cdk/keycodes';
import {Injectable, InjectionToken, OnDestroy, NgZone, inject} from '@angular/core';
import {normalizePassiveListenerOptions, Platform, _getEventTarget} from '@angular/cdk/platform';
import {
Injectable,
InjectionToken,
OnDestroy,
NgZone,
inject,
RendererFactory2,
} from '@angular/core';
import {Platform, _bindEventWithOptions, _getEventTarget} from '@angular/cdk/platform';
import {DOCUMENT} from '@angular/common';
import {BehaviorSubject, Observable} from 'rxjs';
import {distinctUntilChanged, skip} from 'rxjs/operators';
Expand Down Expand Up @@ -69,10 +76,10 @@ export const TOUCH_BUFFER_MS = 650;
* Event listener options that enable capturing and also mark the listener as passive if the browser
* supports it.
*/
const modalityEventListenerOptions = normalizePassiveListenerOptions({
const modalityEventListenerOptions = {
passive: true,
capture: true,
});
};

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

/** Emits whenever an input modality is detected. */
readonly modalityDetected: Observable<InputModality>;
Expand Down Expand Up @@ -193,21 +201,38 @@ export class InputModalityDetector implements OnDestroy {
// If we're not in a browser, this service should do nothing, as there's no relevant input
// modality to detect.
if (this._platform.isBrowser) {
ngZone.runOutsideAngular(() => {
document.addEventListener('keydown', this._onKeydown, modalityEventListenerOptions);
document.addEventListener('mousedown', this._onMousedown, modalityEventListenerOptions);
document.addEventListener('touchstart', this._onTouchstart, modalityEventListenerOptions);
const renderer = inject(RendererFactory2).createRenderer(null, null);

this._listenerCleanups = ngZone.runOutsideAngular(() => {
return [
_bindEventWithOptions(
renderer,
document,
'keydown',
this._onKeydown,
modalityEventListenerOptions,
),
_bindEventWithOptions(
renderer,
document,
'mousedown',
this._onMousedown,
modalityEventListenerOptions,
),
_bindEventWithOptions(
renderer,
document,
'touchstart',
this._onTouchstart,
modalityEventListenerOptions,
),
];
});
}
}

ngOnDestroy() {
this._modality.complete();

if (this._platform.isBrowser) {
document.removeEventListener('keydown', this._onKeydown, modalityEventListenerOptions);
document.removeEventListener('mousedown', this._onMousedown, modalityEventListenerOptions);
document.removeEventListener('touchstart', this._onTouchstart, modalityEventListenerOptions);
}
this._listenerCleanups?.forEach(cleanup => cleanup());
}
}
114 changes: 56 additions & 58 deletions src/cdk/drag-drop/drag-drop-registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,33 @@ import {
ChangeDetectionStrategy,
Component,
Injectable,
ListenerOptions,
NgZone,
OnDestroy,
RendererFactory2,
ViewEncapsulation,
WritableSignal,
inject,
signal,
} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
import {_bindEventWithOptions} from '@angular/cdk/platform';
import {_CdkPrivateStyleLoader} from '@angular/cdk/private';
import {Observable, Observer, Subject, merge} from 'rxjs';
import type {DropListRef} from './drop-list-ref';
import type {DragRef} from './drag-ref';
import type {CdkDrag} from './directives/drag';

/** Event options that can be used to bind a capturing event. */
const capturingEventOptions = {
capture: true,
};

/** Event options that can be used to bind an active, capturing event. */
const activeCapturingEventOptions = normalizePassiveListenerOptions({
const activeCapturingEventOptions = {
passive: false,
capture: true,
});
};

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

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

/** Keeps track of the event listeners that we've bound to the `document`. */
private _globalListeners = new Map<
string,
{
handler: (event: Event) => void;
options?: AddEventListenerOptions | boolean;
}
>();
private _globalListeners: (() => void)[] | undefined;

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

if (this._dragInstances.size === 0) {
this._document.removeEventListener(
'touchmove',
this._persistentTouchmoveListener,
activeCapturingEventOptions,
);
this._cleanupDocumentTouchmove?.();
}
}

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

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

this._globalListeners
.set('scroll', {
handler: (e: Event) => this.scroll.next(e),
// Use capturing so that we pick up scroll changes in any scrollable nodes that aren't
// the document. See https://github.com/angular/components/issues/17144.
options: true,
})
// Preventing the default action on `mousemove` isn't enough to disable text selection
// on Safari so we need to prevent the selection event as well. Alternatively this can
// be done by setting `user-select: none` on the `body`, however it has causes a style
// recalculation which can be expensive on pages with a lot of elements.
.set('selectstart', {
handler: this._preventDefaultWhileDragging,
options: activeCapturingEventOptions,
});
['selectstart', this._preventDefaultWhileDragging, activeCapturingEventOptions],
];

if (isTouchEvent) {
toBind.push(
['touchend', endEventHandler, capturingEventOptions],
['touchcancel', endEventHandler, capturingEventOptions],
);
} else {
toBind.push(['mouseup', endEventHandler, capturingEventOptions]);
}

// We don't have to bind a move event for touch drag sequences, because
// we already have a persistent global one bound from `registerDragItem`.
if (!isTouchEvent) {
this._globalListeners.set('mousemove', {
handler: (e: Event) => this.pointerMove.next(e as MouseEvent),
options: activeCapturingEventOptions,
});
toBind.push([
'mousemove',
(e: Event) => this.pointerMove.next(e as MouseEvent),
activeCapturingEventOptions,
]);
}

this._ngZone.runOutsideAngular(() => {
this._globalListeners.forEach((config, name) => {
this._document.addEventListener(name, config.handler, config.options);
});
this._globalListeners = toBind.map(([name, handler, options]) =>
_bindEventWithOptions(this._renderer, this._document, name, handler, options),
);
});
}
}
Expand Down Expand Up @@ -257,17 +255,20 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {
streams.push(
new Observable((observer: Observer<Event>) => {
return this._ngZone.runOutsideAngular(() => {
const eventOptions = true;
const callback = (event: Event) => {
if (this._activeDragInstances().length) {
observer.next(event);
}
};

(shadowRoot as ShadowRoot).addEventListener('scroll', callback, eventOptions);
const cleanup = _bindEventWithOptions(
this._renderer,
shadowRoot as ShadowRoot,
'scroll',
(event: Event) => {
if (this._activeDragInstances().length) {
observer.next(event);
}
},
capturingEventOptions,
);

return () => {
(shadowRoot as ShadowRoot).removeEventListener('scroll', callback, eventOptions);
cleanup();
};
});
}),
Expand Down Expand Up @@ -338,10 +339,7 @@ export class DragDropRegistry<_ = unknown, __ = unknown> implements OnDestroy {

/** Clears out the global event listeners from the `document`. */
private _clearGlobalListeners() {
this._globalListeners.forEach((config, name) => {
this._document.removeEventListener(name, config.handler, config.options);
});

this._globalListeners.clear();
this._globalListeners?.forEach(cleanup => cleanup());
this._globalListeners = undefined;
}
}
Loading

0 comments on commit 9ad4855

Please sign in to comment.