Skip to content

Tree item folding improvements #33984

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

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

FeatherAntennae
Copy link

@FeatherAntennae FeatherAntennae commented Nov 29, 2019

What is this / TLWR

A few small changes to the Tree and TreeItem classes along with a not so small refactor of all their in-editor implementation.

The goal is to reduce clutter in many classes that each have their own (different) implementation of the same logic, while giving new and old behaviors of the UI trees better consistency across the editor.

Intro

Once upon a time, in an editor near you,
each UI trees were their own.

They all had different implementations of the same functionalities and sometimes completly different behaviour. Some had more functionalities than the others and their handling were are all different.

Users were feeling defeated and engine devs were getting depressions from the Spaghetti Oriented Programing that came from years of awesome contributors trying very hard to make everyone happy by adding small requested features in specific parts of the editor while also working on the more important parts of the core engine.

But all hope is not lost! The community is constantly growing and now, a bored software engineering student and part time UI/UX designer decided to learn C++ to help the devs growing the engine he loves into an healthy and powerful tool for all of your game making needs.

But how?

By making a few small changes to the Tree and TreeItem classes along with a not so small refactor of all their in editor implementation.

Goal

Reducing clutter in many classes that each have their own (different) implementation of the same logic while giving new and old behaviors of the UI trees a good dose of consistency across the editor.

This will also make the implementation of #31815 cleaner and easier.

What's left to do

This is still a WIP as I plan to refactor the folding logic in every corners of the editor to use the new available methods to reduce clutter and have consistent functionalities across the editor.

I also want to write documentation for the new methods, give more appropriate icons to the context menu option and of course would like some feedback on the changes.

Note

I made it a bit easier to change the scene tree dock's context menu's order, so don't hesitate to suggest different placement of items if you think this one is too different or bad, I tried to keep things relatively where they were while reducing the amount of groups and allowing for the tree folding utilities to be added.

I also made it so the comportment of the alt+click functionalities extremely easy to tweak if needed so the current behavior is open to discussion.

TODO

I'm leaving this here as a small reminder of what's left to do in a slightly more exhaustive way. Will change as the status of the PR changes.

Code

Tree / TreeItem methods

Implement advanced tree expending/folding logic directly into TreeItem and adds some helper methods to Tree and TreeItem.

  • TreeItem::set_collapsed_recursive(...)
  • TreeItem::toggle_collapsed_all_descendants()
  • TreeItem::is_active()
  • Tree::set_collapsed_all(...)

Tree Behaviour

Add editor wide support for ALT+Clicking the folding arrows of Tree controls for recursive expansion and collapse of descendants.

  • Add functionality when handling user input.

Refactoring

Remove Tree and TreeItem collapse/expand logic and custom tree behaviour from all independent components of the editor as it should be handled by the TreeItem and Tree classes directly.

  • Scene Tree Dock
  • File System Dock
  • Inspector Dock
  • Find other places where it happens!

UI/UX

Add functionality and common behaviour to all UI trees to keep user interaction consistent throughout the editor.

  • Icons for the context menus
  • Scene Tree Dock -> add expand/collapse utilities to context menu
  • Scene Tree Dock -> reorder context menu elements to keep things in a related order and reduce the amount of lost space caused by abundant separator.
  • File System Dock context menu
  • Node Dock context menu
  • Import Dock? -> Allow collapsing sections to act like other similar UI
  • Search Help context menu
  • Manage Editor features context menu
  • Editor Settings context menu
  • Project Settings context menu
  • Tools -> "Orphan Resource Explorer" context menu
  • Find other places where it's needed!

Documentation

Update the class reference for every new methods and add a small section to the doc to expose some of the more hidden behaviour.

  • TreeItem::set_collapsed_recursive(...)
  • TreeItem::toggle_collapsed_all_descendants()
  • TreeItem::is_active()
  • Tree::set_collapsed_all(...)
  • Editor manual? -> Navigation and Default behaviours? -> UI Trees -> ALT+Click fold arrow

Miscellaneous

Other things not necessarily related to the code, including things related to the pull request itself and this post.

  • GIF of the alt+click folding functionality alone
  • GIF of the normal folding functionality alone
  • GIF of the expand / collapse all functionality
  • GIF of the collapse unselected functionality
  • GIF of the expand / collapse descendants functionality
  • Before/After picture comparison of the Scene Tree Dock context menu (one item selected, one external item selected, multiple item selected)

Preview

Here you can see the context menu, some folding functions, the normal arrow folding and the alt+click arrow folding for the scene tree. The alt+click logic applies to all Tree controls in the editor. I forgot to show it, but the Expand/Collapse Descendant function of the context menu does the same thing as the alt+click on the arrow.

