ValidationMessages map value should be a type of a nullable string

[naamapps]

Hey, In the ValidationMessages map the key and the value are not nullable, I think the value should be nullable because what if you don't want to show a message when the error is present. Before null safety, I put null in this map when I didn't want the message to show when a specific error is present, but now if I put an empty string - there is still a gap below the textfield (it tries to show a message but the string is empty).

I tried using the showErrors function and showed errors conditionally when the specific error is not there. But this solution hides all errors, so if I have 2 errors in the form control and I want to hide one of them, all of the errors are hidden. I expect that the error with a validation message would appear and the one that have a null validation message will be hidden.

Please consider changing the value to a nullable string. Thanks

[kuhnroyal]

What about not adding the key when there is no error. I think that is the intention.

[naamapps]

@kuhnroyal Not adding the key show a default validation message with the name of the error

[kuhnroyal]

Ah, I understand what you mean now. Makes sense.

[vasilich6107]

@naamapps could you describe your flow more clear? It seems that it will be better for you to write your own validator(rather than stacking) for this particular case

[naamapps]

@vasilich6107 What do you mean by stacking? There is no flow really, just wanted to point out that before null safety I could set an error message to null in order to not show it in the UI. Currently the map required a non nullable string. If the message string is null, I expect it to be hidden in the ui when the error is present in the form control.

[vasilich6107]

@naamapps it is hard to me to understand your issues without code.

[naamapps]

@vasilich6107 I have this piece of code:

ReactiveTextField(
     validationMessages: (control) => {
                          ValidationMessage.required:
                              'password.validation.empty'.tr(),
                          ValidationMessage.minLength:
                              'password.validation.invalid'.tr(),
                          ValidationMessage.maxLength: null // I want this to be hidden!
                        },
),

This is not compiling because the value must be non nullable.

Why do I want it to be null? Because I want this specific message to be hidden from the UI.

Why don't I just not put the error in the map? Because if the error is not present in the map, the error will be shown with a default message (the error key name).

What is my proposal? To make the map value nullable like so: Map<String, String?>

Hope this is clearer. Thanks

[vasilich6107]

@naamapps do not hide message in validationMessages, you should avoid adding it in validator function

[naamapps]

@vasilich6107 But I do want to validate the control and mark it as invalid if there is an error. I just don't want to show the error message. If I don't put the validation I miss the functionality of this package.

[vasilich6107]

@naamapps so you are probably having something like this

FormControl(
    validators: [
        required,
        minLength,
        maxLength,
    ]
)

in your case it will be better either to alter the list of validators or to write your custom validator which will include custom logic of displaying the errors

[vasilich6107]

@vasilich6107 But I do want to validate the control and mark it as invalid if there is an error. I just don't want to show the error message. If I don't put the validation I miss the functionality of this package.

This is why it is hard to understand what you are trying to achieve) What is the reason behind not showing the error message when error present?

[vasilich6107]

@naamapps could you clarify what issues are you having with showErrors function? It theoretically seems like valid way to fix your issue

[naamapps]

@vasilich6107 Even if I write a custom validator, the package will automatically add an error message. This package had this feature before null-safety, I just want it to be the same as before.

I don't want to show the message because I write it at the top of the screen as a reminder, so I don't want to show it twice.

The showErrors function hides all errors if it returns true, I just want to hide a specific error.

[vasilich6107]

@naamapps

This package had this feature before null-safety, I just want it to be the same as before.

I'm not sure that this wasn't a bug ;-)

You are getting access to control inside showErrors you can look inside control.errors object and determine what the current error is and then decide whether to show it or not(it is theoretical thoughts I did not check)

[naamapps]

Haha could be, but it is a good feature :)

This is what I write in the showErrors:

showErrors: (x) =>
                            x.invalid &&
                            x.touched &&
                            !x.hasError(ValidationMessage.maxLength),

If there are two errors in the control, it will hide them both - not the desired functionality.

[vasilich6107]

@naamapps I suppose you can improve you showErrors function with more complex logic of detecting existing errors and their amount

[naamapps]

@vasilich6107 You are right but how will the package know which error to show? If there are two errors in the control I need to return true in the showErrors function, and it will show the message I want to hide..

[vasilich6107]

@naamapps the package shows one error at a time, so it is either first in list or last.

[naamapps]

