Merge branch 'main' into feat/avatar-with-fallback

Signed-off-by: Adam Setch <[email protected]>

Author: setchy

src/renderer/components/notifications/NotificationHeader.test.tsx

@@ -26,19 +26,60 @@ describe('renderer/components/notifications/NotificationHeader.tsx', () => {
     expect(tree).toMatchSnapshot();
   });
 
-  it('should render itself & its children - group by date', async () => {
-    const props = {
-      notification: mockSingleNotification,
-    };
+  describe('should render itself & its children - group by date', () => {
+    it('with notification number', async () => {
+      const props = {
+        notification: mockSingleNotification,
+      };
 
-    const tree = render(
-      <AppContext.Provider
-        value={{ settings: { ...mockSettings, groupBy: GroupBy.DATE } }}
-      >
-        <NotificationHeader {...props} />
-      </AppContext.Provider>,
-    );
-    expect(tree).toMatchSnapshot();
+      const tree = render(
+        <AppContext.Provider
+          value={{ settings: { ...mockSettings, groupBy: GroupBy.DATE } }}
+        >
+          <NotificationHeader {...props} />
+        </AppContext.Provider>,
+      );
+      expect(tree).toMatchSnapshot();
+    });
+
+    it('with showNumber setting disabled', async () => {
+      const props = {
+        notification: mockSingleNotification,
+      };
+
+      const tree = render(
+        <AppContext.Provider
+          value={{
+            settings: {
+              ...mockSettings,
+              showNumber: false,
+              groupBy: GroupBy.DATE,
+            },
+          }}
+        >
+          <NotificationHeader {...props} />
+        </AppContext.Provider>,
+      );
+      expect(tree).toMatchSnapshot();
+    });
+
+    it('without notification number', async () => {
+      const props = {
+        notification: {
+          ...mockSingleNotification,
+          subject: { ...mockSingleNotification.subject, number: null },
+        },
+      };
+
+      const tree = render(
+        <AppContext.Provider
+          value={{ settings: { ...mockSettings, groupBy: GroupBy.DATE } }}
+        >
+          <NotificationHeader {...props} />
+        </AppContext.Provider>,
+      );
+      expect(tree).toMatchSnapshot();
+    });
   });
 
   it('should open notification user profile - group by date', () => {

src/renderer/components/notifications/NotificationHeader.tsx

@@ -1,10 +1,11 @@
 import { type FC, type MouseEvent, useContext } from 'react';
 
-import { Box, Tooltip } from '@primer/react';
+import { Box, Stack, Tooltip } from '@primer/react';
 
 import { AppContext } from '../../context/App';
-import { Size } from '../../types';
+import { GroupBy, Opacity, Size } from '../../types';
 import type { Notification } from '../../typesGitHub';
+import { cn } from '../../utils/cn';
 import { openRepository } from '../../utils/links';
 import { AvatarWithFallback } from '../avatars/AvatarWithFallback';
 
@@ -19,29 +20,44 @@ export const NotificationHeader: FC<INotificationHeader> = ({
 
   const repoSlug = notification.repository.full_name;
 
-  const groupByDate = settings.groupBy === 'DATE';
+  const notificationNumber = notification.subject?.number
+    ? `#${notification.subject.number}`
+    : '';
+
+  const groupByDate = settings.groupBy === GroupBy.DATE;
 
   return (
     groupByDate && (
-      <Tooltip text={`View repository: ${repoSlug}`} direction="se">
+      <Stack direction="horizontal" align="center" gap="condensed">
+        <Tooltip text={`View repository: ${repoSlug}`} direction="se">
+          <Box
+            className="text-xs font-medium"
+            onClick={(event: MouseEvent<HTMLElement>) => {
+              // Don't trigger onClick of parent element.
+              event.stopPropagation();
+              openRepository(notification.repository);
+            }}
+            data-testid="view-repository"
+          >
+            <AvatarWithFallback
+              src={notification.repository.owner.avatar_url}
+              alt={repoSlug}
+              name={repoSlug}
+              size={Size.SMALL}
+              userType={notification.repository.owner.type}
+            />
+          </Box>
+        </Tooltip>
         <Box
-          className="text-xs font-medium"
-          onClick={(event: MouseEvent<HTMLElement>) => {
-            // Don't trigger onClick of parent element.
-            event.stopPropagation();
-            openRepository(notification.repository);
-          }}
-          data-testid="view-repository"
+          className={cn(
+            'text-xxs',
+            Opacity.READ,
+            !settings.showNumber && 'hidden',
+          )}
         >
-          <AvatarWithFallback
-            src={notification.repository.owner.avatar_url}
-            alt={repoSlug}
-            name={repoSlug}
-            size={Size.SMALL}
-            userType={notification.repository.owner.type}
-          />
+          {notificationNumber}
         </Box>
-      </Tooltip>
+      </Stack>
     )
   );
 };

src/renderer/components/notifications/NotificationRow.tsx

@@ -10,7 +10,7 @@ import { BellSlashIcon, CheckIcon, ReadIcon } from '@primer/octicons-react';
 import { IconButton, Tooltip } from '@primer/react';
 
 import { AppContext } from '../../context/App';
-import { Opacity, Size } from '../../types';
+import { GroupBy, Opacity, Size } from '../../types';
 import type { Notification } from '../../typesGitHub';
 import { cn } from '../../utils/cn';
 import { isMarkAsDoneFeatureSupported } from '../../utils/features';
@@ -84,6 +84,8 @@ export const NotificationRow: FC<INotificationRow> = ({
   const notificationTitle =
     `${notification.subject.title} ${notificationNumber}`.trim();
 
+  const groupByDate = settings.groupBy === GroupBy.DATE;
+
   return (
     <div
       id={notification.id}
@@ -116,7 +118,7 @@ export const NotificationRow: FC<INotificationRow> = ({
             className={cn(
               'text-xxs',
               Opacity.READ,
-              !settings.showNumber && 'hidden',
+              (groupByDate || !settings.showNumber) && 'hidden',
             )}
           >
             {notificationNumber}