Allow menu options to be updatable #12
Conversation
| self._menu_actions_by_id = set() | ||
| self._menu_options = self._add_ids_to_menu_options(list(menu_options)) | ||
| self._menu_actions_by_id = dict(self._menu_actions_by_id) | ||
| self._menu = None |
There was a problem hiding this comment.
I'm concerned about leaking objects here. Shouldn't we call DestroyMenu to free "self._menu" if it is not None? We can also leak submenus in this case.
|
Updated to use DestroyMenu |
|
Any blockers on getting this merged? |
| DestroyMenu(self._menu) | ||
| self._menu = None | ||
| if self._hicon != 0: | ||
| DestroyIcon(self._hicon) |
There was a problem hiding this comment.
We mustn't destroy the icon if it is "shared". see comment in _load_icon
|
I am still concerned about resource leaks. When using submenus we call "CreatePopupMenu" more times and these resources have to be deleted. We don't do that today (already bad) and this option could make it worse. We should probably keep a collection of all menu and menu icons we create and release them when updating the options. |
|
@wiggin15 Please please please please can you implement some form of this?? I really want to be able to add new menu options with a dialogue on the fly |
This adds a new parameter to the update function such that the menu_options can be updated the same way that the icon/hover text can be updated. I wanted this functionality for an app where an item in the right click menu changes based on state, and this pull request adds this functionality.
I also tried to avoid duplicated code and keep the menu creation code in one central function "_set_menu_options"