Skip to content

Commit

Permalink
Notifications: indicate progress notification if hidden in notificati…
Browse files Browse the repository at this point in the history
…on center (fix #90274)
  • Loading branch information
bpasero committed Feb 21, 2020
1 parent 8b95561 commit 6cea954
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 43 deletions.
84 changes: 59 additions & 25 deletions src/vs/workbench/browser/parts/notifications/notificationsStatus.ts
Expand Up @@ -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';
Expand All @@ -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,
Expand All @@ -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
};

Expand All @@ -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");
}
Expand All @@ -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();
}
}
Expand Down
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}));
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
}
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/vs/workbench/browser/workbench.ts
Expand Up @@ -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);
}
Expand Down
14 changes: 6 additions & 8 deletions src/vs/workbench/common/notifications.ts
Expand Up @@ -261,6 +261,7 @@ export interface INotificationViewItem {

readonly expanded: boolean;
readonly canCollapse: boolean;
readonly hasProgress: boolean;

readonly onDidChangeExpansion: Event<void>;
readonly onDidClose: Event<void>;
Expand All @@ -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;
Expand Down Expand Up @@ -495,7 +493,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie
}

get canCollapse(): boolean {
return !this.hasPrompt();
return !this.hasPrompt;
}

get expanded(): boolean {
Expand All @@ -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
Expand All @@ -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;
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 6cea954

Please sign in to comment.