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

Add hover state to Tree items display #88530

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions doc/classes/Tree.xml
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,12 @@
<theme_item name="font_disabled_color" data_type="color" type="Color" default="Color(0.875, 0.875, 0.875, 0.5)">
Text [Color] for a [constant TreeItem.CELL_MODE_CHECK] mode cell when it's non-editable (see [method TreeItem.set_editable]).
</theme_item>
<theme_item name="font_hovered_color" data_type="color" type="Color" default="Color(0.95, 0.95, 0.95, 1)">
Text [Color] used when the item is hovered.
</theme_item>
<theme_item name="font_hovered_dimmed_color" data_type="color" type="Color" default="Color(0.875, 0.875, 0.875, 1)">
Text [Color] used when the item is hovered, while a button of the same item is hovered as the same time.
</theme_item>
Comment on lines +515 to +517
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

<theme_item name="font_outline_color" data_type="color" type="Color" default="Color(0, 0, 0, 1)">
The tint of text outline of the item.
</theme_item>
Expand Down Expand Up @@ -645,6 +651,9 @@
<theme_item name="updown" data_type="icon" type="Texture2D">
The updown arrow icon to display for the [constant TreeItem.CELL_MODE_RANGE] mode cell.
</theme_item>
<theme_item name="button_hover" data_type="style" type="StyleBox">
[StyleBox] used when a button in the tree is hovered.
</theme_item>
<theme_item name="button_pressed" data_type="style" type="StyleBox">
[StyleBox] used when a button in the tree is pressed.
</theme_item>
Expand All @@ -666,6 +675,12 @@
<theme_item name="focus" data_type="style" type="StyleBox">
The focused style for the [Tree], drawn on top of everything.
</theme_item>
<theme_item name="hovered" data_type="style" type="StyleBox">
[StyleBox] for the item being hovered.
</theme_item>
<theme_item name="hovered_dimmed" data_type="style" type="StyleBox">
[StyleBox] for the item being hovered, while a button of the same item is hovered as the same time.
</theme_item>
Comment on lines +681 to +683
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto.

<theme_item name="panel" data_type="style" type="StyleBox">
The background style for the [Tree].
</theme_item>
Expand Down
2 changes: 1 addition & 1 deletion editor/project_manager/project_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void ProjectListItemControl::_notification(int p_what) {
draw_style_box(get_theme_stylebox(SNAME("selected"), SNAME("Tree")), Rect2(Point2(), get_size()));
}
if (is_hovering) {
draw_style_box(get_theme_stylebox(SNAME("hover"), SNAME("Tree")), Rect2(Point2(), get_size()));
draw_style_box(get_theme_stylebox(SNAME("hovered"), SNAME("Tree")), Rect2(Point2(), get_size()));
}

draw_line(Point2(0, get_size().y + 1), Point2(get_size().x, get_size().y + 1), get_theme_color(SNAME("guide_color"), SNAME("Tree")));
Expand Down
10 changes: 9 additions & 1 deletion editor/themes/editor_theme_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,8 @@ void EditorThemeManager::_populate_standard_styles(const Ref<EditorTheme> &p_the

p_theme->set_color("custom_button_font_highlight", "Tree", p_config.font_hover_color);
p_theme->set_color(SceneStringName(font_color), "Tree", p_config.font_color);
p_theme->set_color("font_hovered_color", "Tree", p_config.mono_color);
p_theme->set_color("font_hovered_dimmed_color", "Tree", p_config.font_color);
p_theme->set_color("font_selected_color", "Tree", p_config.mono_color);
p_theme->set_color("font_disabled_color", "Tree", p_config.font_disabled_color);
p_theme->set_color("font_outline_color", "Tree", p_config.font_outline_color);
Expand Down Expand Up @@ -997,7 +999,13 @@ void EditorThemeManager::_populate_standard_styles(const Ref<EditorTheme> &p_the
Ref<StyleBoxFlat> style_tree_hover = p_config.base_style->duplicate();
style_tree_hover->set_bg_color(p_config.highlight_color * Color(1, 1, 1, 0.4));
style_tree_hover->set_border_width_all(0);
p_theme->set_stylebox("hover", "Tree", style_tree_hover);
p_theme->set_stylebox("hovered", "Tree", style_tree_hover);
p_theme->set_stylebox("button_hover", "Tree", style_tree_hover);

Ref<StyleBoxFlat> style_tree_hover_dimmed = p_config.base_style->duplicate();
style_tree_hover_dimmed->set_bg_color(p_config.highlight_color * Color(1, 1, 1, 0.2));
style_tree_hover_dimmed->set_border_width_all(0);
p_theme->set_stylebox("hovered_dimmed", "Tree", style_tree_hover_dimmed);

p_theme->set_stylebox("selected_focus", "Tree", style_tree_focus);
p_theme->set_stylebox("selected", "Tree", style_tree_selected);
Expand Down
Loading
Loading