fix: regression in delay notification state behavior (#1241)

Author: setchy

src/components/NotificationRow.test.tsx

@@ -350,6 +350,35 @@ describe('components/NotificationRow.tsx', () => {
       expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
     });
 
+    it('should open a notification in the browser - delay notification setting enabled', () => {
+      const removeNotificationFromState = jest.fn();
+
+      const props = {
+        notification: mockSingleNotification,
+        account: mockGitHubCloudAccount,
+      };
+
+      render(
+        <AppContext.Provider
+          value={{
+            settings: {
+              ...mockSettings,
+              markAsDoneOnOpen: false,
+              delayNotificationState: true,
+            },
+            removeNotificationFromState,
+            auth: mockAuth,
+          }}
+        >
+          <NotificationRow {...props} />
+        </AppContext.Provider>,
+      );
+
+      fireEvent.click(screen.getByRole('main'));
+      expect(links.openNotification).toHaveBeenCalledTimes(1);
+      expect(removeNotificationFromState).toHaveBeenCalledTimes(1);
+    });
+
     it('should open a notification in the browser - key down', () => {
       const removeNotificationFromState = jest.fn();
 

src/components/NotificationRow.tsx

@@ -45,9 +45,12 @@ export const NotificationRow: FC<IProps> = ({ notification }) => {
     unsubscribeNotification,
   } = useContext(AppContext);
   const [animateExit, setAnimateExit] = useState(false);
+  const [showAsRead, setShowAsRead] = useState(false);
 
   const handleNotification = useCallback(() => {
-    setAnimateExit(true);
+    setAnimateExit(!settings.delayNotificationState);
+    setShowAsRead(settings.delayNotificationState);
+
     openNotification(notification);
 
     if (settings.markAsDoneOnOpen) {
@@ -102,6 +105,7 @@ export const NotificationRow: FC<IProps> = ({ notification }) => {
         'group flex border-b border-gray-100 bg-white px-3 py-2 hover:bg-gray-100 dark:border-gray-darker dark:bg-gray-dark dark:text-white dark:hover:bg-gray-darker',
         animateExit &&
           'translate-x-full opacity-0 transition duration-[350ms] ease-in-out',
+        showAsRead && 'opacity-50 dark:opacity-50',
       )}
     >
       <div
@@ -218,7 +222,8 @@ export const NotificationRow: FC<IProps> = ({ notification }) => {
           className="h-full hover:text-green-500 focus:outline-none"
           title="Mark as Done"
           onClick={() => {
-            setAnimateExit(true);
+            setAnimateExit(!settings.delayNotificationState);
+            setShowAsRead(settings.delayNotificationState);
             markNotificationDone(notification);
           }}
         >
@@ -239,7 +244,8 @@ export const NotificationRow: FC<IProps> = ({ notification }) => {
           className="h-full hover:text-green-500 focus:outline-none"
           title="Mark as Read"
           onClick={() => {
-            setAnimateExit(true);
+            setAnimateExit(!settings.delayNotificationState);
+            setShowAsRead(settings.delayNotificationState);
             markNotificationRead(notification);
           }}
         >

src/context/App.tsx

@@ -122,6 +122,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
     settings.participating,
     settings.showBots,
     settings.detailedNotifications,
+    settings.delayNotificationState,
     auth.accounts.length,
   ]);
 

src/styles/gitify.ts

@@ -3,5 +3,3 @@ export const BUTTON_CLASS_NAME =
 
 export const BUTTON_SIDEBAR_CLASS_NAME =
   'flex justify-evenly items-center bg-transparent border-0 w-full text-sm text-white my-1 py-2 cursor-pointer hover:text-gray-500 focus:outline-none disabled:text-gray-500 disabled:cursor-default';
-
-export const READ_NOTIFICATION_CLASS_NAME = 'opacity-50 dark:opacity-50';

src/utils/remove-notification.test.ts

@@ -1,6 +1,5 @@
 import { mockSingleAccountNotifications } from '../__mocks__/notifications-mocks';
 import { mockSettings } from '../__mocks__/state-mocks';
-import { READ_NOTIFICATION_CLASS_NAME } from '../styles/gitify';
 import { mockSingleNotification } from './api/__mocks__/response-mocks';
 import { removeNotification } from './remove-notification';
 
@@ -17,11 +16,7 @@ describe('utils/remove-notification.ts', () => {
     expect(result[0].notifications.length).toBe(0);
   });
 
-  it('should set notification as opaque if delayNotificationState enabled', () => {
-    const mockElement = document.createElement('div');
-    mockElement.id = mockSingleAccountNotifications[0].notifications[0].id;
-    jest.spyOn(document, 'getElementById').mockReturnValue(mockElement);
-
+  it('should skip notification removal if delay state enabled', () => {
     expect(mockSingleAccountNotifications[0].notifications.length).toBe(1);
 
     const result = removeNotification(
@@ -31,9 +26,5 @@ describe('utils/remove-notification.ts', () => {
     );
 
     expect(result[0].notifications.length).toBe(1);
-    expect(document.getElementById).toHaveBeenCalledWith(
-      mockSingleAccountNotifications[0].notifications[0].id,
-    );
-    expect(mockElement.className).toContain(READ_NOTIFICATION_CLASS_NAME);
   });
 });

src/utils/remove-notification.ts

@@ -1,4 +1,3 @@
-import { READ_NOTIFICATION_CLASS_NAME } from '../styles/gitify';
 import type { AccountNotifications, SettingsState } from '../types';
 import type { Notification } from '../typesGitHub';
 import { getAccountUUID } from './auth/utils';
@@ -9,8 +8,6 @@ export const removeNotification = (
   notifications: AccountNotifications[],
 ): AccountNotifications[] => {
   if (settings.delayNotificationState) {
-    const notificationRow = document.getElementById(notification.id);
-    notificationRow.className += ` ${READ_NOTIFICATION_CLASS_NAME}`;
     return notifications;
   }