From d323672b292699aefb5fb692e841818366ff9dcc Mon Sep 17 00:00:00 2001 From: Jamie Birch <14055146+shirakaba@users.noreply.github.com> Date: Tue, 7 May 2024 09:50:01 +0900 Subject: [PATCH] fix(core): handle GestureObservers same as event listeners (#10538) --- packages/core/ui/core/view/index.d.ts | 2 +- packages/core/ui/core/view/view-common.ts | 41 +++++++++++++++++--- packages/core/ui/gestures/gestures-common.ts | 19 +-------- 3 files changed, 39 insertions(+), 23 deletions(-) diff --git a/packages/core/ui/core/view/index.d.ts b/packages/core/ui/core/view/index.d.ts index adcea91b23..be75016f7f 100644 --- a/packages/core/ui/core/view/index.d.ts +++ b/packages/core/ui/core/view/index.d.ts @@ -587,7 +587,7 @@ export abstract class View extends ViewCommon { */ public focus(): boolean; - public getGestureObservers(type: GestureTypes): Array; + public getGestureObservers(type: GestureTypes): Array | undefined; /** * Removes listener(s) for the specified event name. diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts index f792d92e2d..77b6079902 100644 --- a/packages/core/ui/core/view/view-common.ts +++ b/packages/core/ui/core/view/view-common.ts @@ -278,7 +278,14 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } } - _observe(type: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void { + protected _observe(type: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void { + thisArg = thisArg || undefined; + + if (this._gestureObservers[type]?.find((observer) => observer.callback === callback && observer.context === thisArg)) { + // Already added. + return; + } + if (!this._gestureObservers[type]) { this._gestureObservers[type] = []; } @@ -291,6 +298,8 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any) { + thisArg = thisArg || undefined; + // Normalize "ontap" -> "tap" const normalizedName = getEventOrGestureName(eventNames); @@ -300,7 +309,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { // If it's a gesture (and this Observable declares e.g. `static tapEvent`) if (gesture && !this._isEvent(normalizedName)) { - this._observe(gesture, callback as unknown as (data: GestureEventData) => void, thisArg); + this._observe(gesture, callback, thisArg); return; } @@ -308,6 +317,8 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } public removeEventListener(eventNames: string, callback?: (data: EventData) => void, thisArg?: any) { + thisArg = thisArg || undefined; + // Normalize "ontap" -> "tap" const normalizedName = getEventOrGestureName(eventNames); @@ -317,7 +328,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { // If it's a gesture (and this Observable declares e.g. `static tapEvent`) if (gesture && !this._isEvent(normalizedName)) { - this._disconnectGestureObservers(gesture); + this._disconnectGestureObservers(gesture, callback, thisArg); return; } @@ -479,14 +490,34 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { return this.constructor && `${name}Event` in this.constructor; } - private _disconnectGestureObservers(type: GestureTypes): void { + private _disconnectGestureObservers(type: GestureTypes, callback?: (data: EventData) => void, thisArg?: any): void { + // Largely mirrors the implementation of Observable.innerRemoveEventListener(). + const observers = this.getGestureObservers(type); if (!observers) { return; } - for (const observer of observers) { + for (let i = 0; i < observers.length; i++) { + const observer = observers[i]; + + // If we have a `thisArg`, refine on both `callback` and `thisArg`. + if (thisArg && (observer.callback !== callback || observer.context !== thisArg)) { + continue; + } + + // If we don't have a `thisArg`, refine only on `callback`. + if (callback && observer.callback !== callback) { + continue; + } + observer.disconnect(); + observers.splice(i, 1); + i--; + } + + if (!observers.length) { + delete this._gestureObservers[type]; } } diff --git a/packages/core/ui/gestures/gestures-common.ts b/packages/core/ui/gestures/gestures-common.ts index 744bafd8d8..2a42e782c1 100644 --- a/packages/core/ui/gestures/gestures-common.ts +++ b/packages/core/ui/gestures/gestures-common.ts @@ -338,7 +338,7 @@ export function fromString(type: string): GestureTypes | undefined { export abstract class GesturesObserverBase implements GesturesObserverDefinition { private _callback: (args: GestureEventData) => void; private _target: View; - private _context: any; + private _context?: any; public type: GestureTypes; @@ -354,7 +354,7 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition return this._context; } - constructor(target: View, callback: (args: GestureEventData) => void, context: any) { + constructor(target: View, callback: (args: GestureEventData) => void, context?: any) { this._target = target; this._callback = callback; this._context = context; @@ -364,21 +364,6 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition public abstract observe(type: GestureTypes); public disconnect() { - // remove gesture observer from map - if (this.target) { - const list = this.target.getGestureObservers(this.type); - if (list && list.length > 0) { - for (let i = 0; i < list.length; i++) { - if (list[i].callback === this.callback) { - break; - } - } - list.length = 0; - - this.target._gestureObservers[this.type] = undefined; - delete this.target._gestureObservers[this.type]; - } - } this._target = null; this._callback = null; this._context = null;