From 610918d48f581d2644745fa51c63515046df9ae9 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Sun, 18 Nov 2018 13:45:25 +0100 Subject: [PATCH] Fix #62861 (Proper fix) --- .../parts/activitybar/activitybarPart.ts | 125 +++++++++++------- src/vs/workbench/browser/parts/views/views.ts | 26 +--- 2 files changed, 77 insertions(+), 74 deletions(-) diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts index dd027b5632c..ed31d13c498 100644 --- a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts +++ b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts @@ -30,30 +30,30 @@ import { IExtensionService } from 'vs/workbench/services/extensions/common/exten import { URI } from 'vs/base/common/uri'; import { ToggleCompositePinnedAction, ICompositeBarColors } from 'vs/workbench/browser/parts/compositeBarActions'; import { ViewletDescriptor } from 'vs/workbench/browser/viewlet'; -import { IViewsService, IViewContainersRegistry, Extensions as ViewContainerExtensions } from 'vs/workbench/common/views'; +import { IViewsService, IViewContainersRegistry, Extensions as ViewContainerExtensions, ViewContainer, TEST_VIEW_CONTAINER_ID, IViewDescriptorCollection } from 'vs/workbench/common/views'; import { IContextKeyService, ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; +import { IViewlet } from 'vs/workbench/common/viewlet'; -interface IPlaceholderComposite { +const SCM_VIEWLET_ID = 'workbench.view.scm'; + +interface ICachedViewlet { id: string; iconUrl: URI; -} - -interface ISerializedPlaceholderComposite extends IPlaceholderComposite { - whens?: string[]; + views?: { when: string }[]; } export class ActivitybarPart extends Part { private static readonly ACTION_HEIGHT = 50; private static readonly PINNED_VIEWLETS = 'workbench.activity.pinnedViewlets'; - private static readonly PLACEHOLDER_VIEWLETS = 'workbench.activity.placeholderViewlets'; + private static readonly CACHED_VIEWLETS = 'workbench.activity.placeholderViewlets'; private dimension: Dimension; private globalActionBar: ActionBar; private globalActivityIdToActions: { [globalActivityId: string]: GlobalActivityAction; } = Object.create(null); - private placeholderComposites: IPlaceholderComposite[] = []; + private cachedViewlets: ICachedViewlet[] = []; private compositeBar: CompositeBar; private compositeActions: { [compositeId: string]: { activityAction: ViewletActivityAction, pinnedAction: ToggleCompositePinnedAction } } = Object.create(null); @@ -67,7 +67,7 @@ export class ActivitybarPart extends Part { @IStorageService private storageService: IStorageService, @IExtensionService private extensionService: IExtensionService, @IViewsService private viewsService: IViewsService, - @IContextKeyService contextKeyService: IContextKeyService + @IContextKeyService private contextKeyService: IContextKeyService ) { super(id, { hasTitle: false }, themeService, storageService); @@ -87,31 +87,23 @@ export class ActivitybarPart extends Part { overflowActionSize: ActivitybarPart.ACTION_HEIGHT })); - const previousState = this.storageService.get(ActivitybarPart.PLACEHOLDER_VIEWLETS, StorageScope.GLOBAL, '[]'); - const serializedPlaceholderComposites = JSON.parse(previousState); - this.placeholderComposites = []; - for (const { id, iconUrl, whens } of serializedPlaceholderComposites) { - if (whens && whens.length > 0) { - if (whens.every(when => !contextKeyService.contextMatchesRules(ContextKeyExpr.deserialize(when)))) { - // Hidden by default - continue; - } + const previousState = this.storageService.get(ActivitybarPart.CACHED_VIEWLETS, StorageScope.GLOBAL, '[]'); + this.cachedViewlets = (JSON.parse(previousState)).map(({ id, iconUrl, views }) => ({ id, views, iconUrl: typeof iconUrl === 'object' ? URI.revive(iconUrl) : void 0 })); + for (const cachedViewlet of this.cachedViewlets) { + if (this.shouldBeHidden(cachedViewlet.id, cachedViewlet)) { + this.compositeBar.hideComposite(cachedViewlet.id); } - this.placeholderComposites.push({ - id, - iconUrl: typeof iconUrl === 'object' ? URI.revive(iconUrl) : void 0 - }); } this.registerListeners(); - this.updateCompositebar(); + this.onDidRegisterViewlets(viewletService.getAllViewlets()); } private registerListeners(): void { - this._register(this.viewletService.onDidViewletRegister(() => this.updateCompositebar())); + this._register(this.viewletService.onDidViewletRegister(viewlet => this.onDidRegisterViewlets([viewlet]))); // Activate viewlet action on opening of a viewlet - this._register(this.viewletService.onDidViewletOpen(viewlet => this.compositeBar.activateComposite(viewlet.getId()))); + this._register(this.viewletService.onDidViewletOpen(viewlet => this.onDidViewletOpen(viewlet))); // Deactivate viewlet action on close this._register(this.viewletService.onDidViewletClose(viewlet => this.compositeBar.deactivateComposite(viewlet.getId()))); @@ -128,7 +120,28 @@ export class ActivitybarPart extends Part { private onDidRegisterExtensions(): void { this.removeNotExistingComposites(); - this.updateCompositebar(); + for (const viewlet of this.viewletService.getAllViewlets()) { + this.enableCompositeActions(viewlet); + const viewContainer = this.getViewContainer(viewlet.id); + if (viewContainer) { + const viewDescriptors = this.viewsService.getViewDescriptors(viewContainer); + this.onDidChangeActiveViews(viewlet, viewDescriptors); + viewDescriptors.onDidChangeActiveViews(() => this.onDidChangeActiveViews(viewlet, viewDescriptors)); + } + } + } + + private onDidChangeActiveViews(viewlet: ViewletDescriptor, viewDescriptors: IViewDescriptorCollection): void { + if (viewDescriptors.activeViewDescriptors.length) { + this.compositeBar.addComposite(viewlet); + } else { + this.removeComposite(viewlet.id, true); + } + } + + private onDidViewletOpen(viewlet: IViewlet): void { + this.compositeBar.addComposite(this.viewletService.getViewlet(viewlet.getId())); + this.compositeBar.activateComposite(viewlet.getId()); } showActivity(viewletOrActionId: string, badge: IBadge, clazz?: string, priority?: number): IDisposable { @@ -253,9 +266,9 @@ export class ActivitybarPart extends Part { pinnedAction: new ToggleCompositePinnedAction(viewlet, this.compositeBar) }; } else { - const placeHolderComposite = this.placeholderComposites.filter(c => c.id === compositeId)[0]; + const cachedComposite = this.cachedViewlets.filter(c => c.id === compositeId)[0]; compositeActions = { - activityAction: this.instantiationService.createInstance(PlaceHolderViewletActivityAction, compositeId, placeHolderComposite && placeHolderComposite.iconUrl), + activityAction: this.instantiationService.createInstance(PlaceHolderViewletActivityAction, compositeId, cachedComposite && cachedComposite.iconUrl), pinnedAction: new PlaceHolderToggleCompositePinnedAction(compositeId, this.compositeBar) }; } @@ -266,25 +279,33 @@ export class ActivitybarPart extends Part { return compositeActions; } - private updateCompositebar(): void { - const viewlets = this.viewletService.getViewlets(); + private onDidRegisterViewlets(viewlets: ViewletDescriptor[]): void { for (const viewlet of viewlets) { - const existsBefore = this.compositeBar.getComposites().some(({ id }) => id === viewlet.id); - this.compositeBar.addComposite(viewlet); + const cachedViewlet = this.cachedViewlets.filter(({ id }) => id === viewlet.id)[0]; + const activeViewlet = this.viewletService.getActiveViewlet(); + const isActive = activeViewlet && activeViewlet.getId() === viewlet.id; - // Pin it by default if it is new - if (!existsBefore) { - this.compositeBar.pin(viewlet.id); - } + if (isActive || !this.shouldBeHidden(viewlet.id, cachedViewlet)) { + this.compositeBar.addComposite(viewlet); - this.enableCompositeActions(viewlet); - const activeViewlet = this.viewletService.getActiveViewlet(); - if (activeViewlet && activeViewlet.getId() === viewlet.id) { - this.compositeBar.activateComposite(viewlet.id); + // Pin it by default if it is new + if (!cachedViewlet) { + this.compositeBar.pin(viewlet.id); + } + + if (isActive) { + this.compositeBar.activateComposite(viewlet.id); + } } } } + private shouldBeHidden(viewletId: string, cachedViewlet: ICachedViewlet): boolean { + return cachedViewlet && cachedViewlet.views && cachedViewlet.views.length + ? cachedViewlet.views.every(({ when }) => when && !this.contextKeyService.contextMatchesRules(ContextKeyExpr.deserialize(when))) + : viewletId === TEST_VIEW_CONTAINER_ID /* Hide Test viewlet for the first time or it had no views registered before */; + } + private removeNotExistingComposites(): void { const viewlets = this.viewletService.getAllViewlets(); for (const { id } of this.compositeBar.getComposites()) { @@ -347,22 +368,28 @@ export class ActivitybarPart extends Part { } protected saveState(): void { - const viewContainerRegistry = Registry.as(ViewContainerExtensions.ViewContainersRegistry); - const state: ISerializedPlaceholderComposite[] = []; + const state: ICachedViewlet[] = []; for (const { id, iconUrl } of this.viewletService.getAllViewlets()) { - const viewContainer = viewContainerRegistry.get(id); - const whens: string[] = []; + const viewContainer = this.getViewContainer(id); + const views: { when: string }[] = []; if (viewContainer) { for (const { when } of this.viewsService.getViewDescriptors(viewContainer).allViewDescriptors) { - if (when) { - whens.push(when.serialize()); - } + views.push({ when: when ? when.serialize() : void 0 }); } } - state.push({ id, iconUrl, whens }); + state.push({ id, iconUrl, views }); } - this.storageService.store(ActivitybarPart.PLACEHOLDER_VIEWLETS, JSON.stringify(state), StorageScope.GLOBAL); + this.storageService.store(ActivitybarPart.CACHED_VIEWLETS, JSON.stringify(state), StorageScope.GLOBAL); super.saveState(); } + + private getViewContainer(viewletId: string): ViewContainer | undefined { + // TODO: @Joao Remove this after moving SCM Viewlet to ViewContainerViewlet - https://github.com/Microsoft/vscode/issues/49054 + if (viewletId === SCM_VIEWLET_ID) { + return null; + } + const viewContainerRegistry = Registry.as(ViewContainerExtensions.ViewContainersRegistry); + return viewContainerRegistry.get(viewletId); + } } diff --git a/src/vs/workbench/browser/parts/views/views.ts b/src/vs/workbench/browser/parts/views/views.ts index a79de8d231d..0c660036ce4 100644 --- a/src/vs/workbench/browser/parts/views/views.ts +++ b/src/vs/workbench/browser/parts/views/views.ts @@ -5,11 +5,9 @@ import 'vs/css!./media/views'; import { Disposable } from 'vs/base/common/lifecycle'; -import { IViewsService, ViewsRegistry, IViewsViewlet, ViewContainer, IViewDescriptor, IViewContainersRegistry, Extensions as ViewContainerExtensions, TEST_VIEW_CONTAINER_ID, IView, IViewDescriptorCollection } from 'vs/workbench/common/views'; +import { IViewsService, ViewsRegistry, IViewsViewlet, ViewContainer, IViewDescriptor, IViewContainersRegistry, Extensions as ViewContainerExtensions, IView, IViewDescriptorCollection } from 'vs/workbench/common/views'; import { Registry } from 'vs/platform/registry/common/platform'; -import { ViewletRegistry, Extensions as ViewletExtensions } from 'vs/workbench/browser/viewlet'; import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; -import { ILifecycleService, LifecyclePhase } from 'vs/platform/lifecycle/common/lifecycle'; import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; import { IContextKeyService, IContextKeyChangeEvent, IReadableSet, IContextKey, RawContextKey, ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; import { Event, chain, filterEvent, Emitter } from 'vs/base/common/event'; @@ -511,8 +509,6 @@ export class PersistentContributableViewsModel extends ContributableViewsModel { } } -const SCM_VIEWLET_ID = 'workbench.view.scm'; - export class ViewsService extends Disposable implements IViewsService { _serviceBrand: any; @@ -521,9 +517,7 @@ export class ViewsService extends Disposable implements IViewsService { private readonly activeViewContextKeys: Map>; constructor( - @ILifecycleService private lifecycleService: ILifecycleService, @IViewletService private viewletService: IViewletService, - @IStorageService private storageService: IStorageService, @IContextKeyService private contextKeyService: IContextKeyService ) { super(); @@ -537,7 +531,6 @@ export class ViewsService extends Disposable implements IViewsService { const viewContainersRegistry = Registry.as(ViewContainerExtensions.ViewContainersRegistry); viewContainersRegistry.all.forEach(viewContainer => this.onDidRegisterViewContainer(viewContainer)); this._register(viewContainersRegistry.onDidRegister(viewContainer => this.onDidRegisterViewContainer(viewContainer))); - this._register(Registry.as(ViewletExtensions.Viewlets).onDidRegister(viewlet => this.viewletService.setViewletEnablement(viewlet.id, this.storageService.getBoolean(`viewservice.${viewlet.id}.enablement`, StorageScope.WORKSPACE, viewlet.id !== TEST_VIEW_CONTAINER_ID)))); } getViewDescriptors(container: ViewContainer): IViewDescriptorCollection { @@ -562,23 +555,12 @@ export class ViewsService extends Disposable implements IViewsService { } private onDidRegisterViewContainer(viewContainer: ViewContainer): void { - const viewDescriptorCollection = this.registerViewDescriptorCollection(viewContainer); - - // TODO: @Joao Remove this after moving SCM Viewlet to ViewContainerViewlet - https://github.com/Microsoft/vscode/issues/49054 - if (viewContainer.id !== SCM_VIEWLET_ID) { - this._register(viewDescriptorCollection.onDidChangeActiveViews(() => this.updateViewletEnablement(viewContainer, viewDescriptorCollection))); - this.lifecycleService.when(LifecyclePhase.Eventually).then(() => this.updateViewletEnablement(viewContainer, viewDescriptorCollection)); - } - } - - private registerViewDescriptorCollection(viewContainer: ViewContainer): IViewDescriptorCollection { const viewDescriptorCollection = this._register(new ViewDescriptorCollection(viewContainer, this.contextKeyService)); this.onDidChangeActiveViews({ added: viewDescriptorCollection.activeViewDescriptors, removed: [] }); this._register(viewDescriptorCollection.onDidChangeActiveViews(changed => this.onDidChangeActiveViews(changed))); this.viewDescriptorCollections.set(viewContainer, viewDescriptorCollection); - return viewDescriptorCollection; } private onDidChangeActiveViews({ added, removed }: { added: IViewDescriptor[], removed: IViewDescriptor[] }): void { @@ -627,10 +609,4 @@ export class ViewsService extends Disposable implements IViewsService { } return contextKey; } - - private updateViewletEnablement(viewContainer: ViewContainer, viewDescriptorCollection: IViewDescriptorCollection): void { - const enabled = viewDescriptorCollection.activeViewDescriptors.length > 0; - this.viewletService.setViewletEnablement(viewContainer.id, enabled); - this.storageService.store(`viewservice.${viewContainer.id}.enablement`, enabled, StorageScope.WORKSPACE); - } } -- GitLab