issue reporter update banner, better messaging, leaked disposables (#242602)

* various issue reporter debt

* remove border
This commit is contained in:
Justin Chen
2025-03-04 12:16:25 -08:00
committed by GitHub
parent 73072ae20b
commit 2c076f5582
6 changed files with 44 additions and 12 deletions

View File

@@ -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();

View File

@@ -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;
});
}

View File

@@ -20,10 +20,15 @@ const reviewGuidanceLabel = localize( // intentionally not escaped because of it
'{Locked="</a>"}'
]
},
'Before you report an issue here please <a href="https://github.com/microsoft/vscode/wiki/Submitting-Bugs-and-Suggestions" target="_blank">review the guidance we provide</a>.'
'Before you report an issue here please <a href="https://github.com/microsoft/vscode/wiki/Submitting-Bugs-and-Suggestions" target="_blank">review the guidance we provide</a>. Please complete the form in English.'
);
export default (): string => `
<div id="update-banner" class="issue-reporter-update-banner hidden">
<span class="update-banner-text" id="update-banner-text">
<!-- To be dynamically filled -->
</span>
</div>
<div class="issue-reporter" id="issue-reporter">
<div id="english" class="input-group hidden">${escape(localize('completeInEnglish', "Please complete the form in English."))}</div>

View File

@@ -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;
}

View File

@@ -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<void> {
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();

View File

@@ -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();
}
}
}