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

Implement contributable tree views #26948

Closed
1 of 7 tasks
sandy081 opened this issue May 19, 2017 · 16 comments
Closed
1 of 7 tasks

Implement contributable tree views #26948

sandy081 opened this issue May 19, 2017 · 16 comments
Assignees
Labels
api plan-item VS Code - planned item for upcoming tree-views Extension tree view issues
Milestone

Comments

@sandy081
Copy link
Member

sandy081 commented May 19, 2017

Implement contributed views those can be shown in Explorer as trees.

  • Contribution point to describe a view
  • API to register a tree data provider for the view
  • Multi views explorer

Actions can be contributed to the following locations by giving proper when clause

  • View titlebar: view/title
  • On item:view/item/context

Example:

"contributes": {
    "menus": {
         "view/title": [
	    {
		"command": "extension.commandId",
		"when": "view == viewId",
		"group": "navigation"
	    }
         ],
         "view/item/context": [
	     {
		"command": "extension.commandId",
		"when": "view == viewId && viewItem == contextValue",
	      }
          ]
   }
}

More

  • Remember the view collapsible state and size

Deferred

  • View visibility
  • Extract icons from the resource Uri
  • Tree state persistence
  • Customizable location of the view inside explorer
  • Get children in pages
  • Ability to update the view label
@sandy081 sandy081 changed the title Implement Tree API Implement contributable tree views May 19, 2017
sandy081 added a commit that referenced this issue May 19, 2017
- Initial version of custom views and tree data provider API
@Tyriar Tyriar added the tree-views Extension tree view issues label May 19, 2017
@formulahendry
Copy link
Member

Awesome! Look forward to this API! 😃

@bpasero
Copy link
Member

bpasero commented May 22, 2017

FTP Explorer:
image

I played around with the API and here is some feedback:

  • overall love the simplicity of the API 👍
  • not restoring the UI state on restart is a very tough one for me. at the minimum I think we should restore the view expansion state and size. my scenario is a FTP explorer and always having to click it to show the contents is very annoying at times
  • in my case I am showing files in the same way the explorer does. I would like to reuse the same icons the explorer uses for files without me having to introduce new icons. for that, maybe the API should allow a TreeItem to have a resource URI that we then simply take as ResourceLabel and support icons with? Even more, if we had an associated resource, we could automatically open a document with that resource on click?
  • API: registerTreeDataProvider is not really related to the view in terms of naming, I suggest registerView (similar to registerCommand)
  • having a generic onDidChange on both text document provider and tree data provider is odd, because that way I cannot have my class implement both interfaces (their type is different)
  • suggestion: we should probably come up with a way to contribute views to a specific position in the same way I can set the location of an action when contributed into a menu. people will have the desire to put a view to the top or in between opened editors and the explorer itself

@sandy081
Copy link
Member Author

@bpasero Thanks for the feedback.

  • Agreed that, we need to support persisting view state. There are multiple use cases, one is what you mentioned and other is if there is a view that shows different tree based on the document opened. Then state should be stored when switching back and forth.

  • Good point to include resourceUri for enabling file icon themes

  • I see your point about registerTreeDataProvider. I think what we are using this API is to register a data provider for a view. So we can name it registerTreeDataProviderForView. View is already registered in package.json and having registerView again might be confusing

  • Reason to have a generic onDidChange listener is to support change on both element and root. To make it more specific and align with the provider, I can call onDidTreeDataChange.

  • Yes, you can contribute actions to the following locations: view/title and view/resource by giving proper when clause. Eg:

"view/title": [
	{
		"command": "extension.commandId",
		"when": "view == viewId",
		"group": "navigation"
	}
],
"view/resource": [
	{
		"command": "extension.commandId",
		"when": "view == viewId && resource == contextValue",
		"group": "navigation"
	}
]
  • Positioning of views, yes thought about it. Was thinking providing a property index in view contribution for the position of the view.

@bpasero
Copy link
Member

bpasero commented May 22, 2017

Re: registerTreeDataProvider

I just like the symmetry between contributing a command via package.json and having a method registerCommand

Re: onDidChange

That is fine, I am more concerned about the name clash with the text document provider. If all providers have the same event onDidChange, you cannot implement all the interfaces in one class in my extension. I suggest onDidChangeTextDocument (unfortunately we cannot change this anymore as it already is API) and onDidChangeTreeData.

Re: Positioning of views

Instead of introducing a new concept with index, why not reuse what we do for menus where you have menu item groups?

@bpasero
Copy link
Member

bpasero commented May 22, 2017

Another thing that might come up is being able to change the view label dynamically. For example for the ftp explorer I could maybe show the currently connected host in there.

@sandy081
Copy link
Member Author

I am having an idea to expose Views as an API just like other objects (editors, extensions, documents) and have some methods to control the view. This would be next step.

@sandy081 sandy081 added this to the May 2017 milestone May 22, 2017
@sandy081 sandy081 added the api label May 22, 2017
@Gama11
Copy link
Contributor

Gama11 commented May 22, 2017

in my case I am showing files in the same way the explorer does [...]

I have a very similar use case - I want to implement a tree view that shows a few folders (of dependencies) outside of the current workspace. That means tree view items should otherwise behave exactly like those in the existing file explorer. They should:

  • have a tooltip showing the URI
  • have the usual context menu for rename / delete / etc
  • support drag and drop into editor tabs or to move them around between folders
  • there might be more I'm not aware of

