Skip to content

Commit 1770886

Browse files
authored
refactor: extract pill section into it's own component (#1296)
1 parent fd24b40 commit 1770886

File tree

6 files changed

+2481
-4949
lines changed

6 files changed

+2481
-4949
lines changed

src/components/NotificationRow.test.tsx

Lines changed: 1 addition & 244 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
} from '../__mocks__/state-mocks';
77
import { AppContext } from '../context/App';
88
import { GroupBy, type Link } from '../types';
9-
import type { Milestone, UserType } from '../typesGitHub';
9+
import type { UserType } from '../typesGitHub';
1010
import { mockSingleNotification } from '../utils/api/__mocks__/response-mocks';
1111
import * as comms from '../utils/comms';
1212
import * as links from '../utils/links';
@@ -103,249 +103,6 @@ describe('components/NotificationRow.tsx', () => {
103103
expect(tree).toMatchSnapshot();
104104
});
105105

106-
describe('notification pills / metrics', () => {
107-
describe('showPills disabled', () => {
108-
it('should not render any pills when showPills is disabled', async () => {
109-
jest
110-
.spyOn(global.Date, 'now')
111-
.mockImplementation(() => new Date('2024').valueOf());
112-
113-
const mockNotification = mockSingleNotification;
114-
mockNotification.subject.linkedIssues = ['#1'];
115-
116-
const props = {
117-
notification: mockNotification,
118-
account: mockGitHubCloudAccount,
119-
};
120-
121-
const tree = render(
122-
<AppContext.Provider
123-
value={{
124-
settings: { ...mockSettings, showPills: false },
125-
}}
126-
>
127-
<NotificationRow {...props} />
128-
</AppContext.Provider>,
129-
);
130-
expect(tree).toMatchSnapshot();
131-
});
132-
});
133-
134-
describe('linked issue pills', () => {
135-
it('should render issues pill when linked to one issue/pr', async () => {
136-
jest
137-
.spyOn(global.Date, 'now')
138-
.mockImplementation(() => new Date('2024').valueOf());
139-
140-
const mockNotification = mockSingleNotification;
141-
mockNotification.subject.linkedIssues = ['#1'];
142-
143-
const props = {
144-
notification: mockNotification,
145-
account: mockGitHubCloudAccount,
146-
};
147-
148-
const tree = render(
149-
<AppContext.Provider
150-
value={{
151-
settings: mockSettings,
152-
}}
153-
>
154-
<NotificationRow {...props} />
155-
</AppContext.Provider>,
156-
);
157-
expect(tree).toMatchSnapshot();
158-
});
159-
160-
it('should render issues pill when linked to multiple issues/prs', async () => {
161-
jest
162-
.spyOn(global.Date, 'now')
163-
.mockImplementation(() => new Date('2024').valueOf());
164-
165-
const mockNotification = mockSingleNotification;
166-
mockNotification.subject.linkedIssues = ['#1', '#2'];
167-
168-
const props = {
169-
notification: mockNotification,
170-
account: mockGitHubCloudAccount,
171-
};
172-
173-
const tree = render(
174-
<AppContext.Provider
175-
value={{
176-
settings: mockSettings,
177-
}}
178-
>
179-
<NotificationRow {...props} />
180-
</AppContext.Provider>,
181-
);
182-
expect(tree).toMatchSnapshot();
183-
});
184-
});
185-
186-
describe('comment pills', () => {
187-
it('should render when no comments', async () => {
188-
jest
189-
.spyOn(global.Date, 'now')
190-
.mockImplementation(() => new Date('2024').valueOf());
191-
192-
const mockNotification = mockSingleNotification;
193-
mockNotification.subject.comments = null;
194-
195-
const props = {
196-
notification: mockNotification,
197-
account: mockGitHubCloudAccount,
198-
};
199-
200-
const tree = render(
201-
<AppContext.Provider
202-
value={{
203-
settings: mockSettings,
204-
}}
205-
>
206-
<NotificationRow {...props} />
207-
</AppContext.Provider>,
208-
);
209-
expect(tree).toMatchSnapshot();
210-
});
211-
212-
it('should render when 1 comment', async () => {
213-
jest
214-
.spyOn(global.Date, 'now')
215-
.mockImplementation(() => new Date('2024').valueOf());
216-
217-
const mockNotification = mockSingleNotification;
218-
mockNotification.subject.comments = 1;
219-
220-
const props = {
221-
notification: mockNotification,
222-
account: mockGitHubCloudAccount,
223-
};
224-
225-
const tree = render(
226-
<AppContext.Provider
227-
value={{
228-
settings: mockSettings,
229-
}}
230-
>
231-
<NotificationRow {...props} />
232-
</AppContext.Provider>,
233-
);
234-
expect(tree).toMatchSnapshot();
235-
});
236-
237-
it('should render when more than 1 comments', async () => {
238-
jest
239-
.spyOn(global.Date, 'now')
240-
.mockImplementation(() => new Date('2024').valueOf());
241-
242-
const mockNotification = mockSingleNotification;
243-
mockNotification.subject.comments = 2;
244-
245-
const props = {
246-
notification: mockNotification,
247-
account: mockGitHubCloudAccount,
248-
};
249-
250-
const tree = render(
251-
<AppContext.Provider
252-
value={{
253-
settings: mockSettings,
254-
}}
255-
>
256-
<NotificationRow {...props} />
257-
</AppContext.Provider>,
258-
);
259-
expect(tree).toMatchSnapshot();
260-
});
261-
});
262-
263-
describe('label pills', () => {
264-
it('should render labels pill', async () => {
265-
jest
266-
.spyOn(global.Date, 'now')
267-
.mockImplementation(() => new Date('2024').valueOf());
268-
269-
const mockNotification = mockSingleNotification;
270-
mockNotification.subject.labels = ['enhancement', 'good-first-issue'];
271-
272-
const props = {
273-
notification: mockNotification,
274-
account: mockGitHubCloudAccount,
275-
};
276-
277-
const tree = render(
278-
<AppContext.Provider
279-
value={{
280-
settings: mockSettings,
281-
}}
282-
>
283-
<NotificationRow {...props} />
284-
</AppContext.Provider>,
285-
);
286-
expect(tree).toMatchSnapshot();
287-
});
288-
});
289-
290-
describe('milestone pills', () => {
291-
it('should render open milestone pill', async () => {
292-
jest
293-
.spyOn(global.Date, 'now')
294-
.mockImplementation(() => new Date('2024').valueOf());
295-
296-
const mockNotification = mockSingleNotification;
297-
mockNotification.subject.milestone = {
298-
title: 'Milestone 1',
299-
state: 'open',
300-
} as Milestone;
301-
302-
const props = {
303-
notification: mockNotification,
304-
account: mockGitHubCloudAccount,
305-
};
306-
307-
const tree = render(
308-
<AppContext.Provider
309-
value={{
310-
settings: mockSettings,
311-
}}
312-
>
313-
<NotificationRow {...props} />
314-
</AppContext.Provider>,
315-
);
316-
expect(tree).toMatchSnapshot();
317-
});
318-
319-
it('should render closed milestone pill', async () => {
320-
jest
321-
.spyOn(global.Date, 'now')
322-
.mockImplementation(() => new Date('2024').valueOf());
323-
324-
const mockNotification = mockSingleNotification;
325-
mockNotification.subject.milestone = {
326-
title: 'Milestone 1',
327-
state: 'closed',
328-
} as Milestone;
329-
330-
const props = {
331-
notification: mockNotification,
332-
account: mockGitHubCloudAccount,
333-
};
334-
335-
const tree = render(
336-
<AppContext.Provider
337-
value={{
338-
settings: mockSettings,
339-
}}
340-
>
341-
<NotificationRow {...props} />
342-
</AppContext.Provider>,
343-
);
344-
expect(tree).toMatchSnapshot();
345-
});
346-
});
347-
});
348-
349106
describe('notification interactions', () => {
350107
it('should open a notification in the browser - click', () => {
351108
const removeNotificationFromState = jest.fn();

src/components/NotificationRow.tsx

Lines changed: 2 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
import {
22
BellSlashIcon,
33
CheckIcon,
4-
CommentIcon,
54
FeedPersonIcon,
6-
IssueClosedIcon,
7-
MilestoneIcon,
85
ReadIcon,
9-
TagIcon,
106
} from '@primer/octicons-react';
117
import {
128
type FC,
@@ -26,15 +22,14 @@ import {
2622
import {
2723
getNotificationTypeIcon,
2824
getNotificationTypeIconColor,
29-
getPullRequestReviewIcon,
3025
} from '../utils/icons';
3126
import { openNotification, openUserProfile } from '../utils/links';
3227
import { formatReason } from '../utils/reason';
3328
import { HoverGroup } from './HoverGroup';
3429
import { InteractionButton } from './buttons/InteractionButton';
35-
import { PillButton } from './buttons/PillButton';
3630
import { AvatarIcon } from './icons/AvatarIcon';
3731
import { NotificationHeader } from './notification/NotificationHeader';
32+
import { Pills } from './notification/Pills';
3833

3934
interface INotificationRow {
4035
notification: Notification;
@@ -92,18 +87,6 @@ export const NotificationRow: FC<INotificationRow> = ({
9287
notification.subject.type,
9388
]);
9489

95-
const commentsPillDescription = `${notification.subject.comments} ${
96-
notification.subject.comments > 1 ? 'comments' : 'comment'
97-
}`;
98-
99-
const labelsPillDescription = notification.subject.labels
100-
?.map((label) => `🏷️ ${label}`)
101-
.join('\n');
102-
103-
const linkedIssuesPillDescription = `Linked to ${
104-
notification.subject.linkedIssues?.length > 1 ? 'issues' : 'issue'
105-
} ${notification.subject?.linkedIssues?.join(', ')}`;
106-
10790
const groupByDate = settings.groupBy === 'DATE';
10891

10992
return (
@@ -171,62 +154,7 @@ export const NotificationRow: FC<INotificationRow> = ({
171154
)}
172155
<div title={reason.description}>{reason.title}</div>
173156
<div title={updatedLabel}>{updatedAt}</div>
174-
{settings.showPills && (
175-
<div className="flex">
176-
{notification.subject?.linkedIssues?.length > 0 && (
177-
<PillButton
178-
title={linkedIssuesPillDescription}
179-
metric={notification.subject.linkedIssues.length}
180-
icon={IssueClosedIcon}
181-
color={IconColor.GREEN}
182-
/>
183-
)}
184-
185-
{notification.subject.reviews?.map((review) => {
186-
const icon = getPullRequestReviewIcon(review);
187-
if (!icon) {
188-
return null;
189-
}
190-
191-
return (
192-
<PillButton
193-
key={review.state}
194-
title={icon.description}
195-
metric={review.users.length}
196-
icon={icon.type}
197-
color={icon.color}
198-
/>
199-
);
200-
})}
201-
{notification.subject?.comments > 0 && (
202-
<PillButton
203-
title={commentsPillDescription}
204-
metric={notification.subject.comments}
205-
icon={CommentIcon}
206-
color={IconColor.GRAY}
207-
/>
208-
)}
209-
{notification.subject?.labels?.length > 0 && (
210-
<PillButton
211-
title={labelsPillDescription}
212-
metric={notification.subject.labels.length}
213-
icon={TagIcon}
214-
color={IconColor.GRAY}
215-
/>
216-
)}
217-
{notification.subject.milestone && (
218-
<PillButton
219-
title={notification.subject.milestone.title}
220-
icon={MilestoneIcon}
221-
color={
222-
notification.subject.milestone.state === 'open'
223-
? IconColor.GREEN
224-
: IconColor.RED
225-
}
226-
/>
227-
)}
228-
</div>
229-
)}
157+
<Pills notification={notification} />
230158
</div>
231159
</div>
232160

0 commit comments

Comments
 (0)