Skip to content
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

Make RNGH workable inside modals on Android #937

Merged
merged 8 commits into from
Feb 3, 2020
Merged

Make RNGH workable inside modals on Android #937

merged 8 commits into from
Feb 3, 2020

Conversation

osdnk
Copy link
Contributor

@osdnk osdnk commented Jan 30, 2020

Motivation

Modals are currently not working with RNGH, because handlers need to be located under RootView in the native hierarchy.

Changes

It's fixed it by wrapping content of modals with gestureHandlerHOC. However, it called for a few native changes.
In all places we were using RootView ref, I made it more generic to accept any ViewGroup. In places where methods of RootView were accessed I added checks and castings.

Also, modal wrapper (ReactModalHostView.DialogRootViewGroup) is not accessible from another package so I added the file to the package (com.facebook.react.views.modal) exporting important functionalities.

Added docs.

Test

I verified it iterating through example app and checking if every example (including these with RN's PanResponer) are working correctly.

@osdnk osdnk marked this pull request as ready for review January 30, 2020 14:16
@osdnk osdnk changed the title Make it workable on modals on Android Make RNGH workable inside modals on Android Jan 30, 2020
*/

public class RNGHModalUtils {
static public void dialogRootViewGroupOnChildStartedNativeGesture(ViewGroup modal, MotionEvent androidEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please change static public to public static? It's more common to see the latter in Java.

((ReactModalHostView.DialogRootViewGroup) modal).onChildStartedNativeGesture(androidEvent);
}

static public boolean isDialogRootViewGroup(ViewParent modal) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Comment on lines +160 to +184
### Usage with modals on Android
On Android RNGH does not work by default because modals are not located under React Native Root view in native hierarchy.
In order to make it workable, components need to be wrapped with `gestureHandlerRootHOC` (it's no-op on iOS and web).

E.g.
```js
const ExampleWithHoc = gestureHandlerRootHOC(
function GestureExample() {
return (
<View>
<DraggableBox />
</View>
);
}
);

export default function Example() {
return (
<Modal animationType="slide" transparent={false}>
<ExampleWithHoc />
</Modal>
);
}
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, would it be possible to export a modal from RNGH that already handles gestures as it should on Android?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but let's do it in follow-up

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to use example, but it does not work :(

Copy link
Contributor

@jkadamczyk jkadamczyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, good to have that addressed. Accepting to not block You as my comments are just tiny nitpicks.

@osdnk osdnk merged commit d9c7553 into master Feb 3, 2020
@osdnk osdnk deleted the @osdnk/modals branch February 3, 2020 11:57
janicduplessis pushed a commit to janicduplessis/react-native-gesture-handler that referenced this pull request Feb 16, 2020
Motivation
Modals are currently not working with RNGH, because handlers need to be located under RootView in the native hierarchy.

Changes
It's fixed it by wrapping content of modals with gestureHandlerHOC. However, it called for a few native changes.
In all places we were using RootView ref, I made it more generic to accept any ViewGroup. In places where methods of RootView were accessed I added checks and castings.

Also, modal wrapper (ReactModalHostView.DialogRootViewGroup) is not accessible from another package so I added the file to the package (com.facebook.react.views.modal) exporting important functionalities.

Added docs.

Test
I verified it iterating through example app and checking if every example (including these with RN's PanResponer) are working correctly.
@grahammendick
Copy link

@osdnk You don't need to use the inaccessible DialogRootViewGroup class. You can use the public RootView interface instead. Both ReactRootView and DialogRootViewGroup implement it and it contains the method onChildStartedNativeGesture that you're after. It would simplify the code and mean that you don't need to use the com.facebook.react.views.modal package.

@khacbac
Copy link

khacbac commented Aug 15, 2020

not work if Modal inside react-native-stack

@jerearaujo03
Copy link

not work if Modal inside react-native-stack

Same here with react-navigation-5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants