Repository: gitify-app/gitify
Pull request #748 (2024-02-09 18:36:40)

feat: better notification type icons
## Context

Make Gitify icons match Github icons

## Discussion

I had to pass the `state` parameter to `getNotificationTypeIcon` because I needed this context to find the correct icon. I made it optional because it felt more accurate.

I tried to find all different states/icons but I might be missing some; if so, I default to the original icon, so there shouldn't be any regression

## Before / After example

Before:
<img width="46" alt="image" src="https://github.com/gitify-app/gitify/assets/26418696/1cbe313f-b4a2-4d2f-8ad8-438d9218428e">

After:
<img width="46" alt="image" src="https://github.com/gitify-app/gitify/assets/26418696/599c392d-eeec-40c5-b3f3-c74ad3c3baf0">



Patch:
diff --git a/src/components/NotificationRow.tsx b/src/components/NotificationRow.tsx
index b5ab1026..732bd569 100644
--- a/src/components/NotificationRow.tsx
+++ b/src/components/NotificationRow.tsx
@@ -49,7 +49,10 @@ export const NotificationRow: React.FC<IProps> = ({
   };
 
   const reason = formatReason(notification.reason);
-  const NotificationIcon = getNotificationTypeIcon(notification.subject.type);
+  const NotificationIcon = getNotificationTypeIcon(
+    notification.subject.type,
+    notification.subject.state,
+  );
   const iconColor = getNotificationTypeIconColor(notification.subject.state);
   const realIconColor = settings
     ? (settings.colors && iconColor) || ''
diff --git a/src/utils/github-api.ts b/src/utils/github-api.ts
index 8a0a5d76..462f39e7 100644
--- a/src/utils/github-api.ts
+++ b/src/utils/github-api.ts
@@ -2,8 +2,14 @@ import {
   AlertIcon,
   CommentDiscussionIcon,
   GitCommitIcon,
+  GitMergeIcon,
+  GitPullRequestClosedIcon,
+  GitPullRequestDraftIcon,
   GitPullRequestIcon,
+  IssueClosedIcon,
+  IssueDraftIcon,
   IssueOpenedIcon,
+  IssueReopenedIcon,
   MailIcon,
   OcticonProps,
   QuestionIcon,
@@ -66,6 +72,7 @@ export function formatReason(reason: Reason): {
 
 export function getNotificationTypeIcon(
   type: SubjectType,
+  state?: StateType,
 ): React.FC<OcticonProps> {
   switch (type) {
     case 'CheckSuite':
@@ -75,9 +82,28 @@ export function getNotificationTypeIcon(
     case 'Discussion':
       return CommentDiscussionIcon;
     case 'Issue':
-      return IssueOpenedIcon;
+      switch (state) {
+        case 'draft':
+          return IssueDraftIcon;
+        case 'closed':
+        case 'completed':
+          return IssueClosedIcon;
+        case 'reopened':
+          return IssueReopenedIcon;
+        default:
+          return IssueOpenedIcon;
+      }
     case 'PullRequest':
-      return GitPullRequestIcon;
+      switch (state) {
+        case 'draft':
+          return GitPullRequestDraftIcon;
+        case 'closed':
+          return GitPullRequestClosedIcon;
+        case 'merged':
+          return GitMergeIcon;
+        default:
+          return GitPullRequestIcon;
+      }
     case 'Release':
       return TagIcon;
     case 'RepositoryInvitation':


Test patch:
diff --git a/src/utils/github-api.test.ts b/src/utils/github-api.test.ts
index 73a7ef44..e1de0cee 100644
--- a/src/utils/github-api.test.ts
+++ b/src/utils/github-api.test.ts
@@ -27,9 +27,30 @@ describe('./utils/github-api.ts', () => {
     expect(getNotificationTypeIcon('Issue').displayName).toBe(
       'IssueOpenedIcon',
     );
+    expect(getNotificationTypeIcon('Issue', 'draft').displayName).toBe(
+      'IssueDraftIcon',
+    );
+    expect(getNotificationTypeIcon('Issue', 'closed').displayName).toBe(
+      'IssueClosedIcon',
+    );
+    expect(getNotificationTypeIcon('Issue', 'completed').displayName).toBe(
+      'IssueClosedIcon',
+    );
+    expect(getNotificationTypeIcon('Issue', 'reopened').displayName).toBe(
+      'IssueReopenedIcon',
+    );
     expect(getNotificationTypeIcon('PullRequest').displayName).toBe(
       'GitPullRequestIcon',
     );
+    expect(getNotificationTypeIcon('PullRequest', 'draft').displayName).toBe(
+      'GitPullRequestDraftIcon',
+    );
+    expect(getNotificationTypeIcon('PullRequest', 'closed').displayName).toBe(
+      'GitPullRequestClosedIcon',
+    );
+    expect(getNotificationTypeIcon('PullRequest', 'merged').displayName).toBe(
+      'GitMergeIcon',
+    );
     expect(getNotificationTypeIcon('Release').displayName).toBe('TagIcon');
     expect(
       getNotificationTypeIcon('RepositoryVulnerabilityAlert').displayName,