Skip to content

Commit

Permalink
Fix nested taps activation (#2759)
Browse files Browse the repository at this point in the history
## Description

While working on #2739 we discovered that our orchestration process doesn't work as expected when we have nested `Tap` gestures . This PR introduces few changes that make those gestures work as intended. 

Fixes #2739 

## Test plan

Tested on example app and on modified code from issue.

<details>
<summary> Test code </summary>

```jsx
import React from 'react';
import { StyleSheet, View } from 'react-native';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';
import Animated from 'react-native-reanimated';

const log = (e: any, handler: string) =>
  console.log(`[${e.handlerTag}][${handler}]: ${e.state}`);

const orangeTapGesture = Gesture.Tap()
  .runOnJS(true)
  .maxDeltaX(5)
  .onBegin((e) => log(e, 'Orange tap'))
  .onStart((e) => log(e, 'Orange tap'))
  .onFinalize((e, success) => {
    log(e, 'Orange tap');
    if (success) {
      console.log('---> Orange Tap <---');
    }
  });

const orangeDoubleTapGesture = Gesture.Tap()
  .numberOfTaps(2)
  .runOnJS(true)
  .onBegin((e) => log(e, 'Orange Double Tap'))
  .onStart((e) => log(e, 'Orange Double Tap'))
  .onFinalize((e, success) => {
    log(e, 'Orange Double Tap');
    if (success) {
      console.log('---> Orange Double Tap <---');
    }
  });

const blueTapGesture = Gesture.Tap()
  .runOnJS(true)
  .requireExternalGestureToFail(orangeTapGesture, orangeDoubleTapGesture)
  .onBegin((e) => log(e, 'Blue Tap'))
  .onStart((e) => log(e, 'Blue Tap'))
  .onFinalize((e, success) => {
    log(e, 'Blue Tap');
    if (success) {
      console.log('---> Blue Tap <---');
    }
  });

const blueDoubleTapGesture = Gesture.Tap()
  .numberOfTaps(2)
  .runOnJS(true)
  .onBegin((e) => log(e, 'Blue Double Tap'))
  .onStart((e) => log(e, 'Blue Double Tap'))
  .onFinalize((e, success) => {
    log(e, 'Blue Double Tap');
    if (success) {
      console.log('---> Blue Double Tap <---');
    }
  });

const redLongPressGesture = Gesture.LongPress()
  .runOnJS(true)
  .onBegin((e) => log(e, 'Red Longpress'))
  .onStart((e) => log(e, 'Red Longpress'))
  .onFinalize((e, success) => {
    log(e, 'Red Longpress');
    if (success) {
      console.log('---> Red Longpress <---');
    }
  });

const redDoubleTapGesture = Gesture.Tap()
  .numberOfTaps(2)
  .runOnJS(true)
  .onBegin((e) => log(e, 'Red Double Tap'))
  .onStart((e) => log(e, 'Red Double Tap'))
  .onFinalize((e, success) => {
    log(e, 'Red Double Tap');
    if (success) {
      console.log('---> Red Double Tap <---');
    }
  });

const Showcase = () => {
  const gesture = Gesture.Race(redDoubleTapGesture, redLongPressGesture);

  return (
    <View style={[styles.center, { flex: 1, backgroundColor: 'white' }]}>
      <GestureDetector gesture={gesture}>
        <Animated.View style={[styles.outer, styles.center]}>
          <Blue />
        </Animated.View>
      </GestureDetector>
    </View>
  );
};

export default Showcase;

const Blue = () => {
  const composedGesture = Gesture.Exclusive(
    blueDoubleTapGesture,
    blueTapGesture
  );

  return (
    <GestureDetector gesture={composedGesture}>
      <Animated.View style={[styles.blue, styles.center]}>
        <Orange />
      </Animated.View>
    </GestureDetector>
  );
};

const Orange = () => {
  const composedGesture = Gesture.Exclusive(
    orangeDoubleTapGesture,
    orangeTapGesture
  );

  return (
    <GestureDetector gesture={composedGesture}>
      <Animated.View style={styles.orange} />
    </GestureDetector>
  );
};

const styles = StyleSheet.create({
  center: {
    display: 'flex',
    justifyContent: 'space-around',
    alignItems: 'center',
  },

  outer: {
    backgroundColor: 'red',
    width: 400,
    height: 400,
    borderRadius: 200,
  },
  blue: {
    width: 250,
    height: 250,
    backgroundColor: 'blue',

    borderRadius: 175,
  },
  orange: {
    width: 100,
    height: 100,
    backgroundColor: 'orange',
    borderRadius: 50,
  },
});

```

</details>
  • Loading branch information
m-bert authored Mar 4, 2024
1 parent 369e45c commit a341f88
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ open class GestureHandler<ConcreteGestureHandlerT : GestureHandler<ConcreteGestu
}

