Add support for "stackAnimation: 'slide'" on iOS & Add a "slide-up" animation for modal screens on Android

[itsjgf]

This PR is a follow-up to #648

It does

1º) Add support for stackAnimation: 'slide' on iOS (same behaviour as default)
2º) Update the animation of stackAnimation: ‘slide’ when the screen is a modal.

stackAnimation: 'slide' & stackPresentation: ' push' .

stackAnimation: 'slide'& stackPresentation: 'transparentModal'.

PD: With this custom animation, we stopped experiencing white flashes/flicks when pushing & going back from screens (in our case, it was directly related to the "default" scale-in/scale-out animation).

[jiong-shen-cb]

awesome, thanks!

[byteab]

Awesome. @itsjgf
but one thing I want to say is. in native android. the foreground screen will start the slide up animation from middle of screen. I mean it's not slide from way bottom to up of screen. it seems that it will slide from 80% of screen up to full page. and also the background screen will also slide a little up

[byteab]

@itsjgf @WoLewicki and as the way I explained. it should be the default transition behaviour is android.the current behaviour is scale from center

[itsjgf]

@EhsanSarshar this PR & #648 are not trying to replicate/re-implement the default animation on Android (which is the one you are describing).

We are just adding a new preset for people who want to be able to use the classic "slide" transition.

[byteab]

@itsjgf oh your right man.
@WoLewicki
and I think the library should replace the current default scale from center transition with the default android transition

[juniorklein]

@itsjgf

I am testing its implementation and it is working well (thanks for your solution, it's very well), but I have a strange situation in the animation.
In my app I have the following stack:

Navigator with animation 'slide'

Screen 1 - default navigator animation
Screen 2 - stackPresentation: 'transparentModal'
Screen 3 - default navigator animation

When user navigates from Screen 2 to Screen 3, the app slides correctly, but screen 2 goes out during the animation showing Screen 1 in back.

<Stack.Navigator initialRouteName="welcome" screenOptions={{ headerShown: false, stackAnimation: 'slide' }}>
      
  <Stack.Screen name="s1" component={Screen1} />
  <Stack.Screen name="s2" component={Screen2} options={{ stackPresentation: 'transparentModal' }} />
  <Stack.Screen name="s3" component={Screen3} />
  <Stack.Screen name="s4" component={Screen4} />
  
</Stack.Navigator>

Debugging ScreenStack.java I found these lines:

for (ScreenStackFragment screen : mScreenFragments) {
  // detach all screens that should not be visible
  if (screen != newTop && screen != belowTop && !mDismissed.contains(screen)) {
    getOrCreateTransaction().remove(screen);
  }
}

During my tests when I navigate from Screen1 to Screen2, belowTop is filled with Screen1 because stackPresentation is transparentModal, but in transition from Screen2 to Screen3, the presentation isn't modal and the if is true to remove Screen2.

This is my understanding of logic, but I haven't analyzed the code completely for better understanding and a possible solution.

Can you help me or suggest a solution?

[WoLewicki]

@juniorklein I think it is the problem of the implementation of the overall transitions, so it would be good to submit it as a separate issue. The workaround I see is not to push screens on top of transparentModal, especially not normal screens. I cannot think of a good example where you would like to push something non-transparent on top of the transparent modal. Can you give a use-case in that issue too?

[WoLewicki]

Since the #648 has been merged, can you rebase to the newest master and add the slide_from_bottom animation in the same manner as the other new animations have been added?

[itsjgf]

@WoLewicki sure, I'll work on this this week.

[itsjgf]

@WoLewicki I have updated the PR with the changes you proposed.

I do have one question though, wouldn't this new approach be hard to use? I guess most of users of this library will expect to be able mimic the iOS slide animation easily.

In order to achieve that, wouldn't it be better to have a single stackAnimation: 'slide' that ...

1º) will use slide_from_right on non RTL devices, on RTL devices will use slide_from_left instead
2º) will use slide_from_bottom when the stackPresentation is set to modal

