Add hover state to Tree items display

[davthedev]

Implements godotengine/godot-proposals#7160

Since this is a bit less trivial than other hover additions, here are some explanations.

This change aims at improving the usability of the scene tree and all places where a Tree control is used. Moving the mouse above a tree item highlights it.

The highlight is also rendered on individual hovered icon buttons. In this case, the main row or cell it belongs to keeps its highlight in a "dimmed" version. This helps focusing on the icon while keeping the visual relationship with the piece of data the action is likely to impact.

Example with a slightly customized theme to illustrate a dense list:

Detail on the "dimmed hover" effect:

To make it more obvious, here it is with customized colors:

Projects can take advantage of the new styling possibilities. All new visual elements are customizable using appropriate theme resources.

Editor and default themes are adjusted to provide the additional theme resources. Other existing stuff is left untouched in this regard, so that there are no regressions on existing projects.

The Tree control is a complex widget, handling many possibilities of display. The display and original click handling algorithm is based on recursive traversal.

I implemented a direct, lighter way to find out which item is hovered that exactly matches, or at best attempt, the algorithm used to determine the clicked item, because mouse movemens happen more often.

This PR is exclusively adding support for hover feedback. It does not fix a few inconsistencies I found in click coordinates resolution, especially for the buttons attached to a cell (think the script, show/hide, etc... you see in the scene tree) with some combination of spacings. Those may be addressed in separate PRs. The goal here is that the hovered zones match the currently clickable zones.

A good part of the complexity is that there is an algorithm to move those icons towards the left in comparison to their original position in the cell if there is horizontal scrolling. This is what keeps the icon buttons visible in the scene tree at all times even if there is a long node name that overflows. The logic to compute that offset is a little complex and I had to replicate it. The said algorithm is not implemented in right-to-left display and some of the public methods that return the position of the in-cell buttons do not apply it - matter for separate issue reports.

[Mickeon]

Unsure of what this is referring to, when looking at the video preview and description. Calling the property itself "dimmed" is not a good idea. The adjective refers to the custom color used, and as such it may not always be dimmed.

[davthedev]

When you hover the action icons, the row still has a hover highlight but different than when you hover the "main" part of the row, as I explained above.

Observe the animation closely and you will spot the difference, when the mouse moves between the main part of the row and the action icons on an unselected row. The row highlight is still here, with less contrast in both the selection box and the text color (hence the two properties you mention). Of course, the user is free to set anything for those and they can be rendered using a completely different color; not just a dimmed aspect.

This allows the icons hover to be visible even if the Tree is in full row highlight mode.

I had to choose a word. Would you have a different suggestion? Ideas: hovered_secondary

[Mickeon]

The current color should've been called font_hovered_highlight_color and this one should be font_hovered_color but that obviously cannot be done, unless we're doing some very messy compatibility work, hmm.... That's a pretty difficult question, actually.

• Anything with "focus" is out of the question as the concept applies to something else outright for Controls.
• Adjectives like "faint", "low" are not that good for the same reasons as "dimmed"

If I have a saying on this, I would call it based on the very thing it is used for, no vague adjectives: font_hovered_on_button_color. Something like this.

[davthedev]

"Hovered on button" may lead to confusion, because there is no mechanism to customize the buttons hover color yet.

The naming question should be taken globally for the font (font_hovered_dimmed_color) and the background (hovered_dimmed) at the same time. If we attack problems one by one without seeing the bigger picture, we lose consistency.

[Calinou]

I think dimmed is fine here (muted would work as a synonym, which is used in Material UI guidelines IIRC). It's hard to convey with a single word what it's exactly supposed to do.

[Mickeon]

Ditto.

[Calinou]

Tested locally, it works as expected. This makes Tree-based controls a lot nicer to use, great work 🙂

Black (OLED) theme

simplescreenrecorder-2024-03-02_20.49.48.mp4

Light theme

simplescreenrecorder-2024-03-02_20.51.01.mp4

I've ensured that continuous redrawing doesn't occur if the mouse stands still on a TreeItem by looking at the update spinner.

Code looks good to me at a glance, aside of the style suggestions.

[KoBeWi]

Suggested change

	return -1; //not found
	return -1; // Not found.

[KoBeWi]

Code and the functionality itself seems fine, though I wonder if some parts can't be simplified (like the button hover code; since buttons are clickable, it should be detected elsewhere, no?)

[davthedev]

This complex control has to be refactored in order to fix some positioning quirks. This waits for a more important upgrade where it is admissible to potentially break some themes.

