react-navigation-shared-element icon indicating copy to clipboard operation
react-navigation-shared-element copied to clipboard

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

Open iamdavidmartin opened this issue 3 years ago • 4 comments

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 avatar Jan 11 '22 21:01 iamdavidmartin

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

iamdavidmartin avatar Jan 11 '22 21:01 iamdavidmartin

Hi @iamdavidmartin !

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

p-syche avatar Jan 26 '23 11:01 p-syche

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.

iamdavidmartin avatar Jan 26 '23 12:01 iamdavidmartin

@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.

p-syche avatar Jan 26 '23 13:01 p-syche