From a02b4b327ee79ab71ccfd829fad1347d6979ebf1 Mon Sep 17 00:00:00 2001 From: Benjamin Goering <171782+gobengo@users.noreply.github.com> Date: Mon, 17 Oct 2022 16:21:55 +0100 Subject: [PATCH 1/5] feat: website link component defaults target to _blank when the href is to an origin different than the current document --- .../components/breadcrumbs/breadcrumbs.js | 2 +- packages/website/components/link/link.js | 63 ++++++++++++++++--- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/packages/website/components/breadcrumbs/breadcrumbs.js b/packages/website/components/breadcrumbs/breadcrumbs.js index 212a1daaa7..1d79f3680c 100644 --- a/packages/website/components/breadcrumbs/breadcrumbs.js +++ b/packages/website/components/breadcrumbs/breadcrumbs.js @@ -10,7 +10,7 @@ import Link from '../link/link'; * @param {Object} props * @param {String} props.variant * @param {Object} [props.items] - * @param {Function} props.click + * @param {React.MouseEventHandler} props.click */ export default function Breadcrumbs({ variant, click, items }) { return ( diff --git a/packages/website/components/link/link.js b/packages/website/components/link/link.js index c243beee5b..e60a4bc665 100644 --- a/packages/website/components/link/link.js +++ b/packages/website/components/link/link.js @@ -2,14 +2,61 @@ import React from 'react'; import PropTypes from 'prop-types'; import Link from 'next/link'; -const WrappedLink = ({ tabIndex = 0, href, target = '_self', ...otherProps }) => ( - - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} - - {otherProps.children} - - -); +/** + * Return whether a url is 'external' relative to another URL. + * The url is considered external if it has a different hostname. + * @param {URL} url + * @param {URL} relativeTo - url to compare for externality + */ +function isExternalLink(url, relativeTo) { + return url.hostname !== relativeTo.hostname; +} + +/** + * @param {string} href - href attribute of an + * @returns {boolean} - whether the provided href is known to be external from the current document + */ +function useHrefExternality(href) { + const [document, setDocument] = React.useState(/** @type {Document|undefined} */ (undefined)); + // useEffect because next ssr wont have a document + React.useEffect(() => { + if (typeof globalThis.document !== 'undefined') { + setDocument(globalThis.document); + } + }, []); + const isExternalHref = React.useMemo(() => { + if (!document) { + return false; + } + const documentURL = new URL(document.URL); + const isExternal = isExternalLink(new URL(href, documentURL), documentURL); + return isExternal; + }, [document, href]); + return isExternalHref; +} + +/** + * A generic hyperlink component. + * * by default, links to external sites will have target=_blank if they are an external link + * @param {object} props + * @param {string} props.href - the href attribute for the link + * @param {number} [props.tabIndex] - the tabIndex attribute for the link + * @param {string} [props.target] - the target attribute for the link + * @param {React.ReactNode} [props.children] + * @param {React.MouseEventHandler} [props.onClick] - the onClick handler for the link + */ +const WrappedLink = ({ tabIndex = 0, href, target, ...otherProps }) => { + const isExternalHref = useHrefExternality(href); + const derrivedTarget = isExternalHref ? '_blank' : '_self'; + return ( + + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} + + {otherProps.children} + + + ); +}; WrappedLink.propTypes = { onClick: PropTypes.func, From 50b8e82d7cbdde8b46e23a4e7c21aceac1ffb965 Mon Sep 17 00:00:00 2001 From: Benjamin Goering <171782+gobengo@users.noreply.github.com> Date: Mon, 17 Oct 2022 16:39:38 +0100 Subject: [PATCH 2/5] fix: bug where new link derivedTarget would always be used even if props.target provided --- packages/website/components/link/link.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/website/components/link/link.js b/packages/website/components/link/link.js index e60a4bc665..912cd5a4d8 100644 --- a/packages/website/components/link/link.js +++ b/packages/website/components/link/link.js @@ -47,11 +47,11 @@ function useHrefExternality(href) { */ const WrappedLink = ({ tabIndex = 0, href, target, ...otherProps }) => { const isExternalHref = useHrefExternality(href); - const derrivedTarget = isExternalHref ? '_blank' : '_self'; + const derivedTarget = target ?? (isExternalHref ? '_blank' : '_self'); return ( {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} - + {otherProps.children} From 16a83801b3ba161e15cf5ecd155750c60af334a7 Mon Sep 17 00:00:00 2001 From: Benjamin Goering <171782+gobengo@users.noreply.github.com> Date: Mon, 17 Oct 2022 17:06:34 +0100 Subject: [PATCH 3/5] link component doesnt always use the isExternalLink -> target=_blank logic. Only footer links use the logic to provide target=_blank --- packages/website/components/footer/footer.js | 20 +++++++++-- packages/website/components/link/link.js | 35 ++++++++++++-------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/packages/website/components/footer/footer.js b/packages/website/components/footer/footer.js index defc1bd799..6d43c7294c 100644 --- a/packages/website/components/footer/footer.js +++ b/packages/website/components/footer/footer.js @@ -4,7 +4,7 @@ import { useRouter } from 'next/router'; import clsx from 'clsx'; import { trackCustomLinkClick, events } from '../../lib/countly'; -import Link from '../link/link'; +import Link, { useIsExternalHref } from '../link/link'; import SiteLogo from '../../assets/icons/w3storage-logo.js'; import Button from '../button/button'; import Img from '../cloudflareImage.js'; @@ -25,6 +25,8 @@ export default function Footer({ isProductApp }) { const resources = GeneralPageData.footer.resources; const getStarted = GeneralPageData.footer.get_started; const copyright = GeneralPageData.footer.copyright; + const isExternalHref = useIsExternalHref(); + const getLinkTarget = useCallback(href => (isExternalHref(href) ? '_blank' : undefined), [isExternalHref]); // ================================================================= Functions const onLinkClick = useCallback(e => { @@ -77,7 +79,13 @@ export default function Footer({ isProductApp }) {
{resources.heading}
{resources.items.map(item => ( - + {item.text} ))} @@ -88,7 +96,13 @@ export default function Footer({ isProductApp }) {
{getStarted.heading}
{getStarted.items.map(item => ( - + {item.text} ))} diff --git a/packages/website/components/link/link.js b/packages/website/components/link/link.js index 912cd5a4d8..0b7cd28699 100644 --- a/packages/website/components/link/link.js +++ b/packages/website/components/link/link.js @@ -13,10 +13,11 @@ function isExternalLink(url, relativeTo) { } /** - * @param {string} href - href attribute of an - * @returns {boolean} - whether the provided href is known to be external from the current document + * React hook that provides an isExternalHref function. + * @returns {(href: string) => boolean} - fn that determines whether the provided href + * is known to be external from the current document */ -function useHrefExternality(href) { +export function useIsExternalHref() { const [document, setDocument] = React.useState(/** @type {Document|undefined} */ (undefined)); // useEffect because next ssr wont have a document React.useEffect(() => { @@ -24,14 +25,22 @@ function useHrefExternality(href) { setDocument(globalThis.document); } }, []); - const isExternalHref = React.useMemo(() => { - if (!document) { - return false; - } - const documentURL = new URL(document.URL); - const isExternal = isExternalLink(new URL(href, documentURL), documentURL); - return isExternal; - }, [document, href]); + const isExternalHref = React.useCallback( + /** + * + * @param {string} href - href attribute of + * @returns {boolean} whether the provided href is external to the current document + */ + href => { + if (!document) { + return false; + } + const documentURL = new URL(document.URL); + const isExternal = isExternalLink(new URL(href, documentURL), documentURL); + return isExternal; + }, + [document] + ); return isExternalHref; } @@ -46,12 +55,10 @@ function useHrefExternality(href) { * @param {React.MouseEventHandler} [props.onClick] - the onClick handler for the link */ const WrappedLink = ({ tabIndex = 0, href, target, ...otherProps }) => { - const isExternalHref = useHrefExternality(href); - const derivedTarget = target ?? (isExternalHref ? '_blank' : '_self'); return ( {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} - + {otherProps.children} From 1db73037734ad188ceb681c2deb90793d64f1c02 Mon Sep 17 00:00:00 2001 From: Benjamin Goering <171782+gobengo@users.noreply.github.com> Date: Mon, 17 Oct 2022 18:14:23 +0100 Subject: [PATCH 4/5] small change for smaller diff --- packages/website/components/link/link.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/website/components/link/link.js b/packages/website/components/link/link.js index 0b7cd28699..52aa7cbe52 100644 --- a/packages/website/components/link/link.js +++ b/packages/website/components/link/link.js @@ -54,16 +54,14 @@ export function useIsExternalHref() { * @param {React.ReactNode} [props.children] * @param {React.MouseEventHandler} [props.onClick] - the onClick handler for the link */ -const WrappedLink = ({ tabIndex = 0, href, target, ...otherProps }) => { - return ( - - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} - - {otherProps.children} - - - ); -}; +const WrappedLink = ({ tabIndex = 0, href, target, ...otherProps }) => ( + + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */} + + {otherProps.children} + + +); WrappedLink.propTypes = { onClick: PropTypes.func, From 8ec8ff89efd0c9cde2e7445311d99e1712276e5f Mon Sep 17 00:00:00 2001 From: Benjamin Goering <171782+gobengo@users.noreply.github.com> Date: Mon, 17 Oct 2022 19:01:40 +0100 Subject: [PATCH 5/5] Update packages/website/components/link/link.js Co-authored-by: Yusef Napora --- packages/website/components/link/link.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/website/components/link/link.js b/packages/website/components/link/link.js index 52aa7cbe52..eb4abae7a7 100644 --- a/packages/website/components/link/link.js +++ b/packages/website/components/link/link.js @@ -46,7 +46,6 @@ export function useIsExternalHref() { /** * A generic hyperlink component. - * * by default, links to external sites will have target=_blank if they are an external link * @param {object} props * @param {string} props.href - the href attribute for the link * @param {number} [props.tabIndex] - the tabIndex attribute for the link