Skip to content

Conversation

@ahmedhamidawan
Copy link
Member

@ahmedhamidawan ahmedhamidawan commented Nov 15, 2025

We have a bug where in the workflow editor, while typing in a text form element, the value can get unexpectedly trimmed or reset because of the build_module API call updating the values. Using a combination of blur and focus to prevent prop values from replacing currently typed values is a solution to this.

Fixes #21205

Before 🔴
debounce_form_element_text_input_BEFORE.mp4
After 🟢 (out of date: no longer using a debounce)
debounce_form_element_text_input_AFTER.mp4

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@ahmedhamidawan ahmedhamidawan added this to the 25.1 milestone Nov 15, 2025
@ahmedhamidawan ahmedhamidawan changed the title [25.1] Add a debounce to text form element to prevent value resetting [25.1] Add a debounce to text form elements to prevent values from resetting Nov 15, 2025
@mvdbeek
Copy link
Member

mvdbeek commented Nov 17, 2025

The test errors look legitimate to me. Maybe debounce is not the right thing? it seems that other fields are fine, is that maybe because LastQueue is used to set those values ?

@ahmedhamidawan ahmedhamidawan force-pushed the debounce_form_element_text_input branch from ced3fd1 to f4528ed Compare November 22, 2025 22:36
@ahmedhamidawan
Copy link
Member Author

I tried a slightly different approach thought it is still via a debounce. Seleniums are still failing (though fewer fails than earlier) and possibly related? Will investigate the fails to confirm.

@mvdbeek mvdbeek marked this pull request as draft December 3, 2025 16:01
@mvdbeek
Copy link
Member

mvdbeek commented Dec 3, 2025

Yes, likely still related

@ahmedhamidawan ahmedhamidawan force-pushed the debounce_form_element_text_input branch from f4528ed to b396761 Compare December 6, 2025 04:03
@ahmedhamidawan ahmedhamidawan marked this pull request as ready for review December 6, 2025 06:31
@ahmedhamidawan ahmedhamidawan changed the title [25.1] Add a debounce to text form elements to prevent values from resetting [25.1] Use focus/blur to prevent prop updates from changing FormText values while typing Dec 6, 2025
@ahmedhamidawan
Copy link
Member Author

Used a strategy that seems to work well!: Used a combination of blur and focus to prevent prop values from replacing currently typed values (if currently in the focus state, wait for the user to complete typing before replacing with the prop value coming from the backend -> which is eventually replaced by the final typed value if needed).

@mvdbeek
Copy link
Member

mvdbeek commented Dec 6, 2025

Have you tried artificially returning values out of order ? I think this is the real issue in production ? I'm not seeing logic that would handle that ? That's what LastQueue does

},
onBlur() {
// When field loses focus, sync with the latest prop value
this.isFocused = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems backwards, it's cool that things update in the editor as you type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things will still update as you type in the editor though, it's just that if for some reason the getModule update tries to change a FormText field's value while the user types, this prevents that (so it's the other way around?).

Co-authored-by: Marius van den Beek <m.vandenbeek@gmail.com>
@ahmedhamidawan ahmedhamidawan force-pushed the debounce_form_element_text_input branch from 5d68d86 to 2fe3c33 Compare December 6, 2025 15:17
@ahmedhamidawan
Copy link
Member Author

Have you tried artificially returning values out of order ? I think this is the real issue in production ? I'm not seeing logic that would handle that ? That's what LastQueue does

But aren't we already using LastQueue here:

onSetData(stepId, newData) {
this.lastQueue
.enqueue(() => getModule(newData, stepId, this.stateStore.setLoadingState))
.then((data) => {
const partialStep = {
content_id: data.content_id,
inputs: data.inputs,
outputs: data.outputs,
config_form: data.config_form,
tool_state: data.tool_state,
tool_version: data.tool_version,
errors: data.errors,
};
this.stepActions.updateStep(stepId, partialStep);
});

... which this propagates all the way up to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants