From 923133f88d91dc253e2c61aa7d259b4439747994 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Wed, 1 May 2024 16:31:34 +0900 Subject: [PATCH 1/3] chore(core): clean up addEventListener() and removeEventListener() in ViewCommon --- packages/core/ui/core/view/view-common.ts | 88 +++++++++++------------ 1 file changed, 42 insertions(+), 46 deletions(-) diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts index f03025fe84..753465022f 100644 --- a/packages/core/ui/core/view/view-common.ts +++ b/packages/core/ui/core/view/view-common.ts @@ -33,6 +33,8 @@ import { SharedTransition, SharedTransitionInteractiveOptions } from '../../tran // helpers (these are okay re-exported here) export * from './view-helper'; +const eventNamesRegex = /\s*,\s*/; + let animationModule: typeof am; function ensureAnimationModule() { if (!animationModule) { @@ -291,56 +293,50 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } public addEventListener(arg: string | GestureTypes, callback: (data: EventData) => void, thisArg?: any) { - if (typeof arg === 'string') { - arg = getEventOrGestureName(arg); + if (typeof arg === 'number') { + this._observe(arg, callback as unknown as (data: GestureEventData) => void, thisArg); + return; + } - const gesture = gestureFromString(arg); - if (gesture && !this._isEvent(arg)) { - this._observe(gesture, callback as unknown as (data: GestureEventData) => void, thisArg); - } else { - const events = arg.split(','); - if (events.length > 0) { - for (let i = 0; i < events.length; i++) { - const evt = events[i].trim(); - const gst = gestureFromString(evt); - if (gst && !this._isEvent(arg)) { - this._observe(gst, callback as unknown as (data: GestureEventData) => void, thisArg); - } else { - super.addEventListener(evt, callback, thisArg); - } - } - } else { - super.addEventListener(arg, callback, thisArg); - } - } - } else if (typeof arg === 'number') { - this._observe(arg, callback as unknown as (data: GestureEventData) => void, thisArg); + // Normalize "ontap" -> "tap" + const normalizedName = getEventOrGestureName(arg); + + // Coerce "tap" -> GestureTypes.tap + // Coerce "loaded" -> undefined + const gesture: GestureTypes | undefined = gestureFromString(normalizedName); + + // 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); + return; + } + + for (const eventName of normalizedName.trim().split(eventNamesRegex)) { + super.addEventListener(eventName, callback, thisArg); } } public removeEventListener(arg: string | GestureTypes, callback?: (data: EventData) => void, thisArg?: any) { - if (typeof arg === 'string') { - const gesture = gestureFromString(arg); - if (gesture && !this._isEvent(arg)) { - this._disconnectGestureObservers(gesture); - } else { - const events = arg.split(','); - if (events.length > 0) { - for (let i = 0; i < events.length; i++) { - const evt = events[i].trim(); - const gst = gestureFromString(evt); - if (gst && !this._isEvent(arg)) { - this._disconnectGestureObservers(gst); - } else { - super.removeEventListener(evt, callback, thisArg); - } - } - } else { - super.removeEventListener(arg, callback, thisArg); - } - } - } else if (typeof arg === 'number') { - this._disconnectGestureObservers(arg); + if (typeof arg === 'number') { + this._disconnectGestureObservers(arg); + return; + } + + // Normalize "ontap" -> "tap" + const normalizedName = getEventOrGestureName(arg); + + // Coerce "tap" -> GestureTypes.tap + // Coerce "loaded" -> undefined + const gesture: GestureTypes | undefined = gestureFromString(normalizedName); + + // If it's a gesture (and this Observable declares e.g. `static tapEvent`) + if (gesture && !this._isEvent(normalizedName)) { + this._disconnectGestureObservers(gesture); + return; + } + + for (const eventName of normalizedName.trim().split(eventNamesRegex)) { + super.removeEventListener(eventName, callback, thisArg); } } @@ -379,7 +375,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { const firstArgument = args[0]; const view = firstArgument instanceof ViewCommon ? firstArgument : Builder.createViewFromEntry({ moduleName: firstArgument, - }); + }); return { view, options }; } From b489207a59912fe9d3db52b2be1c8e32c11c1520 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Wed, 1 May 2024 16:51:24 +0900 Subject: [PATCH 2/3] chore(core): clean up _gestureObservers --- packages/core/ui/core/view/view-common.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts index 753465022f..22bca371a2 100644 --- a/packages/core/ui/core/view/view-common.ts +++ b/packages/core/ui/core/view/view-common.ts @@ -125,7 +125,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { _setMinWidthNative: (value: CoreTypes.LengthType) => void; _setMinHeightNative: (value: CoreTypes.LengthType) => void; - public _gestureObservers = {}; + public readonly _gestureObservers = {} as Record>; _androidContentDescriptionUpdated?: boolean; @@ -179,7 +179,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { onLoaded() { if (!this.isLoaded) { - const hasTap = this.hasListeners('tap') || this.hasListeners('tapChange') || this.getGestureObservers(GestureTypes.tap); + const hasTap = this.hasListeners('tap') || this.hasListeners('tapChange') || !!this.getGestureObservers(GestureTypes.tap); const enableTapAnimations = TouchManager.enableGlobalTapAnimations && hasTap; if (!this.ignoreTouchAnimation && (this.touchAnimation || enableTapAnimations)) { TouchManager.addAnimations(this); @@ -288,7 +288,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { this._gestureObservers[type].push(gestureObserve(this, type, callback, thisArg)); } - public getGestureObservers(type: GestureTypes): Array { + public getGestureObservers(type: GestureTypes): Array | undefined { return this._gestureObservers[type]; } @@ -497,10 +497,12 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { private _disconnectGestureObservers(type: GestureTypes): void { const observers = this.getGestureObservers(type); - if (observers) { - for (let i = 0; i < observers.length; i++) { - observers[i].disconnect(); - } + if (!observers) { + return; + } + + for (const observer of observers) { + observer.disconnect(); } } From 63115923b8e56bf04480c2035db805f4c22a00be Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Thu, 2 May 2024 08:33:41 +0900 Subject: [PATCH 3/3] fix(core): remove redundant event name splitting ViewCommon splits event names redundantly (the superclass, Observable, splits event names too). --- packages/core/ui/core/view/view-common.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts index 22bca371a2..8fd11d1f13 100644 --- a/packages/core/ui/core/view/view-common.ts +++ b/packages/core/ui/core/view/view-common.ts @@ -33,8 +33,6 @@ import { SharedTransition, SharedTransitionInteractiveOptions } from '../../tran // helpers (these are okay re-exported here) export * from './view-helper'; -const eventNamesRegex = /\s*,\s*/; - let animationModule: typeof am; function ensureAnimationModule() { if (!animationModule) { @@ -311,9 +309,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { return; } - for (const eventName of normalizedName.trim().split(eventNamesRegex)) { - super.addEventListener(eventName, callback, thisArg); - } + super.addEventListener(normalizedName, callback, thisArg); } public removeEventListener(arg: string | GestureTypes, callback?: (data: EventData) => void, thisArg?: any) { @@ -335,9 +331,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { return; } - for (const eventName of normalizedName.trim().split(eventNamesRegex)) { - super.removeEventListener(eventName, callback, thisArg); - } + super.removeEventListener(normalizedName, callback, thisArg); } public onBackPressed(): boolean {