Add shortcut handling to OptionButton

[WhalesState]

Fixes #80201

[WhalesState]

maybe adding a new menu item and assigning it's shortcut or creating a new shortcut menu item will not work by default if i removed this line and will require to set shortcuts enabled with an extra line of code , should i really remove it ?
I'm just following the same way that makes shortcuts works in MenuButton by default, I will also need it enabled since i will start making some few cleans to the editor node simplifying the UI for better integration on small screens and removing the useless nodes in editor UI like control spacers which can be done with horizontal flags instead

[AThousandShips]

Added a more descriptive title to the PR, would be good to have the commit message have an equally descriptive one, remember that it's better to describe what is done rather than what problem is fixed

[WhalesState]

@AThousandShips this is not the right title, shortcuts wasn't working at all in menu button to add an option to disable them

[WhalesState]

this is the main reason why i have added this fix

void OptionButton::shortcut_input(const Ref<InputEvent> &p_event) {
	ERR_FAIL_COND(p_event.is_null());

	if (disable_shortcuts) {
		return;
	}

	if (p_event->is_pressed() && !p_event->is_echo() && !is_disabled() && is_visible_in_tree() && popup->activate_item_by_event(p_event, false)) {
		accept_event();
		return;
	}

	Button::shortcut_input(p_event);
}

[AThousandShips]

Then I'd suggest:

Add shortcut handling to `OptionButton`

Also adds option to disable shortcuts

As the commit message

[WhalesState]

@AThousandShips choose whatever you like, just describe it to be addressed correctly in the list of fixes in godot 4.2

[AThousandShips]

I can't change the commit message, but it has to be descriptive as well :)

[WhalesState]

is there anyway to change the commit message without making a force push ?

[AThousandShips]

No, but I don't think the current message is very helpful, as it doesn't specify what has been fixed, i.e. that no handling exists period

[WhalesState]

ok i will change it and remove the extra line that enables the enabled variable again, we also may need to change it later in MenuButton too, just remember to mention it when someone changes their files in a pull request

[AThousandShips]

You can wait until CI has finished to reduce time in next run, as it caches some of the results

[YuriSizov]

On the surface it looks correct as it mimics similar logic from MenuButton. Still, could you provide a test project that you've used to validate the change?

[WhalesState]

i have noticed the issue while working on Editor in c++, for example CanvasItemEditor is using MenuButtons with shortcuts for snapping options, but when you try to use the OptionButton with shortcuts it doesn't work because it was missing shortcut handeling function.

this single line will work fine with MenuButton's PopupMenu while it won't work with OptionButton's PopupMenu

select_popup->set_item_shortcut(TOOL_SELECT, ED_SHORTCUT("canvas_item_editor/select_mode", TTR("Select Mode"), Key::Q));

this fix is important to make OptionButton fully functional in GUI not just for editor but for editor plugins too, i have seen many people struggle asking how to make a shortcut gets triggered in OptionButton while one of them got no replies another one have been told to handle the shortcuts manually using _input() function, which will require an extra 10 or more lines of code in gdscript, another 2 issues was opened on github and got closed with no reply or fixes, if both MenuButton and OptionButton are using PopupMenu then both should handle it correctly not just MenuButton

[WhalesState]

This is how i tested it, which will be hard to be provided as a test project at the moment since it's for godot-2d repo

the issue appeared after moving the selection buttons to be items inside an OptionButton to clean the ui and reduce the CanvasItemEditor minimum width which affects the editor minimum width on screen as well for better integration on small screens and to allow you to split screens, for example the editor on the left side of the screen and the new floating code editor window on the right.

also ignore that the shortcuts offset in tooltip are drawn with wrong offset, this is an editor theme issue and will try to fix it too.

Edit: it appears that there was no issue in the editor theme, it happens because godot now clips text label height automatically when there are no text. which have caused this to happen, adding a space " " in item name instead of leaving it empty will work as a fix for now

[WhalesState]

have found this which may be helpful for testing, just create an option button and add this script to it then press Q and it will change the item, in godot 4.1.1 stable the same code inside the ready function will work for MenuButton but not OptionButton

extends OptionButton


func _ready():
	add_item("test")
	add_item("test with shortcut")
	var sc := Shortcut.new()
	var ev := InputEventKey.new()
	ev.keycode = KEY_Q
	sc.events.append(ev)
	get_popup().set_item_shortcut(1, sc)

[akien-mga]

Thanks!