Repository: kyeotic/raviger (ts)

Issue:
<Link> component does not respect target attribute
Hi there, thanks for the great library!

I am not sure if it's a bug or if working as designed, I'll let you decide that, but the documentation states:

> The preferred method for navigation is the `<Link>` component, which uses all the same properties as the standard `<a>` element, and requires href.

...while in practice, when setting a `target` prop on the `<Link>` component it does not behave as expected (not as an `<a>` tag would).

E.g.: `<Link href="/dashboard" target="_blank" />` for which you would expect the link to open in a new tab.

You could argue that there is no reason to use a `<Link>` component for this use case and just revert to using a simple `<a>` instead, which would be fair.

Pull request:
dont trap Link on target=_blank
closes #24 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c221c7..6b0241a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.5.8] - 2019-10-14
+### Fixed
+- `<Link target="_blank" />` triggering local navigation
+
 ## [0.5.7] - 2019-09-26
 ### Added
 - `<ActiveLink />`
diff --git a/src/Link.js b/src/Link.js
index 4230688..b8c6bf7 100644
--- a/src/Link.js
+++ b/src/Link.js
@@ -13,7 +13,7 @@ export default function Link(props) {
         e.preventDefault()
         throw ex
       }
-      if (shouldTrap(e)) {
+      if (shouldTrap(e, props)) {
         e.preventDefault() // prevent the link from actually navigating
         navigate(e.currentTarget.href)
       }
@@ -38,10 +38,12 @@ function getLinkHref(href, basePath = '') {
   return href.substring(0, 1) === '/' ? basePath + href : href
 }
 
-function shouldTrap(e) {
+function shouldTrap(e, props) {
+  let { target } = props
   return (
     !e.defaultPrevented && // onClick prevented default
     e.button === 0 && // ignore everything but left clicks
+    !(target || target === '_self') && // dont trap target === blank
     !(e.metaKey || e.altKey || e.ctrlKey || e.shiftKey)
   )
 }


Test patch:
diff --git a/test/Link.spec.js b/test/Link.spec.js
index 4858d5d..1586317 100644
--- a/test/Link.spec.js
+++ b/test/Link.spec.js
@@ -40,6 +40,16 @@ describe('Link', () => {
     act(() => void fireEvent.click(getByTestId('link')))
     expect(document.location.pathname).toEqual('/foo')
   })
+  test('doesnt navigate for target=blank', async () => {
+    act(() => navigate('/'))
+    const { getByTestId } = render(
+      <Link href="/foo" className="base" target="_blank" data-testid="link">
+        go to foo
+      </Link>
+    )
+    act(() => void fireEvent.click(getByTestId('link')))
+    expect(document.location.pathname).toEqual('/')
+  })
 })
 describe('ActiveLink', () => {
   test('adds active class when active', async () => {