"handle_on" depends on order of components in a form (it should not)

[lo5]

See discussion: #1480 (reply in thread)

Solution: Add a sort_args=False to handle_on. If set to True, sort q.args before iterating.

[Far0n]

concerns about sort_args approach (for reference):

#1480 (reply in thread)
#1480 (reply in thread)

[Far0n]

Maybe it would be better for now to recommend to use on only for buttons. sort_args wont be needed then and it doesn't solve other tricky cases like trigger=True on toggles:

#1480 (reply in thread):
btw. toggles would still remain troublesome, because they have boolean values and a trigger event might give us toggle: False in q.args. Since that's not truthy handle_on wont invoke the handler for a True -> False switch (trigger = True).
So one actually needs to do @on("toggle", lambda x:True) to even have a chance to make it work.

Code_B9HpcaWUDu.mp4

The only thing that worked for me is to replace on with my own set of decorators, namely: on_click, on_change, on_event and on_path + corresponding handle_on replacements. From video above:

@app("/")
async def serve(q: Q) -> None:
    if q.client.toggle_01 is None:
        q.client.toggle_01 = False
    if q.client.toggle_02 is None:
        q.client.toggle_02 = False

    await handle_changes(q, compared_to=q.client)

    print(f"q.args : {q.args}")
    print(f"on_change.handlers: {on_change.handlers}")
    await setup_page(q)
    await q.page.save()

@on_change("toggle_{id}")
async def toggle_handler(q: Q, val, id):
    print(f"toggle_{id} changed, new val: {val}")
    q.client[f"toggle_{id}"] = val

async def setup_page(q: Q):
    q.page["example"] = ui.form_card(
        box="1 1 2 2",
        items=[
            ui.toggle(name="toggle_01", value=q.client.toggle_01, trigger=True),
            ui.toggle(name="toggle_02", value=q.client.toggle_02, trigger=True),
        ],
    )

[Far0n]

@lo5 wouldn't the following be an option: I believe currently q.events is only used for wave.emit() stuff. What if it would always hold the "source event" that led to invoking serve?

For example: Let's say we have an ui.toggle with trigger = True and we "click" it. Let's further assume the value changes from True to False. That would result in:

q.events = {"toggle.triggered": True}
q.args = {"toggle" : False}

That would enable a quite convenient usecase for @on in my opinion:

@on("toggle.triggered")
def toggle_handler(q: Q):
    ...

edit:
I also think button clicks should find their way into q.events, like so:

q.events = {"button.clicked": True}
q.args = {"button" : True}

For sake of backwards compatibility we can't remove it from q.args. So the redundancy could be an argument against it. But I feel one could still argue, that q.events holds the implicit "trigger = True" for the button and q.args it's value. So non-clicks could still be this, I guess:

q.events = {}
q.args = {"button" : False}

[lo5]

@Far0n - I really like your suggestion. It's backwards-compatible, too. Seems obvious in hindsight :)

[vopani]

This might be the better solution we've been looking for.
@lo5 Can we try this out in a branch? I'd still want to test it on a bunch of written apps to confirm if it works well or not.

[Far0n]

Extra note:

I also think button clicks should find their way into q.events, like so:

q.events = {"button.clicked": True}
q.args = {"button" : True}

For sake of backwards compatibility we can't remove it from q.args. So the redundancy could be an argument against it. But I feel one could still argue, that q.events holds the implicit "trigger = True" for the button and q.args its value. So non-clicks could still be this, I guess:

q.events = {}
q.args = {"button" : False}

[padraic-shafer]

@Far0n : Could you share the code you used in handle_changes() and on_change(), etc.? I can guess at what's in there, but it would be even better to see what worked for you.

+1 for including these handlers in the main branch for dealing with triggered events...or the @on("toggle.triggered") that corresponds to q.events = {"toggle.triggered": True}.

[Far0n]

@pshafer-als our wave extensions are here: https://github.com/h2oai/model-validation/blob/master/mv/ui/wave/_routing_ex.py

[padraic-shafer]

@Far0n : Thanks! It looks like I don't have access to that repo. If ok, could you please add me to viewers?

[Far0n]

ahh sorry, its h2o internal only.

[Far0n]

I could create a little demo an put it into discussions. That's maybe a good place to check if others might find it helpful.

[padraic-shafer]

That would be great. Thank you!

[mturoci]

I like @Far0n 's suggestion and would like to expand on it.

I suggest deprecating the current handle_on in favor of a slightly semantically different routing version in the upcoming 1.0 release. The serve function is always triggered by a single event, which flushes the rest of the changes made and submits them to the server. The new mechanism would always fire a callback for the very last "event" that caused server submission. In practice, that can be a button click, trigger=True component, event or route change.

This way, the callbacks would fire "on changed" rather than "on found" wrt q.args.

The migration would be relatively painless: just import new_handle_on instead of handle_on and check if your app works - most of the apps should.

Needs discussion: @Far0n 's proposal suggest adding .clicked | .triggered info to give more info about the source. I am wondering what would be the usecase for it.

Personally, I wouldn't use @on/handle_on in my apps - it's a lot easier for me to reason about application state without them, because with @on/handle_on the application logic ends up being scattered across different event-handling functions, as opposed to being able to read all the logic in one place inside a single function. The single function might jump into other smaller functions, but the important thing is that I'm in control of the logic, not obscured behind handle_on(), and I can decide how to handle q.args based on the situation at hand :)

Disclaimer: I still fully agree with @lo5 that the best solution is the one that is tailored for the given use case with no extra framework magic to understand what's going on.

Will build the proposal in a separate branch for you folks to try out and get feedback.

[Far0n]

Needs discussion: @Far0n 's proposal suggest adding .clicked | .triggered info to give more info about the source. I am wondering what would be the usecase for it.

It gets a bit tricky currently to identify the root cause of a serve invokation if there are several components with trigger=True present (and maybe a submit button on top of it or some navigation link). Because everything lands in q.args one needs to implement extra logic to figure ot what happened to react accordingly. This is esp. tricky with the @on-handler, because only first match is invoked and it doesn't quite work for toggles atm without using lambda expression (i.e. True -> False toggle is not truthy, see #1484 (comment)). (and textbox if if becomes empty for that matter)

So the typical use case is to trigger the "@on" associated to the root cause instead of what is is found in q.args. But if I'm not mistaken your proposed solution would deal with that issue as well.

[mturoci]

So the typical use case is to trigger the "@on" associated to the root cause instead of what is is found in q.args. But if I'm not mistaken your proposed solution would deal with that issue as well.

Correct. This complexity will be hidden from regular wave app devs. I just asked to make sure you do not need to differentiate between .clicked and .triggered. But seems like the answer is no its whole purpose is to identify the serve invocation cause.

[Far0n]

But seems like the answer is no its whole purpose is to identify the serve invocation cause.

Yes, that's what I believe to remember at least :D (back when I thought about it .. so take it with a grain of salt ^^)

edit: so yeah I think these 2 things: a) root cause b) solution for the "is not truthy" issue with current (handle_on, @on). And a proper solution to a) should also solve b) I guess.

[Far0n]

@mturoci but if I think about it: wouldn't it be in general benefical of have the "root cause" accessible in q.events? So that we can make use of it in apps, that don't use [new]_handle_on?

[mturoci]

I currently expose the name of the cause in q.args['__wave_submission_name__']. It's prefixed with __ to mark it as "do not touch unless you know what you are doing". I suppose that should be enough?

[mturoci]

Looking for feedback in #2113 whoever is interested.