From bb22fc8900a848eabf9add920fdd11094cf65e14 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 6 Jul 2016 09:45:53 +0200 Subject: [PATCH] debt - use modern Events for Action --- src/vs/base/browser/ui/actionbar/actionbar.ts | 65 ++++++++----------- src/vs/base/common/actions.ts | 36 ++++++---- src/vs/platform/actions/common/actions.ts | 20 +++--- .../parts/activitybar/activityAction.ts | 36 +++++----- .../parts/debug/browser/debugViewer.ts | 2 +- .../parts/debug/browser/debugViews.ts | 2 +- .../electron-browser/extensionsActions.ts | 4 +- 7 files changed, 83 insertions(+), 82 deletions(-) diff --git a/src/vs/base/browser/ui/actionbar/actionbar.ts b/src/vs/base/browser/ui/actionbar/actionbar.ts index 644091c2ee8..339afe226aa 100644 --- a/src/vs/base/browser/ui/actionbar/actionbar.ts +++ b/src/vs/base/browser/ui/actionbar/actionbar.ts @@ -11,11 +11,11 @@ import lifecycle = require('vs/base/common/lifecycle'); import {Promise} from 'vs/base/common/winjs.base'; import {Builder, $} from 'vs/base/browser/builder'; import platform = require('vs/base/common/platform'); -import {IAction, IActionRunner, Action, ActionRunner} from 'vs/base/common/actions'; +import {IAction, IActionRunner, Action, IActionChangeEvent, ActionRunner} from 'vs/base/common/actions'; import DOM = require('vs/base/browser/dom'); import {EventType as CommonEventType} from 'vs/base/common/events'; import types = require('vs/base/common/types'); -import {IEventEmitter, EventEmitter, EmitterEvent} from 'vs/base/common/eventEmitter'; +import {IEventEmitter, EventEmitter} from 'vs/base/common/eventEmitter'; import {Gesture, EventType} from 'vs/base/browser/touch'; import {StandardKeyboardEvent} from 'vs/base/browser/keyboardEvent'; import {CommonKeybindings} from 'vs/base/common/keyCodes'; @@ -48,38 +48,33 @@ export class BaseActionItem extends EventEmitter implements IActionItem { this._action = action; if (action instanceof Action) { - let l = (action).addBulkListener2((events: EmitterEvent[]) => { + this._callOnDispose.push(action.onDidChange(event => { if (!this.builder) { // we have not been rendered yet, so there // is no point in updating the UI return; } + this._handleActionChangeEvent(event); + })); + } + } - events.forEach((event: EmitterEvent) => { - switch (event.getType()) { - case Action.ENABLED: - this._updateEnabled(); - break; - case Action.LABEL: - this._updateLabel(); - this._updateTooltip(); - break; - case Action.TOOLTIP: - this._updateTooltip(); - break; - case Action.CLASS: - this._updateClass(); - break; - case Action.CHECKED: - this._updateChecked(); - break; - default: - this._updateUnknown(event); - break; - } - }); - }); - this._callOnDispose.push(l); + protected _handleActionChangeEvent(event: IActionChangeEvent): void { + if (event.enabled !== void 0) { + this._updateEnabled(); + } + if (event.checked !== void 0) { + this._updateChecked(); + } + if (event.class !== void 0) { + this._updateClass(); + } + if (event.label !== void 0) { + this._updateLabel(); + this._updateTooltip(); + } + if (event.tooltip !== void 0) { + this._updateTooltip(); } } @@ -161,30 +156,26 @@ export class BaseActionItem extends EventEmitter implements IActionItem { } } - public _updateEnabled(): void { + protected _updateEnabled(): void { // implement in subclass } - public _updateLabel(): void { + protected _updateLabel(): void { // implement in subclass } - public _updateTooltip(): void { + protected _updateTooltip(): void { // implement in subclass } - public _updateClass(): void { + protected _updateClass(): void { // implement in subclass } - public _updateChecked(): void { + protected _updateChecked(): void { // implement in subclass } - public _updateUnknown(event: EmitterEvent): void { - // can implement in subclass - } - public dispose(): void { super.dispose(); diff --git a/src/vs/base/common/actions.ts b/src/vs/base/common/actions.ts index 5cd31c36ba2..b1a150eaa9a 100644 --- a/src/vs/base/common/actions.ts +++ b/src/vs/base/common/actions.ts @@ -8,6 +8,7 @@ import {TPromise} from 'vs/base/common/winjs.base'; import { IEventEmitter, EventEmitter } from 'vs/base/common/eventEmitter'; import {IDisposable} from 'vs/base/common/lifecycle'; import * as Events from 'vs/base/common/events'; +import Event, {Emitter} from 'vs/base/common/event'; export interface IAction extends IDisposable { id: string; @@ -68,14 +69,17 @@ export interface IActionProvider { getAction(id: string): IAction; } -export class Action extends EventEmitter implements IAction { +export interface IActionChangeEvent { + label?: string; + tooltip?: string; + class?: string; + enabled?: boolean; + checked?: boolean; +} - static LABEL: string = 'label'; - static TOOLTIP: string = 'tooltip'; - static CLASS: string = 'class'; - static ENABLED: string = 'enabled'; - static CHECKED: string = 'checked'; +export class Action implements IAction { + protected _onDidChange = new Emitter(); protected _id: string; protected _label: string; protected _tooltip: string; @@ -86,8 +90,6 @@ export class Action extends EventEmitter implements IAction { protected _order: number; constructor(id: string, label: string = '', cssClass: string = '', enabled: boolean = true, actionCallback: IActionCallback = null) { - super(); - this._id = id; this._label = label; this._cssClass = cssClass; @@ -95,6 +97,14 @@ export class Action extends EventEmitter implements IAction { this._actionCallback = actionCallback; } + public dispose() { + this._onDidChange.dispose(); + } + + public get onDidChange(): Event { + return this._onDidChange.event; + } + public get id(): string { return this._id; } @@ -110,7 +120,7 @@ export class Action extends EventEmitter implements IAction { protected _setLabel(value: string): void { if (this._label !== value) { this._label = value; - this.emit(Action.LABEL, { source: this }); + this._onDidChange.fire({ label: value }); } } @@ -125,7 +135,7 @@ export class Action extends EventEmitter implements IAction { protected _setTooltip(value: string): void { if (this._tooltip !== value) { this._tooltip = value; - this.emit(Action.TOOLTIP, { source: this }); + this._onDidChange.fire({ tooltip: value }); } } @@ -140,7 +150,7 @@ export class Action extends EventEmitter implements IAction { protected _setClass(value: string): void { if (this._cssClass !== value) { this._cssClass = value; - this.emit(Action.CLASS, { source: this }); + this._onDidChange.fire({ class: value }); } } @@ -155,7 +165,7 @@ export class Action extends EventEmitter implements IAction { protected _setEnabled(value: boolean): void { if (this._enabled !== value) { this._enabled = value; - this.emit(Action.ENABLED, { source: this }); + this._onDidChange.fire({ enabled: value }); } } @@ -170,7 +180,7 @@ export class Action extends EventEmitter implements IAction { protected _setChecked(value: boolean): void { if (this._checked !== value) { this._checked = value; - this.emit(Action.CHECKED, { source: this }); + this._onDidChange.fire({ checked: value }); } } diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts index 03d5f26b6f6..3f0eff82a56 100644 --- a/src/vs/platform/actions/common/actions.ts +++ b/src/vs/platform/actions/common/actions.ts @@ -7,7 +7,6 @@ import URI from 'vs/base/common/uri'; import Actions = require('vs/base/common/actions'); import WinJS = require('vs/base/common/winjs.base'); -import Assert = require('vs/base/common/assert'); import Descriptors = require('vs/platform/instantiation/common/descriptors'); import Instantiation = require('vs/platform/instantiation/common/instantiation'); import {KbExpr, IKeybindings, IKeybindingService} from 'vs/platform/keybinding/common/keybindingService'; @@ -168,7 +167,8 @@ export class DeferredAction extends Actions.Action { private _cachedAction: Actions.IAction; private _emitterUnbind: IDisposable; - constructor(private _instantiationService: Instantiation.IInstantiationService, private _descriptor: Descriptors.AsyncDescriptor0, + constructor(private _instantiationService: Instantiation.IInstantiationService, + private _descriptor: Descriptors.AsyncDescriptor0, id: string, label = '', cssClass = '', enabled = true) { super(id, label, cssClass, enabled); @@ -265,13 +265,15 @@ export class DeferredAction extends Actions.Action { let promise = WinJS.TPromise.as(undefined); return promise.then(() => { return this._instantiationService.createInstance(this._descriptor); - - }).then((action) => { - Assert.ok(action instanceof Actions.Action, 'Action must be an instanceof Base Action'); - this._cachedAction = action; - - // Pipe events from the instantated action through this deferred action - this._emitterUnbind = this.addEmitter2(this._cachedAction); + }).then(action => { + if (action instanceof Actions.Action) { + this._cachedAction = action; + // Pipe events from the instantated action through this deferred action + this._emitterUnbind = action.onDidChange(e => this._onDidChange.fire(e)); + + } else { + throw new Error('Action must be an instanceof Base Action'); + } return action; }); diff --git a/src/vs/workbench/browser/parts/activitybar/activityAction.ts b/src/vs/workbench/browser/parts/activitybar/activityAction.ts index 24195b0a4ba..ce5e92acb98 100644 --- a/src/vs/workbench/browser/parts/activitybar/activityAction.ts +++ b/src/vs/workbench/browser/parts/activitybar/activityAction.ts @@ -11,14 +11,13 @@ import {Builder, $} from 'vs/base/browser/builder'; import {DelayedDragHandler} from 'vs/base/browser/dnd'; import {Action} from 'vs/base/common/actions'; import {BaseActionItem} from 'vs/base/browser/ui/actionbar/actionbar'; -import {EmitterEvent} from 'vs/base/common/eventEmitter'; import {ProgressBadge, TextBadge, NumberBadge, IconBadge, IBadge} from 'vs/workbench/services/activity/common/activityService'; +import Event, {Emitter} from 'vs/base/common/event'; export class ActivityAction extends Action { - static BADGE = 'badge'; - private badge: IBadge; + private _onDidChangeBadge = new Emitter(); constructor(id: string, name: string, clazz: string) { super(id, name, clazz); @@ -26,17 +25,19 @@ export class ActivityAction extends Action { this.badge = null; } + public get onDidChangeBadge(): Event { + return this._onDidChangeBadge.event; + } + public activate(): void { if (!this.checked) { - this.checked = true; - this.emit('checked', { source: this }); + this._setChecked(true); } } public deactivate(): void { if (this.checked) { - this.checked = false; - this.emit('checked', { source: this }); + this._setChecked(false); } } @@ -46,7 +47,7 @@ export class ActivityAction extends Action { public setBadge(badge: IBadge): void { this.badge = badge; - this.emit(ActivityAction.BADGE, { source: this }); + this._onDidChangeBadge.fire(this); } } @@ -59,12 +60,13 @@ export class ActivityActionItem extends BaseActionItem { private $badge: Builder; private $badgeContent: Builder; - constructor(action: Action, activityName: string = action.label, keybinding: string = null) { + constructor(action: ActivityAction, activityName: string = action.label, keybinding: string = null) { super(null, action); this.cssClass = action.class; this.name = activityName; this._keybinding = keybinding; + action.onDidChangeBadge(this._handleBadgeChangeEvenet, this, this._callOnDispose); } public render(container: HTMLElement): void { @@ -158,7 +160,7 @@ export class ActivityActionItem extends BaseActionItem { } } - public _updateClass(): void { + protected _updateClass(): void { if (this.cssClass) { this.$badge.removeClass(this.cssClass); } @@ -167,7 +169,7 @@ export class ActivityActionItem extends BaseActionItem { this.$badge.addClass(this.cssClass); } - public _updateChecked(): void { + protected _updateChecked(): void { if (this.getAction().checked) { this.$e.addClass('active'); } else { @@ -175,16 +177,14 @@ export class ActivityActionItem extends BaseActionItem { } } - public _updateUnknown(event: EmitterEvent): void { - if (event.getType() === ActivityAction.BADGE) { - let action = this.getAction(); - if (action instanceof ActivityAction) { - this.updateBadge(( action).getBadge()); - } + private _handleBadgeChangeEvenet(): void { + let action = this.getAction(); + if (action instanceof ActivityAction) { + this.updateBadge((action).getBadge()); } } - public _updateEnabled(): void { + protected _updateEnabled(): void { if (this.getAction().enabled) { this.builder.removeClass('disabled'); } else { diff --git a/src/vs/workbench/parts/debug/browser/debugViewer.ts b/src/vs/workbench/parts/debug/browser/debugViewer.ts index 3b2badeeada..82f9290f331 100644 --- a/src/vs/workbench/parts/debug/browser/debugViewer.ts +++ b/src/vs/workbench/parts/debug/browser/debugViewer.ts @@ -990,7 +990,7 @@ export class BreakpointsActionProvider implements renderer.IActionProvider { actions.push(this.instantiationService.createInstance(debugactions.RemoveAllBreakpointsAction, debugactions.RemoveAllBreakpointsAction.ID, debugactions.RemoveAllBreakpointsAction.LABEL)); actions.push(new actionbar.Separator()); - actions.push(this.instantiationService.createInstance(debugactions.ToggleBreakpointsActivatedAction, debugactions.ToggleBreakpointsActivatedAction.ID, debugactions.ToggleBreakpointsActivatedAction.LABEL)); + actions.push(this.instantiationService.createInstance(debugactions.ToggleBreakpointsActivatedAction, debugactions.ToggleBreakpointsActivatedAction.ID, debugactions.ToggleBreakpointsActivatedAction.ACTIVATE_LABEL)); actions.push(new actionbar.Separator()); actions.push(this.instantiationService.createInstance(debugactions.EnableAllBreakpointsAction, debugactions.EnableAllBreakpointsAction.ID, debugactions.EnableAllBreakpointsAction.LABEL)); diff --git a/src/vs/workbench/parts/debug/browser/debugViews.ts b/src/vs/workbench/parts/debug/browser/debugViews.ts index da0596e6aed..07a58995283 100644 --- a/src/vs/workbench/parts/debug/browser/debugViews.ts +++ b/src/vs/workbench/parts/debug/browser/debugViews.ts @@ -387,7 +387,7 @@ export class BreakpointsView extends viewlet.AdaptiveCollapsibleViewletView { public getActions(): actions.IAction[] { return [ this.instantiationService.createInstance(debugactions.AddFunctionBreakpointAction, debugactions.AddFunctionBreakpointAction.ID, debugactions.AddFunctionBreakpointAction.LABEL), - this.instantiationService.createInstance(debugactions.ToggleBreakpointsActivatedAction, debugactions.ToggleBreakpointsActivatedAction.ID, debugactions.ToggleBreakpointsActivatedAction.LABEL), + this.instantiationService.createInstance(debugactions.ToggleBreakpointsActivatedAction, debugactions.ToggleBreakpointsActivatedAction.ID, debugactions.ToggleBreakpointsActivatedAction.ACTIVATE_LABEL), this.instantiationService.createInstance(debugactions.RemoveAllBreakpointsAction, debugactions.RemoveAllBreakpointsAction.ID, debugactions.RemoveAllBreakpointsAction.LABEL) ]; } diff --git a/src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts b/src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts index b456f786e76..ede9a31dbbe 100644 --- a/src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts +++ b/src/vs/workbench/parts/extensions/electron-browser/extensionsActions.ts @@ -104,9 +104,7 @@ export class CombinedInstallAction extends Action { this.uninstallAction = instantiationService.createInstance(UninstallAction, extension); this.disposables.push(this.installAction, this.uninstallAction); - this.disposables.push(this.installAction.addListener2(Action.ENABLED, () => this.update())); - this.disposables.push(this.installAction.addListener2(Action.LABEL, () => this.update())); - this.disposables.push(this.uninstallAction.addListener2(Action.ENABLED, () => this.update())); + this.installAction.onDidChange(this.update, this, this.disposables); this.update(); } -- GitLab