From e21dabab52f8c5c66a9c263123ebb4ed4178d8f0 Mon Sep 17 00:00:00 2001
From: Franciscus Agnew <franciscusagnew@users.noreply.github.com>
Date: Fri, 31 Jan 2025 09:33:11 -0800
Subject: [PATCH] Navlink merch store target fix (#1848)

---
 common/constants/navigation.js                |  1 +
 components/Nav/Nav.tsx                        | 17 +++++--
 components/Nav/NavListItem/NavListItem.tsx    | 47 +++++++++++++------
 .../__snapshots__/NavListItem.test.tsx.snap   |  2 +-
 .../__tests__/__snapshots__/Nav.test.tsx.snap |  8 ++--
 5 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/common/constants/navigation.js b/common/constants/navigation.js
index 77d0d5f1a..04a8d46bd 100644
--- a/common/constants/navigation.js
+++ b/common/constants/navigation.js
@@ -53,6 +53,7 @@ const getInvolved = {
 const merchStore = {
   name: 'Merch Store',
   href: '/swag',
+  isExternal: true,
 };
 
 const jobs = {
diff --git a/components/Nav/Nav.tsx b/components/Nav/Nav.tsx
index 11ebf6bc6..6cab26bf9 100644
--- a/components/Nav/Nav.tsx
+++ b/components/Nav/Nav.tsx
@@ -7,6 +7,7 @@ import Logo from 'public/static/images/logo.svg';
 import { desktopNavItems, mobileNavItems } from 'common/constants/navigation';
 import NavMobile from 'components/Nav/NavMobile/NavMobile';
 import dynamic from 'next/dynamic';
+import { twMerge } from 'tailwind-merge';
 import UserLogo from '../../public/static/images/icons/FontAwesome/user.svg';
 import styles from './Nav.module.css';
 
@@ -48,7 +49,10 @@ export const Nav = () => {
           <nav data-testid="Desktop Nav">
             <Link href="/" key="Home">
               <a
-                className={classNames(styles.logoLink, styles.link)}
+                className={classNames(
+                  styles.logoLink,
+                  twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3'),
+                )}
                 onContextMenu={event => {
                   event.preventDefault();
                   Router.push('/branding');
@@ -58,7 +62,7 @@ export const Nav = () => {
               </a>
             </Link>
 
-            <ul className={styles.link}>
+            <ul className={twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3')}>
               {desktopNavItems.map(navItem => (
                 <NavListItem
                   key={navItem.name}
@@ -74,7 +78,14 @@ export const Nav = () => {
               {/* stylistic one-off */}
               <li key="Donate">
                 <Link href="/donate">
-                  <a className={classNames(styles.link, styles.donateLink)}>Donate</a>
+                  <a
+                    className={classNames(
+                      twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3'),
+                      styles.donateLink,
+                    )}
+                  >
+                    Donate
+                  </a>
                 </Link>
               </li>
             </ul>
diff --git a/components/Nav/NavListItem/NavListItem.tsx b/components/Nav/NavListItem/NavListItem.tsx
index 58eee068c..6d5949159 100644
--- a/components/Nav/NavListItem/NavListItem.tsx
+++ b/components/Nav/NavListItem/NavListItem.tsx
@@ -3,14 +3,17 @@ import classNames from 'classnames';
 import Link from 'next/link';
 import PlusIcon from 'static/images/icons/plus.svg';
 import MinusIcon from 'static/images/icons/minus.svg';
+import OutboundLink from 'components/OutboundLink/OutboundLink';
+import { twMerge } from 'tailwind-merge';
 import styles from './NavListItem.module.css';
 
-type SublinkType = {
+interface SublinkType {
   name: string;
   href: string;
-};
+  isExternal?: boolean;
+}
 
-export type NavListItemPropsType = {
+export interface NavListItemPropsType {
   /**
    * Text used for the label.
    */
@@ -27,7 +30,7 @@ export type NavListItemPropsType = {
    * Includes an optional icon.
    */
   icon?: React.ReactElement | null;
-};
+}
 
 function NavListItem({ sublinks, href, name, icon = null }: NavListItemPropsType) {
   const [areSublinksVisible, setSublinksVisible] = useState(false);
@@ -58,7 +61,10 @@ function NavListItem({ sublinks, href, name, icon = null }: NavListItemPropsType
     <li className={styles.NavListItem}>
       <Link href={href}>
         <a
-          className={classNames(styles.link, styles.navItemLink)}
+          className={classNames(
+            twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3'),
+            styles.navItemLink,
+          )}
           onMouseEnter={exposeSublinks}
           onMouseLeave={hideSublinks}
           role="link"
@@ -99,18 +105,29 @@ function NavListItem({ sublinks, href, name, icon = null }: NavListItemPropsType
             {sublinks.map((sublink, index) => (
               <li className={styles.sublinkListItem} key={sublink.name}>
                 {/* 😞 next/link fought being mocked, so `prefetch` has test-specific code */}
-                <Link href={sublink.href} prefetch={process.env.NODE_ENV === 'production'}>
-                  <a
-                    className={styles.link}
-                    key={sublink.name}
-                    role="link"
-                    tabIndex={0}
+                {!sublink.isExternal ? (
+                  <Link href={sublink.href} prefetch={process.env.NODE_ENV === 'production'}>
+                    <a
+                      className={twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3')}
+                      role="link"
+                      tabIndex={0}
+                      data-testid={`Nav Item ${sublink.name}`}
+                      onKeyDown={event => handleKeyDown(event, index)}
+                    >
+                      <span className={styles.linkContent}>{sublink.name}</span>
+                    </a>
+                  </Link>
+                ) : (
+                  <OutboundLink
+                    analyticsEventLabel={`Clicked on ${sublink.name} -> ${sublink.href}`}
+                    className={twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3')}
                     data-testid={`Nav Item ${sublink.name}`}
-                    onKeyDown={event => handleKeyDown(event, index)}
+                    href={sublink.href}
+                    hasIcon
                   >
-                    <span className={styles.linkContent}>{sublink.name}</span>
-                  </a>
-                </Link>
+                    <span className={twMerge(styles.link, '[&>svg]:-bottom-2 [&>svg]:right-3')}>{sublink.name}</span>
+                  </OutboundLink>
+                )}
               </li>
             ))}
           </ul>
diff --git a/components/Nav/NavListItem/__tests__/__snapshots__/NavListItem.test.tsx.snap b/components/Nav/NavListItem/__tests__/__snapshots__/NavListItem.test.tsx.snap
index 9f6c64f8f..2e5075f45 100644
--- a/components/Nav/NavListItem/__tests__/__snapshots__/NavListItem.test.tsx.snap
+++ b/components/Nav/NavListItem/__tests__/__snapshots__/NavListItem.test.tsx.snap
@@ -8,7 +8,7 @@ exports[`NavListItem > should render with required props passed 1`] = `
     href="/test"
   >
     <a
-      className="link navItemLink"
+      className="link [&>svg]:-bottom-2 [&>svg]:right-3 navItemLink"
       data-testid="Nav Item Test"
       onMouseEnter={[Function]}
       onMouseLeave={[Function]}
diff --git a/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snap b/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snap
index 937ea1a55..e855dfa03 100644
--- a/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snap
+++ b/components/Nav/__tests__/__snapshots__/Nav.test.tsx.snap
@@ -41,6 +41,7 @@ exports[`Nav > should render with no props passed 1`] = `
         },
         {
           "href": "/swag",
+          "isExternal": true,
           "name": "Merch Store",
         },
         {
@@ -69,7 +70,7 @@ exports[`Nav > should render with no props passed 1`] = `
           href="/"
         >
           <a
-            className="logoLink link"
+            className="logoLink link [&>svg]:-bottom-2 [&>svg]:right-3"
             onContextMenu={[Function]}
           >
             <ForwardRef
@@ -85,7 +86,7 @@ exports[`Nav > should render with no props passed 1`] = `
           </a>
         </ForwardRef(LinkComponent)>
         <ul
-          className="link"
+          className="link [&>svg]:-bottom-2 [&>svg]:right-3"
         >
           <ForwardRef(LoadableComponent)
             href="/about"
@@ -153,6 +154,7 @@ exports[`Nav > should render with no props passed 1`] = `
                 },
                 {
                   "href": "/swag",
+                  "isExternal": true,
                   "name": "Merch Store",
                 },
                 {
@@ -171,7 +173,7 @@ exports[`Nav > should render with no props passed 1`] = `
               href="/donate"
             >
               <a
-                className="link donateLink"
+                className="link [&>svg]:-bottom-2 [&>svg]:right-3 donateLink"
               >
                 Donate
               </a>