Skip to content

Commit

Permalink
refactor(multiple): use renderer for manual event bindings (#30179)
Browse files Browse the repository at this point in the history
Switches our manual calls into `addEventListener` to use the renderer instead so that tooling (e.g. the tracing service) gets notified about them.

Note that these changes only include the events that don't pass event options. The remaining events will be changed in a follow-up, because they require us to update to the latest `next` version of Angular.

(cherry picked from commit 04e5e0f)
  • Loading branch information
crisbeto committed Dec 13, 2024
1 parent c3f22f3 commit eef0536
Show file tree
Hide file tree
Showing 26 changed files with 249 additions and 209 deletions.
10 changes: 6 additions & 4 deletions src/cdk/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
Injector,
NgZone,
OnDestroy,
Renderer2,
ViewChild,
ViewEncapsulation,
afterNextRender,
Expand Down Expand Up @@ -79,6 +80,7 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
protected _ngZone = inject(NgZone);
private _overlayRef = inject(OverlayRef);
private _focusMonitor = inject(FocusMonitor);
private _renderer = inject(Renderer2);

private _platform = inject(Platform);
protected _document = inject(DOCUMENT, {optional: true})!;
Expand Down Expand Up @@ -223,13 +225,13 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
// The tabindex attribute should be removed to avoid navigating to that element again
this._ngZone.runOutsideAngular(() => {
const callback = () => {
element.removeEventListener('blur', callback);
element.removeEventListener('mousedown', callback);
deregisterBlur();
deregisterMousedown();
element.removeAttribute('tabindex');
};

element.addEventListener('blur', callback);
element.addEventListener('mousedown', callback);
const deregisterBlur = this._renderer.listen(element, 'blur', callback);
const deregisterMousedown = this._renderer.listen(element, 'mousedown', callback);
});
}
element.focus(options);
Expand Down
4 changes: 3 additions & 1 deletion src/cdk/drag-drop/drag-drop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {Injectable, NgZone, ElementRef, inject} from '@angular/core';
import {Injectable, NgZone, ElementRef, inject, RendererFactory2} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {DragRef, DragRefConfig} from './drag-ref';
Expand All @@ -28,6 +28,7 @@ export class DragDrop {
private _ngZone = inject(NgZone);
private _viewportRuler = inject(ViewportRuler);
private _dragDropRegistry = inject(DragDropRegistry);
private _renderer = inject(RendererFactory2).createRenderer(null, null);

constructor(...args: unknown[]);
constructor() {}
Expand All @@ -48,6 +49,7 @@ export class DragDrop {
this._ngZone,
this._viewportRuler,
this._dragDropRegistry,
this._renderer,
);
}

