Skip to content

Commit d89b169

Browse files
alexr00alexr00
authored
Switch over to multi-panel (#8495)
* Remove external uses of current panel Part of #3058 * Switch over to multi-panel Fixes #3058 * Remove unused refreshActive
1 parent 1542485 commit d89b169

File tree

3 files changed

+105
-73
lines changed

3 files changed

+105
-73
lines changed

‎src/github/issueOverview.ts‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,16 @@ import { asPromise, formatError } from '../common/utils';
2323
import { generateUuid } from '../common/uuid';
2424
import { IRequestMessage, WebviewBase } from '../common/webview';
2525

26+
export function panelKey(owner: string, repo: string, number: number): string {
27+
return `${owner}/${repo}#${number}`;
28+
}
29+
2630
export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends WebviewBase {
2731
public static ID: string = 'IssueOverviewPanel';
2832
/**
29-
* Track the currently panel. Only allow a single panel to exist at a time.
33+
* All open panels, keyed by "owner/repo#number".
3034
*/
31-
public static currentPanel?: IssueOverviewPanel;
35+
protected static _panels: Map<string, IssueOverviewPanel> = new Map();
3236

3337
public static readonly viewType: string = 'IssueOverview';
3438

@@ -55,13 +59,13 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
5559
? vscode.window.activeTextEditor.viewColumn
5660
: vscode.ViewColumn.One;
5761

58-
// If we already have a panel, show it.
59-
// Otherwise, create a new panel.
60-
if (IssueOverviewPanel.currentPanel) {
61-
IssueOverviewPanel.currentPanel._panel.reveal(activeColumn, true);
62+
const key = panelKey(identity.owner, identity.repo, identity.number);
63+
let panel = this._panels.get(key);
64+
if (panel) {
65+
panel._panel.reveal(activeColumn, true);
6266
} else {
6367
const title = `Issue #${identity.number.toString()}`;
64-
IssueOverviewPanel.currentPanel = new IssueOverviewPanel(
68+
panel = new IssueOverviewPanel(
6569
telemetry,
6670
extensionUri,
6771
activeColumn || vscode.ViewColumn.Active,
@@ -71,30 +75,39 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
7175
existingPanel,
7276
undefined
7377
);
78+
this._panels.set(key, panel);
7479
}
7580

76-
await IssueOverviewPanel.currentPanel!.updateWithIdentity(folderRepositoryManager, identity, issue);
81+
await panel.updateWithIdentity(folderRepositoryManager, identity, issue);
7782
}
7883

79-
public static refresh(_owner?: string, _repo?: string, _number?: number): void {
80-
if (this.currentPanel) {
81-
this.currentPanel.refreshPanel();
84+
public static refresh(owner: string, repo: string, number: number): void {
85+
const panel = this.findPanel(owner, repo, number);
86+
if (panel) {
87+
panel.refreshPanel();
8288
}
8389
}
8490

8591
/**
8692
* Return the panel whose webview is currently active (focused),
8793
* or `undefined` when no issue/PR panel is active.
88-
* Today there is at most one panel; with multiple panels this
89-
* will iterate the panel map.
9094
*/
9195
public static getActivePanel(): IssueOverviewPanel | undefined {
92-
if (this.currentPanel?._panel.active) {
93-
return this.currentPanel;
96+
for (const panel of this._panels.values()) {
97+
if (panel._panel.active) {
98+
return panel;
99+
}
94100
}
95101
return undefined;
96102
}
97103

104+
/**
105+
* Find the panel showing a specific issue.
106+
*/
107+
public static findPanel(owner: string, repo: string, number: number): IssueOverviewPanel | undefined {
108+
return this._panels.get(panelKey(owner, repo, number));
109+
}
110+
98111
protected setPanelTitle(title: string): void {
99112
try {
100113
this._panel.title = title;
@@ -756,13 +769,17 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
756769
this._replyMessage(message, result);
757770
}
758771

759-
protected set _currentPanel(panel: IssueOverviewPanel | undefined) {
760-
IssueOverviewPanel.currentPanel = panel;
772+
protected _removeFromPanels(): void {
773+
if (this._identity) {
774+
const key = panelKey(this._identity.owner, this._identity.repo, this._identity.number);
775+
// Use the subclass's own static _panels map via this.constructor
776+
(this.constructor as unknown as typeof IssueOverviewPanel)._panels.delete(key);
777+
}
761778
}
762779

763780
public override dispose() {
764781
super.dispose();
765-
this._currentPanel = undefined;
782+
this._removeFromPanels();
766783
this._webview = undefined;
767784
}
768785

‎src/github/pullRequestOverview.ts‎

Lines changed: 17 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
ReviewEventEnum,
2222
ReviewState,
2323
} from './interface';
24-
import { IssueOverviewPanel } from './issueOverview';
24+
import { IssueOverviewPanel, panelKey } from './issueOverview';
2525
import { isCopilotOnMyBehalf, PullRequestModel } from './pullRequestModel';
2626
import { PullRequestReviewCommon, ReviewContext } from './pullRequestReviewCommon';
2727
import { branchPicks, pickEmail, reviewersQuickPick } from './quickPicks';
@@ -42,10 +42,11 @@ import { IRequestMessage, PULL_REQUEST_OVERVIEW_VIEW_TYPE } from '../common/webv
4242
export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestModel> {
4343
public static override ID: string = 'PullRequestOverviewPanel';
4444
public static override readonly viewType = PULL_REQUEST_OVERVIEW_VIEW_TYPE;
45+
4546
/**
46-
* Track the currently panel. Only allow a single panel to exist at a time.
47+
* All open PR panels, keyed by "owner/repo#number".
4748
*/
48-
public static override currentPanel?: PullRequestOverviewPanel;
49+
protected static override _panels: Map<string, PullRequestOverviewPanel> = new Map();
4950

5051
/**
5152
* Event emitter for when a PR overview becomes active
@@ -85,41 +86,24 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
8586
? vscode.window.activeTextEditor.viewColumn
8687
: vscode.ViewColumn.One;
8788

88-
// If we already have a panel, show it.
89-
// Otherwise, create a new panel.
90-
if (PullRequestOverviewPanel.currentPanel) {
91-
PullRequestOverviewPanel.currentPanel._panel.reveal(activeColumn, preserveFocus);
89+
const key = panelKey(identity.owner, identity.repo, identity.number);
90+
let panel = this._panels.get(key);
91+
if (panel) {
92+
panel._panel.reveal(activeColumn, preserveFocus);
9293
} else {
9394
const title = `Pull Request #${identity.number.toString()}`;
94-
PullRequestOverviewPanel.currentPanel = new PullRequestOverviewPanel(
95+
panel = new PullRequestOverviewPanel(
9596
telemetry,
9697
extensionUri,
9798
activeColumn || vscode.ViewColumn.Active,
9899
title,
99100
folderRepositoryManager,
100101
existingPanel
101102
);
103+
this._panels.set(key, panel);
102104
}
103105

104-
await PullRequestOverviewPanel.currentPanel!.updateWithIdentity(folderRepositoryManager, identity, issue);
105-
}
106-
107-
protected override set _currentPanel(panel: PullRequestOverviewPanel | undefined) {
108-
PullRequestOverviewPanel.currentPanel = panel;
109-
}
110-
111-
public static refreshActive(): void {
112-
const panel = this.getActivePanel();
113-
if (panel) {
114-
panel.refreshPanel();
115-
}
116-
}
117-
118-
public static override refresh(owner: string, repo: string, number: number): void {
119-
const panel = this.findPanel(owner, repo, number);
120-
if (panel) {
121-
panel.refreshPanel();
122-
}
106+
await panel.updateWithIdentity(folderRepositoryManager, identity, issue);
123107
}
124108

125109
public static scrollToReview(owner: string, repo: string, number: number): void {
@@ -136,40 +120,27 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
136120
this._postMessage({ command: 'pr.scrollToPendingReview' });
137121
}
138122

123+
139124
/**
140-
* Get the currently active pull request from the current panel
125+
* Get the currently active pull request from the active panel
141126
*/
142127
public static getCurrentPullRequest(): PullRequestModel | undefined {
143-
return this.currentPanel?._item;
128+
return this.getActivePanel()?._item;
144129
}
145130

146131
/**
147132
* Return the panel whose webview is currently active (focused),
148133
* or `undefined` when no PR panel is active.
149-
* Today there is at most one panel; with multiple panels this
150-
* will iterate the panel map.
151134
*/
152135
public static override getActivePanel(): PullRequestOverviewPanel | undefined {
153-
if (this.currentPanel?._panel.active) {
154-
return this.currentPanel;
155-
}
156-
return undefined;
136+
return super.getActivePanel() as PullRequestOverviewPanel | undefined;
157137
}
158138

159139
/**
160140
* Find the panel showing a specific pull request.
161-
* Currently there is at most one panel, but this will support
162-
* multiple panels in the future.
163141
*/
164-
public static findPanel(owner: string, repo: string, number: number): PullRequestOverviewPanel | undefined {
165-
const panel = this.currentPanel;
166-
if (panel && panel._item &&
167-
panel._item.remote.owner === owner &&
168-
panel._item.remote.repositoryName === repo &&
169-
panel._item.number === number) {
170-
return panel;
171-
}
172-
return undefined;
142+
public static override findPanel(owner: string, repo: string, number: number): PullRequestOverviewPanel | undefined {
143+
return super.findPanel(owner, repo, number) as PullRequestOverviewPanel | undefined;
173144
}
174145

175146
/**

‎src/test/github/pullRequestOverview.test.ts‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { FolderRepositoryManager } from '../../github/folderRepositoryManager';
1111
import { MockTelemetry } from '../mocks/mockTelemetry';
1212
import { MockRepository } from '../mocks/mockRepository';
1313
import { PullRequestOverviewPanel } from '../../github/pullRequestOverview';
14+
import { panelKey } from '../../github/issueOverview';
1415
import { PullRequestModel } from '../../github/pullRequestModel';
1516
import { MockCommandRegistry } from '../mocks/mockCommandRegistry';
1617
import { Protocol } from '../../common/protocol';
@@ -58,8 +59,9 @@ describe('PullRequestOverview', function () {
5859
});
5960

6061
afterEach(function () {
61-
if (PullRequestOverviewPanel.currentPanel) {
62-
PullRequestOverviewPanel.currentPanel.dispose();
62+
// Dispose all open panels
63+
for (const panel of (PullRequestOverviewPanel as any)._panels.values()) {
64+
panel.dispose();
6365
}
6466

6567
pullRequestManager.dispose();
@@ -69,7 +71,7 @@ describe('PullRequestOverview', function () {
6971

7072
describe('createOrShow', function () {
7173
it('creates a new panel', async function () {
72-
assert.strictEqual(PullRequestOverviewPanel.currentPanel, undefined);
74+
assert.strictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined);
7375
const createWebviewPanel = sinon.spy(vscode.window, 'createWebviewPanel');
7476

7577
repo.addGraphQLPullRequest(builder => {
@@ -94,10 +96,10 @@ describe('PullRequestOverview', function () {
9496
enableFindWidget: true
9597
}),
9698
);
97-
assert.notStrictEqual(PullRequestOverviewPanel.currentPanel, undefined);
99+
assert.notStrictEqual(PullRequestOverviewPanel.findPanel('aaa', 'bbb', 1000), undefined);
98100
});
99101

100-
it('reveals and updates an existing panel', async function () {
102+
it('reveals an existing panel for the same PR', async function () {
101103
const createWebviewPanel = sinon.spy(vscode.window, 'createWebviewPanel');
102104

103105
repo.addGraphQLPullRequest(builder => {
@@ -125,11 +127,50 @@ describe('PullRequestOverview', function () {
125127
sinon.stub(prModel0, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]);
126128
await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity0, prModel0);
127129

128-
const panel0 = PullRequestOverviewPanel.currentPanel;
130+
const panel0 = PullRequestOverviewPanel.findPanel(identity0.owner, identity0.repo, identity0.number);
129131
assert.notStrictEqual(panel0, undefined);
130132
assert.strictEqual(createWebviewPanel.callCount, 1);
131133
assert.strictEqual(panel0!.getCurrentTitle(), 'Pull Request #1000');
132134

135+
// Opening the same PR again should reuse the existing panel
136+
await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity0, prModel0);
137+
138+
assert.strictEqual(panel0, PullRequestOverviewPanel.findPanel(identity0.owner, identity0.repo, identity0.number));
139+
assert.strictEqual(createWebviewPanel.callCount, 1);
140+
});
141+
142+
it('creates separate panels for different PRs', async function () {
143+
const createWebviewPanel = sinon.spy(vscode.window, 'createWebviewPanel');
144+
145+
repo.addGraphQLPullRequest(builder => {
146+
builder.pullRequest(response => {
147+
response.repository(r => {
148+
r.pullRequest(pr => pr.number(1000));
149+
});
150+
});
151+
});
152+
repo.addGraphQLPullRequest(builder => {
153+
builder.pullRequest(response => {
154+
response.repository(r => {
155+
r.pullRequest(pr => pr.number(2000));
156+
});
157+
});
158+
});
159+
160+
const prItem0 = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).build(), repo);
161+
const prModel0 = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem0);
162+
const identity0 = { owner: prModel0.remote.owner, repo: prModel0.remote.repositoryName, number: prModel0.number };
163+
const resolveStub = sinon.stub(pullRequestManager, 'resolvePullRequest').resolves(prModel0);
164+
sinon.stub(prModel0, 'getReviewRequests').resolves([]);
165+
sinon.stub(prModel0, 'getTimelineEvents').resolves([]);
166+
sinon.stub(prModel0, 'validateDraftMode').resolves(true);
167+
sinon.stub(prModel0, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]);
168+
await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity0, prModel0);
169+
170+
const panel0 = PullRequestOverviewPanel.findPanel(identity0.owner, identity0.repo, identity0.number);
171+
assert.notStrictEqual(panel0, undefined);
172+
assert.strictEqual(createWebviewPanel.callCount, 1);
173+
133174
const prItem1 = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(2000).build(), repo);
134175
const prModel1 = new PullRequestModel(credentialStore, telemetry, repo, remote, prItem1);
135176
const identity1 = { owner: prModel1.remote.owner, repo: prModel1.remote.repositoryName, number: prModel1.number };
@@ -140,9 +181,12 @@ describe('PullRequestOverview', function () {
140181
sinon.stub(prModel1, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]);
141182
await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity1, prModel1);
142183

143-
assert.strictEqual(panel0, PullRequestOverviewPanel.currentPanel);
144-
assert.strictEqual(createWebviewPanel.callCount, 1);
145-
assert.strictEqual(panel0!.getCurrentTitle(), 'Pull Request #2000');
184+
const panel1 = PullRequestOverviewPanel.findPanel(identity1.owner, identity1.repo, identity1.number);
185+
assert.notStrictEqual(panel1, undefined);
186+
assert.notStrictEqual(panel0, panel1);
187+
assert.strictEqual(createWebviewPanel.callCount, 2);
188+
assert.strictEqual(panel0!.getCurrentTitle(), 'Pull Request #1000');
189+
assert.strictEqual(panel1!.getCurrentTitle(), 'Pull Request #2000');
146190
});
147191
});
148192
});

0 commit comments

Comments
 (0)