[oxilor]

Detailed description with examples and videos: #139 (comment)

[DigitalZebra]

Possible alternative implementation would be to expose GestureHandlerRootViewNative for clients to use directly?

[oxilor]

Possible alternative implementation would be to expose GestureHandlerRootViewNative for clients to use directly?

One of the options is to add an additional optional parameter to GestureHandlerRootView that determines whether the GestureHandlerRootViewNative component must be rendered forcibly (like forcedRender?: boolean).

[oxilor]

Tested:

[oxilor]

@jakub-gonet Is everything all right? Is it planned to merge?

Why was it decided not to render the nested GestureHandlerRootView? The comment above this code says:

If we already have a parent wrapped in the gesture handler root view, we don't need to wrap it again in root view we still wrap it in a normal view so our styling stays the same.

What styles stays the same (all properties, including styles, are passed to the normal View) and why?
If the GestureHandlerRootView component is not rendered (for example, in a modal on Android), then RNGH will not work in it (on Android if the component placed outside of the root, e.g. a modal).

In what cases, is it necessary to render a normal View instead of the GestureHandlerRootView when one of the parent components already has the root view?

If there are no such cases, then it seems to me better to remove the check whether the root view already has in a parent components using a context. Additionally, we could add a Modal component that uses the native Modal and wraps the content with the GestureHandlerRootView. Thus, all that will be needed is to import a Modal from the RNGH and everything on android will work. What do you think?

[urbanclap-admin]

@oxilor Thanks for raising this MR. We have been facing the same issue with nested Modals using GestureHandlerRootView as the inner nested Modal receives true for the if(available) condition but the root view does not actually exist as the parent and thus any gesture handler components like PanGestureHandler or touchables does not work.
This is a great solution to keep the optimization as well as provide some control to the developer to override default behaviour.

@jakub-gonet please push this fix along as fast as possible as it is a major bug fix for the system but not a big change in terms of code.

[oxilor]

@urbanclap-admin Do you know a case where it is necessary to render nested root view as a normal View? If there are no cases (in the beginning we need to consult with @jakub-gonet or may be you know any case), I think that it is better to remove this condition at all.

Otherwise, we can leave the current implementation, which allows a developer to control whether the root view must be rendered as GestureHandlerRootViewNative or not.

[jgonet]

Hey, sorry for the delay.

Why was it decided not to render the nested GestureHandlerRootView?

The problem with rendering nested GH root views is that they will create different root helpers which in turn will create different orchestrators. This will prevent interaction between handlers across root views as mentioned by @kmagiera and in our docs:

If you use props such as shouldCancelWhenOutside, simultaneousHandlers, waitFor etc. with gesture handlers, the handlers need to be mounted under a single GestureHandlerRootView. So it's important to keep the GestureHandlerRootView as close to the actual root view as possible.

That's why we render only one root view in React tree right now.

---

The comment above this code says:

If we already have a parent wrapped in the gesture handler root view, we don't need to wrap it again in root view we still wrap it in a normal view so our styling stays the same.

What styles stays the same (all properties, including styles, are passed to the normal View) and why?

It's explained in PR linked above. Basically we need consistent styling even if root view isn't applied so that's a difference between adding view and not adding it.

---

The problem with modals is that they're rendered in the same React tree but different native view hierarchy. When creating a new root view we check if we did that earlier and your change most likely doesn't show this info in logs due to modals but will if you use it without one.

I thought about the correct solution and it seems like the simplest one is to always wrap views in the root view (which will make HOC work) and remove the check for mEnabled in the native side (with an appropriate comment about modals in the following if). I haven't tested that though and I won't spend much time on RNGH in the few following weeks so feel free to experiment with that. If you come with an alternative solution, please share it with me - maybe it can be solved better.

EDIT: this won't solve the problem with the orchestrator, it probably should be shared between root views to correctly register and handle all gesture handlers.

[satya164]

@jakub-gonet Wouldn't hasGestureHandlerEnabledRootView return true if there is already a root view? I had added this check natively as well because Expo apps already had a root view so we didn't want to wrap again. We can probably remove the check from JS side.

[jgonet]

Wouldn't hasGestureHandlerEnabledRootView return true if there is already a root view?

If you're talking about a typical React hierarchy, then yes. But modals create their own hierarchy so there's no root view in their branch and this method returns false.

If we remove the check from the JS side then we need to remove Log.i telling that RNGH is already enabled (which will trigger when you have React Navigation + some random component using GestureHandlerHOC for example). But this isn't still solution to this problem since interactions between handlers above Modal and below modal won't work together. We need to change GestureHandler class to have backing view_s_ - basically, generalize it a bit. I'm fine with the first iteration that adds support for modals when you have nested root views in React hierarchy but we have to fix this problem eventually as well.

[satya164]

But this isn't still solution to this problem since interactions between handlers above Modal and below modal won't work together

I think this would be a very niche use case if someone even needs that.

Anyway, my main point was that the check on JS side isn't needed since we already check on native side and since it prevents modals from working.

[jgonet]

#1603 should fix this problem.

[j-piasecki]

Thank you @oxilor for your contribution and coming up with a solution, but since #1603 is already merged and it resolves the issue I decided to close this pull request.

[oxilor]

@jakub-gonet Thank you for fixing the bug. I'm really waiting for #1603 to be published to npm.