-
-
Notifications
You must be signed in to change notification settings - Fork 1k
[Web] v3 typing and using handler data #3651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from all commits
ea8bd73
32fea07
968112c
6a69cd7
2b404af
9c30c1a
d7dd057
56b93d3
ad8ef64
0304177
4aaf700
eeddd24
3242ffd
7147ab3
cc78f74
067becb
d782c95
86046bd
6c34d33
cd793b3
580494d
eb98bf4
af663c8
42e84b4
079fdb4
cb72d7e
de98017
e193ff4
2ba053a
8ab666b
c445262
46a9570
37c4070
20cbc9e
58f8d60
532f35a
7d205dc
fb7af53
cc3b639
0c6f200
00738d6
9c17a96
2353a84
2443971
f3198f5
686fadd
b0d384d
2ba5ae8
38b240a
7c00105
dceba3b
14190fc
b738254
1f62d5c
4d916f5
0b16cdb
8579092
089466e
04ec35f
5837159
0c96928
4d6d1f2
6391704
9e6fa97
323acf6
3336d48
4a75f5f
a5148ac
1cd5eef
2e47b1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,9 +6,9 @@ import { | |
PropsRef, | ||
ResultEvent, | ||
PointerData, | ||
ResultTouchEvent, | ||
TouchEventType, | ||
EventTypes, | ||
GestureHandlerNativeEvent, | ||
} from '../interfaces'; | ||
import EventManager from '../tools/EventManager'; | ||
import GestureHandlerOrchestrator from '../tools/GestureHandlerOrchestrator'; | ||
|
@@ -20,6 +20,7 @@ import { PointerType } from '../../PointerType'; | |
import { GestureHandlerDelegate } from '../tools/GestureHandlerDelegate'; | ||
import { ActionType } from '../../ActionType'; | ||
import { tagMessage } from '../../utils'; | ||
import { StateChangeEvent, TouchEvent, UpdateEvent } from '../../v3/types'; | ||
|
||
export default abstract class GestureHandler implements IGestureHandler { | ||
private lastSentState: State | null = null; | ||
|
@@ -369,7 +370,7 @@ export default abstract class GestureHandler implements IGestureHandler { | |
const { onGestureHandlerEvent, onGestureHandlerTouchEvent }: PropsRef = | ||
this.propsRef!.current; | ||
|
||
const touchEvent: ResultTouchEvent | undefined = | ||
const touchEvent: ResultEvent<TouchEvent> | undefined = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now when I think about it we could probably do the same for |
||
this.transformTouchEvent(event); | ||
|
||
if (touchEvent) { | ||
|
@@ -395,12 +396,14 @@ export default abstract class GestureHandler implements IGestureHandler { | |
onGestureHandlerStateChange, | ||
onGestureHandlerAnimatedEvent, | ||
}: PropsRef = this.propsRef!.current; | ||
const resultEvent: ResultEvent = this.transformEventData( | ||
newState, | ||
oldState | ||
); | ||
|
||
// In the new API oldState field has to be undefined, unless we send event state changed | ||
const resultEvent: ResultEvent = | ||
this.actionType !== ActionType.NATIVE_DETECTOR | ||
? this.transformEventData(newState, oldState) | ||
: this.lastSentState !== newState | ||
? this.transformStateChangeEvent(newState, oldState) | ||
: this.transformUpdateEvent(newState); | ||
|
||
// In the v2 API oldState field has to be undefined, unless we send event state changed | ||
// Here the order is flipped to avoid workarounds such as making backup of the state and setting it to undefined first, then changing it back | ||
// Flipping order with setting oldState to undefined solves issue, when events were being sent twice instead of once | ||
// However, this may cause trouble in the future (but for now we don't know that) | ||
|
@@ -410,18 +413,22 @@ export default abstract class GestureHandler implements IGestureHandler { | |
invokeNullableMethod(onGestureHandlerStateChange, resultEvent); | ||
} | ||
if (this.state === State.ACTIVE) { | ||
resultEvent.nativeEvent.oldState = undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I remember this was necessary in order to keep compatibility with old API. Right, @j-piasecki? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is the case referred in the comment above as
Due to how new events are structures (no |
||
if (this.actionType !== ActionType.NATIVE_DETECTOR) { | ||
(resultEvent.nativeEvent as GestureHandlerNativeEvent).oldState = | ||
undefined; | ||
} | ||
if (onGestureHandlerAnimatedEvent && this.forAnimated) { | ||
invokeNullableMethod(onGestureHandlerAnimatedEvent, resultEvent); | ||
} | ||
invokeNullableMethod(onGestureHandlerEvent, resultEvent); | ||
} | ||
}; | ||
|
||
private transformEventData(newState: State, oldState: State): ResultEvent { | ||
if (!this.viewRef) { | ||
throw new Error(tagMessage('Cannot handle event when target is null')); | ||
} | ||
private transformEventData( | ||
newState: State, | ||
oldState: State | ||
): ResultEvent<GestureHandlerNativeEvent> { | ||
this.ensureViewRef(this.viewRef); | ||
return { | ||
nativeEvent: { | ||
numberOfPointers: this.tracker.trackedPointersCount, | ||
|
@@ -439,9 +446,55 @@ export default abstract class GestureHandler implements IGestureHandler { | |
}; | ||
} | ||
|
||
private transformStateChangeEvent( | ||
newState: State, | ||
oldState: State | ||
): ResultEvent<StateChangeEvent<unknown>> { | ||
this.ensureViewRef(this.viewRef); | ||
return { | ||
nativeEvent: { | ||
state: newState, | ||
handlerTag: this.handlerTag, | ||
pointerType: this.pointerType, | ||
oldState: oldState, | ||
numberOfPointers: this.tracker.trackedPointersCount, | ||
handlerData: { | ||
pointerInside: this.delegate.isPointerInBounds( | ||
this.tracker.getAbsoluteCoordsAverage() | ||
), | ||
...this.transformNativeEvent(), | ||
target: this.viewRef, | ||
}, | ||
}, | ||
timeStamp: Date.now(), | ||
}; | ||
} | ||
|
||
private transformUpdateEvent( | ||
newState: State | ||
): ResultEvent<UpdateEvent<unknown>> { | ||
this.ensureViewRef(this.viewRef); | ||
return { | ||
nativeEvent: { | ||
state: newState, | ||
handlerTag: this.handlerTag, | ||
pointerType: this.pointerType, | ||
numberOfPointers: this.tracker.trackedPointersCount, | ||
handlerData: { | ||
pointerInside: this.delegate.isPointerInBounds( | ||
this.tracker.getAbsoluteCoordsAverage() | ||
), | ||
...this.transformNativeEvent(), | ||
target: this.viewRef, | ||
}, | ||
}, | ||
timeStamp: Date.now(), | ||
}; | ||
} | ||
|
||
private transformTouchEvent( | ||
event: AdaptedEvent | ||
): ResultTouchEvent | undefined { | ||
): ResultEvent<TouchEvent> | undefined { | ||
const rect = this.delegate.measureView(); | ||
|
||
const all: PointerData[] = []; | ||
|
@@ -571,7 +624,7 @@ export default abstract class GestureHandler implements IGestureHandler { | |
}); | ||
}); | ||
|
||
const cancelEvent: ResultTouchEvent = { | ||
const cancelEvent: ResultEvent<TouchEvent> = { | ||
nativeEvent: { | ||
handlerTag: this.handlerTag, | ||
state: this.state, | ||
|
@@ -597,6 +650,12 @@ export default abstract class GestureHandler implements IGestureHandler { | |
} | ||
} | ||
|
||
private ensureViewRef(viewRef: any): asserts viewRef is number { | ||
if (!viewRef) { | ||
throw new Error(tagMessage('Cannot handle event when target is null')); | ||
} | ||
} | ||
Comment on lines
+653
to
+657
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, little deja vu - wasn't it added in other PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar function, |
||
|
||
protected transformNativeEvent(): Record<string, unknown> { | ||
// Those properties are shared by most handlers and if not this method will be overriden | ||
const lastCoords = this.tracker.getAbsoluteCoordsAverage(); | ||
|
@@ -888,10 +947,10 @@ export default abstract class GestureHandler implements IGestureHandler { | |
|
||
function invokeNullableMethod( | ||
method: | ||
| ((event: ResultEvent | ResultTouchEvent) => void) | ||
| { __getHandler: () => (event: ResultEvent | ResultTouchEvent) => void } | ||
| ((event: ResultEvent) => void) | ||
| { __getHandler: () => (event: ResultEvent) => void } | ||
| { __nodeConfig: { argMapping: unknown[] } }, | ||
event: ResultEvent | ResultTouchEvent | ||
event: ResultEvent | ||
): void { | ||
if (!method) { | ||
return; | ||
|
@@ -923,7 +982,7 @@ function invokeNullableMethod( | |
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
const nativeValue = event.nativeEvent[key]; | ||
const nativeValue = (event.nativeEvent as any)[key]; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
if (value?.setValue) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,12 @@ import { | |
ActiveCursor, | ||
MouseButton, | ||
TouchAction, | ||
StylusData, | ||
} from '../handlers/gestureHandlerCommon'; | ||
import { Directions } from '../Directions'; | ||
import { State } from '../State'; | ||
import { PointerType } from '../PointerType'; | ||
import { GestureHandlerEvent } from '../v3/types'; | ||
import { State } from '../State'; | ||
|
||
export interface HitSlop { | ||
left?: number; | ||
|
@@ -83,7 +85,8 @@ export interface Config extends Record<string, ConfigArgs> { | |
} | ||
|
||
type NativeEventArgs = number | State | boolean | undefined; | ||
interface NativeEvent extends Record<string, NativeEventArgs> { | ||
export interface GestureHandlerNativeEvent | ||
extends Record<string, NativeEventArgs> { | ||
numberOfPointers: number; | ||
state: State; | ||
pointerInside: boolean | undefined; | ||
|
@@ -106,42 +109,19 @@ export interface PointerData { | |
absoluteY: number; | ||
} | ||
|
||
type TouchNativeArgs = number | State | TouchEventType | PointerData[]; | ||
|
||
interface NativeTouchEvent extends Record<string, TouchNativeArgs> { | ||
handlerTag: number; | ||
state: State; | ||
eventType: TouchEventType; | ||
changedTouches: PointerData[]; | ||
allTouches: PointerData[]; | ||
numberOfTouches: number; | ||
pointerType: PointerType; | ||
} | ||
|
||
export interface ResultEvent extends Record<string, NativeEvent | number> { | ||
nativeEvent: NativeEvent; | ||
timeStamp: number; | ||
} | ||
|
||
export interface ResultTouchEvent | ||
extends Record<string, NativeTouchEvent | number> { | ||
nativeEvent: NativeTouchEvent; | ||
// Native event has to stay for v2 compatibility | ||
type ResultEventType = GestureHandlerEvent<unknown> | GestureHandlerNativeEvent; | ||
export interface ResultEvent<T extends ResultEventType = ResultEventType> | ||
extends Record<string, T | number> { | ||
nativeEvent: T; | ||
Comment on lines
+113
to
+116
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I'm wrong, but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I find it quite confusing with this default value, wouldn't it be easier to read if we had just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, types in GestureHandlerEvent are wrappers on payload types defined here, and only add
Ok, it was inspired by this. I thought that it would simplify the code as we would not have to write |
||
timeStamp: number; | ||
} | ||
|
||
export interface PropsRef { | ||
onGestureHandlerEvent: (e: any) => void; | ||
onGestureHandlerAnimatedEvent?: (e: any) => void; | ||
onGestureHandlerStateChange: (e: any) => void; | ||
onGestureHandlerTouchEvent?: (e: any) => void; | ||
} | ||
|
||
export interface StylusData { | ||
tiltX: number; | ||
tiltY: number; | ||
azimuthAngle: number; | ||
altitudeAngle: number; | ||
pressure: number; | ||
onGestureHandlerEvent: (e: ResultEvent) => void; | ||
onGestureHandlerAnimatedEvent?: (e: ResultEvent) => void; | ||
onGestureHandlerStateChange: (e: ResultEvent) => void; | ||
onGestureHandlerTouchEvent?: (e: ResultEvent) => void; | ||
} | ||
|
||
export interface AdaptedEvent { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether this is the right place for it, but I had to put it somewhere else than web/interfaces , and this looked like the most promising spot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without moving
StylusData
there is a circular dependency. Moving this interface seemed the most sensible option to resolve it.gestureHandlerCommon
contains common interfaces for all platforms.