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

[@mantine/form] useForm: fix isDirty for list fields #3025

Merged
merged 2 commits into from Dec 4, 2022

Conversation

gdostie
Copy link
Contributor

@gdostie gdostie commented Nov 21, 2022

By computing the isDirty only on demand using deep comparison of the values state, it removes many bugs related to isDirty and nested fields. Manually managing a dirty state parallel to the values state was very much bug prone, considering all the possible updates to form values that can occur.

Disclaimer: by doing this we loose 2 functionalities initialDirty and setDirty. IMHO the pros outweighs the cons, but I am curious to see what the community thinks. Fixed ✅

Fixes: #3001 and #2937

@gdostie gdostie marked this pull request as draft November 21, 2022 18:08
@gdostie
Copy link
Contributor Author

gdostie commented Nov 21, 2022

Converting to draft, will try to retain initialDirty and setDirty

@gdostie gdostie force-pushed the dirty-state-simplification branch 3 times, most recently from 5f83932 to 0d7bf6a Compare November 21, 2022 22:26
@gdostie
Copy link
Contributor Author

gdostie commented Nov 21, 2022

I managed to retain setDirty and initialDirty functionalities. 🎉

I renamed them in the code as "manual overrides" of the standard dirty behaviour.

Reopening for review.

@gdostie gdostie marked this pull request as ready for review November 21, 2022 22:46
Simplify the dirty state functionalities by computing the isDirty
only on demand using deep comparison of values. Manually
managing a dirty state parallel to the values state was very much
bug prone.
@gdostie gdostie changed the title [@mantine/form] useForm: remove dirty state [@mantine/form] useForm: fix isDirty for list fields Nov 21, 2022
@rtivital rtivital changed the base branch from dev to master December 4, 2022 12:03
@rtivital rtivital merged commit fd56c7d into mantinedev:master Dec 4, 2022
@rtivital
Copy link
Member

rtivital commented Dec 4, 2022

Thanks!

@gdostie gdostie deleted the dirty-state-simplification branch December 5, 2022 14:49
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.

form.insertListItem does not make the form dirty
2 participants