rumps icon indicating copy to clipboard operation
rumps copied to clipboard

Add rumps.App.on_before_event_loop callback. Add .menuitem attribute to every function decorated with @rumps.clicked

Open dair-targ opened this issue 6 years ago • 5 comments

Hi! I've added a few lines so the application and menu items could now be initialized more conveniently. Here is an example, also committed as examples/example_init.py:

import rumps

@rumps.clicked('Switch next')
def switch_next(_):
    switch_previous.menuitem.state = not switch_previous.menuitem.state

@rumps.clicked('Switch previous')
def switch_previous(_):
    switch_next.menuitem.state = not switch_next.menuitem.state

def init_app():
    switch_next.menuitem.state = True

if __name__ == "__main__":
    rumps.App('First item should be checked', on_before_event_loop=init_app).run()

My intention was to solve my problem around item state initialization. However it looks like it could be used for little bit more purposes as I tried to demonstrate in the example above.

dair-targ avatar Jan 20 '19 12:01 dair-targ

Interesting idea.

I wonder do we need to worry about the asymmetry this would cause for those functions decorated vs. specified when creating a MenuItem? In other words, only the former would have menuitem attribute. I believe you could fix this by assigning the attribute in MenuItem.set_callback.

Also, it is possible (but maybe not likely) that someone would decorate the same function with different clicked. This would cause the most recent menu item to be the "primary" menu item. Is this acceptable?

jaredks avatar Feb 02 '19 01:02 jaredks

Thanks for catching the asymmetry. I'll address that.

Regarding the function decorated multiple times, I guess there are few options:

  • The current implementation, only the most recent menu item is accessible (as you mentioned)
  • If the menuitem attribute is already set, raise an error saying a single function could be decorated only once.
  • Come up with a more sophisticated mechanism, e.g. have menuitems:List[MenuItem] attribute.

If we want to keep the things simple, then its better to discard the last option. Between the first two options - I would gladly follow any, as that case is definitely out of my concern:)

Any thoughts which way is preferable?

p.s. Should the same behavior be provided for SliderMenuItem?

dair-targ avatar Feb 02 '19 04:02 dair-targ

@jaredks any chance to merge it?

dair-targ avatar Jun 09 '19 04:06 dair-targ

@dair-targ Thanks for your interest in the project and work on this!

There is only one function associated with a menu item but this change would suggest there is also only one menu item associated with a function. Since that isn't necessarily true, I don't want to indicate that with the menuitem attribute.

The example code you have could be written similarly but using MenuItem objects instead of functions like,

import rumps


switch_next = rumps.MenuItem('Switch next')
@switch_next.set_callback
def switch_next_func(_):
    switch_previous.state = not switch_previous.state


switch_previous = rumps.MenuItem('Switch previous')
@switch_previous.set_callback
def switch_previous_func(_):
    switch_next.state = not switch_next.state


if __name__ == "__main__":
    switch_next.state = True
    rumps.App('First item should be checked', menu=[switch_next, switch_previous]).run()

As for on_before_event_loop change, it seems like a good idea. I wonder about the name though - should it be simplified to before_event_loop or maybe before_start. What do you think? If you want to include that in another PR that would be great!

jaredks avatar Jun 19 '19 23:06 jaredks

Nice - didn't knew it's possible to handle menu items this way!

Sure, I'll create another PR for the handler. I'd prefer to leave the naming up to you as the project creator. Which one would you prefer?

dair-targ avatar Jun 20 '19 13:06 dair-targ