From 2c076f5582e95f3e142e510a2465e5b197f1e82d Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Tue, 4 Mar 2025 12:16:25 -0800 Subject: [PATCH] issue reporter update banner, better messaging, leaked disposables (#242602) * various issue reporter debt * remove border --- .../issue/browser/baseIssueReporterService.ts | 8 ++++---- .../contrib/issue/browser/issueFormService.ts | 2 ++ .../contrib/issue/browser/issueReporterPage.ts | 7 ++++++- .../issue/browser/media/issueReporter.css | 10 ++++++++-- .../electron-sandbox/issueReporterService.ts | 16 +++++++++++++++- .../electron-sandbox/nativeIssueFormService.ts | 13 +++++++++---- 6 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts b/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts index 38da042c409..805dcd0205b 100644 --- a/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts +++ b/src/vs/workbench/contrib/issue/browser/baseIssueReporterService.ts @@ -103,7 +103,7 @@ export class BaseIssueReporterService extends Disposable { //TODO: Handle case where extension is not activated const issueReporterElement = this.getElementById('issue-reporter'); if (issueReporterElement) { - this.previewButton = new Button(issueReporterElement, unthemedButtonStyles); + this.previewButton = this._register(new Button(issueReporterElement, unthemedButtonStyles)); const issueRepoName = document.createElement('a'); issueReporterElement.appendChild(issueRepoName); issueRepoName.id = 'show-repo-name'; @@ -141,7 +141,7 @@ export class BaseIssueReporterService extends Disposable { } const delayer = new RunOnceScheduler(updateAll, 0); - iconsStyleSheet.onDidChange(() => delayer.schedule()); + this._register(iconsStyleSheet.onDidChange(() => delayer.schedule())); delayer.schedule(); this.handleExtensionData(data.enabledExtensions); @@ -490,11 +490,11 @@ export class BaseIssueReporterService extends Disposable { this.searchIssues(title, fileOnExtension, fileOnMarketplace); }); - this.previewButton.onDidClick(async () => { + this._register(this.previewButton.onDidClick(async () => { this.delayedSubmit.trigger(async () => { this.createIssue(); }); - }); + })); this.addEventListener('disableExtensions', 'click', () => { this.issueFormService.reloadWithExtensionsDisabled(); diff --git a/src/vs/workbench/contrib/issue/browser/issueFormService.ts b/src/vs/workbench/contrib/issue/browser/issueFormService.ts index bf42337b69e..c587880a09c 100644 --- a/src/vs/workbench/contrib/issue/browser/issueFormService.ts +++ b/src/vs/workbench/contrib/issue/browser/issueFormService.ts @@ -98,11 +98,13 @@ export class IssueFormService implements IIssueFormService { this.issueReporterWindow = auxiliaryWindow.window; } else { console.error('Failed to open auxiliary window'); + disposables.dispose(); } // handle closing issue reporter this.issueReporterWindow?.addEventListener('beforeunload', () => { auxiliaryWindow.window.close(); + disposables.dispose(); this.issueReporterWindow = null; }); } diff --git a/src/vs/workbench/contrib/issue/browser/issueReporterPage.ts b/src/vs/workbench/contrib/issue/browser/issueReporterPage.ts index fbd7fbb3fb8..558d283efec 100644 --- a/src/vs/workbench/contrib/issue/browser/issueReporterPage.ts +++ b/src/vs/workbench/contrib/issue/browser/issueReporterPage.ts @@ -20,10 +20,15 @@ const reviewGuidanceLabel = localize( // intentionally not escaped because of it '{Locked=""}' ] }, - 'Before you report an issue here please review the guidance we provide.' + 'Before you report an issue here please review the guidance we provide. Please complete the form in English.' ); export default (): string => ` +
diff --git a/src/vs/workbench/contrib/issue/browser/media/issueReporter.css b/src/vs/workbench/contrib/issue/browser/media/issueReporter.css index 14ee09bebd1..0aaffebc45e 100644 --- a/src/vs/workbench/contrib/issue/browser/media/issueReporter.css +++ b/src/vs/workbench/contrib/issue/browser/media/issueReporter.css @@ -97,7 +97,6 @@ line-height: 1.5; color: #495057; background-color: #fff; - border: none; } .issue-reporter * { @@ -320,7 +319,6 @@ .issue-reporter select, .issue-reporter textarea { background-color: #3c3c3c; - border: none; color: #cccccc; } @@ -456,3 +454,11 @@ .issue-reporter a { color: var(--vscode-textLink-foreground); } + +.issue-reporter-update-banner { + color: var(--vscode-button-foreground); + background-color: var(--vscode-button-background); + padding: 10px; + text-align: center; + border-radius: 4px; +} diff --git a/src/vs/workbench/contrib/issue/electron-sandbox/issueReporterService.ts b/src/vs/workbench/contrib/issue/electron-sandbox/issueReporterService.ts index e6be6f94d32..0dc796aa7d2 100644 --- a/src/vs/workbench/contrib/issue/electron-sandbox/issueReporterService.ts +++ b/src/vs/workbench/contrib/issue/electron-sandbox/issueReporterService.ts @@ -16,6 +16,7 @@ import { IFileService } from '../../../../platform/files/common/files.js'; import { INativeHostService } from '../../../../platform/native/common/native.js'; import { IProcessMainService } from '../../../../platform/process/common/process.js'; import { IThemeService } from '../../../../platform/theme/common/themeService.js'; +import { IUpdateService, StateType } from '../../../../platform/update/common/update.js'; import { applyZoom } from '../../../../platform/window/electron-sandbox/window.js'; import { BaseIssueReporterService } from '../browser/baseIssueReporterService.js'; import { IssueReporterData as IssueReporterModelData } from '../browser/issueReporterModel.js'; @@ -47,7 +48,8 @@ export class IssueReporter extends BaseIssueReporterService { @IProcessMainService processMainService: IProcessMainService, @IThemeService themeService: IThemeService, @IFileService fileService: IFileService, - @IFileDialogService fileDialogService: IFileDialogService + @IFileDialogService fileDialogService: IFileDialogService, + @IUpdateService private readonly updateService: IUpdateService ) { super(disableExtensions, data, os, product, window, false, issueFormService, themeService, fileService, fileDialogService); this.processMainService = processMainService; @@ -64,6 +66,7 @@ export class IssueReporter extends BaseIssueReporterService { }); } + this.checkForUpdates(); this.setEventHandlers(); applyZoom(this.data.zoomLevel, this.window); this.updateExperimentsInfo(this.data.experiments); @@ -71,6 +74,17 @@ export class IssueReporter extends BaseIssueReporterService { this.updateUnsupportedMode(this.data.isUnsupported); } + private async checkForUpdates(): Promise { + const updateState = this.updateService.state; + if (updateState.type === StateType.Ready || updateState.type === StateType.Downloaded) { + const updateBanner = this.getElementById('update-banner'); + if (updateBanner) { + updateBanner.classList.remove('hidden'); + updateBanner.textContent = localize('updateAvailable', "A new version of {0} is available.", this.product.nameLong); + } + } + } + public override setEventHandlers(): void { super.setEventHandlers(); diff --git a/src/vs/workbench/contrib/issue/electron-sandbox/nativeIssueFormService.ts b/src/vs/workbench/contrib/issue/electron-sandbox/nativeIssueFormService.ts index c202234088a..f6ae15199b8 100644 --- a/src/vs/workbench/contrib/issue/electron-sandbox/nativeIssueFormService.ts +++ b/src/vs/workbench/contrib/issue/electron-sandbox/nativeIssueFormService.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import './media/issueReporter.css'; +import { DisposableStore } from '../../../../base/common/lifecycle.js'; import { IMenuService } from '../../../../platform/actions/common/actions.js'; import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js'; import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js'; @@ -12,13 +12,16 @@ import { IInstantiationService } from '../../../../platform/instantiation/common import { ILogService } from '../../../../platform/log/common/log.js'; import { INativeHostService } from '../../../../platform/native/common/native.js'; import product from '../../../../platform/product/common/product.js'; +import { IAuxiliaryWindowService } from '../../../services/auxiliaryWindow/browser/auxiliaryWindowService.js'; +import { IHostService } from '../../../services/host/browser/host.js'; import { IssueFormService } from '../browser/issueFormService.js'; import { IIssueFormService, IssueReporterData } from '../common/issue.js'; import { IssueReporter } from './issueReporterService.js'; -import { IAuxiliaryWindowService } from '../../../services/auxiliaryWindow/browser/auxiliaryWindowService.js'; -import { IHostService } from '../../../services/host/browser/host.js'; +import './media/issueReporter.css'; export class NativeIssueFormService extends IssueFormService implements IIssueFormService { + private readonly store = new DisposableStore(); + constructor( @IInstantiationService instantiationService: IInstantiationService, @IAuxiliaryWindowService auxiliaryWindowService: IAuxiliaryWindowService, @@ -53,8 +56,10 @@ export class NativeIssueFormService extends IssueFormService implements IIssueFo // create issue reporter and instantiate if (this.issueReporterWindow) { - const issueReporter = this.instantiationService.createInstance(IssueReporter, !!this.environmentService.disableExtensions, data, { type: this.type, arch: this.arch, release: this.release }, product, this.issueReporterWindow); + const issueReporter = this.store.add(this.instantiationService.createInstance(IssueReporter, !!this.environmentService.disableExtensions, data, { type: this.type, arch: this.arch, release: this.release }, product, this.issueReporterWindow)); issueReporter.render(); + } else { + this.store.dispose(); } } }