Skip to content

Commit

Permalink
Issue Reporter: UX issues, fixes #42856
Browse files Browse the repository at this point in the history 
* Open issue reporter in same display as workbench

* Styling updates

* Increase url length limit

* normalize extension ids before checking isEnabled
  • Loading branch information
Rachel Macfarlane committed Feb 3, 2018
1 parent 443fd78 commit 683fcaa
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 42 deletions.
13 changes: 5 additions & 8 deletions src/vs/code/electron-browser/issue/issueReporterMain.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ export class IssueReporter extends Disposable {
const descriptionTitle = document.getElementById('issue-description-label');
const descriptionSubtitle = document.getElementById('issue-description-subtitle');


if (issueType === IssueType.Bug) {
show(systemBlock);
hide(processBlock);
Expand All @@ -358,8 +359,7 @@ export class IssueReporter extends Disposable {
show(disabledExtensions);

descriptionTitle.innerHTML = `${localize('stepsToReproduce', "Steps to Reproduce")} <span class="required-input">*</span>`;
show(descriptionSubtitle);
descriptionSubtitle.innerHTML = localize('bugDescription', "How did you encounter this problem? What steps do you need to perform to reliably reproduce the problem? What did you expect to happen and what actually did happen?");
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.");
} else if (issueType === IssueType.PerformanceIssue) {
show(systemBlock);
show(processBlock);
Expand All @@ -368,8 +368,7 @@ export class IssueReporter extends Disposable {
show(disabledExtensions);

descriptionTitle.innerHTML = `${localize('stepsToReproduce', "Steps to Reproduce")} <span class="required-input">*</span>`;
show(descriptionSubtitle);
descriptionSubtitle.innerHTML = localize('performanceIssueDesciption', "When did this performance issue happen? For example, does it occur on startup or after a specific series of actions? Any details you can provide help our investigation.");
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.");
} else {
hide(systemBlock);
hide(processBlock);
Expand All @@ -378,18 +377,16 @@ export class IssueReporter extends Disposable {
hide(disabledExtensions);

descriptionTitle.innerHTML = `${localize('description', "Description")} <span class="required-input">*</span>`;
hide(descriptionSubtitle);
descriptionSubtitle.innerHTML = localize('featureRequestDescription', "Please describe the feature you would like to see. We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub.");
}
}

private validateInput(inputId: string): boolean {
const inputElement = (<HTMLInputElement>document.getElementById(inputId));
if (!inputElement.value) {
show(document.getElementById(`${inputId}-validation-error`));
inputElement.classList.add('invalid-input');
return false;
} else {
hide(document.getElementById(`${inputId}-validation-error`));
inputElement.classList.remove('invalid-input');
return true;
}
Expand Down Expand Up @@ -438,7 +435,7 @@ export class IssueReporter extends Disposable {
const url = baseUrl + encodeURIComponent(issueBody);

const lengthValidationElement = document.getElementById('url-length-validation-error');
if (url.length > 2081) {
if (url.length > 5400) {
lengthValidationElement.textContent = localize('urlLengthError', "The data exceeds the length limit of 2081. The data is length {0}.", url.length);
show(lengthValidationElement);
return false;
Expand Down
26 changes: 5 additions & 21 deletions src/vs/code/electron-browser/issue/issueReporterPage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,31 @@

import { escape } from 'vs/base/common/strings';
import { localize } from 'vs/nls';
import * as os from 'os';
import pkg from 'vs/platform/node/package';

export default (): string => `
<div id="issue-reporter">
<div id="english" class="input-group hidden">${escape(localize('completeInEnglish', "Please complete the form in English."))}</div>
<div class="section">
<div class="input-group">
<label for="issue-type">${escape(localize('issueTypeLabel', "I want to submit a"))}</label>
<select id="issue-type" class="form-control">
<label id="issue-type-label" class="inline-form-control" for="issue-type">${escape(localize('issueTypeLabel', "This is a"))}</label>
<select id="issue-type" class="inline-form-control">
<option value="0">${escape(localize('bugReporter', "Bug Report"))}</option>
<option value="1">${escape(localize('performanceIssue', "Performance Issue"))}</option>
<option value="2">${escape(localize('featureRequest', "Feature Request"))}</option>
</select>
</div>
<div class="input-group">
<label for="issue-title">${escape(localize('issueTitleLabel', "Title"))} <span class="required-input">*</span></label>
<div id="issue-title-validation-error" class="validation-error hidden" role="alert">${escape(localize('issueTitleRequired', "Please enter a title."))}</div>
<input id="issue-title" type="text" required>
<label id="issue-title-label" for="issue-title">${escape(localize('issueTitleLabel', "Title"))} <span class="required-input">*</span></label>
<input id="issue-title" type="text" class="inline-form-control" placeholder="${escape(localize('issueTitleRequired', "Please enter a title."))}" required>
<small id="similar-issues">
<!-- To be dynamically filled -->
</small>
</div>
</div>
<div class="system-info">
<div class="input-group">
<div class="two-col">
<label for="vscode-version">${escape(localize('vscodeVersion', "VS Code Version"))}</label>
<input id="vscode-version" type="text" value="${pkg.name} ${pkg.version}" disabled/>
</div>
<div class="two-col">
<label for="os">${escape(localize('osVersion', "OS Version"))}</label>
<input id="os" type="text" value="${os.type()} ${os.arch()} ${os.release()}" disabled/>
</div>
</div>
<div id="block-container">
<div class="block block-system">
<details>
Expand Down Expand Up @@ -123,9 +109,7 @@ export default (): string => `
<!-- To be dynamically filled -->
</div>
<div class="block-info-text">
<div class="instructions">${escape(localize('githubMarkdown', "We support GitHub-flavored Markdown. You will be able to edit your issue and add screenshots when we preview it on GitHub."))}</div>
<div id="description-validation-error" class="validation-error hidden" role="alert">${escape(localize('issueDescriptionRequired', "Please enter a description."))}</div>
<textarea name="description" id="description" cols="100" rows="15" required></textarea>
<textarea name="description" id="description" cols="100" rows="12" placeholder="${escape(localize('details', "Please enter details."))}" required></textarea>
</div>
</div>
Expand Down
45 changes: 38 additions & 7 deletions src/vs/code/electron-browser/issue/media/issueReporter.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,29 @@ td {
}

.section {
margin-bottom: 2em;
margin-bottom: 1.5em;
}

#issue-type-label {
width: 12%;
}

#issue-type {
width: calc(88% - 5px);
}

#issue-title-label {
width: 10%;
display: inline-block
}

#issue-title {
width: calc(90% - 5px);
}

#similar-issues {
margin-left: 67px;
display: block;
}

/**
Expand All @@ -48,6 +70,11 @@ input[type="text"], textarea {
border-radius: .25rem;
border: 1px solid #ced4da;
}

.inline-form-control {
display: inline-block !important;
}

textarea {
overflow: auto;
resize: vertical;
Expand All @@ -73,10 +100,9 @@ button {

select {
height: calc(2.25rem + 2px);
display: block;
width: 100%;
padding: 0.375rem 0.75rem;
font-size: 1rem;
display: inline-block;
padding: 3px 3px;
font-size: 14px;
line-height: 1.5;
color: #495057;
background-color: #fff;
Expand All @@ -87,6 +113,11 @@ select {
* {
box-sizing: border-box;
}

textarea {
font-family: -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "HelveticaNeue-Light", "Ubuntu", "Droid Sans", sans-serif;
}

html {
font-family: -apple-system, BlinkMacSystemFont, "Segoe WPC", "Segoe UI", "HelveticaNeue-Light", "Ubuntu", "Droid Sans", sans-serif;
color: #CCCCCC;
Expand All @@ -107,7 +138,7 @@ body {
.block .block-info {
width: 100%;
font-family: 'Menlo', 'Courier New', 'Courier', monospace;
font-size: 14px;
font-size: 12px;
overflow: auto;
overflow-wrap: break-word;
}
Expand Down Expand Up @@ -172,7 +203,7 @@ select, input, textarea {

summary {
border: 1px solid transparent;
padding: 10px;
padding: 0 10px;
margin-bottom: 5px;
}

Expand Down
57 changes: 52 additions & 5 deletions src/vs/platform/issue/electron-main/issueService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ import { localize } from 'vs/nls';
import * as objects from 'vs/base/common/objects';
import { parseArgs } from 'vs/platform/environment/node/argv';
import { IIssueService, IssueReporterData } from 'vs/platform/issue/common/issue';
import { BrowserWindow, ipcMain } from 'electron';
import { BrowserWindow, ipcMain, screen } from 'electron';
import { ILaunchService } from 'vs/code/electron-main/launch';
import { buildDiagnostics, DiagnosticInfo } from 'vs/code/electron-main/diagnostics';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { isMacintosh } from 'vs/base/common/platform';

const DEFAULT_BACKGROUND_COLOR = '#1E1E1E';

export class IssueService implements IIssueService {
_serviceBrand: any;
_issueWindow: BrowserWindow;
_parentWindow: BrowserWindow;

constructor(
private machineId: string,
Expand All @@ -35,14 +37,17 @@ export class IssueService implements IIssueService {
});

ipcMain.on('workbenchCommand', (event, arg) => {
this._issueWindow.getParentWindow().webContents.send('vscode:runAction', { id: arg });
this._parentWindow.webContents.send('vscode:runAction', { id: arg });
});

this._parentWindow = BrowserWindow.getFocusedWindow();
const position = this.getWindowPosition();
this._issueWindow = new BrowserWindow({
width: 750,
height: 1100,
width: position.width,
height: position.height,
x: position.x,
y: position.y,
title: localize('issueReporter', "Issue Reporter"),
parent: BrowserWindow.getFocusedWindow(),
backgroundColor: data.styles.backgroundColor || DEFAULT_BACKGROUND_COLOR
});

Expand All @@ -53,6 +58,48 @@ export class IssueService implements IIssueService {
return TPromise.as(null);
}

private getWindowPosition() {
// We want the new window to open on the same display that the parent is in
let displayToUse: Electron.Display;
const displays = screen.getAllDisplays();

// Single Display
if (displays.length === 1) {
displayToUse = displays[0];
}

// Multi Display
else {

// on mac there is 1 menu per window so we need to use the monitor where the cursor currently is
if (isMacintosh) {
const cursorPoint = screen.getCursorScreenPoint();
displayToUse = screen.getDisplayNearestPoint(cursorPoint);
}

// if we have a last active window, use that display for the new window
if (!displayToUse && this._parentWindow) {
displayToUse = screen.getDisplayMatching(this._parentWindow.getBounds());
}

// fallback to primary display or first display
if (!displayToUse) {
displayToUse = screen.getPrimaryDisplay() || displays[0];
}
}

let state = {
width: 750,
height: 900,
x: undefined,
y: undefined
};
state.x = displayToUse.bounds.x + (displayToUse.bounds.width / 2) - (state.width / 2);
state.y = displayToUse.bounds.y + (displayToUse.bounds.height / 2) - (state.height / 2);

return state;
}

private getStatusInfo(): TPromise<DiagnosticInfo> {
return new Promise((resolve, reject) => {
this.launchService.getMainProcessInfo().then(info => {
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/electron-browser/actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import { IIssueService, IssueReporterData, IssueType, IssueReporterStyles } from
import { IThemeService, ITheme } from 'vs/platform/theme/common/themeService';
import { textLinkForeground, inputBackground, inputBorder, inputForeground, buttonBackground, buttonHoverBackground, buttonForeground, inputValidationErrorBorder, foreground, inputActiveOptionBorder } from 'vs/platform/theme/common/colorRegistry';
import { SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme';
import { getGalleryExtensionIdFromLocal } from 'vs/platform/extensionManagement/common/extensionManagementUtil';

// --- actions

Expand Down Expand Up @@ -896,7 +897,7 @@ export class OpenIssueReporterAction extends Action {

public run(): TPromise<boolean> {
return this.extensionManagementService.getInstalled(LocalExtensionType.User).then(extensions => {
const enabledExtensions = extensions.filter(extension => this.extensionEnablementService.isEnabled(extension.identifier));
const enabledExtensions = extensions.filter(extension => this.extensionEnablementService.isEnabled({ id: getGalleryExtensionIdFromLocal(extension) }));
const theme = this.themeService.getTheme();
const issueReporterData: IssueReporterData = {
styles: getIssueReporterStyles(theme),
Expand Down

0 comments on commit 683fcaa

Please sign in to comment.