From 13c57dfa024ce78ec20571dbe5827978975ecdec Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 14 Jun 2024 17:05:54 -0400 Subject: [PATCH 1/5] fix: regression in delay notification state after tailwind animations --- src/components/NotificationRow.tsx | 20 ++++---------------- src/context/App.tsx | 1 + src/styles/gitify.ts | 3 +++ src/utils/remove-notification.ts | 8 +++++++- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 975d09d94..9ced9d46f 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -8,13 +8,7 @@ import { ReadIcon, TagIcon, } from '@primer/octicons-react'; -import { - type FC, - type MouseEvent, - useCallback, - useContext, - useState, -} from 'react'; +import { type FC, type MouseEvent, useCallback, useContext } from 'react'; import { AppContext } from '../context/App'; import { IconColor } from '../types'; import type { Notification } from '../typesGitHub'; @@ -44,10 +38,8 @@ export const NotificationRow: FC = ({ notification }) => { markNotificationDone, unsubscribeNotification, } = useContext(AppContext); - const [animateExit, setAnimateExit] = useState(false); const handleNotification = useCallback(() => { - setAnimateExit(true); openNotification(notification); if (settings.markAsDoneOnOpen) { @@ -98,11 +90,9 @@ export const NotificationRow: FC = ({ notification }) => { return (
= ({ notification }) => { className="h-full hover:text-green-500 focus:outline-none" title="Mark as Done" onClick={() => { - setAnimateExit(true); markNotificationDone(notification); }} > @@ -239,7 +228,6 @@ export const NotificationRow: FC = ({ notification }) => { className="h-full hover:text-green-500 focus:outline-none" title="Mark as Read" onClick={() => { - setAnimateExit(true); markNotificationRead(notification); }} > diff --git a/src/context/App.tsx b/src/context/App.tsx index c9fa4e19f..6114d3491 100644 --- a/src/context/App.tsx +++ b/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, ]); diff --git a/src/styles/gitify.ts b/src/styles/gitify.ts index 22be10267..d305a155a 100644 --- a/src/styles/gitify.ts +++ b/src/styles/gitify.ts @@ -5,3 +5,6 @@ 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'; + +export const ANIMATE_NOTIFICATION_CLASS_NAME = + 'translate-x-full opacity-0 transition duration-[350ms] ease-in-out'; diff --git a/src/utils/remove-notification.ts b/src/utils/remove-notification.ts index aa5c3d217..38c569a5c 100644 --- a/src/utils/remove-notification.ts +++ b/src/utils/remove-notification.ts @@ -1,4 +1,7 @@ -import { READ_NOTIFICATION_CLASS_NAME } from '../styles/gitify'; +import { + ANIMATE_NOTIFICATION_CLASS_NAME, + READ_NOTIFICATION_CLASS_NAME, +} from '../styles/gitify'; import type { AccountNotifications, SettingsState } from '../types'; import type { Notification } from '../typesGitHub'; import { getAccountUUID } from './auth/utils'; @@ -14,6 +17,9 @@ export const removeNotification = ( return notifications; } + const notificationRow = document.getElementById(notification.id); + notificationRow.className += ` ${ANIMATE_NOTIFICATION_CLASS_NAME}`; + const notificationId = notification.id; const accountIndex = notifications.findIndex( From 9dd817be527c85c154c74d2777613b55a05e2f45 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Fri, 14 Jun 2024 17:06:17 -0400 Subject: [PATCH 2/5] fix: regression in delay notification state after tailwind animations --- src/hooks/useNotifications.test.ts | 5 +++++ src/utils/remove-notification.test.ts | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index c76ab45da..027bdca93 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -2,6 +2,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import axios, { AxiosError } from 'axios'; import nock from 'nock'; +import { mockSingleAccountNotifications } from '../__mocks__/notifications-mocks'; import { mockAuth, mockGitHubCloudAccount, @@ -299,6 +300,10 @@ describe('hooks/useNotifications.ts', () => { describe('removeNotificationFromState', () => { it('should remove a notification from state', async () => { + const mockElement = document.createElement('div'); + mockElement.id = mockSingleAccountNotifications[0].notifications[0].id; + jest.spyOn(document, 'getElementById').mockReturnValue(mockElement); + const notifications = [ { id: 1, title: 'This is a notification.' }, { id: 2, title: 'This is another one.' }, diff --git a/src/utils/remove-notification.test.ts b/src/utils/remove-notification.test.ts index 489fdaaa7..b328d16d4 100644 --- a/src/utils/remove-notification.test.ts +++ b/src/utils/remove-notification.test.ts @@ -6,6 +6,10 @@ import { removeNotification } from './remove-notification'; describe('utils/remove-notification.ts', () => { it('should remove a notification if it exists', () => { + const mockElement = document.createElement('div'); + mockElement.id = mockSingleAccountNotifications[0].notifications[0].id; + jest.spyOn(document, 'getElementById').mockReturnValue(mockElement); + expect(mockSingleAccountNotifications[0].notifications.length).toBe(1); const result = removeNotification( From 789e3c572d39930024ef8bcdbe25f92d2fb603a6 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 16 Jun 2024 06:40:56 -0400 Subject: [PATCH 3/5] fix: delay state --- src/components/NotificationRow.tsx | 20 ++++++++++++++++---- src/hooks/useNotifications.test.ts | 5 ----- src/styles/gitify.ts | 3 --- src/utils/remove-notification.test.ts | 4 ---- src/utils/remove-notification.ts | 8 +------- 5 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 9ced9d46f..d19177fd1 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -8,7 +8,13 @@ import { ReadIcon, TagIcon, } from '@primer/octicons-react'; -import { type FC, type MouseEvent, useCallback, useContext } from 'react'; +import { + type FC, + type MouseEvent, + useCallback, + useContext, + useState, +} from 'react'; import { AppContext } from '../context/App'; import { IconColor } from '../types'; import type { Notification } from '../typesGitHub'; @@ -38,8 +44,10 @@ export const NotificationRow: FC = ({ notification }) => { markNotificationDone, unsubscribeNotification, } = useContext(AppContext); + const [animateExit, setAnimateExit] = useState(false); const handleNotification = useCallback(() => { + setAnimateExit(!settings.delayNotificationState); openNotification(notification); if (settings.markAsDoneOnOpen) { @@ -90,9 +98,11 @@ export const NotificationRow: FC = ({ notification }) => { return (
= ({ notification }) => { className="h-full hover:text-green-500 focus:outline-none" title="Mark as Done" onClick={() => { + setAnimateExit(!settings.delayNotificationState); markNotificationDone(notification); }} > @@ -228,6 +239,7 @@ export const NotificationRow: FC = ({ notification }) => { className="h-full hover:text-green-500 focus:outline-none" title="Mark as Read" onClick={() => { + setAnimateExit(!settings.delayNotificationState); markNotificationRead(notification); }} > diff --git a/src/hooks/useNotifications.test.ts b/src/hooks/useNotifications.test.ts index 027bdca93..c76ab45da 100644 --- a/src/hooks/useNotifications.test.ts +++ b/src/hooks/useNotifications.test.ts @@ -2,7 +2,6 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import axios, { AxiosError } from 'axios'; import nock from 'nock'; -import { mockSingleAccountNotifications } from '../__mocks__/notifications-mocks'; import { mockAuth, mockGitHubCloudAccount, @@ -300,10 +299,6 @@ describe('hooks/useNotifications.ts', () => { describe('removeNotificationFromState', () => { it('should remove a notification from state', async () => { - const mockElement = document.createElement('div'); - mockElement.id = mockSingleAccountNotifications[0].notifications[0].id; - jest.spyOn(document, 'getElementById').mockReturnValue(mockElement); - const notifications = [ { id: 1, title: 'This is a notification.' }, { id: 2, title: 'This is another one.' }, diff --git a/src/styles/gitify.ts b/src/styles/gitify.ts index d305a155a..22be10267 100644 --- a/src/styles/gitify.ts +++ b/src/styles/gitify.ts @@ -5,6 +5,3 @@ 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'; - -export const ANIMATE_NOTIFICATION_CLASS_NAME = - 'translate-x-full opacity-0 transition duration-[350ms] ease-in-out'; diff --git a/src/utils/remove-notification.test.ts b/src/utils/remove-notification.test.ts index b328d16d4..489fdaaa7 100644 --- a/src/utils/remove-notification.test.ts +++ b/src/utils/remove-notification.test.ts @@ -6,10 +6,6 @@ import { removeNotification } from './remove-notification'; describe('utils/remove-notification.ts', () => { it('should remove a notification if it exists', () => { - const mockElement = document.createElement('div'); - mockElement.id = mockSingleAccountNotifications[0].notifications[0].id; - jest.spyOn(document, 'getElementById').mockReturnValue(mockElement); - expect(mockSingleAccountNotifications[0].notifications.length).toBe(1); const result = removeNotification( diff --git a/src/utils/remove-notification.ts b/src/utils/remove-notification.ts index 38c569a5c..aa5c3d217 100644 --- a/src/utils/remove-notification.ts +++ b/src/utils/remove-notification.ts @@ -1,7 +1,4 @@ -import { - ANIMATE_NOTIFICATION_CLASS_NAME, - READ_NOTIFICATION_CLASS_NAME, -} from '../styles/gitify'; +import { READ_NOTIFICATION_CLASS_NAME } from '../styles/gitify'; import type { AccountNotifications, SettingsState } from '../types'; import type { Notification } from '../typesGitHub'; import { getAccountUUID } from './auth/utils'; @@ -17,9 +14,6 @@ export const removeNotification = ( return notifications; } - const notificationRow = document.getElementById(notification.id); - notificationRow.className += ` ${ANIMATE_NOTIFICATION_CLASS_NAME}`; - const notificationId = notification.id; const accountIndex = notifications.findIndex( From 2163a499e9c629603a6539b90b58d01641ac0d82 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 16 Jun 2024 07:07:23 -0400 Subject: [PATCH 4/5] fix: delay state --- src/components/NotificationRow.tsx | 6 ++++++ src/styles/gitify.ts | 2 -- src/utils/remove-notification.test.ts | 11 +---------- src/utils/remove-notification.ts | 3 --- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index d19177fd1..af5fd79a7 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -45,9 +45,12 @@ export const NotificationRow: FC = ({ notification }) => { unsubscribeNotification, } = useContext(AppContext); const [animateExit, setAnimateExit] = useState(false); + const [showAsRead, setShowAsRead] = useState(false); const handleNotification = useCallback(() => { setAnimateExit(!settings.delayNotificationState); + setShowAsRead(settings.delayNotificationState); + openNotification(notification); if (settings.markAsDoneOnOpen) { @@ -102,6 +105,7 @@ export const NotificationRow: FC = ({ 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', )} >
= ({ notification }) => { title="Mark as Done" onClick={() => { setAnimateExit(!settings.delayNotificationState); + setShowAsRead(settings.delayNotificationState); markNotificationDone(notification); }} > @@ -240,6 +245,7 @@ export const NotificationRow: FC = ({ notification }) => { title="Mark as Read" onClick={() => { setAnimateExit(!settings.delayNotificationState); + setShowAsRead(settings.delayNotificationState); markNotificationRead(notification); }} > diff --git a/src/styles/gitify.ts b/src/styles/gitify.ts index 22be10267..ac91a80b3 100644 --- a/src/styles/gitify.ts +++ b/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'; diff --git a/src/utils/remove-notification.test.ts b/src/utils/remove-notification.test.ts index 489fdaaa7..fc80b22cd 100644 --- a/src/utils/remove-notification.test.ts +++ b/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); }); }); diff --git a/src/utils/remove-notification.ts b/src/utils/remove-notification.ts index aa5c3d217..01e0180b5 100644 --- a/src/utils/remove-notification.ts +++ b/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; } From 4c2707dc4502d1e0900ea79b6d4cf0361ec8cd88 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sun, 16 Jun 2024 07:58:30 -0400 Subject: [PATCH 5/5] fix: delay state --- src/components/NotificationRow.test.tsx | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/components/NotificationRow.test.tsx b/src/components/NotificationRow.test.tsx index 0473aa2c5..567b7080b 100644 --- a/src/components/NotificationRow.test.tsx +++ b/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( + + + , + ); + + 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();