search

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

All validation except one instance is ignored if validation elements have no Id attribute #77

Closed markphillips100 closed 4 years ago

markphillips100 commented 4 years ago

I've been trying out the example in the flutter_dynamic_forms_components and stumbled upon an issue relating to validation.

The example forms in the assets folder have validation elements that do not specify id property attributes. The result of not having Ids results in the code here producing a Map with only one Validation element, whichever is the last to be yielded. This is because the key values were null values and you can't have duplicate keys in a Map.

If I provide unique id values in the xml data then all the validations are included correctly. So I guess I'd suggest to enforce Validation elements have an Id or change the FormManager constructor to accept a list of Validations rather than a Map. What do you think @OndrejKunc ?

Great library by the way.

OndrejKunc commented 4 years ago

Hi @markphillips100 , Thanks for the feedback! I am aware of this issue. Besides validation, it also causes problems when collecting data from all the inputs like a checkbox - when you forget to add an id, it doesn't really make sense to send it back to the server, because the server wouldn't be able to identify the source component.

Unfortunately, I don't know about any easy way how to enforce having an id, because we would need to throw some exception during parsing, and that's probably not wanted. Another solution might be to generate some random id when it is missing, but I think that maybe confusing, because it would be different every time. Or maybe instead of random id, it can be the unique path to the element in a tree, something like XPath, but it would not be so easy to implement.

However in case of validations, most of the time we really don't need the id, we just need to check if all validations are valid. Changing it to a list sounds like a good idea! Right now I am rewriting the way how the FormManager is built as part of #74 so I think I might also include this change there.

OndrejKunc commented 4 years ago

I added the change from map to the list via PR #78 and published it to pub.dev I hope it solves your problem.

markphillips100 commented 4 years ago

Awesome @OndrejKunc . I've upgraded to 0.13.0 and all is good. Thank you.