@vasilich6107 I guess. Is there a way to set the order of the validators? I feel it will be a hacky solution to the problem. Is there a specific reason you don't want to change the validationMessages map to nullable string? In my opinion it is so much simpler and intuitive to the developer.

[vasilich6107]

Is there a specific reason you don't want to change the validationMessages map to nullable string?

I'm not a maintainer) You can make PR with this improvement the same way as I can do it. As far as dev iterations of this package has slowed down it will be faster to find a way to fix your issue with existing features)

Is there a way to set the order of the validators?

Not sure that I understand why do you need this.

In my opinion it is so much simpler and intuitive to the developer.

Simple solution is not always the right one)

[naamapps]

@vasilich6107 Okay so I think I'll try to fix it and I hope the maintainer will be be able to see this soon. Thanks for your help 👍

[vasilich6107]

Let's tag him(@joanpablo) to be sure ;-)

[joanpablo]

Hi @kuhnroyal @vasilich6107

What if we modify the method showErrors to something like:

showError: (control, errorName) =>
                            control.invalid &&
                            control.touched &&
                            errorName != ValidationMessage.maxLength,

so you will decide for each error if you should show it or not.

looking forward to hear from you

[kuhnroyal]

That sounds good, but also think that the message in the map should be nullable for the easy use cases.

[joanpablo]

@kuhnroyal @vasilich6107 @naamapps

Yes, I understand that the easy way could be just to define the method to return a String? instead of a String.

But what if the developer makes a mistake and is setting the error message to null by accident and not intentionally? The program will run successfully but the error message will never show up, the compiler will not alert the developer.

What do you think about this?

[naamapps]

As you said, there are 2 ways to look at this. In my perspective, this should be considered a feature and not a bug. Null means non-existent, so when an error message is null, the way I interpret it, there is no error message for the specific error key - which means the UI won't have a message to show when the error exists on the form control.

I, as a developer, always tests the UI manually when developing something (and I would like to believe that others do it as well). It shouldn't be an issue to see that the error message doesn't appear if it's on accident.

[kuhnroyal]

But what if the developer makes a mistake and is setting the error message to null by accident and not intentionally? The program will run successfully but the error message will never show up, the compiler will not alert the developer.

What do you think about this?

I don't see this as an issue. null is not evil, it makes sense to be used here. I mean what if the developer forgets to add the custom validator key to the map, the warning won't show up and the compiler will not alert the developer.

[joanpablo]

Thanks, @naamapps for your quick response, I got your point.

It is just that I'm wondering if it is the right decision or not. I know that in an ideal world each developer should implement a UI test for testing all error messages.

My concern is that we know that a lot of developer don't implement to many tests, at least not to Forms. Many times developers test only manually once, but if the error comes later because something is broken, almost nobody goes again through all the Forms of the application testing manually the error messages, so the error will go unnoticed.

I know that this solution:

showError: (control, errorName) =>
                            control.invalid &&
                            control.touched &&
                            errorName != ValidationMessage.maxLength,

is more verbose, and I am even not sure about the syntax, to be honest, but is an explicit decision of the developer to show or not the error. This use case is not common so if the developer wants to handle it manually it is an explicit implementation, with no margin for misunderstanding or collateral errors.

But I still find myself in this dilemma.

[joanpablo]

I'm also working right now on a Global Settings for ReactiveForm in which you can define ValidationMessages for all Reactive Widgets globally, at ReactiveForm level but also at MainApplication level.

Right now I only have included the validationMessages but maybe we can include the showError as well.

Another question I would like to do to @kuhnroyal @vasilich6107 @naamapps is the next one:

In my prototype implementation of global validations messages I'm planning to change this current syntax:

ReactiveTextField(
   ...
   validationMessages: (control) => {
       ValidationMessages.required: 'Mandatory field'
   }
)

to this one:

ReactiveTextField(
    ...
    validationMessages: {
        ValidationMessages.required: (error) => 'Mandatory field'
    }
)

Notice that I'm not passing the control as an argument, but only the error object.

[joanpablo]

But what if the developer makes a mistake and is setting the error message to null by accident and not intentionally? The program will run successfully but the error message will never show up, the compiler will not alert the developer. What do you think about this?

I don't see this as an issue. null is not evil, it makes sense to be used here. I mean what if the developer forgets to add the custom validator key to the map, the warning won't show up and the compiler will not alert the developer.

If the developer forgets to add the Validator key to the map, the UI Widget will still be showing something (the error key), maybe not a human (simple user) readable text, but at least something.