Is 'utils' belongs to this library ?

lenra-io / lenra_components

MIT License

4 stars 0 forks source link

Is 'utils' belongs to this library ? #7

Open Nesqwik opened 3 years ago

Nesqwik commented 3 years ago

There is a folder 'utils' that give some helpers to :

parse a color
validate textfield input (password/email)

The color parser is not used by the library. It is just exposed as part of the lib. The validator is partially used by the lenra_text_form_field.dart component to check the value of the input based of the type parameter (email or password).

I think we can safely remove the color parser but i'm not sure about the validator :

Is it our job to check the value of the input ? If not, we should remove the validator.
If it is our job, we should provide a way for the user to completely override the validator (not just add on top of the existing validator).
Should we add more types in the futur ? (phone number, address...)

@lenra-io/lenra any thought ?

jonas-martinez commented 3 years ago

I too think that we can safely remove the color parser. For the form validators I think that it should not be here in its actual state for the following reason : The validation strictly depends on the user's needs, for example I don't think that everyone wants to have the same password validator as ours. We could still give some generic validators to compose bigger ones such as "checkLength" or "checkNotEmpty", what do you think?

pichoemr commented 3 years ago

I also think we can remove ColorParser. For form validators I agree with Jonas, we can keep the generic validators like checkLength/checkNotEmpty/checkGitFormat? and use the LenraTextFormFieldTypes for our validators, for example we can move checkPassword and checkEmail directly in our email and password validator.

Nesqwik commented 3 years ago

Imo there is too much issue with the validators :

It is highly opinionated (at least for the "password" check)
There is no 100% accurate regex to check the email
The usage is also opinionated (see the base usage)
We must provide translations for error message AND a way for third part user to override them.

I think it is out of the scope for this library.

@taorepoara what is your opinion ?

taorepoara commented 3 years ago

I think that we should re-think the validator management in order to manage validation level (success, info, warning, error). We'd rather manage only basic validators (length and regex) and permit the users to implement their own validators with the same base usage as Flutter do.

We can also let the users manage the validator results to build a custom UI by defining a callback.