Skip to content

Commit

Permalink
fix(Fabric): back gesture activates Pressable elements (#2131)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
kkafar committed May 15, 2024
1 parent 5f32c70 commit fb0ce28
Show file tree
Hide file tree
Showing 9 changed files with 273 additions and 8 deletions.
1 change: 1 addition & 0 deletions FabricTestExample/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ import Test2008 from './src/Test2008';
import Test2028 from './src/Test2028';
import Test2048 from './src/Test2048';
import Test2069 from './src/Test2069';
import Test2118 from './src/Test2118';

enableFreeze(true);

Expand Down
88 changes: 88 additions & 0 deletions FabricTestExample/src/Test2118.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import React from 'react';
import { View, Text, Button, Pressable, StyleSheet, Alert } from 'react-native';
import { NavigationContainer, useNavigation } from '@react-navigation/native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';

const Stack = createNativeStackNavigator(); // <-- change to createStackNavigator to see a difference

const ModalStack = createNativeStackNavigator();

export default function App() {
return (
<NavigationContainer>
<Stack.Navigator screenOptions={{ animation: 'slide_from_left' }}>
<Stack.Screen name="Home" component={HomeScreen} />
<Stack.Screen name="DetailsStack" component={DetailsScreen} />
<Stack.Screen
name="StackInModal"
component={StackInModal}
options={{ presentation: 'modal' }}
/>
</Stack.Navigator>
</NavigationContainer>
);
}

function HomeScreen() {
const navigation = useNavigation();
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
<Text>Home Screen</Text>
<Button
title="Go to Details"
onPress={() => navigation.navigate('DetailsStack')}
/>
<Button
title="Go to StackInModal"
onPress={() => navigation.navigate('StackInModal')}
/>
</View>
);
}

function DetailsScreen() {
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
{new Array(10).fill(0).map((_, i) => (
<Pressable
key={i.toString()}
onPress={() => {
Alert.alert('Pressed!');
}}
style={({ pressed }) => [
{
backgroundColor: pressed ? 'rgb(210, 230, 255)' : 'white',
},
styles.wrapperCustom,
]}>
{({ pressed }) => (
<Text style={styles.text}>{pressed ? 'Pressed!' : 'Press Me'}</Text>
)}
</Pressable>
))}
</View>
);
}

function StackInModal() {
return (
<ModalStack.Navigator>
<ModalStack.Screen name="ModalHome" component={HomeScreen} />
<ModalStack.Screen name="ModalDetails" component={DetailsScreen} />
</ModalStack.Navigator>
);
}

const styles = StyleSheet.create({
wrapperCustom: {
width: '100%',
height: 100,
marginHorizontal: 30,
borderRadius: 10,
margin: 10,
},
text: {
fontSize: 20,
color: 'black',
},
});
1 change: 1 addition & 0 deletions TestsExample/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ import Test1981 from './src/Test1981';
import Test2008 from './src/Test2008';
import Test2048 from './src/Test2048';
import Test2069 from './src/Test2069';
import Test2118 from './src/Test2118';

enableFreeze(true);

Expand Down
88 changes: 88 additions & 0 deletions TestsExample/src/Test2118.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import React from 'react';
import { View, Text, Button, Pressable, StyleSheet, Alert } from 'react-native';
import { NavigationContainer, useNavigation } from '@react-navigation/native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';

const Stack = createNativeStackNavigator(); // <-- change to createStackNavigator to see a difference

const ModalStack = createNativeStackNavigator();

export default function App() {
return (
<NavigationContainer>
<Stack.Navigator screenOptions={{ animation: 'slide_from_left' }}>
<Stack.Screen name="Home" component={HomeScreen} />
<Stack.Screen name="DetailsStack" component={DetailsScreen} />
<Stack.Screen
name="StackInModal"
component={StackInModal}
options={{ presentation: 'modal' }}
/>
</Stack.Navigator>
</NavigationContainer>
);
}

function HomeScreen() {
const navigation = useNavigation();
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
<Text>Home Screen</Text>
<Button
title="Go to Details"
onPress={() => navigation.navigate('DetailsStack')}
/>
<Button
title="Go to StackInModal"
onPress={() => navigation.navigate('StackInModal')}
/>
</View>
);
}

function DetailsScreen() {
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
{new Array(10).fill(0).map((_, i) => (
<Pressable
key={i.toString()}
onPress={() => {
Alert.alert('Pressed!');
}}
style={({ pressed }) => [
{
backgroundColor: pressed ? 'rgb(210, 230, 255)' : 'white',
},
styles.wrapperCustom,
]}>
{({ pressed }) => (
<Text style={styles.text}>{pressed ? 'Pressed!' : 'Press Me'}</Text>
)}
</Pressable>
))}
</View>
);
}

function StackInModal() {
return (
<ModalStack.Navigator>
<ModalStack.Screen name="ModalHome" component={HomeScreen} />
<ModalStack.Screen name="ModalDetails" component={DetailsScreen} />
</ModalStack.Navigator>
);
}

