From 3f5e013a38937c514201d4ef97fbdfa72f8aa04b Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Fri, 19 Jan 2024 07:32:35 -0500
Subject: [PATCH 1/3] refactor: remove now redundant `mark as read on click`
 functionality

---
 src/components/NotificationRow.test.tsx       | 50 -------------------
 src/components/NotificationRow.tsx            | 16 +-----
 .../NotificationRow.test.tsx.snap             | 33 ------------
 src/routes/Settings.test.tsx                  | 28 -----------
 src/routes/Settings.tsx                       |  6 ---
 .../__snapshots__/Settings.test.tsx.snap      | 23 ---------
 6 files changed, 1 insertion(+), 155 deletions(-)

diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
index e9bfe16a6..cbfc2bec9 100644
--- a/src/components/NotificationRow.test.tsx
+++ b/src/components/NotificationRow.test.tsx
@@ -54,56 +54,6 @@ describe('components/Notification.js', () => {
     expect(shell.openExternal).toHaveBeenCalledTimes(1);
   });
 
-  it('should open a notification in browser & mark it as read', () => {
-    const markNotification = jest.fn();
-
-    const props = {
-      notification: mockedSingleNotification,
-      hostname: 'github.com',
-    };
-
-    const { getByRole } = render(
-      <AppContext.Provider
-        value={{
-          settings: { ...mockSettings, markOnClick: true },
-          markNotification,
-          accounts: mockAccounts,
-        }}
-      >
-        <NotificationRow {...props} />
-      </AppContext.Provider>,
-    );
-
-    fireEvent.click(getByRole('main'));
-    expect(shell.openExternal).toHaveBeenCalledTimes(1);
-    expect(markNotification).toHaveBeenCalledTimes(1);
-  });
-
-  it('should mark a notification as read', () => {
-    const markNotification = jest.fn();
-
-    const props = {
-      notification: mockedSingleNotification,
-      hostname: 'github.com',
-    };
-
-    const { getByTitle } = render(
-      <AppContext.Provider
-        value={{
-          settings: { ...mockSettings, markOnClick: false },
-          accounts: mockAccounts,
-        }}
-      >
-        <AppContext.Provider value={{ markNotification }}>
-          <NotificationRow {...props} />
-        </AppContext.Provider>
-      </AppContext.Provider>,
-    );
-
-    fireEvent.click(getByTitle('Mark as Read'));
-    expect(markNotification).toHaveBeenCalledTimes(1);
-  });
-
   it('should unsubscribe from a notification thread', () => {
     const unsubscribeNotification = jest.fn();
 
diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx
index 07f533f81..fb45486ba 100644
--- a/src/components/NotificationRow.tsx
+++ b/src/components/NotificationRow.tsx
@@ -1,6 +1,6 @@
 import React, { useCallback, useContext } from 'react';
 import { formatDistanceToNow, parseISO } from 'date-fns';
-import { CheckIcon, MuteIcon } from '@primer/octicons-react';
+import { MuteIcon } from '@primer/octicons-react';
 
 import {
   formatReason,
@@ -84,20 +84,6 @@ export const NotificationRow: React.FC<IProps> = ({
           </button>
         </div>
       </div>
-
-      <div className="flex justify-center items-center w-8">
-        <button
-          className="focus:outline-none"
-          title="Mark as Read"
-          onClick={() => markNotification(notification.id, hostname)}
-        >
-          <CheckIcon
-            className="hover:text-green-500"
-            size={20}
-            aria-label="Mark as Read"
-          />
-        </button>
-      </div>
     </div>
   );
 };
diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap
index 8a922c507..3f3c14be1 100644
--- a/src/components/__snapshots__/NotificationRow.test.tsx.snap
+++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap
@@ -86,38 +86,5 @@ exports[`components/Notification.js should render itself & its children 1`] = `
       </button>
     </div>
   </div>
-  <div
-    className="flex justify-center items-center w-8"
-  >
-    <button
-      className="focus:outline-none"
-      onClick={[Function]}
-      title="Mark as Read"
-    >
-      <svg
-        aria-hidden="false"
-        aria-label="Mark as Read"
-        className="hover:text-green-500"
-        fill="currentColor"
-        focusable="false"
-        height={20}
-        role="img"
-        style={
-          {
-            "display": "inline-block",
-            "overflow": "visible",
-            "userSelect": "none",
-            "verticalAlign": "text-bottom",
-          }
-        }
-        viewBox="0 0 16 16"
-        width={20}
-      >
-        <path
-          d="M13.78 4.22a.75.75 0 0 1 0 1.06l-7.25 7.25a.75.75 0 0 1-1.06 0L2.22 9.28a.751.751 0 0 1 .018-1.042.751.751 0 0 1 1.042-.018L6 10.94l6.72-6.72a.75.75 0 0 1 1.06 0Z"
-        />
-      </svg>
-    </button>
-  </div>
 </div>
 `;
diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx
index 46d31292a..d82baf908 100644
--- a/src/routes/Settings.test.tsx
+++ b/src/routes/Settings.test.tsx
@@ -182,34 +182,6 @@ describe('routes/Settings.tsx', () => {
     expect(updateSetting).toHaveBeenCalledWith('showNotifications', false);
   });
 
-  it('should toggle the onClickMarkAsRead checkbox', async () => {
-    let getByLabelText;
-
-    await act(async () => {
-      const { getByLabelText: getByLabelTextLocal } = render(
-        <AppContext.Provider
-          value={{
-            settings: mockSettings,
-            accounts: mockAccounts,
-            updateSetting,
-          }}
-        >
-          <MemoryRouter>
-            <SettingsRoute />
-          </MemoryRouter>
-        </AppContext.Provider>,
-      );
-      getByLabelText = getByLabelTextLocal;
-    });
-
-    fireEvent.click(getByLabelText('Mark as read on click'), {
-      target: { checked: true },
-    });
-
-    expect(updateSetting).toHaveBeenCalledTimes(1);
-    expect(updateSetting).toHaveBeenCalledWith('markOnClick', false);
-  });
-
   it('should toggle the openAtStartup checkbox', async () => {
     let getByLabelText;
 
diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx
index 1e38762a0..433c0e4e4 100644
--- a/src/routes/Settings.tsx
+++ b/src/routes/Settings.tsx
@@ -139,12 +139,6 @@ export const SettingsRoute: React.FC = () => {
             updateSetting('showNotifications', evt.target.checked)
           }
         />
-        <FieldCheckbox
-          name="onClickMarkAsRead"
-          label="Mark as read on click"
-          checked={settings.markOnClick}
-          onChange={(evt) => updateSetting('markOnClick', evt.target.checked)}
-        />
         {!isLinux && (
           <FieldCheckbox
             name="openAtStartUp"
diff --git a/src/routes/__snapshots__/Settings.test.tsx.snap b/src/routes/__snapshots__/Settings.test.tsx.snap
index fe010b535..a11bcec66 100644
--- a/src/routes/__snapshots__/Settings.test.tsx.snap
+++ b/src/routes/__snapshots__/Settings.test.tsx.snap
@@ -230,29 +230,6 @@ exports[`routes/Settings.tsx should render itself & its children 1`] = `
         </label>
       </div>
     </div>
-    <div
-      class="flex items-start mt-1 mb-3"
-    >
-      <div
-        class="flex items-center h-5"
-      >
-        <input
-          class="focus:ring-indigo-500 h-4 w-4 text-indigo-600 border-gray-300 rounded"
-          id="onClickMarkAsRead"
-          type="checkbox"
-        />
-      </div>
-      <div
-        class="ml-3 text-sm"
-      >
-        <label
-          class="font-medium text-gray-700 dark:text-gray-200"
-          for="onClickMarkAsRead"
-        >
-          Mark as read on click
-        </label>
-      </div>
-    </div>
     <div
       class="flex items-start mt-1 mb-3"
     >

From 77d74e5d0d1c122c31ae7bc92be04e069102ba9d Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Fri, 19 Jan 2024 08:01:40 -0500
Subject: [PATCH 2/3] refactor: remove now redundant `mark as read on click`
 functionality

---
 src/__mocks__/mock-state.ts                   |  1 -
 src/components/NotificationRow.test.tsx       | 27 ++++++++++++++-
 src/components/NotificationRow.tsx            | 20 ++++++++---
 .../NotificationRow.test.tsx.snap             | 33 +++++++++++++++++++
 src/context/App.test.tsx                      |  2 --
 src/context/App.tsx                           |  1 -
 src/types.ts                                  |  1 -
 7 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts
index 91043e1a9..accdabd5b 100644
--- a/src/__mocks__/mock-state.ts
+++ b/src/__mocks__/mock-state.ts
@@ -16,7 +16,6 @@ export const mockSettings: SettingsState = {
   participating: false,
   playSound: true,
   showNotifications: true,
-  markOnClick: false,
   openAtStartup: false,
   appearance: Appearance.SYSTEM,
   colors: false,
diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
index cbfc2bec9..a292ff629 100644
--- a/src/components/NotificationRow.test.tsx
+++ b/src/components/NotificationRow.test.tsx
@@ -41,7 +41,7 @@ describe('components/Notification.js', () => {
     const { getByRole } = render(
       <AppContext.Provider
         value={{
-          settings: { ...mockSettings, markOnClick: true },
+          settings: { ...mockSettings },
           markNotification,
           accounts: mockAccounts,
         }}
@@ -54,6 +54,31 @@ describe('components/Notification.js', () => {
     expect(shell.openExternal).toHaveBeenCalledTimes(1);
   });
 
+  it('should mark a notification as read', () => {
+    const markNotification = jest.fn();
+
+    const props = {
+      notification: mockedSingleNotification,
+      hostname: 'github.com',
+    };
+
+    const { getByTitle } = render(
+      <AppContext.Provider
+        value={{
+          settings: { ...mockSettings },
+          accounts: mockAccounts,
+        }}
+      >
+        <AppContext.Provider value={{ markNotification }}>
+          <NotificationRow {...props} />
+        </AppContext.Provider>
+      </AppContext.Provider>,
+    );
+
+    fireEvent.click(getByTitle('Mark as Read'));
+    expect(markNotification).toHaveBeenCalledTimes(1);
+  });
+
   it('should unsubscribe from a notification thread', () => {
     const unsubscribeNotification = jest.fn();
 
diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx
index fb45486ba..612a3bbb6 100644
--- a/src/components/NotificationRow.tsx
+++ b/src/components/NotificationRow.tsx
@@ -1,6 +1,6 @@
 import React, { useCallback, useContext } from 'react';
 import { formatDistanceToNow, parseISO } from 'date-fns';
-import { MuteIcon } from '@primer/octicons-react';
+import { CheckIcon, MuteIcon } from '@primer/octicons-react';
 
 import {
   formatReason,
@@ -25,10 +25,6 @@ export const NotificationRow: React.FC<IProps> = ({
 
   const pressTitle = useCallback(() => {
     openBrowser();
-
-    if (settings.markOnClick) {
-      markNotification(notification.id, hostname);
-    }
   }, [settings]);
 
   const openBrowser = useCallback(
@@ -84,6 +80,20 @@ export const NotificationRow: React.FC<IProps> = ({
           </button>
         </div>
       </div>
+
+      <div className="flex justify-center items-center w-8">
+        <button
+          className="focus:outline-none"
+          title="Mark as Read"
+          onClick={() => markNotification(notification.id, hostname)}
+        >
+          <CheckIcon
+            className="hover:text-green-500"
+            size={20}
+            aria-label="Mark as Read"
+          />
+        </button>
+      </div>
     </div>
   );
 };
diff --git a/src/components/__snapshots__/NotificationRow.test.tsx.snap b/src/components/__snapshots__/NotificationRow.test.tsx.snap
index 3f3c14be1..8a922c507 100644
--- a/src/components/__snapshots__/NotificationRow.test.tsx.snap
+++ b/src/components/__snapshots__/NotificationRow.test.tsx.snap
@@ -86,5 +86,38 @@ exports[`components/Notification.js should render itself & its children 1`] = `
       </button>
     </div>
   </div>
+  <div
+    className="flex justify-center items-center w-8"
+  >
+    <button
+      className="focus:outline-none"
+      onClick={[Function]}
+      title="Mark as Read"
+    >
+      <svg
+        aria-hidden="false"
+        aria-label="Mark as Read"
+        className="hover:text-green-500"
+        fill="currentColor"
+        focusable="false"
+        height={20}
+        role="img"
+        style={
+          {
+            "display": "inline-block",
+            "overflow": "visible",
+            "userSelect": "none",
+            "verticalAlign": "text-bottom",
+          }
+        }
+        viewBox="0 0 16 16"
+        width={20}
+      >
+        <path
+          d="M13.78 4.22a.75.75 0 0 1 0 1.06l-7.25 7.25a.75.75 0 0 1-1.06 0L2.22 9.28a.751.751 0 0 1 .018-1.042.751.751 0 0 1 1.042-.018L6 10.94l6.72-6.72a.75.75 0 0 1 1.06 0Z"
+        />
+      </svg>
+    </button>
+  </div>
 </div>
 `;
diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx
index 05e00520c..d0b45e637 100644
--- a/src/context/App.test.tsx
+++ b/src/context/App.test.tsx
@@ -259,7 +259,6 @@ describe('context/App.tsx', () => {
       { enterpriseAccounts: [], token: null, user: null },
       {
         appearance: 'SYSTEM',
-        markOnClick: false,
         openAtStartup: false,
         participating: true,
         playSound: true,
@@ -297,7 +296,6 @@ describe('context/App.tsx', () => {
       { enterpriseAccounts: [], token: null, user: null },
       {
         appearance: 'SYSTEM',
-        markOnClick: false,
         openAtStartup: true,
         participating: false,
         playSound: true,
diff --git a/src/context/App.tsx b/src/context/App.tsx
index 34543cf38..60197bca5 100644
--- a/src/context/App.tsx
+++ b/src/context/App.tsx
@@ -34,7 +34,6 @@ export const defaultSettings: SettingsState = {
   participating: false,
   playSound: true,
   showNotifications: true,
-  markOnClick: false,
   openAtStartup: false,
   appearance: Appearance.SYSTEM,
   colors: false,
diff --git a/src/types.ts b/src/types.ts
index d3e881b9e..8aac5134c 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -10,7 +10,6 @@ export interface SettingsState {
   participating: boolean;
   playSound: boolean;
   showNotifications: boolean;
-  markOnClick: boolean;
   openAtStartup: boolean;
   appearance: Appearance;
   colors: boolean;

From f0a2b0fe521c4b6fa1eedbc7245567412fa62890 Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Fri, 19 Jan 2024 08:05:22 -0500
Subject: [PATCH 3/3] refactor: remove now redundant `mark as read on click`
 functionality

---
 src/components/NotificationRow.test.tsx | 4 ++--
 src/components/Sidebar.test.tsx         | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx
index a292ff629..cb66ae0ca 100644
--- a/src/components/NotificationRow.test.tsx
+++ b/src/components/NotificationRow.test.tsx
@@ -41,7 +41,7 @@ describe('components/Notification.js', () => {
     const { getByRole } = render(
       <AppContext.Provider
         value={{
-          settings: { ...mockSettings },
+          settings: mockSettings,
           markNotification,
           accounts: mockAccounts,
         }}
@@ -65,7 +65,7 @@ describe('components/Notification.js', () => {
     const { getByTitle } = render(
       <AppContext.Provider
         value={{
-          settings: { ...mockSettings },
+          settings: mockSettings,
           accounts: mockAccounts,
         }}
       >
diff --git a/src/components/Sidebar.test.tsx b/src/components/Sidebar.test.tsx
index 94c5bf878..6582f917c 100644
--- a/src/components/Sidebar.test.tsx
+++ b/src/components/Sidebar.test.tsx
@@ -35,7 +35,7 @@ describe('components/Sidebar.tsx', () => {
     const tree = TestRenderer.create(
       <AppContext.Provider
         value={{
-          settings: { ...mockSettings },
+          settings: mockSettings,
           notifications: mockedAccountNotifications,
         }}
       >