Skip to content

Commit d817cc8

Browse files
committed
Address review feedback for #6835
- Extract IssueReference type into views.ts and use it for closingIssues (alexr00 review feedback). - Sidebar: drop orphan tailwind-style class names (p-2, gap-2, p-4, text-sm, text-gray-500, text-center, h2) that are not defined in the project's CSS — replace with the project's section conventions. - Use the existing issueIcon / issueClosedIcon for the issue state icon instead of settingsIcon / closeIcon. - Switch on GithubItemStateEnum directly rather than lowercasing strings. - Move the React `key` to the iterating element and key by issue.number (titles are not guaranteed unique). - Render the issue number alongside the title. - Initialize PullRequestModel.closingIssues to [] so the field is never undefined when serialized to the webview.
1 parent df87f4d commit d817cc8

3 files changed

Lines changed: 23 additions & 34 deletions

File tree

src/github/pullRequestModel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
138138
public conflicts?: string[];
139139
public suggestedReviewers?: ISuggestedReviewer[];
140140
public hasChangesSinceLastReview?: boolean;
141-
public closingIssues: IssueReference[];
141+
public closingIssues: IssueReference[] = [];
142142
private _showChangesSinceReview: boolean;
143143
private _hasPendingReview: boolean = false;
144144
private _onDidChangePendingReviewState: vscode.EventEmitter<boolean> = this._register(new vscode.EventEmitter<boolean>());

src/github/views.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ export enum ReviewType {
2828
RequestChanges = 'requestChanges',
2929
}
3030

31+
export interface IssueReference {
32+
number: number;
33+
title: string;
34+
state: GithubItemStateEnum;
35+
}
36+
3137
export interface DisplayLabel extends ILabel {
3238
displayName: string;
3339
}
@@ -112,7 +118,7 @@ export interface PullRequest extends Issue {
112118
busy?: boolean;
113119
loadingCommit?: string;
114120
generateDescriptionTitle?: string;
115-
closingIssues: Pick<Issue, 'title' | 'number' | 'state'>[];
121+
closingIssues: IssueReference[];
116122
}
117123

118124
export interface ProjectItemsReply {

webviews/components/sidebar.tsx

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import React, { useContext, useEffect, useRef, useState } from 'react';
7-
import { closeIcon, copilotIcon, settingsIcon } from './icon';
7+
import { closeIcon, copilotIcon, issueClosedIcon, issueIcon, settingsIcon } from './icon';
88
import { Reviewer } from './reviewer';
99
import { COPILOT_LOGINS } from '../../src/common/copilot';
1010
import { gitHubLabelColor } from '../../src/common/utils';
11-
import { IAccount, IMilestone, IProjectItem, Issue, isITeam, reviewerId, reviewerLabel, ReviewState } from '../../src/github/interface';
12-
import { ChangeReviewersReply, PullRequest } from '../../src/github/views';
11+
import { GithubItemStateEnum, IAccount, IMilestone, IProjectItem, isITeam, reviewerId, reviewerLabel, ReviewState } from '../../src/github/interface';
12+
import { ChangeReviewersReply, IssueReference, PullRequest } from '../../src/github/views';
1313
import PullRequestContext from '../common/context';
1414
import { Label } from '../common/label';
1515
import { AuthorLink, Avatar } from '../components/user';
@@ -274,18 +274,14 @@ export default function Sidebar({ reviewers, labels, closingIssues, hasWritePerm
274274
title="Linked Issues"
275275
hasWritePermission={false}
276276
>
277-
{closingIssues.length > 0 ? (
278-
<div className="p-2">
279-
{closingIssues.map(issue => (
280-
<div className="section-item reviewer">
281-
<div className="avatar-with-author gap-2">
282-
<IssueItem key={issue.title} issue={issue} />
283-
</div>
284-
</div>
285-
))}
286-
</div>
277+
{closingIssues.length ? (
278+
closingIssues.map(issue => (
279+
<div key={issue.number} className="section-item">
280+
<IssueItem issue={issue} />
281+
</div>
282+
))
287283
) : (
288-
<div className="p-4 text-sm text-gray-500 text-center">None yet</div>
284+
<div className="section-placeholder">None yet</div>
289285
)}
290286
</Section>
291287
</div>
@@ -598,25 +594,12 @@ function ConvertToDraft() {
598594
);
599595
}
600596

601-
function IssueItem({ issue }: { issue: Pick<Issue, 'title' | 'number' | 'state'> }) {
597+
function IssueItem({ issue }: { issue: IssueReference }) {
602598
return (
603-
<>
604-
<IssueStateIcon state={issue.state} />
605-
<span className="h2">{issue.title}</span>
606-
</>
599+
<div className="avatar-with-author">
600+
{issue.state === GithubItemStateEnum.Open ? issueIcon : issueClosedIcon}
601+
<span>#{issue.number} {issue.title}</span>
602+
</div>
607603
);
608604
}
609605

610-
function IssueStateIcon({ state }: { state: string }) {
611-
const normalizedState = state.toLowerCase().trim();
612-
613-
switch (normalizedState) {
614-
case 'open':
615-
return settingsIcon;
616-
case 'closed':
617-
return closeIcon;
618-
default:
619-
return closeIcon;
620-
}
621-
}
622-

0 commit comments

Comments
 (0)