Notifications: indicate progress notification if hidden in notification center (fix #90274)

src/vs/workbench/browser/parts/notifications/notificationsStatus.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { INotificationsModel, INotificationChangeEvent, NotificationChangeType, INotificationViewItem, IStatusMessageChangeEvent, StatusMessageChangeType, IStatusMessageViewItem } from 'vs/workbench/common/notifications';
+import { INotificationsModel, INotificationChangeEvent, NotificationChangeType, IStatusMessageChangeEvent, StatusMessageChangeType, IStatusMessageViewItem } from 'vs/workbench/common/notifications';
 import { IStatusbarService, StatusbarAlignment, IStatusbarEntryAccessor, IStatusbarEntry } from 'vs/workbench/services/statusbar/common/statusbar';
 import { Disposable, IDisposable, dispose } from 'vs/base/common/lifecycle';
 import { HIDE_NOTIFICATIONS_CENTER, SHOW_NOTIFICATIONS_CENTER } from 'vs/workbench/browser/parts/notifications/notificationsCommands';
@@ -12,11 +12,12 @@ import { localize } from 'vs/nls';
 export class NotificationsStatus extends Disposable {
 
 	private notificationsCenterStatusItem: IStatusbarEntryAccessor | undefined;
-	private currentNotifications = new Set<INotificationViewItem>();
+	private newNotificationsCount = 0;
 
 	private currentStatusMessage: [IStatusMessageViewItem, IDisposable] | undefined;
 
-	private isNotificationsCenterVisible: boolean | undefined;
+	private isNotificationsCenterVisible: boolean = false;
+	private isNotificationsToastsVisible: boolean = false;
 
 	constructor(
 		private model: INotificationsModel,
@@ -39,28 +40,39 @@ export class NotificationsStatus extends Disposable {
 	}
 
 	private onDidChangeNotification(e: INotificationChangeEvent): void {
-		if (this.isNotificationsCenterVisible) {
-			return; // no change if notification center is visible
-		}
 
-		// Notification got Added
-		if (e.kind === NotificationChangeType.ADD) {
-			this.currentNotifications.add(e.item);
-		}
-
-		// Notification got Removed
-		else if (e.kind === NotificationChangeType.REMOVE) {
-			this.currentNotifications.delete(e.item);
+		// Consider a notification as unread as long as it only
+		// appeared as toast and not in the notification center
+		if (!this.isNotificationsCenterVisible) {
+			if (e.kind === NotificationChangeType.ADD) {
+				this.newNotificationsCount++;
+			} else if (e.kind === NotificationChangeType.REMOVE) {
+				this.newNotificationsCount--;
+			}
 		}
 
+		// Update in status bar
 		this.updateNotificationsCenterStatusItem();
 	}
 
 	private updateNotificationsCenterStatusItem(): void {
+
+		// Figure out how many notifications have progress only if neither
+		// toasts are visible nor center is visible. In that case we still
+		// want to give a hint to the user that something is running.
+		let notificationsInProgress = 0;
+		if (!this.isNotificationsCenterVisible && !this.isNotificationsToastsVisible) {
+			for (const notification of this.model.notifications) {
+				if (notification.hasProgress) {
+					notificationsInProgress++;
+				}
+			}
+		}
+
 		const statusProperties: IStatusbarEntry = {
-			text: this.currentNotifications.size === 0 ? '$(bell)' : '$(bell-dot)',
+			text: `${this.newNotificationsCount === 0 ? '$(bell)' : '$(bell-dot)'}${notificationsInProgress > 0 ? ' $(sync~spin)' : ''}`,
 			command: this.isNotificationsCenterVisible ? HIDE_NOTIFICATIONS_CENTER : SHOW_NOTIFICATIONS_CENTER,
-			tooltip: this.getTooltip(),
+			tooltip: this.getTooltip(notificationsInProgress),
 			showBeak: this.isNotificationsCenterVisible
 		};
 
@@ -77,7 +89,7 @@ export class NotificationsStatus extends Disposable {
 		}
 	}
 
-	private getTooltip(): string {
+	private getTooltip(notificationsInProgress: number): string {
 		if (this.isNotificationsCenterVisible) {
 			return localize('hideNotifications', "Hide Notifications");
 		}
@@ -86,23 +98,45 @@ export class NotificationsStatus extends Disposable {
 			return localize('zeroNotifications', "No Notifications");
 		}
 
-		if (this.currentNotifications.size === 0) {
-			return localize('noNotifications', "No New Notifications");
+		if (notificationsInProgress === 0) {
+			if (this.newNotificationsCount === 0) {
+				return localize('noNotifications', "No New Notifications");
+			}
+
+			if (this.newNotificationsCount === 1) {
+				return localize('oneNotification', "1 New Notification");
+			}
+
+			return localize('notifications', "{0} New Notifications", this.newNotificationsCount);
+		}
+
+		if (this.newNotificationsCount === 0) {
+			return localize('noNotificationsWithProgress', "No New Notifications ({0} in progress)", notificationsInProgress);
 		}
 
-		if (this.currentNotifications.size === 1) {
-			return localize('oneNotification', "1 New Notification");
+		if (this.newNotificationsCount === 1) {
+			return localize('oneNotificationWithProgress', "1 New Notification ({0} in progress)", notificationsInProgress);
 		}
 
-		return localize('notifications', "{0} New Notifications", this.currentNotifications.size);
+		return localize('notificationsWithProgress', "{0} New Notifications ({0} in progress)", this.newNotificationsCount, notificationsInProgress);
 	}
 
-	update(isCenterVisible: boolean): void {
+	update(isCenterVisible: boolean, isToastsVisible: boolean): void {
+		let updateNotificationsCenterStatusItem = false;
+
 		if (this.isNotificationsCenterVisible !== isCenterVisible) {
 			this.isNotificationsCenterVisible = isCenterVisible;
+			this.newNotificationsCount = 0; // Showing the notification center resets the unread counter to 0
+			updateNotificationsCenterStatusItem = true;
+		}
+
+		if (this.isNotificationsToastsVisible !== isToastsVisible) {
+			this.isNotificationsToastsVisible = isToastsVisible;
+			updateNotificationsCenterStatusItem = true;
+		}
 
-			// Showing the notification center resets the counter to 0
-			this.currentNotifications.clear();
+		// Update in status bar as needed
+		if (updateNotificationsCenterStatusItem) {
 			this.updateNotificationsCenterStatusItem();
 		}
 	}

src/vs/workbench/browser/parts/notifications/notificationsToasts.ts

@@ -9,7 +9,7 @@ import { IDisposable, dispose, toDisposable, DisposableStore } from 'vs/base/com
 import { addClass, removeClass, isAncestor, addDisposableListener, EventType, Dimension } from 'vs/base/browser/dom';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { NotificationsList } from 'vs/workbench/browser/parts/notifications/notificationsList';
-import { Event } from 'vs/base/common/event';
+import { Event, Emitter } from 'vs/base/common/event';
 import { IWorkbenchLayoutService, Parts } from 'vs/workbench/services/layout/browser/layoutService';
 import { Themable, NOTIFICATIONS_TOAST_BORDER, NOTIFICATIONS_BACKGROUND } from 'vs/workbench/common/theme';
 import { IThemeService } from 'vs/platform/theme/common/themeService';
@@ -53,6 +53,12 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 		return intervals;
 	})();
 
+	private readonly _onDidChangeVisibility = this._register(new Emitter<void>());
+	readonly onDidChangeVisibility = this._onDidChangeVisibility.event;
+
+	private _isVisible = false;
+	get isVisible(): boolean { return !!this._isVisible; }
+
 	private notificationsToastsContainer: HTMLElement | undefined;
 	private workbenchDimensions: Dimension | undefined;
 	private isNotificationsCenterVisible: boolean | undefined;
@@ -125,11 +131,11 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 
 	private addToast(item: INotificationViewItem): void {
 		if (this.isNotificationsCenterVisible) {
-			return; // do not show toasts while notification center is visibles
+			return; // do not show toasts while notification center is visible
 		}
 
 		if (item.silent) {
-			return; // do not show toats for silenced notifications
+			return; // do not show toasts for silenced notifications
 		}
 
 		// Lazily create toasts containers
@@ -174,7 +180,7 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 		this.mapNotificationToToast.set(item, toast);
 
 		itemDisposables.add(toDisposable(() => {
-			if (this.isVisible(toast) && notificationsToastsContainer) {
+			if (this.isToastVisible(toast) && notificationsToastsContainer) {
 				notificationsToastsContainer.removeChild(toast.container);
 			}
 		}));
@@ -229,6 +235,12 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 			removeClass(notificationToast, 'notification-fade-in');
 			addClass(notificationToast, 'notification-fade-in-done');
 		}));
+
+		// Events
+		if (!this._isVisible) {
+			this._isVisible = true;
+			this._onDidChangeVisibility.fire();
+		}
 	}
 
 	private purgeNotification(item: INotificationViewItem, notificationToastContainer: HTMLElement, notificationList: NotificationsList, disposables: DisposableStore): void {
@@ -325,6 +337,12 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 
 		// Context Key
 		this.notificationsToastsVisibleContextKey.set(false);
+
+		// Events
+		if (this._isVisible) {
+			this._isVisible = false;
+			this._onDidChangeVisibility.fire();
+		}
 	}
 
 	hide(): void {
@@ -441,12 +459,12 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 					notificationToasts.push(toast);
 					break;
 				case ToastVisibility.HIDDEN:
-					if (!this.isVisible(toast)) {
+					if (!this.isToastVisible(toast)) {
 						notificationToasts.push(toast);
 					}
 					break;
 				case ToastVisibility.VISIBLE:
-					if (this.isVisible(toast)) {
+					if (this.isToastVisible(toast)) {
 						notificationToasts.push(toast);
 					}
 					break;
@@ -534,7 +552,7 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 	}
 
 	private setVisibility(toast: INotificationToast, visible: boolean): void {
-		if (this.isVisible(toast) === visible) {
+		if (this.isToastVisible(toast) === visible) {
 			return;
 		}
 
@@ -546,7 +564,7 @@ export class NotificationsToasts extends Themable implements INotificationsToast
 		}
 	}
 
-	private isVisible(toast: INotificationToast): boolean {
+	private isToastVisible(toast: INotificationToast): boolean {
 		return !!toast.container.parentElement;
 	}
 }

src/vs/workbench/browser/parts/notifications/notificationsViewer.ts

@@ -455,7 +455,7 @@ export class NotificationTemplateRenderer extends Disposable {
 	private renderProgress(notification: INotificationViewItem): void {
 
 		// Return early if the item has no progress
-		if (!notification.hasProgress()) {
+		if (!notification.hasProgress) {
 			this.template.progress.stop().hide();
 
 			return;

src/vs/workbench/browser/workbench.ts

@@ -384,10 +384,14 @@ export class Workbench extends Layout {
 
 		// Visibility
 		this._register(notificationsCenter.onDidChangeVisibility(() => {
-			notificationsStatus.update(notificationsCenter.isVisible);
+			notificationsStatus.update(notificationsCenter.isVisible, notificationsToasts.isVisible);
 			notificationsToasts.update(notificationsCenter.isVisible);
 		}));
 
+		this._register(notificationsToasts.onDidChangeVisibility(() => {
+			notificationsStatus.update(notificationsCenter.isVisible, notificationsToasts.isVisible);
+		}));
+
 		// Register Commands
 		registerNotificationCommands(notificationsCenter, notificationsToasts);
 	}

src/vs/workbench/common/notifications.ts

@@ -261,6 +261,7 @@ export interface INotificationViewItem {
 
 	readonly expanded: boolean;
 	readonly canCollapse: boolean;
+	readonly hasProgress: boolean;
 
 	readonly onDidChangeExpansion: Event<void>;
 	readonly onDidClose: Event<void>;
@@ -270,9 +271,6 @@ export interface INotificationViewItem {
 	collapse(skipEvents?: boolean): void;
 	toggle(): void;
 
-	hasProgress(): boolean;
-	hasPrompt(): boolean;
-
 	updateSeverity(severity: Severity): void;
 	updateMessage(message: NotificationMessage): void;
 	updateActions(actions?: INotificationActions): void;
@@ -495,7 +493,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie
 	}
 
 	get canCollapse(): boolean {
-		return !this.hasPrompt();
+		return !this.hasPrompt;
 	}
 
 	get expanded(): boolean {
@@ -511,7 +509,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie
 			return true; // explicitly sticky
 		}
 
-		const hasPrompt = this.hasPrompt();
+		const hasPrompt = this.hasPrompt;
 		if (
 			(hasPrompt && this._severity === Severity.Error) || // notification errors with actions are sticky
 			(!hasPrompt && this._expanded) ||					// notifications that got expanded are sticky
@@ -527,7 +525,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie
 		return !!this._silent;
 	}
 
-	hasPrompt(): boolean {
+	private get hasPrompt(): boolean {
 		if (!this._actions) {
 			return false;
 		}
@@ -539,7 +537,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie
 		return this._actions.primary.length > 0;
 	}
 
-	hasProgress(): boolean {
+	get hasProgress(): boolean {
 		return !!this._progress;
 	}
 
@@ -621,7 +619,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie
 	}
 
 	equals(other: INotificationViewItem): boolean {
-		if (this.hasProgress() || other.hasProgress()) {
+		if (this.hasProgress || other.hasProgress) {
 			return false;
 		}