erikringsmuth / app-router

Router for Web Components
https://erikringsmuth.github.io/app-router/
MIT License
610 stars 83 forks source link

Fixed issue when navigating to duplicate route #43

Closed sohjsolwin closed 9 years ago

sohjsolwin commented 9 years ago

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.

erikringsmuth commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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.