Skip to content

Commit

Permalink
Fix tree view reveal() behavior
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@amiramw
amiramw committed Dec 7, 2020
1 parent 2d422af commit ac8719b
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 38 deletions.
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/common/plugin-api-rpc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
10 changes: 7 additions & 3 deletions packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
32 changes: 26 additions & 6 deletions packages/plugin-ext/src/main/browser/view/tree-views-main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
105 changes: 77 additions & 28 deletions packages/plugin-ext/src/plugin/tree/tree-views.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
}
Expand Down Expand Up @@ -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;
Expand All @@ -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> = {
Expand Down

0 comments on commit ac8719b

Please sign in to comment.