Skip to content

Commit

Permalink
Callback update: fire navigation callbacks on all routers with unique…
Browse files Browse the repository at this point in the history
… `match` objects.

Note arity change; callbacks are now (path, navigation, match) from (path, navigation).
`navigation` no longer has a `match` attribute.

Fixes #95.
  • Loading branch information
STRML committed Jun 9, 2016
1 parent 2c18154 commit e4f3446
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 32 deletions.
4 changes: 2 additions & 2 deletions lib/CaptureClicks.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ var CaptureClicks = React.createClass({
// flag if we already found a "not found" case and bailed
var bail = false;

var onBeforeNavigation = function(path, navigation) {
var onBeforeNavigation = function(path, navigation, match) {
if (bail) {
return false;
} else if (!navigation.match || !navigation.match.match) {
} else if (!match || !match.match) {
bail = true;
this.props.gotoURL(el.href);
return false;
Expand Down
30 changes: 13 additions & 17 deletions lib/RouterMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ var RouterMixin = {
var parent = props.contextual && this.getParentRouter();

if (parent) {

var parentMatch = parent.getMatch();
// Build our new path based off the parent. A navigation may be in progress, in which case
// we as well want the newest data so we use the pending match.
var parentMatch = parent._pendingMatch || parent.getMatch();

invariant(
props.path ||
Expand Down Expand Up @@ -86,7 +87,8 @@ var RouterMixin = {
matchProps: match.getProps(),
handler: match.getHandler(),
prefix: prefix,
navigation: {}
navigation: {},
path: path
};
},

Expand Down Expand Up @@ -165,31 +167,22 @@ var RouterMixin = {
* @param {Callback} cb
*/
setPath: function(path, navigation, cb) {
var match = matchRoutes(this.getRoutes(this.props), path, this.getURLPatternOptions());

var state = {
match: match,
matchProps: match.getProps(),
handler: match.getHandler(),
prefix: this.state.prefix,
navigation: navigation
};

assign(navigation, {match: match});
var state = this.getRouterState(this.props);
state.navigation = navigation;

if (this.props.onBeforeNavigation &&
this.props.onBeforeNavigation(path, navigation) === false) {
this.props.onBeforeNavigation(state.path, navigation, state.match) === false) {
return;
}

if (navigation.onBeforeNavigation &&
navigation.onBeforeNavigation(path, navigation) === false) {
navigation.onBeforeNavigation(state.path, navigation, state.match) === false) {
return;
}

this.delegateSetRoutingState(state, function() {
if (this.props.onNavigation) {
this.props.onNavigation(path, navigation);
this.props.onNavigation(state.path, navigation, state.match);
}
cb();
}.bind(this));
Expand All @@ -207,6 +200,9 @@ var RouterMixin = {
* by router itself) or use replaceState.
*/
delegateSetRoutingState: function(state, cb) {
// Store this here so it can be accessed by child contextual routers in onBeforeNavigation.
this._pendingMatch = state.match;

if (this.setRoutingState) {
this.setRoutingState(state, cb);
} else {
Expand Down
4 changes: 1 addition & 3 deletions lib/environment/Environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ Environment.prototype.register = function register(router) {
this.start();
}

if (router.getParentRouter === undefined || !router.getParentRouter()) {
this.routers.push(router);
}
this.routers.push(router);
};

/**
Expand Down
114 changes: 104 additions & 10 deletions tests/browser/browser.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
var assert = require('power-assert');
var assert = require('assert');
var assign = Object.assign || require('object-assign');
var React = require('react');
var ReactDOM = require('react-dom');
var ReactTestUtils = require('react/lib/ReactTestUtils');
Expand Down Expand Up @@ -230,15 +231,19 @@ describe('Routing', function() {
});

describe('Navigation lifecycle callbacks', function () {
it('calls onBeforeNaviation and onNavigation', function(done) {
it('calls onBeforeNavigation and onNavigation', function(done) {
assertRendered('mainpage');
var called = [];
app.setState({
beforeNavigationHandler: function (nextPath) {
beforeNavigationHandler: function (nextPath, navigation, match) {
called.push(nextPath);
assert.equal(match.match.slug, 'hello');
assert(!navigation.match);
},
navigationHandler: function (path) {
navigationHandler: function (path, navigation, match) {
called.push(path);
assert.equal(match.match.slug, 'hello');
assert(!navigation.match);
}
});
router.navigate('/__zuul/hello', function () {
Expand Down Expand Up @@ -390,7 +395,7 @@ describe('Nested routers', function() {
var NestedRouter = React.createClass({
render: function() {
return div(null,
Locations(null,
Locations(this.props,
Location({
path: '/__zuul/nested/',
handler: div(null, 'nested/root')
Expand All @@ -405,9 +410,17 @@ describe('Nested routers', function() {

var App = React.createClass({

getInitialState: function() {
return {};
},

render: function() {
return div(null,
Locations({ref: 'router', className: 'App'},
Locations({
ref: 'router', className: 'App',
onNavigation: this.state.navigationHandler,
onBeforeNavigation: this.state.beforeNavigationHandler
},
Location({
path: '/__zuul',
foo: 'bar',
Expand All @@ -420,7 +433,9 @@ describe('Nested routers', function() {
}),
Location({
path: '/__zuul/nested/*',
handler: NestedRouter
handler: NestedRouter,
onNavigation: this.state.navigationHandler,
onBeforeNavigation: this.state.beforeNavigationHandler
})
),
CaptureClicks({gotoURL: this.gotoURL},
Expand Down Expand Up @@ -457,6 +472,42 @@ describe('Nested routers', function() {
});
});

it('calls onBeforeNaviation and onNavigation with correct match objects w/ multiple routers', function(done) {
assertRendered('mainpage');
var called = [];
router.navigate('/__zuul/nested/');
// Goes beforeNav, beforeNav, nav, nav
app.setState({
beforeNavigationHandler: function (nextPath, navigation, match) {
called.push(nextPath);
if (called.length === 1) {
assert.equal(match.match._[0], 'page');
assert.equal(match.matchedPath, '/__zuul/nested/');
} else {
assert.equal(match.matchedPath, '/__zuul/nested/page');
}
assert(!navigation.match);
},
navigationHandler: function (path, navigation, match) {
called.push(path);
if (called.length === 3) {
assert.equal(match.match._[0], 'page');
assert.equal(match.matchedPath, '/__zuul/nested/');
} else {
assert.equal(match.matchedPath, '/__zuul/nested/page');
}
assert(!navigation.match);
}
});
router.navigate('/__zuul/nested/page', function () {
assert.equal(called.length, 4);
called.forEach(function(c) {
assert.equal(c, '/__zuul/nested/page');
});
done();
});
});

describe('CaptureClicks component', function() {
it('navigates to a subroute via onClick event', function(done) {
assertRendered('mainpage');
Expand Down Expand Up @@ -498,7 +549,7 @@ describe('Contextual routers', function() {

render: function() {
return div(null,
Locations({ref: 'router', contextual: true},
Locations(assign({ref: 'router', contextual: true}, this.props),
Location({
path: '/',
handler: div(null, 'subcat/root')
Expand All @@ -519,16 +570,26 @@ describe('Contextual routers', function() {

var App = React.createClass({

getInitialState: function() {
return {};
},

render: function() {
return Locations({ref: 'router'},
return Locations({
ref: 'router',
onNavigation: this.state.navigationHandler,
onBeforeNavigation: this.state.beforeNavigationHandler
},
Location({
path: '/__zuul',
handler: div(null, "mainpage")
}),
Location({
path: '/__zuul/subcat/*',
handler: SubCat,
ref: 'subcat'
ref: 'subcat',
onNavigation: this.state.navigationHandler,
onBeforeNavigation: this.state.beforeNavigationHandler
})
);
}
Expand Down Expand Up @@ -587,6 +648,39 @@ describe('Contextual routers', function() {
});
});
});

it('calls onBeforeNaviation and onNavigation with correct match objects w/ contextual routers', function(done) {
assertRendered('mainpage');
var called = [];
router.navigate('/__zuul/subcat/');
// Goes beforeNav, beforeNav, nav, nav
app.setState({
beforeNavigationHandler: function (nextPath, navigation, match) {
called.push(nextPath);
if (called.length === 1) {
assert.equal(match.match._[0], 'page');
assert.equal(match.matchedPath, '/__zuul/subcat/');
} else {
assert.equal(match.matchedPath, '/page');
}
assert(!navigation.match);
},
navigationHandler: function (path, navigation, match) {
called.push(path);
if (called.length === 3) {
assert.equal(match.match._[0], 'page');
assert.equal(match.matchedPath, '/__zuul/subcat/');
} else {
assert.equal(match.matchedPath, '/page');
}
assert(!navigation.match);
}
});
router.navigate('/__zuul/subcat/page', function () {
assert.equal(called.length, 4);
done();
});
});
});

describe('Multiple active routers', function() {
Expand Down

0 comments on commit e4f3446

Please sign in to comment.