From 026b7e32cf4ab69a6de3d3ada23b3184c2dc2b60 Mon Sep 17 00:00:00 2001 From: Kevin Jennison Date: Mon, 29 Mar 2021 17:12:35 -0700 Subject: [PATCH 1/3] 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 ( - + {children} @@ -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() - 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() 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() + 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() + expect(wrapper.find('a').first().prop('target')).toEqual('_blank') + }) }) From 473a7bb99f2cf8a869359522f71b1f283d53cadf Mon Sep 17 00:00:00 2001 From: Kevin Jennison Date: Mon, 29 Mar 2021 17:14:22 -0700 Subject: [PATCH 2/3] Fix typo --- src/components/OnboardingFlow.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/OnboardingFlow.js b/src/components/OnboardingFlow.js index f6935e7a..8734da7e 100644 --- a/src/components/OnboardingFlow.js +++ b/src/components/OnboardingFlow.js @@ -63,7 +63,7 @@ export const onboardingStepContents = (classes) => [ align="center" className={classes.childrenTypography} > - Tabbers like you are supporting critical nonprofit all around the + Tabbers like you are supporting critical nonprofit work all around the world. Thank you! From d4ba2ac1470abe7c2e34de9b346b00b6644367a3 Mon Sep 17 00:00:00 2001 From: Kevin Jennison Date: Mon, 29 Mar 2021 17:14:41 -0700 Subject: [PATCH 3/3] Update copy and fix link styling --- src/components/ImpactCounter.js | 4 ++-- src/components/OnboardingFlow.js | 18 ++++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/components/ImpactCounter.js b/src/components/ImpactCounter.js index 63761407..f925e4bb 100644 --- a/src/components/ImpactCounter.js +++ b/src/components/ImpactCounter.js @@ -85,8 +85,8 @@ const ImpactCounter = (props) => { className={classes.dropdownText} gutterBottom > - This shows how many treats you've provided to help shelter cats get - adopted. Every tab you open helps. Keep it up! + This shows how many treats your tabs can provide to help shelter + cats get adopted. Every tab you open helps. Keep it up! diff --git a/src/components/OnboardingFlow.js b/src/components/OnboardingFlow.js index 8734da7e..ff84a543 100644 --- a/src/components/OnboardingFlow.js +++ b/src/components/OnboardingFlow.js @@ -11,7 +11,7 @@ import onboarding3 from 'src/assets/onboarding/onboarding3.png' import PropTypes from 'prop-types' import Link from 'src/components/Link' -export const useStyles = makeStyles(() => ({ +export const useStyles = makeStyles((theme) => ({ card: { display: 'flex', width: '400px', @@ -43,6 +43,11 @@ export const useStyles = makeStyles(() => ({ justifyContent: 'flex-end', display: 'flex', }, + link: { + display: 'inline', + color: theme.palette.primary.main, + textDecoration: 'none', + }, })) export const onboardingStepContents = (classes) => [ @@ -79,9 +84,14 @@ export const onboardingStepContents = (classes) => [ align="center" className={classes.childrenTypography} > - Your tabs will help shelter cats get adopted by - - providing treats used in positive reinforcement training. + Your tabs support initiatives that help shelter cats get adopted, + including initiatives that{' '} + + use treats in positive reinforcement training.