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

Android: Issue with back button handler #189

Closed
hirbod opened this issue May 3, 2020 · 11 comments
Closed

Android: Issue with back button handler #189

hirbod opened this issue May 3, 2020 · 11 comments
Labels
🤕 Valid issue Something isn't working

Comments

@hirbod
Copy link

hirbod commented May 3, 2020

Describe the bug

If I close the modal (which was mounted in a portal) using the "swipe down to close" gesture, my next back button tap will be captured no matter where I am in the navigation hierarchy. After another click it will work normally again.

"Swipe-down-to-close" should remove the Back-button listener and register it again as soon as I open the modal. But if I close the modal with the back-button, there are no problems and everything works as intended.

There is another problem if I render a screen over the modal, for example. Also in this case the following screen cannot trigger a back button event, because the modal is still active and therefore registers the back button tap and closes itself, even though it's not visible in that moment, since another view is on top of it and this view actually should react to the back button (which gets captured through the portal though). I don't know how to fix this though, we might need to pass some ref to the other screen in order to tell modal it should not capture the back button and revert it on pop/goBack.

Dependencies:

  • 1.3.7-rc.28
  • Expo SDK 37
  • react-native-gesture-handler 1.6.0
  • expo, react navigation v5 with react-native-screens native-stack
@hirbod
Copy link
Author

hirbod commented May 3, 2020

#137 is not sufficient enough, because in my case, the Modal is still mounted in the portal but it just got fully swiped down (is invisible).
An invisible modal should not have any back button listeners.

Even if there was still a portion visible (like a page bottom sheet), it should not capture the back button, since it only has a function when its open)

@jeremybarbet
Copy link
Owner

Hi!

From what I see the events should be already handled here and here. Since it's not something I heavily tested and worked on, would you mind come with a snack so it's easier to debug all the different cases.

@jeremybarbet jeremybarbet added the 🤕 Valid issue Something isn't working label May 5, 2020
@hirbod
Copy link
Author

hirbod commented May 23, 2020

Hi @jeremybarbet yeah but those never get called, since you unregister those events in the callback of "useEffect" and the handler will be lost on every re-render.

React native docs say, to use a ref for this.

Something like

const backButtonListener = useRef(null);

// register
backButtonListener.current = ...addEventListener;

// and in your return side-effect
removeEventListener(backButtonListener.current);

I haven't verified it, but it looks like that is the issue. Currently, it's registering multiple backbutton events (if I open the modal 10 times and closing it using the swipe), I have to press the back button 11 times until my navigation receives the handler).

For now, my workaround (its a good idea anyway to save memory and remove views) is to unmount my portal totally when onModalClosed is triggered. I'm using a side-effect now to listen for a "showModal" state change and I wait for InteractionManager.runAfter.. to make sure modelRef is available. Since I unmount, all eventListeners getting removed. It's working great but I would still consider this a bug.

I didn't study the code deeply, just saw those removeListeners in useEffect and thats the first thing that came into my mind (cause I had something similar in the past)

@archansel
Copy link
Contributor

I have this issue too but in my case, it is not only swipe down to close but also any method to close modal except back button (swipe down, backdrop click, and modal.close())

@archansel
Copy link
Contributor

I can confirm example react-navigation in this repo also have the issue

@archansel
Copy link
Contributor

I add some debug code to see whether back handler is executing or not

console.log("handleAnimateOpen BackHandler.addEventListener")
BackHandler.addEventListener("hardwareBackPress", handleBackPress)

console.log("handleAnimateClose BackHandler.removeEventListener")
BackHandler.removeEventListener("hardwareBackPress", handleBackPress)

console.log("useEffect BackHandler.removeEventListener")
BackHandler.removeEventListener("hardwareBackPress", handleBackPress)

This is what the logs look like

=== open modal
 LOG  handleAnimateOpen BackHandler.addEventListener
=== close modal using back press (correct behavior)
 LOG  handleBackPress
 LOG  handleAnimateClose BackHandler.removeEventListener
=== open modal again
 LOG  handleAnimateOpen BackHandler.addEventListener
=== close modal using overlay press or swipe down
 LOG  handleAnimateClose BackHandler.removeEventListener
=== press back button, should navigate back but instead calling modal handleBackPress
 LOG  handleBackPress
 LOG  handleAnimateClose BackHandler.removeEventListener

as you notice, it doesn't event call useEffect BackHandler.removeEventListener

Now following @hirbod suggestion, I use ref for the listener

const backButtonListener = React.useRef(null)

// in handleAnimateOpen
backButtonListener.current = BackHandler.addEventListener("hardwareBackPress", handleBackPress)

// in handleAnimateClose
backButtonListener.current.remove()

// in useEffect cleaning
backButtonListener.current.remove()

And the issue is fixed.

I might be able to create PR but I'm not good with testing though

@archansel
Copy link
Contributor

PR created here #235

@hirbod
Copy link
Author

hirbod commented Jul 1, 2020

Thanks for your PR. Your Fix looks good to me.

@jeremybarbet
Copy link
Owner

Cf: #235

@huzaima
Copy link

huzaima commented Feb 18, 2021

I'm using the most recent version of this library and this issue is still appearing randomly. Is it possible to add a prop and based on that prop we don't handle the back button at all (no BackHandler event subscription) and let the user handle it?

@alessioemireni
Copy link

alessioemireni commented Nov 30, 2023

This issue happens again also to me when I invoked open() method multiple time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤕 Valid issue Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants