Fix tree view reveal() behavior

CHANGELOG.md

@@ -18,6 +18,7 @@
   - Can be configured via `THEIA_MINI_BROWSER_HOST_PATTERN` environment variable.
   - Clients must setup this new hostname in their DNS resolvers.
 - [task] remove bash login shell when run from task to align with vscode.
+- [plugin-ext] TreeViewsMain.$reveal second parameter changed from string element id to string array element parent chain
 
 ## v1.8.0 - 26/11/2020
 

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;
 }

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;
     }

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 {
@@ -97,15 +102,20 @@ export class TreeViewsMainImpl implements TreeViewsMain, Disposable {
         }
     }
 
+    // elementParentChain parameter contain a list of tree ids from root to the revealed node
+    // all parents of the revealed node should be fetched and expanded in order for it to reveal
     // 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 +124,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) {

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,75 @@ 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) {
+            // parents are inconsistent
+            return undefined;
+        }
+        const parentId = chain.length ? chain[chain.length - 1] : '';
+        const treeItem = await this.treeDataProvider.getTreeItem(element);
+        if (treeItem.id) {
+            return chain.concat(treeItem.id);
+        }
+        // getChildren fills this.nodes and generate ids for them which are needed later
+        const children = await this.getChildren(parentId);
+        if (!children) {
+            return undefined; // parent is inconsistent
+        }
+        const idLabel = this.getTreeItemIdLabel(treeItem);
+        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 undefined;
+    }
+
+    private getTreeItemLabel(treeItem: TreeItem2): string | undefined {
+        const treeItemLabel: string | TreeItemLabel | undefined = treeItem.label;
+        if (typeof treeItemLabel === 'object' && typeof treeItemLabel.label === 'string') {
+            return treeItemLabel.label;
+        } else {
+            return treeItem.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 | undefined): 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 +321,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> = {