notifications - fix keepOpen not working (fix microsoft#58689)

jvasileff · Sep 14, 2018 · 4302deb · 4302deb
1 parent 6edaab7
commit 4302deb
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 27 deletions.
diff --git a/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts b/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts
@@ -20,7 +20,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
 import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
 import { DropdownMenuActionItem } from 'vs/base/browser/ui/dropdown/dropdown';
-import { INotificationViewItem, NotificationViewItem, NotificationViewItemLabelKind, INotificationMessage } from 'vs/workbench/common/notifications';
+import { INotificationViewItem, NotificationViewItem, NotificationViewItemLabelKind, INotificationMessage, ChoiceAction } from 'vs/workbench/common/notifications';
 import { ClearNotificationAction, ExpandNotificationAction, CollapseNotificationAction, ConfigureNotificationAction } from 'vs/workbench/browser/parts/notifications/notificationsActions';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar';
@@ -449,8 +449,10 @@ export class NotificationTemplateRenderer {
 					// Run action
 					this.actionRunner.run(action, notification);
 
-					// Hide notification
-					notification.close();
+					// Hide notification (unless explicitly prevented)
+					if (!(action instanceof ChoiceAction) || !action.keepOpen) {
+						notification.close();
+					}
 				}));
 
 				this.inputDisposeables.push(attachButtonStyler(button, this.themeService));

diff --git a/src/vs/workbench/common/notifications.ts b/src/vs/workbench/common/notifications.ts
@@ -5,11 +5,13 @@
 
 'use strict';
 
-import { INotification, INotificationHandle, INotificationActions, INotificationProgress, NoOpNotification, Severity, NotificationMessage } from 'vs/platform/notification/common/notification';
+import { INotification, INotificationHandle, INotificationActions, INotificationProgress, NoOpNotification, Severity, NotificationMessage, IPromptChoice } from 'vs/platform/notification/common/notification';
 import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { Event, Emitter, once } from 'vs/base/common/event';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { isPromiseCanceledError, isErrorWithActions } from 'vs/base/common/errors';
+import { Action } from 'vs/base/common/actions';
+import { TPromise } from 'vs/base/common/winjs.base';
 
 export interface INotificationsModel {
 
@@ -544,4 +546,27 @@ export class NotificationViewItem extends Disposable implements INotificationVie
 
 		return true;
 	}
+}
+
+export class ChoiceAction extends Action {
+	private _clicked: boolean;
+
+	constructor(id: string, private choice: IPromptChoice) {
+		super(id, choice.label, null, true, () => {
+			this._clicked = true;
+
+			// Pass to runner
+			choice.run();
+
+			return TPromise.as(void 0);
+		});
+	}
+
+	get keepOpen(): boolean {
+		return this.choice.keepOpen;
+	}
+
+	get clicked(): boolean {
+		return this._clicked;
+	}
 }
diff --git a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts
@@ -423,10 +423,6 @@ export class ExtensionService extends Disposable implements IExtensionService {
 				this._instantiationService.invokeFunction((accessor) => {
 					const editorService = accessor.get(IEditorService);
 					editorService.openEditor(this._instantiationService.createInstance(RuntimeExtensionsInput), { revealIfOpened: true });
-					// keepOpen does not appear to work
-					setTimeout(() => {
-						this._onResponsiveStateChanged(state);
-					}, 100);
 				});
 			}
 		};

diff --git a/src/vs/workbench/services/notification/common/notificationService.ts b/src/vs/workbench/services/notification/common/notificationService.ts
@@ -6,10 +6,8 @@
 'use strict';
 
 import { INotificationService, INotification, INotificationHandle, Severity, NotificationMessage, INotificationActions, IPromptChoice } from 'vs/platform/notification/common/notification';
-import { INotificationsModel, NotificationsModel } from 'vs/workbench/common/notifications';
+import { INotificationsModel, NotificationsModel, ChoiceAction } from 'vs/workbench/common/notifications';
 import { dispose, Disposable } from 'vs/base/common/lifecycle';
-import { TPromise } from 'vs/base/common/winjs.base';
-import { Action } from 'vs/base/common/actions';
 import { once } from 'vs/base/common/event';
 
 export class NotificationService extends Disposable implements INotificationService {
@@ -58,25 +56,11 @@ export class NotificationService extends Disposable implements INotificationServ
 
 	prompt(severity: Severity, message: string, choices: IPromptChoice[], onCancel?: () => void): INotificationHandle {
 		let handle: INotificationHandle;
-		let choiceClicked = false;
 
 		// Convert choices into primary/secondary actions
 		const actions: INotificationActions = { primary: [], secondary: [] };
 		choices.forEach((choice, index) => {
-			const action = new Action(`workbench.dialog.choice.${index}`, choice.label, null, true, () => {
-				choiceClicked = true;
-
-				// Pass to runner
-				choice.run();
-
-				// Close notification unless we are told to keep open
-				if (!choice.keepOpen) {
-					handle.close();
-				}
-
-				return TPromise.as(void 0);
-			});
-
+			const action = new ChoiceAction(`workbench.dialog.choice.${index}`, choice);
 			if (!choice.isSecondary) {
 				actions.primary.push(action);
 			} else {
@@ -93,7 +77,7 @@ export class NotificationService extends Disposable implements INotificationServ
 			dispose(...actions.primary, ...actions.secondary);
 
 			// Indicate cancellation to the outside if no action was executed
-			if (!choiceClicked && typeof onCancel === 'function') {
+			if (typeof onCancel === 'function' && ![...actions.primary, ...actions.secondary].some((action: ChoiceAction) => action.clicked)) {
 				onCancel();
 			}
 		});