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

Fixed issue when navigating to duplicate route #43

Merged
merged 1 commit into from
Dec 5, 2014

Conversation

sohjsolwin
Copy link
Contributor

When navigating to a duplicate route (possibly with different databound parameters) after navigating to a second route, the app-router would load two instances of the component within the route. This lead to various weirdness. To correct that, I added a "navigate-action" attribute to the routes that I wanted something extra to be done with when they load. If the navigate-action value is set to "removeDuplicateInstance" and the route we're navigating to references the same component as the active route, the "transitionAnimationEnd" will be called with the active route, effectively removing it from the DOM the same as if an animation was still in progress, and the new instance of the component will continue to be loaded like normal. If "stopOnDuplicateLoad" is the navigate-action instead, the call will return, which should stop any further loading and not recreate the component in the DOM.

Example: Previously when navigating to http://example.com/appPageAlpha/1, then to http://example.com/appPageBeta/1, and then to http://example.com/appPageBeta/2, the appPageBeta component would be loaded in the DOM twice. When Navigating to appPageBeta a third time, the component would be cleaned out properly because the 'transitionAnimationInProgress' would still be true, since the core-animated-pages component didn't navigate to a new page (because the path attribute between each of those routes is identical), and the animation never occurred.

When navigating to a duplicate route (possibly with different databound parameters) after navigating to a second route, the app-router would load two instances of the component within the route. This lead to various weirdness. To correct that, I added a "navigate-action" attribute to the routes that I wanted something extra to be done with when they load. If the navigate-action value is set to "removeDuplicateInstance" and the route we're navigating to references the same component as the active route, the "transitionAnimationEnd" will be called with the active route, effectively removing it from the DOM the same as if an animation was still in progress, and the new instance of the component will continue to be loaded like normal. If "stopOnDuplicateLoad" is the navigate-action instead, the call will return, which should stop any further loading and not recreate the component in the DOM.

Example: Previously when navigating to http://somewhere.com/appPageAlpha/1, then to http://somewhere.com/appPageBeta/1, and then to http://somewhere.com/appPageBeta/2, the appPageBeta component would be loaded in the DOM twice. When Navigating to appPageBeta a third time, the component would be cleaned out properly because the 'transitionAnimationInProgress' would still be true, since the core-animated-pages component didn't navigate to a new page (because the path attribute between each of those routes is identical), and the animation never occurred.
@erikringsmuth
Copy link
Owner

This makes sense. I'll probably have it removeDuplicateInstance by default since the main use case should be to swap the route with a new page with the new bindings.

Transitions with core-animated-pages will probably be broken when switching between two pages that use the same route. This is probably a bug with core-animated-pages in general since it does transitions by hiding/showing different pages using CSS.

I'm working on another set of changes right now so I'll try to get this merged in with them before I go to version 2.1.0.

@erikringsmuth erikringsmuth merged commit 3432600 into erikringsmuth:master Dec 5, 2014
@erikringsmuth
Copy link
Owner

I pulled your changes and worked off of them. I ended up moving the logic here where the rest of the DOM replacement happens https://github.com/erikringsmuth/app-router/blob/master/src/app-router.js#L286.

It's only a workaround because there's still the problem that multiple routes matching the same app-route won't animate the transition.

@erikringsmuth
Copy link
Owner

I got version 2.1.0 released with the fix. Right now I'm always using the remove duplicate instance approach and not using the navigate-action to keep it as simple as possible.

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.

2 participants