diff --git a/src/vs/platform/notification/common/notification.ts b/src/vs/platform/notification/common/notification.ts index a81672c8311d55d120ff186fbd2c6acb8d11db5d..f7f2b5a3d9e86b31b04e1395e0bced41f058e0aa 100644 --- a/src/vs/platform/notification/common/notification.ts +++ b/src/vs/platform/notification/common/notification.ts @@ -173,6 +173,13 @@ export interface INotificationHandle { */ readonly onDidClose: Event; + /** + * Will be fired whenever the visibility of the notification changes. + * A notification can either be visible as toast or inside the notification + * center if it is visible. + */ + readonly onDidChangeVisibility: Event; + /** * Allows to indicate progress on the notification even after the * notification is already visible. diff --git a/src/vs/workbench/browser/parts/notifications/notificationsActions.ts b/src/vs/workbench/browser/parts/notifications/notificationsActions.ts index 82301a0448e82f71841446b5edc79ec04ab05fa6..ae63e201e24c3542dd5f7f2ae7f828b93aca7378 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsActions.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsActions.ts @@ -26,10 +26,8 @@ export class ClearNotificationAction extends Action { super(id, label, 'codicon-close'); } - run(notification: INotificationViewItem): Promise { + async run(notification: INotificationViewItem): Promise { this.commandService.executeCommand(CLEAR_NOTIFICATION, notification); - - return Promise.resolve(); } } @@ -46,10 +44,8 @@ export class ClearAllNotificationsAction extends Action { super(id, label, 'codicon-clear-all'); } - run(notification: INotificationViewItem): Promise { + async run(notification: INotificationViewItem): Promise { this.commandService.executeCommand(CLEAR_ALL_NOTIFICATIONS); - - return Promise.resolve(); } } @@ -66,10 +62,8 @@ export class HideNotificationsCenterAction extends Action { super(id, label, 'codicon-chevron-down'); } - run(notification: INotificationViewItem): Promise { + async run(notification: INotificationViewItem): Promise { this.commandService.executeCommand(HIDE_NOTIFICATIONS_CENTER); - - return Promise.resolve(); } } @@ -86,10 +80,8 @@ export class ExpandNotificationAction extends Action { super(id, label, 'codicon-chevron-up'); } - run(notification: INotificationViewItem): Promise { + async run(notification: INotificationViewItem): Promise { this.commandService.executeCommand(EXPAND_NOTIFICATION, notification); - - return Promise.resolve(); } } @@ -106,10 +98,8 @@ export class CollapseNotificationAction extends Action { super(id, label, 'codicon-chevron-down'); } - run(notification: INotificationViewItem): Promise { + async run(notification: INotificationViewItem): Promise { this.commandService.executeCommand(COLLAPSE_NOTIFICATION, notification); - - return Promise.resolve(); } } diff --git a/src/vs/workbench/browser/parts/notifications/notificationsCenter.ts b/src/vs/workbench/browser/parts/notifications/notificationsCenter.ts index 094c9e1567f43b48bc0dc6bc19f85c56a7925615..c37af327a36111df3704fc7817b3f4a696667bb6 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsCenter.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsCenter.ts @@ -100,6 +100,9 @@ export class NotificationsCenter extends Themable implements INotificationsCente // Theming this.updateStyles(); + // Mark as visible + this.model.notifications.forEach(notification => notification.updateVisibility(true)); + // Context Key this.notificationsCenterVisibleContextKey.set(true); @@ -115,7 +118,7 @@ export class NotificationsCenter extends Themable implements INotificationsCente clearAllAction.enabled = false; } else { notificationsCenterTitle.textContent = localize('notifications', "Notifications"); - clearAllAction.enabled = true; + clearAllAction.enabled = this.model.notifications.some(notification => !notification.hasProgress); } } @@ -172,20 +175,22 @@ export class NotificationsCenter extends Themable implements INotificationsCente return; // only if visible } - let focusGroup = false; + let focusEditor = false; // Update notifications list based on event const [notificationsList, notificationsCenterContainer] = assertAllDefined(this.notificationsList, this.notificationsCenterContainer); switch (e.kind) { case NotificationChangeType.ADD: notificationsList.updateNotificationsList(e.index, 0, [e.item]); + e.item.updateVisibility(true); break; case NotificationChangeType.CHANGE: notificationsList.updateNotificationsList(e.index, 1, [e.item]); break; case NotificationChangeType.REMOVE: - focusGroup = isAncestor(document.activeElement, notificationsCenterContainer); + focusEditor = isAncestor(document.activeElement, notificationsCenterContainer); notificationsList.updateNotificationsList(e.index, 1); + e.item.updateVisibility(false); break; } @@ -197,7 +202,7 @@ export class NotificationsCenter extends Themable implements INotificationsCente this.hide(); // Restore focus to editor group if we had focus - if (focusGroup) { + if (focusEditor) { this.editorGroupService.activeGroup.focus(); } } @@ -208,13 +213,16 @@ export class NotificationsCenter extends Themable implements INotificationsCente return; // already hidden } - const focusGroup = isAncestor(document.activeElement, this.notificationsCenterContainer); + const focusEditor = isAncestor(document.activeElement, this.notificationsCenterContainer); // Hide this._isVisible = false; removeClass(this.notificationsCenterContainer, 'visible'); this.notificationsList.hide(); + // Mark as hidden + this.model.notifications.forEach(notification => notification.updateVisibility(false)); + // Context Key this.notificationsCenterVisibleContextKey.set(false); @@ -222,7 +230,7 @@ export class NotificationsCenter extends Themable implements INotificationsCente this._onDidChangeVisibility.fire(); // Restore focus to editor group if we had focus - if (focusGroup) { + if (focusEditor) { this.editorGroupService.activeGroup.focus(); } } diff --git a/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts b/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts index 413e700668d084bbe075cfe47a830e26ff93f63c..b2797f261f30218437e0cccb92964285b0efc86d 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts @@ -75,6 +75,7 @@ export function registerNotificationCommands(center: INotificationsCenterControl // Show Notifications Cneter CommandsRegistry.registerCommand(SHOW_NOTIFICATIONS_CENTER, () => { + toasts.hide(); center.show(); }); @@ -92,6 +93,7 @@ export function registerNotificationCommands(center: INotificationsCenterControl if (center.isVisible) { center.hide(); } else { + toasts.hide(); center.show(); } }); diff --git a/src/vs/workbench/browser/parts/notifications/notificationsToasts.ts b/src/vs/workbench/browser/parts/notifications/notificationsToasts.ts index 95e287fbd49a92436668a5d0081fd3a0a9be9a6b..ecee706fcb97f7aa09fde1e651bb08102ec07b56 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsToasts.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsToasts.ts @@ -179,11 +179,8 @@ export class NotificationsToasts extends Themable implements INotificationsToast const toast: INotificationToast = { item, list: notificationList, container: notificationToastContainer, toast: notificationToast, toDispose: itemDisposables }; this.mapNotificationToToast.set(item, toast); - itemDisposables.add(toDisposable(() => { - if (this.isToastVisible(toast) && notificationsToastsContainer) { - notificationsToastsContainer.removeChild(toast.container); - } - })); + // When disposed, remove as visible + itemDisposables.add(toDisposable(() => this.updateToastVisibility(toast, false))); // Make visible notificationList.show(); @@ -236,6 +233,9 @@ export class NotificationsToasts extends Themable implements INotificationsToast addClass(notificationToast, 'notification-fade-in-done'); })); + // Mark as visible + item.updateVisibility(true); + // Events if (!this._isVisible) { this._isVisible = true; @@ -292,12 +292,13 @@ export class NotificationsToasts extends Themable implements INotificationsToast } private removeToast(item: INotificationViewItem): void { + let focusEditor = false; + const notificationToast = this.mapNotificationToToast.get(item); - let focusGroup = false; if (notificationToast) { const toastHasDOMFocus = isAncestor(document.activeElement, notificationToast.container); if (toastHasDOMFocus) { - focusGroup = !(this.focusNext() || this.focusPrevious()); // focus next if any, otherwise focus editor + focusEditor = !(this.focusNext() || this.focusPrevious()); // focus next if any, otherwise focus editor } // Listeners @@ -317,7 +318,7 @@ export class NotificationsToasts extends Themable implements INotificationsToast this.doHide(); // Move focus back to editor group as needed - if (focusGroup) { + if (focusEditor) { this.editorGroupService.activeGroup.focus(); } } @@ -346,11 +347,11 @@ export class NotificationsToasts extends Themable implements INotificationsToast } hide(): void { - const focusGroup = this.notificationsToastsContainer ? isAncestor(document.activeElement, this.notificationsToastsContainer) : false; + const focusEditor = this.notificationsToastsContainer ? isAncestor(document.activeElement, this.notificationsToastsContainer) : false; this.removeToasts(); - if (focusGroup) { + if (focusEditor) { this.editorGroupService.activeGroup.focus(); } } @@ -459,12 +460,12 @@ export class NotificationsToasts extends Themable implements INotificationsToast notificationToasts.push(toast); break; case ToastVisibility.HIDDEN: - if (!this.isToastVisible(toast)) { + if (!this.isToastInDOM(toast)) { notificationToasts.push(toast); } break; case ToastVisibility.VISIBLE: - if (this.isToastVisible(toast)) { + if (this.isToastInDOM(toast)) { notificationToasts.push(toast); } break; @@ -530,7 +531,7 @@ export class NotificationsToasts extends Themable implements INotificationsToast // In order to measure the client height, the element cannot have display: none toast.container.style.opacity = '0'; - this.setVisibility(toast, true); + this.updateToastVisibility(toast, true); heightToGive -= toast.container.offsetHeight; @@ -542,7 +543,7 @@ export class NotificationsToasts extends Themable implements INotificationsToast } // Hide or show toast based on context - this.setVisibility(toast, makeVisible); + this.updateToastVisibility(toast, makeVisible); toast.container.style.opacity = ''; if (makeVisible) { @@ -551,20 +552,24 @@ export class NotificationsToasts extends Themable implements INotificationsToast }); } - private setVisibility(toast: INotificationToast, visible: boolean): void { - if (this.isToastVisible(toast) === visible) { + private updateToastVisibility(toast: INotificationToast, visible: boolean): void { + if (this.isToastInDOM(toast) === visible) { return; } + // Update visibility in DOM const notificationsToastsContainer = assertIsDefined(this.notificationsToastsContainer); if (visible) { notificationsToastsContainer.appendChild(toast.container); } else { notificationsToastsContainer.removeChild(toast.container); } + + // Update visibility in model + toast.item.updateVisibility(visible); } - private isToastVisible(toast: INotificationToast): boolean { + private isToastInDOM(toast: INotificationToast): boolean { return !!toast.container.parentElement; } } diff --git a/src/vs/workbench/common/notifications.ts b/src/vs/workbench/common/notifications.ts index 73f0ef74053adeddd50278c904840a6acab342b9..809037bf68e83d2ce701e99c904f1ff348060e99 100644 --- a/src/vs/workbench/common/notifications.ts +++ b/src/vs/workbench/common/notifications.ts @@ -92,6 +92,9 @@ export class NotificationHandle extends Disposable implements INotificationHandl private readonly _onDidClose = this._register(new Emitter()); readonly onDidClose = this._onDidClose.event; + private readonly _onDidChangeVisibility = this._register(new Emitter()); + readonly onDidChangeVisibility = this._onDidChangeVisibility.event; + constructor(private readonly item: INotificationViewItem, private readonly onClose: (item: INotificationViewItem) => void) { super(); @@ -99,6 +102,11 @@ export class NotificationHandle extends Disposable implements INotificationHandl } private registerListeners(): void { + + // Visibility + this._register(this.item.onDidChangeVisibility(visible => this._onDidChangeVisibility.fire(visible))); + + // Closing Event.once(this.item.onDidClose)(() => { this._onDidClose.fire(); @@ -265,6 +273,7 @@ export interface INotificationViewItem { readonly onDidChangeExpansion: Event; readonly onDidClose: Event; + readonly onDidChangeVisibility: Event; readonly onDidChangeLabel: Event; expand(): void; @@ -275,6 +284,8 @@ export interface INotificationViewItem { updateMessage(message: NotificationMessage): void; updateActions(actions?: INotificationActions): void; + updateVisibility(visible: boolean): void; + close(): void; equals(item: INotificationViewItem): boolean; @@ -398,6 +409,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie private static readonly MAX_MESSAGE_LENGTH = 1000; private _expanded: boolean | undefined; + private _visible: boolean = false; private _actions: INotificationActions | undefined; private _progress: NotificationViewItemProgress | undefined; @@ -411,6 +423,9 @@ export class NotificationViewItem extends Disposable implements INotificationVie private readonly _onDidChangeLabel = this._register(new Emitter()); readonly onDidChangeLabel = this._onDidChangeLabel.event; + private readonly _onDidChangeVisibility = this._register(new Emitter()); + readonly onDidChangeVisibility = this._onDidChangeVisibility.event; + static create(notification: INotification, filter: NotificationsFilter = NotificationsFilter.OFF): INotificationViewItem | undefined { if (!notification || !notification.message || isPromiseCanceledError(notification.message)) { return undefined; // we need a message to show @@ -600,6 +615,14 @@ export class NotificationViewItem extends Disposable implements INotificationVie this._onDidChangeLabel.fire({ kind: NotificationViewItemLabelKind.ACTIONS }); } + updateVisibility(visible: boolean): void { + if (this._visible !== visible) { + this._visible = visible; + + this._onDidChangeVisibility.fire(visible); + } + } + expand(): void { if (this._expanded || !this.canCollapse) { return; diff --git a/src/vs/workbench/services/progress/browser/progressService.ts b/src/vs/workbench/services/progress/browser/progressService.ts index 6d4723b34e3329de563c7de3f4a169163e1f8d76..e0b2980aad4a8998fc8ed18002834d2e111956e2 100644 --- a/src/vs/workbench/services/progress/browser/progressService.ts +++ b/src/vs/workbench/services/progress/browser/progressService.ts @@ -6,7 +6,7 @@ import 'vs/css!./media/progressService'; import { localize } from 'vs/nls'; -import { IDisposable, dispose, DisposableStore, MutableDisposable, Disposable } from 'vs/base/common/lifecycle'; +import { IDisposable, dispose, DisposableStore, MutableDisposable, Disposable, toDisposable } from 'vs/base/common/lifecycle'; import { IProgressService, IProgressOptions, IProgressStep, ProgressLocation, IProgress, Progress, IProgressCompositeOptions, IProgressNotificationOptions, IProgressRunner, IProgressIndicator, IProgressWindowOptions } from 'vs/platform/progress/common/progress'; import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; import { StatusbarAlignment, IStatusbarService } from 'vs/workbench/services/statusbar/common/statusbar'; @@ -191,6 +191,38 @@ export class ProgressService extends Disposable implements IProgressService { } }; + const createWindowProgress = () => { + + // Create a promise that we can resolve as needed + // when the outside calls dispose on us + let promiseResolve: () => void; + const promise = new Promise(resolve => promiseResolve = resolve); + + this.withWindowProgress({ + location: ProgressLocation.Window, + title: options.title, + command: 'notifications.showList' + }, progress => { + + // Apply any progress that was made already + if (progressStateModel.step) { + progress.report(progressStateModel.step); + } + + // Continue to report progress as it happens + const onDidReportListener = progressStateModel.onDidReport(step => progress.report(step)); + promise.finally(() => onDidReportListener.dispose()); + + // When the progress model gets disposed, we are done as well + Event.once(progressStateModel.onDispose)(() => promiseResolve()); + + return promise; + }); + + // Dispose means completing our promise + return toDisposable(() => promiseResolve()); + }; + const createNotification = (message: string, increment?: number): INotificationHandle => { const notificationDisposables = new DisposableStore(); @@ -229,7 +261,7 @@ export class ProgressService extends Disposable implements IProgressService { primaryActions.push(cancelAction); } - const handle = this.notificationService.notify({ + const notification = this.notificationService.notify({ severity: Severity.Info, message, source: options.source, @@ -237,12 +269,26 @@ export class ProgressService extends Disposable implements IProgressService { progress: typeof increment === 'number' && increment >= 0 ? { total: 100, worked: increment } : { infinite: true } }); - updateProgress(handle, increment); + // Switch to window based progress once the notification + // changes visibility to hidden and is still ongoing. + // Remove that window based progress once the notification + // shows again. + let windowProgressDisposable: IDisposable | undefined = undefined; + notificationDisposables.add(notification.onDidChangeVisibility(visible => { + + // Clear any previous running window progress + dispose(windowProgressDisposable); + + // Create new window progress if notification got hidden + if (!visible && !progressStateModel.done) { + windowProgressDisposable = createWindowProgress(); + } + })); // Clear upon dispose - Event.once(handle.onDidClose)(() => notificationDisposables.dispose()); + Event.once(notification.onDidClose)(() => notificationDisposables.dispose()); - return handle; + return notification; }; const updateProgress = (notification: INotificationHandle, increment?: number): void => { diff --git a/src/vs/workbench/test/common/notifications.test.ts b/src/vs/workbench/test/common/notifications.test.ts index 7eefd25cb8e4966976e7d8225953a3c952e8235b..a050bc2942255c283be43d57116c50ff63d60486 100644 --- a/src/vs/workbench/test/common/notifications.test.ts +++ b/src/vs/workbench/test/common/notifications.test.ts @@ -98,6 +98,17 @@ suite('Notifications', () => { assert.equal(called, 1); + called = 0; + item1.onDidChangeVisibility(e => { + called++; + }); + + item1.updateVisibility(true); + item1.updateVisibility(false); + item1.updateVisibility(false); + + assert.equal(called, 2); + called = 0; item1.onDidClose(() => { called++;