handle_on for args with triggers

[srini-x]

Wave SDK Version, OS

Wave 0.16.0, OS agnostic

Actual behavior

Following code triggers dark_mode instead of click_me

trigger for the toggle is implemented like this:
@on('dark_mode', lambda x: isinstance(x, bool))

Code

from h2o_wave import Q, app, handle_on, main, on, ui  # noqa: F401
from h2o_wave.core import expando_to_dict


def log_q_args(q: Q):
    print('>>>> q.args >>>>')
    for k, v in expando_to_dict(q.args).items():
        print(f'{k}: {v}')
    print('<<<< q.args <<<<')


def meta_card(theme):
    return ui.meta_card(
        box='',
        title='Dark Mode',
        layouts=[
            ui.layout(
                breakpoint='xs',
                zones=[ui.zone('main', direction=ui.ZoneDirection.COLUMN)],
                max_width='350px',
            )
        ],
        theme=theme,
    )


async def initialize_client(q: Q):
    if q.client.initialized:
        print('Already Initialized')
        return
    print('Initializing App')

    q.user.dark_mode = False
    q.client.clicks = 0

    q.page['meta'] = meta_card(theme='light')

    q.page['dark_mode'] = ui.section_card(
        box='main',
        title='',
        subtitle='',
        items=[
            ui.toggle(
                name='dark_mode',
                label='Dark Mode',
                value=q.user.dark_mode,
                trigger=True,
            )
        ],
    )
    q.page['clicker'] = ui.form_card(
        box='main',
        items=[
            ui.text(f'Clicked {q.client.clicks} times'),
            ui.button(name='click_me', label='Click Me', primary=True),
        ],
    )
    q.client.initialized = True
    await q.page.save()


@app('/')
async def serve(q: Q):
    print('Enter Serve')
    log_q_args(q)
    await initialize_client(q)
    await handle_on(q)
    print('Exit Serve')


@on('dark_mode', lambda x: isinstance(x, bool))
async def dark_mode(q: Q):
    print('Triggered "dark_mode" handler')
    q.user.dark_mode = q.args.dark_mode
    q.page['dark_mode'].items[0].toggle.value = q.user.dark_mode
    q.page['meta'].theme = 'neon' if q.user.dark_mode else 'light'
    await q.page.save()


@on()
async def click_me(q: Q):
    q.client.clicks += 1
    q.page['clicker'].items[0].text.content = f'Clicked {q.client.clicks} times'
    await q.page.save()

Video of the broken output

click_me_broken.mp4

Expected behavior

Trigger dark_mode only when dark_mode is toggled. Clicking the Click Me button should increment the count as shown in the video below.

click_me.mp4

Steps To Reproduce

1. Save the above code to dark_mode.py
2. Run it

$ wave run dark_mode.py

A proposal

Pass q to the predicate on this line

wave/py/h2o_wave/routing.py

Line 113 in f7a9922

if predicate(arg_value):

changing it to if predicate(q, arg_value):

Change the app code from above to

def dark_mode_check(q: Q, dark_mode: bool) -> bool:
    return dark_mode != q.user.dark_mode

@on('dark_mode', dark_mode_check)
async def dark_mode(q: Q):
...

[srini-x]

@lo5
I think my proposal above should not cause any side effects. If you like the proposal, is it possible to make a patch release?

[lo5]

Thanks for taking the time to provide a repro!

Help me understand the bug:

• The handler is defined as @on('dark_mode', lambda x: isinstance(x, bool)), which translates to "if dark_mode is a boolean, invoke this handler".
• When you click on the click_me button, the args contain click_me: true, dark_mode: false.

Here, dark_mode is a boolean, therefore dark_mode() is invoked.

Changing @on('dark_mode', lambda x: isinstance(x, bool)) to simply on() makes the app behave correctly.

[srini-x]

I don't think it is a bug either but an improvement.

If we change @on('dark_mode', lambda x: isinstance(x, bool)) to @on(), the dark_mode toggle will not be invoked when it goes from True to False.

I am looking for a way to invoke dark_mode when it's value changes, either True to False or False to True.

[lo5]

OK. I understand this better now - thanks for the explanation.

Changing the predicate signature from f(x) to f(q, x) would be a breaking change - any existing code that relies on the first argument being a value will break.

Changing the predicate signature from f(x) to f(x, q=None) would be a non-breaking change, but I'm inclined to hold out unless this really is a problem in practice.

The example above is a corner case that cannot be handled by the @on(), since you want a global True/False toggle to operate as a route handler. The easiest (and proper?) way to mirror global front-end state would be to blindly set q.user.dark_mode = q.args.dark_mode before calling handle_on(q). I think that's more readable than trying to handle the mode change across 3 functions.

What do you think?

[lo5]

Something like this:

@app('/')
async def serve(q: Q):
    await initialize_client(q)
    if q.user.dark_mode != q.args.dark_mode:
        await dark_mode(q)
        return
    await handle_on(q)

[srini-x]

The easiest (and proper?) way to mirror global front-end state would be to blindly set q.user.dark_mode = q.args.dark_mode before calling handle_on(q). I think that's more readable than trying to handle the mode change across 3 functions.

I like that.

[lo5]

Thanks for your inputs, @srini-x! I'll leave this open for now, but deprioritize.

[padraic-shafer]

I see this as closely related to #1484 (comment) , where the app wants to know which component changed in order to update the state correctly.

---

Consider two form components, where the state of each affects the other.

• Component A is a dropdown selector; Component B is a toggle.
• When B is True, the dropdown value is constrained to a default value.
• When the user selects a non-default value from the dropdown, this sets B = False, so that A can be set to any non-default value.
• Note that B cannot be replaced by a button for the case where the user is scrolling through many datasets (via another component C, the details of which are not important here), and the state of this form needs to be consistent across these datasets.

In the current framework, both states get passed in q.args. Without comparing to the previous state, it's not possible to know which component the user most recently changed.

• A == True, B = non-default: Did the user set toggle A = True to return to the default value? Or did they select a non-default value for B, thereby intending to set A = False as a side effect?

To deal with this, I route all triggers to a central handler that then compares each arg in q.args to its counterpart in q.client to check which component was changed and therefore triggered. Then flow gets routed to the appropriate handler. It's not a big deal for a few components, but with many components in the form it is a long if-else chain that becomes more difficult to maintain. This seems like what @on/handle_on were intended to simplify.

---

The suggestion in #1484 (comment) to include the triggered component in events would simplify this considerably. The suggestion above to include q:Q in the predicate signature would enable a general version that could respond to more complex situations beyond simple changed values.

I think signaling which component was triggered (#1484 (comment)) is the simpler, more straightforward approach. I can imagine cases where adding Q in the predicate would allow more complex routing of trigger handlers, but I don't have any need for those hypothetical cases.