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

Show "Collapse All" command in tree view toolbar. #12514

Merged
merged 2 commits into from
May 23, 2023

Conversation

tsmaeder
Copy link
Contributor

@tsmaeder tsmaeder commented May 10, 2023

Contributed on behalf of STMicroelectronics

What it does

Closes #12513.

Adds a "Collapse All" toolbar item in tree views contributed by plugins if the plugin requests it in the options (showCollapseAll). This PR introduces the concept of DynamicWidget. It allows to update the toolbar after the contents of a view widget has been create lazily.

How to test

Use the procedure from the related issue and make sure the icon appears. You can also use the built-in "NPM SCRIPTS" view as a second test case.

Review checklist

Reminder for reviewers

@tsmaeder tsmaeder mentioned this pull request May 11, 2023
11 tasks
@vince-fugnitto vince-fugnitto added tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility labels May 15, 2023
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Hi @tsmaeder,

Thank you very much for that change! I tested it with a single-root and a multi-root tree and both times the behavior was as expected. The flag is also correctly considered (i.e., if missing of false the button is not shown) so everything is working very nicely.

I do have some minor comments from which at least the command label one should be addressed. The others are more general thoughts and questions.

@tsmaeder
Copy link
Contributor Author

Hmh...I noticed that the shown actions are not correct when starting up with the view open in a view container with only one view. This is the case where the ToolbarAwareTabBar is showing the toolbar instead of the ViewContainerPart. The problem is that widget responsible for contributing to the toolbar is created lazily and we're not re-updating the toolbar at the right time in this case. I've tried to find a solution, but to no avail, so far.

tsmaeder added 2 commits May 22, 2023 14:57
…#12513.

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@tsmaeder tsmaeder force-pushed the 12513_collapse_all branch from fa7c60d to e8ed7dd Compare May 22, 2023 13:48
@tsmaeder
Copy link
Contributor Author

@martin-fleck-at I addressed the comments and fixed the behavior when a single view is present in a view container: you can test this by opening the "Test View Drag and Drop" and then refreshing the browser.

Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

I re-tested the code and everything seems to be working now fine. Thank you @tsmaeder!

@tsmaeder tsmaeder merged commit e832ef7 into eclipse-theia:master May 23, 2023
tsmaeder added a commit to tsmaeder/theia that referenced this pull request May 23, 2023
Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@vince-fugnitto vince-fugnitto added this to the 1.38.0 milestone May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TreeViewOptions.showCollapseAll has no effect
3 participants