Merge pull request #13 from caseyWebb/increase-coverage

moar test coverage plz
Profiscience · Jan 5, 2016 · 0ce1ab1 · 0ce1ab1
2 parents ed147c0 + 812134f
commit 0ce1ab1
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 55 deletions.
diff --git a/dist/ko-component-router.js b/dist/ko-component-router.js
@@ -179,31 +179,13 @@ return /******/ (function(modules) { // webpackBootstrap
 	        return;
 	      }
 
-	      // Ignore if tag has
-	      // 1. "download" attribute
-	      // 2. rel="external" attribute
-	      if (el.hasAttribute('download') || el.getAttribute('rel') === 'external') {
-	        return;
-	      }
-
-	      // ensure non-hash for the same path
-	      var link = el.getAttribute('href');
-	      if (!this.config.hashbang && el.pathname === location.pathname && (el.hash || '#' === link)) {
-	        return;
-	      }
-
-	      // Check for mailto: in the href
-	      if (link && link.indexOf('mailto:') > -1) {
-	        return;
-	      }
-
-	      // check target
-	      if (el.target) {
-	        return;
-	      }
+	      var isDownload = el.hasAttribute('download');
+	      var hasOtherTarget = el.hasAttribute('target');
+	      var hasExternalRel = el.getAttribute('rel') === 'external';
+	      var isMailto = ~(el.getAttribute('href') || '').indexOf('mailto:');
+	      var isCrossOrigin = !sameOrigin(el.href);
 
-	      // x-origin
-	      if (!sameOrigin(el.href)) {
+	      if (isDownload || hasOtherTarget || hasExternalRel || isMailto || isCrossOrigin) {
 	        return;
 	      }
 

diff --git a/dist/ko-component-router.min.js b/dist/ko-component-router.min.js
diff --git a/example/dist/bundle.js b/example/dist/bundle.js
diff --git a/example/dist/bundle.js.map b/example/dist/bundle.js.map
diff --git a/karma.conf.js b/karma.conf.js
@@ -39,7 +39,8 @@ module.exports = function(config) {
             loader: 'babel'
           }
         ]
-      }
+      },
+      devtool: 'eval-source-map'
     },
 
     webpackMiddleware: {

diff --git a/src/router.js b/src/router.js
@@ -89,31 +89,13 @@ class Router {
       return
     }
 
-    // Ignore if tag has
-    // 1. "download" attribute
-    // 2. rel="external" attribute
-    if (el.hasAttribute('download') || el.getAttribute('rel') === 'external') {
-      return
-    }
-
-    // ensure non-hash for the same path
-    const link = el.getAttribute('href')
-    if (!this.config.hashbang && el.pathname === location.pathname && (el.hash || '#' === link)) {
-      return
-    }
-
-    // Check for mailto: in the href
-    if (link && link.indexOf('mailto:') > -1) {
-      return
-    }
-
-    // check target
-    if (el.target) {
-      return
-    }
+    const isDownload = el.hasAttribute('download')
+    const hasOtherTarget = el.hasAttribute('target')
+    const hasExternalRel = el.getAttribute('rel') === 'external'
+    const isMailto = ~(el.getAttribute('href') || '').indexOf('mailto:')
+    const isCrossOrigin = !sameOrigin(el.href)
 
-    // x-origin
-    if (!sameOrigin(el.href)) {
+    if (isDownload || hasOtherTarget || hasExternalRel || isMailto || isCrossOrigin) {
       return
     }
 

diff --git a/test.js b/test.js
@@ -168,9 +168,42 @@ function runTests(t, config) {
     .step(() => {
       const activeLink = $('#should-be-active', dom)
       t.ok(activeLink.hasClass('active-path'), 'path binding sets `active` class')
+    })
 
-      resolve()
+    // anchors
+    .step(() => {
+      router.update('/anchors')
+    })
+    .step(() => {
+      $('body').append($('#about-link', dom))
+      const aboutLink = $('#about-link').get(0)
+
+      aboutLink.click()
+    })
+    .step(() => {
+      const aboutLink = $('#about-link').get(0)
+      t.equal(ko.contextFor(aboutLink).$router.component(), 'about', 'clicking a link navigates')
+    })
+    .step(() => {
+      router.update('/anchors')
+    })
+    .step(() => {
+      $('body').append($('#ignored-links', dom))
+
+      let count = 0
+      $('body').on('click', (e) => {
+        count++
+        e.preventDefault()
+      })
+
+      $('#ignored-links *').each((i, el) => {
+        el.click()
+      })
+
+      t.equal(count, $('#ignored-links *').length, 'ignores appropriate links')
     })
+
+    .step(() => resolve())
   })
 }
 
@@ -188,8 +221,9 @@ class RoutingTest {
       // route w/ nested router
       '/nested/!': 'nested',
 
-      // bindings test component
+      // various test components
       '/bindings': 'bindings',
+      '/anchors': 'anchors',
 
       // named wildcard segment
       '/file/:file(*)': 'file',
@@ -227,9 +261,23 @@ ko.components.register('bindings', {
     <a id="query-binding-anchor" data-bind="query: { bar: 'bar' }"></a>
   `
 })
+ko.components.register('anchors', {
+  synchronous: true,
+  template: `
+    <a id="about-link" href="/about"></a>
+    <div id="ignored-links">
+      <button id="not-a-link"></button>
+      <a id="download-link" download="/foo"></a>
+      <a id="blank-target-link" target="_blank"></a>
+      <a id="external-link" rel="external"></a>
+      <a id="mailto-link" href="mailto:foobar@example.com"></a>
+      <a id="cross-origin-link" href="http://example.com/"></a>
+    </div>
+  `
+})
 
 test('ko-component-router', (t) => {
-  const NUM_TESTS = 26 * 4 + 4
+  const NUM_TESTS = 28 * 4 + 4
   t.plan(NUM_TESTS)
 
   t.assert(ko.components.isRegistered('ko-component-router'), 'should register <ko-component-router />')