Fix fieldSubmit not calling validate method #509
Conversation
|
@qduc Thanks a lot for this very good PR ! |
| let handleErrors = errors => { | ||
| if (!isEmpty(errors) && isFunction(this.schema.onValidationError)) { | ||
| this.schema.onValidationError(this.model, this.schema, errors, $event); | ||
| if ((validateAsync && !isEmpty(errors)) || (!validateAsync && !errors)) { |
There was a problem hiding this comment.
Why the addition of this extra conditional? Haven’t had my coffee yet, but it seems to be redundant?
There was a problem hiding this comment.
Because at this point, we still don't know if this form is valid or not.
If you mean there is already a check here
if (errors && isFunction(errors.then))
Because of the isFunction(errors.then) condition, a non-async valid result still go to the else clause and calls handleErrors function
|
That logic was intentional, iirc, to make it backwards compatible with how non-async validation worked in the past. Valid or not, the submit callback was always called previously. The newer async code changed this, and to be backwards compat ... you have to enable async to get onValidationError enabled as well. This may introduce breaking changes to code that hasn’t updated to enable async validation (v3 should force async validation, so all validate calls return a promise). |
|
The reason I changed this logic is because I think it is unexpected to enable validateBeforeSubmit but the form will call onSubmit anyway, whether it is valid or not. So we let the user decide if they still want to submit or not but we don't provide a way for them to retrieve the validation result or an error array. As far as I can see, onSubmit's arguments doesn't have that data. I'm sorry that I make this change without discussion first, if you think this is intentional and should be kept that way I'll make another PR to revert this. |
|
@qduc your logic is correct, and I agree with it ... but I'm just not sure if it should be added to the v2 branch. @lionel-bijaoui do you have any thoughts on this? My concern is backwards compatibility... perhaps we can curtail any issues there with a more elaborate update in the CHANGELOG, and a note in the README? |
|
If this is really breaking thing, I will revert this commit. |
|
I don’t think it’s “breaking”, but we need to clearly state the change in the release logs |
|
Ok, sorry again |
…o v3 * 'v3' of github.com:vue-generators/vue-form-generator: Bump version to 3.0.0-beta.7 - Update dependencies - Export dist files Single export - No more core vs full since both are the same right now - Optionnal fields will be removed from it in future update Clean useless folders Update submodule dev Custom legend block for group - Possibility to use scoped-slot to customise fully how legend is build - Expose `group` and `groupLegend` in slot prop vue-generators#509 vue-generators#522 vue-generators#521 vue-generators#501 Fix deep property path not working
Bug fix
Submit button does not call form validator #506
Beside the bug fix, schema.onValidationError should be optional.
Previous behaviour, when validation failed, onSubmit still get called if onValidationError is not exists.
No