From 86ea0127a3b6c27878a28ae5e3879055e7d6476f Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Mon, 23 Sep 2024 14:30:07 -0300 Subject: [PATCH] Add a focus border on `ScrollContainer` Also added new unit tests for `Control`. Co-authored-by: ator-dev --- doc/classes/EditorInspector.xml | 2 + doc/classes/ScrollContainer.xml | 6 +++ editor/editor_inspector.cpp | 3 ++ editor/editor_properties.cpp | 2 + editor/themes/editor_theme_manager.cpp | 10 ++++- scene/gui/scroll_container.cpp | 37 +++++++++++++++++++ scene/gui/scroll_container.h | 8 ++++ scene/theme/default_theme.cpp | 10 ++++- tests/scene/test_control.h | 51 ++++++++++++++++++++++++++ 9 files changed, 127 insertions(+), 2 deletions(-) diff --git a/doc/classes/EditorInspector.xml b/doc/classes/EditorInspector.xml index 6b25be490ed..0b56060bda7 100644 --- a/doc/classes/EditorInspector.xml +++ b/doc/classes/EditorInspector.xml @@ -28,6 +28,8 @@ + + diff --git a/doc/classes/ScrollContainer.xml b/doc/classes/ScrollContainer.xml index 405bba35643..959721b115d 100644 --- a/doc/classes/ScrollContainer.xml +++ b/doc/classes/ScrollContainer.xml @@ -40,6 +40,9 @@ + + If [code]true[/code], [theme_item focus] is drawn when the ScrollContainer or one of its descendant nodes is focused. + If [code]true[/code], the ScrollContainer will automatically scroll to focused children (including indirect children) to make sure they are fully visible. @@ -107,6 +110,9 @@ + + The focus border [StyleBox] of the [ScrollContainer]. Only used if [member draw_focus_border] is [code]true[/code]. + The background [StyleBox] of the [ScrollContainer]. diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp index 6e837560f6b..91bf3f323af 100644 --- a/editor/editor_inspector.cpp +++ b/editor/editor_inspector.cpp @@ -4662,6 +4662,7 @@ EditorInspector::EditorInspector() { search_box = nullptr; _prop_edited = "property_edited"; set_process(false); + set_focus_mode(FocusMode::FOCUS_ALL); property_focusable = -1; property_clipboard = Variant(); @@ -4680,4 +4681,6 @@ EditorInspector::EditorInspector() { // `use_settings_name_style` is true by default, set the name style accordingly. set_property_name_style(EditorPropertyNameProcessor::get_singleton()->get_settings_style()); + + set_draw_focus_border(true); } diff --git a/editor/editor_properties.cpp b/editor/editor_properties.cpp index 2b2b32eb22d..cae11b1efea 100644 --- a/editor/editor_properties.cpp +++ b/editor/editor_properties.cpp @@ -3281,6 +3281,8 @@ void EditorPropertyResource::update_property() { sub_inspector->set_read_only(is_read_only()); sub_inspector->set_use_folding(is_using_folding()); + sub_inspector->set_draw_focus_border(false); + sub_inspector->set_mouse_filter(MOUSE_FILTER_STOP); add_child(sub_inspector); set_bottom_editor(sub_inspector); diff --git a/editor/themes/editor_theme_manager.cpp b/editor/themes/editor_theme_manager.cpp index a4251bfd290..bb567a80cac 100644 --- a/editor/themes/editor_theme_manager.cpp +++ b/editor/themes/editor_theme_manager.cpp @@ -1822,11 +1822,19 @@ void EditorThemeManager::_populate_editor_styles(const Ref &p_theme // Editor focus. p_theme->set_stylebox("Focus", EditorStringName(EditorStyles), p_config.button_style_focus); - // Use a less opaque color to be less distracting for the 2D and 3D editor viewports. + Ref style_widget_focus_viewport = p_config.button_style_focus->duplicate(); + // Make the focus outline appear to be flush with the buttons it's focusing, so not draw on top of the content. + style_widget_focus_viewport->set_expand_margin_all(2); + // Use a less opaque color to be less distracting for the 2D and 3D editor viewports. style_widget_focus_viewport->set_border_color(p_config.accent_color * Color(1, 1, 1, 0.5)); p_theme->set_stylebox("FocusViewport", EditorStringName(EditorStyles), style_widget_focus_viewport); + Ref style_widget_scroll_container = p_config.button_style_focus->duplicate(); + // Make the focus outline appear to be flush with the buttons it's focusing, so not draw on top of the content. + style_widget_scroll_container->set_expand_margin_all(4); + p_theme->set_stylebox("focus", "ScrollContainer", style_widget_scroll_container); + // This stylebox is used in 3d and 2d viewports (no borders). Ref style_content_panel_vp = p_config.content_panel_style->duplicate(); style_content_panel_vp->set_content_margin_individual(p_config.border_width * 2, p_config.base_margin * EDSCALE, p_config.border_width * 2, p_config.border_width * 2); diff --git a/scene/gui/scroll_container.cpp b/scene/gui/scroll_container.cpp index 312b538a990..f3c50c626c1 100644 --- a/scene/gui/scroll_container.cpp +++ b/scene/gui/scroll_container.cpp @@ -281,6 +281,12 @@ void ScrollContainer::_gui_focus_changed(Control *p_control) { if (follow_focus && is_ancestor_of(p_control)) { ensure_control_visible(p_control); } + if (draw_focus_border) { + const bool _should_draw_focus_border = has_focus() || child_has_focus(); + if (focus_border_is_drawn != _should_draw_focus_border) { + queue_redraw(); + } + } } void ScrollContainer::ensure_control_visible(Control *p_control) { @@ -366,6 +372,15 @@ void ScrollContainer::_notification(int p_what) { case NOTIFICATION_DRAW: { draw_style_box(theme_cache.panel_style, Rect2(Vector2(), get_size())); + if (draw_focus_border && (has_focus() || child_has_focus())) { + RID ci = get_canvas_item(); + RenderingServer::get_singleton()->canvas_item_add_clip_ignore(ci, true); + draw_style_box(theme_cache.focus_style, Rect2(Point2(), get_size())); + RenderingServer::get_singleton()->canvas_item_add_clip_ignore(ci, false); + focus_border_is_drawn = true; + } else { + focus_border_is_drawn = false; + } } break; case NOTIFICATION_INTERNAL_PHYSICS_PROCESS: { @@ -602,10 +617,14 @@ void ScrollContainer::_bind_methods() { ClassDB::bind_method(D_METHOD("get_v_scroll_bar"), &ScrollContainer::get_v_scroll_bar); ClassDB::bind_method(D_METHOD("ensure_control_visible", "control"), &ScrollContainer::ensure_control_visible); + ClassDB::bind_method(D_METHOD("set_draw_focus_border", "draw"), &ScrollContainer::set_draw_focus_border); + ClassDB::bind_method(D_METHOD("get_draw_focus_border"), &ScrollContainer::get_draw_focus_border); + ADD_SIGNAL(MethodInfo("scroll_started")); ADD_SIGNAL(MethodInfo("scroll_ended")); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "follow_focus"), "set_follow_focus", "is_following_focus"); + ADD_PROPERTY(PropertyInfo(Variant::BOOL, "draw_focus_border"), "set_draw_focus_border", "get_draw_focus_border"); ADD_GROUP("Scroll", "scroll_"); ADD_PROPERTY(PropertyInfo(Variant::INT, "scroll_horizontal", PROPERTY_HINT_NONE, "suffix:px"), "set_h_scroll", "get_h_scroll"); @@ -623,10 +642,28 @@ void ScrollContainer::_bind_methods() { BIND_ENUM_CONSTANT(SCROLL_MODE_RESERVE); BIND_THEME_ITEM_CUSTOM(Theme::DATA_TYPE_STYLEBOX, ScrollContainer, panel_style, "panel"); + BIND_THEME_ITEM_CUSTOM(Theme::DATA_TYPE_STYLEBOX, ScrollContainer, focus_style, "focus"); GLOBAL_DEF("gui/common/default_scroll_deadzone", 0); } +void ScrollContainer::set_draw_focus_border(bool p_draw) { + if (draw_focus_border == p_draw) { + return; + } + draw_focus_border = p_draw; + queue_redraw(); +} + +bool ScrollContainer::get_draw_focus_border() { + return draw_focus_border; +} + +bool ScrollContainer::child_has_focus() { + const Control *focus_owner = get_viewport() ? get_viewport()->gui_get_focus_owner() : nullptr; + return focus_owner && is_ancestor_of(focus_owner); +} + ScrollContainer::ScrollContainer() { h_scroll = memnew(HScrollBar); h_scroll->set_name("_h_scroll"); diff --git a/scene/gui/scroll_container.h b/scene/gui/scroll_container.h index afd3c8bd57a..ea6309f57e8 100644 --- a/scene/gui/scroll_container.h +++ b/scene/gui/scroll_container.h @@ -72,6 +72,7 @@ private: struct ThemeCache { Ref panel_style; + Ref focus_style; } theme_cache; void _cancel_drag(); @@ -79,6 +80,10 @@ private: bool _is_h_scroll_visible() const; bool _is_v_scroll_visible() const; + bool draw_focus_border = false; + bool focus_border_is_drawn = false; + bool child_has_focus(); + protected: Size2 get_minimum_size() const override; @@ -125,6 +130,9 @@ public: PackedStringArray get_configuration_warnings() const override; + void set_draw_focus_border(bool p_draw); + bool get_draw_focus_border(); + ScrollContainer(); }; diff --git a/scene/theme/default_theme.cpp b/scene/theme/default_theme.cpp index f5065e8de10..2d47b5f8977 100644 --- a/scene/theme/default_theme.cpp +++ b/scene/theme/default_theme.cpp @@ -157,7 +157,7 @@ void fill_default_theme(Ref &theme, const Ref &default_font, const const Ref button_pressed = make_flat_stylebox(style_pressed_color); const Ref button_disabled = make_flat_stylebox(style_disabled_color); Ref focus = make_flat_stylebox(style_focus_color, default_margin, default_margin, default_margin, default_margin, default_corner_radius, false, 2); - // Make the focus outline appear to be flush with the buttons it's focusing. + // Make the focus outline appear to be flush with the buttons it's focusing, so not draw on top of the content. focus->set_expand_margin_all(Math::round(2 * scale)); theme->set_stylebox(CoreStringName(normal), "Button", button_normal); @@ -659,6 +659,14 @@ void fill_default_theme(Ref &theme, const Ref &default_font, const empty.instantiate(); theme->set_stylebox(SceneStringName(panel), "ScrollContainer", empty); + const Ref focus_style = make_flat_stylebox(style_focus_color); + // Make the focus outline appear to be flush with the buttons it's focusing, so not draw on top of the content. + sb_expand(focus_style, 4, 4, 4, 4); + focus_style->set_border_width_all(Math::round(2 * scale)); + focus_style->set_draw_center(false); + focus_style->set_border_color(style_focus_color); + theme->set_stylebox("focus", "ScrollContainer", focus_style); + // Window theme->set_stylebox("embedded_border", "Window", sb_expand(make_flat_stylebox(style_popup_color, 10, 28, 10, 8), 8, 32, 8, 6)); diff --git a/tests/scene/test_control.h b/tests/scene/test_control.h index 3d7e389e0a7..557c5b232cf 100644 --- a/tests/scene/test_control.h +++ b/tests/scene/test_control.h @@ -61,6 +61,57 @@ TEST_CASE("[SceneTree][Control]") { } } +TEST_CASE("[SceneTree][Control] Focus") { + Control *ctrl = memnew(Control); + SceneTree::get_singleton()->get_root()->add_child(ctrl); + + SUBCASE("[SceneTree][Control] Default focus") { + CHECK_FALSE(ctrl->has_focus()); + } + + SUBCASE("[SceneTree][Control] Can't grab focus with default focus mode") { + ERR_PRINT_OFF + ctrl->grab_focus(); + ERR_PRINT_ON + + CHECK_FALSE(ctrl->has_focus()); + } + + SUBCASE("[SceneTree][Control] Can grab focus") { + ctrl->set_focus_mode(Control::FocusMode::FOCUS_ALL); + ctrl->grab_focus(); + + CHECK(ctrl->has_focus()); + } + + SUBCASE("[SceneTree][Control] Can release focus") { + ctrl->set_focus_mode(Control::FocusMode::FOCUS_ALL); + ctrl->grab_focus(); + CHECK(ctrl->has_focus()); + + ctrl->release_focus(); + CHECK_FALSE(ctrl->has_focus()); + } + + SUBCASE("[SceneTree][Control] Only one can grab focus at the same time") { + ctrl->set_focus_mode(Control::FocusMode::FOCUS_ALL); + ctrl->grab_focus(); + CHECK(ctrl->has_focus()); + + Control *other_ctrl = memnew(Control); + SceneTree::get_singleton()->get_root()->add_child(other_ctrl); + other_ctrl->set_focus_mode(Control::FocusMode::FOCUS_ALL); + other_ctrl->grab_focus(); + + CHECK(other_ctrl->has_focus()); + CHECK_FALSE(ctrl->has_focus()); + + memdelete(other_ctrl); + } + + memdelete(ctrl); +} + } // namespace TestControl #endif // TEST_CONTROL_H