gorhom / react-native-bottom-sheet

A performant interactive bottom sheet with fully configurable options 🚀
https://gorhom.github.io/react-native-bottom-sheet/
MIT License
6.71k stars 742 forks source link

BottomSheetModal crashes app when closed with Back Button #465

Closed alexco2 closed 3 years ago

alexco2 commented 3 years ago

Bug

Hello, I came across a Bug while working with the Backbutton and BottomSheetModal. When the BottomSheetModal ist open and extended, and the user navigates back to the previous screen with the Back Button, while the Modal is extended, the BottomSheetModal can not be opened again after navigation back to the second screen. In porduction mode, I get the error Cannot read property 'minimize' of null, and Maximum call stack size exceeded (native stack depth).h. Also, look here: [https://github.com/gorhom/react-native-portal/issues/15#issuecomment-850577091]()

Environment info

Library Version
@gorhom/bottom-sheet ^3.6.5 && 4.0.0-alpha.11
react-native 0.63
react-native-reanimated ^2.1.0
react-native-gesture-handler ~1.10.2

Steps To Reproduce

Describe what you expected to happen:

  1. The Modal should be able to open itself again.
  2. No crashes

Reproducible sample code

I was able to reproduce the bug in Expo Snack. Here is the Link: Expo Snack Reproductible Sample Code

https://user-images.githubusercontent.com/61253912/120039688-9f9a6f80-c005-11eb-8ccd-82ed21c88ee5.mp4

kingdark1234 commented 3 years ago

I facing like this issue but in my case the error is

Cannot read property 'dismiss' of null

When i click device back button on android

How can i resolve this?

alexco2 commented 3 years ago

@gorhom it would be amazing if you could give a quick feedback on this topic. Did we perhaps miss something out?

gorhom commented 3 years ago

@kingdark1234 @alexco2 i will look into this issue later today 👍

gorhom commented 3 years ago

meanwhile, @alexco2 could you try to reproduce it with v4.alpha11 ?

yarn add @gorhom/bottom-sheet@4.0.0-alpha.11
alexco2 commented 3 years ago

@gorhom thank you! I updated the library in the snack code and was able to reproduce the error after some trying. Sometimes the Bottom Sheet Modal is closing on Backbuttonpress, sometimes it is not. In that case, sometimes the Bottomsheetmodal can not be opened again. I think it happens more often if the BottomSheetModal is expaned fully, I will later try to reproduce it in my vanilla rn project.

alexco2 commented 3 years ago

I was able to reproduce the error in dev mode with the latest version alpa 11. Most of the time, after using the back button when the Bottom Sheet modal is open, the Sheet can not be opened again. Sometimes I also get this error.

alpha 11 error

gorhom commented 3 years ago

@alexco2 i have tried your snack and i did not managed to reproduce it 😩

am i doing something wrong here ?

https://user-images.githubusercontent.com/4061838/121576978-e84c2280-ca20-11eb-9f7a-cd3927e9cfd9.mp4

alexco2 commented 3 years ago

That is really weird. For me I was able to reproduce it directly.

My steps are:

  1. Navigating to second screen
  2. Opening modal
  3. Extending modal
  4. Using hardware back button
  5. Close modal if it is not closed
  6. Navigating back
  7. Trying to open modal again

Perhaps you have to try it more "aggressively"?

https://user-images.githubusercontent.com/61253912/121578375-e4250280-ca2a-11eb-86f0-bada22cd7e5c.mp4

alexco2 commented 3 years ago

@gorhom So I just tried it an a Samsung s7. There I was also not able to reproduce the error. (I have a s21 btw). It seems like the error is just reproducible if you enable gestures instead of buttons in the settings of the app. If I switch to buttons for navigating my phone, I can not reproduce the bug on my phone as well. That's really interesting

gorhom commented 3 years ago

i have tested with different navigation gesture but i couldn't reproduce it either

alexco2 commented 3 years ago

How can I help you further to reproduce the bug?

gorhom commented 3 years ago

@alexco2 thanks for helping so far, i would appreciate if you could test @gorhom/portal v1.0.7 👏

alexco2 commented 3 years ago

@gorhom I tried to reproduce the error in the snack you provided https://snack.expo.io/@gorhom/portal-example, but was not able to. Do you think the bug is in the react-native-portal library?

@kingdark1234 could you try to reproduce the error in the snack I provided? (https://snack.expo.io/@expoco2/bottomsheet-bug-repro)

RohovDmytro commented 3 years ago

Not sure if my input is valuable, but I constantly experiencing this issue.

"@gorhom/bottom-sheet": "^3.6.5",
alexco2 commented 3 years ago

@RohovDmytro can you reporoduce the error in the snack? https://snack.expo.io/@expoco2/bottomsheet-bug-repro

kingdark1234 commented 3 years ago

@alexco2 Hi alexco sorry for late reply I have solved my issue by using ref.current?.close() instant disMissAll() before navigating to another page. It's work for me now.

alexco2 commented 3 years ago

Hello @kingdark1234, I tried your suggestion like the following. Both calling dismiss() with the BackHandler and with useFocusEffect Hook does not work. Also close() did not help. Did you do something different?

useFocusEffect(
    useCallback(() => {
      return () => bottomSheetModalRef.current?.dismiss();
    }, [])
  );

  useEffect(() => {
    const backAction = () => {
      bottomSheetModalRef.current?.dismiss();
      return false;
    };

    const backHandler = BackHandler.addEventListener(
      'hardwareBackPress',
      backAction
    );

    return () => backHandler.remove();
  }, []);
kingdark1234 commented 3 years ago

@alexco2 Edit: my solution does not fix it. In my case i try to reproduce and i found my issue when i open the bottomSheet and don't scroll down to load more data then goBack everything is work well, but when I open bottomSheet and scrolldown to load more when load data success then goback I found this issue but. I think maybe something wrong in my code when handle load more data.

alexco2 commented 3 years ago

@gorhom I upgraded to v4 and it looks like the bug does not appear anymore. I will test further and if I do not encounter this bug anymore I will close this issue.

theMasix commented 2 years ago

I have this issue on v2.4.1. Is there any solution to fix the issue in this version? @gorhom @alexco2