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 <amiram.wingarten@sap.com>
eclipse-theia · Nov 26, 2020 · 070ced0 · 070ced0
1 parent 6ef0867
commit 070ced0
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 38 deletions.
diff --git 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<void>;
-    $reveal(treeViewId: string, treeItemId: string, options: TreeViewRevealOptions): Promise<any>;
+    $reveal(treeViewId: string, elementParentChain: string[], options: TreeViewRevealOptions): Promise<any>;
     $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
@@ -292,10 +292,14 @@ export class PluginViewRegistry implements FrontendApplicationContribution {
         return this.widgetManager.getWidget<PluginViewWidget>(PLUGIN_VIEW_FACTORY_ID, this.toPluginViewWidgetIdentifier(viewId));
     }
 
-    async openView(viewId: string, options?: { activate?: boolean }): Promise<PluginViewWidget | undefined> {
+    async openView(viewId: string, options?: { activate?: boolean, reveal?: boolean }): Promise<PluginViewWidget | undefined> {
         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
@@ -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<any> {
-        const viewPanel = await this.viewRegistry.openView(treeViewId, { activate: options.focus });
+    async $reveal(treeViewId: string, elementParentChain: string[], options: TreeViewRevealOptions): Promise<any> {
+        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<void> {
+        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<void> {
         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
@@ -199,15 +199,9 @@ class TreeViewExtImpl<T> implements Disposable {
     async reveal(element: T, options?: Partial<TreeViewRevealOptions>): Promise<void> {
         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<T> 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<string[] | undefined> {
+        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<TreeViewItem[] | undefined> {
         const parentNode = this.nodes.get(parentId);
         const parent = parentNode && parentNode.value;
@@ -258,28 +323,12 @@ class TreeViewExtImpl<T> 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<T> = {