diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts index 01cb113a3..657f846fb 100644 --- a/src/__mocks__/mock-state.ts +++ b/src/__mocks__/mock-state.ts @@ -18,4 +18,5 @@ export const mockSettings: SettingsState = { detailedNotifications: false, markAsDoneOnOpen: false, showAccountHostname: false, + showAllNotifications: false, }; diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index 517b5d7f9..42ea6b755 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -16,6 +16,7 @@ import { import { AppContext } from '../context/App'; import type { Notification } from '../typesGithub'; import { openExternalLink } from '../utils/comms'; +import Constants from '../utils/constants'; import { formatForDisplay, openInBrowser } from '../utils/helpers'; import { getNotificationTypeIcon, @@ -32,9 +33,9 @@ export const NotificationRow: FC = ({ notification, hostname }) => { const { settings, accounts, - removeNotificationFromState, markNotificationRead, markNotificationDone, + removeNotificationFromState, unsubscribeNotification, notifications, } = useContext(AppContext); @@ -44,10 +45,8 @@ export const NotificationRow: FC = ({ notification, hostname }) => { if (settings.markAsDoneOnOpen) { markNotificationDone(notification.id, hostname); - } else { - // no need to mark as read, github does it by default when opening it - removeNotificationFromState(notification.id, hostname); } + handleNotificationState(); }, [notifications, notification, accounts, settings]); // notifications required here to prevent weird state issues const unsubscribe = (event: MouseEvent) => { @@ -55,6 +54,18 @@ export const NotificationRow: FC = ({ notification, hostname }) => { event.stopPropagation(); unsubscribeNotification(notification.id, hostname); + markNotificationRead(notification.id, hostname); + handleNotificationState(); + }; + + const markAsRead = () => { + markNotificationRead(notification.id, hostname); + handleNotificationState(); + }; + + const markAsDone = () => { + markNotificationDone(notification.id, hostname); + handleNotificationState(); }; const openUserProfile = ( @@ -66,6 +77,20 @@ export const NotificationRow: FC = ({ notification, hostname }) => { openExternalLink(notification.subject.user.html_url); }; + const handleNotificationState = useCallback(() => { + if (!settings.showAllNotifications) { + removeNotificationFromState(notification.id, hostname); + } + + if (notification.unread) { + const notificationRow = document.getElementById(notification.id); + notificationRow.className += Constants.READ_CLASS_NAME; + } + + // TODO FIXME - this is not updating the notification count + notification.unread = false; + }, [notification, notifications]); + const reason = formatReason(notification.reason); const NotificationIcon = getNotificationTypeIcon(notification.subject); const iconColor = getNotificationTypeIconColor(notification.subject); @@ -82,18 +107,21 @@ export const NotificationRow: FC = ({ notification, hostname }) => { ]); return ( -
+
-
openNotification()} - onKeyDown={() => openNotification()} + onClick={openNotification} + onKeyDown={openNotification} >
= ({ notification, hostname }) => { type="button" className="focus:outline-none h-full hover:text-green-500" title="Mark as Done" - onClick={() => markNotificationDone(notification.id, hostname)} + onClick={markAsDone} > @@ -155,14 +183,18 @@ export const NotificationRow: FC = ({ notification, hostname }) => { - + {notification.unread ? ( + + ) : ( +
+ )}
); diff --git a/src/components/Repository.test.tsx b/src/components/Repository.test.tsx index a2e5d877f..75973f5ab 100644 --- a/src/components/Repository.test.tsx +++ b/src/components/Repository.test.tsx @@ -10,7 +10,7 @@ jest.mock('./NotificationRow', () => ({ })); describe('components/Repository.tsx', () => { - const markRepoNotifications = jest.fn(); + const markRepoNotificationsRead = jest.fn(); const markRepoNotificationsDone = jest.fn(); const props = { @@ -20,7 +20,7 @@ describe('components/Repository.tsx', () => { }; beforeEach(() => { - markRepoNotifications.mockReset(); + markRepoNotificationsRead.mockReset(); jest.spyOn(shell, 'openExternal'); }); @@ -51,14 +51,14 @@ describe('components/Repository.tsx', () => { it('should mark a repo as read', () => { render( - + , ); fireEvent.click(screen.getByTitle('Mark Repository as Read')); - expect(markRepoNotifications).toHaveBeenCalledWith( + expect(markRepoNotificationsRead).toHaveBeenCalledWith( 'manosim/notifications-test', 'github.com', ); diff --git a/src/components/Repository.tsx b/src/components/Repository.tsx index 28bef678f..d0d72b21b 100644 --- a/src/components/Repository.tsx +++ b/src/components/Repository.tsx @@ -6,6 +6,7 @@ import { CSSTransition, TransitionGroup } from 'react-transition-group'; import { AppContext } from '../context/App'; import type { Notification } from '../typesGithub'; import { openExternalLink } from '../utils/comms'; +import Constants from '../utils/constants'; import { NotificationRow } from './NotificationRow'; interface IProps { @@ -19,8 +20,13 @@ export const RepositoryNotifications: FC = ({ repoNotifications, hostname, }) => { - const { markRepoNotifications, markRepoNotificationsDone } = - useContext(AppContext); + const { + markRepoNotificationsRead, + markRepoNotificationsDone, + settings, + notifications, + removeNotificationsFromState, + } = useContext(AppContext); const openBrowser = useCallback(() => { const url = repoNotifications[0].repository.html_url; @@ -29,14 +35,35 @@ export const RepositoryNotifications: FC = ({ const markRepoAsRead = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; - markRepoNotifications(repoSlug, hostname); + markRepoNotificationsRead(repoSlug, hostname); + handleNotificationState(repoSlug); }, [repoNotifications, hostname]); const markRepoAsDone = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; markRepoNotificationsDone(repoSlug, hostname); + handleNotificationState(repoSlug); }, [repoNotifications, hostname]); + const handleNotificationState = useCallback( + (repoSlug: string) => { + if (!settings.showAllNotifications) { + removeNotificationsFromState(repoSlug, notifications, hostname); + } + + for (const notification of repoNotifications) { + if (notification.unread) { + const notificationRow = document.getElementById(notification.id); + notificationRow.className += Constants.READ_CLASS_NAME; + } + + // TODO FIXME - this is not updating the notification count + notification.unread = false; + } + }, + [repoNotifications, hostname, notifications], + ); + const avatarUrl = repoNotifications[0].repository.owner.avatar_url; const repoSlug = repoNotifications[0].repository.full_name; @@ -74,14 +101,18 @@ export const RepositoryNotifications: FC = ({
- + {repoNotifications.some((notification) => notification.unread) ? ( + + ) : ( +
+ )}
diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index 216e5b2a4..91edfce2f 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -13,7 +13,7 @@ import { Logo } from '../components/Logo'; import { AppContext } from '../context/App'; import { openExternalLink } from '../utils/comms'; import { Constants } from '../utils/constants'; -import { getNotificationCount } from '../utils/notifications'; +import { getUnreadNotificationCount } from '../utils/notifications'; export const Sidebar: FC = () => { const navigate = useNavigate(); @@ -35,7 +35,7 @@ export const Sidebar: FC = () => { }, []); const notificationsCount = useMemo(() => { - return getNotificationCount(notifications); + return getUnreadNotificationCount(notifications); }, [notifications]); const sidebarButtonClasses = diff --git a/src/context/App.test.tsx b/src/context/App.test.tsx index 3280d5773..65fc8dad4 100644 --- a/src/context/App.test.tsx +++ b/src/context/App.test.tsx @@ -37,11 +37,11 @@ describe('context/App.tsx', () => { describe('api methods', () => { const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth'); - const getNotificationCountMock = jest.spyOn( + const getUnreadNotificationCountMock = jest.spyOn( notifications, - 'getNotificationCount', + 'getUnreadNotificationCount', ); - getNotificationCountMock.mockReturnValue(1); + getUnreadNotificationCountMock.mockReturnValue(1); const fetchNotificationsMock = jest.fn(); const markNotificationReadMock = jest.fn(); @@ -193,15 +193,15 @@ describe('context/App.tsx', () => { ); }); - it('should call markRepoNotifications', async () => { + it('should call markRepoNotificationsRead', async () => { const TestComponent = () => { - const { markRepoNotifications } = useContext(AppContext); + const { markRepoNotificationsRead } = useContext(AppContext); return (