Skip to content

Commit 8d114ae

Browse files
authored
Fix GestureDetector not working when the underlying view changes (#2092)
## Description `GestureDetector` was not reattaching gestures if the underlying view has changed, which was especially noticeable when using layout animations. This PR updates `GestureDetector` to keep track of the tag of the view it's attached to and to reattach gestures it the tag changes. The second commit also fixes gestures not reattaching when manually changing the underlying view (at the expense of forcing another render), but only when Reanimated is not used. Applying the following patch: <details> <summary>Expand</summary> ```diff diff --git a/node_modules/react-native-reanimated/src/createAnimatedComponent.tsx b/node_modules/react-native-reanimated/src/createAnimatedComponent.tsx index 1cf0c3f..3f22437 100644 --- a/node_modules/react-native-reanimated/src/createAnimatedComponent.tsx +++ b/node_modules/react-native-reanimated/src/createAnimatedComponent.tsx @@ -294,19 +294,12 @@ export default function createAnimatedComponent( const node = this._getEventViewRef(); const attached = new Set(); const nextEvts = new Set(); - let viewTag: number | undefined; + let viewTag: number | undefined = RNRenderer.findHostInstance_DEPRECATED(this)._nativeTag; for (const key in this.props) { const prop = this.props[key]; if (prop instanceof AnimatedEvent) { nextEvts.add((prop as AnimatedEvent).__nodeID); - } else if ( - has('current', prop) && - prop.current instanceof WorkletEventHandler - ) { - if (viewTag === undefined) { - viewTag = prop.current.viewTag; - } } } for (const key in prevProps) { ``` </details> also makes it work when using Reanimated, but I'm not sure whether it's fine to change it this way upstream. This needs to be discussed. ## Test plan Tested on the Example app and on the following code: <details> <summary>Expand</summary> ```jsx import React, { useState } from 'react'; import { Text, View } from 'react-native'; import { FlatList, Gesture, GestureDetector, } from 'react-native-gesture-handler'; import Animated, { BounceIn } from 'react-native-reanimated'; const items = [ { name: 'Item A' }, { name: 'Item B' }, { name: 'Item C' }, { name: 'Item D' }, { name: 'Item A' }, { name: 'Item B' }, { name: 'Item C' }, { name: 'Item D' }, { name: 'Item A' }, { name: 'Item B' }, { name: 'Item C' }, { name: 'Item D' }, { name: 'Item A' }, { name: 'Item B' }, { name: 'Item C' }, { name: 'Item D' }, { name: 'Item A' }, { name: 'Item B' }, { name: 'Item C' }, { name: 'Item D' }, ]; function Item() { const [faved, setFaved] = useState(false); const color = faved ? '#900' : '#aaa'; const tap = Gesture.Tap() .onEnd(() => { setFaved(!faved); }) .runOnJS(true); return ( <GestureDetector gesture={tap}> <Animated.View key={color} entering={BounceIn} style={{ backgroundColor: color, width: 30, height: 30 }} /> </GestureDetector> ); } function renderItem({ item }: { item: { name: string } }) { return ( <View style={{ width: '100%', height: 50, backgroundColor: 'red', flexDirection: 'row', justifyContent: 'space-between', padding: 10, alignItems: 'center', }}> <Text>{item.name}</Text> <Item /> </View> ); } export default function Example() { return ( <View style={{ flex: 1 }}> <FlatList style={{ flex: 1 }} data={items} renderItem={renderItem} /> </View> ); } ``` </details> Code to test the second commit: <details> <summary>Expand</summary> ```jsx import React from 'react'; import { View } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; import Animated from 'react-native-reanimated'; function Item() { console.log('render item'); return ( <Animated.View style={{ alignSelf: 'center', width: 200, height: 200, backgroundColor: 'red', }} /> ); } export default function Example() { const gesture = Gesture.Tap() .onStart(() => { console.log('a', _WORKLET); }) .runOnJS(true); console.log('render parent'); return ( <View style={{ flex: 1 }}> <GestureDetector gesture={gesture}> <Item /> </GestureDetector> </View> ); } ``` Change between `View` and `Animated.View` while the app is running and check if the tap still works. Remove `.runOnJS(true)` to test using Reanimated. </details>
1 parent 57761e4 commit 8d114ae

File tree

1 file changed

+72
-29
lines changed

1 file changed

+72
-29
lines changed

src/handlers/gestures/GestureDetector.tsx

Lines changed: 72 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useEffect, useRef, RefObject } from 'react';
1+
import React, { useEffect, useRef, useState } from 'react';
22
import {
33
GestureType,
44
HandlerCallbacks,
@@ -238,7 +238,7 @@ function updateHandlers(
238238
preparedGesture: GestureConfigReference,
239239
gestureConfig: ComposedGesture | GestureType,
240240
gesture: GestureType[],
241-
mountedRef: RefObject<boolean>
241+
mountedRef: React.RefObject<boolean>
242242
) {
243243
gestureConfig.prepare();
244244

@@ -596,6 +596,12 @@ interface GestureDetectorProps {
596596
userSelect?: UserSelect;
597597
children?: React.ReactNode;
598598
}
599+
interface GestureDetectorState {
600+
firstRender: boolean;
601+
viewRef: React.Component | null;
602+
previousViewTag: number;
603+
forceReattach: boolean;
604+
}
599605
export const GestureDetector = (props: GestureDetectorProps) => {
600606
const gestureConfig = props.gesture;
601607

@@ -605,8 +611,14 @@ export const GestureDetector = (props: GestureDetectorProps) => {
605611

606612
const gesture = gestureConfig.toGestureArray();
607613
const useReanimatedHook = gesture.some((g) => g.shouldUseReanimated);
608-
const viewRef = useRef(null);
609-
const firstRenderRef = useRef(true);
614+
615+
// store state in ref to prevent unnecessary renders
616+
const state = useRef<GestureDetectorState>({
617+
firstRender: true,
618+
viewRef: null,
619+
previousViewTag: -1,
620+
forceReattach: false,
621+
}).current;
610622
const mountedRef = useRef(false);
611623
const webEventHandlersRef = useRef<WebEventHandler>({
612624
onGestureHandlerEvent: (e: HandlerStateChangeEvent<unknown>) => {
@@ -619,6 +631,11 @@ export const GestureDetector = (props: GestureDetectorProps) => {
619631
: undefined,
620632
});
621633

634+
const [renderState, setRenderState] = useState(false);
635+
function forceRender() {
636+
setRenderState(!renderState);
637+
}
638+
622639
const preparedGesture = React.useRef<GestureConfigReference>({
623640
config: gesture,
624641
animatedEventHandler: null,
@@ -635,10 +652,41 @@ export const GestureDetector = (props: GestureDetectorProps) => {
635652
);
636653
}
637654

655+
function onHandlersUpdate(skipConfigUpdate?: boolean) {
656+
// if the underlying view has changed we need to reattach handlers to the new view
657+
const viewTag = findNodeHandle(state.viewRef) as number;
658+
const forceReattach = viewTag !== state.previousViewTag;
659+
660+
if (forceReattach || needsToReattach(preparedGesture, gesture)) {
661+
validateDetectorChildren(state.viewRef);
662+
dropHandlers(preparedGesture);
663+
attachHandlers({
664+
preparedGesture,
665+
gestureConfig,
666+
gesture,
667+
webEventHandlersRef,
668+
viewTag,
669+
mountedRef,
670+
});
671+
672+
state.previousViewTag = viewTag;
673+
state.forceReattach = forceReattach;
674+
if (forceReattach) {
675+
forceRender();
676+
}
677+
} else if (!skipConfigUpdate) {
678+
updateHandlers(preparedGesture, gestureConfig, gesture, mountedRef);
679+
}
680+
}
681+
638682
// Reanimated event should be rebuilt only when gestures are reattached, otherwise
639683
// config update will be enough as all necessary items are stored in shared values anyway
640684
const needsToRebuildReanimatedEvent =
641-
preparedGesture.firstExecution || needsToReattach(preparedGesture, gesture);
685+
preparedGesture.firstExecution ||
686+
needsToReattach(preparedGesture, gesture) ||
687+
state.forceReattach;
688+
689+
state.forceReattach = false;
642690

643691
if (preparedGesture.firstExecution) {
644692
gestureConfig.initialize();
@@ -651,17 +699,18 @@ export const GestureDetector = (props: GestureDetectorProps) => {
651699
}
652700

653701
useEffect(() => {
654-
firstRenderRef.current = true;
702+
const viewTag = findNodeHandle(state.viewRef) as number;
703+
state.firstRender = true;
655704
mountedRef.current = true;
656-
const viewTag = findNodeHandle(viewRef.current) as number;
657705

658-
validateDetectorChildren(viewRef.current);
706+
validateDetectorChildren(state.viewRef);
707+
659708
attachHandlers({
660709
preparedGesture,
661710
gestureConfig,
662711
gesture,
663-
viewTag,
664712
webEventHandlersRef,
713+
viewTag,
665714
mountedRef,
666715
});
667716

@@ -672,32 +721,26 @@ export const GestureDetector = (props: GestureDetectorProps) => {
672721
}, []);
673722

674723
useEffect(() => {
675-
if (!firstRenderRef.current) {
676-
const viewTag = findNodeHandle(viewRef.current) as number;
677-
678-
if (needsToReattach(preparedGesture, gesture)) {
679-
validateDetectorChildren(viewRef.current);
680-
dropHandlers(preparedGesture);
681-
attachHandlers({
682-
preparedGesture,
683-
gestureConfig,
684-
gesture,
685-
viewTag,
686-
webEventHandlersRef,
687-
mountedRef,
688-
});
689-
} else {
690-
updateHandlers(preparedGesture, gestureConfig, gesture, mountedRef);
691-
}
724+
if (!state.firstRender) {
725+
onHandlersUpdate();
692726
} else {
693-
firstRenderRef.current = false;
727+
state.firstRender = false;
694728
}
695729
}, [props]);
696730

697731
const refFunction = (ref: unknown) => {
698732
if (ref !== null) {
699-
//@ts-ignore Just setting the ref
700-
viewRef.current = ref;
733+
// @ts-ignore Just setting the view ref
734+
state.viewRef = ref;
735+
736+
// if it's the first render, also set the previousViewTag to prevent reattaching gestures when not needed
737+
if (state.previousViewTag === -1) {
738+
state.previousViewTag = findNodeHandle(state.viewRef) as number;
739+
}
740+
741+
// pass true as `skipConfigUpdate`, here we only want to trigger the eventual reattaching of handlers
742+
// in case the view has changed, while config update would be handled be the `useEffect` above
743+
onHandlersUpdate(true);
701744

702745
if (isFabric()) {
703746
const node = getShadowNodeFromRef(ref);

0 commit comments

Comments
 (0)