Skip to content

Add target _blank on external links for improve user experience #887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add target _blank on external links for improve user experience #887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cdf618e
feat: add isExternal prop
iamdevmarcos Jan 3, 2023
e350d5c
test: update snapshots
iamdevmarcos Jan 3, 2023
da4bfa7
feat: add target blank on all external links
iamdevmarcos Jan 3, 2023
14618ef
refactor: remove commented code
iamdevmarcos Jan 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/BlmBanner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const Banner = styled.a`

export const BlmBanner = () => {
return (
<Banner href="https://support.eji.org/give/153413/#!/donation/checkout">
<Banner href="https://support.eji.org/give/153413/#!/donation/checkout" target="_blank">
#BlackLivesMatter ✊🏿 <span style={{ textDecoration: `underline` }}>Support the Equal Justice Initiative</span>
</Banner>
);
Expand Down
9 changes: 7 additions & 2 deletions components/Link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const InlineLink = styled.a.attrs((/* props */) => ({
}
`;

const Link = ({ ['aria-label']: ariaLabel, children, className, inline, unstyled, white, ...rest }) => {
const Link = ({ ['aria-label']: ariaLabel, children, className, inline, unstyled, white, isExternal, ...rest }) => {
let Child = StyledLink;
if (inline) {
Child = InlineLink;
Expand All @@ -46,9 +46,14 @@ const Link = ({ ['aria-label']: ariaLabel, children, className, inline, unstyled
dataAttrs = { 'data-white': white };
}

let target;
if (isExternal) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if instead of an explicit isExternal flag we just looked at href and determined if it's a relative path or not?

target = '_blank';
}

return (
<UnstyledLink {...rest}>
<Child aria-label={ariaLabel} href={rest.href} className={className} {...dataAttrs}>
<Child aria-label={ariaLabel} href={rest.href} className={className} {...dataAttrs} target={target}>
{children}
</Child>
</UnstyledLink>
Expand Down
21 changes: 10 additions & 11 deletions components/Nav/Social.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ const SocialLink = styled(Link).attrs((/* props */) => ({
`;

const Svg = styled.svg`
width: ${p => rem(Number(p.width))};
height: ${p => rem(Number(p.height))};
width: ${(p) => rem(Number(p.width))};
height: ${(p) => rem(Number(p.height))};
`;

const StyledIcon = styled.div`
&& {
width: ${p => rem(Number(p.width))};
height: ${p => rem(Number(p.height))};
width: ${(p) => rem(Number(p.width))};
height: ${(p) => rem(Number(p.height))};
}
`;

Expand All @@ -73,18 +73,17 @@ const Spectrum = () => (
</Svg>
);

const Social = props => (
const Social = (props) => (
<Wrapper {...props}>
<SocialLink href="https://spectrum.chat/styled-components/">
<SocialLink href="https://spectrum.chat/styled-components/" isExternal>
<Spectrum />
</SocialLink>
{/* <SocialLink href="https://twitter.com/someone">
<Twitter />
</SocialLink> */}
<SocialLink href="https://github.com/styled-components">

<SocialLink href="https://github.com/styled-components" isExternal>
<StyledIcon as={Github} height="18" />
</SocialLink>
<SocialLink href="https://medium.com/styled-components">

<SocialLink href="https://medium.com/styled-components" isExternal>
<StyledIcon as={MediumM} height="18" />
</SocialLink>
</Wrapper>
Expand Down
3 changes: 3 additions & 0 deletions test/components/NavBar/__snapshots__/MobileNavbar.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ exports[`MobileNavbar renders correctly 1`] = `
<a
className="c16"
href="https://spectrum.chat/styled-components/"
target="_blank"
>
<svg
className="c17"
Expand All @@ -387,6 +388,7 @@ exports[`MobileNavbar renders correctly 1`] = `
<a
className="c16"
href="https://github.com/styled-components"
target="_blank"
>
<svg
className="c18"
Expand All @@ -396,6 +398,7 @@ exports[`MobileNavbar renders correctly 1`] = `
<a
className="c16"
href="https://medium.com/styled-components"
target="_blank"
>
<svg
className="c18"
Expand Down
6 changes: 6 additions & 0 deletions test/components/NavBar/__snapshots__/Navbar.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,7 @@ exports[`Navbar renders correctly 1`] = `
<a
className="c17"
href="https://spectrum.chat/styled-components/"
target="_blank"
>
<svg
className="c18"
Expand All @@ -691,6 +692,7 @@ exports[`Navbar renders correctly 1`] = `
<a
className="c17"
href="https://github.com/styled-components"
target="_blank"
>
<svg
className="c19"
Expand All @@ -700,6 +702,7 @@ exports[`Navbar renders correctly 1`] = `
<a
className="c17"
href="https://medium.com/styled-components"
target="_blank"
>
<svg
className="c19"
Expand Down Expand Up @@ -819,6 +822,7 @@ exports[`Navbar renders correctly 1`] = `
<a
className="c17"
href="https://spectrum.chat/styled-components/"
target="_blank"
>
<svg
className="c18"
Expand All @@ -838,6 +842,7 @@ exports[`Navbar renders correctly 1`] = `
<a
className="c17"
href="https://github.com/styled-components"
target="_blank"
>
<svg
className="c19"
Expand All @@ -847,6 +852,7 @@ exports[`Navbar renders correctly 1`] = `
<a
className="c17"
href="https://medium.com/styled-components"
target="_blank"
>
<svg
className="c19"
Expand Down
18 changes: 18 additions & 0 deletions test/components/NavBar/__snapshots__/index.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,10 +874,12 @@ exports[`Nav renders correctly 1`] = `
>
<Social__SocialLink
href="https://spectrum.chat/styled-components/"
isExternal={true}
>
<Link
className="c17"
href="https://spectrum.chat/styled-components/"
isExternal={true}
unstyled={true}
>
<Link
Expand All @@ -886,6 +888,7 @@ exports[`Nav renders correctly 1`] = `
<a
className="c17"
href="https://spectrum.chat/styled-components/"
target="_blank"
>
<Spectrum>
<Social__Svg
Expand Down Expand Up @@ -915,10 +918,12 @@ exports[`Nav renders correctly 1`] = `
</Social__SocialLink>
<Social__SocialLink
href="https://github.com/styled-components"
isExternal={true}
>
<Link
className="c17"
href="https://github.com/styled-components"
isExternal={true}
unstyled={true}
>
<Link
Expand All @@ -927,6 +932,7 @@ exports[`Nav renders correctly 1`] = `
<a
className="c17"
href="https://github.com/styled-components"
target="_blank"
>
<Social__StyledIcon
as={[Function]}
Expand All @@ -948,10 +954,12 @@ exports[`Nav renders correctly 1`] = `
</Social__SocialLink>
<Social__SocialLink
href="https://medium.com/styled-components"
isExternal={true}
>
<Link
className="c17"
href="https://medium.com/styled-components"
isExternal={true}
unstyled={true}
>
<Link
Expand All @@ -960,6 +968,7 @@ exports[`Nav renders correctly 1`] = `
<a
className="c17"
href="https://medium.com/styled-components"
target="_blank"
>
<Social__StyledIcon
as={[Function]}
Expand Down Expand Up @@ -1228,10 +1237,12 @@ exports[`Nav renders correctly 1`] = `
>
<Social__SocialLink
href="https://spectrum.chat/styled-components/"
isExternal={true}
>
<Link
className="c17"
href="https://spectrum.chat/styled-components/"
isExternal={true}
unstyled={true}
>
<Link
Expand All @@ -1240,6 +1251,7 @@ exports[`Nav renders correctly 1`] = `
<a
className="c17"
href="https://spectrum.chat/styled-components/"
target="_blank"
>
<Spectrum>
<Social__Svg
Expand Down Expand Up @@ -1269,10 +1281,12 @@ exports[`Nav renders correctly 1`] = `
</Social__SocialLink>
<Social__SocialLink
href="https://github.com/styled-components"
isExternal={true}
>
<Link
className="c17"
href="https://github.com/styled-components"
isExternal={true}
unstyled={true}
>
<Link
Expand All @@ -1281,6 +1295,7 @@ exports[`Nav renders correctly 1`] = `
<a
className="c17"
href="https://github.com/styled-components"
target="_blank"
>
<Social__StyledIcon
as={[Function]}
Expand All @@ -1302,10 +1317,12 @@ exports[`Nav renders correctly 1`] = `
</Social__SocialLink>
<Social__SocialLink
href="https://medium.com/styled-components"
isExternal={true}
>
<Link
className="c17"
href="https://medium.com/styled-components"
isExternal={true}
unstyled={true}
>
<Link
Expand All @@ -1314,6 +1331,7 @@ exports[`Nav renders correctly 1`] = `
<a
className="c17"
href="https://medium.com/styled-components"
target="_blank"
>
<Social__StyledIcon
as={[Function]}
Expand Down
Loading