feat: mark notification as read to allow transition effects time (#1389)

* feat: mark notification as read to allow transition effects time

* feat: remove unused removeNotificationFromState

Author: setchy

src/components/NotificationRow.test.tsx

@@ -82,7 +82,7 @@ describe('components/NotificationRow.tsx', () => {
 
   describe('notification interactions', () => {
     it('should open a notification in the browser - click', () => {
-      const removeNotificationFromState = jest.fn();
+      const markNotificationRead = jest.fn();
 
       const props = {
         notification: mockSingleNotification,
@@ -93,7 +93,7 @@ describe('components/NotificationRow.tsx', () => {
         <AppContext.Provider
           value={{
             settings: { ...mockSettings, markAsDoneOnOpen: false },
-            removeNotificationFromState,
+            markNotificationRead,
             auth: mockAuth,
           }}
         >
@@ -103,11 +103,11 @@ describe('components/NotificationRow.tsx', () => {
 
       fireEvent.click(screen.getByRole('main'));
       expect(links.openNotification).toHaveBeenCalledTimes(1);
-      expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
+      expect(markNotificationRead).toHaveBeenCalledTimes(1);
     });
 
     it('should open a notification in the browser - delay notification setting enabled', () => {
-      const removeNotificationFromState = jest.fn();
+      const markNotificationRead = jest.fn();
 
       const props = {
         notification: mockSingleNotification,
@@ -122,7 +122,7 @@ describe('components/NotificationRow.tsx', () => {
               markAsDoneOnOpen: false,
               delayNotificationState: true,
             },
-            removeNotificationFromState,
+            markNotificationRead,
             auth: mockAuth,
           }}
         >
@@ -132,11 +132,11 @@ describe('components/NotificationRow.tsx', () => {
 
       fireEvent.click(screen.getByRole('main'));
       expect(links.openNotification).toHaveBeenCalledTimes(1);
-      expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
+      expect(markNotificationRead).toHaveBeenCalledTimes(1);
     });
 
     it('should open a notification in the browser - key down', () => {
-      const removeNotificationFromState = jest.fn();
+      const markNotificationRead = jest.fn();
 
       const props = {
         notification: mockSingleNotification,
@@ -147,7 +147,7 @@ describe('components/NotificationRow.tsx', () => {
         <AppContext.Provider
           value={{
             settings: { ...mockSettings, markAsDoneOnOpen: false },
-            removeNotificationFromState,
+            markNotificationRead,
             auth: mockAuth,
           }}
         >
@@ -157,7 +157,7 @@ describe('components/NotificationRow.tsx', () => {
 
       fireEvent.click(screen.getByRole('main'));
       expect(links.openNotification).toHaveBeenCalledTimes(1);
-      expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
+      expect(markNotificationRead).toHaveBeenCalledTimes(1);
     });
 
     it('should open a notification in browser & mark it as done', () => {

src/components/NotificationRow.tsx

@@ -34,7 +34,6 @@ export const NotificationRow: FC<INotificationRow> = ({
 }: INotificationRow) => {
   const {
     settings,
-    removeNotificationFromState,
     markNotificationRead,
     markNotificationDone,
     unsubscribeNotification,
@@ -51,15 +50,9 @@ export const NotificationRow: FC<INotificationRow> = ({
     if (settings.markAsDoneOnOpen) {
       markNotificationDone(notification);
     } else {
-      // no need to mark as read, github does it by default when opening it
-      removeNotificationFromState(settings, notification);
+      markNotificationRead(notification);
     }
-  }, [
-    notification,
-    markNotificationDone,
-    removeNotificationFromState,
-    settings,
-  ]);
+  }, [notification, markNotificationDone, markNotificationRead, settings]);
 
   const unsubscribeFromThread = (event: MouseEvent<HTMLElement>) => {
     // Don't trigger onClick of parent element.

src/context/App.tsx

@@ -102,10 +102,6 @@ interface AppContextState {
   notifications: AccountNotifications[];
   status: Status;
   errorDetails: GitifyError;
-  removeNotificationFromState: (
-    settings: SettingsState,
-    notification: Notification,
-  ) => void;
   fetchNotifications: () => Promise<void>;
   markNotificationRead: (notification: Notification) => Promise<void>;
   markNotificationDone: (notification: Notification) => Promise<void>;
@@ -129,7 +125,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     notifications,
     errorDetails,
     status,
-    removeNotificationFromState,
     markNotificationRead,
     markNotificationDone,
     unsubscribeNotification,
@@ -317,7 +312,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
         notifications,
         status,
         errorDetails,
-        removeNotificationFromState,
         fetchNotifications: fetchNotificationsWithAccounts,
         markNotificationRead: markNotificationReadWithAccounts,
         markNotificationDone: markNotificationDoneWithAccounts,

src/hooks/useNotifications.test.ts

@@ -297,45 +297,6 @@ describe('hooks/useNotifications.ts', () => {
     });
   });
 
-  describe('removeNotificationFromState', () => {
-    it('should remove a notification from state', async () => {
-      const notifications = [
-        { id: 1, title: 'This is a notification.' },
-        { id: 2, title: 'This is another one.' },
-      ];
-
-      nock('https://api.github.com')
-        .get('/notifications?participating=false')
-        .reply(200, notifications);
-
-      nock('https://github.gitify.io/api/v3')
-        .get('/notifications?participating=false')
-        .reply(200, notifications);
-
-      const { result } = renderHook(() => useNotifications());
-
-      act(() => {
-        result.current.fetchNotifications({
-          ...mockState,
-          settings: { ...mockSettings, detailedNotifications: false },
-        });
-      });
-
-      await waitFor(() => {
-        expect(result.current.status).toBe('success');
-      });
-
-      act(() => {
-        result.current.removeNotificationFromState(
-          mockSettings,
-          result.current.notifications[0].notifications[0],
-        );
-      });
-
-      expect(result.current.notifications[0].notifications.length).toBe(1);
-    });
-  });
-
   describe('markNotificationRead', () => {
     it('should mark a notification as read with success', async () => {
       nock('https://api.github.com/')

src/hooks/useNotifications.ts

@@ -3,7 +3,6 @@ import type {
   AccountNotifications,
   GitifyError,
   GitifyState,
-  SettingsState,
   Status,
 } from '../types';
 import type { Notification } from '../typesGitHub';
@@ -25,10 +24,6 @@ import { removeNotifications } from '../utils/remove-notifications';
 
 interface NotificationsState {
   notifications: AccountNotifications[];
-  removeNotificationFromState: (
-    settings: SettingsState,
-    notification: Notification,
-  ) => void;
   fetchNotifications: (state: GitifyState) => Promise<void>;
   markNotificationRead: (
     state: GitifyState,
@@ -226,26 +221,11 @@ export const useNotifications = (): NotificationsState => {
     [notifications, markNotificationDone],
   );
 
-  const removeNotificationFromState = useCallback(
-    (settings: SettingsState, notification: Notification) => {
-      const updatedNotifications = removeNotification(
-        settings,
-        notification,
-        notifications,
-      );
-
-      setNotifications(updatedNotifications);
-      setTrayIconColor(updatedNotifications);
-    },
-    [notifications],
-  );
-
   return {
     status,
     errorDetails,
     notifications,
 
-    removeNotificationFromState,
     fetchNotifications,
     markNotificationRead,
     markNotificationDone,