Merge pull request #66748 from EricEzaM/66308-66403-menubutton-improvements

Improve MenuButton and OptionButton

doc/classes/MenuButton.xml

@@ -25,6 +25,12 @@
 				If [code]true[/code], shortcuts are disabled and cannot be used to trigger the button.
 			</description>
 		</method>
+		<method name="show_popup">
+			<return type="void" />
+			<description>
+				Adjusts popup position and sizing for the [MenuButton], then shows the [PopupMenu]. Prefer this over using [code]get_popup().popup()[/code].
+			</description>
+		</method>
 	</methods>
 	<members>
 		<member name="action_mode" type="int" setter="set_action_mode" getter="get_action_mode" overrides="BaseButton" enum="BaseButton.ActionMode" default="0" />

doc/classes/OptionButton.xml

@@ -190,6 +190,12 @@
 				Sets the tooltip of the item at index [param idx].
 			</description>
 		</method>
+		<method name="show_popup">
+			<return type="void" />
+			<description>
+				Adjusts popup position and sizing for the [OptionButton], then shows the [PopupMenu]. Prefer this over using [code]get_popup().popup()[/code].
+			</description>
+		</method>
 	</methods>
 	<members>
 		<member name="action_mode" type="int" setter="set_action_mode" getter="get_action_mode" overrides="BaseButton" enum="BaseButton.ActionMode" default="0" />

scene/gui/menu_button.cpp

@@ -44,15 +44,12 @@ void MenuButton::shortcut_input(const Ref<InputEvent> &p_event) {
 		return;
 	}
 
