Skip to content

Commit fb0ce28

Browse files
kkafarkkafar
authored
fix(Fabric): back gesture activates Pressable elements (#2131)
## Description When cancelling touches we previously relied on `touchHandler` instance being exposed by some react managed view above us (screen stack) in the view hierarchy (on Paper it was `RCTRootContentView`). However this changed with some recent Fabric version and no view exposes such property any longer, despite the fact that touch handler is obviously still utilised by these views even on Fabric, but the field has been removed from public API. Currently on Fabric (as of version `0.74.1`) `RCTSurfaceTouchHandler` is owned by instance of `RCTFabricSurface` owned by instance of `RCTSurfaceView` which we can expect to live above us in the view hierarchy (RN mounts it in the very beginning of application runtime). To get access to the private touchHandler (which is a `UIGestureRecognizer`) we observe that it is attached to the `RCTSurfaceView` and thus it is retained in its `gestureRecognizer` array (native API). > [!tip] > We could potentially cache the instance of `RCTSurfaceView`, but for now I've come to a conclusion that this is an minor minor computation (there is only one gesture recognizer in the array & the parent lookup is O(log n) (I know this is not a balanced tree, but you know what I mean). However this is always an option for future. > [!note] > I thought of adding warning in case of `RCTSurfaceView` being not present above screen stack in view hierarchy but this might be the case with `modal` stack presentation. Closes #2118 ## Changes * Added utility extensions on `RCTTouchHandler` & `RCTSurfaceTouchHandler` classes to make the code more expressive * Splited `RNSScreenStackView.cancelTouchesInParent` implementation into two separate for Paper & Fabric * In case of Fabric added lookup for `RCTSurfaceView` & got access to its touch handler. ## Test code and steps to reproduce `Test2118` See #2118 for issue description. ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
1 parent 5f32c70 commit fb0ce28

File tree

9 files changed

+273
-8
lines changed

9 files changed

+273
-8
lines changed

‎FabricTestExample/App.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ import Test2008 from './src/Test2008';
9797
import Test2028 from './src/Test2028';
9898
import Test2048 from './src/Test2048';
9999
import Test2069 from './src/Test2069';
100+
import Test2118 from './src/Test2118';
100101

101102
enableFreeze(true);
102103

‎FabricTestExample/src/Test2118.tsx

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import React from 'react';
2+
import { View, Text, Button, Pressable, StyleSheet, Alert } from 'react-native';
3+
import { NavigationContainer, useNavigation } from '@react-navigation/native';
4+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
5+
6+
const Stack = createNativeStackNavigator(); // <-- change to createStackNavigator to see a difference
7+
8+
const ModalStack = createNativeStackNavigator();
9+
10+
export default function App() {
11+
return (
12+
<NavigationContainer>
13+
<Stack.Navigator screenOptions={{ animation: 'slide_from_left' }}>
14+
<Stack.Screen name="Home" component={HomeScreen} />
15+
<Stack.Screen name="DetailsStack" component={DetailsScreen} />
16+
<Stack.Screen
17+
name="StackInModal"
18+
component={StackInModal}
19+
options={{ presentation: 'modal' }}
20+
/>
21+
</Stack.Navigator>
22+
</NavigationContainer>
23+
);
24+
}
25+
26+
function HomeScreen() {
27+
const navigation = useNavigation();
28+
return (
29+
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
30+
<Text>Home Screen</Text>
31+
<Button
32+
title="Go to Details"
33+
onPress={() => navigation.navigate('DetailsStack')}
34+
/>
35+
<Button
36+
title="Go to StackInModal"
37+
onPress={() => navigation.navigate('StackInModal')}
38+
/>
39+
</View>
40+
);
41+
}
42+
43+
function DetailsScreen() {
44+
return (
45+
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
46+
{new Array(10).fill(0).map((_, i) => (
47+
<Pressable
48+
key={i.toString()}
49+
onPress={() => {
50+
Alert.alert('Pressed!');
51+
}}
52+
style={({ pressed }) => [
53+
{
54+
backgroundColor: pressed ? 'rgb(210, 230, 255)' : 'white',
55+
},
56+
styles.wrapperCustom,
57+
]}>
58+
{({ pressed }) => (
59+
<Text style={styles.text}>{pressed ? 'Pressed!' : 'Press Me'}</Text>
60+
)}
61+
</Pressable>
62+
))}
63+
</View>
64+
);
65+
}
66+
67+
function StackInModal() {
68+
return (
69+
<ModalStack.Navigator>
70+
<ModalStack.Screen name="ModalHome" component={HomeScreen} />
71+
<ModalStack.Screen name="ModalDetails" component={DetailsScreen} />
72+
</ModalStack.Navigator>
73+
);
74+
}
75+
76+
const styles = StyleSheet.create({
77+
wrapperCustom: {
78+
width: '100%',
79+
height: 100,
80+
marginHorizontal: 30,
81+
borderRadius: 10,
82+
margin: 10,
83+
},
84+
text: {
85+
fontSize: 20,
86+
color: 'black',
87+
},
88+
});

‎TestsExample/App.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ import Test1981 from './src/Test1981';
9898
import Test2008 from './src/Test2008';
9999
import Test2048 from './src/Test2048';
100100
import Test2069 from './src/Test2069';
101+
import Test2118 from './src/Test2118';
101102

102103
enableFreeze(true);
103104

‎TestsExample/src/Test2118.tsx

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import React from 'react';
2+
import { View, Text, Button, Pressable, StyleSheet, Alert } from 'react-native';
3+
import { NavigationContainer, useNavigation } from '@react-navigation/native';
4+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
5+
6+
const Stack = createNativeStackNavigator(); // <-- change to createStackNavigator to see a difference
7+
8+
const ModalStack = createNativeStackNavigator();
9+
10+
export default function App() {
11+
return (
12+
<NavigationContainer>
13+
<Stack.Navigator screenOptions={{ animation: 'slide_from_left' }}>
14+
<Stack.Screen name="Home" component={HomeScreen} />
15+
<Stack.Screen name="DetailsStack" component={DetailsScreen} />
16+
<Stack.Screen
17+
name="StackInModal"
18+
component={StackInModal}
19+
options={{ presentation: 'modal' }}
20+
/>
21+
</Stack.Navigator>
22+
</NavigationContainer>
23+
);
24+
}
25+
26+
function HomeScreen() {
27+
const navigation = useNavigation();
28+
return (
29+
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
30+
<Text>Home Screen</Text>
31+
<Button
32+
title="Go to Details"
33+
onPress={() => navigation.navigate('DetailsStack')}
34+
/>
35+
<Button
36+
title="Go to StackInModal"
37+
onPress={() => navigation.navigate('StackInModal')}
38+
/>
39+
</View>
40+
);
41+
}
42+
43+
function DetailsScreen() {
44+
return (
45+
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
46+
{new Array(10).fill(0).map((_, i) => (
47+
<Pressable
48+
key={i.toString()}
49+
onPress={() => {
50+
Alert.alert('Pressed!');
51+
}}
52+
style={({ pressed }) => [
53+
{
54+
backgroundColor: pressed ? 'rgb(210, 230, 255)' : 'white',
55+
},
56+
styles.wrapperCustom,
57+
]}>
58+
{({ pressed }) => (
59+
<Text style={styles.text}>{pressed ? 'Pressed!' : 'Press Me'}</Text>
60+
)}
61+
</Pressable>
62+
))}
63+
</View>
64+
);
65+
}
66+
67+
function StackInModal() {
68+
return (
69+
<ModalStack.Navigator>
70+
<ModalStack.Screen name="ModalHome" component={HomeScreen} />
71+
<ModalStack.Screen name="ModalDetails" component={DetailsScreen} />
72+
</ModalStack.Navigator>
73+
);
74+
}
75+
76+
const styles = StyleSheet.create({
77+
wrapperCustom: {
78+
width: '100%',
79+
height: 100,
80+
marginHorizontal: 30,
81+
borderRadius: 10,
82+
margin: 10,
83+
},
84+
text: {
85+
fontSize: 20,
86+
color: 'black',
87+
},
88+
});

‎ios/RNSScreenStack.mm

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
#ifdef RCT_NEW_ARCH_ENABLED
22
#import <React/RCTFabricComponentsPlugins.h>
3+
#import <React/RCTFabricSurface.h>
34
#import <React/RCTMountingTransactionObserving.h>
45
#import <React/RCTSurfaceTouchHandler.h>
6+
#import <React/RCTSurfaceView.h>
57
#import <React/UIView+React.h>
68
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
79
#import <react/renderer/components/rnscreens/EventEmitters.h>
810
#import <react/renderer/components/rnscreens/Props.h>
911
#import <react/renderer/components/rnscreens/RCTComponentViewHelpers.h>
12+
#import "RCTSurfaceTouchHandler+RNSUtility.h"
1013
#else
1114
#import <React/RCTBridge.h>
1215
#import <React/RCTRootContentView.h>
1316
#import <React/RCTShadowView.h>
1417
#import <React/RCTTouchHandler.h>
1518
#import <React/RCTUIManager.h>
1619
#import <React/RCTUIManagerUtils.h>
20+
#import "RCTTouchHandler+RNSUtility.h"
1721
#endif // RCT_NEW_ARCH_ENABLED
1822

1923
#import "RNSScreen.h"
@@ -731,19 +735,43 @@ - (void)cancelTouchesInParent
731735
// item is close to an edge and we start pulling from edge we want the Touchable to be cancelled.
732736
// Without the below code the Touchable will remain active (highlighted) for the duration of back
733737
// gesture and onPress may fire when we release the finger.
738+
#ifdef RCT_NEW_ARCH_ENABLED
739+
// On Fabric there is no view that exposes touchHandler above us in the view hierarchy, however it is still
740+
// utilised. `RCTSurfaceView` should be present above us, which hosts `RCTFabricSurface` instance, which in turn
741+
// hosts `RCTSurfaceTouchHandler` as a private field. When initialised, `RCTSurfaceTouchHandler` is attached to the
742+
// surface view as a gestureRecognizer <- and this is where we can lay our hands on it.
734743
UIView *parent = _controller.view;
735-
while (parent != nil && ![parent respondsToSelector:@selector(touchHandler)])
744+
while (parent != nil && ![parent isKindOfClass:RCTSurfaceView.class]) {
736745
parent = parent.superview;
737-
if (parent != nil) {
738-
#ifdef RCT_NEW_ARCH_ENABLED
739-
RCTSurfaceTouchHandler *touchHandler = [parent performSelector:@selector(touchHandler)];
746+
}
747+
748+
// This could be possible in modal context
749+
if (parent == nil) {
750+
return;
751+
}
752+
753+
RCTSurfaceTouchHandler *touchHandler = nil;
754+
// Experimentation shows that RCTSurfaceTouchHandler is the only gestureRecognizer registered here,
755+
// so we should not be afraid of any performance hit here.
756+
for (UIGestureRecognizer *recognizer in parent.gestureRecognizers) {
757+
if ([recognizer isKindOfClass:RCTSurfaceTouchHandler.class]) {
758+
touchHandler = static_cast<RCTSurfaceTouchHandler *>(recognizer);
759+
}
760+
}
761+
762+
[touchHandler rnscreens_cancelTouches];
740763
#else
764+
// On Paper we can access touchHandler hosted by `RCTRootContentView` which should be above ScreenStack
765+
// in view hierarchy.
766+
UIView *parent = _controller.view;
767+
while (parent != nil && ![parent respondsToSelector:@selector(touchHandler)]) {
768+
parent = parent.superview;
769+
}
770+
if (parent != nil) {
741771
RCTTouchHandler *touchHandler = [parent performSelector:@selector(touchHandler)];
742-
#endif
743-
[touchHandler setEnabled:NO];
744-
[touchHandler setEnabled:YES];
745-
[touchHandler reset];
772+
[touchHandler rnscreens_cancelTouches];
746773
}
774+
#endif // RCT_NEW_ARCH_ENABLED
747775
}
748776

749777
- (BOOL)gestureRecognizerShouldBegin:(UIGestureRecognizer *)gestureRecognizer

‎ios/utils/RCTSurfaceTouchHandler+RNSUtility.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#ifdef RCT_NEW_ARCH_ENABLED
2+
3+
#import <React/RCTSurfaceTouchHandler.h>
4+
5+
NS_ASSUME_NONNULL_BEGIN
6+
7+
@interface RCTSurfaceTouchHandler (RNSUtility)
8+
9+
- (void)rnscreens_cancelTouches;
10+
11+
@end
12+
13+
NS_ASSUME_NONNULL_END
14+
15+
#endif // RCT_NEW_ARCH_ENABLED

‎ios/utils/RCTSurfaceTouchHandler+RNSUtility.mm

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#ifdef RCT_NEW_ARCH_ENABLED
2+
#import "RCTSurfaceTouchHandler+RNSUtility.h"
3+
4+
@implementation RCTSurfaceTouchHandler (RNSUtility)
5+
6+
- (void)rnscreens_cancelTouches
7+
{
8+
[self setEnabled:NO];
9+
[self setEnabled:YES];
10+
[self reset];
11+
}
12+
13+
@end
14+
#endif // RCT_NEW_ARCH_ENABLED

‎ios/utils/RCTTouchHandler+RNSUtility.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#ifndef RCT_NEW_ARCH_ENABLED
2+
3+
#import <React/RCTTouchHandler.h>
4+
5+
NS_ASSUME_NONNULL_BEGIN
6+
7+
@interface RCTTouchHandler (RNSUtility)
8+
9+
- (void)rnscreens_cancelTouches;
10+
11+
@end
12+
13+
NS_ASSUME_NONNULL_END
14+
15+
#endif // !RCT_NEW_ARCH_ENABLED

‎ios/utils/RCTTouchHandler+RNSUtility.mm

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#ifndef RCT_NEW_ARCH_ENABLED
2+
#import "RCTTouchHandler+RNSUtility.h"
3+
4+
@implementation RCTTouchHandler (RNSUtility)
5+
6+
- (void)rnscreens_cancelTouches
7+
{
8+
[self setEnabled:NO];
9+
[self setEnabled:YES];
10+
[self reset];
11+
}
12+
13+
@end
14+
15+
#endif // !RCT_NEW_ARCH_ENABLED

0 commit comments

Comments
 (0)