Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mofojed opened this issue Sep 18, 2024 · 5 comments
Open
Labels
bug Something isn't working
Milestone

Comments

@mofojed
Copy link
Member

mofojed commented Sep 18, 2024

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()
  1. 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

Screencast.from.2024-09-18.16-19-03.mp4

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

@mofojed mofojed added bug Something isn't working triage labels Sep 18, 2024
@dsmmcken
Copy link
Contributor

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

@dsmmcken
Copy link
Contributor

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
Copy link
Member Author

mofojed commented Sep 18, 2024

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
Copy link
Contributor

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
Copy link
Collaborator

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

@vbabich vbabich added this to the October 2024 milestone Sep 24, 2024
@vbabich vbabich removed the triage label Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants