Skip to content

refactor: extract pill section into it's own component #1296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 25, 2024
Merged

refactor: extract pill section into it's own component #1296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f45645d
refactor: extract pill section into it's own component
setchy Jun 20, 2024
7fbc4d2
refactor
setchy Jun 21, 2024
a468583
Merge branch 'main' into refactor/pills-component
setchy Jun 21, 2024
b735f4b
update snapshot
setchy Jun 21, 2024
254e1bc
Merge branch 'main' into refactor/pills-component
setchy Jun 21, 2024
bc4f2cc
Merge branch 'main' into refactor/pills-component
setchy Jun 22, 2024
ea5eb63
Merge branch 'main' into refactor/pills-component
setchy Jun 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 1 addition & 244 deletions src/components/NotificationRow.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '../__mocks__/state-mocks';
import { AppContext } from '../context/App';
import { GroupBy, type Link } from '../types';
import type { Milestone, UserType } from '../typesGitHub';
import type { UserType } from '../typesGitHub';
import { mockSingleNotification } from '../utils/api/__mocks__/response-mocks';
import * as comms from '../utils/comms';
import * as links from '../utils/links';
Expand Down Expand Up @@ -103,249 +103,6 @@ describe('components/NotificationRow.tsx', () => {
expect(tree).toMatchSnapshot();
});

describe('notification pills / metrics', () => {
describe('showPills disabled', () => {
it('should not render any pills when showPills is disabled', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.linkedIssues = ['#1'];

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: { ...mockSettings, showPills: false },
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});
});

describe('linked issue pills', () => {
it('should render issues pill when linked to one issue/pr', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.linkedIssues = ['#1'];

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});

it('should render issues pill when linked to multiple issues/prs', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.linkedIssues = ['#1', '#2'];

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});
});

describe('comment pills', () => {
it('should render when no comments', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.comments = null;

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});

it('should render when 1 comment', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.comments = 1;

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});

it('should render when more than 1 comments', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.comments = 2;

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});
});

describe('label pills', () => {
it('should render labels pill', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.labels = ['enhancement', 'good-first-issue'];

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});
});

describe('milestone pills', () => {
it('should render open milestone pill', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.milestone = {
title: 'Milestone 1',
state: 'open',
} as Milestone;

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});

it('should render closed milestone pill', async () => {
jest
.spyOn(global.Date, 'now')
.mockImplementation(() => new Date('2024').valueOf());

const mockNotification = mockSingleNotification;
mockNotification.subject.milestone = {
title: 'Milestone 1',
state: 'closed',
} as Milestone;

const props = {
notification: mockNotification,
account: mockGitHubCloudAccount,
};

const tree = render(
<AppContext.Provider
value={{
settings: mockSettings,
}}
>
<NotificationRow {...props} />
</AppContext.Provider>,
);
expect(tree).toMatchSnapshot();
});
});
});

describe('notification interactions', () => {
it('should open a notification in the browser - click', () => {
const removeNotificationFromState = jest.fn();
Expand Down
76 changes: 2 additions & 74 deletions src/components/NotificationRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import {
BellSlashIcon,
CheckIcon,
CommentIcon,
FeedPersonIcon,
IssueClosedIcon,
MilestoneIcon,
ReadIcon,
TagIcon,
} from '@primer/octicons-react';
import {
type FC,
Expand All @@ -26,15 +22,14 @@ import {
import {
getNotificationTypeIcon,
getNotificationTypeIconColor,
getPullRequestReviewIcon,
} from '../utils/icons';
import { openNotification, openUserProfile } from '../utils/links';
import { formatReason } from '../utils/reason';
import { HoverGroup } from './HoverGroup';
import { InteractionButton } from './buttons/InteractionButton';
import { PillButton } from './buttons/PillButton';
import { AvatarIcon } from './icons/AvatarIcon';
import { NotificationHeader } from './notification/NotificationHeader';
import { Pills } from './notification/Pills';

interface INotificationRow {
notification: Notification;
Expand Down Expand Up @@ -92,18 +87,6 @@ export const NotificationRow: FC<INotificationRow> = ({
notification.subject.type,
]);

const commentsPillDescription = `${notification.subject.comments} ${
notification.subject.comments > 1 ? 'comments' : 'comment'
}`;

const labelsPillDescription = notification.subject.labels
?.map((label) => `🏷️ ${label}`)
.join('\n');

const linkedIssuesPillDescription = `Linked to ${
notification.subject.linkedIssues?.length > 1 ? 'issues' : 'issue'
} ${notification.subject?.linkedIssues?.join(', ')}`;

const groupByDate = settings.groupBy === 'DATE';

return (
Expand Down Expand Up @@ -171,62 +154,7 @@ export const NotificationRow: FC<INotificationRow> = ({
)}
<div title={reason.description}>{reason.title}</div>
<div title={updatedLabel}>{updatedAt}</div>
{settings.showPills && (
<div className="flex">
{notification.subject?.linkedIssues?.length > 0 && (
<PillButton
title={linkedIssuesPillDescription}
metric={notification.subject.linkedIssues.length}
icon={IssueClosedIcon}
color={IconColor.GREEN}
/>
)}

{notification.subject.reviews?.map((review) => {
const icon = getPullRequestReviewIcon(review);
if (!icon) {
return null;
}

return (
<PillButton
key={review.state}
title={icon.description}
metric={review.users.length}
icon={icon.type}
color={icon.color}
/>
);
})}
{notification.subject?.comments > 0 && (
<PillButton
title={commentsPillDescription}
metric={notification.subject.comments}
icon={CommentIcon}
color={IconColor.GRAY}
/>
)}
{notification.subject?.labels?.length > 0 && (
<PillButton
title={labelsPillDescription}
metric={notification.subject.labels.length}
icon={TagIcon}
color={IconColor.GRAY}
/>
)}
{notification.subject.milestone && (
<PillButton
title={notification.subject.milestone.title}
icon={MilestoneIcon}
color={
notification.subject.milestone.state === 'open'
? IconColor.GREEN
: IconColor.RED
}
/>
)}
</div>
)}
<Pills notification={notification} />
</div>
</div>

Expand Down
Loading