Skip to content

Commit

Permalink
feat: color coding for PRs and issues (#565)
Browse files Browse the repository at this point in the history
  • Loading branch information
@afonsojramos
afonsojramos committed Sep 15, 2023
1 parent a37814a commit 4328c7f
Show file tree
Hide file tree
Showing 14 changed files with 255 additions and 27 deletions.
1 change: 1 addition & 0 deletions src/__mocks__/mock-state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ export const mockSettings: SettingsState = {
markOnClick: false,
openAtStartup: false,
appearance: Appearance.SYSTEM,
colors: false,
};
1 change: 1 addition & 0 deletions src/__mocks__/mockedData.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const mockedSingleNotification: Notification = {
url: 'https://api.github.com/repos/manosim/notifications-test/issues/1',
latest_comment_url: 'https://api.github.com/repos/manosim/notifications-test/issues/comments/302888448',
type: 'Issue',
state: 'open',
},
repository: {
id: 57216596,
Expand Down
12 changes: 10 additions & 2 deletions src/components/NotificationRow.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import React, { useCallback, useContext } from 'react';
import { formatDistanceToNow, parseISO } from 'date-fns';
import { CheckIcon, MuteIcon } from '@primer/octicons-react';

import { formatReason, getNotificationTypeIcon } from '../utils/github-api';
import {
formatReason,
getNotificationTypeIcon,
getNotificationTypeIconColor,
} from '../utils/github-api';
import { openInBrowser } from '../utils/helpers';
import { Notification } from '../typesGithub';
import { AppContext } from '../context/App';
Expand Down Expand Up @@ -41,13 +45,17 @@ export const NotificationRow: React.FC<IProps> = ({

const reason = formatReason(notification.reason);
const NotificationIcon = getNotificationTypeIcon(notification.subject.type);
const iconColor = getNotificationTypeIconColor(notification.subject.state);
const realIconColor = settings
? (settings.colors && iconColor) || ''
: iconColor;
const updatedAt = formatDistanceToNow(parseISO(notification.updated_at), {
addSuffix: true,
});

return (
<div className="flex space-x-2 p-2 bg-white dark:bg-gray-dark dark:text-white hover:bg-gray-100 dark:hover:bg-gray-darker border-b border-gray-100 dark:border-gray-darker">
<div className="flex justify-center items-center w-8">
<div className={`flex justify-center items-center w-8 ${realIconColor}`}>
<NotificationIcon size={18} aria-label={notification.subject.type} />
</div>

Expand Down
2 changes: 1 addition & 1 deletion src/components/__snapshots__/NotificationRow.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ exports[`components/Notification.js should render itself & its children 1`] = `
className="flex space-x-2 p-2 bg-white dark:bg-gray-dark dark:text-white hover:bg-gray-100 dark:hover:bg-gray-darker border-b border-gray-100 dark:border-gray-darker"
>
<div
className="flex justify-center items-center w-8"
className="flex justify-center items-center w-8 text-green-500"
>
<svg
aria-hidden="false"
Expand Down
2 changes: 2 additions & 0 deletions src/context/App.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ describe('context/App.tsx', () => {
participating: true,
playSound: true,
showNotifications: true,
colors: true,
}
);
});
Expand Down Expand Up @@ -289,6 +290,7 @@ describe('context/App.tsx', () => {
participating: false,
playSound: true,
showNotifications: true,
colors: true,
}
);
});
Expand Down
4 changes: 3 additions & 1 deletion src/context/App.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const defaultSettings: SettingsState = {
markOnClick: false,
openAtStartup: false,
appearance: Appearance.SYSTEM,
colors: true,
};

interface AppContextState {
Expand Down Expand Up @@ -72,7 +73,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
markNotification,
unsubscribeNotification,
markRepoNotifications,
} = useNotifications();
} = useNotifications(settings.colors);

useEffect(() => {
restoreSettings();
Expand Down Expand Up @@ -161,6 +162,7 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {

if (existing.settings) {
setSettings({ ...defaultSettings, ...existing.settings });
return existing.settings;
}
}, []);

Expand Down
125 changes: 107 additions & 18 deletions src/hooks/useNotifications.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200, notifications);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down Expand Up @@ -59,7 +59,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400, { message });

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down Expand Up @@ -92,7 +92,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200, notifications);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -118,7 +118,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400, { message: 'Oops! Something went wrong.' });

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down Expand Up @@ -149,7 +149,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200, notifications);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -173,7 +173,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400, { message: 'Oops! Something went wrong.' });

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -185,6 +185,95 @@ describe('hooks/useNotifications.ts', () => {
expect(result.current.requestFailed).toBe(true);
});
});

describe('with colors', () => {
it('should fetch notifications with success - with colors', async () => {
const accounts: AuthState = {
...mockAccounts,
enterpriseAccounts: [],
user: mockedUser,
};

const notifications = [
{
id: 1,
title: 'This is a notification.',
subject: { type: 'Issue', url: 'https://api.github.com/1' },
},
{
id: 2,
title: 'A merged PR.',
subject: { type: 'PullRequest', url: 'https://api.github.com/2' },
},
{
id: 3,
title: 'A closed PR.',
subject: { type: 'PullRequest', url: 'https://api.github.com/3' },
},
{
id: 4,
title: 'A draft PR.',
subject: { type: 'PullRequest', url: 'https://api.github.com/4' },
},
{
id: 5,
title: 'A draft PR.',
subject: { type: 'PullRequest', url: 'https://api.github.com/5' },
},
];

nock('https://api.github.com')
.get('/notifications?participating=false')
.reply(200, notifications);

nock('https://api.github.com').get('/1').reply(200, { state: 'open' });
nock('https://api.github.com')
.get('/2')
.reply(200, { state: 'closed', merged: true });
nock('https://api.github.com')
.get('/3')
.reply(200, { state: 'closed', merged: false });
nock('https://api.github.com')
.get('/4')
.reply(200, { state: 'open', draft: false });
nock('https://api.github.com')
.get('/5')
.reply(200, { state: 'open', draft: true });

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications(true)
);

act(() => {
result.current.fetchNotifications(accounts, {
...mockSettings,
colors: true,
});
});

expect(result.current.isFetching).toBe(true);

await waitForNextUpdate();

expect(result.current.notifications[0].hostname).toBe('github.com');
expect(result.current.notifications[0].notifications.length).toBe(5);
expect(
result.current.notifications[0].notifications[0].subject.state
).toBe('open');
expect(
result.current.notifications[0].notifications[1].subject.state
).toBe('merged');
expect(
result.current.notifications[0].notifications[2].subject.state
).toBe('closed');
expect(
result.current.notifications[0].notifications[3].subject.state
).toBe('open');
expect(
result.current.notifications[0].notifications[4].subject.state
).toBe('draft');
});
});
});

describe('markNotification', () => {
Expand All @@ -200,7 +289,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -218,7 +307,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -241,7 +330,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -259,7 +348,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down Expand Up @@ -292,7 +381,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200);

const { result, waitForValueToChange } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -318,7 +407,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400);

const { result, waitForValueToChange } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down Expand Up @@ -349,7 +438,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200);

const { result, waitForValueToChange } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -375,7 +464,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400);

const { result, waitForValueToChange } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down Expand Up @@ -404,7 +493,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -422,7 +511,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -445,7 +534,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(200);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand All @@ -463,7 +552,7 @@ describe('hooks/useNotifications.ts', () => {
.reply(400);

const { result, waitForNextUpdate } = renderHook(() =>
useNotifications()
useNotifications(false)
);

act(() => {
Expand Down
Loading

0 comments on commit 4328c7f

Please sign in to comment.