notifications - tweak some or convert to modal dialog

src/vs/workbench/parts/cli/electron-browser/cli.contribution.ts

@@ -105,7 +105,7 @@ class InstallAction extends Action {
 		return new TPromise<void>((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"';

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 => {

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

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;

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<IConfirmationResult> {
-		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<boolean> {
-		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<number> {
 		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<number> {
@@ -163,30 +178,47 @@ export class DialogService implements IChoiceService, IConfirmationService {
 		return promise;
 	}
 
-	private doShowMessageBox(opts: Electron.MessageBoxOptions): TPromise<number> {
-		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 };
 	}
 }
 

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<void>(new Error(nls.localize('trashFailed', "Failed to move '{0}' to the trash", paths.basename(absolutePath))));
+			return TPromise.wrapError<void>(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));