From 2c78a870424c82a22fc3ef98f49476d52c9ab6a3 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sat, 4 May 2024 14:42:16 +0900 Subject: [PATCH 1/8] chore(core): drop support for plural events --- packages/core/data/observable/index.ts | 134 ++++++++++++------------- 1 file changed, 62 insertions(+), 72 deletions(-) diff --git a/packages/core/data/observable/index.ts b/packages/core/data/observable/index.ts index 53d801e172..54aa92f9a0 100644 --- a/packages/core/data/observable/index.ts +++ b/packages/core/data/observable/index.ts @@ -153,74 +153,72 @@ export class Observable { /** * A basic method signature to hook an event listener (shortcut alias to the addEventListener method). - * @param eventNames - String corresponding to events (e.g. "propertyChange"). Optionally could be used more events separated by `,` (e.g. "propertyChange", "change"). + * @param eventName Name of the event to attach to. * @param callback - Callback function which will be executed when event is raised. * @param thisArg - An optional parameter which will be used as `this` context for callback execution. */ - public on(eventNames: string, callback: (data: EventData) => void, thisArg?: any): void { - this.addEventListener(eventNames, callback, thisArg); + public on(eventName: string, callback: (data: EventData) => void, thisArg?: any): void { + this.addEventListener(eventName, callback, thisArg); } /** * Adds one-time listener function for the event named `event`. - * @param event Name of the event to attach to. + * @param eventName Name of the event to attach to. * @param callback A function to be called when the specified event is raised. * @param thisArg An optional parameter which when set will be used as "this" in callback method call. */ - public once(event: string, callback: (data: EventData) => void, thisArg?: any): void { - this.addEventListener(event, callback, thisArg, true); + public once(eventName: string, callback: (data: EventData) => void, thisArg?: any): void { + this.addEventListener(eventName, callback, thisArg, true); } /** * Shortcut alias to the removeEventListener method. */ - public off(eventNames: string, callback?: (data: EventData) => void, thisArg?: any): void { - this.removeEventListener(eventNames, callback, thisArg); + public off(eventName: string, callback?: (data: EventData) => void, thisArg?: any): void { + this.removeEventListener(eventName, callback, thisArg); } /** * Adds a listener for the specified event name. - * @param eventNames Comma delimited names of the events to attach the listener to. + * @param eventName Name of the event to attach to. * @param callback A function to be called when some of the specified event(s) is raised. * @param thisArg An optional parameter which when set will be used as "this" in callback method call. */ - public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void { + public addEventListener(eventName: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void { once = once || undefined; thisArg = thisArg || undefined; - if (typeof eventNames !== 'string') { - throw new TypeError('Event name(s) must be a string.'); + if (typeof eventName !== 'string') { + throw new TypeError('Event name must be a string.'); } if (typeof callback !== 'function') { throw new TypeError('Callback, if provided, must be a function.'); } - for (const eventName of eventNames.trim().split(eventNamesRegex)) { - const list = this._getEventList(eventName, true); - if (Observable._indexOfListener(list, callback, thisArg) !== -1) { - // Already added. - continue; - } - - list.push({ - callback, - thisArg, - once, - }); + const list = this._getEventList(eventName, true); + if (Observable._indexOfListener(list, callback, thisArg) !== -1) { + // Already added. + return; } + + list.push({ + callback, + thisArg, + once, + }); } /** * Removes listener(s) for the specified event name. - * @param eventNames Comma delimited names of the events the specified listener is associated with. + * @param eventName Name of the event to attach to. * @param callback An optional parameter pointing to a specific listener. If not defined, all listeners for the event names will be removed. * @param thisArg An optional parameter which when set will be used to refine search of the correct callback which will be removed as event listener. */ - public removeEventListener(eventNames: string, callback?: (data: EventData) => void, thisArg?: any): void { + public removeEventListener(eventName: string, callback?: (data: EventData) => void, thisArg?: any): void { thisArg = thisArg || undefined; - if (typeof eventNames !== 'string') { + if (typeof eventName !== 'string') { throw new TypeError('Events name(s) must be string.'); } @@ -228,18 +226,16 @@ export class Observable { throw new TypeError('callback must be function.'); } - for (const eventName of eventNames.trim().split(eventNamesRegex)) { - const entries = this._observers[eventName]; - if (!entries) { - continue; - } + const entries = this._observers[eventName]; + if (!entries) { + return; + } - Observable.innerRemoveEventListener(entries, callback, thisArg); + Observable.innerRemoveEventListener(entries, callback, thisArg); - if (!entries.length) { - // Clear all entries of this type - delete this._observers[eventName]; - } + if (!entries.length) { + // Clear all entries of this type + delete this._observers[eventName]; } } @@ -297,11 +293,11 @@ export class Observable { * in future. * @deprecated */ - public static removeEventListener(eventNames: string, callback?: (data: EventData) => void, thisArg?: any): void { + public static removeEventListener(eventName: string, callback?: (data: EventData) => void, thisArg?: any): void { thisArg = thisArg || undefined; - if (typeof eventNames !== 'string') { - throw new TypeError('Event name(s) must be a string.'); + if (typeof eventName !== 'string') { + throw new TypeError('Event name must be a string.'); } if (callback && typeof callback !== 'function') { @@ -310,24 +306,22 @@ export class Observable { const eventClass = this.name === 'Observable' ? '*' : this.name; - for (const eventName of eventNames.trim().split(eventNamesRegex)) { - const entries = _globalEventHandlers?.[eventClass]?.[eventName]; - if (!entries) { - continue; - } + const entries = _globalEventHandlers?.[eventClass]?.[eventName]; + if (!entries) { + return; + } - Observable.innerRemoveEventListener(entries, callback, thisArg); + Observable.innerRemoveEventListener(entries, callback, thisArg); - if (!entries.length) { - // Clear all entries of this type - delete _globalEventHandlers[eventClass][eventName]; - } + if (!entries.length) { + // Clear all entries of this type + delete _globalEventHandlers[eventClass][eventName]; + } - // Clear the primary class grouping if no list are left - const keys = Object.keys(_globalEventHandlers[eventClass]); - if (keys.length === 0) { - delete _globalEventHandlers[eventClass]; - } + // Clear the primary class grouping if no list are left + const keys = Object.keys(_globalEventHandlers[eventClass]); + if (keys.length === 0) { + delete _globalEventHandlers[eventClass]; } } @@ -336,12 +330,12 @@ export class Observable { * in future. * @deprecated */ - public static addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void { + public static addEventListener(eventName: string, callback: (data: EventData) => void, thisArg?: any, once?: boolean): void { once = once || undefined; thisArg = thisArg || undefined; - if (typeof eventNames !== 'string') { - throw new TypeError('Event name(s) must be a string.'); + if (typeof eventName !== 'string') { + throw new TypeError('Event name must be a string.'); } if (typeof callback !== 'function') { @@ -353,17 +347,15 @@ export class Observable { _globalEventHandlers[eventClass] = {}; } - for (const eventName of eventNames.trim().split(eventNamesRegex)) { - if (!_globalEventHandlers[eventClass][eventName]) { - _globalEventHandlers[eventClass][eventName] = []; - } - if (Observable._indexOfListener(_globalEventHandlers[eventClass][eventName], callback, thisArg) !== -1) { - // Already added. - return; - } - - _globalEventHandlers[eventClass][eventName].push({ callback, thisArg, once }); + if (!_globalEventHandlers[eventClass][eventName]) { + _globalEventHandlers[eventClass][eventName] = []; + } + if (Observable._indexOfListener(_globalEventHandlers[eventClass][eventName], callback, thisArg) !== -1) { + // Already added. + return; } + + _globalEventHandlers[eventClass][eventName].push({ callback, thisArg, once }); } private _globalNotify(eventClass: string, eventType: string, data: T): void { @@ -464,10 +456,8 @@ export class Observable { }; } - public _emit(eventNames: string): void { - for (const eventName of eventNames.trim().split(eventNamesRegex)) { - this.notify({ eventName, object: this }); - } + public _emit(eventName: string): void { + this.notify({ eventName, object: this }); } private _getEventList(eventName: string, createIfNeeded?: boolean): Array | undefined { From ba5893c742b493d63ea862b93c5d1ba0b211ef7a Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sat, 4 May 2024 14:51:58 +0900 Subject: [PATCH 2/8] chore(core): indicate all plural gesture types # Conflicts: # packages/core/ui/core/view/view-common.ts --- packages/core/ui/core/view/view-common.ts | 34 +++-- packages/core/ui/gestures/gestures-common.ts | 27 ++-- packages/core/ui/gestures/index.android.ts | 73 +++++---- packages/core/ui/gestures/index.ios.ts | 153 ++++++++++--------- 4 files changed, 159 insertions(+), 128 deletions(-) diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts index 77b6079902..ab609e9cb1 100644 --- a/packages/core/ui/core/view/view-common.ts +++ b/packages/core/ui/core/view/view-common.ts @@ -278,23 +278,25 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } } - protected _observe(type: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void { + protected _observe(pluralType: GestureTypes, callback: (args: GestureEventData) => void, thisArg?: any): void { thisArg = thisArg || undefined; - if (this._gestureObservers[type]?.find((observer) => observer.callback === callback && observer.context === thisArg)) { + if (this._gestureObservers[pluralType]?.find((observer) => observer.callback === callback && observer.context === thisArg)) { // Already added. return; } - if (!this._gestureObservers[type]) { - this._gestureObservers[type] = []; + if (!this._gestureObservers[pluralType]) { + this._gestureObservers[pluralType] = []; } - this._gestureObservers[type].push(gestureObserve(this, type, callback, thisArg)); + this._gestureObservers[pluralType].push(gestureObserve(this, pluralType, callback, thisArg)); } - public getGestureObservers(type: GestureTypes): Array | undefined { - return this._gestureObservers[type]; + // Although this method accepts a plural type, in practice, the only case in + // which it is used searches for a singular type. + public getGestureObservers(pluralType: GestureTypes): Array | undefined { + return this._gestureObservers[pluralType]; } public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any) { @@ -305,11 +307,11 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { // Coerce "tap" -> GestureTypes.tap // Coerce "loaded" -> undefined - const gesture: GestureTypes | undefined = gestureFromString(normalizedName); + const pluralGestureType: 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, thisArg); + if (pluralGestureType && !this._isEvent(normalizedName)) { + this._observe(pluralGestureType, callback, thisArg); return; } @@ -324,11 +326,11 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { // Coerce "tap" -> GestureTypes.tap // Coerce "loaded" -> undefined - const gesture: GestureTypes | undefined = gestureFromString(normalizedName); + const pluralGestureType: 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, callback, thisArg); + if (pluralGestureType && !this._isEvent(normalizedName)) { + this._disconnectGestureObservers(pluralGestureType, callback, thisArg); return; } @@ -490,10 +492,10 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { return this.constructor && `${name}Event` in this.constructor; } - private _disconnectGestureObservers(type: GestureTypes, callback?: (data: EventData) => void, thisArg?: any): void { + private _disconnectGestureObservers(pluralType: GestureTypes, callback?: (data: EventData) => void, thisArg?: any): void { // Largely mirrors the implementation of Observable.innerRemoveEventListener(). - const observers = this.getGestureObservers(type); + const observers = this.getGestureObservers(pluralType); if (!observers) { return; } @@ -517,7 +519,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } if (!observers.length) { - delete this._gestureObservers[type]; + delete this._gestureObservers[pluralType]; } } diff --git a/packages/core/ui/gestures/gestures-common.ts b/packages/core/ui/gestures/gestures-common.ts index 2a42e782c1..af58c7b49c 100644 --- a/packages/core/ui/gestures/gestures-common.ts +++ b/packages/core/ui/gestures/gestures-common.ts @@ -280,45 +280,45 @@ export interface RotationGestureEventData extends GestureEventDataWithState { /** * Returns a string representation of a gesture type. - * @param type - Type of the gesture. + * @param pluralType - Type of the gesture. * @param separator(optional) - Text separator between gesture type strings. */ -export function toString(type: GestureTypes, separator?: string): string { +export function toString(pluralType: GestureTypes, separator?: string): string { // We can get stronger typings with `keyof typeof GestureTypes`, but sadly // indexing into an enum simply returns `string`, so we'd have to type-assert // all of the below anyway. Even this `(typeof GestureTypes)[GestureTypes]` is // more for documentation than for type-safety (it resolves to `string`, too). const types = new Array<(typeof GestureTypes)[GestureTypes]>(); - if (type & GestureTypes.tap) { + if (pluralType & GestureTypes.tap) { types.push(GestureTypes[GestureTypes.tap]); } - if (type & GestureTypes.doubleTap) { + if (pluralType & GestureTypes.doubleTap) { types.push(GestureTypes[GestureTypes.doubleTap]); } - if (type & GestureTypes.pinch) { + if (pluralType & GestureTypes.pinch) { types.push(GestureTypes[GestureTypes.pinch]); } - if (type & GestureTypes.pan) { + if (pluralType & GestureTypes.pan) { types.push(GestureTypes[GestureTypes.pan]); } - if (type & GestureTypes.swipe) { + if (pluralType & GestureTypes.swipe) { types.push(GestureTypes[GestureTypes.swipe]); } - if (type & GestureTypes.rotation) { + if (pluralType & GestureTypes.rotation) { types.push(GestureTypes[GestureTypes.rotation]); } - if (type & GestureTypes.longPress) { + if (pluralType & GestureTypes.longPress) { types.push(GestureTypes[GestureTypes.longPress]); } - if (type & GestureTypes.touch) { + if (pluralType & GestureTypes.touch) { types.push(GestureTypes[GestureTypes.touch]); } @@ -340,7 +340,10 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition private _target: View; private _context?: any; - public type: GestureTypes; + protected _pluralType: GestureTypes; + public get type(): GestureTypes { + return this._pluralType; + } public get callback(): (args: GestureEventData) => void { return this._callback; @@ -361,7 +364,7 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition } public abstract androidOnTouchEvent(motionEvent: android.view.MotionEvent); - public abstract observe(type: GestureTypes); + public abstract observe(pluralType: GestureTypes); public disconnect() { this._target = null; diff --git a/packages/core/ui/gestures/index.android.ts b/packages/core/ui/gestures/index.android.ts index 3d3e7e3e29..8ee5fd30b2 100644 --- a/packages/core/ui/gestures/index.android.ts +++ b/packages/core/ui/gestures/index.android.ts @@ -27,19 +27,19 @@ function initializeTapAndDoubleTapGestureListener() { class TapAndDoubleTapGestureListenerImpl extends android.view.GestureDetector.SimpleOnGestureListener { private _observer: GesturesObserver; private _target: View; - private _type: number; + private _pluralType: number; private _lastUpTime = 0; private _tapTimeoutId: number; private static DoubleTapTimeout = android.view.ViewConfiguration.getDoubleTapTimeout(); - constructor(observer: GesturesObserver, target: View, type: number) { + constructor(observer: GesturesObserver, target: View, pluralType: GestureTypes) { super(); this._observer = observer; this._target = target; - this._type = type; + this._pluralType = pluralType; return global.__native(this); } @@ -61,7 +61,7 @@ function initializeTapAndDoubleTapGestureListener() { } public onLongPress(motionEvent: android.view.MotionEvent): void { - if (this._type & GestureTypes.longPress) { + if (this._pluralType & GestureTypes.longPress) { const args = _getLongPressArgs(GestureTypes.longPress, this._target, GestureStateTypes.began, motionEvent); _executeCallback(this._observer, args); } @@ -70,14 +70,14 @@ function initializeTapAndDoubleTapGestureListener() { private _handleSingleTap(motionEvent: android.view.MotionEvent): void { if (this._target.getGestureObservers(GestureTypes.doubleTap)) { this._tapTimeoutId = timer.setTimeout(() => { - if (this._type & GestureTypes.tap) { + if (this._pluralType & GestureTypes.tap) { const args = _getTapArgs(GestureTypes.tap, this._target, motionEvent); _executeCallback(this._observer, args); } timer.clearTimeout(this._tapTimeoutId); }, TapAndDoubleTapGestureListenerImpl.DoubleTapTimeout); } else { - if (this._type & GestureTypes.tap) { + if (this._pluralType & GestureTypes.tap) { const args = _getTapArgs(GestureTypes.tap, this._target, motionEvent); _executeCallback(this._observer, args); } @@ -88,7 +88,7 @@ function initializeTapAndDoubleTapGestureListener() { if (this._tapTimeoutId) { timer.clearTimeout(this._tapTimeoutId); } - if (this._type & GestureTypes.doubleTap) { + if (this._pluralType & GestureTypes.doubleTap) { const args = _getTapArgs(GestureTypes.doubleTap, this._target, motionEvent); _executeCallback(this._observer, args); } @@ -251,13 +251,13 @@ export class GesturesObserver extends GesturesObserverBase { private _onTargetLoaded: (data: EventData) => void; private _onTargetUnloaded: (data: EventData) => void; - public observe(type: GestureTypes) { + public observe(pluralType: GestureTypes) { if (this.target) { - this.type = type; - this._onTargetLoaded = (args) => { - this._attach(this.target, type); + this._pluralType = pluralType; + this._onTargetLoaded = () => { + this._attach(this.target, pluralType); }; - this._onTargetUnloaded = (args) => { + this._onTargetUnloaded = () => { this._detach(); }; @@ -265,7 +265,7 @@ export class GesturesObserver extends GesturesObserverBase { this.target.on('unloaded', this._onTargetUnloaded); if (this.target.isLoaded) { - this._attach(this.target, type); + this._attach(this.target, pluralType); } } } @@ -294,41 +294,45 @@ export class GesturesObserver extends GesturesObserverBase { this._eventData = null; } - private _attach(target: View, type: GestureTypes) { + private _attach(target: View, pluralType: GestureTypes) { this._detach(); let recognizer; - if (type & GestureTypes.tap || type & GestureTypes.doubleTap || type & GestureTypes.longPress) { + // Whether it's a tap, doubleTap, or longPress, we handle with the same + // listener. It'll listen for all three of these gesture types, but only + // notify if the plural type it was registered with included the relevant + // gesture. + if (pluralType & GestureTypes.tap || pluralType & GestureTypes.doubleTap || pluralType & GestureTypes.longPress) { initializeTapAndDoubleTapGestureListener(); - recognizer = this._simpleGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new TapAndDoubleTapGestureListener(this, this.target, type)); + recognizer = this._simpleGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new TapAndDoubleTapGestureListener(this, this.target, pluralType)); } - if (type & GestureTypes.pinch) { + if (pluralType & GestureTypes.pinch) { initializePinchGestureListener(); recognizer = this._scaleGestureDetector = new android.view.ScaleGestureDetector(target._context, new PinchGestureListener(this, this.target)); } - if (type & GestureTypes.swipe) { + if (pluralType & GestureTypes.swipe) { initializeSwipeGestureListener(); recognizer = this._swipeGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new SwipeGestureListener(this, this.target)); } - if (type & GestureTypes.pan) { + if (pluralType & GestureTypes.pan) { recognizer = this._panGestureDetector = new CustomPanGestureDetector(this, this.target); } - if (type & GestureTypes.rotation) { + if (pluralType & GestureTypes.rotation) { recognizer = this._rotateGestureDetector = new CustomRotateGestureDetector(this, this.target); } - if (type & GestureTypes.touch) { + if (pluralType & GestureTypes.touch) { this._notifyTouch = true; } else { this.target.notify({ eventName: GestureEvents.gestureAttached, object: this.target, - type, + type: pluralType, view: this.target, android: recognizer, }); @@ -367,28 +371,28 @@ export class GesturesObserver extends GesturesObserverBase { } } -function _getTapArgs(type: GestureTypes, view: View, e: android.view.MotionEvent): TapGestureEventData { +function _getTapArgs(singularType: GestureTypes, view: View, e: android.view.MotionEvent): TapGestureEventData { return { - type: type, + type: singularType, view: view, android: e, ios: undefined, object: view, - eventName: toString(type), + eventName: toString(singularType), getPointerCount: () => e.getPointerCount(), getX: () => layout.toDeviceIndependentPixels(e.getX()), getY: () => layout.toDeviceIndependentPixels(e.getY()), }; } -function _getLongPressArgs(type: GestureTypes, view: View, state: GestureStateTypes, e: android.view.MotionEvent): GestureEventDataWithState { +function _getLongPressArgs(singularType: GestureTypes, view: View, state: GestureStateTypes, e: android.view.MotionEvent): GestureEventDataWithState { return { - type: type, + type: singularType, view: view, android: e, ios: undefined, object: view, - eventName: toString(type), + eventName: toString(singularType), state: state, }; } @@ -430,7 +434,13 @@ class PinchGestureEventData implements PinchGestureEventData { public eventName = toString(GestureTypes.pinch); public ios; - constructor(public view: View, public android: android.view.ScaleGestureDetector, public scale: number, public object: any, public state: GestureStateTypes) {} + constructor( + public view: View, + public android: android.view.ScaleGestureDetector, + public scale: number, + public object: any, + public state: GestureStateTypes, + ) {} getFocusX(): number { return this.android.getFocusX() / layout.getDisplayDensity(); @@ -669,7 +679,10 @@ class Pointer implements Pointer { public android: number; public ios: any = undefined; - constructor(id: number, private event: android.view.MotionEvent) { + constructor( + id: number, + private event: android.view.MotionEvent, + ) { this.android = id; } diff --git a/packages/core/ui/gestures/index.ios.ts b/packages/core/ui/gestures/index.ios.ts index 82b2aa804e..96c5aab55b 100644 --- a/packages/core/ui/gestures/index.ios.ts +++ b/packages/core/ui/gestures/index.ios.ts @@ -91,27 +91,29 @@ class UIGestureRecognizerImpl extends NSObject { } export class GesturesObserver extends GesturesObserverBase { - private _recognizers: {}; + private readonly _recognizers: { [singularGestureType: string]: RecognizerCache } = {}; private _onTargetLoaded: (data: EventData) => void; private _onTargetUnloaded: (data: EventData) => void; - constructor(target: View, callback: (args: GestureEventData) => void, context: any) { - super(target, callback, context); - this._recognizers = {}; - } - public androidOnTouchEvent(motionEvent: android.view.MotionEvent): void { // } - public observe(type: GestureTypes) { + /** + * Given a plural GestureTypes value, observes each of the constituent + * GestureTypes values. + * + * For example, if given the plural GestureTypes.tap & GestureTypes.doubleTap, + * would observe both "tap" and "doubleTap" gestures. + */ + public observe(pluralType: GestureTypes) { if (this.target) { - this.type = type; - this._onTargetLoaded = (args) => { - this._attach(this.target, type); + this._pluralType = pluralType; + this._onTargetLoaded = () => { + this._attach(this.target, pluralType); }; - this._onTargetUnloaded = (args) => { + this._onTargetUnloaded = () => { this._detach(); }; @@ -119,58 +121,67 @@ export class GesturesObserver extends GesturesObserverBase { this.target.on('unloaded', this._onTargetUnloaded); if (this.target.isLoaded) { - this._attach(this.target, type); + this._attach(this.target, pluralType); } } } - private _attach(target: View, type: GestureTypes) { + /** + * Given a plural GestureTypes value, adds a UIGestureRecognizer for each + * constituent GestureTypes value and populates a RecognizerCache entry in + * this._recognizers. + * + * For example, if given the plural GestureTypes.tap & GestureTypes.doubleTap, + * would add a gesture recognizer for both "tap" and "doubleTap" gestures, + * firing the same callback for each. + */ + private _attach(target: View, pluralType: GestureTypes) { this._detach(); - if (target && target.nativeViewProtected && target.nativeViewProtected.addGestureRecognizer) { + if (target?.nativeViewProtected?.addGestureRecognizer) { const nativeView = target.nativeViewProtected; - if (type & GestureTypes.tap) { + if (pluralType & GestureTypes.tap) { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.tap, (args) => { if (args.view) { this._executeCallback(_getTapData(args)); } - }) + }), ); } - if (type & GestureTypes.doubleTap) { + if (pluralType & GestureTypes.doubleTap) { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.doubleTap, (args) => { if (args.view) { this._executeCallback(_getTapData(args)); } - }) + }), ); } - if (type & GestureTypes.pinch) { + if (pluralType & GestureTypes.pinch) { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.pinch, (args) => { if (args.view) { this._executeCallback(_getPinchData(args)); } - }) + }), ); } - if (type & GestureTypes.pan) { + if (pluralType & GestureTypes.pan) { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.pan, (args) => { if (args.view) { this._executeCallback(_getPanData(args, target.nativeViewProtected)); } - }) + }), ); } - if (type & GestureTypes.swipe) { + if (pluralType & GestureTypes.swipe) { nativeView.addGestureRecognizer( this._createRecognizer( GestureTypes.swipe, @@ -179,8 +190,8 @@ export class GesturesObserver extends GesturesObserverBase { this._executeCallback(_getSwipeData(args)); } }, - UISwipeGestureRecognizerDirection.Down - ) + UISwipeGestureRecognizerDirection.Down, + ), ); nativeView.addGestureRecognizer( @@ -191,8 +202,8 @@ export class GesturesObserver extends GesturesObserverBase { this._executeCallback(_getSwipeData(args)); } }, - UISwipeGestureRecognizerDirection.Left - ) + UISwipeGestureRecognizerDirection.Left, + ), ); nativeView.addGestureRecognizer( @@ -203,8 +214,8 @@ export class GesturesObserver extends GesturesObserverBase { this._executeCallback(_getSwipeData(args)); } }, - UISwipeGestureRecognizerDirection.Right - ) + UISwipeGestureRecognizerDirection.Right, + ), ); nativeView.addGestureRecognizer( @@ -215,49 +226,45 @@ export class GesturesObserver extends GesturesObserverBase { this._executeCallback(_getSwipeData(args)); } }, - UISwipeGestureRecognizerDirection.Up - ) + UISwipeGestureRecognizerDirection.Up, + ), ); } - if (type & GestureTypes.rotation) { + if (pluralType & GestureTypes.rotation) { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.rotation, (args) => { if (args.view) { this._executeCallback(_getRotationData(args)); } - }) + }), ); } - if (type & GestureTypes.longPress) { + if (pluralType & GestureTypes.longPress) { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.longPress, (args) => { if (args.view) { this._executeCallback(_getLongPressData(args)); } - }) + }), ); } - if (type & GestureTypes.touch) { + if (pluralType & GestureTypes.touch) { nativeView.addGestureRecognizer(this._createRecognizer(GestureTypes.touch)); } } } private _detach() { - if (this.target && this.target.nativeViewProtected) { - for (const name in this._recognizers) { - if (this._recognizers.hasOwnProperty(name)) { - const item = this._recognizers[name]; - this.target.nativeViewProtected.removeGestureRecognizer(item.recognizer); - - item.recognizer = null; - item.target = null; - } - } - this._recognizers = {}; + for (const pluralGestureType in this._recognizers) { + const item = this._recognizers[pluralGestureType]; + this.target?.nativeViewProtected?.removeGestureRecognizer(item.recognizer); + + item.recognizer = null; + item.target = null; + delete this._recognizers[pluralGestureType]; } } @@ -281,36 +288,42 @@ export class GesturesObserver extends GesturesObserverBase { } } - private _createRecognizer(type: GestureTypes, callback?: (args: GestureEventData) => void, swipeDirection?: UISwipeGestureRecognizerDirection): UIGestureRecognizer { - let recognizer: UIGestureRecognizer; - let name = toString(type); - const target = _createUIGestureRecognizerTarget(this, type, callback, this.context); - const recognizerType = _getUIGestureRecognizerType(type); + /** + * Creates a UIGestureRecognizer (and populates a RecognizerCache entry in + * this._recognizers) corresponding to the singular GestureTypes value passed + * in. + */ + private _createRecognizer(singularType: GestureTypes, callback?: (args: GestureEventData) => void, swipeDirection?: UISwipeGestureRecognizerDirection): UIGestureRecognizer | undefined { + let recognizer: UIGestureRecognizer | undefined; + let singularTypeString = toString(singularType); + const target = _createUIGestureRecognizerTarget(this, singularType, callback, this.context); + const recognizerType = _getUIGestureRecognizerType(singularType); if (recognizerType) { recognizer = recognizerType.alloc().initWithTargetAction(target, 'recognize'); - if (type === GestureTypes.swipe && swipeDirection) { - name = name + swipeDirection.toString(); + if (singularType === GestureTypes.swipe && swipeDirection) { + // e.g. "swipe1" + singularTypeString += swipeDirection.toString(); (recognizer).direction = swipeDirection; - } else if (type === GestureTypes.touch) { + } else if (singularType === GestureTypes.touch) { (recognizer).observer = this; - } else if (type === GestureTypes.doubleTap) { + } else if (singularType === GestureTypes.doubleTap) { (recognizer).numberOfTapsRequired = 2; } if (recognizer) { recognizer.delegate = recognizerDelegateInstance; - this._recognizers[name] = { - recognizer: recognizer, - target: target, + this._recognizers[singularTypeString] = { + recognizer, + target, }; } this.target.notify({ eventName: GestureEvents.gestureAttached, object: this.target, - type, + type: singularType, view: this.target, ios: recognizer, }); @@ -329,24 +342,24 @@ interface RecognizerCache { target: any; } -function _getUIGestureRecognizerType(type: GestureTypes): any { - let nativeType = null; +function _getUIGestureRecognizerType(singularType: GestureTypes): typeof UIGestureRecognizer | null { + let nativeType: typeof UIGestureRecognizer | null = null; - if (type === GestureTypes.tap) { + if (singularType === GestureTypes.tap) { nativeType = UITapGestureRecognizer; - } else if (type === GestureTypes.doubleTap) { + } else if (singularType === GestureTypes.doubleTap) { nativeType = UITapGestureRecognizer; - } else if (type === GestureTypes.pinch) { + } else if (singularType === GestureTypes.pinch) { nativeType = UIPinchGestureRecognizer; - } else if (type === GestureTypes.pan) { + } else if (singularType === GestureTypes.pan) { nativeType = UIPanGestureRecognizer; - } else if (type === GestureTypes.swipe) { + } else if (singularType === GestureTypes.swipe) { nativeType = UISwipeGestureRecognizer; - } else if (type === GestureTypes.rotation) { + } else if (singularType === GestureTypes.rotation) { nativeType = UIRotationGestureRecognizer; - } else if (type === GestureTypes.longPress) { + } else if (singularType === GestureTypes.longPress) { nativeType = UILongPressGestureRecognizer; - } else if (type === GestureTypes.touch) { + } else if (singularType === GestureTypes.touch) { nativeType = TouchGestureRecognizer; } From 1f976fd286566603e56b18ca17e0a426da40547f Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sun, 5 May 2024 02:16:15 +0900 Subject: [PATCH 3/8] fix(core): drop (the already limited) support for plural gesture events --- packages/core/ui/core/view/view-common.ts | 34 ++-- packages/core/ui/gestures/gestures-common.ts | 72 ++++----- packages/core/ui/gestures/index.android.ts | 138 +++++++++-------- packages/core/ui/gestures/index.d.ts | 4 +- packages/core/ui/gestures/index.ios.ts | 154 ++++++++++--------- 5 files changed, 206 insertions(+), 196 deletions(-) diff --git a/packages/core/ui/core/view/view-common.ts b/packages/core/ui/core/view/view-common.ts index ab609e9cb1..6b68d42cc9 100644 --- a/packages/core/ui/core/view/view-common.ts +++ b/packages/core/ui/core/view/view-common.ts @@ -278,25 +278,23 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } } - protected _observe(pluralType: 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[pluralType]?.find((observer) => observer.callback === callback && observer.context === thisArg)) { + if (this._gestureObservers[type]?.find((observer) => observer.callback === callback && observer.context === thisArg)) { // Already added. return; } - if (!this._gestureObservers[pluralType]) { - this._gestureObservers[pluralType] = []; + if (!this._gestureObservers[type]) { + this._gestureObservers[type] = []; } - this._gestureObservers[pluralType].push(gestureObserve(this, pluralType, callback, thisArg)); + this._gestureObservers[type].push(gestureObserve(this, type, callback, thisArg)); } - // Although this method accepts a plural type, in practice, the only case in - // which it is used searches for a singular type. - public getGestureObservers(pluralType: GestureTypes): Array | undefined { - return this._gestureObservers[pluralType]; + public getGestureObservers(type: GestureTypes): Array | undefined { + return this._gestureObservers[type]; } public addEventListener(eventNames: string, callback: (data: EventData) => void, thisArg?: any) { @@ -307,11 +305,11 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { // Coerce "tap" -> GestureTypes.tap // Coerce "loaded" -> undefined - const pluralGestureType: GestureTypes | undefined = gestureFromString(normalizedName); + const gestureType: GestureTypes | undefined = gestureFromString(normalizedName); // If it's a gesture (and this Observable declares e.g. `static tapEvent`) - if (pluralGestureType && !this._isEvent(normalizedName)) { - this._observe(pluralGestureType, callback, thisArg); + if (gestureType && !this._isEvent(normalizedName)) { + this._observe(gestureType, callback, thisArg); return; } @@ -326,11 +324,11 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { // Coerce "tap" -> GestureTypes.tap // Coerce "loaded" -> undefined - const pluralGestureType: GestureTypes | undefined = gestureFromString(normalizedName); + const gestureType: GestureTypes | undefined = gestureFromString(normalizedName); // If it's a gesture (and this Observable declares e.g. `static tapEvent`) - if (pluralGestureType && !this._isEvent(normalizedName)) { - this._disconnectGestureObservers(pluralGestureType, callback, thisArg); + if (gestureType && !this._isEvent(normalizedName)) { + this._disconnectGestureObservers(gestureType, callback, thisArg); return; } @@ -492,10 +490,10 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { return this.constructor && `${name}Event` in this.constructor; } - private _disconnectGestureObservers(pluralType: GestureTypes, callback?: (data: EventData) => void, thisArg?: any): void { + private _disconnectGestureObservers(type: GestureTypes, callback?: (data: EventData) => void, thisArg?: any): void { // Largely mirrors the implementation of Observable.innerRemoveEventListener(). - const observers = this.getGestureObservers(pluralType); + const observers = this.getGestureObservers(type); if (!observers) { return; } @@ -519,7 +517,7 @@ export abstract class ViewCommon extends ViewBase implements ViewDefinition { } if (!observers.length) { - delete this._gestureObservers[pluralType]; + delete this._gestureObservers[type]; } } diff --git a/packages/core/ui/gestures/gestures-common.ts b/packages/core/ui/gestures/gestures-common.ts index af58c7b49c..177b0bc4be 100644 --- a/packages/core/ui/gestures/gestures-common.ts +++ b/packages/core/ui/gestures/gestures-common.ts @@ -280,59 +280,45 @@ export interface RotationGestureEventData extends GestureEventDataWithState { /** * Returns a string representation of a gesture type. - * @param pluralType - Type of the gesture. - * @param separator(optional) - Text separator between gesture type strings. + * @param type - The singular type of the gesture. Looks for an exact match, so + * passing plural types like `GestureTypes.tap & GestureTypes.doubleTap` will + * simply return undefined. */ -export function toString(pluralType: GestureTypes, separator?: string): string { - // We can get stronger typings with `keyof typeof GestureTypes`, but sadly - // indexing into an enum simply returns `string`, so we'd have to type-assert - // all of the below anyway. Even this `(typeof GestureTypes)[GestureTypes]` is - // more for documentation than for type-safety (it resolves to `string`, too). - const types = new Array<(typeof GestureTypes)[GestureTypes]>(); - - if (pluralType & GestureTypes.tap) { - types.push(GestureTypes[GestureTypes.tap]); - } +export function toString(type: GestureTypes): (typeof GestureTypes)[GestureTypes] | undefined { + switch (type) { + case GestureTypes.tap: + return GestureTypes[GestureTypes.tap]; - if (pluralType & GestureTypes.doubleTap) { - types.push(GestureTypes[GestureTypes.doubleTap]); - } + case GestureTypes.doubleTap: + return GestureTypes[GestureTypes.doubleTap]; - if (pluralType & GestureTypes.pinch) { - types.push(GestureTypes[GestureTypes.pinch]); - } + case GestureTypes.pinch: + return GestureTypes[GestureTypes.pinch]; - if (pluralType & GestureTypes.pan) { - types.push(GestureTypes[GestureTypes.pan]); - } + case GestureTypes.pan: + return GestureTypes[GestureTypes.pan]; - if (pluralType & GestureTypes.swipe) { - types.push(GestureTypes[GestureTypes.swipe]); - } + case GestureTypes.swipe: + return GestureTypes[GestureTypes.swipe]; - if (pluralType & GestureTypes.rotation) { - types.push(GestureTypes[GestureTypes.rotation]); - } + case GestureTypes.rotation: + return GestureTypes[GestureTypes.rotation]; - if (pluralType & GestureTypes.longPress) { - types.push(GestureTypes[GestureTypes.longPress]); - } + case GestureTypes.longPress: + return GestureTypes[GestureTypes.longPress]; - if (pluralType & GestureTypes.touch) { - types.push(GestureTypes[GestureTypes.touch]); + case GestureTypes.touch: + return GestureTypes[GestureTypes.touch]; } - - return types.join(separator); } -// NOTE: toString could return the text of multiple GestureTypes. -// Souldn't fromString do split on separator and return multiple GestureTypes? /** * Returns a gesture type enum value from a string (case insensitive). - * @param type - A string representation of a gesture type (e.g. Tap). + * + * @param type - A string representation of a single gesture type (e.g. "tap"). */ -export function fromString(type: string): GestureTypes | undefined { - return GestureTypes[type.trim()]; +export function fromString(type: (typeof GestureTypes)[GestureTypes]): GestureTypes | undefined { + return GestureTypes[type]; } export abstract class GesturesObserverBase implements GesturesObserverDefinition { @@ -340,10 +326,8 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition private _target: View; private _context?: any; - protected _pluralType: GestureTypes; - public get type(): GestureTypes { - return this._pluralType; - } + /** This is populated on the first call to observe(). */ + type: GestureTypes; public get callback(): (args: GestureEventData) => void { return this._callback; @@ -364,7 +348,7 @@ export abstract class GesturesObserverBase implements GesturesObserverDefinition } public abstract androidOnTouchEvent(motionEvent: android.view.MotionEvent); - public abstract observe(pluralType: GestureTypes); + public abstract observe(type: GestureTypes); public disconnect() { this._target = null; diff --git a/packages/core/ui/gestures/index.android.ts b/packages/core/ui/gestures/index.android.ts index 8ee5fd30b2..aef28d4218 100644 --- a/packages/core/ui/gestures/index.android.ts +++ b/packages/core/ui/gestures/index.android.ts @@ -27,19 +27,19 @@ function initializeTapAndDoubleTapGestureListener() { class TapAndDoubleTapGestureListenerImpl extends android.view.GestureDetector.SimpleOnGestureListener { private _observer: GesturesObserver; private _target: View; - private _pluralType: number; + private _type: GestureTypes; private _lastUpTime = 0; private _tapTimeoutId: number; private static DoubleTapTimeout = android.view.ViewConfiguration.getDoubleTapTimeout(); - constructor(observer: GesturesObserver, target: View, pluralType: GestureTypes) { + constructor(observer: GesturesObserver, target: View, type: GestureTypes) { super(); this._observer = observer; this._target = target; - this._pluralType = pluralType; + this._type = type; return global.__native(this); } @@ -61,7 +61,7 @@ function initializeTapAndDoubleTapGestureListener() { } public onLongPress(motionEvent: android.view.MotionEvent): void { - if (this._pluralType & GestureTypes.longPress) { + if (this._type === GestureTypes.longPress) { const args = _getLongPressArgs(GestureTypes.longPress, this._target, GestureStateTypes.began, motionEvent); _executeCallback(this._observer, args); } @@ -70,14 +70,14 @@ function initializeTapAndDoubleTapGestureListener() { private _handleSingleTap(motionEvent: android.view.MotionEvent): void { if (this._target.getGestureObservers(GestureTypes.doubleTap)) { this._tapTimeoutId = timer.setTimeout(() => { - if (this._pluralType & GestureTypes.tap) { + if (this._type === GestureTypes.tap) { const args = _getTapArgs(GestureTypes.tap, this._target, motionEvent); _executeCallback(this._observer, args); } timer.clearTimeout(this._tapTimeoutId); }, TapAndDoubleTapGestureListenerImpl.DoubleTapTimeout); } else { - if (this._pluralType & GestureTypes.tap) { + if (this._type === GestureTypes.tap) { const args = _getTapArgs(GestureTypes.tap, this._target, motionEvent); _executeCallback(this._observer, args); } @@ -88,7 +88,7 @@ function initializeTapAndDoubleTapGestureListener() { if (this._tapTimeoutId) { timer.clearTimeout(this._tapTimeoutId); } - if (this._pluralType & GestureTypes.doubleTap) { + if (this._type === GestureTypes.doubleTap) { const args = _getTapArgs(GestureTypes.doubleTap, this._target, motionEvent); _executeCallback(this._observer, args); } @@ -251,22 +251,25 @@ export class GesturesObserver extends GesturesObserverBase { private _onTargetLoaded: (data: EventData) => void; private _onTargetUnloaded: (data: EventData) => void; - public observe(pluralType: GestureTypes) { - if (this.target) { - this._pluralType = pluralType; - this._onTargetLoaded = () => { - this._attach(this.target, pluralType); - }; - this._onTargetUnloaded = () => { - this._detach(); - }; + public observe(type: GestureTypes) { + this.type = type; + + if (!this.target) { + return; + } - this.target.on('loaded', this._onTargetLoaded); - this.target.on('unloaded', this._onTargetUnloaded); + this._onTargetLoaded = () => { + this._attach(this.target, type); + }; + this._onTargetUnloaded = () => { + this._detach(); + }; - if (this.target.isLoaded) { - this._attach(this.target, pluralType); - } + this.target.on('loaded', this._onTargetLoaded); + this.target.on('unloaded', this._onTargetUnloaded); + + if (this.target.isLoaded) { + this._attach(this.target, type); } } @@ -280,6 +283,7 @@ export class GesturesObserver extends GesturesObserverBase { this._onTargetLoaded = null; this._onTargetUnloaded = null; } + // clears target, context and callback references super.disconnect(); } @@ -294,49 +298,59 @@ export class GesturesObserver extends GesturesObserverBase { this._eventData = null; } - private _attach(target: View, pluralType: GestureTypes) { + private _attach(target: View, type: GestureTypes) { this._detach(); - let recognizer; - - // Whether it's a tap, doubleTap, or longPress, we handle with the same - // listener. It'll listen for all three of these gesture types, but only - // notify if the plural type it was registered with included the relevant - // gesture. - if (pluralType & GestureTypes.tap || pluralType & GestureTypes.doubleTap || pluralType & GestureTypes.longPress) { - initializeTapAndDoubleTapGestureListener(); - recognizer = this._simpleGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new TapAndDoubleTapGestureListener(this, this.target, pluralType)); - } + let recognizer: unknown; + + switch (type) { + // Whether it's a tap, doubleTap, or longPress, we handle with the same + // listener. It'll listen for all three of these gesture types, but only + // notify if the type it was registered with matched the relevant gesture. + case GestureTypes.tap: + case GestureTypes.doubleTap: + case GestureTypes.longPress: { + initializeTapAndDoubleTapGestureListener(); + recognizer = this._simpleGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new TapAndDoubleTapGestureListener(this, this.target, type)); + break; + } + case GestureTypes.pinch: { + initializePinchGestureListener(); + recognizer = this._scaleGestureDetector = new android.view.ScaleGestureDetector(target._context, new PinchGestureListener(this, this.target)); + break; + } - if (pluralType & GestureTypes.pinch) { - initializePinchGestureListener(); - recognizer = this._scaleGestureDetector = new android.view.ScaleGestureDetector(target._context, new PinchGestureListener(this, this.target)); - } + case GestureTypes.swipe: { + initializeSwipeGestureListener(); + recognizer = this._swipeGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new SwipeGestureListener(this, this.target)); + break; + } - if (pluralType & GestureTypes.swipe) { - initializeSwipeGestureListener(); - recognizer = this._swipeGestureDetector = new androidx.core.view.GestureDetectorCompat(target._context, new SwipeGestureListener(this, this.target)); - } + case GestureTypes.pan: { + recognizer = this._panGestureDetector = new CustomPanGestureDetector(this, this.target); + break; + } - if (pluralType & GestureTypes.pan) { - recognizer = this._panGestureDetector = new CustomPanGestureDetector(this, this.target); - } + case GestureTypes.rotation: { + recognizer = this._rotateGestureDetector = new CustomRotateGestureDetector(this, this.target); + break; + } - if (pluralType & GestureTypes.rotation) { - recognizer = this._rotateGestureDetector = new CustomRotateGestureDetector(this, this.target); + case GestureTypes.touch: { + this._notifyTouch = true; + // For touch events, return early rather than breaking from the switch + // statement. + return; + } } - if (pluralType & GestureTypes.touch) { - this._notifyTouch = true; - } else { - this.target.notify({ - eventName: GestureEvents.gestureAttached, - object: this.target, - type: pluralType, - view: this.target, - android: recognizer, - }); - } + this.target.notify({ + eventName: GestureEvents.gestureAttached, + object: this.target, + type: type, + view: this.target, + android: recognizer, + }); } public androidOnTouchEvent(motionEvent: android.view.MotionEvent) { @@ -371,28 +385,28 @@ export class GesturesObserver extends GesturesObserverBase { } } -function _getTapArgs(singularType: GestureTypes, view: View, e: android.view.MotionEvent): TapGestureEventData { +function _getTapArgs(type: GestureTypes, view: View, e: android.view.MotionEvent): TapGestureEventData { return { - type: singularType, + type: type, view: view, android: e, ios: undefined, object: view, - eventName: toString(singularType), + eventName: toString(type), getPointerCount: () => e.getPointerCount(), getX: () => layout.toDeviceIndependentPixels(e.getX()), getY: () => layout.toDeviceIndependentPixels(e.getY()), }; } -function _getLongPressArgs(singularType: GestureTypes, view: View, state: GestureStateTypes, e: android.view.MotionEvent): GestureEventDataWithState { +function _getLongPressArgs(type: GestureTypes, view: View, state: GestureStateTypes, e: android.view.MotionEvent): GestureEventDataWithState { return { - type: singularType, + type: type, view: view, android: e, ios: undefined, object: view, - eventName: toString(singularType), + eventName: toString(type), state: state, }; } diff --git a/packages/core/ui/gestures/index.d.ts b/packages/core/ui/gestures/index.d.ts index 077473139f..81e13e7ae4 100644 --- a/packages/core/ui/gestures/index.d.ts +++ b/packages/core/ui/gestures/index.d.ts @@ -27,7 +27,9 @@ export class GesturesObserver { disconnect(); /** - * Gesture type attached to the observer. + * Singular gesture type (e.g. GestureTypes.tap) attached to the observer. + * Does not support plural gesture types (e.g. + * GestureTypes.tap & GestureTypes.doubleTap). */ type: GestureTypes; diff --git a/packages/core/ui/gestures/index.ios.ts b/packages/core/ui/gestures/index.ios.ts index 96c5aab55b..5a401cd0b6 100644 --- a/packages/core/ui/gestures/index.ios.ts +++ b/packages/core/ui/gestures/index.ios.ts @@ -91,7 +91,7 @@ class UIGestureRecognizerImpl extends NSObject { } export class GesturesObserver extends GesturesObserverBase { - private readonly _recognizers: { [singularGestureType: string]: RecognizerCache } = {}; + private readonly _recognizers: { [type: string]: RecognizerCache } = {}; private _onTargetLoaded: (data: EventData) => void; private _onTargetUnloaded: (data: EventData) => void; @@ -101,47 +101,51 @@ export class GesturesObserver extends GesturesObserverBase { } /** - * Given a plural GestureTypes value, observes each of the constituent - * GestureTypes values. + * Observes a singular GestureTypes value (e.g. GestureTypes.tap). * - * For example, if given the plural GestureTypes.tap & GestureTypes.doubleTap, - * would observe both "tap" and "doubleTap" gestures. + * Does not support observing plural GestureTypes values, e.g. + * GestureTypes.tap & GestureTypes.doubleTap. */ - public observe(pluralType: GestureTypes) { - if (this.target) { - this._pluralType = pluralType; - this._onTargetLoaded = () => { - this._attach(this.target, pluralType); - }; - this._onTargetUnloaded = () => { - this._detach(); - }; - - this.target.on('loaded', this._onTargetLoaded); - this.target.on('unloaded', this._onTargetUnloaded); - - if (this.target.isLoaded) { - this._attach(this.target, pluralType); - } + public observe(type: GestureTypes) { + this.type = type; + + if (!this.target) { + return; + } + + this._onTargetLoaded = () => { + this._attach(this.target, type); + }; + this._onTargetUnloaded = () => { + this._detach(); + }; + + this.target.on('loaded', this._onTargetLoaded); + this.target.on('unloaded', this._onTargetUnloaded); + + if (this.target.isLoaded) { + this._attach(this.target, type); } } /** - * Given a plural GestureTypes value, adds a UIGestureRecognizer for each - * constituent GestureTypes value and populates a RecognizerCache entry in + * Given a singular GestureTypes value (e.g. GestureTypes.tap), adds a + * UIGestureRecognizer for it and populates a RecognizerCache entry in * this._recognizers. * - * For example, if given the plural GestureTypes.tap & GestureTypes.doubleTap, - * would add a gesture recognizer for both "tap" and "doubleTap" gestures, - * firing the same callback for each. + * Does not support attaching plural GestureTypes values, e.g. + * GestureTypes.tap & GestureTypes.doubleTap. */ - private _attach(target: View, pluralType: GestureTypes) { + private _attach(target: View, type: GestureTypes) { this._detach(); - if (target?.nativeViewProtected?.addGestureRecognizer) { - const nativeView = target.nativeViewProtected; + const nativeView = target?.nativeViewProtected as UIView | undefined; + if (!nativeView?.addGestureRecognizer) { + return; + } - if (pluralType & GestureTypes.tap) { + switch (type) { + case GestureTypes.tap: { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.tap, (args) => { if (args.view) { @@ -149,9 +153,10 @@ export class GesturesObserver extends GesturesObserverBase { } }), ); + break; } - if (pluralType & GestureTypes.doubleTap) { + case GestureTypes.doubleTap: { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.doubleTap, (args) => { if (args.view) { @@ -159,9 +164,10 @@ export class GesturesObserver extends GesturesObserverBase { } }), ); + break; } - if (pluralType & GestureTypes.pinch) { + case GestureTypes.pinch: { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.pinch, (args) => { if (args.view) { @@ -169,9 +175,10 @@ export class GesturesObserver extends GesturesObserverBase { } }), ); + break; } - if (pluralType & GestureTypes.pan) { + case GestureTypes.pan: { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.pan, (args) => { if (args.view) { @@ -179,9 +186,10 @@ export class GesturesObserver extends GesturesObserverBase { } }), ); + break; } - if (pluralType & GestureTypes.swipe) { + case GestureTypes.swipe: { nativeView.addGestureRecognizer( this._createRecognizer( GestureTypes.swipe, @@ -229,9 +237,10 @@ export class GesturesObserver extends GesturesObserverBase { UISwipeGestureRecognizerDirection.Up, ), ); + break; } - if (pluralType & GestureTypes.rotation) { + case GestureTypes.rotation: { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.rotation, (args) => { if (args.view) { @@ -239,9 +248,10 @@ export class GesturesObserver extends GesturesObserverBase { } }), ); + break; } - if (pluralType & GestureTypes.longPress) { + case GestureTypes.longPress: { nativeView.addGestureRecognizer( this._createRecognizer(GestureTypes.longPress, (args) => { if (args.view) { @@ -249,22 +259,24 @@ export class GesturesObserver extends GesturesObserverBase { } }), ); + break; } - if (pluralType & GestureTypes.touch) { + case GestureTypes.touch: { nativeView.addGestureRecognizer(this._createRecognizer(GestureTypes.touch)); + break; } } } private _detach() { - for (const pluralGestureType in this._recognizers) { - const item = this._recognizers[pluralGestureType]; + for (const type in this._recognizers) { + const item = this._recognizers[type]; this.target?.nativeViewProtected?.removeGestureRecognizer(item.recognizer); item.recognizer = null; item.target = null; - delete this._recognizers[pluralGestureType]; + delete this._recognizers[type]; } } @@ -278,6 +290,7 @@ export class GesturesObserver extends GesturesObserverBase { this._onTargetLoaded = null; this._onTargetUnloaded = null; } + // clears target, context and callback references super.disconnect(); } @@ -293,28 +306,28 @@ export class GesturesObserver extends GesturesObserverBase { * this._recognizers) corresponding to the singular GestureTypes value passed * in. */ - private _createRecognizer(singularType: GestureTypes, callback?: (args: GestureEventData) => void, swipeDirection?: UISwipeGestureRecognizerDirection): UIGestureRecognizer | undefined { + private _createRecognizer(type: GestureTypes, callback?: (args: GestureEventData) => void, swipeDirection?: UISwipeGestureRecognizerDirection): UIGestureRecognizer | undefined { let recognizer: UIGestureRecognizer | undefined; - let singularTypeString = toString(singularType); - const target = _createUIGestureRecognizerTarget(this, singularType, callback, this.context); - const recognizerType = _getUIGestureRecognizerType(singularType); + let typeString = toString(type); + const target = _createUIGestureRecognizerTarget(this, type, callback, this.context); + const recognizerType = _getUIGestureRecognizerType(type); if (recognizerType) { recognizer = recognizerType.alloc().initWithTargetAction(target, 'recognize'); - if (singularType === GestureTypes.swipe && swipeDirection) { + if (type === GestureTypes.swipe && swipeDirection) { // e.g. "swipe1" - singularTypeString += swipeDirection.toString(); + typeString += swipeDirection.toString(); (recognizer).direction = swipeDirection; - } else if (singularType === GestureTypes.touch) { + } else if (type === GestureTypes.touch) { (recognizer).observer = this; - } else if (singularType === GestureTypes.doubleTap) { + } else if (type === GestureTypes.doubleTap) { (recognizer).numberOfTapsRequired = 2; } if (recognizer) { recognizer.delegate = recognizerDelegateInstance; - this._recognizers[singularTypeString] = { + this._recognizers[typeString] = { recognizer, target, }; @@ -323,7 +336,7 @@ export class GesturesObserver extends GesturesObserverBase { this.target.notify({ eventName: GestureEvents.gestureAttached, object: this.target, - type: singularType, + type: type, view: this.target, ios: recognizer, }); @@ -342,28 +355,27 @@ interface RecognizerCache { target: any; } -function _getUIGestureRecognizerType(singularType: GestureTypes): typeof UIGestureRecognizer | null { - let nativeType: typeof UIGestureRecognizer | null = null; - - if (singularType === GestureTypes.tap) { - nativeType = UITapGestureRecognizer; - } else if (singularType === GestureTypes.doubleTap) { - nativeType = UITapGestureRecognizer; - } else if (singularType === GestureTypes.pinch) { - nativeType = UIPinchGestureRecognizer; - } else if (singularType === GestureTypes.pan) { - nativeType = UIPanGestureRecognizer; - } else if (singularType === GestureTypes.swipe) { - nativeType = UISwipeGestureRecognizer; - } else if (singularType === GestureTypes.rotation) { - nativeType = UIRotationGestureRecognizer; - } else if (singularType === GestureTypes.longPress) { - nativeType = UILongPressGestureRecognizer; - } else if (singularType === GestureTypes.touch) { - nativeType = TouchGestureRecognizer; +function _getUIGestureRecognizerType(type: GestureTypes): typeof UIGestureRecognizer | null { + switch (type) { + case GestureTypes.tap: + return UITapGestureRecognizer; + case GestureTypes.doubleTap: + return UITapGestureRecognizer; + case GestureTypes.pinch: + return UIPinchGestureRecognizer; + case GestureTypes.pan: + return UIPanGestureRecognizer; + case GestureTypes.swipe: + return UISwipeGestureRecognizer; + case GestureTypes.rotation: + return UIRotationGestureRecognizer; + case GestureTypes.longPress: + return UILongPressGestureRecognizer; + case GestureTypes.touch: + return TouchGestureRecognizer; + default: + return null; } - - return nativeType; } function getState(recognizer: UIGestureRecognizer) { From ae439609348212d20a4a12bf709e971f9172ade7 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sun, 5 May 2024 11:22:20 +0900 Subject: [PATCH 4/8] fix(core): update tests --- apps/automated/src/data/observable-tests.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/automated/src/data/observable-tests.ts b/apps/automated/src/data/observable-tests.ts index 9bc36cb9e2..32efd1f956 100644 --- a/apps/automated/src/data/observable-tests.ts +++ b/apps/automated/src/data/observable-tests.ts @@ -163,7 +163,7 @@ export var test_Observable_addEventListener_MultipleEvents = function () { obj.addEventListener(events, callback); obj.set('testName', 1); obj.test(); - TKUnit.assert(receivedCount === 2, 'Callbacks not raised properly.'); + TKUnit.assert(receivedCount === 0, "Expected no event handlers to fire upon the 'propertyChange' event when listening for event name 'propertyChange,tested', as we have dropped support for listening to plural event names."); }; export var test_Observable_addEventListener_MultipleEvents_ShouldTrim = function () { @@ -176,13 +176,14 @@ export var test_Observable_addEventListener_MultipleEvents_ShouldTrim = function var events = Observable.propertyChangeEvent + ' , ' + TESTED_NAME; obj.addEventListener(events, callback); - TKUnit.assert(obj.hasListeners(Observable.propertyChangeEvent), 'Observable.addEventListener for multiple events should trim each event name.'); - TKUnit.assert(obj.hasListeners(TESTED_NAME), 'Observable.addEventListener for multiple events should trim each event name.'); + TKUnit.assert(obj.hasListeners(events), "Expected a listener to be present for event name 'propertyChange , tested', as we have dropped support for splitting plural event names."); + TKUnit.assert(!obj.hasListeners(Observable.propertyChangeEvent), "Expected no listeners to be present for event name 'propertyChange', as we have dropped support for splitting plural event names."); + TKUnit.assert(!obj.hasListeners(TESTED_NAME), "Expected no listeners to be present for event name 'tested', as we have dropped support for splitting plural event names."); obj.set('testName', 1); obj.test(); - TKUnit.assert(receivedCount === 2, 'Callbacks not raised properly.'); + TKUnit.assert(receivedCount === 0, "Expected no event handlers to fire upon the 'propertyChange' event when listening for event name 'propertyChange , tested', as we have dropped support for listening to plural event names (and trimming whitespace in event names)."); }; export var test_Observable_addEventListener_MultipleCallbacks = function () { @@ -223,7 +224,7 @@ export var test_Observable_addEventListener_MultipleCallbacks_MultipleEvents = f obj.set('testName', 1); obj.test(); - TKUnit.assert(receivedCount === 4, 'The propertyChanged notification should be raised twice.'); + TKUnit.assert(receivedCount === 0, "Expected no event handlers to fire upon the 'propertyChange' event when listening for event name 'propertyChange , tested' with two different callbacks, as we have dropped support for listening to plural event names (and trimming whitespace in event names)."); }; export var test_Observable_removeEventListener_SingleEvent_SingleCallback = function () { @@ -341,19 +342,22 @@ export var test_Observable_removeEventListener_MultipleEvents_SingleCallback = f var events = Observable.propertyChangeEvent + ' , ' + TESTED_NAME; obj.addEventListener(events, callback); + TKUnit.assert(obj.hasListeners(events), "Expected a listener to be present for event name 'propertyChange , tested', as we have dropped support for splitting plural event names."); + TKUnit.assert(!obj.hasListeners(Observable.propertyChangeEvent), "Expected no listeners to be present for event name 'propertyChange', as we have dropped support for splitting plural event names."); + TKUnit.assert(!obj.hasListeners(TESTED_NAME), "Expected no listeners to be present for event name 'tested', as we have dropped support for splitting plural event names."); + TKUnit.assert(receivedCount === 0, "Expected no event handlers to fire upon the 'propertyChange' event when listening for event name 'propertyChange , tested', as we have dropped support for listening to plural event names (and trimming whitespace in event names)."); obj.set('testName', 1); obj.test(); obj.removeEventListener(events, callback); - TKUnit.assert(!obj.hasListeners(Observable.propertyChangeEvent), 'Expected result for hasObservers is false'); - TKUnit.assert(!obj.hasListeners(TESTED_NAME), 'Expected result for hasObservers is false.'); + TKUnit.assert(!obj.hasListeners(events), "Expected the listener for event name 'propertyChange , tested' to have been removed, as we have dropped support for splitting plural event names."); obj.set('testName', 2); obj.test(); - TKUnit.assert(receivedCount === 2, 'Expected receive count is 2'); + TKUnit.assert(receivedCount === 0, "Expected no event handlers to fire upon the 'propertyChange' event when listening for event name 'propertyChange , tested', as we have dropped support for listening to plural event names (and trimming whitespace in event names)."); }; export var test_Observable_removeEventListener_SingleEvent_NoCallbackSpecified = function () { From 08b8cb389852a6b593b10d45c4648a017ff0b721 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sun, 5 May 2024 15:01:49 +0900 Subject: [PATCH 5/8] fix(core): identify gestures by exact match --- packages/core/ui/core/bindable/index.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/ui/core/bindable/index.ts b/packages/core/ui/core/bindable/index.ts index 5e13cb6112..a78d25b9c2 100644 --- a/packages/core/ui/core/bindable/index.ts +++ b/packages/core/ui/core/bindable/index.ts @@ -4,6 +4,7 @@ import { ViewBase } from '../view-base'; // Requires import { unsetValue } from '../properties'; import { Observable, PropertyChangeData } from '../../../data/observable'; +import { fromString as gestureFromString } from '../../../ui/gestures/gestures-common'; import { addWeakEventListener, removeWeakEventListener } from '../weak-event-listener'; import { bindingConstants, parentsRegex } from '../../builder/binding-builder'; import { escapeRegexSymbols } from '../../../utils'; @@ -91,11 +92,8 @@ export function getEventOrGestureName(name: string): string { return name.indexOf('on') === 0 ? name.substr(2, name.length - 2) : name; } -// NOTE: method fromString from "ui/gestures"; export function isGesture(eventOrGestureName: string): boolean { - const t = eventOrGestureName.trim().toLowerCase(); - - return t === 'tap' || t === 'doubletap' || t === 'pinch' || t === 'pan' || t === 'swipe' || t === 'rotation' || t === 'longpress' || t === 'touch'; + return !!gestureFromString(eventOrGestureName); } // TODO: Make this instance function so that we dont need public statc tapEvent = "tap" @@ -105,7 +103,7 @@ export function isEventOrGesture(name: string, view: ViewBase): boolean { const eventOrGestureName = getEventOrGestureName(name); const evt = `${eventOrGestureName}Event`; - return (view.constructor && evt in view.constructor) || isGesture(eventOrGestureName.toLowerCase()); + return (view.constructor && evt in view.constructor) || isGesture(eventOrGestureName); } return false; From 8cdd4e62f09ca90b8743c89bce61e7f9db53ad05 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sun, 5 May 2024 16:16:54 +0900 Subject: [PATCH 6/8] chore(core): remove unused eventNamesRegex --- packages/core/data/observable/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/core/data/observable/index.ts b/packages/core/data/observable/index.ts index 54aa92f9a0..f1d2d4a605 100644 --- a/packages/core/data/observable/index.ts +++ b/packages/core/data/observable/index.ts @@ -89,8 +89,6 @@ const _globalEventHandlers: { }; } = {}; -const eventNamesRegex = /\s*,\s*/; - /** * Observable is used when you want to be notified when a change occurs. Use on/off methods to add/remove listener. * Please note that should you be using the `new Observable({})` constructor, it is **obsolete** since v3.0, From 201af04174b7d0e664664ed994e0554375f052c5 Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Sun, 5 May 2024 17:09:32 +0900 Subject: [PATCH 7/8] fix(core): prevent regression in isGesture() --- packages/core/ui/core/bindable/index.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/core/ui/core/bindable/index.ts b/packages/core/ui/core/bindable/index.ts index a78d25b9c2..a614acafa0 100644 --- a/packages/core/ui/core/bindable/index.ts +++ b/packages/core/ui/core/bindable/index.ts @@ -93,7 +93,19 @@ export function getEventOrGestureName(name: string): string { } export function isGesture(eventOrGestureName: string): boolean { - return !!gestureFromString(eventOrGestureName); + // I believe we perform a case-insensitive lookup rather than an exact match + // for the original camelCase, mainly out of caution for upstream callers that + // might have converted the event name to lowercase (which was certainly a + // problem in Svelte 3). + // + // Not sure whether it's still needed in practice, though (all Core tests pass + // without case-insensitive matching and without trimming whitespace), so + // worth revisiting in future. + const t = eventOrGestureName.trim().toLowerCase(); + + // Would be nice to have a convenience function for getting all GestureState + // names in `gestures-common.ts`, but it creates a circular dependency. + return t === 'tap' || t === 'doubletap' || t === 'pinch' || t === 'pan' || t === 'swipe' || t === 'rotation' || t === 'longpress' || t === 'touch'; } // TODO: Make this instance function so that we dont need public statc tapEvent = "tap" From c469a403eda1379064195e3d416fc6d0e715a13e Mon Sep 17 00:00:00 2001 From: shirakaba <14055146+shirakaba@users.noreply.github.com> Date: Tue, 7 May 2024 10:04:47 +0900 Subject: [PATCH 8/8] chore(core): write up thoughts on getEventOrGestureName() and isGesture() --- packages/core/ui/core/bindable/index.ts | 33 ++++++++++++++++--------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/core/ui/core/bindable/index.ts b/packages/core/ui/core/bindable/index.ts index a614acafa0..d893e51a7a 100644 --- a/packages/core/ui/core/bindable/index.ts +++ b/packages/core/ui/core/bindable/index.ts @@ -88,27 +88,38 @@ export interface ValueConverter { toView: (...params: any[]) => any; } +/** + * Normalizes "ontap" to "tap", and "ondoubletap" to "ondoubletap". + * + * Removes the leading "on" from an event gesture name, for example: + * - "ontap" -> "tap" + * - "ondoubletap" -> "doubletap" + * - "onTap" -> "Tap" + * + * Be warned that, as event/gesture names in NativeScript are case-sensitive, + * this may produce an invalid event/gesture name (i.e. "doubletap" would fail + * to match the "doubleTap" gesture name), and so it is up to the consumer to + * handle the output properly. + */ export function getEventOrGestureName(name: string): string { - return name.indexOf('on') === 0 ? name.substr(2, name.length - 2) : name; + return name.indexOf('on') === 0 ? name.slice(2) : name; } export function isGesture(eventOrGestureName: string): boolean { - // I believe we perform a case-insensitive lookup rather than an exact match - // for the original camelCase, mainly out of caution for upstream callers that - // might have converted the event name to lowercase (which was certainly a - // problem in Svelte 3). - // - // Not sure whether it's still needed in practice, though (all Core tests pass - // without case-insensitive matching and without trimming whitespace), so - // worth revisiting in future. + // Not sure whether this trimming and lowercasing is still needed in practice + // (all Core tests pass without it), so worth revisiting in future. I think + // this is used exclusively by the XML flavour, and my best guess is that + // maybe it's to handle how getEventOrGestureName("onTap") might pass "Tap" + // into this. const t = eventOrGestureName.trim().toLowerCase(); // Would be nice to have a convenience function for getting all GestureState - // names in `gestures-common.ts`, but it creates a circular dependency. + // names in `gestures-common.ts`, but when I tried introducing it, it created + // a circular dependency that crashed the automated tests app. return t === 'tap' || t === 'doubletap' || t === 'pinch' || t === 'pan' || t === 'swipe' || t === 'rotation' || t === 'longpress' || t === 'touch'; } -// TODO: Make this instance function so that we dont need public statc tapEvent = "tap" +// TODO: Make this instance function so that we dont need public static tapEvent = "tap" // in controls. They will just override this one and provide their own event support. export function isEventOrGesture(name: string, view: ViewBase): boolean { if (typeof name === 'string') {