IjzerenHein / react-navigation-shared-element

React Navigation bindings for react-native-shared-element 💫
https://github.com/IjzerenHein/react-native-shared-element
MIT License
1.27k stars 124 forks source link

Navigating back to Details screen from Details screen doesn't animate #220

Open iamdavidmartin opened 2 years ago

iamdavidmartin commented 2 years ago

Hi there, Love this library!

When you have a details screen that you navigate to and from, the library doesn't know how to animate "backwards" to a previous details screen one at a time. This means the animation correctly runs when you navigate forward to a new details screen, but once you go backwards, every single previous details screen in the stack with the same name animates at the same time, and after that there are no more animations going backwards because they all animated on the first "backwards" navigation.

I found the root causes, fixed them, and made a pull request.

There are 2 reasons:

  1. This line uses route.name to determine whether to animate a scene, but should use route.key: https://github.com/IjzerenHein/react-navigation-shared-element/blob/9fc6720a6c1a6f0b06dbf82321143a881a33f814/src/createSharedElementScene.tsx#L59 Note that route.key is used elsewhere:https://github.com/IjzerenHein/react-navigation-shared-element/blob/9fc6720a6c1a6f0b06dbf82321143a881a33f814/src/SharedElementRendererData.ts#L275 When route.name is used, if you navigate to a "Details" screen, all existing, rendered "Details" screens on the stack believe they are the active route and will transition, even though you only want the one with the matching key to transition. This is a bug.
  2. Line 319 and 322 hardcodes "true" and "false" values to be send to the screen's sharedElement function as the showing variable.https://github.com/IjzerenHein/react-navigation-shared-element/blob/9fc6720a6c1a6f0b06dbf82321143a881a33f814/src/SharedElementRendererData.ts#L315-L324 However, when screens are the same name, true is always sent regardless of whether the screen is showing or not. What should really be sent is the closing value from the event: https://github.com/IjzerenHein/react-navigation-shared-element/blob/9fc6720a6c1a6f0b06dbf82321143a881a33f814/src/createSharedElementScene.tsx#L112 This is stored under isTransitionClosing https://github.com/IjzerenHein/react-navigation-shared-element/blob/9fc6720a6c1a6f0b06dbf82321143a881a33f814/src/SharedElementRendererData.ts#L110 And that's the variable that should be sent to sharedElements as showing.

I've created an example that shows navigation working backwards and forwards to screens of the same name.

iamdavidmartin commented 2 years ago

Here's my PR https://github.com/IjzerenHein/react-navigation-shared-element/pull/221

p-syche commented 1 year ago

Hi @iamdavidmartin !

Sorry this is taking so long. I noticed you closed your PR - where you unhappy with the solution you created?

iamdavidmartin commented 1 year ago

I thought the PR was useful when it was made over a year ago. If you want it I can reopen it. I just assumed the project had been abandoned and it was just sitting in my list of PRs and I was cleaning it up.

p-syche commented 1 year ago

@iamdavidmartin thanks for the quick answer. If you could re-open it, or open a new one, I'll do my best to check it next week so we can add your improvement.