Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
iamdavidmartin opened this issue Jan 11, 2022 · 4 comments

Comments

@iamdavidmartin
Copy link

iamdavidmartin commented Jan 11, 2022

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:
    return route.name === activeRoute.name;
    Note that route.key is used elsewhere:
    (this.route && this.route.key === scene.route.key) ||
    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.
    // Update shared elements
    let sharedElements: SharedElementsStrictConfig | null = null;
    let isShowing = true;
    if (sceneAnimValue && scene && prevScene && route && prevRoute) {
    sharedElements = getSharedElements(scene, prevScene, true);
    if (!sharedElements) {
    isShowing = false;
    sharedElements = getSharedElements(prevScene, scene, false);
    }
    }
    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:
    const closing: boolean = event.data.closing;
    This is stored under isTransitionClosing
    this.isTransitionClosing = closing;
    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
Copy link
Author

Here's my PR #221

@p-syche
Copy link
Collaborator

p-syche commented Jan 26, 2023

Hi @iamdavidmartin !

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

@iamdavidmartin
Copy link
Author

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
Copy link
Collaborator

p-syche commented Jan 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants