Skip to content

Commit c3cbcef

Browse files
setchysetchy
committed
feat: show all notifications
1 parent 7aa8200 commit c3cbcef

File tree

11 files changed

+78
-75
lines changed

11 files changed

+78
-75
lines changed

‎src/__mocks__/mock-state.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@ export const mockSettings: SettingsState = {
1818
detailedNotifications: false,
1919
markAsDoneOnOpen: false,
2020
showAccountHostname: false,
21-
showReadNotifications: false,
21+
showAllNotifications: false,
2222
};

‎src/components/NotificationRow.tsx

+18-32
Original file line numberDiff line numberDiff line change
@@ -40,26 +40,13 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
4040
notifications,
4141
} = useContext(AppContext);
4242

43-
const setNotificationAsOpaque = () => {
44-
if (notification.unread) {
45-
const notificationRow = document.getElementById(notification.id);
46-
notificationRow.className += Constants.READ_CLASS_NAME;
47-
}
48-
notification.unread = false;
49-
};
50-
5143
const openNotification = useCallback(() => {
5244
openInBrowser(notification, accounts);
5345

5446
if (settings.markAsDoneOnOpen) {
5547
markNotificationDone(notification.id, hostname);
5648
}
57-
58-
if (!settings.showReadNotifications) {
59-
removeNotificationFromState(notification.id, hostname);
60-
} else {
61-
setNotificationAsOpaque();
62-
}
49+
handleNotificationState();
6350
}, [notifications, notification, accounts, settings]); // notifications required here to prevent weird state issues
6451

6552
const unsubscribe = (event: MouseEvent<HTMLElement>) => {
@@ -68,32 +55,17 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
6855

6956
unsubscribeNotification(notification.id, hostname);
7057
markNotificationRead(notification.id, hostname);
71-
72-
if (!settings.showReadNotifications) {
73-
removeNotificationFromState(notification.id, hostname);
74-
} else {
75-
setNotificationAsOpaque();
76-
}
58+
handleNotificationState();
7759
};
7860

7961
const markAsRead = () => {
8062
markNotificationRead(notification.id, hostname);
81-
82-
if (!settings.showReadNotifications) {
83-
removeNotificationFromState(notification.id, hostname);
84-
} else {
85-
setNotificationAsOpaque();
86-
}
63+
handleNotificationState();
8764
};
8865

8966
const markAsDone = () => {
9067
markNotificationDone(notification.id, hostname);
91-
92-
if (!settings.showReadNotifications) {
93-
removeNotificationFromState(notification.id, hostname);
94-
} else {
95-
setNotificationAsOpaque();
96-
}
68+
handleNotificationState();
9769
};
9870

