-
Notifications
You must be signed in to change notification settings - Fork 273
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
Improve react-native-gesture-handler::pressable
handling
#1738
Comments
I've checked the current state. These are the host components that RNGH Pressable renders in test (with Jest include they recommend):
It definitely lacks any event handler exposed on the host component level. |
Hey, to clarify, |
Hi @latekvo thanks for providing some context. I can definitely help with the testing library part. I first need to better understand the RNGH inner workings. Can you explain how RNGH and RNGH Button connects native event to In RN, |
Thank you, @latekvo and @mdjastrzebski, for helping with this goal! I used to think the |
@mdjastrzebski I went through the event flow on android and web, and it seems like the only feasible entry point for the touch events would be to provide regular touch events directly to the native button ( Android:On android, events are dispatched at the root of the view-tree, and travel downwards. iOS:Likely similar behaviour as on Web:On web, all callbacks provided in the gesture config ( Flowchart:I've attached a visual graph of the flow of events, and as far as I see, no other entry points than listed are available. You could theoretically force flowchart TD
subgraph "JS Side"
DeviceEventEmitter -- onGestureHandlerEvent, onGestureHandlerStateChange ---> onGestureHandlerEvent
onGestureHandlerEvent --> Gesture.Native
subgraph "Pressable"
InternalLogic[Pressable JS Logic]
Gesture.Native --> InternalLogic
InternalLogic --> onPress
InternalLogic --> onPressIn
InternalLogic --> onPressOut
end
end
subgraph "Android Implementation [Native]"
Event{{"MotionEvent::ACTION_DOWN"}}
AndroidView([RNGestureHandlerButton])
Event --> AndroidView
subgraph RNGestureHandlerModule
sendEventForDeviceEvent --> DeviceEventEmitter
end
subgraph GestureHandler
dispatchHandlerUpdate --> sendEventForDeviceEvent
end
subgraph Orchestrator
orchestratorLogic["Internal Gesture<br/>Activation Logic"]
orchestratorLogic --> deliverEventToGestureHandler
deliverEventToGestureHandler --> dispatchHandlerUpdate
end
AndroidView ---> RNGestureHandlerButton.onTouchEvent
AndroidView -- First event ---> RNGestureHandlerButton.onInterceptTouchEvent
RNGestureHandlerButtonViewManager --> NativeViewGestureHandler.onHandle
subgraph RNGestureHandlerButtonViewManager
RNGestureHandlerButton.onTouchEvent[onTouchEvent]
RNGestureHandlerButton.onInterceptTouchEvent[onInterceptTouchEvent]
RNGestureHandlerButton.onInterceptTouchEvent --> RNGestureHandlerButton.onTouchEvent
end
subgraph "NativeViewGestureHandler"
NativeViewGestureHandler.onHandle[onHandle]
NativeViewGestureHandler.onHandle --> orchestratorLogic
end
end
subgraph "Web Implementation"
WebEvent{{"pointerdown"}}
WebView([RNGestureHandlerButton])
Gesture.Native -. passes callbacks .-> attachEventManager
attachEventManager --> registerListeners
registerListeners --> WebView.addEventListener
WebView.addEventListener["RNGestureHandlerButton.addEventListener('pointerdown')"]
WebView.addEventListener -. attaches callbacks .-> WebView
WebView -- calls callbacks directly --> Gesture.Native
end
WebEvent --> WebView
|
Yeah that's exactly how it works from the user's perspective. |
@latekvo Thanks for detailed explanation. So as far as I understand, in order to raise You already seem to have RNGH specific tooling for invoking different gestures in form of I see a simple solution which we could use to bridge our libraries:
This way, just in testing env. RNTL would have access to relevant event handlers on host components @latekvo, @sidferreira wdyt? |
The I tested
I'm not sure if you need Also, should we also be handling Lastly, I couldn't find any information on what will Here's the PR applying those changes: software-mansion/react-native-gesture-handler#3357 |
@latekvo, will you add onClick as well? Some people often use it... |
@sidferreira, I think If Let's wait for @mdjastrzebski's opinion on this, as I'm completely unfamiliar with the inner-workings of |
@sidferreira what is the story with |
@latekvo I've checked your PR, looks good. RNTL would be responsible for inspecting and calling |
Sounds great. I'll wait for other |
@latekvo and @mdjastrzebski Sorry for my very short comment before, I was helping the kid with his homework. Now, if you search My understanding is that TLDR: I think |
@sidferreira I've looked through the React Native repo, and while it looks like there's some initial implementation present, one of the occurrences mentions that To me it sounds like any usages of |
I would not mix supporting |
@latekvo @mdjastrzebski TIL onClick is experimental... I assumed it was legacy or something. Will fix our code to prioritize onPress over onClick. Still, if you use |
Describe the Feature
I want to start acknowledging that's not
RNTL
responsibility, but asking for help to improve it.As customer (and fan) I can offer some help but I believe https://github.com/callstack/react-native-testing-library and https://github.com/software-mansion/react-native-gesture-handler would solve these issues WAY faster.
At this moment, while the app is running, the
RNGH::Pressable
will deliver a very similar experience toRN::Pressable
, but if we run tests, many things start to differ.Some items are very easy to solve (like missing a11y states) but some are trickier (
RN::Pressable
can respond tofireEvent(el, 'click')
butRNGH::Pressable
can't).I'm creating this issue to get some help on this.
Known Differences on Testing:
RNGH::Pressable
won't work withuserEvent.press
RNGH::Pressable
won't work withfireEvent(el, 'click')
Possible Implementations
#1741
software-mansion/react-native-gesture-handler#3357
Related Issues
software-mansion/react-native-gesture-handler#2385
The text was updated successfully, but these errors were encountered: