From 739dac65c502ba88c703b95a046150bb051b7e46 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 26 Feb 2018 07:51:00 +0100 Subject: [PATCH] notifications - tweak some or convert to modal dialog --- .../cli/electron-browser/cli.contribution.ts | 2 +- .../electron-browser/extensionTipsService.ts | 3 +- .../node/extensionsWorkbenchService.ts | 9 +- .../files/electron-browser/fileActions.ts | 42 +++++++-- .../dialogs/electron-browser/dialogs.ts | 86 +++++++++++++------ .../files/electron-browser/fileService.ts | 4 +- 6 files changed, 100 insertions(+), 46 deletions(-) diff --git a/src/vs/workbench/parts/cli/electron-browser/cli.contribution.ts b/src/vs/workbench/parts/cli/electron-browser/cli.contribution.ts index b101ed20f12..599e2d524ad 100644 --- a/src/vs/workbench/parts/cli/electron-browser/cli.contribution.ts +++ b/src/vs/workbench/parts/cli/electron-browser/cli.contribution.ts @@ -105,7 +105,7 @@ class InstallAction extends Action { return new TPromise((c, e) => { const choices: Choice[] = [nls.localize('ok', "OK"), nls.localize('cancel2', "Cancel")]; - this.choiceService.choose(Severity.Info, nls.localize('warnEscalation', "Code will now prompt with 'osascript' for Administrator privileges to install the shell command."), choices).then(choice => { + this.choiceService.choose(Severity.Info, nls.localize('warnEscalation', "Code will now prompt with 'osascript' for Administrator privileges to install the shell command."), choices, 1, true).then(choice => { switch (choice) { case 0 /* OK */: const command = 'osascript -e "do shell script \\"mkdir -p /usr/local/bin && chown \\" & (do shell script (\\"whoami\\")) & \\" /usr/local/bin\\" with administrator privileges"'; diff --git a/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts b/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts index e8f205a4bd8..22aa8b5b00b 100644 --- a/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts +++ b/src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts @@ -687,8 +687,7 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe const message = localize('ignoreExtensionRecommendations', "Do you want to ignore all extension recommendations?"); const options = [ localize('ignoreAll', "Yes, Ignore All"), - localize('no', "No"), - localize('cancel', "Cancel") + localize('no', "No") ]; this.choiceService.choose(Severity.Info, message, options).done(choice => { diff --git a/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts b/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts index 5cc2c739f7b..db3317f7ec9 100644 --- a/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts +++ b/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts @@ -957,15 +957,14 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService { return this.open(extension).then(() => { const message = nls.localize('installConfirmation', "Would you like to install the '{0}' extension?", extension.displayName, extension.publisher); const options = [ - nls.localize('install', "Install"), - nls.localize('cancel', "Cancel") + nls.localize('install', "Install") ]; return this.choiceService.choose(Severity.Info, message, options).then(value => { - if (value !== 0) { - return TPromise.as(null); + if (value === 0) { + return this.install(extension); } - return this.install(extension); + return TPromise.as(null); }); }); }); diff --git a/src/vs/workbench/parts/files/electron-browser/fileActions.ts b/src/vs/workbench/parts/files/electron-browser/fileActions.ts index 24a519c554e..3b3b3e5c975 100644 --- a/src/vs/workbench/parts/files/electron-browser/fileActions.ts +++ b/src/vs/workbench/parts/files/electron-browser/fileActions.ts @@ -51,7 +51,7 @@ import { IListService, ListWidget } from 'vs/platform/list/browser/listService'; import { RawContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { distinctParents, basenameOrAuthority } from 'vs/base/common/resources'; import { Schemas } from 'vs/base/common/network'; -import { IConfirmationService, IConfirmationResult, IConfirmation } from 'vs/platform/dialogs/common/dialogs'; +import { IConfirmationService, IConfirmationResult, IConfirmation, IChoiceService } from 'vs/platform/dialogs/common/dialogs'; import { getConfirmMessage } from 'vs/workbench/services/dialogs/electron-browser/dialogs'; import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; @@ -568,7 +568,8 @@ class BaseDeleteFileAction extends BaseFileAction { @INotificationService notificationService: INotificationService, @IConfirmationService private confirmationService: IConfirmationService, @ITextFileService textFileService: ITextFileService, - @IConfigurationService private configurationService: IConfigurationService + @IConfigurationService private configurationService: IConfigurationService, + @IChoiceService private choiceService: IChoiceService ) { super('moveFileToTrash', MOVE_FILE_TO_TRASH_LABEL, fileService, notificationService, textFileService); @@ -689,17 +690,40 @@ class BaseDeleteFileAction extends BaseFileAction { this.tree.setFocus(distinctElements[0].parent); // move focus to parent } }, (error: any) => { - - // Allow to retry - let extraAction: Action; + const choices = [nls.localize('retry', "Retry"), nls.localize('cancel', "Cancel")]; if (this.useTrash) { - extraAction = new Action('permanentDelete', nls.localize('permDelete', "Delete Permanently"), null, true, () => { this.useTrash = false; this.skipConfirm = true; return this.run(); }); + choices.unshift(nls.localize('permDelete', "Delete Permanently")); } - this.onErrorWithRetry(error, () => this.run(), extraAction); + return this.choiceService.choose(Severity.Error, toErrorMessage(error, false), choices, choices.length - 1, true).then(choice => { + + // Focus back to tree + this.tree.DOMFocus(); + + + if (this.useTrash) { + switch (choice) { + case 0: /* Delete Permanently*/ + this.useTrash = false; + this.skipConfirm = true; + + return this.run(); + case 1: /* Retry */ + this.skipConfirm = true; - // Focus back to tree - this.tree.DOMFocus(); + return this.run(); + } + } else { + switch (choice) { + case 0: /* Retry */ + this.skipConfirm = true; + + return this.run(); + } + } + + return TPromise.as(void 0); + }); }); return servicePromise; diff --git a/src/vs/workbench/services/dialogs/electron-browser/dialogs.ts b/src/vs/workbench/services/dialogs/electron-browser/dialogs.ts index 23fb5e74959..f848ba8961e 100644 --- a/src/vs/workbench/services/dialogs/electron-browser/dialogs.ts +++ b/src/vs/workbench/services/dialogs/electron-browser/dialogs.ts @@ -9,7 +9,7 @@ import nls = require('vs/nls'); import product from 'vs/platform/node/product'; import { TPromise } from 'vs/base/common/winjs.base'; import Severity from 'vs/base/common/severity'; -import { isLinux } from 'vs/base/common/platform'; +import { isLinux, isMacintosh } from 'vs/base/common/platform'; import { Action } from 'vs/base/common/actions'; import { IWindowService } from 'vs/platform/windows/common/windows'; import { mnemonicButtonLabel } from 'vs/base/common/labels'; @@ -19,6 +19,21 @@ import { once } from 'vs/base/common/event'; import URI from 'vs/base/common/uri'; import { basename } from 'vs/base/common/paths'; +interface IMassagedMessageBoxOptions { + + /** + * OS massaged message box options. + */ + options: Electron.MessageBoxOptions; + + /** + * Since the massaged result of the message box options potentially + * changes the order of buttons, we have to keep a map of these + * changes so that we can still return the correct index to the caller. + */ + buttonIndexMap: number[]; +} + export class DialogService implements IChoiceService, IConfirmationService { public _serviceBrand: any; @@ -30,22 +45,20 @@ export class DialogService implements IChoiceService, IConfirmationService { } public confirmWithCheckbox(confirmation: IConfirmation): TPromise { - const opts = this.massageMessageBoxOptions(this.getConfirmOptions(confirmation)); - - return this.windowService.showMessageBox(opts).then(result => { - const button = isLinux ? opts.buttons.length - result.button - 1 : result.button; + const { options, buttonIndexMap } = this.massageMessageBoxOptions(this.getConfirmOptions(confirmation)); + return this.windowService.showMessageBox(options).then(result => { return { - confirmed: button === 0 ? true : false, + confirmed: buttonIndexMap[result.button] === 0 ? true : false, checkboxChecked: result.checkboxChecked } as IConfirmationResult; }); } public confirm(confirmation: IConfirmation): TPromise { - const opts = this.getConfirmOptions(confirmation); + const { options, buttonIndexMap } = this.massageMessageBoxOptions(this.getConfirmOptions(confirmation)); - return this.doShowMessageBox(opts).then(result => result === 0 ? true : false); + return this.windowService.showMessageBox(options).then(result => buttonIndexMap[result.button] === 0 ? true : false); } private getConfirmOptions(confirmation: IConfirmation): Electron.MessageBoxOptions { @@ -97,16 +110,18 @@ export class DialogService implements IChoiceService, IConfirmationService { private doChooseWithDialog(severity: Severity, message: string, choices: Choice[], cancelId?: number): TPromise { const type: 'none' | 'info' | 'error' | 'question' | 'warning' = severity === Severity.Info ? 'question' : severity === Severity.Error ? 'error' : severity === Severity.Warning ? 'warning' : 'none'; - const options: string[] = []; + const stringChoices: string[] = []; choices.forEach(choice => { if (typeof choice === 'string') { - options.push(choice); + stringChoices.push(choice); } else { - options.push(choice.label); + stringChoices.push(choice.label); } }); - return this.doShowMessageBox({ message, buttons: options, type, cancelId }); + const { options, buttonIndexMap } = this.massageMessageBoxOptions({ message, buttons: stringChoices, type, cancelId }); + + return this.windowService.showMessageBox(options).then(result => buttonIndexMap[result.button]); } private doChooseWithNotification(severity: Severity, message: string, choices: Choice[]): TPromise { @@ -163,30 +178,47 @@ export class DialogService implements IChoiceService, IConfirmationService { return promise; } - private doShowMessageBox(opts: Electron.MessageBoxOptions): TPromise { - opts = this.massageMessageBoxOptions(opts); + private massageMessageBoxOptions(options: Electron.MessageBoxOptions): IMassagedMessageBoxOptions { + let buttonIndexMap = options.buttons.map((button, index) => index); - return this.windowService.showMessageBox(opts).then(result => isLinux ? opts.buttons.length - result.button - 1 : result.button); - } + options.buttons = options.buttons.map(button => mnemonicButtonLabel(button)); - private massageMessageBoxOptions(opts: Electron.MessageBoxOptions): Electron.MessageBoxOptions { - opts.buttons = opts.buttons.map(button => mnemonicButtonLabel(button)); - opts.buttons = isLinux ? opts.buttons.reverse() : opts.buttons; + // Linux: order of buttons is reverse + // macOS: also reverse, but the OS handles this for us! + if (isLinux) { + options.buttons = options.buttons.reverse(); + buttonIndexMap = buttonIndexMap.reverse(); + } - if (opts.defaultId !== void 0) { - opts.defaultId = isLinux ? opts.buttons.length - opts.defaultId - 1 : opts.defaultId; + // Default Button + if (options.defaultId !== void 0) { + options.defaultId = buttonIndexMap[options.defaultId]; } else if (isLinux) { - opts.defaultId = opts.buttons.length - 1; // since we reversed the buttons + options.defaultId = buttonIndexMap[0]; } - if (opts.cancelId !== void 0) { - opts.cancelId = isLinux ? opts.buttons.length - opts.cancelId - 1 : opts.cancelId; + // Cancel Button + if (options.cancelId !== void 0) { + + // macOS: the cancel button should always be to the left of the primary action + // if we see more than 2 buttons, move the cancel one to the left of the primary + if (isMacintosh && options.buttons.length > 2 && options.cancelId !== 1) { + const cancelButton = options.buttons[options.cancelId]; + options.buttons.splice(options.cancelId, 1); + options.buttons.splice(1, 0, cancelButton); + + const cancelButtonIndex = buttonIndexMap[options.cancelId]; + buttonIndexMap.splice(cancelButtonIndex, 1); + buttonIndexMap.splice(1, 0, cancelButtonIndex); + } + + options.cancelId = buttonIndexMap[options.cancelId]; } - opts.noLink = true; - opts.title = opts.title || product.nameLong; + options.noLink = true; + options.title = options.title || product.nameLong; - return opts; + return { options, buttonIndexMap }; } } diff --git a/src/vs/workbench/services/files/electron-browser/fileService.ts b/src/vs/workbench/services/files/electron-browser/fileService.ts index 53b48bd8607..8704b7f68f2 100644 --- a/src/vs/workbench/services/files/electron-browser/fileService.ts +++ b/src/vs/workbench/services/files/electron-browser/fileService.ts @@ -21,7 +21,7 @@ import { IStorageService, StorageScope } from 'vs/platform/storage/common/storag import Event, { Emitter } from 'vs/base/common/event'; import { shell } from 'electron'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/resourceConfiguration'; -import { isMacintosh } from 'vs/base/common/platform'; +import { isMacintosh, isWindows } from 'vs/base/common/platform'; import product from 'vs/platform/node/product'; import { Schemas } from 'vs/base/common/network'; import { Severity } from 'vs/platform/notification/common/notification'; @@ -238,7 +238,7 @@ export class FileService implements IFileService { const absolutePath = resource.fsPath; const result = shell.moveItemToTrash(absolutePath); if (!result) { - return TPromise.wrapError(new Error(nls.localize('trashFailed', "Failed to move '{0}' to the trash", paths.basename(absolutePath)))); + return TPromise.wrapError(new Error(isWindows ? nls.localize('binFailed', "Failed to move '{0}' to the recycle bin", paths.basename(absolutePath)) : nls.localize('trashFailed', "Failed to move '{0}' to the trash", paths.basename(absolutePath)))); } this._onAfterOperation.fire(new FileOperationEvent(resource, FileOperation.DELETE)); -- GitLab