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

Allow GestureHandlerRootView to be manually made active #2401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

j-piasecki
Copy link
Member

Description

This PR allows users to override whether a particular GestureHandlerRootView is enabled or not. By default, only the topmost root view would be active, allowing all gestures to interact with each other.

The problem with that approach is that if any of the native views underneath calledrequestDisallowInterceptTouchEvent, all gestures would be canceled. It comes from the fact that requestDisallowInterceptTouchEvent calls are propagated up the view hierarchy and we expect GestureHandlerRootView to be relatively close to the top.

The simple solution would be to add another root view underneath the view that may call requestDisallowInterceptTouchEvent to prevent gestures from being canceled, this, however, resulted in two problems:

  • the inner root view would not be enabled
  • after adding the option to enable it manually, both root views would start handling the event, resulting in duplicated event streams

I've fixed the second problem by adding checks at the beginning of extractGestureHandlers and traverseWithPointerEvents to ensure that we're not extracting gestures attached under another enabled root view.

This allows for solving the problem described in #2383.

Test plan

Tested on the reproduction from #2383

Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

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

LGTM

@lpatrun
Copy link

lpatrun commented Mar 21, 2024

Can we expect this to be merged? It's approved after all
🙈

@j-piasecki
Copy link
Member Author

Actually, I'm not sure if this is the best solution to this problem. I'll try to get back to it in the near future to see if I can find other approaches to this.

@lpatrun
Copy link

lpatrun commented Mar 21, 2024

@j-piasecki thank you for the quick response

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.

None yet

4 participants