refactor: consolidate open links fns (#1211)

* refactor: consolidate open links fns * Update src/utils/links.test.ts Co-authored-by: Afonso Jorge Ramos <[email protected]> --------- Co-authored-by: Afonso Jorge Ramos <[email protected]>
gitify-app · Jun 10, 2024 · 7c62dd8 · 7c62dd8
1 parent fb99d49
commit 7c62dd8
Show file tree

Hide file tree

Showing 14 changed files with 227 additions and 116 deletions.
diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
@@ -4,12 +4,12 @@ import { AppContext } from '../context/App';
 import type { Milestone, UserType } from '../typesGitHub';
 import { mockSingleNotification } from '../utils/api/__mocks__/response-mocks';
 import * as comms from '../utils/comms';
-import * as helpers from '../utils/helpers';
+import * as links from '../utils/links';
 import { NotificationRow } from './NotificationRow';
 
 describe('components/NotificationRow.tsx', () => {
   beforeEach(() => {
-    jest.spyOn(helpers, 'openInBrowser');
+    jest.spyOn(links, 'openNotification');
   });
 
   afterEach(() => {
@@ -341,7 +341,7 @@ describe('components/NotificationRow.tsx', () => {
       );
 
       fireEvent.click(screen.getByRole('main'));
-      expect(helpers.openInBrowser).toHaveBeenCalledTimes(1);
+      expect(links.openNotification).toHaveBeenCalledTimes(1);
       expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
     });
 
@@ -366,7 +366,7 @@ describe('components/NotificationRow.tsx', () => {
       );
 
       fireEvent.keyDown(screen.getByRole('main'));
-      expect(helpers.openInBrowser).toHaveBeenCalledTimes(1);
+      expect(links.openNotification).toHaveBeenCalledTimes(1);
       expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
     });
 
@@ -391,7 +391,7 @@ describe('components/NotificationRow.tsx', () => {
       );
 
       fireEvent.click(screen.getByRole('main'));
-      expect(helpers.openInBrowser).toHaveBeenCalledTimes(1);
+      expect(links.openNotification).toHaveBeenCalledTimes(1);
       expect(markNotificationDone).toHaveBeenCalledTimes(1);
     });
 

diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx
@@ -8,29 +8,22 @@ import {
   ReadIcon,
   TagIcon,
 } from '@primer/octicons-react';
-import {
-  type FC,
-  type KeyboardEvent,
-  type MouseEvent,
-  useCallback,
-  useContext,
-} from 'react';
+import { type FC, type MouseEvent, useCallback, useContext } from 'react';
 
 import { AppContext } from '../context/App';
 import { PILL_CLASS_NAME } from '../styles/gitify';
 import { IconColor } from '../types';
 import type { Notification } from '../typesGitHub';
-import { openExternalLink } from '../utils/comms';
 import {
   formatForDisplay,
   formatNotificationUpdatedAt,
-  openInBrowser,
 } from '../utils/helpers';
 import {
   getNotificationTypeIcon,
   getNotificationTypeIconColor,
   getPullRequestReviewIcon,
 } from '../utils/icons';
+import { openNotification, openUserProfile } from '../utils/links';
 import { formatReason } from '../utils/reason';
 
 interface IProps {
@@ -49,8 +42,8 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
     notifications,
   } = useContext(AppContext);
 
-  const openNotification = useCallback(() => {
-    openInBrowser(notification);
+  const handleNotification = useCallback(() => {
+    openNotification(notification);
 
     if (settings.markAsDoneOnOpen) {
       markNotificationDone(notification.id, hostname);
@@ -67,15 +60,6 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
     unsubscribeNotification(notification.id, hostname);
   };
 
-  const openUserProfile = (
-    event: MouseEvent<HTMLElement> | KeyboardEvent<HTMLElement>,
-  ) => {
-    // Don't trigger onClick of parent element.
-    event.stopPropagation();
-
-    openExternalLink(notification.subject.user.html_url);
-  };
-
   const reason = formatReason(notification.reason);
   const NotificationIcon = getNotificationTypeIcon(notification.subject);
   const iconColor = getNotificationTypeIconColor(notification.subject);
@@ -116,8 +100,8 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
 
       <div
         className="flex-1 whitespace-nowrap overflow-hidden overflow-ellipsis"
-        onClick={() => openNotification()}
-        onKeyDown={() => openNotification()}
+        onClick={() => handleNotification()}
+        onKeyDown={() => handleNotification()}
       >
         <div
           className="mb-1 text-sm truncate cursor-pointer"
@@ -129,10 +113,14 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
 
         <div className="flex flex-wrap items-center text-xs text-capitalize gap-1">
           {notification.subject.user ? (
-            <div
+            <button
+              type="button"
               title="View User Profile"
-              onClick={openUserProfile}
-              onKeyDown={openUserProfile}
+              onClick={(event: MouseEvent<HTMLElement>) => {
+                // Don't trigger onClick of parent element.
+                event.stopPropagation();
+                openUserProfile(notification.subject.user);
+              }}
               className="flex-shrink-0"
             >
               <img
@@ -141,7 +129,7 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
                 title={notification.subject.user.login}
                 alt={`${notification.subject.user.login}'s avatar`}
               />
-            </div>
+            </button>
           ) : (
             <div>
               <FeedPersonIcon

diff --git a/src/components/Repository.tsx b/src/components/Repository.tsx
@@ -3,7 +3,7 @@ import { type FC, useCallback, useContext } from 'react';
 import { CSSTransition, TransitionGroup } from 'react-transition-group';
 import { AppContext } from '../context/App';
 import type { Notification } from '../typesGitHub';
-import { openExternalLink } from '../utils/comms';
+import { openRepository } from '../utils/links';
 import { NotificationRow } from './NotificationRow';
 
 interface IProps {
@@ -20,11 +20,6 @@ export const RepositoryNotifications: FC<IProps> = ({
   const { markRepoNotificationsRead, markRepoNotificationsDone } =
     useContext(AppContext);
 
-  const openBrowser = useCallback(() => {
-    const url = repoNotifications[0].repository.html_url;
-    openExternalLink(url);
-  }, [repoNotifications]);
-
   const markRepoAsRead = useCallback(() => {
     const repoSlug = repoNotifications[0].repository.full_name;
     markRepoNotificationsRead(repoSlug, hostname);
@@ -53,8 +48,8 @@ export const RepositoryNotifications: FC<IProps> = ({
           )}
           <span
             className="cursor-pointer truncate"
-            onClick={openBrowser}
-            onKeyDown={openBrowser}
+            onClick={() => openRepository(repoNotifications[0].repository)}
+            onKeyDown={() => openRepository(repoNotifications[0].repository)}
           >
             {repoName}
           </span>

diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx
@@ -4,13 +4,13 @@ import {
   SyncIcon,
   XCircleIcon,
 } from '@primer/octicons-react';
-import { type FC, useCallback, useContext, useMemo } from 'react';
+import { type FC, useContext, useMemo } from 'react';
 import { useLocation, useNavigate } from 'react-router-dom';
 import { Logo } from '../components/Logo';
 import { AppContext } from '../context/App';
 import { BUTTON_SIDEBAR_CLASS_NAME } from '../styles/gitify';
-import { openExternalLink, quitApp } from '../utils/comms';
-import { Constants } from '../utils/constants';
+import { quitApp } from '../utils/comms';
+import { openGitHubNotifications, openGitifyRepository } from '../utils/links';
 import { getNotificationCount } from '../utils/notifications';
 
 export const Sidebar: FC = () => {
@@ -20,14 +20,6 @@ export const Sidebar: FC = () => {
   const { notifications, fetchNotifications, isLoggedIn, status } =
     useContext(AppContext);
 
-  const onOpenBrowser = useCallback(() => {
-    openExternalLink(`https://github.com/${Constants.REPO_SLUG}`);
-  }, []);
-
-  const onOpenGitHubNotifications = useCallback(() => {
-    openExternalLink('https://github.com/notifications');
-  }, []);
-
   const toggleSettings = () => {
     if (location.pathname.startsWith('/settings')) {
       navigate('/', { replace: true });
@@ -47,7 +39,7 @@ export const Sidebar: FC = () => {
           type="button"
           className="w-5 my-3 mx-auto cursor-pointer outline-none"
           title="Open Gitify on GitHub"
-          onClick={onOpenBrowser}
+          onClick={() => openGitifyRepository()}
           data-testid="gitify-logo"
         >
           <Logo aria-label="Open Gitify" />
@@ -58,7 +50,7 @@ export const Sidebar: FC = () => {
           className={`flex justify-around self-stretch items-center my-1 py-1 px-2 text-xs font-extrabold cursor-pointer ${
             notificationsCount > 0 ? 'text-green-500' : 'text-white'
           }`}
-          onClick={onOpenGitHubNotifications}
+          onClick={() => openGitHubNotifications()}
           title={`${notificationsCount} Unread Notifications`}
         >
           <BellIcon

diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap
diff --git a/src/routes/Accounts.tsx b/src/routes/Accounts.tsx
@@ -15,16 +15,17 @@ import { AuthMethodIcon } from '../components/icons/AuthMethodIcon';
 import { PlatformIcon } from '../components/icons/PlatformIcon';
 import { BUTTON_CLASS_NAME } from '../styles/gitify';
 import type { Account } from '../types';
-import { getAccountUUID, getDeveloperSettingsURL } from '../utils/auth/utils';
-import {
-  openExternalLink,
-  updateTrayIcon,
-  updateTrayTitle,
-} from '../utils/comms';
+import { getAccountUUID } from '../utils/auth/utils';
+import { updateTrayIcon, updateTrayTitle } from '../utils/comms';
 import {
   isOAuthAppLoggedIn,
   isPersonalAccessTokenLoggedIn,
 } from '../utils/helpers';
+import {
+  openAccountProfile,
+  openDeveloperSettings,
+  openHost,
+} from '../utils/links';
 
 export const AccountsRoute: FC = () => {
   const { auth, logoutFromAccount } = useContext(AppContext);
@@ -37,21 +38,6 @@ export const AccountsRoute: FC = () => {
     updateTrayTitle();
   }, []);
 
-  const openProfile = (account: Account) => {
-    const url = new URL(`https://${account.hostname}`);
-    url.pathname = account.user.login;
-    openExternalLink(url.toString());
-  };
-
-  const openHost = (hostname: string) => {
-    openExternalLink(`https://${hostname}`);
-  };
-
-  const openDeveloperSettings = (account: Account) => {
-    const url = getDeveloperSettingsURL(account);
-    openExternalLink(url);
-  };
-
   const loginWithPersonalAccessToken = useCallback(() => {
     return navigate('/login-personal-access-token', { replace: true });
   }, []);
@@ -95,7 +81,7 @@ export const AccountsRoute: FC = () => {
                     type="button"
                     className="cursor-pointer font-semibold mb-1 text-sm"
                     title="Open Profile"
-                    onClick={() => openProfile(account)}
+                    onClick={() => openAccountProfile(account)}
                   >
                     @{account.user.login}
                     <span