Skip to content

Record scroll position in dispatch to support alternative batch strategies #524

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

Merged
merged 1 commit into from
Nov 26, 2014

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Nov 26, 2014

Recording scroll position in dispatch method is necessary for clients using alternative batching strategies, such as react-raf-batching.

With an alternative batching strategy, by the time componentWillUpdate is called, browser may have tried to restore position itself (not always correctly), and the recorded position is thus wrong.

This also happens if you don't immediately render inside Router.run callback for some reason.

This fix records scroll position while it's “fresh”.

Also, don't record position for REPLACE action since there is no way to get back to it.

Recording scroll position in dispatch method is necessary for clients using alternative batching strategies, such as react-raf-batching[1].
With an alternative batching strategy, by the time componentWillUpdate is called, browser may have tried to restore position itself (not always correctly), and the recorded position is thus wrong.

Also, don't record position for REPLACE action since there is no way to get back to it.

[1]: https://github.com/petehunt/react-raf-batching
@gaearon
Copy link
Contributor Author

gaearon commented Nov 26, 2014

Do you mean in willReceiveProps? The problem I saw is that Chrome tries to restore scroll state some milliseconds right after popstate. This really leaves us no time to record it other than in dispatch.

@mjackson
Copy link
Member

@gaearon Ah, I see. ok, let's keep it in dispatch then.

mjackson added a commit that referenced this pull request Nov 26, 2014
Record scroll position in dispatch to support alternative batch strategies
@mjackson mjackson merged commit 3ff2a96 into remix-run:master Nov 26, 2014
@gaearon gaearon deleted the record-scroll-asap branch November 26, 2014 18:51
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants