Add "hidden" prop to Screen to support iOS Picture-in-Picture

[jaredly]

Description

Thanks for making this wonderful library! I've really enjoyed using it with react-navigation. I'm transitioning a large app from custom native navigation code (written before we adopted react-native) to react-navigation, and the only thing I'm having trouble with is iOS Picture-In-Picture.

If you're unfamiilar, Picture-in-Picture (PIP) refers to popping a video out into an "overlay" that persists as you navigate around the app, until you either close it or "restore" it, which results in the originating screen being pushed back onto the stack, and the video sliding back into its original location.

Here's a little video of that in action, for context:

pip.mp4

There are two things needed to support PIP well:

1. the ability to dismiss a screen without letting the resources be garbage collected. (when the originating ViewController is garbage collected, the PIP window automatically closes). Additionally, the screen must not be unmounted on the react side of things, so that events will still be handled properly (critically, the onRestoreUserInterfaceForPictureInPictureStop callback for react-native-video).
2. the ability to bring back the screen that the PIP overlay originated from, in response to the user tapping on the relevant icon in the overlay.

Changes

The solution I've arrived at requires fairly minimal changes to react-native-screens, just adding a single hidden prop to the Screen component, which instructs the ScreenStack to ignore that screen for the purposes of constructing the arrays of ViewControllers. Additionally, when the hidden prop is modified (for the case of "bringing back" a dismissed screen), I clear the _dismissed flag.

The above screen recording demonstrates this change, in concert with the react-navigation changes.

Test code and steps to reproduce

Here's a minimal repo illustrating the problem behavior: https://github.com/jaredly/rnsdemo
And a video:
https://user-images.githubusercontent.com/112170/207732805-7750c1e8-7276-4f51-9244-f86ec312092b.mp4

Checklist

• Included code example that can be used to test this change
• Updated TS types
• Updated documentation:
  • https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
  • https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
  • https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
  • https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
• Ensured that CI passes

[kacperkapusciak]

Hi @jaredly 👋

I think thanks to this PR we've made a full circle in react-native-screens development 😅

Screens where developed to fight a problem directly opposite to what you are facing - freeing resources of a screen that the user can't see.

In Stack, Drawer and Bottom Tabs navigators there's a prop to disable this optimisation (disabling screens) - detachInactiveScreens. I think using a JS stack with screens disabled should work well for your use-case.

Fair, there's no prop like this in native-stack because it always felt like a step back but maybe, as you've presented a valid use-case, we should consider adding it

[jaredly]

Interesting! I think detatchInactiveScreens is a slightly different mechanism, although I may be misunderstanding; with the current react-native-screens, picture in picture works if you navigate forward (adding screens to the stack), and only breaks when you navigate back (popping the originating screen off the stack, and causing it to get garbage collected).
At any rate, going with a js-only screens impl is a no-go for us, as we care a lot about native navigation animations & gestures.
Thanks for taking a look!

[jaredly]

btw I put together a minimal react-native project illustrating the issue: https://github.com/jaredly/rnsdemo
And a video:
https://user-images.githubusercontent.com/112170/207732805-7750c1e8-7276-4f51-9244-f86ec312092b.mp4

[jaredly]

@kacperkapusciak for what it's worth, switching to the js-implemented stack didn't help with the "originating view needs to stick around for PIP to keep going"

smol.mp4

[jaredly]

@kkafar sorry to bump, but this feature is blocking our adoption of react-native-screens & react-navigation. Anything I can do to make this easier to review?

[kkafar]

Sorry for the delay! I'm recently a little bit buried in todo stuff. I'll try to take care of this PR soon.

[jaredly]

bump? 🙏

[jaredly]

@kkafar it would be really nice to get this merged in! Is there anything I can do on my end? I'd love to stop depending on a fork.

[hirbod]

Very good feature, seems like so many valid and good PRs are stale in RNS. @kkafar I don't want to sound harsh, but I went through quite a bit of PRs (including mine) and most of the "looking into it soon" answers been months already. Anything we can do to help here?

[kkafar]

Hey, @hirbod, @maksg is currently assigned to implementing API for preload functionality & it should cover this use case.

As to the "stale PRs" I think you're absolutely right & I do remember about the PR of yours. Moreover we appreciate every PR. Currently in react-native-screens, however, Fabric related issues have the highest priority before 0.75.0 release of RN. Once the PR is open & waiting for my attention best thing to do is to tag & bump me from time to time. If you are unhappy with current pace of development & require prioritised attention it is best to contact us through https://swmansion.com/contact#contact-form.

[tboba]

According to changes from this PR, I believe these changes have been superseded by checking if activityState from screen is inactive?
@jaredly Could you get the newest alpha of Screens 4.x and check if preload prop gives you the same behavior as requested here? You can check from TestPreload.tsx how you can dispatch preload action, using navigation.dispatch method.