From ac8719b6fc053811ed37cae2703df4dcab9a4576 Mon Sep 17 00:00:00 2001 From: Amiram Wingarten Date: Thu, 26 Nov 2020 14:03:52 +0200 Subject: [PATCH] Fix tree view reveal() behavior Various fixes to support tree view reveal in several unsupported scenarios and align it with vscode. - reveal called without focus now opens the view container (left pane) and view (section), without focus - reveal of a node that its ancestors are collapsed now expand all parents and reveals it - reveal of a node that was not cached by backend or not fetched by client (for example when showing the tree for the first time) now works Signed-off-by: Amiram Wingarten --- .../plugin-ext/src/common/plugin-api-rpc.ts | 2 +- .../main/browser/view/plugin-view-registry.ts | 10 +- .../src/main/browser/view/tree-views-main.ts | 32 +++++- .../plugin-ext/src/plugin/tree/tree-views.ts | 105 +++++++++++++----- 4 files changed, 111 insertions(+), 38 deletions(-) diff --git a/packages/plugin-ext/src/common/plugin-api-rpc.ts b/packages/plugin-ext/src/common/plugin-api-rpc.ts index a0fdb59a50181..68a86ab5dfa70 100644 --- a/packages/plugin-ext/src/common/plugin-api-rpc.ts +++ b/packages/plugin-ext/src/common/plugin-api-rpc.ts @@ -587,7 +587,7 @@ export interface TreeViewsMain { $registerTreeDataProvider(treeViewId: string): void; $unregisterTreeDataProvider(treeViewId: string): void; $refresh(treeViewId: string): Promise; - $reveal(treeViewId: string, treeItemId: string, options: TreeViewRevealOptions): Promise; + $reveal(treeViewId: string, elementParentChain: string[], options: TreeViewRevealOptions): Promise; $setMessage(treeViewId: string, message: string): void; $setTitle(treeViewId: string, title: string): void; } diff --git a/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts b/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts index cd8eb6573c589..fb0add26c710f 100644 --- a/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts +++ b/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts @@ -292,10 +292,14 @@ export class PluginViewRegistry implements FrontendApplicationContribution { return this.widgetManager.getWidget(PLUGIN_VIEW_FACTORY_ID, this.toPluginViewWidgetIdentifier(viewId)); } - async openView(viewId: string, options?: { activate?: boolean }): Promise { + async openView(viewId: string, options?: { activate?: boolean, reveal?: boolean }): Promise { const view = await this.doOpenView(viewId); - if (view && options && options.activate === true) { - await this.shell.activateWidget(view.id); + if (view && options) { + if (options.activate === true) { + await this.shell.activateWidget(view.id); + } else if (options.reveal === true) { + await this.shell.revealWidget(view.id); + } } return view; } diff --git a/packages/plugin-ext/src/main/browser/view/tree-views-main.ts b/packages/plugin-ext/src/main/browser/view/tree-views-main.ts index 96e29e572b2c4..f3eec6f2d8d44 100644 --- a/packages/plugin-ext/src/main/browser/view/tree-views-main.ts +++ b/packages/plugin-ext/src/main/browser/view/tree-views-main.ts @@ -18,10 +18,15 @@ import { interfaces } from 'inversify'; import { MAIN_RPC_CONTEXT, TreeViewsMain, TreeViewsExt, TreeViewRevealOptions } from '../../../common/plugin-api-rpc'; import { RPCProtocol } from '../../../common/rpc-protocol'; import { PluginViewRegistry, PLUGIN_VIEW_DATA_FACTORY_ID } from './plugin-view-registry'; -import { SelectableTreeNode, ExpandableTreeNode, CompositeTreeNode, WidgetManager } from '@theia/core/lib/browser'; +import { + SelectableTreeNode, + ExpandableTreeNode, + CompositeTreeNode, + WidgetManager +} from '@theia/core/lib/browser'; import { ViewContextKeyService } from './view-context-key-service'; import { Disposable, DisposableCollection } from '@theia/core'; -import { TreeViewWidget, TreeViewNode } from './tree-view-widget'; +import { TreeViewWidget, TreeViewNode, PluginTreeModel } from './tree-view-widget'; import { PluginViewWidget } from './plugin-view-widget'; export class TreeViewsMainImpl implements TreeViewsMain, Disposable { @@ -98,14 +103,17 @@ export class TreeViewsMainImpl implements TreeViewsMain, Disposable { } // eslint-disable-next-line @typescript-eslint/no-explicit-any - async $reveal(treeViewId: string, treeItemId: string, options: TreeViewRevealOptions): Promise { - const viewPanel = await this.viewRegistry.openView(treeViewId, { activate: options.focus }); + async $reveal(treeViewId: string, elementParentChain: string[], options: TreeViewRevealOptions): Promise { + const viewPanel = await this.viewRegistry.openView(treeViewId, { activate: options.focus, reveal: true }); const widget = viewPanel && viewPanel.widgets[0]; if (widget instanceof TreeViewWidget) { - const treeNode = widget.model.getNode(treeItemId); + // pop last element which is the node to reveal + const elementId = elementParentChain.pop(); + await this.expandParentChain(widget.model, elementParentChain); + const treeNode = widget.model.getNode(elementId); if (treeNode) { if (options.expand && ExpandableTreeNode.is(treeNode)) { - widget.model.expandNode(treeNode); + await widget.model.expandNode(treeNode); } if (options.select && SelectableTreeNode.is(treeNode)) { widget.model.selectNode(treeNode); @@ -114,6 +122,18 @@ export class TreeViewsMainImpl implements TreeViewsMain, Disposable { } } + /** + * Expand all parents of the node to reveal from root. This should also fetch missing nodes to the frontend. + */ + private async expandParentChain(model: PluginTreeModel, elementParentChain: string[]): Promise { + for (const elementId of elementParentChain) { + const treeNode = model.getNode(elementId); + if (ExpandableTreeNode.is(treeNode)) { + await model.expandNode(treeNode); + } + } + } + async $setMessage(treeViewId: string, message: string): Promise { const viewPanel = await this.viewRegistry.getView(treeViewId); if (viewPanel instanceof PluginViewWidget) { diff --git a/packages/plugin-ext/src/plugin/tree/tree-views.ts b/packages/plugin-ext/src/plugin/tree/tree-views.ts index 5eccbe65dae0c..7cb38d8104f5e 100644 --- a/packages/plugin-ext/src/plugin/tree/tree-views.ts +++ b/packages/plugin-ext/src/plugin/tree/tree-views.ts @@ -199,15 +199,9 @@ class TreeViewExtImpl implements Disposable { async reveal(element: T, options?: Partial): Promise { await this.pendingRefresh; - let elementId; - this.nodes.forEach((el, id) => { - if (Object.is(el.value, element)) { - elementId = id; - } - }); - - if (elementId) { - return this.proxy.$reveal(this.treeViewId, elementId, { + const elementParentChain = await this.calculateRevealParentChain(element); + if (elementParentChain) { + return this.proxy.$reveal(this.treeViewId, elementParentChain, { select: true, focus: false, expand: false, ...options }); } @@ -238,6 +232,77 @@ class TreeViewExtImpl implements Disposable { return element && element.value; } + /** + * calculate the chain of node ids from root to element so that the frontend can expand all of them and reveal element. + * this is needed as the frontend may not have the full tree nodes. + * throughout the parent chain this.getChildren is called in order to fill this.nodes cache. + * + * returns undefined if wasn't able to calculate the path due to inconsistencies. + * + * @param element element to reveal + */ + private async calculateRevealParentChain(element: T | undefined): Promise { + if (!element) { + // root + return ['']; + } + const parent = this.treeDataProvider.getParent && await this.treeDataProvider.getParent(element); + const chain = await this.calculateRevealParentChain(parent); + if (!chain || chain.length === 0) { + // parents are inconsistent + return; + } + const parentId = chain[chain.length - 1]; + const treeItem = await this.treeDataProvider.getTreeItem(element); + if (treeItem.id) { + return chain.concat(treeItem.id); + } + const idLabel = this.getTreeItemIdLabel(treeItem); + // getChildren fills this.nodes and generate ids for them which are needed later + const children = await this.getChildren(parentId); + if (!children) { + return; // parent is inconsistent + } + let possibleIndex = children.length; + // find the right element id by searching all possible id names in the cache + while (possibleIndex-- > 0) { + const candidateId = this.buildTreeItemId(parentId, possibleIndex, idLabel); + if (this.nodes.has(candidateId)) { + return chain.concat(candidateId); + } + } + // couldn't calculate consistent parent chain and id + return; + } + + private getTreeItemLabel(treeItem: TreeItem2): string | undefined { + let label: string | undefined; + const treeItemLabel: string | TreeItemLabel | undefined = treeItem.label; + if (typeof treeItemLabel === 'object' && typeof treeItemLabel.label === 'string') { + label = treeItemLabel.label; + } else { + label = treeItem.label; + } + return label; + } + + private getTreeItemIdLabel(treeItem: TreeItem2): string | undefined { + let idLabel = this.getTreeItemLabel(treeItem); + // Use resource URI if label is not set + if (idLabel === undefined && treeItem.resourceUri) { + idLabel = treeItem.resourceUri.path.toString(); + idLabel = decodeURIComponent(idLabel); + if (idLabel.indexOf('/') >= 0) { + idLabel = idLabel.substring(idLabel.lastIndexOf('/') + 1); + } + } + return idLabel; + } + + private buildTreeItemId(parentId: string, index: number, idLabel?: string) { + return `${parentId}/${index}:${idLabel}`; + } + async getChildren(parentId: string): Promise { const parentNode = this.nodes.get(parentId); const parent = parentNode && parentNode.value; @@ -258,28 +323,12 @@ class TreeViewExtImpl implements Disposable { const treeItem: TreeItem2 = await this.treeDataProvider.getTreeItem(value); // Convert theia.TreeItem to the TreeViewItem - // Take a label - let label: string | undefined; - const treeItemLabel: string | TreeItemLabel | undefined = treeItem.label; - if (typeof treeItemLabel === 'object' && typeof treeItemLabel.label === 'string') { - label = treeItemLabel.label; - } else { - label = treeItem.label; - } - - let idLabel = label; - // Use resource URI if label is not set - if (idLabel === undefined && treeItem.resourceUri) { - idLabel = treeItem.resourceUri.path.toString(); - idLabel = decodeURIComponent(idLabel); - if (idLabel.indexOf('/') >= 0) { - idLabel = idLabel.substring(idLabel.lastIndexOf('/') + 1); - } - } + const label = this.getTreeItemLabel(treeItem); + const idLabel = this.getTreeItemIdLabel(treeItem); // Generate the ID // ID is used for caching the element - const id = treeItem.id || `${parentId}/${index}:${idLabel}`; + const id = treeItem.id || this.buildTreeItemId(parentId, index, idLabel); const toDisposeElement = new DisposableCollection(); const node: TreeExtNode = {