9971
const openUserProfile = (
@@ -105,6 +77,20 @@ export const NotificationRow: FC<IProps> = ({ notification, hostname }) => {
10577
openExternalLink(notification.subject.user.html_url);
10678
};
10779

80+
const handleNotificationState = useCallback(() => {
81+
if (!settings.showAllNotifications) {
82+
removeNotificationFromState(notification.id, hostname);
83+
}
84+
85+
if (notification.unread) {
86+
const notificationRow = document.getElementById(notification.id);
87+
notificationRow.className += Constants.READ_CLASS_NAME;
88+
}
89+
90+
// TODO FIXME - this is not updating the notification count
91+
notification.unread = false;
92+
}, [notification, notifications]);
93+
10894
const reason = formatReason(notification.reason);
10995
const NotificationIcon = getNotificationTypeIcon(notification.subject);
11096
const iconColor = getNotificationTypeIconColor(notification.subject);

‎src/components/Repository.tsx

+21-20
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,6 @@ export const RepositoryNotifications: FC<IProps> = ({
2828
removeNotificationsFromState,
2929
} = useContext(AppContext);
3030

31-
const setNotificationsAsOpaque = (repoNotifications: Notification[]) => {
32-
for (const notification of repoNotifications) {
33-
if (notification.unread) {
34-
const notificationRow = document.getElementById(notification.id);
35-
notificationRow.className += Constants.READ_CLASS_NAME;
36-
}
37-
notification.unread = false;
38-
}
39-
};
40-
4131
const openBrowser = useCallback(() => {
4232
const url = repoNotifications[0].repository.html_url;
4333
openExternalLink(url);
@@ -46,23 +36,34 @@ export const RepositoryNotifications: FC<IProps> = ({
4636
const markRepoAsRead = useCallback(() => {
4737
const repoSlug = repoNotifications[0].repository.full_name;
4838
markRepoNotificationsRead(repoSlug, hostname);
49-
if (!settings.showReadNotifications) {
50-
removeNotificationsFromState(repoSlug, notifications, hostname);
51-
} else {
52-
setNotificationsAsOpaque(repoNotifications);
53-
}
39+
handleNotificationState(repoSlug);
5440
}, [repoNotifications, hostname]);
5541

5642
const markRepoAsDone = useCallback(() => {
5743
const repoSlug = repoNotifications[0].repository.full_name;
5844
markRepoNotificationsDone(repoSlug, hostname);
59-
if (!settings.showReadNotifications) {
60-
removeNotificationsFromState(repoSlug, notifications, hostname);
61-
} else {
62-
setNotificationsAsOpaque(repoNotifications);
63-
}
45+
handleNotificationState(repoSlug);
6446
}, [repoNotifications, hostname]);
6547

48+
const handleNotificationState = useCallback(
49+
(repoSlug: string) => {
50+
if (!settings.showAllNotifications) {
51+
removeNotificationsFromState(repoSlug, notifications, hostname);
52+
}
53+
54+
for (const notification of repoNotifications) {
55+
if (notification.unread) {
56+
const notificationRow = document.getElementById(notification.id);
57+
notificationRow.className += Constants.READ_CLASS_NAME;
58+
}
59+
60+
// TODO FIXME - this is not updating the notification count
61+
notification.unread = false;
62+
}
63+
},
64+
[repoNotifications, hostname, notifications],
65+
);
66+
6667
const avatarUrl = repoNotifications[0].repository.owner.avatar_url;
6768
const repoSlug = repoNotifications[0].repository.full_name;
6869

‎src/components/Sidebar.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { Logo } from '../components/Logo';
1313
import { AppContext } from '../context/App';
1414
import { openExternalLink } from '../utils/comms';
1515
import { Constants } from '../utils/constants';
16-
import { getNotificationCount } from '../utils/notifications';
16+
import { getUnreadNotificationCount } from '../utils/notifications';
1717

1818
export const Sidebar: FC = () => {
1919
const navigate = useNavigate();
@@ -35,7 +35,7 @@ export const Sidebar: FC = () => {
3535
}, []);
3636

3737
const notificationsCount = useMemo(() => {
38-
return getNotificationCount(notifications);
38+
return getUnreadNotificationCount(notifications);
3939
}, [notifications]);
4040

4141
const sidebarButtonClasses =

‎src/context/App.test.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ describe('context/App.tsx', () => {
3737

3838
describe('api methods', () => {
3939
const apiRequestAuthMock = jest.spyOn(apiRequests, 'apiRequestAuth');
40-
const getNotificationCountMock = jest.spyOn(
40+
const getUnreadNotificationCountMock = jest.spyOn(
4141
notifications,
42-
'getNotificationCount',
42+
'getUnreadNotificationCount',
4343
);
44-
getNotificationCountMock.mockReturnValue(1);
44+
getUnreadNotificationCountMock.mockReturnValue(1);
4545

4646
const fetchNotificationsMock = jest.fn();
4747
const markNotificationReadMock = jest.fn();

‎src/context/App.tsx

+4-4
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { addAccount, authGitHub, getToken, getUserData } from '../utils/auth';
2323
import { setAutoLaunch, updateTrayTitle } from '../utils/comms';
2424
import Constants from '../utils/constants';
2525
import { generateGitHubAPIUrl } from '../utils/helpers';
26-
import { getNotificationCount } from '../utils/notifications';
26+
import { getUnreadNotificationCount } from '../utils/notifications';
2727
import { clearState, loadState, saveState } from '../utils/storage';
2828
import { setTheme } from '../utils/theme';
2929

@@ -44,7 +44,7 @@ export const defaultSettings: SettingsState = {
4444
detailedNotifications: false,
4545
markAsDoneOnOpen: false,
4646
showAccountHostname: false,
47-
showReadNotifications: false,
47+
showAllNotifications: false,
4848
};
4949

5050
interface AppContextState {
@@ -114,7 +114,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
114114
settings.participating,
115115
settings.showBots,
116116
settings.detailedNotifications,
117-
settings.showReadNotifications,
117+
settings.showAllNotifications,
118118
accounts.token,
119119
accounts.enterpriseAccounts.length,
120120
]);
@@ -125,7 +125,7 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
125125

126126
// biome-ignore lint/correctness/useExhaustiveDependencies: We need to update tray title when settings or notifications changes.
127127
useEffect(() => {
128-
const count = getNotificationCount(notifications);
128+
const count = getUnreadNotificationCount(notifications);
129129

130130
if (settings.showNotificationsCountInTray && count > 0) {
131131
updateTrayTitle(count.toString());

‎src/hooks/useNotifications.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export const useNotifications = (): NotificationsState => {
8080
const fetchNotifications = useCallback(
8181
async (accounts: AuthState, settings: SettingsState) => {
8282
function getNotifications(hostname: string, token: string): AxiosPromise {
83-
const endpointSuffix = `notifications?all=${settings.showReadNotifications}&participating=${settings.participating}`;
83+
const endpointSuffix = `notifications?all=${settings.showAllNotifications}&participating=${settings.participating}`;
8484
const url = `${generateGitHubAPIUrl(hostname)}${endpointSuffix}`;
8585
return apiRequestAuth(url, 'GET', token);
8686
}

‎src/routes/Notifications.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { AllRead } from '../components/AllRead';
55
import { Oops } from '../components/Oops';
66
import { AppContext } from '../context/App';
77
import { Errors } from '../utils/constants';
8-
import { getNotificationCount } from '../utils/notifications';
8+
import { getUnreadNotificationCount } from '../utils/notifications';
99

1010
export const NotificationsRoute: FC = () => {
1111
const { notifications, requestFailed, errorDetails, settings } =
@@ -16,7 +16,7 @@ export const NotificationsRoute: FC = () => {
1616
[notifications],
1717
);
1818
const notificationsCount = useMemo(() => {
19-
return getNotificationCount(notifications);
19+
return getUnreadNotificationCount(notifications);
2020
}, [notifications]);
2121

2222
const hasNotifications = useMemo(
@@ -28,7 +28,7 @@ export const NotificationsRoute: FC = () => {
2828
return <Oops error={errorDetails ?? Errors.UNKNOWN} />;
2929
}
3030

31-
if (!hasNotifications) {
31+
if (!hasNotifications && !settings.showAllNotifications) {
3232
return <AllRead />;
3333
}
3434

‎src/routes/Settings.tsx

+16-4
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,23 @@ export const SettingsRoute: FC = () => {
183183
Notifications
184184
</legend>
185185
<Checkbox
186-
name="showReadNotifications"
187-
label="Show read notifications"
188-
checked={settings.showReadNotifications}
186+
name="showAllNotifications"
187+
label="Show all notifications"
188+
checked={settings.showAllNotifications}
189189
onChange={(evt) =>
190-
updateSetting('showReadNotifications', evt.target.checked)
190+
updateSetting('showAllNotifications', evt.target.checked)
191+
}
192+
tooltip={
193+
<div>
194+
<div className="pb-3">
195+
Will show up to 50 latest notifications regardless of status
196+
(read, unread, done).
197+
</div>
198+
<div className="text-orange-600">
199+
⚠️ Users <i>may</i> experience rate limiting under certain
200+
circumstances. Disable this setting if you experience this.
201+
</div>
202+
</div>
191203
}
192204
/>
193205
<Checkbox

‎src/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ interface NotificationSettingsState {
2121
showNotifications: boolean;
2222
showBots: boolean;
2323
markAsDoneOnOpen: boolean;
24-
showReadNotifications: boolean;
24+
showAllNotifications: boolean;
2525
}
2626

2727
interface SystemSettingsState {

‎src/utils/notifications.ts

+8-4
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,18 @@ import { updateTrayIcon } from './comms';
66
import type { AccountNotifications, AuthState, SettingsState } from '../types';
77

88
export const setTrayIconColor = (notifications: AccountNotifications[]) => {
9-
const allNotificationsCount = getNotificationCount(notifications);
9+
const notificationCount = getUnreadNotificationCount(notifications);
1010

11-
updateTrayIcon(allNotificationsCount);
11+
updateTrayIcon(notificationCount);
1212
};
1313

14-
export function getNotificationCount(notifications: AccountNotifications[]) {
14+
export function getUnreadNotificationCount(
15+
notifications: AccountNotifications[],
16+
) {
1517
return notifications.reduce(
16-
(memo, acc) => memo + acc.notifications.length,
18+
(memo, acc) =>
19+
memo +
20+
acc.notifications.filter((notification) => notification.unread).length,
1721
0,
1822
);
1923
}

0 commit comments

Comments
 (0)