justpy icon indicating copy to clipboard operation
justpy copied to clipboard

Adding **kwargs to event handlers Interface

Open cheak1974 opened this issue 4 years ago • 2 comments

Hi Everybody,

while I am playing with justPy it turns out that I find the way additional objects are passed to event handlers a little unpythonic and from an OOP point of view not clean. I personally don't like to take an object (the emitter of the event in this case) as a vehicle to inject other objects into the handler routine in order to gain access to them.

My suggestion would be to go the python way and add the **kwargs to the standard interface of the event handlers and of course pass them from the registering function. The advantage would be a better readability of the code and no mixing or forgotten objects that are bound to other objects that have nothing to do with each other. Furthermore the in my opinion not really advisable practive to use functions inside function for the sake of having access to the former UI objects would not be needed any more.

# from your examples but with an additional button to make clear the difference
import justpy as jp

def my_click(self, msg):
    self.d.text = 'I was clicked'
    print(msg.event_type)
    print(msg['event_type'])
    print(msg)

def event_demo():
    wp = jp.WebPage()
    d = jp.P(text='Not clicked yet', a=wp, classes='text-xl m-2 p-2 bg-blue-500 text-white')
    b = jp.Button(text='Click me', a=wp)
    b.d = d                                     # you often do this, passing the reference of the div tag via the button instance
    b.on('click', my_click)
    return wp

jp.justpy(event_demo)

I would suggest the following changes:

import justpy as jp

def my_click(emitter, msg, **kwargs):                              # add keyword arguments to the interface
    kwargs['mymessagebox'].text = 'I was clicked'            # Reference to the div Tag via kwargs
    print(msg.event_type)
    print(msg['event_type'])
    print(msg)

def event_demo():
    wp = jp.WebPage()
    d = jp.P(text='Not clicked yet', a=wp, classes='text-xl m-2 p-2 bg-blue-500 text-white')
    b = jp.Button(text='Click me', a=wp)
    b.on('click', my_click, mymessagebox=d)      # Add the reference of the dic Tag as arbitrary kwarg
    return wp

jp.justpy(event_demo)

...or with inline event linking like that

def event_demo():
    wp = jp.WebPage()
    d = jp.P(text='Not clicked yet', a=wp, classes='text-xl m-2 p-2 bg-blue-500 text-white')
    b = jp.Button(text='Click me', a=wp, click=my_click, mymessagebox=d)
    return wp

jp.justpy(event_demo)

Or here a function in function example from your sessions example code. You create the event handler inside the WebPage function because you want to access the session_data dict. Of course this can be done, but it is a little hacky.

import justpy as jp

session_dict = {}

def session_test(request):
   wp = jp.WebPage()
   if request.session_id not in session_dict:
        session_dict[request.session_id] = {'visits': 0, 'events': 0}
   session_data = session_dict[request.session_id]
   session_data['visits'] += 1
   jp.Div(text=f'My session id: {request.session_id}', classes='m-2 p-1 text-xl', a=wp)
   visits_div = jp.Div(text=f'Number of visits: {session_data["visits"]}', classes='m-2 p-1 text-xl', a=wp)
   b = jp.Button(text=f'Number of Click Events: {session_data["events"]}', classes='m-1 bg-blue-500 hover:bg-blue-700 text-white font-bold py-2 px-4 rounded-full', a=wp)
   b.visits_div = visits_div

   def my_click(self, msg):
       session_data = session_dict[msg.session_id]
       session_data['events'] += 1
       self.text =f'Number of Click Events: {session_data["events"]}'
       self.visits_div.text = f'Number of visits: {session_data["visits"]}'

   b.on('click', my_click)
   return wp

jp.justpy(session_test)

...would have the minimalistic change of:

import justpy as jp

session_dict = {}

def session_test(request):
   wp = jp.WebPage()
   if request.session_id not in session_dict:
        session_dict[request.session_id] = {'visits': 0, 'events': 0}
   session_data = session_dict[request.session_id]
   session_data['visits'] += 1
   jp.Div(text=f'My session id: {request.session_id}', classes='m-2 p-1 text-xl', a=wp)
   visits_div = jp.Div(text=f'Number of visits: {session_data["visits"]}', classes='m-2 p-1 text-xl', a=wp)
   b = jp.Button(text=f'Number of Click Events: {session_data["events"]}', classes='m-1 bg-blue-500 hover:bg-blue-700 text-white font-bold py-2 px-4 rounded-full', a=wp)
   # b.visits_div = visits_div            # OLD
   b.on('click', my_click, session_data=session_data, visits_div=visits_div)      # NEW with objects passed via kwargs
   return wp


def my_click(emitter, msg, **kwargs):             # Not a function in a function anymore, just to access session_data
    kwargs['session_data']['events'] += 1
    emitter.text =f'Number of Click Events: {kwargs["session_data"]["events"]}'
    kwargs['visits_div'].text = f'Number of visits: {kwargs["session_data"]["visits"]}'
   
jp.justpy(session_test)


Looks not like a big difference, but if you have a lot of reference to be passed into the handler this could improve code readability. So there would be no more Objects bound to other Object instances that will stay there forever.

The good news is that this would not be very hard to include and at the same time keep complete compatibility to code made with the "glueing objects to objects" approach.

Just a suggestion maybe you consider to make this possible in future code.

Thanks for this great project.

regards

Chris

cheak1974 avatar Feb 13 '21 07:02 cheak1974

Thank you for this suggestion. This is an interesting suggestion that I think is easy to implement. I'll take a look an think that I can add this to the next version. The only issue is the inline call to the keyword argument. That needs a different syntax from the one you proposed (to differentiate between kwargs for the event handler and kwargs for the component instance) so I think at first the feature will not be available for inline.

elimintz avatar Feb 13 '21 15:02 elimintz

It would be absolutely OK not to implement this for inline event binding. I do prefer the explicit version.

Great that you like the idea. I am looking forward to the next release.

Thank you advance

Cheers Chris

cheak1974 avatar Feb 13 '21 20:02 cheak1974

see #685

WolfgangFahl avatar Sep 20 '23 09:09 WolfgangFahl