Maybe this could be tied to the planned resourceUri?

@sandy081
Copy link
Member Author

sandy081 commented May 22, 2017

@bpasero FYI Implemented remembering the view collapsible state and size.

kapture 2017-05-22 at 20 13 59

@kieferrm kieferrm mentioned this issue May 22, 2017
44 tasks
@rebornix
Copy link
Member

rebornix commented May 23, 2017

Implement Git History Explorer (used to view commit history and view diff in each commit) real quick.

screen shot 2017-05-22 at 8 15 06 pm

My 2 cents:

  • I really love the simplicity of the API, intuitive, easy to adopt. I spent 99% of the time on the real business code.
  • Currently getChildren doesn't support paging., in my extension, I don't have a way to load more commits if users scroll to the end of list.
  • Not quite sure what's contextValue in TreeItem used for.
  • It would be awesome if we can customize the context menu (I suppose it's on your list)
  • We can add a toolbar with limited actions for explorer, e.g., collapse and refresh buttons.

Tiny issues

  • I tried to use shortcuts for file explorer on custom explorer, not all of them work. For example, enter doesn't expand/collapse the node.
  • If we set icon path as string or Uri, it only works in light theme. After switching to Dark theme, the icon is gone.
  • Label of node is not vertically centered.
    screen shot 2017-05-22 at 8 28 04 pm

cc @joaomoreno you might be interested in this tiny stuff.

@sandy081
Copy link
Member Author

sandy081 commented May 23, 2017

@rebornix First of all thanks for the feedback. Very much appreciated.

Currently getChildren doesn't support paging., in my extension, I don't have a way to load more commits if users scroll to the end of list.

Yes there is no support for Paging and I thought about it. I think we need paging support in internal Tree API too.

Not quite sure what's contextValue in TreeItem used for.
It would be awesome if we can customize the context menu (I suppose it's on your list)
We can add a toolbar with limited actions for explorer, e.g., collapse and refresh buttons.

Actions on View/Title and View/Item are supported. Its my bad that I did not document them well. I will update the API doc explaining how to contribute actions with examples. Please see the description for how to contribute menu actions to view and view items. Context value should be used for when expression of menu item contributions.

I tried to use shortcuts for file explorer on custom explorer, not all of them work. For example, enter doesn't expand/collapse the node.

Right, I will see why it is not working

If we set icon path as string or Uri, it only works in light theme. After switching to Dark theme, the icon is gone.
Label of node is not vertically centered.

Fixed these yesterday and should work in today's builds

@jacobdufault
Copy link
Contributor

Nice! I put together a quick integration into cquery here.

image

Some thoughts:

  • TreeItemCollapsibleState should have a third state, ie, NotExpandable. It's confusing/unexpected that undefined carries unique semantic meaning.
  • Expanding/collapsing a tree view node with the mouse always invokes the associated command. This is not the case with keyboard navigation (always invoking is a bit strange, since in my integration it jumps to a code location - but I suppose I'm not using the API for the intended purpose :))
  • Commands arguments are ignored, instead, the tree node instance is passed as the first argument.
  • It'd be really nice to reuse the standard vscode icons that are used in, ie, document outline or code completion.

@sandy081
Copy link
Member Author

sandy081 commented May 30, 2017

@jacobdufault Thanks for the feedback

TreeItemCollapsibleState should have a third state, ie, NotExpandable. It's confusing/unexpected that undefined carries unique semantic meaning.

This is good one. Added a new entry called None.

Expanding/collapsing a tree view node with the mouse always invokes the associated command. This is not the case with keyboard navigation (always invoking is a bit strange, since in my integration it jumps to a code location - but I suppose I'm not using the API for the intended purpose :))

You can contribute commands only to those items you want to. These commands gets called when an user selects an item. This should be true with mouse and keyboard. (Will check if it is not working with keyboard).

Commands arguments are ignored, instead, the tree node instance is passed as the first argument.

Tree commands are triggered by the platform and it only knows the node the command belongs to. Hence only the item model is passed as an argument.

It'd be really nice to reuse the standard vscode icons that are used in, ie, document outline or code completion.

Yes, there is a plan to reuse file based icons provided that item gives a file uri. Will think about the other icons too.

@sandy081
Copy link
Member Author

@jacobdufault

Commands arguments are ignored, instead, the tree node instance is passed as the first argument.

Ignore my previous comment regarding this. I am mistaken. Yes this is a bug which will be fixed here #27535

@alvarorahul
Copy link

Is there a plan to provide virtualization support for this tree views? I would love to connect this to trees that may have 100k+ nodes or even 1million nodes under a single folder. It would be nice to be able to load more items on demand using either ranges (e.g. give me items 101 to 200) and/or using continuation tokens to get the next page when needed.

@sandy081
Copy link
Member Author

@alvarorahul It is paging support and mentioned in the list in the description. There are no plans as of now.

sandy081 added a commit that referenced this issue Jun 1, 2017
@sandy081
Copy link
Member Author

sandy081 commented Jun 1, 2017

Closing this and further improvements will be tracked under #27823

@sandy081 sandy081 closed this as completed Jun 1, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api plan-item VS Code - planned item for upcoming tree-views Extension tree view issues
Projects
None yet
Development

No branches or pull requests

8 participants