Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dialogs - better massage options #172022

Merged
merged 1 commit into from Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vs/workbench/browser/parts/editor/editorPanes.ts
Expand Up @@ -198,7 +198,7 @@ export class EditorPanes extends Disposable {
buttons.push(errorAction.label);
}
} else {
buttons.push(localize('ok', 'OK'));
buttons.push(localize({ key: 'ok', comment: ['&& denotes a mnemonic'] }, "&&OK"));
}

let cancelId: number | undefined = undefined;
Expand Down
Expand Up @@ -122,7 +122,7 @@ class BulkEditPreviewContribution {
const choice = await this._dialogService.show(
Severity.Info,
localize('overlap', "Another refactoring is being previewed."),
[localize('continue', "Continue"), localize('cancel', "Cancel")],
[localize({ key: 'continue', comment: ['&& denotes a mnemonic'] }, "&&Continue"), localize('cancel', "Cancel")],
{
detail: localize('detail', "Press 'Continue' to discard the previous refactoring and continue with the current refactoring."),
cancelId: 1
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/debug/browser/debugService.ts
Expand Up @@ -878,7 +878,7 @@ export class DebugService implements IDebugService {
const actions = errorActions.filter((action) => action.id.endsWith('.command')).length > 0 ?
errorActions :
[...errorActions, ...(promptLaunchJson ? [configureAction] : [])];
const { choice } = await this.dialogService.show(severity.Error, message, actions.map(a => a.label).concat(nls.localize('cancel', "Cancel")), { cancelId: actions.length });
const { choice } = await this.dialogService.show(severity.Error, message, actions.map(a => a.label).concat(nls.localize('cancel', "Cancel")), { cancelId: actions.length - 1 });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roblourens @connor4312 fyi, the cancel button was never assigned properly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks for the fix

if (choice < actions.length) {
await actions[choice].run();
}
Expand Down
Expand Up @@ -500,13 +500,13 @@ export class EditSessionsContribution extends Disposable implements IWorkbenchCo
conflictingChanges.length > 1 ?
localize('resume edit session warning many', 'Resuming your working changes from the cloud will overwrite the following {0} files. Do you want to proceed?', conflictingChanges.length) :
localize('resume edit session warning 1', 'Resuming your working changes from the cloud will overwrite {0}. Do you want to proceed?', basename(conflictingChanges[0].uri)),
[cancel, yes],
[yes, cancel],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joyceerhl please check, cancel buttons should be last in the dialog.

{
detail: conflictingChanges.length > 1 ? getFileNamesMessage(conflictingChanges.map((c) => c.uri)) : undefined,
cancelId: 0
cancelId: 1
});

if (result.choice === 0) {
if (result.choice === 1) {
return;
}
}
Expand Down
Expand Up @@ -104,8 +104,8 @@ export class PromptExtensionInstallFailureAction extends Action {
if (this.error.name === ExtensionManagementErrorCode.Unsupported) {
const productName = isWeb ? localize('VS Code for Web', "{0} for the Web", this.productService.nameLong) : this.productService.nameLong;
const message = localize('cannot be installed', "The '{0}' extension is not available in {1}. Click 'More Information' to learn more.", this.extension.displayName || this.extension.identifier.id, productName);
const result = await this.dialogService.show(Severity.Info, message, [localize('close', "Close"), localize('more information', "More Information")], { cancelId: 0 });
if (result.choice === 1) {
const result = await this.dialogService.show(Severity.Info, message, [localize('more information', "More Information"), localize('close', "Close")], { cancelId: 1 });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sandy081 fyi

if (result.choice === 0) {
this.openerService.open(isWeb ? URI.parse('https://aka.ms/vscode-web-extensions-guide') : URI.parse('https://aka.ms/vscode-remote'));
}
return;
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/files/browser/fileImportExport.ts
Expand Up @@ -466,7 +466,7 @@ export class ExternalFileImport {
localize('copyfolder', "Are you sure to want to copy '{0}'?", basename(folders[0].uri));
}

const { choice } = await this.dialogService.show(Severity.Info, message, buttons);
const { choice } = await this.dialogService.show(Severity.Info, message, buttons, { cancelId: buttons.length - 1 });

// Add folders
if (choice === buttons.length - 3) {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/tasks/browser/taskQuickPick.ts
Expand Up @@ -216,10 +216,10 @@ export class TaskQuickPick extends Disposable {
const changeSettingResult = await this._dialogService.show(Severity.Warning,
nls.localize('TaskQuickPick.changeSettingDetails',
"Task detection for {0} tasks causes files in any workspace you open to be run as code. Enabling {0} task detection is a user setting and will apply to any workspace you open. \n\n Do you want to enable {0} task detection for all workspaces?", selectedType),
[noButton, yesButton],
[yesButton, noButton],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@meganrogge fyi, I changed this so that cancelling the dialog would not execute the "Yes" button.

{ cancelId: 1 }
);
if (changeSettingResult.choice === 1) {
if (changeSettingResult.choice === 0) {
await this._configurationService.updateValue(`${selectedType}.autoDetect`, 'on');
await new Promise<void>(resolve => setTimeout(() => resolve(), 100));
return this.show(nls.localize('TaskService.pickRunTask', 'Select the task to run'), undefined, selectedType);
Expand Down
Expand Up @@ -113,12 +113,12 @@ export class OpenerValidatorContributions implements IWorkbenchContribution {
[
localize('open', 'Open'),
localize('copy', 'Copy'),
localize('cancel', 'Cancel'),
localize('configureTrustedDomains', 'Configure Trusted Domains')
localize('configureTrustedDomains', 'Configure Trusted Domains'),
localize('cancel', 'Cancel')
],
{
detail: typeof originalResource === 'string' ? originalResource : formattedLink,
cancelId: 2
cancelId: 3
}
);

Expand All @@ -131,7 +131,7 @@ export class OpenerValidatorContributions implements IWorkbenchContribution {
this._clipboardService.writeText(typeof originalResource === 'string' ? originalResource : resource.toString(true));
}
// Configure Trusted Domains
else if (choice === 3) {
else if (choice === 2) {
const pickedDomains = await configureOpenerTrustedDomainsHandler(
trustedDomains,
domainToOpen,
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/electron-sandbox/window.ts
Expand Up @@ -237,7 +237,7 @@ export class NativeWindow extends Disposable {
const result = await this.dialogService.input(Severity.Warning, localize('proxyAuthRequired', "Proxy Authentication Required"),
[
localize({ key: 'loginButton', comment: ['&& denotes a mnemonic'] }, "&&Log In"),
localize({ key: 'cancelButton', comment: ['&& denotes a mnemonic'] }, "&&Cancel")
localize('cancelButton', "Cancel")
],
[
{ placeholder: localize('username', "Username"), value: payload.username },
Expand Down
Expand Up @@ -232,7 +232,7 @@ export class FileDialogService extends AbstractFileDialogService implements IFil
buttons,
{
detail: localize('unsupportedBrowserDetail', "Your browser doesn't support opening local folders.\nYou can either open single files or open a remote repository."),
cancelId: -1 // no "Cancel" button offered
cancelId: buttons.length // TODO@bpasero leverage new support for `cancelId: null` in the future
}
);

Expand Down
10 changes: 9 additions & 1 deletion src/vs/workbench/services/host/browser/browserHostService.ts
Expand Up @@ -456,7 +456,15 @@ export class BrowserHostService extends Disposable implements IHostService {

const opened = await this.workspaceProvider.open(workspace, options);
if (!opened) {
const showResult = await this.dialogService.show(Severity.Warning, localize('unableToOpenExternal', "The browser interrupted the opening of a new tab or window. Press 'Open' to open it anyway."), [localize('open', "Open"), localize('cancel', "Cancel")], { cancelId: 1 });
const showResult = await this.dialogService.show(
Severity.Warning,
localize('unableToOpenExternal', "The browser interrupted the opening of a new tab or window. Press 'Open' to open it anyway."),
[
localize({ key: 'open', comment: ['&& denotes a mnemonic'] }, "&&Open"),
localize('cancel', "Cancel")
],
{ cancelId: 1 }
);
if (showResult.choice === 0) {
await this.workspaceProvider.open(workspace, options);
}
Expand Down
Expand Up @@ -27,7 +27,6 @@ import { IHostService } from 'vs/workbench/services/host/browser/host';
import { AbstractWorkspaceEditingService } from 'vs/workbench/services/workspaces/browser/abstractWorkspaceEditingService';
import { INativeHostService } from 'vs/platform/native/electron-sandbox/native';
import { isMacintosh } from 'vs/base/common/platform';
import { mnemonicButtonLabel } from 'vs/base/common/labels';
import { WorkingCopyBackupService } from 'vs/workbench/services/workingCopy/common/workingCopyBackupService';
import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
import { IWorkspaceTrustManagementService } from 'vs/platform/workspace/common/workspaceTrust';
Expand Down Expand Up @@ -95,8 +94,8 @@ export class NativeWorkspaceEditingService extends AbstractWorkspaceEditingServi
}

const buttons = [
{ label: mnemonicButtonLabel(localize('save', "Save")), result: ConfirmResult.SAVE },
{ label: mnemonicButtonLabel(localize('doNotSave', "Don't Save")), result: ConfirmResult.DONT_SAVE },
{ label: localize({ key: 'save', comment: ['&& denotes a mnemonic'] }, "&&Save"), result: ConfirmResult.SAVE },
{ label: localize({ key: 'doNotSave', comment: ['&& denotes a mnemonic'] }, "Do&&n't Save"), result: ConfirmResult.DONT_SAVE },
{ label: localize('cancel', "Cancel"), result: ConfirmResult.CANCEL }
];
const message = localize('saveWorkspaceMessage', "Do you want to save your workspace configuration as a file?");
Expand Down