Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additions to Tree view API #52300

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 50 additions & 4 deletions src/vs/vscode.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6079,6 +6079,30 @@ declare module 'vscode' {

}

/**
* The event that is fired when there is a change in [tree view's selection](#TreeView.selection)
*/
export interface TreeViewSelectionChangeEvent<T> {

/**
* Selected elements.
*/
selection: T[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add readonly and ReadonlyArray<T>?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was there in last milestone. Hope the change will not break.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just readonly should be enough and non breaking


}

/**
* The event that is fired when there is a change in [tree view's visibility](#TreeView.visible)
*/
export interface TreeViewVisibilityChangeEvent {

/**
* `true` if the [tree view](#TreeView) is visible otherwise `false`.
*/
visible: boolean;

}

/**
* Represents a Tree view
*/
Expand All @@ -6100,16 +6124,33 @@ declare module 'vscode' {
readonly selection: ReadonlyArray<T>;

/**
* Reveal an element. By default revealed element is selected.
* Event that is fired when the [selection](#TreeView.selection) has changed
*/
readonly onDidChangeSelection: Event<TreeViewSelectionChangeEvent<T>>;

/**
* `true` if the [tree view](#TreeView) is visible otherwise `false`.
*/
readonly visible: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only means visible and not focused, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes


/**
* Event that is fired when [visibility](TreeView.visible) has changed
*/
readonly onDidChangeVisibility: Event<TreeViewVisibilityChangeEvent>;

/**
* Reveals the given element in the tree view.
* If the tree view is not visible then the tree view is shown and element is revealed.
*
* By default revealed element is selected and not focused.
* In order to not to select, set the option `select` to `false`.
* In order to focus, set the option `focus` to `true`.
*
* **NOTE:** [TreeDataProvider](#TreeDataProvider) is required to implement [getParent](#TreeDataProvider.getParent) method to access this API.
*/
reveal(element: T, options?: { select?: boolean }): Thenable<void>;
reveal(element: T, options?: { select?: boolean, focus?: boolean }): Thenable<void>;
}


/**
* A data provider that provides tree data
*/
Expand Down Expand Up @@ -6183,10 +6224,15 @@ declare module 'vscode' {
tooltip?: string | undefined;

/**
* The [command](#Command) which should be run when the tree item is selected.
* The [command](#Command) that should run when the user selects the tree item.
*/
command?: Command;

/**
* The [command](#Command) that should run when the user double clicks on the tree item.
*/
doubleClickCommand?: Command;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually don't mention user-gestures like click, doubleClick but we use terms that describe their meaning... Something like alternativeCommand etc. Unsure about a better name... What is this being used for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I am aware that we do not mention user-gestures. But here I did not find a better name. Usage is pretty simple and from its name, this command gets executed when user double clicks on a node.

Just to note, even though it is not in API, we talk about single click / double click behaviour in one of our settings workbench.settings.openMode. Thought this will align with that.

Copy link
Member

@jrieken jrieken Jun 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, settings name are the wild-wild-west ;-) What I was asking is why we are adding this and how is this being used? I would find a it rather weird that double vs single clicking an item is different. Since you brought it up, openMode is now being ignored for contributed trees, right? Is that something we want?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was asking is why we are adding this and how is this being used?

One of the use cases is that in a script view, run script on double click and not on single click. Another use case is to pin a document after opening it by double clicking from tree view.

openMode is now being ignored for contributed trees, right? Is that something we want?

I do not know but looks like it is being respected. But I do not want that behaviour in custom trees.

In general, Single click selects or highlights an element and whereas double click executes the function associated to that element. Currently command property provides a function that should be executed on select. We need a placeholder for a property that provides a function associated to the given element. command looks to me the right word for that but it is already taken.

It seems Single clicking is usually a primary action of the mouse we can call double click command as secondaryCommand ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I do not want that behaviour in custom trees.

That's something for the UX call I'd say.. Personally, I like when they look and feel alike.

In general, Single click selects or highlights an element and whereas double click executes the function associated to that element.

Not sure so about that... Clicking an element in the explorer always opens it, double clicking it also opens it but pins it as well. That reminds me of the alternative menu item which are meant to be a similar but slightly different action. So, maybe call it altCommand or alternativeCommand.

What would really confuse me as a user would be a tree that doesn't do anything on single click but that does do something on double click. So, maybe we should gear the API towards that not being possible. something like { command: Command | { cmd: Command, alt: Command }}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's something for the UX call I'd say.. Personally, I like when they look and feel alike.

Custom trees cannot inherit generic behaviour because each node behaves differently. For example, if a node has both command and altCommand and user has openMode configured to doubleClick then what is expected behaviour? I would say in this case commands take the precedence over setting and execute command on select (single click) and altCommand on doubleClick is expected.

So, maybe we should gear the API towards that not being possible. something like { command: Command | { cmd: Command, alt: Command }}

I like it 👍


/**
* [TreeItemCollapsibleState](#TreeItemCollapsibleState) of the tree item.
*/
Expand Down
5 changes: 3 additions & 2 deletions src/vs/workbench/api/electron-browser/mainThreadTreeViews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie
}
}

