Logic Detector

[akwasniewski]

Description

This PR implements new component -> LogicDetector. It resolves the issue of attaching gestures to inner SVG components. LogicDetector communicates with a NativeDetector higher in the hierarchy, which will be responsible for attaching gestures.

Test plan

tested on the following code

import React from 'react';
import { Text, View, StyleSheet } from 'react-native';
import { NativeDetector, LogicDetector, useGesture, SingleGestureName } from 'react-native-gesture-handler';

import Svg, { Circle, Rect } from 'react-native-svg';

export default function SvgExample() {
  const circleElementTap = useGesture(SingleGestureName.Tap, {
    onStart: () => {
      'worklet';
      console.log('RNGH: clicked circle')
    }
  });
  const rectElementTap = useGesture(SingleGestureName.Tap, {
    onStart: () => {
      'worklet';
      console.log('RNGH: clicked parallelogram')
    }
  });
  const containerTap = useGesture(SingleGestureName.Tap, {
    onStart: () => {
      'worklet';
      console.log('RNGH: clicked container')
    }
  });
  const vbContainerTap = useGesture(SingleGestureName.Tap, {
    onStart: () => {
      'worklet';
      console.log('RNGH: clicked viewbox container')
    }
  });
  const vbInnerContainerTap = useGesture(SingleGestureName.Tap, {
    onStart: () => {
      'worklet';
      console.log('RNGH: clicked inner viewbox container')
    }
  });
  const vbCircleTap = useGesture(SingleGestureName.Tap, {
    onStart: () => {
      'worklet';
      console.log('RNGH: clicked viewbox circle')
    }
  });

  return (
    <View>
      <View style={styles.container}>
        <Text style={styles.header}>
          Overlapping SVGs with gesture detectors
        </Text>
        <View style={{ backgroundColor: 'tomato' }}>
          <NativeDetector gesture={containerTap}>
            <Svg
              height="250"
              width="250"
              onPress={() => console.log('SVG: clicked container')}>
              <LogicDetector gesture={circleElementTap}>
                <Circle
                  cx="125"
                  cy="125"
                  r="125"
                  fill="green"
                  onPress={() => console.log('SVG: clicked circle')}
                />
              </LogicDetector>
              <LogicDetector gesture={rectElementTap}>
                <Rect
                  skewX="45"
                  width="125"
                  height="250"
                  fill="yellow"
                  onPress={() => console.log('SVG: clicked parallelogram')}
                />
              </LogicDetector>
            </Svg>
          </NativeDetector>
        </View>
        <Text>
          Tapping each color should read to a different console.log output
        </Text>
      </View>
      <View style={styles.container}>
        <Text style={styles.header}>SvgView with SvgView with ViewBox</Text>
        <View style={{ backgroundColor: 'tomato' }}>
          <NativeDetector gesture={vbContainerTap}>
            <Svg
              height="250"
              width="250"
              viewBox="-50 -50 150 150"
              onPress={() => console.log('SVG: clicked viewbox container')}>
              <LogicDetector gesture={vbInnerContainerTap}>
                <Svg
                  height="250"
                  width="250"
                  viewBox="-300 -300 600 600"
                  onPress={() =>
                    console.log('SVG: clicked inner viewbox container')
                  }>
                  <Rect
                    x="-300"
                    y="-300"
                    width="600"
                    height="600"
                    fill="yellow"
                  />
                  <LogicDetector gesture={vbCircleTap}>
                    <Circle
                      r="300"
                      fill="green"
                      onPress={() => console.log('SVG: clicked viewbox circle')}
                    />
                  </LogicDetector>
                </Svg>
              </LogicDetector>
            </Svg>
          </NativeDetector>
        </View>
        <Text>The viewBox property remaps SVG's coordinate space</Text>
      </View>
    </View >
  );
}

const styles = StyleSheet.create({
  container: {
    alignItems: 'center',
    justifyContent: 'center',
    marginBottom: 48,
  },
  header: {
    fontSize: 18,
    fontWeight: 'bold',
    margin: 10,
  },
});

[j-piasecki]

Did a fist pass, but some stuff will likely change due to #3682 :/

Anyway, you have the hardest part figured out, now it's just a matter of polishing it up.

[j-piasecki]

I'm not sure this is the way to go. Once #3682 is merged, you should be able to cleanly handle all cases in hooks it adds.

[akwasniewski]

Unfortunately changes in #3682 did not help in any way, this workaround is the best thing we could do.

[j-piasecki]

Are reanimated events still triggered on the UI thread this way?

[akwasniewski]

It were not triggered on the ui thread. It is finally solved in https://github.com/software-mansion/react-native-gesture-handler/pull/3689/commits. Thank you for your help in office.

[j-piasecki]

Any chance to extract this to a method? It repeats for every event.

[akwasniewski]

Okay, e6fbdba

[j-piasecki]

We can go deeper - you can change the new function to return a function which will handle the event and assign that directly to the event handler, i.e.:

        onGestureHandlerStateChange={createGestureEventHandler('onGestureHandlerStateChange', e);}

instead of

        onGestureHandlerStateChange={(e) => {
          handleGestureEvent('onGestureHandlerStateChange', e);
        }}

[akwasniewski]

That is a really good idea, thanks. Done in 832a039

[m-bert]

I know this is not exact place, but shouldn't we also update canCoalesce above?

[m-bert]

Also I'm not sure about TouchEvents, since they have touchesMove callback (cc @j-piasecki)

[m-bert]

I'm not sure if I like this approach, could we extract it to a function and then call it in both?

[m-bert]

At this point, isn't attachedNativeHandlers empty? If so, maybe add a comment that it is intentionally empty?

[m-bert]

Maybe it won't change much in the optimization, but we can create EMPTY_SET variable and then pass it to avoid calling these constructors.

[m-bert]

if(!node) {
  return;
}

[m-bert]

Is this cast necessary?

I'll be on vacation and I don't want to block this PR - I believe it will be good to go when @j-piasecki approves it :D