WIth the current approach, users
1º) will have to set stackAnimation: 'slide_from_bottom' every time they use stackPresentation: 'transparentModal' which is a bit weird (I'm not sure if there's a valid scenario where a modal slides from right to left or viceversa).
2º) Will have t check whether the current language is RTL and set slide_from_right & slide_from_left based on the result.

PD: I have tested this PR with a few projects we have at Z1 and so far so good. I noticed there are a few example folders in the repo, let me know if there are some specific scenarios (or even example files) where I should test this PR.

[WoLewicki]

I can see you didn't update the information about the new transition to all files. You can check the list of them here: #648 (comment).
As for the use-cases of using different animations for different stack presentations, I think the modal slide_from_bottom option can be a good default option, but I'm not sure about the RTL option.

[WoLewicki]

I think we could make something like this in NativeStackView (the same in createNativeStackNavigator.js):

        if (stackAnimation === 'slide') {
          if (direction !== 'ltr' && I18nManager.isRTL) {
            stackAnimation = 'slide_from_left';
          } else {
            stackAnimation = 'slide_from_right';
          }
        } else if (stackAnimation === undefined) {
          if (stackPresentation === 'transparentModal' || stackPresentation === 'containedTransparentModal') {
            stackAnimation = 'slide_from_bottom';
          }
        }

It would require adding slide animation to available options and describing the behavior in all the files from the previous comment. What is your opinion on this?

[itsjgf]

@WoLewicki I have updated the README & the types.tsx file that were missing.

I'll open a new PR creating the new slide animation. As I see it, there are two options:
1º) As you said, we could handle this in the JavaScript layer (NativeStackView & createNativeStackNavigator.js) – I don't like this approach too much because this code is Android specific.
2º) Native layer (Java), I feel this approach might be better because the code is specific to Android and there is no reason to "pollute" the cross-platform layer (JS)

Based on what you said, we would probably want to go with the first approach (JS), but I want to double check with you that we are on the same before I start working on this new PR.

[WoLewicki]

Ok, if you want to apply those changes in another PR, I think this one is ready to be merged 🎉 As for the further changes, I think we can take the discussion to that PR. You can try and implement it on the native side, and we will discuss if it is better than JS solution.

[WoLewicki]

I tested it before merging, and it seems that the behavior shown on the GIF is only available when setting stackPresentation: 'transparentModal', otherwise, the transitions look very bad. They also look very bad even for that stackPresentation, but for more than 2 screens on the stack. I think that the previous screen should never animate when going back/forward. This PR cannot be merged with the current behavior. I think it would be good to start with changing a check in here: https://github.com/software-mansion/react-native-screens/blob/master/android/src/main/java/com/swmansion/rnscreens/ScreenStack.java#L158 to something like this:

  if (newTop.getScreen().getStackPresentation() != Screen.StackPresentation.TRANSPARENT_MODAL && newTop.getScreen().getStackAnimation() != Screen.StackAnimation.SLIDE_FROM_BOTTOM) {

and look at this PR: #658 to properly handle the attaching/detaching the screens. Another approach I can think of is to find a way to apply the transition only to the topmost screen.

It will be also required to have a test example with more than 2 screens in TestsExample project. Could you add it?

[itsjgf]

@WoLewicki that was the previous implementation that I had on this PR (like 1 month ago, as shown in the gif @ OP) before you asked to me re do it in #662 (comment) to match the new slide animations.

I will try to find some time next week to bring back the old implementation.

[WoLewicki]

I am afraid it is not the case of the implementation. Maybe you tested it with having only 2 screens in your stack? Then it works correctly, but it is not enough.

[itsjgf]

@WoLewicki I'm talking about the old implementation (the one that had stackAnimation: 'slide' instead of the current one, which has slide_from_right, slide_from_left & slide_from_bottom as stated in OP) and everything was handled in the Java layer.

I think that was the right approach, where the user would set stackAnimation to slide, and based on the other stuff (RTL, presentation set to modal, etc) it would set the appropriate transition (right to left, left to right, bottom to top).

[jp928]

I am keen to have this.

[WoLewicki]

I'll close this PR since #848 has been merged and it has those animations. The case of making it a default for modals is another topic and can be discussed in another PR. Feel free to comment here if something is wrong.