From f08007bf9dc0654ea5e400e54423276759ad5241 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 22 Aug 2018 16:49:49 +0200 Subject: [PATCH] Fix #54495 --- .../browser/viewsContainersExtensionPoint.ts | 10 +- .../parts/activitybar/activitybarPart.ts | 43 +++--- .../workbench/browser/parts/compositeBar.ts | 141 ++++++++++-------- .../browser/parts/panel/panelPart.ts | 2 +- src/vs/workbench/browser/parts/views/views.ts | 2 +- .../progress/test/progressService.test.ts | 4 + .../services/viewlet/browser/viewlet.ts | 5 + .../viewlet/browser/viewletService.ts | 2 +- 8 files changed, 117 insertions(+), 92 deletions(-) diff --git a/src/vs/workbench/api/browser/viewsContainersExtensionPoint.ts b/src/vs/workbench/api/browser/viewsContainersExtensionPoint.ts index fc67667e524..ae49565113f 100644 --- a/src/vs/workbench/api/browser/viewsContainersExtensionPoint.ts +++ b/src/vs/workbench/api/browser/viewsContainersExtensionPoint.ts @@ -94,6 +94,7 @@ class ViewsContainersExtensionHandler implements IWorkbenchContribution { } private handleAndRegisterCustomViewContainers() { + let order = TEST_VIEW_CONTAINER_ORDER + 1; viewsContainersExtensionPoint.setHandler((extensions) => { for (let extension of extensions) { const { value, collector } = extension; @@ -103,7 +104,7 @@ class ViewsContainersExtensionHandler implements IWorkbenchContribution { } switch (entry.key) { case 'activitybar': - this.registerCustomViewContainers(entry.value, extension.description); + order = this.registerCustomViewContainers(entry.value, extension.description, order); break; } }); @@ -139,12 +140,13 @@ class ViewsContainersExtensionHandler implements IWorkbenchContribution { return true; } - private registerCustomViewContainers(containers: IUserFriendlyViewsContainerDescriptor[], extension: IExtensionDescription) { - containers.forEach((descriptor, index) => { + private registerCustomViewContainers(containers: IUserFriendlyViewsContainerDescriptor[], extension: IExtensionDescription, order: number): number { + containers.forEach(descriptor => { const cssClass = `extensionViewlet-${descriptor.id}`; const icon = resources.joinPath(extension.extensionLocation, descriptor.icon); - this.registerCustomViewlet({ id: `workbench.view.extension.${descriptor.id}`, title: descriptor.title, icon }, TEST_VIEW_CONTAINER_ORDER + index + 1, cssClass, extension.id); + this.registerCustomViewlet({ id: `workbench.view.extension.${descriptor.id}`, title: descriptor.title, icon }, order++, cssClass, extension.id); }); + return order; } private registerCustomViewlet(descriptor: IUserFriendlyViewsContainerDescriptor2, order: number, cssClass: string, extensionId: string): void { diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts index 52d0ff4ccf3..70aa55141f3 100644 --- a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts +++ b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts @@ -87,20 +87,15 @@ export class ActivitybarPart extends Part { overflowActionSize: ActivitybarPart.ACTION_HEIGHT })); - const previousState = this.storageService.get(ActivitybarPart.PLACEHOLDER_VIEWLETS, StorageScope.GLOBAL, void 0); - if (previousState) { - let parsedPreviousState = JSON.parse(previousState); - parsedPreviousState.forEach((s) => { - if (typeof s.iconUrl === 'object') { - s.iconUrl = URI.revive(s.iconUrl); - } else { - s.iconUrl = void 0; - } - }); - this.placeholderComposites = parsedPreviousState; - } else { - this.placeholderComposites = this.compositeBar.getCompositesFromStorage().map(id => ({ id, iconUrl: void 0 })); - } + const previousState = this.storageService.get(ActivitybarPart.PLACEHOLDER_VIEWLETS, StorageScope.GLOBAL, '[]'); + this.placeholderComposites = JSON.parse(previousState); + this.placeholderComposites.forEach((s) => { + if (typeof s.iconUrl === 'object') { + s.iconUrl = URI.revive(s.iconUrl); + } else { + s.iconUrl = void 0; + } + }); this.registerListeners(); this.updateCompositebar(); @@ -119,7 +114,7 @@ export class ActivitybarPart extends Part { if (enabled) { this.compositeBar.addComposite(this.viewletService.getViewlet(id)); } else { - this.removeComposite(id); + this.removeComposite(id, true); } })); @@ -127,7 +122,7 @@ export class ActivitybarPart extends Part { } private onDidRegisterExtensions(): void { - this.removeNotExistingPlaceholderComposites(); + this.removeNotExistingComposites(); this.updateCompositebar(); } @@ -283,17 +278,21 @@ export class ActivitybarPart extends Part { } } - private removeNotExistingPlaceholderComposites(): void { - const viewlets = this.viewletService.getViewlets(); + private removeNotExistingComposites(): void { + const viewlets = this.viewletService.getAllViewlets(); for (const { id } of this.placeholderComposites) { if (viewlets.every(viewlet => viewlet.id !== id)) { - this.removeComposite(id); + this.removeComposite(id, false); } } } - private removeComposite(compositeId: string): void { - this.compositeBar.removeComposite(compositeId); + private removeComposite(compositeId: string, hide: boolean): void { + if (hide) { + this.compositeBar.hideComposite(compositeId); + } else { + this.compositeBar.removeComposite(compositeId); + } const compositeActions = this.compositeActions[compositeId]; if (compositeActions) { compositeActions.activityAction.dispose(); @@ -337,7 +336,7 @@ export class ActivitybarPart extends Part { } shutdown(): void { - const state = this.viewletService.getViewlets().map(viewlet => ({ id: viewlet.id, iconUrl: viewlet.iconUrl })); + const state = this.viewletService.getAllViewlets().map(({ id, iconUrl }) => ({ id, iconUrl })); this.storageService.store(ActivitybarPart.PLACEHOLDER_VIEWLETS, JSON.stringify(state), StorageScope.GLOBAL); super.shutdown(); diff --git a/src/vs/workbench/browser/parts/compositeBar.ts b/src/vs/workbench/browser/parts/compositeBar.ts index 8731222e1fe..266ad88a156 100644 --- a/src/vs/workbench/browser/parts/compositeBar.ts +++ b/src/vs/workbench/browser/parts/compositeBar.ts @@ -20,6 +20,7 @@ import { Dimension, $, addDisposableListener, EventType, EventHelper } from 'vs/ import { StandardMouseEvent } from 'vs/base/browser/mouseEvent'; import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; import { Widget } from 'vs/base/browser/ui/widget'; +import { isUndefinedOrNull } from 'vs/base/common/types'; export interface ICompositeBarOptions { icon: boolean; @@ -46,28 +47,22 @@ export class CompositeBar extends Widget implements ICompositeBar { private compositeOverflowActionItem: CompositeOverflowActivityActionItem; private model: CompositeBarModel; - private storedState: ISerializedCompositeBarItem[]; private visibleComposites: string[]; private compositeSizeInBar: Map; constructor( private options: ICompositeBarOptions, @IInstantiationService private instantiationService: IInstantiationService, - @IStorageService private storageService: IStorageService, + @IStorageService storageService: IStorageService, @IContextMenuService private contextMenuService: IContextMenuService ) { super(); - this.model = new CompositeBarModel(options); - this.storedState = this.loadCompositeItemsFromStorage(); + this.model = new CompositeBarModel(options, storageService); this.visibleComposites = []; this.compositeSizeInBar = new Map(); } - getCompositesFromStorage(): string[] { - return this.storedState.map(s => s.id); - } - create(parent: HTMLElement): HTMLElement { const actionBarDiv = parent.appendChild($('.composite-bar')); this.compositeSwitcherBar = this._register(new ActionBar(actionBarDiv, { @@ -93,7 +88,7 @@ export class CompositeBar extends Widget implements ICompositeBar { EventHelper.stop(e, true); CompositeActionItem.clearDraggedComposite(); - const targetItem = this.model.items[this.model.items.length - 1]; + const targetItem = this.model.visibleItems[this.model.visibleItems.length - 1]; if (targetItem && targetItem.id !== draggedCompositeId) { this.move(draggedCompositeId, targetItem.id); } @@ -113,38 +108,17 @@ export class CompositeBar extends Widget implements ICompositeBar { if (this.compositeSizeInBar.size === 0) { // Compute size of each composite by getting the size from the css renderer // Size is later used for overflow computation - this.computeSizes(this.model.items); + this.computeSizes(this.model.visibleItems); } this.updateCompositeSwitcher(); } addComposite({ id, name, order }: { id: string; name: string, order: number }): void { - const state = this.storedState.filter(s => s.id === id)[0]; - const pinned = state ? state.pinned : true; - let index = order >= 0 ? order : this.model.items.length; - - if (state) { - // Find the index by looking its previous item - index = 0; - for (let i = this.storedState.indexOf(state) - 1; i >= 0; i--) { - const previousItemId = this.storedState[i].id; - const previousItemIndex = this.model.findIndex(previousItemId); - if (previousItemIndex !== -1) { - index = previousItemIndex + 1; - break; - } - } - } - // Add to the model - if (this.model.add(id, name, order, index)) { + if (this.model.add(id, name, order)) { this.computeSizes([this.model.findItem(id)]); - if (pinned) { - this.pin(id); - } else { - this.updateCompositeSwitcher(); - } + this.updateCompositeSwitcher(); } } @@ -161,6 +135,12 @@ export class CompositeBar extends Widget implements ICompositeBar { } } + hideComposite(id: string): void { + if (this.model.hide(id)) { + this.updateCompositeSwitcher(); + } + } + activateComposite(id: string): void { const previousActiveItem = this.model.activeItem; if (this.model.activate(id)) { @@ -282,7 +262,7 @@ export class CompositeBar extends Widget implements ICompositeBar { return; // We have not been rendered yet so there is nothing to update. } - let compositesToShow = this.model.items.filter(item => + let compositesToShow = this.model.visibleItems.filter(item => item.pinned || (this.model.activeItem && this.model.activeItem.id === item.id) /* Show the active composite even if it is not pinned */ ).map(item => item.id); @@ -385,11 +365,11 @@ export class CompositeBar extends Widget implements ICompositeBar { } // Persist - this.saveCompositeItems(); + this.model.saveState(); } private getOverflowingComposites(): { id: string, name: string }[] { - let overflowingIds = this.model.items.filter(item => item.pinned).map(item => item.id); + let overflowingIds = this.model.visibleItems.filter(item => item.pinned).map(item => item.id); // Show the active composite even if it is not pinned if (this.model.activeItem && !this.model.activeItem.pinned) { @@ -397,13 +377,13 @@ export class CompositeBar extends Widget implements ICompositeBar { } overflowingIds = overflowingIds.filter(compositeId => this.visibleComposites.indexOf(compositeId) === -1); - return this.model.items.filter(c => overflowingIds.indexOf(c.id) !== -1); + return this.model.visibleItems.filter(c => overflowingIds.indexOf(c.id) !== -1); } private showContextMenu(e: MouseEvent): void { EventHelper.stop(e, true); const event = new StandardMouseEvent(e); - const actions: IAction[] = this.model.items + const actions: IAction[] = this.model.visibleItems .map(({ id, name, activityAction }) => ({ id, label: name, @@ -427,24 +407,13 @@ export class CompositeBar extends Widget implements ICompositeBar { getActions: () => TPromise.as(actions), }); } - - private loadCompositeItemsFromStorage(): ISerializedCompositeBarItem[] { - const storedStates = >JSON.parse(this.storageService.get(this.options.storageId, StorageScope.GLOBAL, '[]')); - const compositeStates = storedStates.map(c => - typeof c === 'string' /* migration from pinned states to composites states */ ? { id: c, pinned: true } : c); - return compositeStates; - } - - private saveCompositeItems(): void { - this.storedState = this.model.toJSON(); - this.storageService.store(this.options.storageId, JSON.stringify(this.storedState), StorageScope.GLOBAL); - } } interface ISerializedCompositeBarItem { id: string; pinned: boolean; order: number; + visible: boolean; } interface ICompositeBarItem extends ISerializedCompositeBarItem { @@ -456,15 +425,28 @@ interface ICompositeBarItem extends ISerializedCompositeBarItem { class CompositeBarModel { - readonly items: ICompositeBarItem[] = []; + private readonly options: ICompositeBarOptions; + readonly items: ICompositeBarItem[]; + activeItem: ICompositeBarItem; - constructor(private options: ICompositeBarOptions) { } + constructor( + options: ICompositeBarOptions, + private storageService: IStorageService, + ) { + this.options = options; + this.items = this.loadItemStates(); + } + + get visibleItems(): ICompositeBarItem[] { + return this.items.filter(item => item.visible); + } - private createCompositeBarItem(id: string, name: string, order: number, pinned: boolean): ICompositeBarItem { + private createCompositeBarItem(id: string, name: string, order: number, pinned: boolean, visible: boolean): ICompositeBarItem { const options = this.options; return { - id, name, pinned, order, activity: [], + id, name, pinned, order, visible, + activity: [], get activityAction() { return options.getActivityAction(id); }, @@ -474,20 +456,31 @@ class CompositeBarModel { }; } - add(id: string, name: string, order: number, index: number): boolean { + add(id: string, name: string, order: number): boolean { const item = this.findItem(id); if (item) { - item.order = order; + let changed = false; item.name = name; - return false; + if (!isUndefinedOrNull(order)) { + changed = item.order !== order; + item.order = order; + } + if (!item.visible) { + item.visible = true; + changed = true; + } + return changed; } else { - if (index === void 0) { - index = 0; + const item = this.createCompositeBarItem(id, name, order, false, true); + if (isUndefinedOrNull(order)) { + this.items.push(item); + } else { + let index = 0; while (index < this.items.length && this.items[index].order < order) { index++; } + this.items.splice(index, 0, item); } - this.items.splice(index, 0, this.createCompositeBarItem(id, name, order, false)); return true; } } @@ -502,6 +495,19 @@ class CompositeBarModel { return false; } + hide(id: string): boolean { + for (const item of this.items) { + if (item.id === id) { + if (item.visible) { + item.visible = false; + return true; + } + return false; + } + } + return false; + } + move(compositeId: string, toCompositeId: string): boolean { const fromIndex = this.findIndex(compositeId); @@ -610,7 +616,7 @@ class CompositeBarModel { return this.items.filter(item => item.id === id)[0]; } - findIndex(id: string): number { + private findIndex(id: string): number { for (let index = 0; index < this.items.length; index++) { if (this.items[index].id === id) { return index; @@ -619,7 +625,16 @@ class CompositeBarModel { return -1; } - toJSON(): ISerializedCompositeBarItem[] { - return this.items.map(({ id, pinned, order }) => ({ id, pinned, order })); + private loadItemStates(): ICompositeBarItem[] { + const storedStates = >JSON.parse(this.storageService.get(this.options.storageId, StorageScope.GLOBAL, '[]')); + return storedStates.map(c => { + const serialized: ISerializedCompositeBarItem = typeof c === 'string' /* migration from pinned states to composites states */ ? { id: c, pinned: true, order: void 0, visible: true } : c; + return this.createCompositeBarItem(serialized.id, void 0, serialized.order, serialized.pinned, isUndefinedOrNull(serialized.visible) ? true : serialized.visible); + }); + } + + saveState(): void { + const serialized = this.items.map(({ id, pinned, order, visible }) => ({ id, pinned, order, visible })); + this.storageService.store(this.options.storageId, JSON.stringify(serialized), StorageScope.GLOBAL); } } diff --git a/src/vs/workbench/browser/parts/panel/panelPart.ts b/src/vs/workbench/browser/parts/panel/panelPart.ts index 13df252c82f..1880db49212 100644 --- a/src/vs/workbench/browser/parts/panel/panelPart.ts +++ b/src/vs/workbench/browser/parts/panel/panelPart.ts @@ -281,7 +281,7 @@ export class PanelPart extends CompositePart implements IPanelService { } private removeComposite(compositeId: string): void { - this.compositeBar.removeComposite(compositeId); + this.compositeBar.hideComposite(compositeId); const compositeActions = this.compositeActions[compositeId]; if (compositeActions) { compositeActions.activityAction.dispose(); diff --git a/src/vs/workbench/browser/parts/views/views.ts b/src/vs/workbench/browser/parts/views/views.ts index 612b59f257f..2a49c79c496 100644 --- a/src/vs/workbench/browser/parts/views/views.ts +++ b/src/vs/workbench/browser/parts/views/views.ts @@ -496,7 +496,7 @@ 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.GLOBAL, viewlet.id !== TEST_VIEW_CONTAINER_ID)))); + 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)))); } openView(id: string, focus: boolean): TPromise { diff --git a/src/vs/workbench/services/progress/test/progressService.test.ts b/src/vs/workbench/services/progress/test/progressService.test.ts index f1f23e0cea1..67dd539396e 100644 --- a/src/vs/workbench/services/progress/test/progressService.test.ts +++ b/src/vs/workbench/services/progress/test/progressService.test.ts @@ -40,6 +40,10 @@ class TestViewletService implements IViewletService { return []; } + public getAllViewlets(): ViewletDescriptor[] { + return []; + } + public getActiveViewlet(): IViewlet { return activeViewlet; } diff --git a/src/vs/workbench/services/viewlet/browser/viewlet.ts b/src/vs/workbench/services/viewlet/browser/viewlet.ts index 02006a2397a..a6903bb9fb9 100644 --- a/src/vs/workbench/services/viewlet/browser/viewlet.ts +++ b/src/vs/workbench/services/viewlet/browser/viewlet.ts @@ -41,6 +41,11 @@ export interface IViewletService { */ getViewlet(id: string): ViewletDescriptor; + /** + * Returns all viewlets + */ + getAllViewlets(): ViewletDescriptor[]; + /** * Returns all enabled viewlets */ diff --git a/src/vs/workbench/services/viewlet/browser/viewletService.ts b/src/vs/workbench/services/viewlet/browser/viewletService.ts index 979b3ca5f19..30f05343f48 100644 --- a/src/vs/workbench/services/viewlet/browser/viewletService.ts +++ b/src/vs/workbench/services/viewlet/browser/viewletService.ts @@ -92,7 +92,7 @@ export class ViewletService extends Disposable implements IViewletService { .filter(v => v.enabled); } - private getAllViewlets(): ViewletDescriptor[] { + getAllViewlets(): ViewletDescriptor[] { return this.viewletRegistry.getViewlets() .sort((v1, v2) => v1.order - v2.order); } -- GitLab