Merge pull request #17809 from RandomShaper/menu-item-on-release

Improve popup menus usability

3 files changed, 47 insertions, 28 deletions

diff --git a/scene/gui/menu_button.cpp b/scene/gui/menu_button.cpp
index 2e74faa61d..87cf4dc334 100644
--- a/scene/gui/menu_button.cpp
+++ b/scene/gui/menu_button.cpp
@@ -59,7 +59,6 @@ void MenuButton::pressed() {
 	popup->set_size(Size2(size.width, 0));
 	popup->set_parent_rect(Rect2(Point2(gp - popup->get_global_position()), get_size()));
 	popup->popup();
-	popup->set_invalidate_click_until_motion();
 }
 
 void MenuButton::_gui_input(Ref<InputEvent> p_event) {
@@ -109,7 +108,6 @@ MenuButton::MenuButton() {
 	add_child(popup);
 	popup->set_as_toplevel(true);
 	popup->set_pass_on_modal_close_click(false);
-	connect("button_up", popup, "call_deferred", make_binds("grab_click_focus"));
 	set_process_unhandled_key_input(true);
 	set_action_mode(ACTION_MODE_BUTTON_PRESS);
 }
diff --git a/scene/gui/popup_menu.cpp b/scene/gui/popup_menu.cpp
index 747230e69f..27853a8238 100644
--- a/scene/gui/popup_menu.cpp
+++ b/scene/gui/popup_menu.cpp
@@ -284,7 +284,8 @@ void PopupMenu::_gui_input(const Ref<InputEvent> &p_event) {
 		if (b->is_pressed())
 			return;
 
-		switch (b->get_button_index()) {
+		int button_idx = b->get_button_index();
+		switch (button_idx) {
 
 			case BUTTON_WHEEL_DOWN: {
 
@@ -298,30 +299,37 @@ void PopupMenu::_gui_input(const Ref<InputEvent> &p_event) {
 					_scroll(b->get_factor(), b->get_position());
 				}
 			} break;
-			case BUTTON_LEFT: {
-
-				int over = _get_mouse_over(b->get_position());
-
-				if (invalidated_click) {
-					invalidated_click = false;
-					break;
-				}
-				if (over < 0) {
-					hide();
-					break; //non-activable
-				}
-
-				if (items[over].separator || items[over].disabled)
-					break;
-
-				if (items[over].submenu != "") {
-
-					_activate_submenu(over);
-					return;
+			default: {
+				// Allow activating item by releasing the LMB or any that was down when the popup appeared
+				if (button_idx == BUTTON_LEFT || (initial_button_mask & (1 << (button_idx - 1)))) {
+
+					bool was_during_grabbed_click = during_grabbed_click;
+					during_grabbed_click = false;
+
+					int over = _get_mouse_over(b->get_position());
+
+					if (invalidated_click) {
+						invalidated_click = false;
+						break;
+					}
+					if (over < 0) {
+						if (!was_during_grabbed_click) {
+							hide();
+						}
+						break; //non-activable
+					}
+
+					if (items[over].separator || items[over].disabled)
+						break;
+
+					if (items[over].submenu != "") {
+
+						_activate_submenu(over);
+						return;
+					}
+					activate_item(over);
 				}
-				activate_item(over);
-
-			} break;
+			}
 		}
 
 		//update();
@@ -503,6 +511,11 @@ void PopupMenu::_notification(int p_what) {
 				update();
 			}
 		} break;
+		case NOTIFICATION_POST_POPUP: {
+
+			initial_button_mask = Input::get_singleton()->get_mouse_button_mask();
+			during_grabbed_click = (bool)initial_button_mask;
+		} break;
 		case NOTIFICATION_POPUP_HIDE: {
 
 			if (mouse_over >= 0) {
@@ -1216,15 +1229,20 @@ void PopupMenu::_bind_methods() {
 	ADD_SIGNAL(MethodInfo("index_pressed", PropertyInfo(Variant::INT, "index")));
 }
 
-void PopupMenu::set_invalidate_click_until_motion() {
+void PopupMenu::popup(const Rect2 &p_bounds) {
+
+	grab_click_focus();
 	moved = Vector2();
 	invalidated_click = true;
+	Popup::popup(p_bounds);
 }
 
 PopupMenu::PopupMenu() {
 
 	mouse_over = -1;
 	submenu_over = -1;
+	initial_button_mask = 0;
+	during_grabbed_click = false;
 
 	set_focus_mode(FOCUS_ALL);
 	set_as_toplevel(true);
diff --git a/scene/gui/popup_menu.h b/scene/gui/popup_menu.h
index 60f36e95ec..4a62f81a65 100644
--- a/scene/gui/popup_menu.h
+++ b/scene/gui/popup_menu.h
@@ -78,6 +78,8 @@ class PopupMenu : public Popup {
 	Timer *submenu_timer;
 	List<Rect2> autohide_areas;
 	Vector<Item> items;
+	int initial_button_mask;
+	bool during_grabbed_click;
 	int mouse_over;
 	int submenu_over;
 	Rect2 parent_rect;
@@ -178,7 +180,6 @@ public:
 	void add_autohide_area(const Rect2 &p_area);
 	void clear_autohide_areas();
 
-	void set_invalidate_click_until_motion();
 	void set_hide_on_item_selection(bool p_enabled);
 	bool is_hide_on_item_selection() const;
 
@@ -188,6 +189,8 @@ public:
 	void set_hide_on_multistate_item_selection(bool p_enabled);
 	bool is_hide_on_multistate_item_selection() const;
 
+	virtual void popup(const Rect2 &p_bounds = Rect2());
+
 	PopupMenu();
 	~PopupMenu();
 };