Skip to content

Using "remove" on an array row is not making form dirty #56

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

Closed
supersime opened this issue Nov 20, 2018 · 7 comments
Closed

Using "remove" on an array row is not making form dirty #56

supersime opened this issue Nov 20, 2018 · 7 comments

Comments

@supersime
Copy link

supersime commented Nov 20, 2018

Are you submitting a bug report or a feature request?

bug report

What is the current behavior?

Have a form that has a FieldArray in it. The form has initialValues set for the array.
If you use field.remove(index) to remove a row from the FieldArray, it does not make the parent form dirty.
This is an issue where the disabled of the submit button on the form is tied to "pristine".

The final form documentation at https://github.com/final-form/react-final-form-arrays#simple-example has a link to a codesandbox at https://codesandbox.io/s/kx8qv67nk5.
In this example, if you insert the following in the <Form> tag:
initialValues={{ company: 'MinuteMe.com', customers: [ { firstName: 'Simon', lastName: 'Steele' } ] }}
You will see the form Submit button is immediately disabled - form is pristine.
Delete the Customer row, and form becomes dirty - Submit button is enabled.

NOW... This sandbox uses old versions of the libraries.

I am using newer versions, per this sandbox https://codesandbox.io/s/p34l93w81j. NOTE this sandbox already has my initialValues entered.

See that the same outcome is not achieved by deleting the row! The form is still dirty even though the row was removed.

What is the expected behavior?

Expected behaviour is that the form should be marked 'dirty' when the array row is removed.

Sandbox Link

See above.

What's your environment?

The version that fails (per sandbox link above):
"react-final-form-arrays": "1.1.0",
"react-final-form": "3.6.7",
"react-dom": "16.6.0",
"react": "16.6.0",
"final-form-arrays": "1.1.0",
"final-form": "4.10.0"

@PerfectPixel
Copy link
Contributor

This is a bug / regression / breaking change that was introduced by #40. Tests will not show it as they only test what happens with the FieldArray if a field within it was changed. However, if you just change the length of the array (pop, push, etc.), it will stay pristine.

The culprit is this line: https://github.com/final-form/react-final-form-arrays/blob/master/src/FieldArray.js#L72

If you do not pass in your custom isEquals method, then it defaults to returning true. It should not have a default, but rather not pass the function to final-form and then let final-form default to === which makes a lot more sense. Or a combined length and identity check on the array and the children. Just not skipping the comparison. What do you think @erikras ?

On a site note: using the mutators never marks the FieldArray as touched.

@ncphillips
Copy link

ncphillips commented Dec 10, 2018

We just confirmed this issue, and implemented isEquals as the combination length/identity check.

I think it would be better if FieldsArray automatically took care of the length check, and the props version of isEquals was to compare items, and defaulted to identity comparison.

FieldsArray

isEquals(a, b) {
  if (a.length != b.length) return false

  let isEquals = this.props.isEquals || ((a, b) => a === b)

  for (let i=0; i < a.length; i++) {
    if (isEquals(a[i], b[i])) return false
  }
  return true
}

@shun-liang
Copy link

Quoting @ncphillips

...it would be better if FieldsArray automatically took care of the length check, and the props version of isEquals was to compare items, and defaulted to identity comparison.

May I ask if @erikras agrees too? Is this considered a bug, and will it ever be fixed? Many thanks.

@VincentCharpentier
Copy link

A fix for this bug would be really awesome 🙏

@PerfectPixel
Copy link
Contributor

@VincentCharpentier Depending on your use-case this should be fixed in the newer version.

@erikras
Copy link
Member

erikras commented Jul 10, 2019

Yeah, I think this is fixed now. The defaultIsEqual definitely includes a length check.

@erikras erikras closed this as completed Jul 10, 2019
@VincentCharpentier
Copy link

I can confirm this is fixed with these versions:

"final-form": "4.16.1",
"final-form-arrays": "1.1.2",
"react-final-form": "6.3.0",
"react-final-form-arrays": "3.1.0",

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

No branches or pull requests

6 participants