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

Clear errors on form reset #1568

Merged
merged 5 commits into from
Oct 25, 2023
Merged

Clear errors on form reset #1568

merged 5 commits into from
Oct 25, 2023

Conversation

reinink
Copy link
Member

@reinink reinink commented May 25, 2023

Right now when using the form helper, calling form.reset() does not clear the form errors. This feels like a bug, since if you're resetting the form to its initial state you likely also want the errors to be cleared out.

This PR updates the form helper to clear form errors when resetting the form:

// will reset all form errors
form.reset()

If you're only resetting certain fields, only those corresponding errors will be cleared:

// will only reset "name" and "email" errors
form.reset('name', 'email')

@daleweaver777
Copy link

daleweaver777 commented May 25, 2023

The Svelte version of the useForm helper actually has a few bugs in it. I believe it boils down to trying to directly read values in a Svelte store using this..

Look on line 68 of the setError function. It is directly referencing this.errors variable to get a list of the current errors to add back into the store along with any new errors. This is not the correct way to get current values from the Svelte store. You need to subscribe to the store or use the store's update function which passes in the stores current values to the callback function. In this case when you call clearErrors and then immediately setError the internal variable this.errors still contains the old errors that were cleared in the clearErrors function.

From my understanding of Svelte stores, in JS you can’t directly acccess the value. You need to subscribe to the store.

@reinink
Copy link
Member Author

reinink commented May 30, 2023

@weavdale hey thanks for this feedback? I use Vue and React on a regular basis, but I don't use Svelte often, so that's probably why I've got things wrong here 😅

Are you able to submit a PR to correct these issues? 🙏

/cc @mariojankovic @buhrmi @pedroborges

@daleweaver777
Copy link

@reinink I just PR'd a fix #1574

@reinink reinink force-pushed the clear-errors-on-reset branch from 5cd32e1 to 3e08b36 Compare October 25, 2023 01:17
@reinink reinink merged commit 74d682c into master Oct 25, 2023
6 checks passed
@reinink reinink deleted the clear-errors-on-reset branch October 25, 2023 01:25
jessarcher added a commit that referenced this pull request Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants