From 026b7e32cf4ab69a6de3d3ada23b3184c2dc2b60 Mon Sep 17 00:00:00 2001
From: Kevin Jennison <kevin.jennison1@gmail.com>
Date: Mon, 29 Mar 2021 17:12:35 -0700
Subject: [PATCH] Support the target prop on the Link anchor

---
 src/components/Link.js                | 20 +++++++++++++++-----
 src/components/__tests__/Link.test.js | 24 +++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/src/components/Link.js b/src/components/Link.js
index 565117df..7128db5b 100644
--- a/src/components/Link.js
+++ b/src/components/Link.js
@@ -14,10 +14,12 @@ const useStyles = makeStyles(() => ({
 }))
 
 const Link = (props) => {
-  const { children, className, to } = props
+  const { children, className, target, to } = props
   const classes = useStyles()
   const [destInternal, setDestInternal] = useState(true)
 
+  // Explicitly set target="_top" for external links, because our
+  // app may be served in an iframe.
   // We won't always server-render the "target" prop correctly.
   // If that causes problems, use the URL from the request when
   // server-side rendering.
@@ -31,12 +33,18 @@ const Link = (props) => {
     }
   }, [to])
 
+  // Always use the anchor prop, if provided; otherwise, fall
+  // back to the default.
+  let anchorTarget
+  if (target) {
+    anchorTarget = target
+  } else {
+    anchorTarget = destInternal ? undefined : '_top'
+  }
+
   return (
     <NextJsLink href={to}>
-      <a
-        target={destInternal ? undefined : '_top'}
-        className={clsx(classes.anchor, className)}
-      >
+      <a target={anchorTarget} className={clsx(classes.anchor, className)}>
         {children}
       </a>
     </NextJsLink>
@@ -52,10 +60,12 @@ Link.propTypes = {
   ]).isRequired,
   className: PropTypes.string,
   to: PropTypes.string.isRequired,
+  target: PropTypes.string,
 }
 
 Link.defaultProps = {
   className: '',
+  target: undefined,
 }
 
 export default Link
diff --git a/src/components/__tests__/Link.test.js b/src/components/__tests__/Link.test.js
index 5acb862a..db901855 100644
--- a/src/components/__tests__/Link.test.js
+++ b/src/components/__tests__/Link.test.js
@@ -11,6 +11,7 @@ const getMockProps = () => ({
   children: 'hi',
   className: 'some-class',
   to: 'https://tab.gladly.io/newtab/blah/',
+  target: undefined,
 })
 
 beforeEach(() => {
@@ -61,7 +62,7 @@ describe('Link component', () => {
       className: 'some-class-here',
     }
     const wrapper = shallow(<Link {...mockProps} />)
-    const anchor = wrapper.childAt(0)
+    const anchor = wrapper.find('a').first()
     expect(anchor.prop('className')).toContain('some-class-here')
   })
 
@@ -96,4 +97,25 @@ describe('Link component', () => {
     mount(<Link {...mockProps} />)
     expect(isURLForDifferentApp).toHaveBeenCalledWith(`/my-base-path/my/thing`)
   })
+
+  it('assigns the "target" prop to the anchor if it is provided', () => {
+    const Link = require('src/components/Link').default
+    const mockProps = {
+      ...getMockProps(),
+      target: '_blank',
+    }
+    const wrapper = shallow(<Link {...mockProps} />)
+    expect(wrapper.find('a').first().prop('target')).toEqual('_blank')
+  })
+
+  it('uses the "target" prop when provided, rather than target="_top", when it is an external link', () => {
+    const Link = require('src/components/Link').default
+    const mockProps = {
+      ...getMockProps(),
+      url: 'https://blahblah.com',
+      target: '_blank',
+    }
+    const wrapper = shallow(<Link {...mockProps} />)
+    expect(wrapper.find('a').first().prop('target')).toEqual('_blank')
+  })
 })