OndrejKunc / flutter_dynamic_forms

A collection of flutter and dart libraries allowing you to consume complex external forms at runtime.
MIT License
204 stars 60 forks source link

disable/enable validation #94

Closed Overman775 closed 3 years ago

Overman775 commented 3 years ago

Disable/enable validation when element is not visible.

The problem is in form.isFormValid, need a trigger that will disable/enable validation

example json

{
    "@name": "requiredValidation",
    "isVisible": {
        "expression": "@choice == \"test\""
    },
    "message": "Required!!!"
}
codecov-io commented 3 years ago

Codecov Report

Merging #94 (11288a4) into master (6b2403d) will increase coverage by 0.02%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   52.60%   52.63%   +0.02%     
==========================================
  Files         105      105              
  Lines        1823     1822       -1     
==========================================
  Hits          959      959              
+ Misses        864      863       -1     
Impacted Files Coverage Δ
...namic_forms/lib/src/form_manager/form_manager.dart 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6b2403d...11288a4. Read the comment docs.

OndrejKunc commented 3 years ago

Hi @Overman775 , Thanks for the PR! The RequiredValidation was originally just a shorthand for the Validation component with an assumption that the parent component has a property named "value" of type String and it will automatically fill isValid property with an expression that checks if this String is not empty. Looking back at the RequiredValidation design I don't think it was a good idea, because this can be confusing for many people.

I think in your case you can simply use default Validation and registerValidationParser<Validation>() in your parser list. This ValidationParser comes from dynamic_formspackages so you can use it even if you don't use the components package. So your JSON will look something like this (assuming "parentId" is the id of the parent element):

{
    "@name": "validation",
    "isValid": {
        "expression": "@choice == \"test\" && @parentId.value.length > 0"
    },
    "message": "Required!!!"
}

About your PR: I think isVisible on the Validation doesn't make sense, because Validation is not something that can be shown on the screen - this property comes from the base class FormElement and I think in the future we should come up with a different base class for those cases without isVisible property. On the other hand, I can imagine a situation where your implementation can be useful - especially when you want to turn off some validations so they won't be counted in the final formManager.isFormValid property. If we decide to remove isVisible property in the future we can create some property like isEnabled instead.

So please let me know if the solution with just a simple Validation element would solve your problem or if you still find the solution with checking the visibility useful - in that case, I don't see a reason why not to merge it.

Overman775 commented 3 years ago

@OndrejKunc thanks for answer, property isEnabled looking beter. I will wait for null-safety and help this project 😉