const styles = StyleSheet.create({
wrapperCustom: {
width: '100%',
height: 100,
marginHorizontal: 30,
borderRadius: 10,
margin: 10,
},
text: {
fontSize: 20,
color: 'black',
},
});
44 changes: 36 additions & 8 deletions ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
#ifdef RCT_NEW_ARCH_ENABLED
#import <React/RCTFabricComponentsPlugins.h>
#import <React/RCTFabricSurface.h>
#import <React/RCTMountingTransactionObserving.h>
#import <React/RCTSurfaceTouchHandler.h>
#import <React/RCTSurfaceView.h>
#import <React/UIView+React.h>
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
#import <react/renderer/components/rnscreens/EventEmitters.h>
#import <react/renderer/components/rnscreens/Props.h>
#import <react/renderer/components/rnscreens/RCTComponentViewHelpers.h>
#import "RCTSurfaceTouchHandler+RNSUtility.h"
#else
#import <React/RCTBridge.h>
#import <React/RCTRootContentView.h>
#import <React/RCTShadowView.h>
#import <React/RCTTouchHandler.h>
#import <React/RCTUIManager.h>
#import <React/RCTUIManagerUtils.h>
#import "RCTTouchHandler+RNSUtility.h"
#endif // RCT_NEW_ARCH_ENABLED

#import "RNSScreen.h"
Expand Down Expand Up @@ -731,19 +735,43 @@ - (void)cancelTouchesInParent
// item is close to an edge and we start pulling from edge we want the Touchable to be cancelled.
// Without the below code the Touchable will remain active (highlighted) for the duration of back
// gesture and onPress may fire when we release the finger.
#ifdef RCT_NEW_ARCH_ENABLED
// On Fabric there is no view that exposes touchHandler above us in the view hierarchy, however it is still
// utilised. `RCTSurfaceView` should be present above us, which hosts `RCTFabricSurface` instance, which in turn
// hosts `RCTSurfaceTouchHandler` as a private field. When initialised, `RCTSurfaceTouchHandler` is attached to the
// surface view as a gestureRecognizer <- and this is where we can lay our hands on it.
UIView *parent = _controller.view;
while (parent != nil && ![parent respondsToSelector:@selector(touchHandler)])
while (parent != nil && ![parent isKindOfClass:RCTSurfaceView.class]) {
parent = parent.superview;
if (parent != nil) {
#ifdef RCT_NEW_ARCH_ENABLED
RCTSurfaceTouchHandler *touchHandler = [parent performSelector:@selector(touchHandler)];
}

// This could be possible in modal context
if (parent == nil) {
return;
}

RCTSurfaceTouchHandler *touchHandler = nil;
// Experimentation shows that RCTSurfaceTouchHandler is the only gestureRecognizer registered here,
// so we should not be afraid of any performance hit here.
for (UIGestureRecognizer *recognizer in parent.gestureRecognizers) {
if ([recognizer isKindOfClass:RCTSurfaceTouchHandler.class]) {
touchHandler = static_cast<RCTSurfaceTouchHandler *>(recognizer);
}
}

[touchHandler rnscreens_cancelTouches];
#else
// On Paper we can access touchHandler hosted by `RCTRootContentView` which should be above ScreenStack
// in view hierarchy.
UIView *parent = _controller.view;
while (parent != nil && ![parent respondsToSelector:@selector(touchHandler)]) {
parent = parent.superview;
}
if (parent != nil) {
RCTTouchHandler *touchHandler = [parent performSelector:@selector(touchHandler)];
#endif
[touchHandler setEnabled:NO];
[touchHandler setEnabled:YES];
[touchHandler reset];
[touchHandler rnscreens_cancelTouches];
}
#endif // RCT_NEW_ARCH_ENABLED
}

- (BOOL)gestureRecognizerShouldBegin:(UIGestureRecognizer *)gestureRecognizer
Expand Down
15 changes: 15 additions & 0 deletions ios/utils/RCTSurfaceTouchHandler+RNSUtility.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifdef RCT_NEW_ARCH_ENABLED

#import <React/RCTSurfaceTouchHandler.h>

NS_ASSUME_NONNULL_BEGIN

@interface RCTSurfaceTouchHandler (RNSUtility)

- (void)rnscreens_cancelTouches;

@end

NS_ASSUME_NONNULL_END

#endif // RCT_NEW_ARCH_ENABLED
14 changes: 14 additions & 0 deletions ios/utils/RCTSurfaceTouchHandler+RNSUtility.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#ifdef RCT_NEW_ARCH_ENABLED
#import "RCTSurfaceTouchHandler+RNSUtility.h"

@implementation RCTSurfaceTouchHandler (RNSUtility)

- (void)rnscreens_cancelTouches
{
[self setEnabled:NO];
[self setEnabled:YES];
[self reset];
}

@end
#endif // RCT_NEW_ARCH_ENABLED
15 changes: 15 additions & 0 deletions ios/utils/RCTTouchHandler+RNSUtility.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef RCT_NEW_ARCH_ENABLED

#import <React/RCTTouchHandler.h>

NS_ASSUME_NONNULL_BEGIN

@interface RCTTouchHandler (RNSUtility)

- (void)rnscreens_cancelTouches;

@end

NS_ASSUME_NONNULL_END

#endif // !RCT_NEW_ARCH_ENABLED
15 changes: 15 additions & 0 deletions ios/utils/RCTTouchHandler+RNSUtility.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef RCT_NEW_ARCH_ENABLED
#import "RCTTouchHandler+RNSUtility.h"

@implementation RCTTouchHandler (RNSUtility)

- (void)rnscreens_cancelTouches
{
[self setEnabled:NO];
[self setEnabled:YES];
[self reset];
}

@end

#endif // !RCT_NEW_ARCH_ENABLED

0 comments on commit fb0ce28

Please sign in to comment.