From 4d8c7a10abd946b98918b5087dfc21df15048df2 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Mon, 6 Oct 2014 08:02:34 -0700 Subject: [PATCH] Fixed everything [removed] [removed] --- modules/actions/LocationActions.js | 7 +- modules/components/Routes.js | 45 ++---------- modules/components/__tests__/Routes-test.js | 54 -------------- modules/mixins/ScrollContext.js | 28 ++++++++ modules/stores/PathStore.js | 3 - modules/stores/ScrollStore.js | 79 --------------------- tests.js | 2 - 7 files changed, 36 insertions(+), 182 deletions(-) delete mode 100644 modules/stores/ScrollStore.js diff --git a/modules/actions/LocationActions.js b/modules/actions/LocationActions.js index cd591d0aad..46a1be6634 100644 --- a/modules/actions/LocationActions.js +++ b/modules/actions/LocationActions.js @@ -21,12 +21,7 @@ var LocationActions = { /** * Indicates the most recent entry should be removed from the history stack. */ - POP: 'pop', - - /** - * Indicates that a route transition is finished. - */ - FINISHED_TRANSITION: 'finished-transition' + POP: 'pop' }; diff --git a/modules/components/Routes.js b/modules/components/Routes.js index d17035d9d6..3d4738a4c9 100644 --- a/modules/components/Routes.js +++ b/modules/components/Routes.js @@ -2,10 +2,7 @@ var React = require('react'); var warning = require('react/lib/warning'); var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM; var copyProperties = require('react/lib/copyProperties'); -var LocationActions = require('../actions/LocationActions'); -var LocationDispatcher = require('../dispatchers/LocationDispatcher'); var PathStore = require('../stores/PathStore'); -var ScrollStore = require('../stores/ScrollStore'); var reversedArray = require('../utils/reversedArray'); var Transition = require('../utils/Transition'); var Redirect = require('../utils/Redirect'); @@ -265,13 +262,6 @@ function computeHandlerProps(matches, query) { var BrowserTransitionHandling = { - handleTransition: function (transition) { - LocationDispatcher.handleViewAction({ - type: LocationActions.FINISHED_TRANSITION, - path: transition.path - }); - }, - handleTransitionError: function (error) { throw error; // This error probably originated in a transition hook. }, @@ -290,10 +280,6 @@ var BrowserTransitionHandling = { var ServerTransitionHandling = { - handleTransition: function (transition) { - // TODO - }, - handleTransitionError: function (error) { // TODO }, @@ -330,20 +316,9 @@ var Routes = React.createClass({ mixins: [ ActiveContext, LocationContext, RouteContext, ScrollContext ], propTypes: { - onTransition: React.PropTypes.func.isRequired, - onTransitionError: React.PropTypes.func.isRequired, - onAbortedTransition: React.PropTypes.func.isRequired, initialPath: React.PropTypes.string }, - getDefaultProps: function () { - return { - onTransition: TransitionHandling.handleTransition, - onTransitionError: TransitionHandling.handleTransitionError, - onAbortedTransition: TransitionHandling.handleAbortedTransition - }; - }, - getInitialState: function () { return { matches: [] @@ -356,41 +331,35 @@ var Routes = React.createClass({ componentDidMount: function () { PathStore.addChangeListener(this.handlePathChange); - ScrollStore.addChangeListener(this.handleScrollChange); }, componentWillUnmount: function () { - ScrollStore.removeChangeListener(this.handleScrollChange); PathStore.removeChangeListener(this.handlePathChange); }, handlePathChange: function (_path) { var path = _path || PathStore.getCurrentPath(); + var actionType = PathStore.getCurrentActionType(); if (this.state.path === path) return; // Nothing to do! + if (this.state.path) + this.recordScroll(this.state.path); + var self = this; this.dispatch(path, function (error, transition) { if (error) { - self.props.onTransitionError.call(self, error); + TransitionHandling.handleTransitionError.call(self, error); } else if (transition.isAborted) { - self.props.onAbortedTransition.call(self, transition); + TransitionHandling.handleAbortedTransition.call(self, transition); } else { - self.props.onTransition.call(self, transition); + self.updateScroll(path, actionType); } }); }, - handleScrollChange: function () { - var behavior = this.getScrollBehavior(); - var position = ScrollStore.getCurrentScrollPosition(); - - if (behavior && position) - behavior.updateScrollPosition(position, PathStore.getCurrentActionType()); - }, - /** * Performs a depth-first search for the first route in the tree that matches on * the given path. Returns an array of all routes in the tree leading to the one diff --git a/modules/components/__tests__/Routes-test.js b/modules/components/__tests__/Routes-test.js index 59e19ad567..ac647c5b30 100644 --- a/modules/components/__tests__/Routes-test.js +++ b/modules/components/__tests__/Routes-test.js @@ -69,58 +69,4 @@ describe('A Routes', function () { }); }); - - describe('when a transition is aborted', function () { - it('triggers onAbortedTransition', function (done) { - var AbortHandler = React.createClass({ - statics: { - willTransitionTo: function (transition) { - transition.abort(); - } - }, - render: function () { - return null; - } - }); - - function handleAbortedTransition(transition) { - assert(transition); - done(); - } - - ReactTestUtils.renderIntoDocument( - Routes({ onAbortedTransition: handleAbortedTransition }, - Route({ handler: AbortHandler }) - ) - ); - }); - }); - - describe('when there is an error in a transition hook', function () { - it('triggers onTransitionError', function (done) { - var ErrorHandler = React.createClass({ - statics: { - willTransitionTo: function (transition) { - throw new Error('boom!'); - } - }, - render: function () { - return null; - } - }); - - function handleTransitionError(error) { - assert(error); - expect(error.message).toEqual('boom!'); - done(); - } - - ReactTestUtils.renderIntoDocument( - Routes({ onTransitionError: handleTransitionError }, - Route({ handler: ErrorHandler }) - ) - ); - }); - }); - }); diff --git a/modules/mixins/ScrollContext.js b/modules/mixins/ScrollContext.js index 0fe3a2e4f4..e4c714ee13 100644 --- a/modules/mixins/ScrollContext.js +++ b/modules/mixins/ScrollContext.js @@ -4,6 +4,18 @@ var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM; var ImitateBrowserBehavior = require('../behaviors/ImitateBrowserBehavior'); var ScrollToTopBehavior = require('../behaviors/ScrollToTopBehavior'); +function getWindowScrollPosition() { + invariant( + canUseDOM, + 'Cannot get current scroll position without a DOM' + ); + + return { + x: window.scrollX, + y: window.scrollY + }; +} + /** * A hash of { name: scrollBehavior } pairs. */ @@ -52,6 +64,22 @@ var ScrollContext = { behavior == null || canUseDOM, 'Cannot use scroll behavior without a DOM' ); + + if (behavior != null) + this._scrollPositions = {}; + }, + + recordScroll: function (path) { + if (this._scrollPositions) + this._scrollPositions[path] = getWindowScrollPosition(); + }, + + updateScroll: function (path, actionType) { + var behavior = this.getScrollBehavior(); + var position = this._scrollPositions[path]; + + if (behavior && position) + behavior.updateScrollPosition(position, actionType); }, /** diff --git a/modules/stores/PathStore.js b/modules/stores/PathStore.js index e423adc71b..5bce5baa4e 100644 --- a/modules/stores/PathStore.js +++ b/modules/stores/PathStore.js @@ -1,7 +1,6 @@ var EventEmitter = require('events').EventEmitter; var LocationActions = require('../actions/LocationActions'); var LocationDispatcher = require('../dispatchers/LocationDispatcher'); -var ScrollStore = require('./ScrollStore'); var CHANGE_EVENT = 'change'; var _events = new EventEmitter; @@ -51,8 +50,6 @@ var PathStore = { case LocationActions.PUSH: case LocationActions.REPLACE: case LocationActions.POP: - LocationDispatcher.waitFor([ ScrollStore.dispatchToken ]); - if (_currentPath !== action.path) { _currentPath = action.path; _currentActionType = action.type; diff --git a/modules/stores/ScrollStore.js b/modules/stores/ScrollStore.js deleted file mode 100644 index 7fcb0a2ce5..0000000000 --- a/modules/stores/ScrollStore.js +++ /dev/null @@ -1,79 +0,0 @@ -var EventEmitter = require('events').EventEmitter; -var invariant = require('react/lib/invariant'); -var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM; -var LocationActions = require('../actions/LocationActions'); -var LocationDispatcher = require('../dispatchers/LocationDispatcher'); -var PathStore; // TODO: Fix circular requires. - -var CHANGE_EVENT = 'change'; -var _events = new EventEmitter; - -function notifyChange() { - _events.emit(CHANGE_EVENT); -} - -function getCurrentScrollPosition() { - invariant( - canUseDOM, - 'Cannot get current scroll position without a DOM' - ); - - return { - x: window.scrollX, - y: window.scrollY - }; -} - -var _scrollPositions = {}, _currentScrollPosition; - -/** - * The ScrollStore keeps track of the current URL path. - */ -var ScrollStore = { - - addChangeListener: function (listener) { - _events.addListener(CHANGE_EVENT, listener); - }, - - removeChangeListener: function (listener) { - _events.removeListener(CHANGE_EVENT, listener); - }, - - removeAllChangeListeners: function () { - _events.removeAllListeners(CHANGE_EVENT); - }, - - /** - * Returns the last known scroll position for the current URL. - */ - getCurrentScrollPosition: function () { - return _currentScrollPosition; - }, - - dispatchToken: LocationDispatcher.register(function (payload) { - if (PathStore == null) - PathStore = require('./PathStore'); - - var action = payload.action; - - switch (action.type) { - case LocationActions.SETUP: - case LocationActions.PUSH: - case LocationActions.REPLACE: - case LocationActions.POP: - var currentPath = PathStore.getCurrentPath(); - - if (currentPath) - _scrollPositions[currentPath] = getCurrentScrollPosition(); - break; - - case LocationActions.FINISHED_TRANSITION: - _currentScrollPosition = _scrollPositions[action.path]; - notifyChange(); - break; - } - }) - -}; - -module.exports = ScrollStore; diff --git a/tests.js b/tests.js index d62e764200..5b159d1f00 100644 --- a/tests.js +++ b/tests.js @@ -16,10 +16,8 @@ require('./modules/utils/__tests__/Path-test'); var PathStore = require('./modules/stores/PathStore'); -var ScrollStore = require('./modules/stores/ScrollStore'); afterEach(function () { // For some reason unmountComponentAtNode doesn't call componentWillUnmount :/ PathStore.removeAllChangeListeners(); - ScrollStore.removeAllChangeListeners(); });