deephaven / deephaven-plugins

Deephaven Plugins
11 stars 15 forks source link

Unexpected results when pressing a button quickly after editing a field #894

Open mofojed opened 1 month ago

mofojed commented 1 month ago

Description

If you have a "submit" button on a field and press the button very quickly after modifying a field, you may not get the latest field.

Steps to reproduce

  1. Run the following:
    
    from deephaven import ui

@ui.component def ui_my_form(): value, set_value = ui.use_state('')

def handle_submit():
    print(f"Value is {value}")
    set_value('')

return [
    ui.text_field(value=value, label="Name", on_change=set_value),
    ui.button("Submit", on_press=handle_submit),
]

my_form = ui_my_form()


2. Enter a name and really quickly click the "Submit" button

**Expected results**
2. The value you've entered should be printed, and then it should reset

**Actual results**
2. A blank value gets printed, and the value doesn't reset

**Additional details and attachments**

https://github.com/user-attachments/assets/d014465f-6dab-46b9-9a4c-25b9d91f44af

**Versions**

Engine Version: 0.36.1
Web UI Version: 0.90.0
Java Version: 11.0.24
Barrage Version: 0.6.0
Browser Name: Chrome 128
OS Name: Linux
@deephaven/js-plugin-ui: 0.21.0
dsmmcken commented 1 month ago

One possible fix is flush any input that uses debounced values on blur, as the click event will be after.

dsmmcken commented 1 month ago

Note this is not form specific:

Any time you try to use the debounced value before it flushes based on another triggering event, it can be wrong.

from deephaven import ui

@ui.component
def example():
    x, set_x = ui.use_state("")

    return [
        ui.text_field(value=x, label="x", on_change=set_x),
        ui.text("Press me while still typing and again 1 second after you stop"),
        ui.button("what's the value of x?", on_press=lambda: print(x))
        # value will be wrong while still typing
    ]

ex = example()
mofojed commented 1 month ago

One possible fix is flush any input that uses debounced values on blur, as the click event will be after.

@dsmmcken Note that this may be tricky, because the "click" may be calling a previous version of the callback regardless, even if the blur event flushes the debounced events. Consider the order of operations.

from deephaven import ui

@ui.component
def ui_my_form():
    value, set_value = ui.use_state('')

    return [
        ui.text_field(value=value, label="Name", on_change=set_value),
        ui.button("Submit", on_press=lambda: print(value)),
    ]

my_form = ui_my_form()
  1. Component renders with on_change and on_press callbacks passed to the client (where on_press is lambda: print(''), since the value is '' to start)
  2. In the browser, you enter "foo" in the text field, then click "Submit". At that point, on_change("foo") is sent from the client, and then a call to on_press()
  3. Server gets the on_change and calls set_value. This causes the component to re-render, now with on_change and on_press' (where on_press' is lambda('foo'), since value is now "foo"
  4. Server gets call to on_change, which is still referencing the lambda: print('') version of on_change.
dsmmcken commented 1 month ago

Isn't the print evaluted on the server? The first thing to fire in my mind would be client triggering an on_change from the blur. Wouldn't the server process that change before responding to the on_press event?

mattrunyon commented 1 month ago

The value is bound to the previous render. It would be somewhat similar to not using callback set state in React and spam clicking a counter button.

  1. Client sends server new state
  2. Server starts processing state change
  3. Client sends server button click event
  4. Server responds to client w/ new document after value change in step 2
  5. Server handles click event, but that value is bound to the value before step 1

A potential fix to that would be to serialize and re-use callable IDs in a stable way on the server. Such that the pending callable request after step 3 calls the updated callable after step 4 (because they have the same ID). Not sure how feasible this would be though since if you had some conditional rendering then the callable creation/serialization order could change

Another edge case is what would happen if the field was supposed to validate and disable the submit button on invalid values. If you quickly typed an invalid value and then submitted you could submit w/ an invalid value. It would probably be best to validate in the submit callback anyway, but just another potential edge case