From bce30062a552fd493e5507959bc977d541b4997b Mon Sep 17 00:00:00 2001 From: Michael Alexsander Date: Sat, 27 Aug 2022 13:41:58 -0300 Subject: [PATCH] Fix some corner cases in the `Menu/OptionButton` item auto-highlight --- doc/classes/PopupMenu.xml | 1 + scene/gui/base_button.cpp | 3 ++- scene/gui/menu_button.cpp | 11 ++++++++--- scene/gui/option_button.cpp | 15 ++++++++++++--- scene/gui/popup_menu.cpp | 20 ++++++++++++++++---- scene/main/viewport.cpp | 2 ++ 6 files changed, 41 insertions(+), 11 deletions(-) diff --git a/doc/classes/PopupMenu.xml b/doc/classes/PopupMenu.xml index 507dd368f..ab1e7b3a4 100644 --- a/doc/classes/PopupMenu.xml +++ b/doc/classes/PopupMenu.xml @@ -317,6 +317,7 @@ Sets the currently focused item as the given [code]index[/code]. + Passing [code]-1[/code] as the index makes so that no item is focused. diff --git a/scene/gui/base_button.cpp b/scene/gui/base_button.cpp index fb0f3937b..19584d505 100644 --- a/scene/gui/base_button.cpp +++ b/scene/gui/base_button.cpp @@ -66,8 +66,9 @@ void BaseButton::_gui_input(Ref p_event) { bool button_masked = mouse_button.is_valid() && ((1 << (mouse_button->get_button_index() - 1)) & button_mask) > 0; if (button_masked || ui_accept) { was_mouse_pressed = button_masked; - on_action_event(p_event); + was_mouse_pressed = false; + return; } diff --git a/scene/gui/menu_button.cpp b/scene/gui/menu_button.cpp index 00cd44d2c..e621d03bc 100644 --- a/scene/gui/menu_button.cpp +++ b/scene/gui/menu_button.cpp @@ -65,9 +65,14 @@ void MenuButton::pressed() { popup->set_scale(get_global_transform().get_scale()); popup->set_parent_rect(Rect2(Point2(gp - popup->get_global_position()), get_size())); - // If not triggered by the mouse, start the popup with its first item selected. - if (popup->get_item_count() > 0 && !_was_pressed_by_mouse()) { - popup->set_current_index(0); + // If not triggered by the mouse, start the popup with its first enabled item focused. + if (!_was_pressed_by_mouse()) { + for (int i = 0; i < popup->get_item_count(); i++) { + if (!popup->is_item_disabled(i)) { + popup->set_current_index(i); + break; + } + } } popup->popup(); diff --git a/scene/gui/option_button.cpp b/scene/gui/option_button.cpp index dd7f7a515..1f1924bf3 100644 --- a/scene/gui/option_button.cpp +++ b/scene/gui/option_button.cpp @@ -115,9 +115,18 @@ void OptionButton::pressed() { popup->set_size(Size2(size.width, 0)); popup->set_scale(get_global_transform().get_scale()); - // If not triggered by the mouse, start the popup with its first item selected. - if (popup->get_item_count() > 0 && !_was_pressed_by_mouse()) { - popup->set_current_index(0); + // If not triggered by the mouse, start the popup with the checked item (or the first enabled one) focused. + if (!_was_pressed_by_mouse()) { + if (current > -1 && !popup->is_item_disabled(current)) { + popup->set_current_index(current); + } else { + for (int i = 0; i < popup->get_item_count(); i++) { + if (!popup->is_item_disabled(i)) { + popup->set_current_index(i); + break; + } + } + } } popup->popup(); diff --git a/scene/gui/popup_menu.cpp b/scene/gui/popup_menu.cpp index e8c832c08..0a034c5ad 100644 --- a/scene/gui/popup_menu.cpp +++ b/scene/gui/popup_menu.cpp @@ -191,9 +191,14 @@ void PopupMenu::_activate_submenu(int over, bool p_by_keyboard) { PopupMenu *pum = Object::cast_to(pm); if (pum) { - // If not triggered by the mouse, start the popup with its first item selected. - if (pum->get_item_count() > 0 && p_by_keyboard) { - pum->set_current_index(0); + // If not triggered by the mouse, start the popup with its first enabled item focused. + if (p_by_keyboard) { + for (int i = 0; i < pum->get_item_count(); i++) { + if (!pum->is_item_disabled(i)) { + pum->set_current_index(i); + break; + } + } } // Setting autohide areas *must* be done after `popup()` which can move the popup (to fit it into the viewport). @@ -1071,7 +1076,14 @@ bool PopupMenu::is_item_shortcut_disabled(int p_idx) const { } void PopupMenu::set_current_index(int p_idx) { - ERR_FAIL_INDEX(p_idx, items.size()); + if (p_idx != -1) { + ERR_FAIL_INDEX(p_idx, items.size()); + } + + if (mouse_over == p_idx) { + return; + } + mouse_over = p_idx; update(); } diff --git a/scene/main/viewport.cpp b/scene/main/viewport.cpp index e67fbf5ea..d390f669a 100644 --- a/scene/main/viewport.cpp +++ b/scene/main/viewport.cpp @@ -2080,6 +2080,8 @@ void Viewport::_gui_input_event(Ref p_event) { popup_menu->hide(); menu_button->pressed(); + // As the popup wasn't triggered by a mouse click, the item focus needs to be removed manually. + menu_button->get_popup()->set_current_index(-1); } else { over = nullptr; //nothing can be found outside the modal stack }