Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow enter to select items from combobox's list. #6570

Closed
wants to merge 1 commit into from
Closed

Allow enter to select items from combobox's list. #6570

wants to merge 1 commit into from

Conversation

basicer
Copy link
Contributor

@basicer basicer commented Oct 29, 2017

Currently if you use the keyboard to search though a combobox and try to select an item with enter, it will close the form without sending the update.

@SmallJoker SmallJoker added Feature ✨ PRs that add or enhance a feature Rebase needed The PR needs to be rebased by its author labels Nov 19, 2017
@basicer
Copy link
Contributor Author

basicer commented Nov 24, 2017

@SmallJoker Rebased

@SmallJoker SmallJoker removed the Rebase needed The PR needs to be rebased by its author label Nov 24, 2017
@paramat
Copy link
Contributor

paramat commented Nov 24, 2017

Do you know of a mod which allows me to test it?

@basicer
Copy link
Contributor Author

basicer commented Nov 24, 2017

minetest.register_chatcommand("z", {
	func = function(name, param)
		minetest.show_formspec(name, "z", "size[5,1]\ndropdown[0,0;5;test;A,B,C,D,E,F,G,H,I,J,K,L,M,N,O,P;1]")
	end,
})

minetest.register_on_player_receive_fields(function(p, formname, fields)
	if formname == "z" then minetest.chat_send_all(fields.test or 'nil') end
end)

With the patch you can click the combo-box and enter a letter on the keyboard then press enter to select. The correct letter will show in chat.

Without the patch, doing the above will close the form and previously selected letter will remain.

@paramat
Copy link
Contributor

paramat commented Nov 24, 2017

Thanks.

@@ -3000,7 +3000,8 @@ bool GUIFormSpecMenu::preprocessEvent(const SEvent& event)
gui::IGUIElement *focused = Environment->getFocus();
if (focused && isMyChild(focused) &&
(focused->getType() == gui::EGUIET_LIST_BOX ||
focused->getType() == gui::EGUIET_CHECK_BOX)) {
focused->getType() == gui::EGUIET_CHECK_BOX) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks weird, please remove any spaces.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's logical, as it's within the parentheses.

Copy link
Contributor

@paramat paramat Dec 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's done to line stuff up, but we have a strict rule to never use spaces in indentation.

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paramat paramat added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Dec 27, 2017
@paramat
Copy link
Contributor

paramat commented Dec 27, 2017

	// Fix Esc/Return key being eaten by checkboxen and tables
	if(event.EventType==EET_KEY_INPUT_EVENT) {
		KeyPress kp(event.KeyInput);
		if (kp == EscapeKey || kp == CancelKey
				|| kp == getKeySetting("keymap_inventory")
				|| event.KeyInput.Key==KEY_RETURN) {

Only RETURN should select, Escape / Cancel / Inventory keys should continue to close the form.

@paramat
Copy link
Contributor

paramat commented Feb 5, 2018

No response to requests for a month, closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Action / change needed Code still needs changes (PR) / more information requested (Issues) Feature ✨ PRs that add or enhance a feature One approval ✅ ◻️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants