Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] back gesture activates Pressable element #2118

Closed
kirillzyusko opened this issue May 2, 2024 · 2 comments · Fixed by #2131
Closed

[iOS] back gesture activates Pressable element #2118

kirillzyusko opened this issue May 2, 2024 · 2 comments · Fixed by #2131
Assignees
Labels
Area: Native Stack Bug Something isn't working Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided

Comments

@kirillzyusko
Copy link
Contributor

kirillzyusko commented May 2, 2024

Description

If your Pressable "touches" the border of the screen and you do a back gesture on iOS, then after finger releasing this Pressable will be activated.

A difference between native-stack and stack:

Stack Native-Stack
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-05-02.at.15.33.58.mp4
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-05-02.at.15.33.27.mp4

The issue can be reproducible in RNS example app, so I'm just posting a code sample instead of providing a full reproduction repo (let me know if it's not sufficient - I'll post full reproduction example then).

import React, { useEffect } from 'react';
import {View, Text, Button, Pressable, StyleSheet, Alert} from 'react-native';
import {GestureHandlerRootView} from 'react-native-gesture-handler';

const fill = {flex: 1};

import { NavigationContainer } from '@react-navigation/native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';
import { createStackNavigator } from '@react-navigation/stack';

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

const styles = StyleSheet.create({
    wrapperCustom: {
        width: "100%",
        height: 100,
        marginHorizontal: 30,
        borderRadius: 10,
        margin: 10,
    },
    text: {
        fontSize: 20,
        color: 'black',
    },
});

function DetailsScreen({ navigation }) {
    return (
      <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
        {new Array(10).fill(0).map((_, i) => (
          <Pressable
            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>
    );
  }

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

function App() {
    return (
        <GestureHandlerRootView style={fill}>
            <NavigationContainer>
                <Stack.Navigator screenOptions={{animation: 'slide_from_left'}}>
                    <Stack.Screen name="Home" component={HomeScreen} />
                    <Stack.Screen name="DetailsStack" component={DetailsScreen} />
                </Stack.Navigator>
            </NavigationContainer>
        </GestureHandlerRootView>
    );
}

export default App;

Steps to reproduce

  1. go to details screen
  2. swipe from left
  3. release a finger

Expected result

Alert shouldn't be shown

Actual result

Alert is shown

Snack or a link to a repository

https://github.com/software-mansion/react-native-screens

Screens version

3.31.0

React Native version

0.73.4

Platforms

iOS

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Fabric (New Architecture)

Build type

Debug mode

Device

iOS simulator

Device model

iPhone 15 Pro (iOS 17.4)

Acknowledgements

Yes

@github-actions github-actions bot added Platform: iOS This issue is specific to iOS Missing repro This issue need minimum repro scenario labels May 2, 2024
Copy link

github-actions bot commented May 2, 2024

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

@kkafar
Copy link
Member

kkafar commented May 2, 2024

Good catch & thanks for report! Can confirm it happens.

@kkafar kkafar self-assigned this May 2, 2024
@kkafar kkafar added Bug Something isn't working Area: Native Stack Repro provided A reproduction with a snack or repo is provided and removed Missing repro This issue need minimum repro scenario labels May 2, 2024
kkafar added a commit that referenced this issue May 15, 2024
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Native Stack Bug Something isn't working Platform: iOS This issue is specific to iOS Repro provided A reproduction with a snack or repo is provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants