Default hardware BackButton behavior is eliminated when the Modal is visible but no `onBackButtonPress` is specified

[libekonst]

Environment

System:
    OS: macOS 11.6
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
    Memory: 81.05 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.1 - /usr/local/opt/node@14/bin/node
    Yarn: Not Found
    npm: 6.14.13 - /usr/local/opt/node@14/bin/npm
    Watchman: 2021.06.07.00 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.2 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: iOS 14.4, DriverKit 20.2, macOS 11.1, tvOS 14.3, watchOS 7.2
    Android SDK:
      API Levels: 28, 29, 30
      Build Tools: 28.0.3, 29.0.2, 29.0.3, 30.0.1, 30.0.2
      System Images: android-24 | Intel x86 Atom_64, android-28 | Intel x86 Atom_64, android-29 | Intel x86 Atom_64, android-30 | Google APIs Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: 4.0 AI-193.6911.18.40.6626763
    Xcode: 12.4/12D4e - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_262 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 17.0.2 => 17.0.2 
    react-native: 0.65.1 => 0.65.1 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Platforms

Android

Versions

Android: 30 iOS: 14.4 react-native-modal: 13.0.0 react-native: 0.65.1 react: 17.0.2

Description & Reproducible Demo

After upgrading the package to version 13.0.0 my team and I noticed some unexpected behavior on how the app responds to hardware back button presses when the Modal is visible.

More specifically, rendering a Modal visible but omitting the onBackButtonPress, results in earlier back button subscriptions to not be getting called.

Steps to reproduce are:

1. Setup a parent view that registers a BackHandler listener and renders a child View
2. In the child view, render a Modal and omit its onBackButtonPress prop
3. Set Modal's prop visible=true
4. Press the back button, notice that the first handler registered in step 1 is not getting called.

Example

Run this snack on Android: https://snack.expo.dev/uZ2_p6RkK

In this demo app, a simple react-navigation Stack Navigator manages two screens. The Navigator internally registers a BackHandler that pops a screen from the stack each time it's called. Notice that when the modal is visible, the Navigator's BackHandler never runs.

Why it happens

Looking through the source code we pinned down the cause to this line:

  onBackButtonPress = () => {
    if (this.props.onBackButtonPress && this.props.isVisible) {
      this.props.onBackButtonPress();
      return true;
    }
    return false;
  };

Omitting the onBackButtonPress prop should result in not entering the if statement, thus returning false and allowing other handlers registered earlier to be called.

However, the component's default props set the onBackButtonPress prop to an empty function. So, omitting the prop means the default prop takes its place, getting initialised to an empty function instead of it being undefined.

Empty functions are truthy, so whenever the Modal is visible the if branch is entered, calling the empty function and then returning true, which in turn prevents all previous handlers registered from getting called.

We have two proposals for this:

Solution 1

Allow the prop to also be null. Setting it to null overrides the default empty function prop, so now it properly is falsy and the if branch is not entered.

This seems to be the easiest way, but the API gets dirty. This requires a change in the types and the documentation as well, describing how to correctly omit a back handler in modals and let the parent deal with that.

Solution 2

Do not default the prop to an empty function, leave it undefined so that the if branch is not entered.

Not sure if this is has any implications with the rest of the implementation though or even if the current behavior is actually intended.

[mmazzarolo]

Hey @KostasLib , thanks so much for this fantastic issue description 🙌 I can't test it right now, but I agree, the API for this is confusing.

Before going any further, there's also one thing that we might wanna take into account. A long time has passed since the initial implementation of the back handler flow, so I had to refresh a bit how it works: by checking the React Native's built-in modal onRequestClose API it seems that it overrides the built-in back-button press behavior. If that's the case, I don't think that solution 1 would work here, right? Could you give it a try (sorry can't run the Expo snack now 😞 ) by updating the Expo snack providing null to onBackButtonPress?

[Splanis]

Hey @KostasLib , thanks so much for this fantastic issue description 🙌 I can't test it right now, but I agree, the API for this is confusing.

Before going any further, there's also one thing that we might wanna take into account. A long time has passed since the initial implementation of the back handler flow, so I had to refresh a bit how it works: by checking the React Native's built-in modal onRequestClose API it seems that it overrides the built-in back-button press behavior. If that's the case, I don't think that solution 1 would work here, right? Could you give it a try (sorry can't run the Expo snack now 😞 ) by updating the Expo snack providing null to onBackButtonPress?

Passing null seems to work but we got a linting error.

[dcalhoun]

Before going any further, there's also one thing that we might wanna take into account. A long time has passed since the initial implementation of the back handler flow, so I had to refresh a bit how it works: by checking the React Native's built-in modal onRequestClose API it seems that it overrides the built-in back-button press behavior. If that's the case, I don't think that solution 1 would work here, right? Could you give it a try (sorry can't run the Expo snack now 😞 ) by updating the Expo snack providing null to onBackButtonPress?

@mmazzarolo contrary to @Splanis' report — not sure why we are seeing different outcomes 😕 — I found that adding onBackButtonPress={null} does not change the outcome of pressing the hardware back button. I updated the reproduction Snack to showcase this. I would imagine this is caused by the React Native Modal documentation you referenced.

Warning for modal users: If your app shows an opened Modal, BackHandler will not publish any events.

The onRequestClose callback is called when the user taps the hardware back button on Android or the menu button on Apple TV. Because of this required prop, be aware that BackHandler events will not be emitted as long as the modal is open.

If an open Modal prevents bubbling of the BackHandler event, I am not sure of a library-specific solution to address this. I would imagine this would need to be managed in the application leveraging the react-native-modal library.

[mmazzarolo]

Hey @dcalhoun, thanks for giving it a try — that's what I was suspicious about as well. Open to suggestions, but I think this might need to be handled by the react-native-modal consumer :/