fun cancel() {
if (state == STATE_ACTIVE || state == STATE_UNDETERMINED || state == STATE_BEGAN) {
if (state == STATE_ACTIVE || state == STATE_UNDETERMINED || state == STATE_BEGAN || this.isAwaiting) {
onCancel()
moveToState(STATE_CANCELLED)
}
Expand Down Expand Up @@ -762,7 +762,6 @@ open class GestureHandler<ConcreteGestureHandlerT : GestureHandler<ConcreteGestu
orchestrator = null
Arrays.fill(trackedPointerIDs, -1)
trackedPointersIDsCount = 0

trackedPointersCount = 0
trackedPointers.fill(null)
touchEventType = RNGestureHandlerTouchEvent.EVENT_UNDETERMINED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import android.view.View
import android.view.ViewGroup
import android.widget.EditText
import java.util.*
import kotlin.collections.HashSet

class GestureHandlerOrchestrator(
private val wrapperView: ViewGroup,
Expand All @@ -22,6 +23,14 @@ class GestureHandlerOrchestrator(
private val gestureHandlers = arrayListOf<GestureHandler<*>>()
private val awaitingHandlers = arrayListOf<GestureHandler<*>>()
private val preparedHandlers = arrayListOf<GestureHandler<*>>()

// In `onHandlerStateChange` method we iterate through `awaitingHandlers`, but calling `tryActivate` may modify this list.
// To avoid `ConcurrentModificationException` we iterate through copy. There is one more problem though - if handler was
// removed from `awaitingHandlers`, it was still present in copy of original list. This hashset helps us identify which handlers
// are really inside `awaitingHandlers`.
// `contains` method on HashSet has O(1) complexity, so calling it inside for loop won't result in O(n^2) (contrary to ArrayList)
private val awaitingHandlersTags = HashSet<Int>()

private var isHandlingTouch = false
private var handlingChangeSemaphore = 0
private var finishedHandlersCleanupScheduled = false
Expand Down Expand Up @@ -73,16 +82,18 @@ class GestureHandlerOrchestrator(
finishedHandlersCleanupScheduled = false
}

private fun hasOtherHandlerToWaitFor(handler: GestureHandler<*>): Boolean {
for (otherHandler in gestureHandlers) {
if (!isFinished(otherHandler.state) && shouldHandlerWaitForOther(handler, otherHandler)) {
return true
}
}
return false
}
private fun hasOtherHandlerToWaitFor(handler: GestureHandler<*>) =
gestureHandlers.any { !isFinished(it.state) && shouldHandlerWaitForOther(handler, it) }

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

private fun tryActivate(handler: GestureHandler<*>) {
// If we are waiting for a gesture that has successfully finished, we should cancel handler
if (shouldBeCancelledByFinishedHandler(handler)) {
handler.cancel()
return
}

// see if there is anyone else who we need to wait for
if (hasOtherHandlerToWaitFor(handler)) {
addAwaitingHandler(handler)
Expand All @@ -94,7 +105,14 @@ class GestureHandlerOrchestrator(
}

private fun cleanupAwaitingHandlers() {
awaitingHandlers.removeAll { !it.isAwaiting }
val awaitingHandlersCopy = awaitingHandlers.toList()

for (handler in awaitingHandlersCopy) {
if (!handler.isAwaiting) {
awaitingHandlers.remove(handler)
awaitingHandlersTags.remove(handler.tag)
}
}
}

/*package*/
Expand All @@ -107,7 +125,7 @@ class GestureHandlerOrchestrator(

// if there were handlers awaiting completion of this handler, we can trigger active state
for (otherHandler in currentlyAwaitingHandlers) {
if (!shouldHandlerWaitForOther(otherHandler, handler)) {
if (!shouldHandlerWaitForOther(otherHandler, handler) || !awaitingHandlersTags.contains(otherHandler.tag)) {
continue
}

Expand Down Expand Up @@ -174,7 +192,6 @@ class GestureHandlerOrchestrator(
// Clear all awaiting handlers waiting for the current handler to fail
for (otherHandler in awaitingHandlers.reversed()) {
if (shouldHandlerBeCancelledBy(otherHandler, handler)) {
otherHandler.cancel()
otherHandler.isAwaiting = false
}
}
Expand Down Expand Up @@ -389,6 +406,8 @@ class GestureHandlerOrchestrator(
}

awaitingHandlers.add(handler)
awaitingHandlersTags.add(handler.tag)

with(handler) {
isAwaiting = true
activationIndex = this@GestureHandlerOrchestrator.activationIndex++
Expand Down
13 changes: 13 additions & 0 deletions src/web/tools/GestureHandlerOrchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,20 @@ export default class GestureHandlerOrchestrator {
return hasToWait;
}

private shouldBeCancelledByFinishedHandler(handler: GestureHandler): boolean {
return this.gestureHandlers.some(
(otherHandler) =>
this.shouldHandlerWaitForOther(handler, otherHandler) &&
otherHandler.getState() === State.END
);
}

private tryActivate(handler: GestureHandler): void {
if (this.shouldBeCancelledByFinishedHandler(handler)) {
handler.cancel();
return;
}

if (this.hasOtherHandlerToWaitFor(handler)) {
this.addAwaitingHandler(handler);
} else if (
Expand Down

0 comments on commit a341f88

Please sign in to comment.