From 79c18d8d960783fc30c83c0ef972aa8469a56b15 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 14 Feb 2018 09:59:49 +0100 Subject: [PATCH] notifications - introduce and use model --- .../parts/notifications/notificationList.ts | 101 ++++++------ src/vs/workbench/common/notifications.ts | 145 +++++++++++++++--- .../workbench/electron-browser/workbench.ts | 4 +- .../common/notificationService.ts | 32 ++-- 4 files changed, 200 insertions(+), 82 deletions(-) diff --git a/src/vs/workbench/browser/parts/notifications/notificationList.ts b/src/vs/workbench/browser/parts/notifications/notificationList.ts index 972dc0d3630..f175a9a14d9 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationList.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationList.ts @@ -6,7 +6,7 @@ 'use strict'; import 'vs/css!./media/notificationList'; -import { addClass } from 'vs/base/browser/dom'; +import { addClass, removeClass } from 'vs/base/browser/dom'; import { WorkbenchList } from 'vs/platform/list/browser/listService'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IListOptions } from 'vs/base/browser/ui/list/listWidget'; @@ -14,28 +14,45 @@ import { localize } from 'vs/nls'; import { Themable } from 'vs/workbench/common/theme'; import { IThemeService, registerThemingParticipant, ITheme, ICssStyleCollector } from 'vs/platform/theme/common/themeService'; import { contrastBorder, widgetShadow, textLinkForeground } from 'vs/platform/theme/common/colorRegistry'; -import { INotification, INotificationHandle } from 'vs/platform/notification/common/notification'; -import { INotificationViewItem, NotificationViewItem } from 'vs/workbench/common/notifications'; -import { INotificationHandler } from 'vs/workbench/services/notification/common/notificationService'; +import { INotificationViewItem, INotificationsModel, INotificationChangeEvent, NotificationChangeType } from 'vs/workbench/common/notifications'; import { NotificationsListDelegate, NotificationRenderer } from 'vs/workbench/browser/parts/notifications/notificationViewer'; import { Severity } from 'vs/platform/message/common/message'; import { alert } from 'vs/base/browser/ui/aria/aria'; -export class NotificationList extends Themable implements INotificationHandler { - - private static NO_OP_NOTIFICATION: INotificationHandle = { dispose: () => void 0 }; +export class NotificationList extends Themable { private listContainer: HTMLElement; private list: WorkbenchList; constructor( private container: HTMLElement, + private model: INotificationsModel, @IInstantiationService private instantiationService: IInstantiationService, @IThemeService themeService: IThemeService ) { super(themeService); this.create(); + + // Show initial notifications if any + this.onNotificationsAdded(0, model.notifications); + + this.registerListeners(); + } + + private registerListeners(): void { + this.toUnbind.push(this.model.onDidNotificationsChange(e => this.onDidNotificationsChange(e))); + } + + private onDidNotificationsChange(e: INotificationChangeEvent): void { + switch (e.kind) { + case NotificationChangeType.ADD: + return this.onNotificationsAdded(e.index, [e.item]); + case NotificationChangeType.CHANGE: + return this.onNotificationChanged(e.index, e.item); + case NotificationChangeType.REMOVE: + return this.onNotificationRemoved(e.index, e.item); + } } protected updateStyles(): void { @@ -66,65 +83,53 @@ export class NotificationList extends Themable implements INotificationHandler { [renderer], { ariaLabel: localize('notificationsList', "Notifications List"), - openController: { shouldOpen: e => this.shouldExpand(e) } + multipleSelectionSupport: true } as IListOptions ); this.toUnbind.push(this.list); - // Expand/Collapse - this.list.onOpen(e => { - const notification = e.elements[0]; - const index = e.indexes[0]; - - if (notification.canCollapse) { - if (notification.expanded) { - notification.collapse(); - } else { - notification.expand(); - } - } - - this.list.splice(index, 1, [notification]); - this.list.layout(); - this.list.setSelection([index]); - this.list.setFocus([index]); - - setTimeout(() => this.list.domFocus()); // TODO@notification why? - }); - this.container.appendChild(this.listContainer); this.updateStyles(); } - private shouldExpand(event: UIEvent): boolean { - const target = event.target as HTMLElement; - if (target.tagName.toLowerCase() === 'a') { - return false; // do not overwrite links/buttons - } + private onNotificationsAdded(index: number, items: INotificationViewItem[]): void { - return true; + // Support in Screen Readers too + items.forEach(item => this.ariaAlert(item)); + + // Update list + this.updateNotificationsList(index, 0, items); } - public show(notification: INotification): INotificationHandle { - const viewItem = NotificationViewItem.create(notification); - if (!viewItem) { - return NotificationList.NO_OP_NOTIFICATION; - } + private onNotificationChanged(index: number, item: INotificationViewItem): void { + this.updateNotificationsList(index, 1, [item]); + } - // Support in Screen Readers too - this.ariaAlert(viewItem); + private onNotificationRemoved(index: number, item: INotificationViewItem): void { + this.updateNotificationsList(index, 1); + } - viewItem.onDidExpansionChange(() => { - // TODO expand/collapse using model index - }); // TODO@Notification dispose + private updateNotificationsList(start: number, deleteCount: number, items: INotificationViewItem[] = []) { - addClass(this.listContainer, 'visible'); + // Ensure visibility is proper + if (this.model.notifications.length > 0) { + this.show(); + } else { + this.hide(); + } - this.list.splice(0, 0, [viewItem]); + // Update list + this.list.splice(start, deleteCount, items); this.list.layout(); + } + + private show(): void { + addClass(this.listContainer, 'visible'); + } - return viewItem; // TODO@notification what if message replaced with other? + private hide(): void { + removeClass(this.listContainer, 'visible'); } private ariaAlert(notifiation: INotificationViewItem): void { diff --git a/src/vs/workbench/common/notifications.ts b/src/vs/workbench/common/notifications.ts index 4c41cbd8b0b..55c5dfa1a63 100644 --- a/src/vs/workbench/common/notifications.ts +++ b/src/vs/workbench/common/notifications.ts @@ -8,29 +8,119 @@ import { Severity } from 'vs/platform/message/common/message'; import { IMarkdownString } from 'vs/base/common/htmlContent'; import { IAction } from 'vs/base/common/actions'; -import { INotification } from 'vs/platform/notification/common/notification'; +import { INotification, INotificationHandle } from 'vs/platform/notification/common/notification'; import { toErrorMessage } from 'vs/base/common/errorMessage'; import { localize } from 'vs/nls'; -import Event, { Emitter } from 'vs/base/common/event'; +import Event, { Emitter, once } from 'vs/base/common/event'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -export class INotificationsModel { +export interface INotificationsModel { + readonly notifications: INotificationViewItem[]; + readonly onDidNotificationsChange: Event; + + notify(notification: INotification): INotificationHandle; +} + +export enum NotificationChangeType { + ADD, + CHANGE, + REMOVE +} + +export interface INotificationChangeEvent { + index: number; + item: INotificationViewItem; + kind: NotificationChangeType; +} + +class NoOpNotification implements INotificationHandle { + dispose() { } } export class NotificationsModel implements INotificationsModel { + private static NO_OP_NOTIFICATION = new NoOpNotification(); + + private _notifications: INotificationViewItem[]; + + private _onDidNotificationsChange: Emitter; + private toDispose: IDisposable[]; + + constructor() { + this._notifications = []; + this.toDispose = []; + + this._onDidNotificationsChange = new Emitter(); + this.toDispose.push(this._onDidNotificationsChange); + } + + public get notifications(): INotificationViewItem[] { + return this._notifications; + } + + public get onDidNotificationsChange(): Event { + return this._onDidNotificationsChange.event; + } + + public notify(notification: INotification): INotificationHandle { + const item = this.createViewItem(notification); + if (item instanceof NoOpNotification) { + return item; // return early if this is a no-op + } + + // Add to list as first entry (TODO@notification dedupe!) + this._notifications.splice(0, 0, item); + + // Events + this._onDidNotificationsChange.fire({ item, index: 0, kind: NotificationChangeType.ADD }); + + return item; + } + + private createViewItem(notification: INotification): INotificationViewItem | NoOpNotification { + const item = NotificationViewItem.create(notification); + if (!item) { + return NotificationsModel.NO_OP_NOTIFICATION; + } + + // Item Events + const itemChangeListener = item.onDidChange(() => { + const index = this._notifications.indexOf(item); + if (index >= 0) { + this._onDidNotificationsChange.fire({ item, index, kind: NotificationChangeType.CHANGE }); + } + }); + + once(item.onDidDispose)(() => { + itemChangeListener.dispose(); + + const index = this._notifications.indexOf(item); + if (index >= 0) { + this._notifications.splice(index, 1); + this._onDidNotificationsChange.fire({ item, index, kind: NotificationChangeType.REMOVE }); + } + }); + + return item; + } + + public dispose(): void { + this.toDispose = dispose(this.toDispose); + } } export interface INotificationViewItem { readonly severity: Severity; readonly message: IMarkdownString; - readonly expanded: boolean; - readonly canCollapse: boolean; readonly source: string; readonly actions: IAction[]; - readonly onDidExpansionChange: Event; + readonly expanded: boolean; + readonly canCollapse: boolean; + + readonly onDidChange: Event; + readonly onDidDispose: Event; expand(): void; collapse(): void; @@ -44,21 +134,9 @@ export class NotificationViewItem implements INotificationViewItem { private _expanded: boolean; private toDispose: IDisposable[]; - private _onDidExpansionChange; - constructor(private _severity: Severity, private _message: IMarkdownString, private _source: string, private _actions: IAction[]) { - this.toDispose = []; - this._expanded = _actions.length > 0; - - this._onDidExpansionChange = new Emitter(); - this.toDispose.push(this._onDidExpansionChange); - - this.toDispose.push(..._actions); - } - - public get onDidExpansionChange(): Event { - return this._onDidExpansionChange.event; - } + private _onDidChange: Emitter; + private _onDidDispose: Emitter; public static create(notification: INotification): INotificationViewItem { if (!notification || !notification.message) { @@ -81,6 +159,27 @@ export class NotificationViewItem implements INotificationViewItem { return new NotificationViewItem(notification.severity, message, notification.source || NotificationViewItem.DEFAULT_SOURCE, notification.actions || []); } + private constructor(private _severity: Severity, private _message: IMarkdownString, private _source: string, private _actions: IAction[]) { + this.toDispose = []; + this._expanded = _actions.length > 0; + + this._onDidChange = new Emitter(); + this.toDispose.push(this._onDidChange); + + this._onDidDispose = new Emitter(); + this.toDispose.push(this._onDidDispose); + + this.toDispose.push(..._actions); + } + + public get onDidChange(): Event { + return this._onDidChange.event; + } + + public get onDidDispose(): Event { + return this._onDidDispose.event; + } + public get canCollapse(): boolean { return this._actions.length === 0; } @@ -107,15 +206,17 @@ export class NotificationViewItem implements INotificationViewItem { public expand(): void { this._expanded = true; - this._onDidExpansionChange.fire(); + this._onDidChange.fire(); } public collapse(): void { this._expanded = false; - this._onDidExpansionChange.fire(); + this._onDidChange.fire(); } public dispose(): void { + this._onDidDispose.fire(); + this.toDispose = dispose(this.toDispose); } } \ No newline at end of file diff --git a/src/vs/workbench/electron-browser/workbench.ts b/src/vs/workbench/electron-browser/workbench.ts index f5d83e36b89..78e083f0fba 100644 --- a/src/vs/workbench/electron-browser/workbench.ts +++ b/src/vs/workbench/electron-browser/workbench.ts @@ -1245,8 +1245,8 @@ export class Workbench implements IPartService { } private createNotificationsList(): void { - const notificationsList = this.instantiationService.createInstance(NotificationList, this.workbench.getHTMLElement()); - this.notificationService.setHandler(notificationsList); + const notificationsList = this.instantiationService.createInstance(NotificationList, this.workbench.getHTMLElement(), this.notificationService.model); + this.toUnbind.push(notificationsList); } public getInstantiationService(): IInstantiationService { diff --git a/src/vs/workbench/services/notification/common/notificationService.ts b/src/vs/workbench/services/notification/common/notificationService.ts index 52fdfbd72f9..356babef2e7 100644 --- a/src/vs/workbench/services/notification/common/notificationService.ts +++ b/src/vs/workbench/services/notification/common/notificationService.ts @@ -8,21 +8,34 @@ import { INotificationService, INotification, INotificationHandle } from 'vs/platform/notification/common/notification'; import { Severity } from 'vs/platform/message/common/message'; import { Action } from 'vs/base/common/actions'; - -export interface INotificationHandler { - show(notification: INotification): INotificationHandle; -} +import { INotificationsModel, NotificationsModel } from 'vs/workbench/common/notifications'; +import { IDisposable, dispose } from 'vs/base/common/lifecycle'; export class NotificationService implements INotificationService { public _serviceBrand: any; - private handler: INotificationHandler; + private _model: INotificationsModel; + private toDispose: IDisposable[]; constructor( container: HTMLElement ) { + this.toDispose = []; + + const model = new NotificationsModel(); + this.toDispose.push(model); + this._model = model; + // TODO@notification remove me + this.showFakeNotifications(); + } + + public get model(): INotificationsModel { + return this._model; + } + + private showFakeNotifications(): void { setTimeout(() => { this.notify({ severity: Severity.Info, @@ -43,12 +56,11 @@ export class NotificationService implements INotificationService { }, 500); } - public setHandler(handler: INotificationHandler): void { - this.handler = handler; - // TODO@notification release buffered + public notify(notification: INotification): INotificationHandle { + return this.model.notify(notification); } - public notify(notification: INotification): INotificationHandle { - return this.handler.show(notification); + public dispose(): void { + this.toDispose = dispose(this.toDispose); } } \ No newline at end of file -- GitLab