Sceme Tree Folding Logic

The new context menu layout for easier analysis, feedback, insults, kudos, etc.
image

is_active() returns true if the TreeItem or at least one of its descendant
is selected.
…ile also allowing to

start with the node's children and/or to skip active items if wanted.
…ode recursively.

This method is called when a user clicks on the folding arrow while holding
"ALT" and is handled in the Tree directly to ensure consistent behaviour
throughout the editor while also allowing the removal of a lot of duplicated
and inconsistent logic in multiple area of the editor.
Added shortcuts and tools to improve the tree folding capacities using the
new methods exposed by Tree and TreeItem.
Split the context menu's generation code into commented sections to make it
easier to maintain and modify as needed.
Changed order and grouping of items in the context menu to reduce lost space in
the menu and accommodate the tree folding logic.
@ghost
Copy link

ghost commented Nov 29, 2019

Interesting, will check it out soon. Definitely could use a collapse and expand all in many kinds of scenes.

@FeatherAntennae
Copy link
Author

Still working on this, I'll push an update soon, I've been rewriting the filesystem context menu from scratch as I found a few bugs that gave me about a dozen ways to crash the editor.

@FeatherAntennae
Copy link
Author

FeatherAntennae commented Jan 31, 2020

The refactor is pretty big and is going to take a while to complete, but I think I pretty much nailed down the global changes needed to make everything compatible and consistent. I think some of these changes would help some other propositions and PR too.

I think we should keep this PR just for the refactor and I could put it aside for now, complete the Tree and TreeItem documentation, and open two new and smaller PR.

-One PR for the changes to the Tree and TreeItem, including doc for the new methods. It already makes the click, alt-click and folding consistent to all trees by default.

-One PR for the scene tree menu overhaul, as it's complete and can serve as a solid base for the other menus I'm working on. It would also facilitate feedback on that part in itself.

I would then come back to work on this refactor while everyone can take advantages of the other changes and also give proper feedback on them specifically.

What do you guys think?

@aaronfranke
Copy link
Member

@FeatherAntennae Is this still desired? If so, it needs to be rebased on the latest master branch.

@FeatherAntennae
Copy link
Author

@aaronfranke still desired, but on hold as I wait for #36926 which has been split from this PR. It made it simpler to get feedback on this part of the PR and makes it possible for it to get merged before the complete refactor is done.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 16, 2020

Note that the scene tree node folding is already implemented in #19936 (comment), but would be nice to make it work across other editors of course.

Also note that somehow, recursive folding works as seen in #28626 with Alt + left/right arrows, which was surprising for me to find out. 👀

@Xrayez
Copy link
Contributor

Xrayez commented Jul 16, 2020

Some people wouldn't like the sheer number of menu items just for node folding, but I think it's less of an issue if we eventually decrease the main font size, like in #32061.

@FeatherAntennae
Copy link
Author

Note that the scene tree node folding is already implemented in #19936 (comment), but would be nice to make it work across other editors of course.

Also note that somehow, recursive folding works as seen in #28626 with Alt + left/right arrows, which was surprising for me to find out. 👀

Yes, it's one of the reason I started this, I used it a lot, but was missing it elsewhere!
I re-implemented it directly in the TreeItem class, using the new "call_recursive()" function that was available there in #36926
This way, it works by default in every Tree. If it gets merged, this PR here will be split in two, first will be to remove any manual handling in specific parts of the editor, and second will be to add the context menu elements in a single way across the editor, as right now they are either non-existent, or handled a few different ways!

I also added a bit of functionality to allow ignoring the active branch of the Tree if we want to keep it open.

The alt + arrow thing I didn't notice I think, I'll see to make sure it keeps working too!

Some people wouldn't like the sheer number of menu items just for node folding, but I think it's less of an issue if we eventually decrease the main font size, like in #32061.

I agree that it is taking a lot of space, this is actually one of the UI thing that doesn't have a real 100% satisfying solution to my knowledge.

I think people who use these all the time (like me) likes having them on hand and spread out in a way where they're fast to reach, but everyone else will see them as in the way. Putting them in a sub menu makes them slow to reach for people using them a lot, but can be a good middle ground as it would use just a single entry in the main context menu.

This is exactly why the context menu was posted here though, I wanna see if people think it's too much.

@akien-mga akien-mga modified the milestones: 4.0, 4.x Jul 26, 2022
@AThousandShips
Copy link
Member

How relevant is this now? Recursive folding has been added to Tree for example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants