From 60b9d1ba02cdb846c240f307daf253fd02a193c5 Mon Sep 17 00:00:00 2001 From: Rachel Macfarlane Date: Mon, 26 Mar 2018 11:49:44 -0700 Subject: [PATCH] Add support for filing an issue on an extension within the issue reporter, #45673 --- .../issue/issueReporterMain.ts | 123 ++++++++++++------ .../issue/issueReporterModel.ts | 18 ++- .../issue/issueReporterPage.ts | 41 +++--- .../issue/media/issueReporter.css | 35 +---- .../issue/test/testReporterModel.test.ts | 3 +- .../common/extensionManagement.ts | 3 + 6 files changed, 122 insertions(+), 101 deletions(-) diff --git a/src/vs/code/electron-browser/issue/issueReporterMain.ts b/src/vs/code/electron-browser/issue/issueReporterMain.ts index f6bbf228784..e179fc5c52a 100644 --- a/src/vs/code/electron-browser/issue/issueReporterMain.ts +++ b/src/vs/code/electron-browser/issue/issueReporterMain.ts @@ -213,9 +213,9 @@ export class IssueReporter extends Disposable { if (this.environmentService.disableExtensions || extensions.length === 0) { (document.getElementById('disableExtensions')).disabled = true; - (document.getElementById('reproducesWithoutExtensions')).checked = true; - this.issueReporterModel.update({ reprosWithoutExtensions: true }); } + + this.updateExtensionSelector(extensions); } private handleSettingsSearchData(data: ISettingsSearchIssueReporterData): void { @@ -322,30 +322,21 @@ export class IssueReporter extends Disposable { }); } - this.addEventListener('reproducesWithoutExtensions', 'click', (e) => { - this.issueReporterModel.update({ reprosWithoutExtensions: true }); - }); - - this.addEventListener('reproducesWithExtensions', 'click', (e) => { - this.issueReporterModel.update({ reprosWithoutExtensions: false }); + this.addEventListener('issue-source', 'change', (event: Event) => { + const fileOnExtension = JSON.parse((event.target).value); + this.issueReporterModel.update({ fileOnExtension: fileOnExtension, includeExtensions: !fileOnExtension }); + this.render(); + this.search(); }); this.addEventListener('description', 'input', (event: Event) => { const issueDescription = (event.target).value; this.issueReporterModel.update({ issueDescription }); - - const title = (document.getElementById('issue-title')).value; - if (title || issueDescription) { - this.searchDuplicates(title, issueDescription); - } else { - this.clearSearchResults(); - } + this.search(); }); this.addEventListener('issue-title', 'input', (e) => { - const description = this.issueReporterModel.getData().issueDescription; const title = (event.target).value; - const lengthValidationMessage = document.getElementById('issue-title-length-validation-error'); if (title && this.getIssueUrlWithTitle(title).length > MAX_URL_LENGTH) { show(lengthValidationMessage); @@ -353,11 +344,7 @@ export class IssueReporter extends Disposable { hide(lengthValidationMessage); } - if (title || description) { - this.searchDuplicates(title, description); - } else { - this.clearSearchResults(); - } + this.search(); }); this.addEventListener('github-submit-btn', 'click', () => this.createIssue()); @@ -373,16 +360,6 @@ export class IssueReporter extends Disposable { } }); - this.addEventListener('showRunning', 'click', () => { - ipcRenderer.send('workbenchCommand', 'workbench.action.showRuntimeExtensions'); - }); - - this.addEventListener('showRunning', 'keydown', (e: KeyboardEvent) => { - if (e.keyCode === 13 || e.keyCode === 32) { - ipcRenderer.send('workbenchCommand', 'workbench.action.showRuntimeExtensions'); - } - }); - // Cmd+Enter or Mac or Ctrl+Enter on other platforms previews issue and closes window if (platform.isMacintosh) { let prevKeyWasCommand = false; @@ -438,6 +415,23 @@ export class IssueReporter extends Disposable { return false; } + private search(): void { + // Only search issues in VSCode for now. + const fileOnExtension = this.issueReporterModel.getData().fileOnExtension; + if (fileOnExtension) { + this.clearSearchResults(); + return; + } + + const title = (document.getElementById('issue-title')).value; + const issueDescription = (document.getElementById('description')).value; + if (title || issueDescription) { + this.searchDuplicates(title, issueDescription); + } else { + this.clearSearchResults(); + } + } + private clearSearchResults(): void { const similarIssues = document.getElementById('similar-issues'); similarIssues.innerHTML = ''; @@ -551,7 +545,7 @@ export class IssueReporter extends Disposable { private renderBlocks(): void { // Depending on Issue Type, we render different blocks and text - const { issueType } = this.issueReporterModel.getData(); + const { issueType, fileOnExtension } = this.issueReporterModel.getData(); const blockContainer = document.getElementById('block-container'); const systemBlock = document.querySelector('.block-system'); const processBlock = document.querySelector('.block-process'); @@ -560,9 +554,10 @@ export class IssueReporter extends Disposable { const searchedExtensionsBlock = document.querySelector('.block-searchedExtensions'); const settingsSearchResultsBlock = document.querySelector('.block-settingsSearchResults'); - const disabledExtensions = document.getElementById('disabledExtensions'); + const problemSource = document.getElementById('problem-source'); const descriptionTitle = document.getElementById('issue-description-label'); const descriptionSubtitle = document.getElementById('issue-description-subtitle'); + const extensionSelector = document.getElementById('extension-selection'); // Hide all by default hide(blockContainer); @@ -572,13 +567,20 @@ export class IssueReporter extends Disposable { hide(extensionsBlock); hide(searchedExtensionsBlock); hide(settingsSearchResultsBlock); - hide(disabledExtensions); + hide(problemSource); if (issueType === IssueType.Bug) { show(blockContainer); show(systemBlock); - show(extensionsBlock); - show(disabledExtensions); + show(problemSource); + + if (fileOnExtension) { + hide(extensionsBlock); + show(extensionSelector); + } else { + show(extensionsBlock); + hide(extensionSelector); + } descriptionTitle.innerHTML = `${localize('stepsToReproduce', "Steps to Reproduce")} *`; descriptionSubtitle.innerHTML = localize('bugDescription', "Share the steps needed to reliably reproduce the problem. Please include actual and expected results. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."); @@ -588,7 +590,15 @@ export class IssueReporter extends Disposable { show(processBlock); show(workspaceBlock); show(extensionsBlock); - show(disabledExtensions); + show(problemSource); + + if (fileOnExtension) { + hide(extensionsBlock); + show(extensionSelector); + } else { + show(extensionsBlock); + hide(extensionSelector); + } descriptionTitle.innerHTML = `${localize('stepsToReproduce', "Steps to Reproduce")} *`; descriptionSubtitle.innerHTML = localize('performanceIssueDesciption', "When did this performance issue happen? Does it occur on startup or after a specific series of actions? We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."); @@ -618,9 +628,8 @@ export class IssueReporter extends Disposable { private validateInputs(): boolean { let isValid = true; - ['issue-title', 'description'].forEach(elementId => { + ['issue-title', 'description', 'issue-source', 'extension-selector'].forEach(elementId => { isValid = this.validateInput(elementId) && isValid; - }); return isValid; @@ -630,7 +639,10 @@ export class IssueReporter extends Disposable { if (!this.validateInputs()) { // If inputs are invalid, set focus to the first one and add listeners on them // to detect further changes - (document.getElementsByClassName('invalid-input')[0]).focus(); + const invalidInput = document.getElementsByClassName('invalid-input'); + if (invalidInput.length) { + (invalidInput[0]).focus(); + } document.getElementById('issue-title').addEventListener('input', (event) => { this.validateInput('issue-title'); @@ -664,9 +676,19 @@ export class IssueReporter extends Disposable { return true; } - private getIssueUrlWithTitle(issueTitle: string) { + private getIssueUrlWithTitle(issueTitle: string): string { + let repositoryUrl = product.reportIssueUrl; + if (this.issueReporterModel.getData().fileOnExtension) { + const selectedExtension = this.issueReporterModel.getData().selectedExtension; + const extensionUrl = selectedExtension && selectedExtension.manifest && selectedExtension.manifest.repository && selectedExtension.manifest.repository.url; + if (extensionUrl) { + // Remove '.git' suffix + repositoryUrl = `${extensionUrl.indexOf('.git') !== -1 ? extensionUrl.substr(0, extensionUrl.length - 4) : extensionUrl}/issues/new/`; + } + } + const queryStringPrefix = product.reportIssueUrl.indexOf('?') === -1 ? '?' : '&'; - return `${product.reportIssueUrl}${queryStringPrefix}title=${encodeURIComponent(issueTitle)}`; + return `${repositoryUrl}${queryStringPrefix}title=${encodeURIComponent(issueTitle)}`; } private updateSystemInfo = (state) => { @@ -682,6 +704,21 @@ export class IssueReporter extends Disposable { target.innerHTML = `${tableHtml}
`; } + private updateExtensionSelector(extensions: ILocalExtension[]): void { + const makeOption = (extension: ILocalExtension) => ``; + const extensionsSelector = document.getElementById('extension-selector'); + extensionsSelector.innerHTML = '' + extensions.map(makeOption).join('\n'); + + this.addEventListener('extension-selector', 'change', (e: Event) => { + const selectedExtensionId = (e.target).value; + const extensions = this.issueReporterModel.getData().enabledNonThemeExtesions; + const matches = extensions.filter(extension => extension.identifier.id === selectedExtensionId); + if (matches.length) { + this.issueReporterModel.update({ selectedExtension: matches[0] }); + } + }); + } + private updateProcessInfo = (state) => { const target = document.querySelector('.block-process .block-info'); target.innerHTML = `${state.processInfo}`; diff --git a/src/vs/code/electron-browser/issue/issueReporterModel.ts b/src/vs/code/electron-browser/issue/issueReporterModel.ts index a6ec8153424..8be6fa1bea9 100644 --- a/src/vs/code/electron-browser/issue/issueReporterModel.ts +++ b/src/vs/code/electron-browser/issue/issueReporterModel.ts @@ -28,7 +28,8 @@ export interface IssueReporterData { numberOfThemeExtesions?: number; enabledNonThemeExtesions?: ILocalExtension[]; extensionsDisabled?: boolean; - reprosWithoutExtensions?: boolean; + fileOnExtension?: boolean; + selectedExtension?: ILocalExtension; actualSearchResults?: ISettingSearchResult[]; query?: string; filterResultCount?: number; @@ -44,8 +45,7 @@ export class IssueReporterModel { includeProcessInfo: true, includeExtensions: true, includeSearchedExtensions: true, - includeSettingsSearchDetails: true, - reprosWithoutExtensions: false + includeSettingsSearchDetails: true }; this._data = initialData ? assign(defaultData, initialData) : defaultData; @@ -64,7 +64,7 @@ export class IssueReporterModel { Issue Type: ${this.getIssueTypeTitle()} ${this._data.issueDescription} - +${this.getExtensionVersion()} VS Code version: ${this._data.versionInfo && this._data.versionInfo.vscodeVersion} OS version: ${this._data.versionInfo && this._data.versionInfo.os} @@ -72,6 +72,14 @@ ${this.getInfos()} `; } + private getExtensionVersion(): string { + if (this._data.fileOnExtension) { + return `\nExtension version: ${this._data.selectedExtension.manifest.version}`; + } else { + return ''; + } + } + private getIssueTypeTitle(): string { if (this._data.issueType === IssueType.Bug) { return 'Bug'; @@ -108,8 +116,6 @@ ${this.getInfos()} if (this._data.includeExtensions) { info += this.generateExtensionsMd(); } - - info += this._data.reprosWithoutExtensions ? '\nReproduces without extensions' : '\nReproduces only with extensions'; } if (this._data.issueType === IssueType.SettingsSearchIssue) { diff --git a/src/vs/code/electron-browser/issue/issueReporterPage.ts b/src/vs/code/electron-browser/issue/issueReporterPage.ts index abf3c7b4ad6..045a35fb717 100644 --- a/src/vs/code/electron-browser/issue/issueReporterPage.ts +++ b/src/vs/code/electron-browser/issue/issueReporterPage.ts @@ -27,6 +27,25 @@ export default (): string => ` + +
+ + +
${escape(localize('disableExtensionsLabelText', "Try to reproduce the problem after {0}. If the problem only reproduces when extensions are active, it is likely an issue with an extension.")) + .replace('{0}', `${escape(localize('disableExtensions', "disabling all extensions and reloading the window"))}`)} +
+ +
+ + +
+
+
@@ -112,28 +131,6 @@ export default (): string => `
-
-
- -
-
- - -
-
- - -
-
-
-
${escape(localize('disableExtensionsLabelText', "Try to reproduce the problem after {0}.")) - .replace('{0}', `${escape(localize('disableExtensions', "disabling all extensions and reloading the window"))}`)} -
-
${escape(localize('showRunningExtensionsLabelText', "If you suspect it's an extension issue, {0} to report the issue on the extension.")) - .replace('{0}', `${escape(localize('showRunningExtensions', "view all running extensions"))}`)} -
-
-