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

UIDropDownMenu support for passing a dictionary of callables #257

Conversation

ConquerProgramming1
Copy link
Contributor

@ConquerProgramming1 ConquerProgramming1 commented Feb 4, 2022

Added option to pass a dictionary of strings and method pointers (callables) as the options_list for a UIDropDownMenu as an alternative to menu item event processing. Test provided.

Resolves #256.

…lables) as the options_list for a UIDropDownMenu as an alternative to menu item event processing.
@ConquerProgramming1
Copy link
Contributor Author

ConquerProgramming1 commented Feb 6, 2022

Hmm, noticed that UISelectionList also allows two ways of passing options, but uses a tuple instead:

 item_list: A list of items as strings (item name only), or tuples of two strings (name, theme_object_id).

While a dictionary is a bit easier to work with in the implementation, we could also take this tuple approach for consistency, with the second item being the callable, and then it's extensible in the future by simply adding more things to the tuple. Thoughts?

Copy link
Owner

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me, just had a couple of minor comments.

@@ -23,6 +23,7 @@ class UIExpandedDropDownState:

:param drop_down_menu_ui: The UIDropDownElement this state belongs to.
:param options_list: The list of options in this drop down.
:param options_dict: Optional dictionary of methods to call when an option is chosen.
Copy link
Owner

Choose a reason for hiding this comment

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

I feel like this parameter should have a different name to avoid confusion with options_list and to better represent what it is. Something like per_option_callbacks but perhaps snappier - if you can think of anything?

selected_text = self.drop_down_menu_ui.selected_option
if selected_text and self.options_dict and self.options_dict[selected_text]:
self.options_dict[selected_text]()
return True # In this case, consume the event.
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should be consistent in consuming the UI_SELECTION_LIST_NEW_SELECTION event whether we post the UI_DROP_DOWN_MENU_CHANGED event or not.

I actually think your approach is correct and we should be consuming the UI_SELECTION_LIST_NEW_SELECTION event here. There is no real loss in available functionality as users will still have their call backs or the UI_DROP_DOWN_MENU_CHANGED events if they want to ping a sound effect or something here. This is really an internal event for the drop down and not consuming it could potentially lead to confusion or double event triggering.

@@ -627,7 +636,16 @@ class UIDropDownMenu(UIContainer):
The drop down is implemented through two states, one representing the 'closed' menu state
and one for when it has been 'expanded'.

:param options_list: The list of of options to choose from. They must be strings.
:param options_list: The list of options to choose from. They must be strings.
Copy link
Owner

Choose a reason for hiding this comment

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

shame I didn't name this parameter just 'options' eh? But I guess it is a bit too late now.

@MyreMylar MyreMylar closed this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option for menu items to directly call a method rather than using events
2 participants