From 64f0f17246754c45c37f65c5d7d6ff79adeb4d10 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 26 Nov 2014 18:57:54 +0100 Subject: [PATCH] Record scroll position in dispatch 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 --- modules/mixins/Scrolling.js | 41 ++++++++++++++++++++--------------- modules/utils/createRouter.js | 8 ++++++- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/modules/mixins/Scrolling.js b/modules/mixins/Scrolling.js index 6da2313462..01e2d9c372 100644 --- a/modules/mixins/Scrolling.js +++ b/modules/mixins/Scrolling.js @@ -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; @@ -73,7 +78,7 @@ var Scrolling = { if (scrollBehavior) scrollBehavior.updateScrollPosition( - this.getScrollPosition(this.state.path), + this.constructor.getScrollPosition(this.state.path), this.state.action ); } diff --git a/modules/utils/createRouter.js b/modules/utils/createRouter.js index 561d924f54..946217031e 100644 --- a/modules/utils/createRouter.js +++ b/modules/utils/createRouter.js @@ -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'); @@ -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(