diff --git a/src/__mocks__/mock-state.ts b/src/__mocks__/mock-state.ts index 22b06f519..657f846fb 100644 --- a/src/__mocks__/mock-state.ts +++ b/src/__mocks__/mock-state.ts @@ -18,5 +18,5 @@ export const mockSettings: SettingsState = { detailedNotifications: false, markAsDoneOnOpen: false, showAccountHostname: false, - showReadNotifications: false, + showAllNotifications: false, }; diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx index cfcb1c998..42ea6b755 100644 --- a/src/components/NotificationRow.tsx +++ b/src/components/NotificationRow.tsx @@ -40,26 +40,13 @@ export const NotificationRow: FC = ({ notification, hostname }) => { notifications, } = useContext(AppContext); - const setNotificationAsOpaque = () => { - if (notification.unread) { - const notificationRow = document.getElementById(notification.id); - notificationRow.className += Constants.READ_CLASS_NAME; - } - notification.unread = false; - }; - const openNotification = useCallback(() => { openInBrowser(notification, accounts); if (settings.markAsDoneOnOpen) { markNotificationDone(notification.id, hostname); } - - if (!settings.showReadNotifications) { - removeNotificationFromState(notification.id, hostname); - } else { - setNotificationAsOpaque(); - } + handleNotificationState(); }, [notifications, notification, accounts, settings]); // notifications required here to prevent weird state issues const unsubscribe = (event: MouseEvent) => { @@ -68,32 +55,17 @@ export const NotificationRow: FC = ({ notification, hostname }) => { unsubscribeNotification(notification.id, hostname); markNotificationRead(notification.id, hostname); - - if (!settings.showReadNotifications) { - removeNotificationFromState(notification.id, hostname); - } else { - setNotificationAsOpaque(); - } + handleNotificationState(); }; const markAsRead = () => { markNotificationRead(notification.id, hostname); - - if (!settings.showReadNotifications) { - removeNotificationFromState(notification.id, hostname); - } else { - setNotificationAsOpaque(); - } + handleNotificationState(); }; const markAsDone = () => { markNotificationDone(notification.id, hostname); - - if (!settings.showReadNotifications) { - removeNotificationFromState(notification.id, hostname); - } else { - setNotificationAsOpaque(); - } + handleNotificationState(); }; const openUserProfile = ( @@ -105,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); diff --git a/src/components/Repository.tsx b/src/components/Repository.tsx index 2f2a6fe9f..d0d72b21b 100644 --- a/src/components/Repository.tsx +++ b/src/components/Repository.tsx @@ -28,16 +28,6 @@ export const RepositoryNotifications: FC = ({ removeNotificationsFromState, } = useContext(AppContext); - const setNotificationsAsOpaque = (repoNotifications: Notification[]) => { - for (const notification of repoNotifications) { - if (notification.unread) { - const notificationRow = document.getElementById(notification.id); - notificationRow.className += Constants.READ_CLASS_NAME; - } - notification.unread = false; - } - }; - const openBrowser = useCallback(() => { const url = repoNotifications[0].repository.html_url; openExternalLink(url); @@ -46,23 +36,34 @@ export const RepositoryNotifications: FC = ({ const markRepoAsRead = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; markRepoNotificationsRead(repoSlug, hostname); - if (!settings.showReadNotifications) { - removeNotificationsFromState(repoSlug, notifications, hostname); - } else { - setNotificationsAsOpaque(repoNotifications); - } + handleNotificationState(repoSlug); }, [repoNotifications, hostname]); const markRepoAsDone = useCallback(() => { const repoSlug = repoNotifications[0].repository.full_name; markRepoNotificationsDone(repoSlug, hostname); - if (!settings.showReadNotifications) { - removeNotificationsFromState(repoSlug, notifications, hostname); - } else { - setNotificationsAsOpaque(repoNotifications); - } + 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; 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 8e8742179..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(); diff --git a/src/context/App.tsx b/src/context/App.tsx index 522792cea..92bc609d4 100644 --- a/src/context/App.tsx +++ b/src/context/App.tsx @@ -23,7 +23,7 @@ import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth'; import { setAutoLaunch, updateTrayTitle } from '../utils/comms'; import Constants from '../utils/constants'; import { generateGitHubAPIUrl } from '../utils/helpers'; -import { getNotificationCount } from '../utils/notifications'; +import { getUnreadNotificationCount } from '../utils/notifications'; import { clearState, loadState, saveState } from '../utils/storage'; import { setTheme } from '../utils/theme'; @@ -44,7 +44,7 @@ export const defaultSettings: SettingsState = { detailedNotifications: false, markAsDoneOnOpen: false, showAccountHostname: false, - showReadNotifications: false, + showAllNotifications: false, }; interface AppContextState { @@ -114,7 +114,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { settings.participating, settings.showBots, settings.detailedNotifications, - settings.showReadNotifications, + settings.showAllNotifications, accounts.token, accounts.enterpriseAccounts.length, ]); @@ -125,7 +125,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { // biome-ignore lint/correctness/useExhaustiveDependencies: We need to update tray title when settings or notifications changes. useEffect(() => { - const count = getNotificationCount(notifications); + const count = getUnreadNotificationCount(notifications); if (settings.showNotificationsCountInTray && count > 0) { updateTrayTitle(count.toString()); diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts index 7f4f33e71..8302a5989 100644 --- a/src/hooks/useNotifications.ts +++ b/src/hooks/useNotifications.ts @@ -80,7 +80,7 @@ export const useNotifications = (): NotificationsState => { const fetchNotifications = useCallback( async (accounts: AuthState, settings: SettingsState) => { function getNotifications(hostname: string, token: string): AxiosPromise { - const endpointSuffix = `notifications?all=${settings.showReadNotifications}&participating=${settings.participating}`; + const endpointSuffix = `notifications?all=${settings.showAllNotifications}&participating=${settings.participating}`; const url = `${generateGitHubAPIUrl(hostname)}${endpointSuffix}`; return apiRequestAuth(url, 'GET', token); } diff --git a/src/routes/Notifications.tsx b/src/routes/Notifications.tsx index f006efe29..72d38df91 100644 --- a/src/routes/Notifications.tsx +++ b/src/routes/Notifications.tsx @@ -5,7 +5,7 @@ import { AllRead } from '../components/AllRead'; import { Oops } from '../components/Oops'; import { AppContext } from '../context/App'; import { Errors } from '../utils/constants'; -import { getNotificationCount } from '../utils/notifications'; +import { getUnreadNotificationCount } from '../utils/notifications'; export const NotificationsRoute: FC = () => { const { notifications, requestFailed, errorDetails, settings } = @@ -16,7 +16,7 @@ export const NotificationsRoute: FC = () => { [notifications], ); const notificationsCount = useMemo(() => { - return getNotificationCount(notifications); + return getUnreadNotificationCount(notifications); }, [notifications]); const hasNotifications = useMemo( @@ -28,7 +28,7 @@ export const NotificationsRoute: FC = () => { return ; } - if (!hasNotifications) { + if (!hasNotifications && !settings.showAllNotifications) { return ; } diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx index 3df4cf185..658a02a4a 100644 --- a/src/routes/Settings.tsx +++ b/src/routes/Settings.tsx @@ -183,11 +183,23 @@ export const SettingsRoute: FC = () => { Notifications - updateSetting('showReadNotifications', evt.target.checked) + updateSetting('showAllNotifications', evt.target.checked) + } + tooltip={ +
+
+ Will show up to 50 latest notifications regardless of status + (read, unread, done). +
+
+ ⚠️ Users may experience rate limiting under certain + circumstances. Disable this setting if you experience this. +
+
} /> { - const allNotificationsCount = getNotificationCount(notifications); + const notificationCount = getUnreadNotificationCount(notifications); - updateTrayIcon(allNotificationsCount); + updateTrayIcon(notificationCount); }; -export function getNotificationCount(notifications: AccountNotifications[]) { +export function getUnreadNotificationCount( + notifications: AccountNotifications[], +) { return notifications.reduce( - (memo, acc) => memo + acc.notifications.length, + (memo, acc) => + memo + + acc.notifications.filter((notification) => notification.unread).length, 0, ); }