Create by default, but give dropdown for Preview on Github (#257593)

* dropdown in issue reporter to preview

* wip

* cleanup

* L/R padding for dropdown + preview by default

* some comment cleaning

* found the actual root fix. copilot wrote too much code.

* some cleanup

* preserve a comment

* fix cutoff rendering in the browser layer due to inner div dropping height css

* explanation

* wip -- race condition loading stylings?

* explanation of race condition, and gate the stylesheet behind the correct check

* fix not being able to scroll, and fix the custom browser contextmenu anchor math -- needs testing

* working -- needs cleanup + repo url appearing above button fix

* wip

* fix CSS for dropdown button

* polish

* cleanup

---------

Co-authored-by: Michael Lively <milively@microsoft.com>
This commit is contained in:
Justin Chen
2025-08-26 11:27:14 -07:00
committed by GitHub
parent 709b8fabf5
commit d2ff8d1606
8 changed files with 202 additions and 75 deletions
@@ -351,8 +351,13 @@ export class ContextView extends Disposable {
this.view.classList.toggle('fixed', this.useFixedPosition);
const containerPosition = DOM.getDomNodePagePosition(this.container!);
this.view.style.top = `${top - (this.useFixedPosition ? DOM.getDomNodePagePosition(this.view).top : containerPosition.top)}px`;
this.view.style.left = `${left - (this.useFixedPosition ? DOM.getDomNodePagePosition(this.view).left : containerPosition.left)}px`;
// Account for container scroll when positioning the context view
const containerScrollTop = this.container!.scrollTop || 0;
const containerScrollLeft = this.container!.scrollLeft || 0;
this.view.style.top = `${top - (this.useFixedPosition ? DOM.getDomNodePagePosition(this.view).top : containerPosition.top) + containerScrollTop}px`;
this.view.style.left = `${left - (this.useFixedPosition ? DOM.getDomNodePagePosition(this.view).left : containerPosition.left) + containerScrollLeft}px`;
this.view.style.width = 'initial';
}
+1 -1
View File
@@ -1020,7 +1020,7 @@ export function formatRule(c: ThemeIcon) {
return `.codicon-${c.id}:before { content: '\\${fontCharacter.toString(16)}'; }`;
}
function getMenuWidgetCSS(style: IMenuStyles, isForShadowDom: boolean): string {
export function getMenuWidgetCSS(style: IMenuStyles, isForShadowDom: boolean): string {
let result = /* css */`
.monaco-menu {
font-size: 13px;
@@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { $, isHTMLInputElement, isHTMLTextAreaElement, reset, windowOpenNoOpener } from '../../../../base/browser/dom.js';
import { createStyleSheet } from '../../../../base/browser/domStylesheets.js';
import { Button, unthemedButtonStyles } from '../../../../base/browser/ui/button/button.js';
import { Button, ButtonWithDropdown, unthemedButtonStyles } from '../../../../base/browser/ui/button/button.js';
import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js';
import { mainWindow } from '../../../../base/browser/window.js';
import { Delayer, RunOnceScheduler } from '../../../../base/common/async.js';
@@ -21,14 +21,17 @@ import { joinPath } from '../../../../base/common/resources.js';
import { escape } from '../../../../base/common/strings.js';
import { ThemeIcon } from '../../../../base/common/themables.js';
import { URI } from '../../../../base/common/uri.js';
import { Action } from '../../../../base/common/actions.js';
import { localize } from '../../../../nls.js';
import { IFileDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { IFileService } from '../../../../platform/files/common/files.js';
import { getIconsStyleSheet } from '../../../../platform/theme/browser/iconsStyleSheet.js';
import { IThemeService } from '../../../../platform/theme/common/themeService.js';
import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js';
import { IIssueFormService, IssueReporterData, IssueReporterExtensionData, IssueType } from '../common/issue.js';
import { normalizeGitHubUrl } from '../common/issueReporterUtil.js';
import { IssueReporterModel, IssueReporterData as IssueReporterModelData } from './issueReporterModel.js';
import { IAuthenticationService } from '../../../services/authentication/common/authentication.js';
const MAX_URL_LENGTH = 7500;
@@ -63,10 +66,12 @@ export class BaseIssueReporterService extends Disposable {
public loadingExtensionData = false;
public selectedExtension = '';
public delayedSubmit = new Delayer<void>(300);
public previewButton!: Button;
public onGithubButton!: Button | ButtonWithDropdown;
public nonGitHubIssueUrl = false;
public needsUpdate = false;
public acknowledged = false;
private createAction: Action;
private previewAction: Action;
constructor(
public disableExtensions: boolean,
@@ -83,6 +88,8 @@ export class BaseIssueReporterService extends Disposable {
@IThemeService public readonly themeService: IThemeService,
@IFileService public readonly fileService: IFileService,
@IFileDialogService public readonly fileDialogService: IFileDialogService,
@IContextMenuService public readonly contextMenuService: IContextMenuService,
@IAuthenticationService public readonly authenticationService: IAuthenticationService
) {
super();
const targetExtension = data.extensionId ? data.enabledExtensions.find(extension => extension.id.toLocaleLowerCase() === data.extensionId?.toLocaleLowerCase()) : undefined;
@@ -98,19 +105,50 @@ export class BaseIssueReporterService extends Disposable {
selectedExtension: targetExtension
});
this._register(this.authenticationService.onDidChangeSessions(async () => {
const previousAuthState = !!this.data.githubAccessToken;
let githubAccessToken = '';
try {
const githubSessions = await this.authenticationService.getSessions('github');
const potentialSessions = githubSessions.filter(session => session.scopes.includes('repo'));
githubAccessToken = potentialSessions[0]?.accessToken;
} catch (e) {
// Ignore
}
this.data.githubAccessToken = githubAccessToken;
const currentAuthState = !!githubAccessToken;
if (previousAuthState !== currentAuthState) {
this.recreateGithubButton();
}
}));
const fileOnMarketplace = data.issueSource === IssueSource.Marketplace;
const fileOnProduct = data.issueSource === IssueSource.VSCode;
this.issueReporterModel.update({ fileOnMarketplace, fileOnProduct });
//TODO: Handle case where extension is not activated
this.createAction = this._register(new Action('issueReporter.create', localize('create', "Create on GitHub"), undefined, true, async () => {
this.delayedSubmit.trigger(async () => {
this.createIssue();
});
}));
this.previewAction = this._register(new Action('issueReporter.preview', localize('preview', "Preview on GitHub"), undefined, true, async () => {
this.delayedSubmit.trigger(async () => {
this.createIssue(true);
});
}));
const issueReporterElement = this.getElementById('issue-reporter');
if (issueReporterElement) {
this.previewButton = this._register(new Button(issueReporterElement, unthemedButtonStyles));
// Create button based on GitHub access token availability
this.recreateGithubButton();
const issueRepoName = document.createElement('a');
issueReporterElement.appendChild(issueRepoName);
issueRepoName.id = 'show-repo-name';
issueRepoName.classList.add('hidden');
this.updatePreviewButtonState();
}
const issueTitle = data.issueTitle;
@@ -433,11 +471,7 @@ export class BaseIssueReporterService extends Disposable {
this.searchIssues(title, fileOnExtension, fileOnMarketplace);
});
this._register(this.previewButton.onDidClick(async () => {
this.delayedSubmit.trigger(async () => {
this.createIssue();
});
}));
// We handle clicks in the dropdown actions now
this.addEventListener('disableExtensions', 'click', () => {
this.issueFormService.reloadWithExtensionsDisabled();
@@ -503,21 +537,67 @@ export class BaseIssueReporterService extends Disposable {
this.updatePreviewButtonState();
}
public updatePreviewButtonState() {
private recreateGithubButton(): void {
const issueReporterElement = this.getElementById('issue-reporter');
if (!issueReporterElement) {
return;
}
if (!this.acknowledged && this.needsUpdate) {
this.previewButton.label = localize('acknowledge', "Confirm Version Acknowledgement");
this.previewButton.enabled = false;
} else if (this.isPreviewEnabled()) {
if (this.data.githubAccessToken) {
this.previewButton.label = localize('createOnGitHub', "Create on GitHub");
} else {
this.previewButton.label = localize('previewOnGitHub', "Preview on GitHub");
}
this.previewButton.enabled = true;
// Dispose of the existing button
if (this.onGithubButton) {
this.onGithubButton.dispose();
}
// Find the repo name element to insert the button before it
const issueRepoName = this.getElementById('show-repo-name');
// Create button based on GitHub access token availability
if (this.data.githubAccessToken) {
this.onGithubButton = this._register(new ButtonWithDropdown(issueReporterElement, {
contextMenuProvider: this.contextMenuService,
actions: [this.previewAction],
addPrimaryActionToDropdown: false,
...unthemedButtonStyles
}));
// Set up click handler for primary button (create)
this._register(this.onGithubButton.onDidClick(() => {
this.createAction.run();
}));
} else {
this.previewButton.enabled = false;
this.previewButton.label = localize('loadingData', "Loading data...");
// No access token: create simple Button (preview only)
this.onGithubButton = this._register(new Button(issueReporterElement, unthemedButtonStyles));
// Set up click handler for preview
this._register(this.onGithubButton.onDidClick(() => {
this.previewAction.run();
}));
}
// Ensure button appears before repo name by moving it if necessary
if (issueRepoName && this.onGithubButton.element.nextSibling !== issueRepoName) {
issueReporterElement.insertBefore(this.onGithubButton.element, issueRepoName);
}
// Update the button state after recreation
this.updatePreviewButtonState();
}
public updatePreviewButtonState() {
if (!this.acknowledged && this.needsUpdate) {
this.onGithubButton.label = localize('acknowledge', "Confirm Version Acknowledgement");
this.onGithubButton.enabled = false;
} else if (this.isPreviewEnabled()) {
// Set button label to match the primary action
if (this.data.githubAccessToken) {
this.onGithubButton.label = localize('createOnGitHub', "Create on GitHub");
} else {
this.onGithubButton.label = localize('previewOnGitHub', "Preview on GitHub");
}
this.onGithubButton.enabled = true;
} else {
this.onGithubButton.enabled = false;
this.onGithubButton.label = localize('loadingData', "Loading data...");
}
const issueRepoName = this.getElementById('show-repo-name')! as HTMLAnchorElement;
@@ -539,7 +619,7 @@ export class BaseIssueReporterService extends Disposable {
width: 'auto'
});
show(issueRepoName);
} else {
} else if (issueRepoName) {
// clear styles
issueRepoName.removeAttribute('style');
hide(issueRepoName);
@@ -874,7 +954,7 @@ export class BaseIssueReporterService extends Disposable {
hide(descriptionTextArea);
reset(descriptionTitle, localize('handlesIssuesElsewhere', "This extension handles issues outside of VS Code"));
reset(descriptionSubtitle, localize('elsewhereDescription', "The '{0}' extension prefers to use an external issue reporter. To be taken to that issue reporting experience, click the button below.", selectedExtension.displayName));
this.previewButton.label = localize('openIssueReporter', "Open External Issue Reporter");
this.onGithubButton.label = localize('openIssueReporter', "Open External Issue Reporter");
return;
}
@@ -997,7 +1077,7 @@ export class BaseIssueReporterService extends Disposable {
return true;
}
public async createIssue(): Promise<boolean> {
public async createIssue(preview?: boolean): Promise<boolean> {
const selectedExtension = this.issueReporterModel.getData().selectedExtension;
const hasUri = this.nonGitHubIssueUrl;
// Short circuit if the extension provides a custom issue handler
@@ -1055,7 +1135,7 @@ export class BaseIssueReporterService extends Disposable {
}
const gitHubDetails = this.parseGitHubUrl(issueUrl);
if (this.data.githubAccessToken && gitHubDetails) {
if (this.data.githubAccessToken && gitHubDetails && !preview) {
return this.submitToGitHub(issueTitle, issueBody, gitHubDetails);
}
@@ -1212,7 +1292,7 @@ export class BaseIssueReporterService extends Disposable {
const extension = this.issueReporterModel.getData().selectedExtension;
if (!extension) {
this.previewButton.enabled = true;
this.onGithubButton.enabled = true;
return;
}
@@ -1222,10 +1302,10 @@ export class BaseIssueReporterService extends Disposable {
const hasValidGitHubUrl = this.getExtensionGitHubUrl();
if (hasValidGitHubUrl) {
this.previewButton.enabled = true;
this.onGithubButton.enabled = true;
} else {
this.setExtensionValidationMessage();
this.previewButton.enabled = false;
this.onGithubButton.enabled = false;
}
}
@@ -3,6 +3,8 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { safeSetInnerHtml } from '../../../../base/browser/domSanitize.js';
import { createStyleSheet } from '../../../../base/browser/domStylesheets.js';
import { getMenuWidgetCSS, Menu, unthemedMenuStyles } from '../../../../base/browser/ui/menu/menu.js';
import { DisposableStore } from '../../../../base/common/lifecycle.js';
import { isLinux, isWindows } from '../../../../base/common/platform.js';
import Severity from '../../../../base/common/severity.js';
@@ -86,12 +88,20 @@ export class IssueFormService implements IIssueFormService {
auxiliaryWindow.window.document.title = 'Issue Reporter';
auxiliaryWindow.window.document.body.classList.add('issue-reporter-body', 'monaco-workbench', platformClass);
// custom issue reporter wrapper
// removes preset monaco-workbench container
auxiliaryWindow.container.remove();
// The Menu class uses a static globalStyleSheet that's created lazily on first menu creation.
// Since auxiliary windows clone stylesheets from main window, but Menu.globalStyleSheet
// may not exist yet in main window, we need to ensure menu styles are available here.
if (!Menu.globalStyleSheet) {
const menuStyleSheet = createStyleSheet(auxiliaryWindow.window.document.head);
menuStyleSheet.textContent = getMenuWidgetCSS(unthemedMenuStyles, false);
}
// custom issue reporter wrapper that preserves critical auxiliary window container styles
const div = document.createElement('div');
div.classList.add('monaco-workbench');
// removes preset monaco-workbench
auxiliaryWindow.container.remove();
auxiliaryWindow.window.document.body.appendChild(div);
safeSetInnerHtml(div, BaseHtml(), {
// Also allow input elements
@@ -4,9 +4,11 @@
*--------------------------------------------------------------------------------------------*/
import { IProductConfiguration } from '../../../../base/common/product.js';
import { localize } from '../../../../nls.js';
import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js';
import { IFileDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { IFileService } from '../../../../platform/files/common/files.js';
import { IThemeService } from '../../../../platform/theme/common/themeService.js';
import { IAuthenticationService } from '../../../services/authentication/common/authentication.js';
import { IIssueFormService, IssueReporterData } from '../common/issue.js';
import { BaseIssueReporterService } from './baseIssueReporterService.js';
@@ -27,9 +29,11 @@ export class IssueWebReporter extends BaseIssueReporterService {
@IIssueFormService issueFormService: IIssueFormService,
@IThemeService themeService: IThemeService,
@IFileService fileService: IFileService,
@IFileDialogService fileDialogService: IFileDialogService
@IFileDialogService fileDialogService: IFileDialogService,
@IContextMenuService contextMenuService: IContextMenuService,
@IAuthenticationService authenticationService: IAuthenticationService
) {
super(disableExtensions, data, os, product, window, true, issueFormService, themeService, fileService, fileDialogService);
super(disableExtensions, data, os, product, window, true, issueFormService, themeService, fileService, fileDialogService, contextMenuService, authenticationService);
const target = this.window.document.querySelector<HTMLElement>('.block-system .block-info');
@@ -77,16 +77,22 @@
/**
* Button
*/
.issue-reporter-body .monaco-text-button {
display: block;
width: auto;
padding: 4px 10px;
align-self: flex-end;
margin-bottom: 1em;
font-size: 13px;
}
.issue-reporter-body .monaco-button-dropdown {
align-self: flex-end;
}
.issue-reporter-body .monaco-button-dropdown > .monaco-button.monaco-dropdown-button {
padding: 0 4px;
}
.issue-reporter-body select {
height: calc(2.25rem + 2px);
display: inline-block;
@@ -188,6 +194,11 @@ body.issue-reporter-body {
background-color: var(--vscode-editor-background)
}
.issue-reporter-body .monaco-workbench {
height: 100%;
overflow: auto;
}
.issue-reporter-body .hidden {
display: none;
}
@@ -10,6 +10,7 @@ import { IProductConfiguration } from '../../../../base/common/product.js';
import { joinPath } from '../../../../base/common/resources.js';
import { URI } from '../../../../base/common/uri.js';
import { localize } from '../../../../nls.js';
import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js';
import { isRemoteDiagnosticError } from '../../../../platform/diagnostics/common/diagnostics.js';
import { IFileDialogService } from '../../../../platform/dialogs/common/dialogs.js';
import { IFileService } from '../../../../platform/files/common/files.js';
@@ -18,6 +19,7 @@ import { IProcessService } 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-browser/window.js';
import { IAuthenticationService } from '../../../services/authentication/common/authentication.js';
import { BaseIssueReporterService } from '../browser/baseIssueReporterService.js';
import { IssueReporterData as IssueReporterModelData } from '../browser/issueReporterModel.js';
import { IIssueFormService, IssueReporterData, IssueType } from '../common/issue.js';
@@ -49,9 +51,11 @@ export class IssueReporter extends BaseIssueReporterService {
@IThemeService themeService: IThemeService,
@IFileService fileService: IFileService,
@IFileDialogService fileDialogService: IFileDialogService,
@IUpdateService private readonly updateService: IUpdateService
@IUpdateService private readonly updateService: IUpdateService,
@IContextMenuService contextMenuService: IContextMenuService,
@IAuthenticationService authenticationService: IAuthenticationService
) {
super(disableExtensions, data, os, product, window, false, issueFormService, themeService, fileService, fileDialogService);
super(disableExtensions, data, os, product, window, false, issueFormService, themeService, fileService, fileDialogService, contextMenuService, authenticationService);
this.processService = processService;
this.processService.getSystemInfo().then(info => {
this.issueReporterModel.update({ systemInfo: info });
@@ -164,7 +168,7 @@ export class IssueReporter extends BaseIssueReporterService {
return true;
}
public override async createIssue(): Promise<boolean> {
public override async createIssue(preview?: boolean): Promise<boolean> {
const selectedExtension = this.issueReporterModel.getData().selectedExtension;
const hasUri = this.nonGitHubIssueUrl;
// Short circuit if the extension provides a custom issue handler
@@ -230,7 +234,7 @@ export class IssueReporter extends BaseIssueReporterService {
url = this.addTemplateToUrl(url, gitHubDetails?.owner, gitHubDetails?.repositoryName);
if (this.data.githubAccessToken && gitHubDetails) {
if (this.data.githubAccessToken && gitHubDetails && !preview) {
if (await this.submitToGitHub(issueTitle, issueBody, gitHubDetails)) {
return true;
}
@@ -128,7 +128,14 @@ class NativeContextMenuService extends Disposable implements IContextMenuService
let zoom = getZoomFactor(dom.isHTMLElement(anchor) ? dom.getWindow(anchor) : dom.getActiveWindow());
if (dom.isHTMLElement(anchor)) {
const elementPosition = dom.getDomNodePagePosition(anchor);
const clientRect = anchor.getBoundingClientRect();
const elementPosition = { left: clientRect.left, top: clientRect.top, width: clientRect.width, height: clientRect.height };
// Determine if element is clipped by viewport; if so we'll use the bottom-right of the visible portion
const win = dom.getWindow(anchor);
const vw = win.innerWidth;
const vh = win.innerHeight;
const isClipped = clientRect.left < 0 || clientRect.top < 0 || clientRect.right > vw || clientRect.bottom > vh;
// When drawing context menus, we adjust the pixel position for native menus using zoom level
// In areas where zoom is applied to the element or its ancestors, we need to adjust accordingly
@@ -136,37 +143,43 @@ class NativeContextMenuService extends Disposable implements IContextMenuService
// Window Zoom Level: 1.5, Title Bar Zoom: 1/1.5, Coordinate Multiplier: 1.5 * 1.0 / 1.5 = 1.0
zoom *= dom.getDomNodeZoomLevel(anchor);
// Position according to the axis alignment and the anchor alignment:
// `HORIZONTAL` aligns at the top left or right of the anchor and
// `VERTICAL` aligns at the bottom left of the anchor.
if (delegate.anchorAxisAlignment === AnchorAxisAlignment.HORIZONTAL) {
if (delegate.anchorAlignment === AnchorAlignment.LEFT) {
x = elementPosition.left;
y = elementPosition.top;
} else {
x = elementPosition.left + elementPosition.width;
y = elementPosition.top;
}
if (!isMacintosh) {
const window = dom.getWindow(anchor);
const availableHeightForMenu = window.screen.height - y;
if (availableHeightForMenu < actions.length * (isWindows ? 45 : 32) /* guess of 1 menu item height */) {
// this is a guess to detect whether the context menu would
// open to the bottom from this point or to the top. If the
// menu opens to the top, make sure to align it to the bottom
// of the anchor and not to the top.
// this seems to be only necessary for Windows and Linux.
y += elementPosition.height;
}
}
if (isClipped) {
// Element is partially out of viewport: always place at bottom-right visible corner
x = Math.min(Math.max(clientRect.right, 0), vw);
y = Math.min(Math.max(clientRect.bottom, 0), vh);
} else {
if (delegate.anchorAlignment === AnchorAlignment.LEFT) {
x = elementPosition.left;
y = elementPosition.top + elementPosition.height;
// Position according to the axis alignment and the anchor alignment:
// `HORIZONTAL` aligns at the top left or right of the anchor and
// `VERTICAL` aligns at the bottom left of the anchor.
if (delegate.anchorAxisAlignment === AnchorAxisAlignment.HORIZONTAL) {
if (delegate.anchorAlignment === AnchorAlignment.LEFT) {
x = elementPosition.left;
y = elementPosition.top;
} else {
x = elementPosition.left + elementPosition.width;
y = elementPosition.top;
}
if (!isMacintosh) {
const window = dom.getWindow(anchor);
const availableHeightForMenu = window.screen.height - y;
if (availableHeightForMenu < actions.length * (isWindows ? 45 : 32) /* guess of 1 menu item height */) {
// this is a guess to detect whether the context menu would
// open to the bottom from this point or to the top. If the
// menu opens to the top, make sure to align it to the bottom
// of the anchor and not to the top.
// this seems to be only necessary for Windows and Linux.
y += elementPosition.height;
}
}
} else {
x = elementPosition.left + elementPosition.width;
y = elementPosition.top + elementPosition.height;
if (delegate.anchorAlignment === AnchorAlignment.LEFT) {
x = elementPosition.left;
y = elementPosition.top + elementPosition.height;
} else {
x = elementPosition.left + elementPosition.width;
y = elementPosition.top + elementPosition.height;
}
}
}