For the changes covered in this PR, I reproduced in the hover code the way side buttons are positioned including the quirks. For example, you can see those giggling sideways if you scroll the inspector horizontally when names are longer. The original click detection is a recursive tree traversal; for the hover I found a simpler way without having to hook up to this recursivity. That is less execution complexity as well for an event that occurs more often than clicks. As some refactoring is planned for later, I opted of the simpler solution.

[akien-mga]

CC @passivestar as I believe you had Opinions™ on this type of hover/select UX ;)

[passivestar]

CC @passivestar as I believe you had Opinions™ on this type of hover/select UX ;)

I like it, I think godot needs it. I actually wanted to work on this myself before I discovered this PR

I think I found a bug where it sometimes won't unhover when the mouse leaves the dock:

Screen.Recording.mp4

[AThousandShips]

Suggested change

	// Find eventual hovered button in cell.
	// Find possible hovered button in cell.

Eventual = sooner or later

[davthedev]

Also noted another couple bugs I should fix:

• If the content is refreshed while hovering, the highlight is lost until the next mouse move.
  This happens on the "Distant" nodes tree while a scene is running.

• Hover highlight is also visible during a drag-drop operation and can be misleading at that moment, as the one thing that should be highlighted during this operation is the drop target.

[davthedev]

Just fixed the two bugs above. Changed the mouse over (or not) tracking logic to tentatively fix @passivestar hover feedback bug, relying on MOUSE_ENTER / _EXIT notifications. As I was not able to reproduce on my side, can you have a look?

[passivestar]

As I was not able to reproduce on my side, can you have a look?

The bug is gone 🥳

I noticed two other things though:

1. Hovering in the project manager is broken now? It uses a bright thin outline instead of whatever is was before

1.mp4

2. When hovering in a tree that has more than one main non-button column it highlights the right column, which doesn't always make sense semantically. In the shortcuts editor for example those buttons are related to the entire row, not just the rightmost column. When using trees in games it might not always make sense either

2.mp4

Perhaps when a tree has more than 2 main columns and you're hovering over a button it should highlight ALL of the columns or NONE of them, instead of the right one?

[davthedev]

I noticed two other things though:

1. Hovering in the project manager is broken now? It uses a bright thin outline instead of whatever is was before

Looks like a broken editor theme. The thin grey outline is the default StyleBox when nothing is specified in the system theme; I will have a look. I rebased my month-old work with lots of commits in between, probable collateral damage...

2. When hovering in a tree that has more than one main non-button column it highlights the right column, which doesn't always make sense semantically. In the shortcuts editor for example those buttons are related to the entire row, not just the rightmost column. When using trees in games it might not always make sense either

Because there are multiple possible select modes in the Tree control that can be set on each instance: entire row or single cell. The view you are referring to is in single-cell mode, unlike the node tree. It works like a spreadsheet editor. I think it is up to this specific screen to work its way around instead by configuring the Tree control instance correctly.

Here, the cell is the actual binding, which you can edit using the dedicated buttons. Makes sense IMHO.

If some actions need to relate to the whole row, while keeping the separate cell selection possible, there are multiple options:

• Configure this screen in single-row selection mode - check UX as, for now, double-clicking the left cell unfolds the tree, and double-clicking the right cell opens the binding edit dialog.
• Make a separate third column for the buttons
• Add a setting to the Tree control mandating to highlight the whole row on specific cells / columns or buttons. Or a "select single cell but highlight whole row on buttons" selection mode

[passivestar]

I guess my main concern is not the shortcut window but the fact that the last column will always be highlighted in this mode whenever somebody makes a game with a spreadsheet in it. Kind of random? It seems like checking if there's more than one column and not highlighting any column (only highlighting the button) in this mode would be a more sensible default imo

[Calinou]

@davthedev Could you look into rebasing this PR on top of master? I'd like to give this a final spin so we can look into merging this for 4.4.

[davthedev]

Rebasing done, but wait for my green light before merging. I must double check that previously mentioned side effects are resolved. The one with the project chooser window is kind of critical.

[Calinou]

Rebasing done, but wait for my green light before merging. I must double check that previously mentioned side effects are resolved. The one with the project chooser window is kind of critical.

The theming issue in the project list is still present, I just tested:

The rest seems to work fine.

[davthedev]

Theme issue in the project selector should be fixed now. Good to merge!

[Calinou]

Tested again, it works great now, including in the project manager 🙂

[Repiteo]

Thanks!