Call validation before submitting the order #4561
Conversation
senadir
added
type: bug
|
Size Change: +30.2 kB (+3%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
@nerrad what are your concerns on having |
I don't think we should have business logic inside the input component, especially if we are looking at exporting them at some point. Because there is no event we could catch on autofill, I'm thinking we could we have something like a |
I agree with Raluca here. If there's need to force revalidation via some logic, it makes sense to abstract the logic outside of the input and use props to trigger that re-validation. |
|
I agree that we should have some sort of external trigger that calls validation, but having a prop that forces validation doesn't make sense to me. I might be missing something, but do we have other examples or precedents of using such pattern? IMHO, we should probably be rethinking of how we do validation to make it more accessible. We store validation errors outside components yet can decide if we can delete them inside the component, which is already a lot of coupling. Libraries like formik have the validation schema (function) that is accessible from the from level. This allows you to run validation inside the component reacting to certain events like onChange or onBlur, and also outside the component on the form level on demand or at certain lifecycle events. Such workaround should be currently acceptable until we find a better refactor. Having an extra I did attempt to add that prop to the component and it caused significant prop drilling since inputs are really deep into the tree. |
|
@senadir if we could move the logic at the form level it would be great. I agree that the input should just receive info about the state it should render (error, valid, etc) and the form is the place where the state should be available. |
mikejolley
added
the
status: blocker
Remove extra validation call after refactoring the valida...Remove extra validation call after refactoring the validation system. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/17cd6699b9cc780e7c50b136704627836901091d/assets/js/base/components/text-input/validated-text-input.tsx#L121-L130🚀 This comment was generated by the automations bot based on a
|
This PR fixes the autofill issue, see #4495 for what's happening.
TL;DR, we only run validation on mount and blur, autoFill doesn't trigger blur, meaning we didn't run any validation. Additionally, all inputs mount with an error if empty, meaning when we submit an order, errors aren't cleared.
This PR runs validation on
isBeforeProcessing.Fixes #4495
How to test the changes in this Pull Request:
Changelog