Skip to content

Conversation

@diegoimbert
Copy link
Contributor

No description provided.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 22, 2025

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: 692f3e8
Status: ✅  Deploy successful!
Preview URL: https://6e4e324a.windmill.pages.dev
Branch Preview URL: https://di-clear-app-forms-on-submit.windmill.pages.dev

View logs

@claude
Copy link
Contributor

claude bot commented Dec 22, 2025

Claude finished @diegoimbert's task in 1m 32s —— View job


Reviewing PR #7428: Clear app form on submit option

  • Gather context and analyze PR changes
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance and security implications
  • Provide detailed feedback

Summary

This PR adds a new configuration option clearFormInputs to the Form and Modal Form components in Windmill apps. This allows users to specify when form inputs should be cleared: never, on success, on submit, or on error.

Code Quality Assessment

Overall: Good implementation

The changes follow existing patterns in the codebase and integrate well with the existing side-effect handling mechanism in RunnableWrapper.svelte.

Positive Aspects

  1. Follows existing patterns: The clearFormInputs configuration uses the same oneOf pattern as onSuccessClick, onSubmitClick, and onErrorClick.

  2. Clean integration: The implementation hooks into the existing handleSubmitSideEffect and handleSideEffect functions, which is the appropriate place for this functionality.

  3. Consistent across components: Both AppForm.svelte and AppFormButton.svelte receive the same implementation pattern.

  4. Type safety: The clearFormInputs prop is properly typed in the Props interface.

Minor Observations

  1. RunnableWrapper.svelte:114: The clearFormInputs prop is correctly added to the destructuring but doesn't have a default value specified, which is fine since it's optional.

  2. components.ts:823-838: The clearFormInputs configuration is well-structured with appropriate tooltips and labels for the UI.

  3. Form clearing approach: Using runnableComponent?.setArgs({}) to clear the form inputs is appropriate as it resets all user inputs to empty values.

No Issues Found

  • No bugs detected
  • No security concerns
  • No performance issues
  • Code style is consistent with the rest of the codebase

Testing Instructions

To test this feature, navigate to the App Editor and add either a Form component or a Modal Form component to your app. In the component's configuration panel (right sidebar), look for a new setting called "Clear form inputs" (or similar tooltip "When to clear the form inputs"). This setting should offer four options: Never (default), On success, On submit, and On error. Configure the form to run a script, fill in some input values, and submit the form. Verify that the form inputs are cleared according to the selected option—for example, selecting "On success" should clear inputs only after a successful script execution, while "On submit" should clear them immediately when the submit button is clicked.

@rubenfiszel rubenfiszel merged commit 980dfcc into main Dec 22, 2025
13 checks passed
@rubenfiszel rubenfiszel deleted the di/clear-app-forms-on-submit branch December 22, 2025 10:40
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants