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 Tree Compression #10713

Merged
merged 2 commits into from
Feb 10, 2022
Merged

Implement Tree Compression #10713

merged 2 commits into from
Feb 10, 2022

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Feb 4, 2022

What it does

Closes #4783
Closes #10268

This implements compact nodes for TreeNodes with a single child that is also a CompositeTreeNode and uses those services in the NavigatorTree. It is based mostly on #10268.

I also refactored the createTreeContainer method to reduce the amount of boilerplate you have to write to set up a custom tree. It's not really necessary, but it does both make it clearer to callers what they get in their container (interfaces define the full service set) and make it easier to override those services.

How to test

  1. This commit includes a demo workspace under api-samples that has various structures of interest to compression. Open Theia or open that workspace directly.
  2. Test various behaviors with compressed nodes:
  1. Test changing the explorer.compactFolders preference and observe that the tree compresses and uncompresses as expected.

Review checklist

Reminder for reviewers

Sorry, something went wrong.

@colin-grant-work colin-grant-work added navigator issues related to the navigator/explorer tree issues related to the tree (ex: tree widget) labels Feb 4, 2022
@msujew msujew self-requested a review February 4, 2022 22:24
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

That looks really good! The changes to the tree container construction are done quite well.

  • Opening/closing collapsed tree nodes works as expected
  • Keyboard navigation for the explorer works just like in VSCode
  • Feedback to changing the compact preference is applied instantly

I have a minor suggestion to improve the usability of this feature.

Sorry, something went wrong.

 - Implements generic tree services for trees that compress multiple nodes into a row
 - Converts the FileTree and NavigatorTree to use those services
 - Refactors the `createTreeContainer` function to reduce boilerplate when creating custom trees

Co-authored by: Colin Grant <colin.grant@ericsson.com>
Co-authored by: Esther Perelman <esther.perelman@sap.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I verified the functionality and confirmed that:

  • the behavior is consistent with vscode
  • the preference exploer.compactFolders works and is aligned with vscode
  • performing a search in the explorer works (even better than vscode)
  • creating folders simultaneously with the dialog works (folders are created compactly)
  • creating a file at a compact folder updates accordingly
  • expanding and collapsing folders works well - even collapse all
  • navigation of collapsed folders works well with the arrow keys

Sorry, something went wrong.

@colin-grant-work colin-grant-work merged commit 4f5b0b0 into master Feb 10, 2022
@colin-grant-work colin-grant-work deleted the cjg/tree-compression branch February 10, 2022 16:46
@github-actions github-actions bot added this to the 1.23.0 milestone Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
navigator issues related to the navigator/explorer tree issues related to the tree (ex: tree widget)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[explorer] flatten/compress folders in the explorer tree when possible
3 participants