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

[scm] show changes as a tree #4835

Closed
akosyakov opened this issue Apr 8, 2019 · 13 comments · Fixed by #7505
Closed

[scm] show changes as a tree #4835

akosyakov opened this issue Apr 8, 2019 · 13 comments · Fixed by #7505
Assignees
Labels
enhancement issues that are enhancements to current functionality - nice to haves git issues related to git scm issues related to the source control manager ui/ux issues related to user interface / user experience

Comments

@akosyakov
Copy link
Member

For big PRs i would like to see changes grouped by folders to understand the scope of PR easier and review one folder after another. It could be an optional mode to the SCM view.

@akosyakov akosyakov added enhancement issues that are enhancements to current functionality - nice to haves git issues related to git proposal feature proposals (potential future features) ui/ux issues related to user interface / user experience scm issues related to the source control manager labels Apr 8, 2019
@vince-fugnitto
Copy link
Member

The idea is to do something similar to the POCs for the same feature request in VSCode?

@akosyakov
Copy link
Member Author

Yes, scm should be rewritten as the tree widget and support hierarchical representation.

@vince-fugnitto
Copy link
Member

@akosyakov it looks like VSCode has recently implemented it.
https://code.visualstudio.com/updates/v1_39#_updated-source-control-view

@akosyakov
Copy link
Member Author

@vince-fugnitto good for them, some of us should rewrite SCM widget as a tree as well

@akosyakov akosyakov added help wanted issues meant to be picked up, require help and removed proposal feature proposals (potential future features) labels Oct 14, 2019
@akosyakov akosyakov changed the title [git] show changes as a tree [scm] show changes as a tree Oct 29, 2019
@akosyakov
Copy link
Member Author

It would be nice if the same is done to the git diff view, i.e. one should be able to switch between tree and flat modes and search files.

@westbury westbury self-assigned this Mar 23, 2020
@westbury
Copy link
Contributor

We have a problem where a large number of files (in the thousands) appear in the changes and this locks up the application. This is caused when stuff goes missing from .gitignore. Switching to tree would solve this, even if the model returned everything in a flat structure as we get react virtualized.

I have started looking into this. The first step is to split tree-widget into a Widget and a React Component. Embedding Phospor Widgets in React is not, IMO, a good idea. So I want to be able to embed a Component. This may also benefit the debug view. I'm in the process of cleaning up and testing that split. Essentially the search and key bindings are part of the Widget. Most of the rest is in the React Component. Once this is done I expect it to be straight forward to use in the Source Control view.

@akosyakov
Copy link
Member Author

akosyakov commented Mar 23, 2020

@westbury we were planning to work on it :( not really matter, if you do it, it helps

I don't think there is a need to refactors trees, but embed a tree widget inside the scm widget. It was already done for view containers, i.e. the scm widget can be rewritten with pure phosphor without react. Avoiding tree refactoring is a good idea since it is used a lot and has to be stable.

@westbury
Copy link
Contributor

embed a tree widget inside the scm widget

@akosyakov That was the approach I looked at on Friday. However I was concerned that will be a less flexible solution. Presumably you are expecting the rendering of the stuff around the trees, such as the commit message box, to be done inside a widget. It is going to be tricky to get the widget sizes to react correctly. In the view containers and the debug view the nested widget layout does not react to the content size.

I'm aware of the core role that the tree widget plays and the extensive testing that will be needed. I think it will be worth it but if you are not willing to do that, and if you think we can get the phosphor widgets to react to content size relatively easily, then I can go back to plan A and embed the tree widget.

@akosyakov
Copy link
Member Author

In the view containers and the debug view the nested widget layout does not react to the content size.

I'm pretty sure they do. It is not about the debug view, we use view containers everywhere now starting from the explore. The view container is using the panel layout which can properly dispatch resize events to child view container parts, each part then dispatch resize event to a concrete widget, each widget then implements onResize widget callback.

@westbury
Copy link
Contributor

the scm widget can be rewritten with pure phosphor without react

Yes, that is going to work. I'm working on it today (Wednesday).

@westbury
Copy link
Contributor

westbury commented Apr 1, 2020

Work so far is available here https://github.com/westbury/theia/tree/scm-tree

It mostly works. Work left includes fixing key bindings and saving of state (including view mode selection and expansion), and search. It does not cover the git diff view. I won't be able to get back to this until next week. If anyone has ideas about the two icons (tree and flat) let me know, otherwise I can ask our team.

@akosyakov
Copy link
Member Author

@westbury We have octicons support from Monaco, you can use them. @RomanNikitenko Could you give a hint?

I won't be able to get back to this until next week.

You can open a draft PR?

@RomanNikitenko
Copy link
Contributor

@akosyakov @westbury
About codicons https://code.visualstudio.com/api/references/icons-in-labels

I think it depends where it will be used.
For example:

  • I tried just add $(link) to a label for QuickOpenItem - It's rendered automatically
  • to reuse VS Code icons for workspace symbols I applied the corresponding style
  • for Configure Task Action example

@akosyakov akosyakov removed the help wanted issues meant to be picked up, require help label Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement issues that are enhancements to current functionality - nice to haves git issues related to git scm issues related to the source control manager ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants