Skip to content

Fix modals - #1603

Merged
j-piasecki merged 4 commits into
masterfrom
jgonet/fix-modals
Sep 24, 2021
Merged

j-piasecki merged 4 commits into
masterfrom
jgonet/fix-modals

Conversation

@jgonet

@jgonet jgonet commented Aug 27, 2021 •

Copy link
Copy Markdown
Member

Description

Fixes #139

  1. Removed RNGHModalUtils

    This class gave access to ReactModalHostView.DialogRootViewGroup in order to
    call #onChildStartedNativeGesture() and use instanceof on it. Checks for
    ReactRootView / DialogRootViewGroup can be unified by utilizing RootView (which
    both of those classes implement).

  2. Always render GestureHandlerRootView

    This prevents gestures from not being delivered to RNGH when another
    GestureHandlerRootView is used before modal (e.g. from React Navigation).

    Note: This isn't an ideal fix since a new orchestrator will be created, preventing modals from interacting with handlers outside of them. However, this use case is rare. Having gestures working in modals is already an improvement.

Test plan

Tested on nestedGHRootViewWithModal.tsx example.

jgonet added 2 commits August 27, 2021 10:32
1. Removed RNGHModalUtils

This class gave access to ReactModalHostView.DialogRootViewGroup in order to
call #onChildStartedNativeGesture() and use instanceof on it. Checks for
ReactRootView/DialogRootViewGroup can be unified by utilizing RootView (which
both of those classes implement).

2. Always render GestureHandlerRootView

This prevents gestures from not being delivered to RNGH when another
GestureHandlerRootView is used before modal (e.g. from React Navigation).

Note: This isn't an ideal fix since a new orchestrator will be created, preventing
modals from interacting with handlers outside of them. However, this use case is
rare. Having gestures working in modals is already an improvement.
@jgonet
jgonet requested review from j-piasecki and kmagiera August 27, 2021 08:33
@jgonet jgonet self-assigned this Aug 27, 2021
@danilo-vieira

Copy link
Copy Markdown

Can anyone merge this? 👀

@jcgertig

jcgertig commented Sep 24, 2021 •

Copy link
Copy Markdown

@jakub-gonet @kmagiera any movement on this its a big blocker

@j-piasecki
j-piasecki merged commit 5572e8d into master Sep 24, 2021
fluiddot pushed a commit to wordpress-mobile/react-native-gesture-handler that referenced this pull request Feb 25, 2022
Fixes software-mansion#139

Removed RNGHModalUtils

This class gave access to ReactModalHostView.DialogRootViewGroup in order to
call #onChildStartedNativeGesture() and use instanceof on it. Checks for
ReactRootView / DialogRootViewGroup can be unified by utilizing RootView (which
both of those classes implement).

Always render GestureHandlerRootView

This prevents gestures from not being delivered to RNGH when another
GestureHandlerRootView is used before modal (e.g. from React Navigation).

Note: This isn't an ideal fix since a new orchestrator will be created, preventing modals from interacting with handlers outside of them. However, this use case is rare. Having gestures working in modals is already an improvement.
@hugoh59

hugoh59 commented Aug 3, 2022

Copy link
Copy Markdown

I still have the issue on 2.5.0 with PanGestureHandler inside a Modal.

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.

Doesn't work in a modal

5 participants