YahooArchive / fluxible-router

MOVED TO FLUXIBLE REPO
104 stars 26 forks source link

Prevent history update before navigation complete #28

Closed ertrzyiks closed 9 years ago

ertrzyiks commented 9 years ago

Hey, i migrated from router component to fluxible-router and after click on NavLink i see bottom of page instead of top. I also noticed that before migration url was changing after action succeed. Now it change url first and then action is called. Is is working as expected?

This PR is just example of solution which fixed both issues for me :) It will not fire history update before action completes.

What do you think?

yahoocla commented 9 years ago

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! :smile:

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.28%) to 99.15% when pulling d12e19096c7c4fcb2453e5b6fd3bba77d8eb37dd on ertrzyiks:prevent-update-before-complete into caab18457f321312498ef41ca8b33ee0c559a9c9 on yahoo:master.

mridgway commented 9 years ago

We will probably not want to do this in the library. Ideally, you should render the new page immediately, but if the data isn't available for every component, render a loading or temporary view of the component. This way, the user gets instant feedback about the page navigation and the page will fill in the view as that data becomes available instead of waiting for the full page data to be available (which is preceptively slower).

ertrzyiks commented 9 years ago

@mridgway you are right. I dug little more and turns out that my application has condition race on stores and that cause invalid transition between routes. Thanks for answer!