-
-
Notifications
You must be signed in to change notification settings - Fork 955
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
Prepare for DragGestureHandler #1035
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks for the PR!
Left a few comments. I'd love a more detailed description of this PR, the current one isn't really self-contained and feels a bit enigmatic.
I also didn't read into the logic much, wanted to get some overview first.
Also, we have a merge conflict here but should be fairly easy to resolve.
android/lib/src/main/java/com/swmansion/gesturehandler/GestureHandlerInteractionManager.java
Show resolved
Hide resolved
android/lib/src/main/java/com/swmansion/gesturehandler/GestureHandlerInteractionManager.java
Show resolved
Hide resolved
android/lib/src/main/java/com/swmansion/gesturehandler/GestureHandlerRegistryImpl.java
Outdated
Show resolved
Hide resolved
android/lib/src/main/java/com/swmansion/gesturehandler/GestureHandlerRegistryImpl.java
Show resolved
Hide resolved
android/lib/src/main/java/com/swmansion/gesturehandler/GestureHandlerRegistryImpl.java
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerRegistry.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/gesturehandler/react/RNGestureHandlerRegistry.java
Outdated
Show resolved
Hide resolved
rename `mViewForTag` -> `mViewForHandlerTag` (less confusing) add methods for dropping handlers
updated PR description |
fix concurrent access error
Hey all, is there any intention to continue this PR? Drag and drop is something that RN is really missing, so I'd love to see an addition like this, especially now that Reanimated 2 is out. Thanks so much! |
This is the first PR preparing the repo for #963
It enhances the registry and the interaction controller for simultaneous drag gestures which are a UI/UX standard and opens the door for simultaneous gestures across different views.
Build passes.
Once this will be merged I'll create the next.
Motivation
Making drag and drop in react-native match not only UI/UX standards but DevX as well, making it very simple to create a complex drag and drop UX with no overhead and complicated
js
.Please refer to this example to get a feeling of how simple it can be! It is the code behind the
gif
below.In Depth
GestureHandlerRegistry
In order to prepare a handler for gestures
GestureHandlerOrchestrator
callsGestureHandler.prepare
viaGestureHandlerOrchestrator.recordHandlerIfNotPresent
. This is done via complex logic once a touch is being recorded by the system by traversing the view tree. In order to support logic for simultaneous dragging there's a need to prepare the drag/drop handlers differently by callingrecordHandlerIfNotPresent
once a drag gesture begins (not leaning on logic that searches the top most view that contains the pointer). Hence a need to have a methodgetViewForHandler
to obtain the views attached to the simultaneous handlers that should run with the drag gesture's main handler.GestureHandlerInteractionManager
Exposed
getSimultaneousRelations
method and moved logic fromRNGestureHandlerInteractionManager
toGestureHandlerInteractionManager
for clarity, logic separation and for the android library (which now I know to be deprecated - I have updated it in the main PR and it is more than fantastic)