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

tree indent guidelines support #8298

Merged
merged 1 commit into from
Aug 5, 2020
Merged

Conversation

akosyakov
Copy link
Member

What it does

  • Fixes [ui/ux][tree] Help the tree indentation with vertical lines, mimic VS Code #6586:
    • Add preference to set 'workbench.tree.renderIndentGuides' to 'onHover' (default), 'none' or 'always'
    • When nodes are selected, indent guidelines are rendered for all the sibling nodes
    • When parent node is selected, indent guidelines are rendered for all the child nodes
    • When hovering over a tree, indent guidelines are displayed for all expanded nodes
    • Selecting multiple nodes works in a similar fashion

How to test

  • Set the workspace preference to 'onHover', 'none', 'always' and the rendering of the indent guidelines should change accordingly.
  • In different locations in Theia where the tree is being used (for example, in navigator, search in workspace, outline, problems-view and scm (list mode and tree mode)), select one/more nodes to test if the indent guidelines are rendered correctly.

Review checklist

Reminder for reviewers

@akosyakov akosyakov added the tree issues related to the tree (ex: tree widget) label Aug 4, 2020
@akosyakov akosyakov force-pushed the ak/tree_indent_guidelines branch from cea71eb to 04be1dd Compare August 4, 2020 09:54
@akosyakov akosyakov changed the title fix #6586: tree indent guidelins tree indent guidelines support Aug 4, 2020
Co-authored-By: Kaiyue Pan <kaiyue.pan@ericsson.com>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the ak/tree_indent_guidelines branch from 04be1dd to 8903412 Compare August 4, 2020 11:05
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.

The changes work well, I verified with multiple trees in the application 👍
The only minor issue is the styling of the indent guideline is bit off given the tree (the indent guideline in respect to the collapse icon):

Explorer:

nav

Problems:

problems

Search:

search

I do think this is a minor issue however and something we can fix in the future if necessary.

@akosyakov
Copy link
Member Author

Search looks off indeed, but other don't much different compare to VS Code.

@akosyakov
Copy link
Member Author

For search we can remove custom styling of the expand toggle to align with other trees then it looks like:
Screenshot 2020-08-04 at 14 33 15

Should we? For comparison in VS Code:
Screenshot 2020-08-04 at 14 34 12

@vince-fugnitto
Copy link
Member

@akosyakov I don't think it's necessary, at least not for the moment (should not block the pull-request).
The issue is that some trees define their own custom styling which is different than the base tree which is what @kaiyue0329 was trying to fix with adding new props, but it makes the changes complex. I'd be fine with keeping the approach simple and if we ever update we can do so later on.

@akosyakov
Copy link
Member Author

akosyakov commented Aug 4, 2020

@vince-fugnitto I would rather looking into normalising such trees with defaults and removing custom styling. It seems to be also only one case. I've checked other trees like scm, problems, outlines and could not understand why we need to change their styling. They looked aligned with the default implementation without changes.

@vince-fugnitto
Copy link
Member

@vince-fugnitto I would rather looking into normalising such trees with defaults and removing custom styling.

@akosyakov I agree as well, it'd be best if all trees were normalized this way and their behavior was consistent.

@akosyakov
Copy link
Member Author

merging tomorrow morning if no objections

Copy link
Contributor

@Anasshahidd21 Anasshahidd21 left a comment

Choose a reason for hiding this comment

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

The changes look pretty good to me. Tested on multiple views such as navigator, scm, problems, etc,

@akosyakov akosyakov merged commit 6086b28 into master Aug 5, 2020
@akosyakov akosyakov deleted the ak/tree_indent_guidelines branch August 5, 2020 05:08
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui/ux][tree] Help the tree indentation with vertical lines, mimic VS Code
3 participants