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

GestureDetector is not responsive #2920

Closed
rezkiy37 opened this issue May 20, 2024 · 6 comments · Fixed by #2925
Closed

GestureDetector is not responsive #2920

rezkiy37 opened this issue May 20, 2024 · 6 comments · Fixed by #2925
Labels
Can repro It is confirmed we can reproduce the issue Platform: iOS This issue is specific to iOS

Comments

@rezkiy37
Copy link

rezkiy37 commented May 20, 2024

Description

The app uses GestureDetector to handle a tap on the send button. GestureDetector works properly when the user opens a chat screen. However, if the user opens nested chats (threads), returns to the 1st one, and taps on the send button, GestureDetector does not react to this action. The expected behavior is to have the button responsive.

Note: the bug is replicated for a case when there are 2 nested screens e.g. Main chat -> Subchat -> Subchat.

There is an issue with the Expensify app - Expensify/App#41528.

Video

Bug6469224_1714674407897.RPReplay_Final1714674254.mp4

Looks like at some moment GestureDetector loses a "connection" for the initial screen send button.

I've tried to do a force update for GestureDetector via setting and updating a key when the screen is focused, and it helps. So it requires a rerender of GestureDetector to refresh the "connection" and handle tap events.

Details

Screenshot 2024-05-20 at 16 32 24

Steps to reproduce

  1. Launch the "New Expensify" app on IOS.
  2. Click on "+".
  3. Start a chat with someone.
  4. Open the chat.
  5. Click on "+" on the left of the input.
  6. Click "Submit expense".
  7. Submit this expense.
  8. Submit one more expense.
  9. Click on the newly created expense preview in the chat.
  10. Click on any expense preview.
  11. Go back to the main chat.
  12. Enter any message.
  13. Click on the send button on the right of the input.
  14. Current behavior: the send button is not responsive. Expected behavior: the message is sent.

Snack or a link to a repository

https://github.com/Expensify/App

Gesture Handler version

2.14.1

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 Max (IOS 17.5)

Acknowledgements

Yes

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

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?

@rezkiy37
Copy link
Author

I've attached the link - https://github.com/Expensify/App.

@j-piasecki j-piasecki added Can repro It is confirmed we can reproduce the issue and removed Missing repro labels May 22, 2024
@j-piasecki
Copy link
Member

I spent some time looking into this and I managed to track it down to be caused by react-freeze (so essentially by Suspense).

This is enough to reproduce the issue:

import {useState} from 'react';
import {Freeze} from 'react-freeze';
import {View, Button, SafeAreaView} from 'react-native';
import {
  Gesture,
  GestureDetector,
  GestureHandlerRootView,
} from 'react-native-gesture-handler';

export default function FirstScreen() {
  const [frozen, setFrozen] = useState(false);
  const tap = Gesture.Tap().onStart(() => {
    console.log('Tap');
  });

  return (
    <GestureHandlerRootView style={{flex: 1}}>
      <SafeAreaView style={{flex: 1, backgroundColor: 'red'}}>
        <Button title="Unfreeze" onPress={() => setFrozen(false)} />
        <Freeze freeze={frozen}>
          <View>
            <GestureDetector gesture={tap}>
              <View
                style={{width: 100, height: 100, backgroundColor: 'blue'}}
              />
            </GestureDetector>
            <View style={{width: 100, height: 100, backgroundColor: 'green'}} />
            <Button title="Freeze" onPress={() => setFrozen(true)} />
          </View>
        </Freeze>
      </SafeAreaView>
    </GestureHandlerRootView>
  );
}

It's only reproducible on iOS, because Android keeps hidden views mounted (at the moment of writing): https://github.com/facebook/react-native/blob/9dfcb9ec3ae6987a3231e4d6dd2bf8fb557440c8/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp#L217-L223

I will come back to it next week, after App.js conf.

j-piasecki added a commit that referenced this issue May 28, 2024
…#2925)

## Description

Fixes
#2920

When the subtree containing `GestureDetector` was suspended, all the
native views were unmounted without the `Detector` component being
aware. As a consequence, the view with the attached native recognizer
was put in the recycling pool and then reused in other places in the UI
(which is bad). Then, when the tree was unsuspended, the gesture didn't
work correctly - again, `Detector` wasn't aware of any change, and a
different native view was put as its child so the native recognizer
wasn't attached to it.

This PR changes the effect responsible for the initial setup and cleanup
to be a layout effect, which gets triggered when the tree is suspended.
This means that when tree suspends, native recognizers will be dropped
and recreated when the tree unsuspends.

Note that this will only fix the issue on RN 0.74, since
`useLayoutEffect` is broken on versions below that.

## Test plan

<details>
<summary>Tested on the example app and the following code</summary>

```jsx
import {useState} from 'react';
import {Freeze} from 'react-freeze';
import {View, Button, SafeAreaView} from 'react-native';
import {
  Gesture,
  GestureDetector,
  GestureHandlerRootView,
} from 'react-native-gesture-handler';

export default function FirstScreen() {
  const [frozen, setFrozen] = useState(false);
  const tap = Gesture.Tap().onStart(() => {
    console.log('Tap');
  });

  return (
    <GestureHandlerRootView style={{flex: 1}}>
      <SafeAreaView style={{flex: 1, backgroundColor: 'red'}}>
        <Button title="Unfreeze" onPress={() => setFrozen(false)} />
        <Freeze freeze={frozen}>
          <View>
            <GestureDetector gesture={tap}>
              <View
                style={{width: 100, height: 100, backgroundColor: 'blue'}}
              />
            </GestureDetector>
            <View style={{width: 100, height: 100, backgroundColor: 'green'}} />
            <Button title="Freeze" onPress={() => setFrozen(true)} />
          </View>
        </Freeze>
      </SafeAreaView>
    </GestureHandlerRootView>
  );
}
```
</details>
@rezkiy37
Copy link
Author

Thank you guys! I am going to test it as well 🙂

@rezkiy37
Copy link
Author

Btw, when can we expect the next release?

@j-piasecki
Copy link
Member

Btw, when can we expect the next release?

Next week at the earliest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can repro It is confirmed we can reproduce the issue Platform: iOS This issue is specific to iOS
Projects
None yet
2 participants