-	if (p_event->is_pressed() && !p_event->is_echo() && (Object::cast_to<InputEventKey>(p_event.ptr()) || Object::cast_to<InputEventJoypadButton>(p_event.ptr()) || Object::cast_to<InputEventAction>(*p_event) || Object::cast_to<InputEventShortcut>(*p_event))) {
+	if (p_event->is_pressed() && !p_event->is_echo() && !is_disabled() && is_visible_in_tree() && popup->activate_item_by_event(p_event, false)) {
-		if (!get_parent() || !is_visible_in_tree() || is_disabled()) {
+		accept_event();
-			return;
+		return;
-		}
-
-		if (popup->activate_item_by_event(p_event, false)) {
-			accept_event();
-		}
 	}
+
+	Button::shortcut_input(p_event);
 }
 
 void MenuButton::_popup_visibility_changed(bool p_visible) {
@@ -91,6 +88,18 @@ void MenuButton::pressed() {
 		return;
 	}
 
+	show_popup();
+}
+
+PopupMenu *MenuButton::get_popup() const {
+	return popup;
+}
+
+void MenuButton::show_popup() {
+	if (!get_viewport()) {
+		return;
+	}
+
 	emit_signal(SNAME("about_to_popup"));
 	Size2 size = get_size() * get_viewport()->get_canvas_transform().get_scale();
 
@@ -116,14 +125,6 @@ void MenuButton::pressed() {
 	popup->popup();
 }
 
-void MenuButton::gui_input(const Ref<InputEvent> &p_event) {
-	BaseButton::gui_input(p_event);
-}
-
-PopupMenu *MenuButton::get_popup() const {
-	return popup;
-}
-
 void MenuButton::set_switch_on_hover(bool p_enabled) {
 	switch_on_hover = p_enabled;
 }
@@ -226,6 +227,7 @@ void MenuButton::_get_property_list(List<PropertyInfo> *p_list) const {
 
 void MenuButton::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_popup"), &MenuButton::get_popup);
+	ClassDB::bind_method(D_METHOD("show_popup"), &MenuButton::show_popup);
 	ClassDB::bind_method(D_METHOD("set_switch_on_hover", "enable"), &MenuButton::set_switch_on_hover);
 	ClassDB::bind_method(D_METHOD("is_switch_on_hover"), &MenuButton::is_switch_on_hover);
 	ClassDB::bind_method(D_METHOD("set_disable_shortcuts", "disabled"), &MenuButton::set_disable_shortcuts);

scene/gui/menu_button.h

@@ -44,8 +44,6 @@ class MenuButton : public Button {
 
 	Vector2i mouse_pos_adjusted;
 
-	virtual void gui_input(const Ref<InputEvent> &p_event) override;
-
 	void _popup_visibility_changed(bool p_visible);
 
 protected:
@@ -60,6 +58,8 @@ public:
 	virtual void pressed() override;
 
 	PopupMenu *get_popup() const;
+	void show_popup();
+
 	void set_switch_on_hover(bool p_enabled);
 	bool is_switch_on_hover();
 	void set_disable_shortcuts(bool p_disabled);

scene/gui/option_button.cpp

@@ -231,32 +231,7 @@ void OptionButton::pressed() {
 		return;
 	}
 
-	Size2 size = get_size() * get_viewport()->get_canvas_transform().get_scale();
+	show_popup();
-	popup->set_position(get_screen_position() + Size2(0, size.height * get_global_transform().get_scale().y));
-	popup->set_size(Size2(size.width, 0));
-
-	// If not triggered by the mouse, start the popup with the checked item (or the first enabled one) focused.
-	if (current != NONE_SELECTED && !popup->is_item_disabled(current)) {
-		if (!_was_pressed_by_mouse()) {
-			popup->set_focused_item(current);
-		} else {
-			popup->scroll_to_item(current);
-		}
-	} else {
-		for (int i = 0; i < popup->get_item_count(); i++) {
-			if (!popup->is_item_disabled(i)) {
-				if (!_was_pressed_by_mouse()) {
-					popup->set_focused_item(i);
-				} else {
-					popup->scroll_to_item(i);
-				}
-
-				break;
-			}
-		}
-	}
-
-	popup->popup();
 }
 
 void OptionButton::add_icon_item(const Ref<Texture2D> &p_icon, const String &p_label, int p_id) {
@@ -511,6 +486,39 @@ PopupMenu *OptionButton::get_popup() const {
 	return popup;
 }
 
+void OptionButton::show_popup() {
+	if (!get_viewport()) {
+		return;
+	}
+
+	Size2 size = get_size() * get_viewport()->get_canvas_transform().get_scale();
+	popup->set_position(get_screen_position() + Size2(0, size.height * get_global_transform().get_scale().y));
+	popup->set_size(Size2(size.width, 0));
+
+	// If not triggered by the mouse, start the popup with the checked item (or the first enabled one) focused.
+	if (current != NONE_SELECTED && !popup->is_item_disabled(current)) {
+		if (!_was_pressed_by_mouse()) {
+			popup->set_focused_item(current);
+		} else {
+			popup->scroll_to_item(current);
+		}
+	} else {
+		for (int i = 0; i < popup->get_item_count(); i++) {
+			if (!popup->is_item_disabled(i)) {
+				if (!_was_pressed_by_mouse()) {
+					popup->set_focused_item(i);
+				} else {
+					popup->scroll_to_item(i);
+				}
+
+				break;
+			}
+		}
+	}
+
+	popup->popup();
+}
+
 void OptionButton::get_translatable_strings(List<String> *p_strings) const {
 	popup->get_translatable_strings(p_strings);
 }
@@ -548,6 +556,7 @@ void OptionButton::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("_select_int", "idx"), &OptionButton::_select_int);
 
 	ClassDB::bind_method(D_METHOD("get_popup"), &OptionButton::get_popup);
+	ClassDB::bind_method(D_METHOD("show_popup"), &OptionButton::show_popup);
 
 	ClassDB::bind_method(D_METHOD("set_item_count", "count"), &OptionButton::set_item_count);
 	ClassDB::bind_method(D_METHOD("get_item_count"), &OptionButton::get_item_count);

scene/gui/option_button.h

@@ -123,6 +123,7 @@ public:
 	void remove_item(int p_idx);
 
 	PopupMenu *get_popup() const;
+	void show_popup();
 
 	virtual void get_translatable_strings(List<String> *p_strings) const override;