Expand Down
11 changes: 7 additions & 4 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
ElementRef,
EmbeddedViewRef,
NgZone,
Renderer2,
TemplateRef,
ViewContainerRef,
signal,
Expand Down Expand Up @@ -378,6 +379,7 @@ export class DragRef<T = any> {
private _ngZone: NgZone,
private _viewportRuler: ViewportRuler,
private _dragDropRegistry: DragDropRegistry,
private _renderer: Renderer2,
) {
this.withRootElement(element).withParent(_config.parentDragRef || null);
this._parentPositions = new ParentPositionTracker(_document);
Expand Down Expand Up @@ -853,6 +855,7 @@ export class DragRef<T = any> {
this._pickupPositionOnPage,
this._initialTransform,
this._config.zIndex || 1000,
this._renderer,
);
this._preview.attach(this._getPreviewInsertionPoint(parent, shadowRoot));

Expand Down Expand Up @@ -1106,24 +1109,24 @@ export class DragRef<T = any> {

return this._ngZone.runOutsideAngular(() => {
return new Promise(resolve => {
const handler = ((event: TransitionEvent) => {
const handler = (event: TransitionEvent) => {
if (
!event ||
(this._preview &&
_getEventTarget(event) === this._preview.element &&
event.propertyName === 'transform')
) {
this._preview?.removeEventListener('transitionend', handler);
cleanupListener();
resolve();
clearTimeout(timeout);
}
}) as EventListenerOrEventListenerObject;
};

// If a transition is short enough, the browser might not fire the `transitionend` event.
// Since we know how long it's supposed to take, add a timeout with a 50% buffer that'll
// fire if the transition hasn't completed when it was supposed to.
const timeout = setTimeout(handler as Function, duration * 1.5);
this._preview!.addEventListener('transitionend', handler);
const cleanupListener = this._preview!.addEventListener('transitionend', handler);
});
});
}
Expand Down
11 changes: 4 additions & 7 deletions src/cdk/drag-drop/preview-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {EmbeddedViewRef, TemplateRef, ViewContainerRef} from '@angular/core';
import {EmbeddedViewRef, Renderer2, TemplateRef, ViewContainerRef} from '@angular/core';
import {Direction} from '@angular/cdk/bidi';
import {
extendStyles,
Expand Down Expand Up @@ -56,6 +56,7 @@ export class PreviewRef {
},
private _initialTransform: string | null,
private _zIndex: number,
private _renderer: Renderer2,
) {}

attach(parent: HTMLElement): void {
Expand Down Expand Up @@ -91,12 +92,8 @@ export class PreviewRef {
return getTransformTransitionDurationInMs(this._preview);
}

addEventListener(name: string, handler: EventListenerOrEventListenerObject) {
this._preview.addEventListener(name, handler);
}

removeEventListener(name: string, handler: EventListenerOrEventListenerObject) {
this._preview.removeEventListener(name, handler);
addEventListener(name: string, handler: (event: any) => void): () => void {
return this._renderer.listen(this._preview, name, handler);
}

private _createPreview(): HTMLElement {
Expand Down
15 changes: 10 additions & 5 deletions src/cdk/observers/private/shared-resize-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/
import {inject, Injectable, NgZone, OnDestroy} from '@angular/core';
import {inject, Injectable, NgZone, OnDestroy, RendererFactory2} from '@angular/core';
import {Observable, Subject} from 'rxjs';
import {filter, shareReplay, takeUntil} from 'rxjs/operators';

Expand Down Expand Up @@ -98,6 +98,8 @@ class SingleBoxSharedResizeObserver {
providedIn: 'root',
})
export class SharedResizeObserver implements OnDestroy {
private _cleanupErrorListener: (() => void) | undefined;

/** Map of box type to shared resize observer. */
private _observers = new Map<ResizeObserverBoxOptions, SingleBoxSharedResizeObserver>();

Expand All @@ -107,7 +109,12 @@ export class SharedResizeObserver implements OnDestroy {
constructor() {
if (typeof ResizeObserver !== 'undefined' && (typeof ngDevMode === 'undefined' || ngDevMode)) {
this._ngZone.runOutsideAngular(() => {
window.addEventListener('error', loopLimitExceededErrorHandler);
const renderer = inject(RendererFactory2).createRenderer(null, null);
this._cleanupErrorListener = renderer.listen(
'window',
'error',
loopLimitExceededErrorHandler,
);
});
}
}
Expand All @@ -117,9 +124,7 @@ export class SharedResizeObserver implements OnDestroy {
observer.destroy();
}
this._observers.clear();
if (typeof ResizeObserver !== 'undefined' && (typeof ngDevMode === 'undefined' || ngDevMode)) {
window.removeEventListener('error', loopLimitExceededErrorHandler);
}
this._cleanupErrorListener?.();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,17 @@ describe('OverlayKeyboardDispatcher', () => {
it('should dispose of the global keyboard event handler correctly', () => {
const overlayRef = overlay.create();
const body = document.body;

spyOn(body, 'addEventListener');
spyOn(body, 'removeEventListener');

keyboardDispatcher.add(overlayRef);
expect(body.addEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function));
expect(body.addEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function), false);

overlayRef.dispose();
expect(body.removeEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function));
expect(document.body.removeEventListener).toHaveBeenCalledWith(
'keydown',
jasmine.any(Function),
);
});

it('should skip overlays that do not have keydown event subscriptions', () => {
Expand Down
28 changes: 10 additions & 18 deletions src/cdk/overlay/dispatchers/overlay-keyboard-dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {Injectable, NgZone, inject} from '@angular/core';
import {Injectable, NgZone, RendererFactory2, inject} from '@angular/core';
import {BaseOverlayDispatcher} from './base-overlay-dispatcher';
import type {OverlayRef} from '../overlay-ref';

Expand All @@ -17,30 +17,28 @@ import type {OverlayRef} from '../overlay-ref';
*/
@Injectable({providedIn: 'root'})
export class OverlayKeyboardDispatcher extends BaseOverlayDispatcher {
private _ngZone = inject(NgZone, {optional: true});
private _ngZone = inject(NgZone);
private _renderer = inject(RendererFactory2).createRenderer(null, null);
private _cleanupKeydown: (() => void) | undefined;

/** Add a new overlay to the list of attached overlay refs. */
override add(overlayRef: OverlayRef): void {
super.add(overlayRef);

// Lazily start dispatcher once first overlay is added
if (!this._isAttached) {
/** @breaking-change 14.0.0 _ngZone will be required. */
if (this._ngZone) {
this._ngZone.runOutsideAngular(() =>
this._document.body.addEventListener('keydown', this._keydownListener),
);
} else {
this._document.body.addEventListener('keydown', this._keydownListener);
}
this._ngZone.runOutsideAngular(() => {
this._cleanupKeydown = this._renderer.listen('body', 'keydown', this._keydownListener);
});

this._isAttached = true;
}
}

/** Detaches the global keyboard event listener. */
protected detach() {
if (this._isAttached) {
this._document.body.removeEventListener('keydown', this._keydownListener);
this._cleanupKeydown?.();
this._isAttached = false;
}
}
Expand All @@ -57,13 +55,7 @@ export class OverlayKeyboardDispatcher extends BaseOverlayDispatcher {
// because we don't want overlays that don't handle keyboard events to block the ones below
// them that do.
if (overlays[i]._keydownEvents.observers.length > 0) {
const keydownEvents = overlays[i]._keydownEvents;
/** @breaking-change 14.0.0 _ngZone will be required. */
if (this._ngZone) {
this._ngZone.run(() => keydownEvents.next(event));
} else {
keydownEvents.next(event);
}
this._ngZone.run(() => overlays[i]._keydownEvents.next(event));
break;
}
}
Expand Down
40 changes: 15 additions & 25 deletions src/cdk/overlay/fullscreen-overlay-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {Injectable, OnDestroy} from '@angular/core';
import {inject, Injectable, OnDestroy, RendererFactory2} from '@angular/core';
import {OverlayContainer} from './overlay-container';

/**
Expand All @@ -18,8 +18,9 @@ import {OverlayContainer} from './overlay-container';
*/
@Injectable({providedIn: 'root'})
export class FullscreenOverlayContainer extends OverlayContainer implements OnDestroy {
private _renderer = inject(RendererFactory2).createRenderer(null, null);
private _fullScreenEventName: string | undefined;
private _fullScreenListener: () => void;
private _cleanupFullScreenListener: (() => void) | undefined;

constructor(...args: unknown[]);

Expand All @@ -29,38 +30,27 @@ export class FullscreenOverlayContainer extends OverlayContainer implements OnDe

override ngOnDestroy() {
super.ngOnDestroy();

if (this._fullScreenEventName && this._fullScreenListener) {
this._document.removeEventListener(this._fullScreenEventName, this._fullScreenListener);
}
this._cleanupFullScreenListener?.();
}

protected override _createContainer(): void {
const eventName = this._getEventName();
super._createContainer();
this._adjustParentForFullscreenChange();
this._addFullscreenChangeListener(() => this._adjustParentForFullscreenChange());
}

private _adjustParentForFullscreenChange(): void {
if (!this._containerElement) {
return;
if (eventName) {
this._cleanupFullScreenListener?.();
this._cleanupFullScreenListener = this._renderer.listen('document', eventName, () => {
this._adjustParentForFullscreenChange();
});
}

const fullscreenElement = this.getFullscreenElement();
const parent = fullscreenElement || this._document.body;
parent.appendChild(this._containerElement);
}

private _addFullscreenChangeListener(fn: () => void) {
const eventName = this._getEventName();

if (eventName) {
if (this._fullScreenListener) {
this._document.removeEventListener(eventName, this._fullScreenListener);
}

this._document.addEventListener(eventName, fn);
this._fullScreenListener = fn;
private _adjustParentForFullscreenChange(): void {
if (this._containerElement) {
const fullscreenElement = this.getFullscreenElement();
const parent = fullscreenElement || this._document.body;
parent.appendChild(this._containerElement);
}
}

Expand Down
Loading

0 comments on commit eef0536

Please sign in to comment.