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

Make tree widget indentation configurable #13179

Merged

Conversation

AlexandraBuzila
Copy link
Contributor

@AlexandraBuzila AlexandraBuzila commented Dec 15, 2023

What it does

Make the indentation of the tree widget configurable via preference.

Contributed on behalf of STMicroelectronics

How to test

  1. Open the settings view from File → Preferences → Settings
  2. Search for workbench.tree.indent
  3. Change the Workbench › Tree: Indent preference and notice how the indentation of the tree widget, e.g. in the workspace navigator view, changes accordingly

Review checklist

Reminder for reviewers

@AlexandraBuzila AlexandraBuzila force-pushed the configure-tree-indentation branch 2 times, most recently from cb08a95 to 4f07454 Compare December 18, 2023 08:13
@JonasHelming JonasHelming requested a review from msujew December 18, 2023 15:30
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.

I don't think this change is working as expected/as it should.

  1. Indent pinning doesn't work, see below.
  2. Increasing the indent also increases the indent of the root tree items. This isn't the case in vscode. In vscode, the root tree items always stay in the same spot. This indicates that we still need the leftPadding property that tells us how much the root tree items are padded, while adding a indentPadding that tells us how much each indented tree item needs to be padded.

By fixing 2. you should also be able to fix 1. as a drive by.

@AlexandraBuzila AlexandraBuzila force-pushed the configure-tree-indentation branch from 4f07454 to 38ae7d5 Compare January 11, 2024 09:28
@AlexandraBuzila
Copy link
Contributor Author

Thank you for the review, @msujew

I updated the code and restored the leftPadding prop: it now controls the padding for the tree root, while the new preference only affects the indentation of the nodes below it.

@AlexandraBuzila AlexandraBuzila force-pushed the configure-tree-indentation branch from 38ae7d5 to fe913b7 Compare January 16, 2024 10:31
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.

Looks mostly good to me, just one more issue see below. This should be good to go afterwards.

packages/core/src/browser/tree/tree-preference.ts Outdated Show resolved Hide resolved
Make the indentation of the tree widget configurable via preference.

Contributed on behalf of STMicroelectronics

Signed-off-by: Alexandra Buzila <abuzila@eclipsesource.com>
@AlexandraBuzila AlexandraBuzila force-pushed the configure-tree-indentation branch from fe913b7 to 588fedb Compare January 16, 2024 15:56
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.

Alright, looks good to me 👍

@msujew msujew merged commit 6a84444 into eclipse-theia:master Jan 17, 2024
14 checks passed
@jfaltermeier jfaltermeier added this to the 1.46.0 milestone Jan 25, 2024
@AlexandraBuzila AlexandraBuzila deleted the configure-tree-indentation branch May 29, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants