-
Notifications
You must be signed in to change notification settings - Fork 26
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
Feature proposal: Add action closure to MenuItem #203
Comments
Thanks for the detailed proposal. I didn't implement this action closure before because I didn't want to force users to share their app state through an Another reason was, users may expect this callback closure to have access to the menu item that is being triggered in case they want to modify it, like changing the icon or something. This can get complicated really fast as all menu items are basically a wrapper around I don't mind having this closure callbacks but ofc without removing the events, so if you want to work on this, please do, we appreciate it. |
Thank you for the quick and friendly answer :D |
Hey, first of all, thanks for the project. I think it's really great to be able to easily create a cross platform menu bar, and I like the way this crate specifically works.
I especially think the way that you can create a menu recursively (like how it's done in the examples) is really nice:
However, I noticed one thing. As it's necessary to process the menu events (otherwise the menu would be pretty useless), you need to create all menu items explicitly as variables to get their
id
s, so that you can process the events like this:However, this destroys the benefit of the upper structure.
Instead of being able to write a recursive block like above, you have to do it like this:
This means that the menu definition has to be split up between the (sub-)menus and the menu items, essentially not allowing to use this great feature of defining menus recursively in-line. That can make it harder to intuitively see the structure of nested menus and make the code less comprehensible.
This system also means that the menu definition and the menu logic are separated. This makes it easier to introduce errors, e.g. by changing the structure or logic in one place and forgetting it in the other, and less easy to oversee.
Therefore, I'd suggest a feature that fixes this and allows for a full menu definition in one place: Including an
FnMut
action closure directly in theMenuItem
.This would allow creating a menu like this:
with all the advantages I mentioned earlier.
Of course you could also link to a real function for more advanced actions or different actions that overlap.
The only other reason why someone needs to create a menu item before integrating it in the menu is to keep it for the future, e.g. when you want to disable some menu items in specific situations. This is not as common as the actions (as they are basically necessary for every menu item), but it's still important to be able to do it.
I propose a second constructor,
MenuItem::new_with_handle
to solve this (and not require a prior menu item definition for this use case).If you want to be able to access the
MenuItem
after the menu creation, you could use the constructor like this:This would create the menu and save the created menu item in
item_1_handle
. This way, it allows to change the item in the future without having to separate it from the menu structure.(To avoid the
mut
s, you could also create a newMenuItemHandle
struct that uses internal mutability and a function to expose theMenuItem
, but I don't think that's necessary (although I wouldn't be opposed to that either)).What do you think about these proposals? If you agree to these changes, I'd be very happy to open a PR implementing it. I think this would improve the way we can create menus with
muda
and allow us to use more of the potential this crate already has.The text was updated successfully, but these errors were encountered: