From 867d299d27d37b42e855785e5fe93d1ec6697d65 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 14 Oct 2025 21:04:08 -0700 Subject: [PATCH 1/5] feat: stabilize notification order during notification interactions Signed-off-by: Adam Setch --- .../notifications/AccountNotifications.tsx | 36 +++++++++--------- src/renderer/routes/Notifications.tsx | 24 ++++++------ src/renderer/typesGitHub.ts | 2 + .../utils/api/__mocks__/response-mocks.ts | 8 ++++ src/renderer/utils/notifications/group.ts | 38 +++++++++++++++++++ .../utils/notifications/notifications.ts | 33 ++++++++++++++++ 6 files changed, 112 insertions(+), 29 deletions(-) create mode 100644 src/renderer/utils/notifications/group.ts diff --git a/src/renderer/components/notifications/AccountNotifications.tsx b/src/renderer/components/notifications/AccountNotifications.tsx index 7d8c3376f..c1f919519 100644 --- a/src/renderer/components/notifications/AccountNotifications.tsx +++ b/src/renderer/components/notifications/AccountNotifications.tsx @@ -13,6 +13,10 @@ import { openGitHubIssues, openGitHubPulls, } from '../../utils/links'; +import { + groupNotificationsByRepository, + isGroupByRepository, +} from '../../utils/notifications/group'; import { AllRead } from '../AllRead'; import { AvatarWithFallback } from '../avatars/AvatarWithFallback'; import { Oops } from '../Oops'; @@ -31,28 +35,26 @@ interface IAccountNotifications { export const AccountNotifications: FC = ( props: IAccountNotifications, ) => { - const { account, showAccountHeader, notifications } = props; + const { account, showAccountHeader, error, notifications } = props; const { settings } = useContext(AppContext); const [showAccountNotifications, setShowAccountNotifications] = useState(true); - const groupedNotifications = Object.values( - notifications.reduce( - (acc: { [key: string]: Notification[] }, notification) => { - const key = notification.repository.full_name; - if (!acc[key]) { - acc[key] = []; - } - - acc[key].push(notification); - return acc; - }, - {}, - ), + const sortedNotifications = useMemo( + () => [...notifications].sort((a, b) => a.order - b.order), + [notifications], ); + const groupedNotifications = useMemo(() => { + const map = groupNotificationsByRepository([ + { account, error, notifications: sortedNotifications }, + ]); + + return Array.from(map.values()); + }, [account, error, sortedNotifications]); + const hasNotifications = useMemo( () => notifications.length > 0, [notifications], @@ -68,8 +70,6 @@ export const AccountNotifications: FC = ( 'account', ); - const isGroupByRepository = settings.groupBy === 'REPOSITORY'; - return ( <> {showAccountHeader && ( @@ -130,7 +130,7 @@ export const AccountNotifications: FC = ( <> {props.error && } {!hasNotifications && !props.error && } - {isGroupByRepository + {isGroupByRepository(settings) ? Object.values(groupedNotifications).map((repoNotifications) => { const repoSlug = repoNotifications[0].repository.full_name; @@ -142,7 +142,7 @@ export const AccountNotifications: FC = ( /> ); }) - : notifications.map((notification) => ( + : sortedNotifications.map((notification) => ( { return ( - {notifications.map((accountNotifications) => ( - - ))} + {notifications.map((accountNotification) => { + return ( + + ); + })} ); diff --git a/src/renderer/typesGitHub.ts b/src/renderer/typesGitHub.ts index d38ff2c27..d30d3c2c5 100644 --- a/src/renderer/typesGitHub.ts +++ b/src/renderer/typesGitHub.ts @@ -112,6 +112,8 @@ export interface GitHubNotification { // Note: This is not in the official GitHub API. We add this to make notification interactions easier. export interface GitifyNotification { account: Account; + order: number; + groupName: string | null; } export type UserDetails = User & UserProfile; diff --git a/src/renderer/utils/api/__mocks__/response-mocks.ts b/src/renderer/utils/api/__mocks__/response-mocks.ts index 6aa43e4a9..85248bc39 100644 --- a/src/renderer/utils/api/__mocks__/response-mocks.ts +++ b/src/renderer/utils/api/__mocks__/response-mocks.ts @@ -45,6 +45,8 @@ export const mockNotificationUser: User = { export const mockGitHubNotifications: Notification[] = [ { account: mockGitHubCloudAccount, + groupName: null, + order: 0, id: '138661096', unread: true, reason: 'subscribed', @@ -197,6 +199,8 @@ export const mockGitHubNotifications: Notification[] = [ }, { account: mockGitHubCloudAccount, + groupName: null, + order: 1, id: '148827438', unread: true, reason: 'author', @@ -260,6 +264,8 @@ export const mockGitHubNotifications: Notification[] = [ export const mockEnterpriseNotifications: Notification[] = [ { account: mockGitHubEnterpriseServerAccount, + groupName: null, + order: 0, id: '3', unread: true, reason: 'subscribed', @@ -316,6 +322,8 @@ export const mockEnterpriseNotifications: Notification[] = [ }, { account: mockGitHubEnterpriseServerAccount, + groupName: null, + order: 1, id: '4', unread: true, reason: 'subscribed', diff --git a/src/renderer/utils/notifications/group.ts b/src/renderer/utils/notifications/group.ts new file mode 100644 index 000000000..921fef0eb --- /dev/null +++ b/src/renderer/utils/notifications/group.ts @@ -0,0 +1,38 @@ +import type { AccountNotifications, SettingsState } from '../../types'; +import type { Notification } from '../../typesGitHub'; + +export function isGroupByRepository(settings: SettingsState) { + return settings.groupBy === 'REPOSITORY'; +} + +/** + * Group notifications by repository.full_name preserving first-seen repo order. + * Returns a Map where keys are repo full_names and values are arrays of notifications. + */ +export function groupNotificationsByRepository( + accounts: AccountNotifications[], +): Map { + const repoGroups = new Map(); + + for (const account of accounts) { + for (const notification of account.notifications) { + const repo = notification.repository?.full_name ?? ''; + const group = repoGroups.get(repo); + + if (group) { + group.push(notification); + } else { + repoGroups.set(repo, [notification]); + } + } + } + + return repoGroups; +} + +/** + * Flatten the Map values into a single array, preserving Map insertion order (first-seen repo order). + */ +export function flattenRepoGroups(repoGroups: Map) { + return Array.from(repoGroups.values()).flat(); +} diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 1aef06d0b..2825672e6 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -12,6 +12,11 @@ import { filterBaseNotifications, filterDetailedNotifications, } from './filters/filter'; +import { + flattenRepoGroups, + groupNotificationsByRepository, + isGroupByRepository, +} from './group'; import { createNotificationHandler } from './handlers'; export function setTrayIconColor(notifications: AccountNotifications[]) { @@ -90,6 +95,9 @@ export async function getAllNotifications( }), ); + // Set the order property for the notifications + stabilizeNotificationsOrder(notifications, state.settings); + return notifications; } @@ -141,3 +149,28 @@ export async function enrichNotification( }, }; } + +export function stabilizeNotificationsOrder( + notifications: AccountNotifications[], + settings: SettingsState, +) { + if (isGroupByRepository(settings)) { + const repoGroups = groupNotificationsByRepository(notifications); + const flattened = flattenRepoGroups(repoGroups); + + let orderIndex = 0; + + for (const n of flattened) { + n.order = orderIndex++; + } + } else { + // Non-repository grouping: assign sequential order across all notifications + let orderIndex = 0; + + notifications.forEach((accountNotifications) => { + accountNotifications.notifications.forEach((notification) => { + notification.order = orderIndex++; + }); + }); + } +} From 2b02a9a679d8423b5da2993d642ec6e4eaae165b Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 14 Oct 2025 21:06:51 -0700 Subject: [PATCH 2/5] feat: stabilize notification order during notification interactions Signed-off-by: Adam Setch --- src/renderer/utils/notifications/notifications.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 2825672e6..4f49bbc01 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -167,10 +167,10 @@ export function stabilizeNotificationsOrder( // Non-repository grouping: assign sequential order across all notifications let orderIndex = 0; - notifications.forEach((accountNotifications) => { - accountNotifications.notifications.forEach((notification) => { + for (const accountNotifications of notifications) { + for (const notification of accountNotifications.notifications) { notification.order = orderIndex++; - }); - }); + } + } } } From 986581c80160ba923fe1ac766162e2d6f83b56fc Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 14 Oct 2025 21:18:24 -0700 Subject: [PATCH 3/5] feat: stabilize notification order during notification interactions Signed-off-by: Adam Setch --- src/renderer/utils/notifications/group.ts | 18 ++++++++-- .../utils/notifications/notifications.ts | 33 ++++++------------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/renderer/utils/notifications/group.ts b/src/renderer/utils/notifications/group.ts index 921fef0eb..0c118071a 100644 --- a/src/renderer/utils/notifications/group.ts +++ b/src/renderer/utils/notifications/group.ts @@ -1,6 +1,9 @@ import type { AccountNotifications, SettingsState } from '../../types'; import type { Notification } from '../../typesGitHub'; +/** + * Returns true when settings say to group by repository. + */ export function isGroupByRepository(settings: SettingsState) { return settings.groupBy === 'REPOSITORY'; } @@ -31,8 +34,17 @@ export function groupNotificationsByRepository( } /** - * Flatten the Map values into a single array, preserving Map insertion order (first-seen repo order). + * Returns a flattened, ordered Notification[] according to repository-first-seen order + * (when grouped) or the natural account->notification order otherwise. */ -export function flattenRepoGroups(repoGroups: Map) { - return Array.from(repoGroups.values()).flat(); +export function getFlattenedNotificationsByRepo( + accounts: AccountNotifications[], + settings: SettingsState, +): Notification[] { + if (isGroupByRepository(settings)) { + const repoGroups = groupNotificationsByRepository(accounts); + return Array.from(repoGroups.values()).flat(); + } + + return accounts.flatMap((a) => a.notifications); } diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 4f49bbc01..4df75b76d 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -12,11 +12,7 @@ import { filterBaseNotifications, filterDetailedNotifications, } from './filters/filter'; -import { - flattenRepoGroups, - groupNotificationsByRepository, - isGroupByRepository, -} from './group'; +import { getFlattenedNotificationsByRepo } from './group'; import { createNotificationHandler } from './handlers'; export function setTrayIconColor(notifications: AccountNotifications[]) { @@ -154,23 +150,14 @@ export function stabilizeNotificationsOrder( notifications: AccountNotifications[], settings: SettingsState, ) { - if (isGroupByRepository(settings)) { - const repoGroups = groupNotificationsByRepository(notifications); - const flattened = flattenRepoGroups(repoGroups); - - let orderIndex = 0; - - for (const n of flattened) { - n.order = orderIndex++; - } - } else { - // Non-repository grouping: assign sequential order across all notifications - let orderIndex = 0; - - for (const accountNotifications of notifications) { - for (const notification of accountNotifications.notifications) { - notification.order = orderIndex++; - } - } + const flattenedNotifications = getFlattenedNotificationsByRepo( + notifications, + settings, + ); + + let orderIndex = 0; + + for (const n of flattenedNotifications) { + n.order = orderIndex++; } } From 73b412d19fcba496b0f2f7a6776996cd7c440a0c Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Tue, 14 Oct 2025 21:22:38 -0700 Subject: [PATCH 4/5] feat: stabilize notification order during notification interactions Signed-off-by: Adam Setch --- src/renderer/typesGitHub.ts | 1 - src/renderer/utils/api/__mocks__/response-mocks.ts | 4 ---- 2 files changed, 5 deletions(-) diff --git a/src/renderer/typesGitHub.ts b/src/renderer/typesGitHub.ts index d30d3c2c5..a3c7f8e35 100644 --- a/src/renderer/typesGitHub.ts +++ b/src/renderer/typesGitHub.ts @@ -113,7 +113,6 @@ export interface GitHubNotification { export interface GitifyNotification { account: Account; order: number; - groupName: string | null; } export type UserDetails = User & UserProfile; diff --git a/src/renderer/utils/api/__mocks__/response-mocks.ts b/src/renderer/utils/api/__mocks__/response-mocks.ts index 85248bc39..2b79c20d3 100644 --- a/src/renderer/utils/api/__mocks__/response-mocks.ts +++ b/src/renderer/utils/api/__mocks__/response-mocks.ts @@ -45,7 +45,6 @@ export const mockNotificationUser: User = { export const mockGitHubNotifications: Notification[] = [ { account: mockGitHubCloudAccount, - groupName: null, order: 0, id: '138661096', unread: true, @@ -199,7 +198,6 @@ export const mockGitHubNotifications: Notification[] = [ }, { account: mockGitHubCloudAccount, - groupName: null, order: 1, id: '148827438', unread: true, @@ -264,7 +262,6 @@ export const mockGitHubNotifications: Notification[] = [ export const mockEnterpriseNotifications: Notification[] = [ { account: mockGitHubEnterpriseServerAccount, - groupName: null, order: 0, id: '3', unread: true, @@ -322,7 +319,6 @@ export const mockEnterpriseNotifications: Notification[] = [ }, { account: mockGitHubEnterpriseServerAccount, - groupName: null, order: 1, id: '4', unread: true, From 5610c7513b135fe8be4c310963c0b07f69fa82b8 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Wed, 15 Oct 2025 06:54:28 -0700 Subject: [PATCH 5/5] feat: stabilize notification order during notification interactions Signed-off-by: Adam Setch --- src/renderer/utils/notifications/notifications.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/renderer/utils/notifications/notifications.ts b/src/renderer/utils/notifications/notifications.ts index 4df75b76d..62b92cb0c 100644 --- a/src/renderer/utils/notifications/notifications.ts +++ b/src/renderer/utils/notifications/notifications.ts @@ -146,6 +146,13 @@ export async function enrichNotification( }; } +/** + * Assign an order property to each notification to stabilize how they are displayed + * during notification interaction events (mark as read, mark as done, etc.) + * + * @param notifications + * @param settings + */ export function stabilizeNotificationsOrder( notifications: AccountNotifications[], settings: SettingsState,