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

fix(vue-app): use child transition name when navigating to parent #6946

Merged
merged 4 commits into from
Feb 11, 2020
Merged

fix(vue-app): use child transition name when navigating to parent #6946

merged 4 commits into from
Feb 11, 2020

Conversation

reegodev
Copy link

@reegodev reegodev commented Feb 7, 2020

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

After #6803 fixed the issue i reported about child transition properties, i discovered that also the transition name did not work correctly when leaving a child route to its parent.

I've created a quick demo to show the problem: https://mllwe.sse.codesandbox.io/
If you enter a child route from its parent (ie: B -> BB ) you can see the correct transition applied ( rotation + fade-in ).
If you leave the child route and return to its parent ( ie: BB -> B ), instead of using the child route transition ( rotation + fade-out ), you see the default page transition applied ( swipe to the right ).
This is particularly bad when you use child routes to open modals, as the opening animation works correctly while the leave animation does not.

This PR provides a fix for this scenario, along with some E2E tests to assure that page transitions name and callbacks merging do not break in the future.

This is the same demo with the fix applied: https://ghdkd.sse.codesandbox.io/

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.


// Combine transitions & prefer `enter` callbacks and name of "to" route
Object.keys(toTransitions)
.filter(key => typeof toTransitions[key] !== 'undefined' && !key.toLowerCase().includes('leave'))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reversed the logic so that we start from the "from" properties and merge only existing "to" properties, ignoring the leave properties as before. This allows to keep the transition name from the "from" component in case there is no "to" component at the same depth ( as it happens when going from child to parent ).

Also just checking for truthiness of toTransitions[key] was preventing falsy values from being merged, ie: css: false, so typeof toTransitions[key] !== 'undefined' fixes that as well.

@atinux atinux self-requested a review February 11, 2020 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants