Skip to content

Commit

Permalink
Record scroll position in dispatch
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gaearon committed Nov 26, 2014
1 parent 7e9aead commit 64f0f17
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
41 changes: 23 additions & 18 deletions modules/mixins/Scrolling.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,43 @@ function shouldUpdateScroll(state, prevState) {
*/
var Scrolling = {

statics: {
/**
* Records curent scroll position as the last known position for the given URL path.
*/
recordScrollPosition: function (path) {
if (!this.scrollHistory)
this.scrollHistory = {};

this.scrollHistory[path] = getWindowScrollPosition();
},

/**
* Returns the last known scroll position for the given URL path.
*/
getScrollPosition: function (path) {
if (!this.scrollHistory)
this.scrollHistory = {};

return this.scrollHistory[path] || null;
}
},

componentWillMount: function () {
invariant(
this.getScrollBehavior() == null || canUseDOM,
'Cannot use scroll behavior without a DOM'
);

this._scrollHistory = {};
},

componentDidMount: function () {
this._updateScroll();
},

componentWillUpdate: function () {
this._scrollHistory[this.state.path] = getWindowScrollPosition();
},

componentDidUpdate: function (prevProps, prevState) {
this._updateScroll(prevState);
},

componentWillUnmount: function () {
delete this._scrollHistory;
},

/**
* Returns the last known scroll position for the given URL path.
*/
getScrollPosition: function (path) {
return this._scrollHistory[path] || null;
},

_updateScroll: function (prevState) {
if (!shouldUpdateScroll(this.state, prevState)) {
return;
Expand All @@ -73,7 +78,7 @@ var Scrolling = {

if (scrollBehavior)
scrollBehavior.updateScrollPosition(
this.getScrollPosition(this.state.path),
this.constructor.getScrollPosition(this.state.path),
this.state.action
);
}
Expand Down
8 changes: 7 additions & 1 deletion modules/utils/createRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var invariant = require('react/lib/invariant');
var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM;
var ImitateBrowserBehavior = require('../behaviors/ImitateBrowserBehavior');
var RouteHandler = require('../components/RouteHandler');
var LocationActions = require('../actions/LocationActions');
var HashLocation = require('../locations/HashLocation');
var HistoryLocation = require('../locations/HistoryLocation');
var RefreshLocation = require('../locations/RefreshLocation');
Expand Down Expand Up @@ -264,9 +265,14 @@ function createRouter(options) {
* hooks wait, the transition is fully synchronous.
*/
dispatch: function (path, action, callback) {
if (state.path === path)
var prevPath = state.path;
if (prevPath === path)
return; // Nothing to do!

if (prevPath && action !== LocationActions.REPLACE) {
this.recordScrollPosition(prevPath);
}

var match = this.match(path);

warning(
Expand Down

0 comments on commit 64f0f17

Please # to comment.