react-native-screens icon indicating copy to clipboard operation
react-native-screens copied to clipboard

iOS transitionStart - transitionEnd not firing when calling navigation.goBack().

Open Cal-um opened this issue 3 years ago • 12 comments

Description

When calling navigation.goBack()/ pop() the transitionStart/End events are not fired when in stackPresentation "modal" in iOS.

Screenshots

Steps To Reproduce

  1. Subscribe to the transitionStart and transitionEnd listeners on a modal screen on iOS that has a header close button with an navigation.goBack() onPress function.
  2. Open the screen and notice the call-backs firing.
  3. tap the close button and notice no transitionStart and transitionEnd events firing.
  4. drag to close does have the events.

Expected behaviour

on calling navigation.goBack() the transitionStart and transitionEnd call backs are triggered.

Actual behaviour

on calling navigation.goBack() the transitionStart and transitionEnd call backs are not triggered.

Snack or minimal code example

https://github.com/Cal-um/iOS13-screens-bug

Package versions

latest versions.

Cal-um avatar Nov 16 '20 12:11 Cal-um

Events added in #https://github.com/software-mansion/react-native-screens/pull/499 @janicduplessis

Cal-um avatar Nov 16 '20 12:11 Cal-um

Having an issue similar to #https://github.com/software-mansion/react-native-screens/issues/513 where the alert is firing before a transition is finished causing the app to lock up

Cal-um avatar Nov 16 '20 12:11 Cal-um

Seems that updating navigation state via the navigation prop causes these events to not fire. So tapping the back button on a stack triggers a ViewController dismiss which in turn updates navigation state so we get these events. I wonder if we can expose something on the native side to trigger a dismiss so we can get these events when dismissing with react navigation?

Cal-um avatar Nov 17 '20 11:11 Cal-um

Isn't it the same for screens with stackPresentation: 'push'? The callbacks are not fired for the screen we are leaving when it is done by doing it from the JS side. Am I right?

WoLewicki avatar Nov 19 '20 13:11 WoLewicki

Yes that is correct

Cal-um avatar Nov 19 '20 13:11 Cal-um

I dont think this is going to be a big issue for me because in the next react-native release UIAlerts are going to be presented from the window but it would be nice if we could find a fix for this. I think it would involve building a new StackRouter to handle it from removing the screen from state until the transition is finished.

Cal-um avatar Nov 19 '20 13:11 Cal-um

Can you elaborate on what is the use-case for these events that cannot be handled by e.g. useEffect cleanups?

WoLewicki avatar Nov 23 '20 13:11 WoLewicki

I believe it is helpful to have a way of knowing when a screen transition has finished. useEffect cleanups do not happen when transitions are complete.

Cal-um avatar Nov 23 '20 14:11 Cal-um

It would require much change of logic both on the native side and on the JS side to handle this and I think it will not be done unless there is a strong use-case that will show that it is needed. Can I help you any more regarding this?

WoLewicki avatar Nov 23 '20 14:11 WoLewicki

Agree this is probably more work than its worth unless if people are crying out for it. @janicduplessis What use cases have you used these events for? I think at least can add something in the docs since its known that it doesn't work for react-navigation navigation actions. Im happy to add that in.

Cal-um avatar Nov 24 '20 09:11 Cal-um

What I can suggest is that you can use onFinishTransitioning of ScreenStack, which should fire at the end of transitions. You can change https://github.com/software-mansion/react-native-screens/blob/master/src/native-stack/views/NativeStackView.tsx#L61 to something like this:

    <ScreenStack 
      style={styles.container} 
      onFinishTransitioning={() =>  {
        navigation.emit({
          type: 'transitionEnd',
          data: { closing: true },
          target: <your_key>,
        })
      }}>

We are not using this event in v5 native-stack, but you can see how it is used in the old version: https://github.com/software-mansion/react-native-screens/blob/master/src/createNativeStackNavigator.js#L270

WoLewicki avatar Nov 24 '20 11:11 WoLewicki

I will experiment with this but will need to find a way to trigger a transition start as well.

Cal-um avatar Nov 30 '20 09:11 Cal-um