$reveal(treeViewId: string, item: ITreeItem, parentChain: ITreeItem[], options?: { select?: boolean }): TPromise<void> {
return this.viewsService.openView(treeViewId)
$reveal(treeViewId: string, item: ITreeItem, parentChain: ITreeItem[], options: { select: boolean, focus: boolean }): TPromise<void> {
return this.viewsService.openView(treeViewId, options.focus)
.then(() => {
const viewer = this.getTreeViewer(treeViewId);
return viewer ? viewer.reveal(item, parentChain, options) : null;
Expand All @@ -61,6 +61,7 @@ export class MainThreadTreeViews extends Disposable implements MainThreadTreeVie
this._register(treeViewer.onDidExpandItem(item => this._proxy.$setExpanded(treeViewId, item.handle, true)));
this._register(treeViewer.onDidCollapseItem(item => this._proxy.$setExpanded(treeViewId, item.handle, false)));
this._register(treeViewer.onDidChangeSelection(items => this._proxy.$setSelection(treeViewId, items.map(({ handle }) => handle))));
this._register(treeViewer.onDidChangeVisibility(isVisible => this._proxy.$setVisible(treeViewId, isVisible)));
}

private getTreeViewer(treeViewId: string): ITreeViewer {
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/api/node/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export interface MainThreadTextEditorsShape extends IDisposable {
export interface MainThreadTreeViewsShape extends IDisposable {
$registerTreeViewDataProvider(treeViewId: string): void;
$refresh(treeViewId: string, itemsToRefresh?: { [treeItemHandle: string]: ITreeItem }): TPromise<void>;
$reveal(treeViewId: string, treeItem: ITreeItem, parentChain: ITreeItem[], options?: { select?: boolean }): TPromise<void>;
$reveal(treeViewId: string, treeItem: ITreeItem, parentChain: ITreeItem[], options: { select: boolean, focus: boolean }): TPromise<void>;
}

export interface MainThreadErrorsShape extends IDisposable {
Expand Down Expand Up @@ -654,6 +654,7 @@ export interface ExtHostTreeViewsShape {
$getChildren(treeViewId: string, treeItemHandle?: string): TPromise<ITreeItem[]>;
$setExpanded(treeViewId: string, treeItemHandle: string, expanded: boolean): void;
$setSelection(treeViewId: string, treeItemHandles: string[]): void;
$setVisible(treeViewId: string, visible: boolean): void;
}

export interface ExtHostWorkspaceShape {
Expand Down
47 changes: 41 additions & 6 deletions src/vs/workbench/api/node/extHostTreeViews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { ExtHostCommands, CommandsConverter } from 'vs/workbench/api/node/extHos
import { asWinJsPromise } from 'vs/base/common/async';
import { TreeItemCollapsibleState, ThemeIcon } from 'vs/workbench/api/node/extHostTypes';
import { isUndefinedOrNull } from 'vs/base/common/types';
import { equals } from 'vs/base/common/arrays';

type TreeItemHandle = string;

Expand Down Expand Up @@ -52,7 +53,10 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
get onDidCollapseElement() { return treeView.onDidCollapseElement; },
get onDidExpandElement() { return treeView.onDidExpandElement; },
get selection() { return treeView.selectedElements; },
reveal: (element: T, options?: { select?: boolean }): Thenable<void> => {
get onDidChangeSelection() { return treeView.onDidChangeSelection; },
get visible() { return treeView.visible; },
get onDidChangeVisibility() { return treeView.onDidChangeVisibility; },
reveal: (element: T, options?: { select?: boolean, focus?: boolean }): Thenable<void> => {
return treeView.reveal(element, options);
},
dispose: () => {
Expand Down Expand Up @@ -86,6 +90,14 @@ export class ExtHostTreeViews implements ExtHostTreeViewsShape {
treeView.setSelection(treeItemHandles);
}

$setVisible(treeViewId: string, isVisible: boolean): void {
const treeView = this.treeViews.get(treeViewId);
if (!treeView) {
throw new Error(localize('treeView.notRegistered', 'No tree view with id \'{0}\' registered.', treeViewId));
}
treeView.setVisible(isVisible);
}

private createExtHostTreeViewer<T>(id: string, dataProvider: vscode.TreeDataProvider<T>): ExtHostTreeView<T> {
const treeView = new ExtHostTreeView<T>(id, dataProvider, this._proxy, this.commands.converter);
this.treeViews.set(id, treeView);
Expand Down Expand Up @@ -113,15 +125,24 @@ class ExtHostTreeView<T> extends Disposable {
private elements: Map<TreeItemHandle, T> = new Map<TreeItemHandle, T>();
private nodes: Map<T, TreeNode> = new Map<T, TreeNode>();

private _selectedElements: T[] = [];
get selectedElements(): T[] { return this._selectedElements; }
private _visible: boolean = true;
get visible(): boolean { return this._visible; }

private _selectedHandles: TreeItemHandle[] = [];
get selectedElements(): T[] { return this._selectedHandles.map(handle => this.getExtensionElement(handle)).filter(element => !isUndefinedOrNull(element)); }

private _onDidExpandElement: Emitter<vscode.TreeViewExpansionEvent<T>> = this._register(new Emitter<vscode.TreeViewExpansionEvent<T>>());
readonly onDidExpandElement: Event<vscode.TreeViewExpansionEvent<T>> = this._onDidExpandElement.event;

private _onDidCollapseElement: Emitter<vscode.TreeViewExpansionEvent<T>> = this._register(new Emitter<vscode.TreeViewExpansionEvent<T>>());
readonly onDidCollapseElement: Event<vscode.TreeViewExpansionEvent<T>> = this._onDidCollapseElement.event;

private _onDidChangeSelection: Emitter<vscode.TreeViewSelectionChangeEvent<T>> = this._register(new Emitter<vscode.TreeViewSelectionChangeEvent<T>>());
readonly onDidChangeSelection: Event<vscode.TreeViewSelectionChangeEvent<T>> = this._onDidChangeSelection.event;

private _onDidChangeVisibility: Emitter<vscode.TreeViewVisibilityChangeEvent> = this._register(new Emitter<vscode.TreeViewVisibilityChangeEvent>());
readonly onDidChangeVisibility: Event<vscode.TreeViewVisibilityChangeEvent> = this._onDidChangeVisibility.event;

private refreshPromise: TPromise<void> = TPromise.as(null);

constructor(private viewId: string, private dataProvider: vscode.TreeDataProvider<T>, private proxy: MainThreadTreeViewsShape, private commands: CommandsConverter) {
Expand Down Expand Up @@ -160,14 +181,18 @@ class ExtHostTreeView<T> extends Disposable {
return this.elements.get(treeItemHandle);
}

reveal(element: T, options?: { select?: boolean }): TPromise<void> {
reveal(element: T, options?: { select?: boolean, focus?: boolean }): TPromise<void> {
options = options ? options : { select: true, focus: false };
const select = isUndefinedOrNull(options.select) ? true : options.select;
const focus = isUndefinedOrNull(options.focus) ? false : options.focus;

if (typeof this.dataProvider.getParent !== 'function') {
return TPromise.wrapError(new Error(`Required registered TreeDataProvider to implement 'getParent' method to access 'reveal' method`));
}
return this.refreshPromise
.then(() => this.resolveUnknownParentChain(element))
.then(parentChain => this.resolveTreeNode(element, parentChain[parentChain.length - 1])
.then(treeNode => this.proxy.$reveal(this.viewId, treeNode.item, parentChain.map(p => p.item), options)));
.then(treeNode => this.proxy.$reveal(this.viewId, treeNode.item, parentChain.map(p => p.item), { select, focus })));
}

setExpanded(treeItemHandle: TreeItemHandle, expanded: boolean): void {
Expand All @@ -182,7 +207,17 @@ class ExtHostTreeView<T> extends Disposable {
}

setSelection(treeItemHandles: TreeItemHandle[]): void {
this._selectedElements = treeItemHandles.map(handle => this.getExtensionElement(handle)).filter(element => !isUndefinedOrNull(element));
if (!equals(this._selectedHandles, treeItemHandles)) {
this._selectedHandles = treeItemHandles;
this._onDidChangeSelection.fire({ selection: this.selectedElements });
}
}

setVisible(visible: boolean): void {
if (visible !== this._visible) {
this._visible = visible;
this._onDidChangeVisibility.fire({ visible: this._visible });
}
}

private resolveUnknownParentChain(element: T): TPromise<TreeNode[]> {
Expand Down
17 changes: 14 additions & 3 deletions src/vs/workbench/browser/parts/views/customView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ export class CustomTreeViewer extends Disposable implements ITreeViewer {
private _onDidChangeSelection: Emitter<ITreeItem[]> = this._register(new Emitter<ITreeItem[]>());
readonly onDidChangeSelection: Event<ITreeItem[]> = this._onDidChangeSelection.event;

private _onDidChangeVisibility: Emitter<boolean> = this._register(new Emitter<boolean>());
readonly onDidChangeVisibility: Event<boolean> = this._onDidChangeVisibility.event;

constructor(
private id: string,
private container: ViewContainer,
Expand Down Expand Up @@ -266,6 +269,8 @@ export class CustomTreeViewer extends Disposable implements ITreeViewer {
this.elementsToRefresh = [];
}
}

this._onDidChangeVisibility.fire(this.isVisible);
}

focus(): void {
Expand Down Expand Up @@ -336,13 +341,15 @@ export class CustomTreeViewer extends Disposable implements ITreeViewer {
return TPromise.as(null);
}

reveal(item: ITreeItem, parentChain: ITreeItem[], options?: { select?: boolean }): TPromise<void> {
reveal(item: ITreeItem, parentChain: ITreeItem[], options?: { select?: boolean, focus?: boolean }): TPromise<void> {
if (this.tree && this.isVisible) {
options = options ? options : { select: true };
options = options ? options : { select: false, focus: false };
const select = isUndefinedOrNull(options.select) ? false : options.select;
const focus = isUndefinedOrNull(options.focus) ? false : options.focus;

const root: Root = this.tree.getInput();
const promise = root.children ? TPromise.as(null) : this.refresh(); // Refresh if root is not populated
return promise.then(() => {
const select = isUndefinedOrNull(options.select) ? true : options.select;
var result = TPromise.as(null);
parentChain.forEach((e) => {
result = result.then(() => this.tree.expand(e));
Expand All @@ -352,6 +359,10 @@ export class CustomTreeViewer extends Disposable implements ITreeViewer {
if (select) {
this.tree.setSelection([item], { source: 'api' });
}
if (focus) {
this.focus();
this.tree.setFocus(item);
}
});
});
}
Expand Down
4 changes: 3 additions & 1 deletion src/vs/workbench/browser/parts/views/viewsViewlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ export abstract class ViewContainerViewlet extends PanelViewlet implements IView
}
view = this.getView(id);
view.setExpanded(true);
view.focus();
if (focus) {
view.focus();
}
return TPromise.as(view);
}

Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/common/views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ export interface ITreeViewer extends IDisposable {

readonly onDidChangeSelection: Event<ITreeItem[]>;

readonly onDidChangeVisibility: Event<boolean>;

refresh(treeItems?: ITreeItem[]): TPromise<void>;

setVisibility(visible: boolean): void;
Expand Down