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 blocksGesture symmetric on native iOS #3322

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

j-piasecki
Copy link
Member

Description

Should fix #3321

This PR makes the native implementation of blocksGesture symmetric. Previously, the logic was implemented only in shouldBeRequiredToFailByGestureRecognizer, now, it's also in shouldRequireFailureOfGestureRecognizer.

Test plan

Tested on the code from the issue

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.

Looks good!

What I'd suggest is to move this nil check before other checks, like this:

code
- (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer
    shouldRequireFailureOfGestureRecognizer:(UIGestureRecognizer *)otherGestureRecognizer
{
  RNGestureHandler *handler = [RNGestureHandler findGestureHandlerByRecognizer:otherGestureRecognizer];

  if(handler == nil) {
    return NO;
  }

  if ([_handlersToWaitFor count]) {
    for (NSNumber *handlerTag in _handlersToWaitFor) {
      if ([handler.tag isEqual:handlerTag]) {
        return YES;
      }
    }
  }


  for (NSNumber *handlerTag in handler->_handlersThatShouldWait) {
    if ([_tag isEqual:handlerTag]) {
      return YES;
    }
  }

  return NO;
}

or at least combine it with and:

if (handler != nil && [_handlersToWaitFor count]) {

Comment on lines 476 to 479
for (NSNumber *handlerTag in handler->_handlersThatShouldWait) {
if ([_tag isEqual:handlerTag]) {
return YES;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we also check [_handlersThatShouldWait count], as above?

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved the null check up and removed the check for [_handlersThatShouldWait count] entirely - I don't really see the difference it makes when the list is empty.

@gaearon
Copy link

gaearon commented Jan 10, 2025

Yes, this does seem to resolve the issue!

@gaearon
Copy link

gaearon commented Jan 10, 2025

Curiously, moving to blocksExternalGesture (now that it's fixed on iOS) has also resolved a gesture conflict on Android. I don't have a minimal repro case for that. Just curious that they're not really symmetrical on Android either.

Edit: here's the Android issue, #3326

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.

blocksExternalGesture doesn't work together with activeOffsetX
3 participants