[fixed] isActive on <Link onlyActiveOnIndex>

Fixes remix-run#2066 Closes remix-run#2069
danieljuhl · Sep 25, 2015 · 37d9bac · 37d9bac
1 parent 029646f
commit 37d9bac
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 41 deletions.
diff --git a/modules/__tests__/IndexLink-test.js b/modules/__tests__/IndexLink-test.js
@@ -8,56 +8,56 @@ import Router from '../Router'
 import Route from '../Route'
 import IndexLink from '../IndexLink'
 
-describe('an <IndexLink/>', function () {
+describe('An <IndexLink>', function () {
 
-  var node
-  beforeEach(function () {
-    node = document.createElement('div')
-  })
-
-  afterEach(function () {
-    React.unmountComponentAtNode(node)
-  })
-
-  var App = React.createClass({
-    render () {
+  const App = React.createClass({
+    render() {
       return (
         <div>
           <ul>
-            <li><IndexLink id="appLink" to="/" activeClassName="active">app </IndexLink></li>
-            <li><IndexLink id="deepLink" to="/deep" activeClassName="active">deep </IndexLink></li>
+            <li><IndexLink id="appLink" to="/" activeClassName="active">app</IndexLink></li>
+            <li><IndexLink id="deepLink" to="/deep" activeClassName="active">deep</IndexLink></li>
           </ul>
           {this.props.children}
         </div>
       )
     }
   })
 
-  var Parent = React.createClass({
-    render () {
+  const Parent = React.createClass({
+    render() {
       return <div>parent {this.props.children}</div>
     }
   })
 
-  var Child = React.createClass({
-    render () {
+  const Child = React.createClass({
+    render() {
       return <div>child </div>
     }
   })
 
-  var routes = (
+  const routes = (
     <Route path="/" component={App}>
-      <IndexRoute component={Child}/>
+      <IndexRoute component={Child} />
       <Route path="/deep" component={Parent}>
-        <IndexRoute component={Child}/>
+        <IndexRoute component={Child} />
       </Route>
     </Route>
   )
 
+  let node
+  beforeEach(function () {
+    node = document.createElement('div')
+  })
+
+  afterEach(function () {
+    React.unmountComponentAtNode(node)
+  })
+
   describe('when linking to the root', function () {
-    it('is active when the parent’s route is active', function (done) {
+    it("is active when the parent's route is active", function (done) {
       React.render((
-        <Router history={createHistory('/')} routes={routes}/>
+        <Router history={createHistory('/')} routes={routes} />
       ), node, function () {
         expect(node.querySelector('#appLink').className).toEqual('active')
         expect(node.querySelector('#deepLink').className).toEqual('')
@@ -67,9 +67,9 @@ describe('an <IndexLink/>', function () {
   })
 
   describe('when linking deep into the route hierarchy', function () {
-    it('is active when the parent’s route is active', function (done) {
+    it("is active when the parent's route is active", function (done) {
       React.render((
-        <Router history={createHistory('/deep')} routes={routes}/>
+        <Router history={createHistory('/deep')} routes={routes} />
       ), node, function () {
         expect(node.querySelector('#appLink').className).toEqual('')
         expect(node.querySelector('#deepLink').className).toEqual('active')

diff --git a/modules/isActive.js b/modules/isActive.js
@@ -1,19 +1,25 @@
 import { matchPattern } from './PatternUtils'
 
 /**
- * Returns true if a route and params that match the given
- * pathname are currently active.
+ * Returns true if the given pathname matches the active pathname.
  */
-function pathnameIsActive(pathname, activePathname, activeRoutes, activeParams) {
-  if (pathname === activePathname || activePathname.indexOf(pathname + '/') === 0)
-    return true
+function pathnameIsActive(pathname, activePathname) {
+  return pathname === activePathname || activePathname.indexOf(pathname + '/') === 0
+}
+
+function paramsAreActive(paramNames, paramValues, activeParams) {
+  return paramNames.every(function (paramName, index) {
+    return String(paramValues[index]) === String(activeParams[paramName])
+  })
+}
 
+function getMatchingRoute(pathname, activeRoutes, activeParams) {
   let route, pattern, basename = ''
   for (let i = 0, len = activeRoutes.length; i < len; ++i) {
     route = activeRoutes[i]
 
-    if (!route.path)
-      return false
+    //if (!route.path)
+    //  return false
 
     pattern = route.path || ''
 
@@ -22,16 +28,29 @@ function pathnameIsActive(pathname, activePathname, activeRoutes, activeParams)
 
     let { remainingPathname, paramNames, paramValues } = matchPattern(pattern, pathname)
 
-    if (remainingPathname === '') {
-      return paramNames.every(function (paramName, index) {
-        return String(paramValues[index]) === String(activeParams[paramName])
-      })
-    }
+    if (remainingPathname === '' && paramsAreActive(paramNames, paramValues, activeParams))
+      return route
 
     basename = pattern
   }
 
-  return false
+  return null
+}
+
+/**
+ * Returns true if the given pathname matches the active routes
+ * and params.
+ */
+function routeIsActive(pathname, activeRoutes, activeParams, indexOnly) {
+  let route = getMatchingRoute(pathname, activeRoutes, activeParams)
+
+  if (route == null)
+    return false
+
+  if (indexOnly)
+    return activeRoutes.length > 1 && activeRoutes[activeRoutes.length - 2].indexRoute === route
+
+  return true
 }
 
 /**
@@ -60,11 +79,10 @@ function isActive(pathname, query, indexOnly, location, routes, params) {
   if (location == null)
     return false
 
-  if (indexOnly && (routes.length < 2 || routes[routes.length - 2].indexRoute !== routes[routes.length - 1]))
+  if (!pathnameIsActive(pathname, location.pathname) && !routeIsActive(pathname, routes, params, indexOnly))
     return false
 
-  return pathnameIsActive(pathname, location.pathname, routes, params) &&
-    queryIsActive(query, location.query)
+  return queryIsActive(query, location.query)
 }
 
 export default isActive