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

[Android] Fix gestures being able to activate despite their parent already being active #3095

Merged

Conversation

latekvo
Copy link
Contributor

@latekvo latekvo commented Sep 12, 2024

Description

This PR fixes invalid activation of gestures nested inside other gestures, like Pan gesture nested inside Native gesture attached to ScrollView

Gestures nested inside native elements such as ScrollView used to be able to steal pointers from their already active parents.

That is no longer possible, already active parents cannot have their active pointers stolen.

Related to #2622

Test plan

  • use the attached code in place of EmptyExample.tsx
  • start scrolling the ScrollView
  • while scrolling the ScrollView, drag the Pan gesture
  • see how before this PR, the Pan gesture activated, and with this PR it doesn't anymore

Notes

  • tested this PR on each of the available examples, found no breaking changes
  • nested gestures may still be run simultaneously if it's explicitly stated using Gesture.Simultaneous() or simultaneousWithExternalGesture()

Code

Collapsed code
import React from 'react';
import { StyleSheet, Text, View, ScrollView } from 'react-native';
import {
  Gesture,
  GestureDetector,
  GestureUpdateEvent,
  PanGestureHandlerEventPayload,
} from 'react-native-gesture-handler';
import Animated, {
  SharedValue,
  useAnimatedStyle,
  useSharedValue,
  withSpring,
} from 'react-native-reanimated';

export default function EmptyExample() {
  const firstExternalPosition = useSharedValue<{ x: number; y: number }>({
    x: 0,
    y: 0,
  });

  const secondExternalPosition = useSharedValue<{ x: number; y: number }>({
    x: 0,
    y: 0,
  });

  const nestedPosition = useSharedValue<{ x: number; y: number }>({
    x: 0,
    y: 0,
  });

  const setter = (
    position: SharedValue<{
      x: number;
      y: number;
    }>
  ) => {
    return (event: GestureUpdateEvent<PanGestureHandlerEventPayload>) => {
      'worklet';
      position.value = {
        x: event.translationX,
        y: event.translationY,
      };
    };
  };

  const resetter = (
    position: SharedValue<{
      x: number;
      y: number;
    }>
  ) => {
    return () => {
      'worklet';
      position.value = {
        x: withSpring(0),
        y: withSpring(0),
      };
    };
  };

  const scrollGesture = Gesture.Native();

  const firstExternalPan = Gesture.Pan()
    .onUpdate(setter(firstExternalPosition))
    .onFinalize(resetter(firstExternalPosition));

  const secondExternalPan = Gesture.Pan()
    .onUpdate(setter(secondExternalPosition))
    .onFinalize(resetter(secondExternalPosition));

  const nestedPan = Gesture.Pan()
    // .simultaneousWithExternalGesture(scrollGesture)
    .onUpdate(setter(nestedPosition))
    .onFinalize(resetter(nestedPosition));

  const firstExternalAnimation = useAnimatedStyle(() => {
    return {
      ...styles.box,
      transform: [
        { translateX: firstExternalPosition.value.x },
        { translateY: firstExternalPosition.value.y },
      ],
    };
  });

  const secondExternalAnimation = useAnimatedStyle(() => {
    return {
      ...styles.box,
      transform: [
        { translateX: secondExternalPosition.value.x },
        { translateY: secondExternalPosition.value.y },
      ],
    };
  });

  const nestedAnimation = useAnimatedStyle(() => {
    return {
      ...styles.box,
      transform: [
        { translateX: nestedPosition.value.x },
        { translateY: nestedPosition.value.y },
      ],
    };
  });

  return (
    <View style={styles.container}>
      <View style={styles.externalContainer}>
        <GestureDetector gesture={firstExternalPan}>
          <Animated.View style={firstExternalAnimation}>
            <Text>
              Square showcasing 2 disconnected gestures can be moved
              independantly regardless of changes in this PR, and regardless if
              one of them is nested inside a native handler.
            </Text>
          </Animated.View>
        </GestureDetector>
        <GestureDetector gesture={secondExternalPan}>
          <Animated.View style={secondExternalAnimation}>
            <Text>
              Square showcasing 2 disconnected gestures can be moved
              independantly regardless of changes in this PR, and regardless if
              one of them is nested inside a native handler.
            </Text>
          </Animated.View>
        </GestureDetector>
      </View>

      <View>
        <GestureDetector gesture={scrollGesture}>
          <ScrollView style={styles.list}>
            <GestureDetector gesture={nestedPan}>
              <Animated.View style={nestedAnimation}>
                <Text>GH Gesture</Text>
              </Animated.View>
            </GestureDetector>

            {new Array(20)
              .fill(1)
              .map((value, index) => value * index)
              .map((value) => (
                <View key={value} style={styles.element}>
                  <Text>Entry no. {value}</Text>
                </View>
              ))}
          </ScrollView>
        </GestureDetector>
      </View>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#F5FCFF',
    gap: 20,
  },
  externalContainer: {
    flexDirection: 'row',
    gap: 20,
    marginTop: 300,
  },
  box: {
    position: 'relative',
    backgroundColor: 'tomato',
    width: 200,
    height: 200,
  },
  list: {
    width: 200,
    backgroundColor: 'plum',
  },
  element: {
    margin: 1,
    height: 40,
    backgroundColor: 'orange',
  },
});

@latekvo latekvo changed the title [Android] Fix gestures being able to activate despite their parent being already active [Android] Fix gestures being able to activate despite their parent already being active Sep 12, 2024
@latekvo latekvo marked this pull request as draft September 19, 2024 13:39
@latekvo latekvo requested a review from m-bert September 25, 2024 14:54
@latekvo latekvo marked this pull request as ready for review September 25, 2024 14:54
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.

Good job! 🚀

Please have a look at the comments below 😅

Comment on lines 713 to 714
private fun isActive(state: Int) =
state == GestureHandler.STATE_ACTIVE
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is used anywhere 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed ef986a0

private fun shouldBeCancelledByFinishedHandler(handler: GestureHandler<*>) =
gestureHandlers.any { shouldHandlerWaitForOther(handler, it) && it.state == GestureHandler.STATE_END }

private fun handlersWithinHandlerBounds(handler: GestureHandler<*>) =
Copy link
Contributor

Choose a reason for hiding this comment

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

This name is quite misleading. For me it suggests that we compare positions of native views, while in reality we check for common pointers.

What about handlersWithCommonPointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function was removed in 6d11f86

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, please wait for @j-piasecki approval 🟢

Copy link
Member

@j-piasecki j-piasecki left a comment

Choose a reason for hiding this comment

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

LGTM

@latekvo latekvo merged commit 28ba683 into main Sep 26, 2024
3 checks passed
@latekvo latekvo deleted the @latekvo/fix-nested-gesture-stealing-activity-from-native-gesture branch September 26, 2024 08:37
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.

3 participants