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

Validation of potential values still takes place when changes are cancelled #3381

Closed
jdmcnair opened this Issue Apr 7, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@jdmcnair
Copy link
Contributor

jdmcnair commented Apr 7, 2016

When changes are cancelled inside beforeChange validation still takes place based on those cancelled values.

Here's a jsfiddle: http://jsfiddle.net/kpccz12k/1/

In that jsfiddle, if you copy the cells for the top table and paste them into the second table, you'll see that the values do not change because we return false; from beforeChange, but the validation highlighting still shows up based on the invalid values of the cancelled changes.

Validation based on the new values should be prevented if the changes are cancelled.

@AMBudnik

This comment has been minimized.

Copy link
Contributor

AMBudnik commented Apr 7, 2016

Confirmed. Thanks for sharing @jdmcnair

@jdmcnair

This comment has been minimized.

Copy link
Contributor Author

jdmcnair commented Apr 13, 2016

Just opened a PR to fix this issue, including tests.

@AMBudnik

This comment has been minimized.

Copy link
Contributor

AMBudnik commented Apr 15, 2016

Hi,
I think that this issue is related: #3402
If we're performing validation before beforeChange is made then beforeChange can't block the validation.

@jdmcnair

This comment has been minimized.

Copy link
Contributor Author

jdmcnair commented Apr 15, 2016

@AMBudnik I agree, the two seem related. That's exactly what I changed in my PR. I took the beforeChange routine and moved it from after where the asynchronous validation is called to before validation begins, so that the validation only occurs on changes that were confirmed in the beforeChange hook.

@jdmcnair

This comment has been minimized.

Copy link
Contributor Author

jdmcnair commented Apr 15, 2016

@AMBudnik to add more detail on the negative effects of this, we're cancelling a large multi-row paste in one table and instead triggering that paste into a second table for editing/correction before incorporating the data back into the first. This bug means that validation of these cancelled values does take place in the original table, marking tons of cells as invalid when nothing has changed. If the paste is larger than the original table rows are even added, apparently to give these false validations a place to land. It's also effecting the performance of opening up the second table, because the full validation routine runs once (incorrectly), then we have to re-validate to clear the bad validation. That's two full runs through validation before we can even open the second table, which has its own validations. We've got some pretty complex custom validations happening, so this triplicate validation for a paste is really taking a toll.

@AMBudnik

This comment has been minimized.

Copy link
Contributor

AMBudnik commented Feb 1, 2018

The issue still present. Proposed fix at #4509

@AMBudnik

This comment has been minimized.

Copy link
Contributor

AMBudnik commented Mar 6, 2019

I am so happy to finally close this issue as done/added. Sorry for keeping you waiting @jdmcnair

I hope that Handsontable 7.0.0 will serve you well.

@AMBudnik AMBudnik closed this Mar 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.