diff --git a/package.json b/package.json index 91d442f4b..56ca76411 100644 --- a/package.json +++ b/package.json @@ -146,7 +146,7 @@ ] }, "lint-staged": { - "*": "biome check --fix --no-errors-on-unmatched", + "*": "biome check --no-errors-on-unmatched", "*.{js,ts,tsx}": "pnpm test --findRelatedTests --passWithNoTests --updateSnapshot" } } diff --git a/src/renderer/components/notifications/NotificationRow.test.tsx b/src/renderer/components/notifications/NotificationRow.test.tsx index 9329a3a77..13c3e2d05 100644 --- a/src/renderer/components/notifications/NotificationRow.test.tsx +++ b/src/renderer/components/notifications/NotificationRow.test.tsx @@ -136,7 +136,7 @@ describe('renderer/components/notifications/NotificationRow.tsx', () => { expect(markNotificationsAsDoneMock).toHaveBeenCalledTimes(1); }); - it('should mark as read (not done) when markAsDoneOnOpen is true but fetchReadNotifications is enabled', async () => { + it('should mark as done when markAsDoneOnOpen is true even with fetchReadNotifications enabled', async () => { const markNotificationsAsReadMock = jest.fn(); const markNotificationsAsDoneMock = jest.fn(); @@ -158,8 +158,8 @@ describe('renderer/components/notifications/NotificationRow.tsx', () => { await userEvent.click(screen.getByTestId('notification-row')); expect(links.openNotification).toHaveBeenCalledTimes(1); - expect(markNotificationsAsReadMock).toHaveBeenCalledTimes(1); - expect(markNotificationsAsDoneMock).not.toHaveBeenCalled(); + expect(markNotificationsAsDoneMock).toHaveBeenCalledTimes(1); + expect(markNotificationsAsReadMock).not.toHaveBeenCalled(); }); it('should mark notifications as read', async () => { @@ -216,7 +216,25 @@ describe('renderer/components/notifications/NotificationRow.tsx', () => { expect(markNotificationsAsDoneMock).toHaveBeenCalledTimes(1); }); - it('should hide mark as done button when fetchReadNotifications is enabled', async () => { + it('should hide mark as done button when notification is already read', async () => { + const readNotification = { + ...mockGitifyNotification, + unread: false, + }; + + const props = { + notification: readNotification, + account: mockGitHubCloudAccount, + }; + + renderWithAppContext(); + + expect( + screen.queryByTestId('notification-mark-as-done'), + ).not.toBeInTheDocument(); + }); + + it('should show mark as done button when fetchReadNotifications is enabled and notification is unread', async () => { const props = { notification: mockGitifyNotification, account: mockGitHubCloudAccount, @@ -227,8 +245,8 @@ describe('renderer/components/notifications/NotificationRow.tsx', () => { }); expect( - screen.queryByTestId('notification-mark-as-done'), - ).not.toBeInTheDocument(); + screen.getByTestId('notification-mark-as-done'), + ).toBeInTheDocument(); expect( screen.getByTestId('notification-mark-as-read'), ).toBeInTheDocument(); diff --git a/src/renderer/components/notifications/NotificationRow.tsx b/src/renderer/components/notifications/NotificationRow.tsx index 02f1e480e..54da4bfdd 100644 --- a/src/renderer/components/notifications/NotificationRow.tsx +++ b/src/renderer/components/notifications/NotificationRow.tsx @@ -9,6 +9,7 @@ import { cn } from '../../utils/cn'; import { isMarkAsDoneFeatureSupported } from '../../utils/features'; import { openNotification } from '../../utils/links'; import { isGroupByDate } from '../../utils/notifications/group'; +import { shouldRemoveNotificationsFromState } from '../../utils/notifications/remove'; import { HoverButton } from '../primitives/HoverButton'; import { HoverGroup } from '../primitives/HoverGroup'; import { NotificationFooter } from './NotificationFooter'; @@ -31,17 +32,13 @@ export const NotificationRow: FC = ({ } = useAppContext(); const [animateExit, setAnimateExit] = useState(false); + const shouldAnimateExit = shouldRemoveNotificationsFromState(settings); + const handleNotification = useCallback(() => { - // Don't animate exit when fetchReadNotifications is enabled - // as notifications will stay in the list with reduced opacity - const shouldAnimateExit = - !settings.delayNotificationState && !settings.fetchReadNotifications; setAnimateExit(shouldAnimateExit); openNotification(notification); - // Fall back to mark as read when fetchReadNotifications is enabled - // since marking as done won't work correctly (API limitation) - if (settings.markAsDoneOnOpen && !settings.fetchReadNotifications) { + if (settings.markAsDoneOnOpen) { markNotificationsAsDone([notification]); } else { markNotificationsAsRead([notification]); @@ -51,19 +48,16 @@ export const NotificationRow: FC = ({ markNotificationsAsRead, markNotificationsAsDone, settings, + shouldAnimateExit, ]); const actionMarkAsDone = () => { - setAnimateExit(!settings.delayNotificationState); + setAnimateExit(shouldAnimateExit); markNotificationsAsDone([notification]); }; const actionMarkAsRead = () => { - // Don't animate exit when fetchReadNotifications is enabled - // as the notification will stay in the list with reduced opacity - if (!settings.fetchReadNotifications) { - setAnimateExit(!settings.delayNotificationState); - } + setAnimateExit(shouldAnimateExit); markNotificationsAsRead([notification]); }; @@ -152,7 +146,7 @@ export const NotificationRow: FC = ({ action={actionMarkAsDone} enabled={ isMarkAsDoneFeatureSupported(notification.account) && - !settings.fetchReadNotifications + notification.unread } icon={CheckIcon} label="Mark as done" diff --git a/src/renderer/components/notifications/RepositoryNotifications.tsx b/src/renderer/components/notifications/RepositoryNotifications.tsx index 60d622ef1..7a9ef26e6 100644 --- a/src/renderer/components/notifications/RepositoryNotifications.tsx +++ b/src/renderer/components/notifications/RepositoryNotifications.tsx @@ -9,6 +9,7 @@ import { cn } from '../../utils/cn'; import { isMarkAsDoneFeatureSupported } from '../../utils/features'; import { getChevronDetails } from '../../utils/helpers'; import { openRepository } from '../../utils/links'; +import { shouldRemoveNotificationsFromState } from '../../utils/notifications/remove'; import { AvatarWithFallback } from '../avatars/AvatarWithFallback'; import { HoverButton } from '../primitives/HoverButton'; import { HoverGroup } from '../primitives/HoverGroup'; @@ -30,18 +31,15 @@ export const RepositoryNotifications: FC = ({ useState(true); const avatarUrl = repoNotifications[0].repository.owner.avatarUrl; + const shouldAnimateExit = shouldRemoveNotificationsFromState(settings); const actionMarkAsDone = () => { - setAnimateExit(!settings.delayNotificationState); + setAnimateExit(shouldAnimateExit); markNotificationsAsDone(repoNotifications); }; const actionMarkAsRead = () => { - // Don't animate exit when fetchReadNotifications is enabled - // as the notifications will stay in the list with reduced opacity - if (!settings.fetchReadNotifications) { - setAnimateExit(!settings.delayNotificationState); - } + setAnimateExit(shouldAnimateExit); markNotificationsAsRead(repoNotifications); }; @@ -107,7 +105,7 @@ export const RepositoryNotifications: FC = ({ action={actionMarkAsDone} enabled={ isMarkAsDoneFeatureSupported(repoNotifications[0].account) && - !settings.fetchReadNotifications + !areAllRepoNotificationsRead } icon={CheckIcon} label="Mark repository as done" diff --git a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap index 5791dca2b..95d680116 100644 --- a/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap +++ b/src/renderer/components/notifications/__snapshots__/NotificationRow.test.tsx.snap @@ -2710,35 +2710,6 @@ exports[`renderer/components/notifications/NotificationRow.tsx should render its data-padding="none" data-wrap="nowrap" > -