Adding links to namespaces has undesireable UX

[MilanPospisil]

Issue: AAH-149

https://issues.redhat.com/browse/AAH-149

This issue is to track some follow-on work from AAH-18. Now at least adding links works mostly as expected, but there is still some undesirable behavior:

Links without http:// are valid and users will enter them. "Enter a valid URL" does not make it clear what is missing, either. Schema can be added if missing.
Every time the namespace is saved, the order of the links is reversed.

[MilanPospisil]

Before:

After:

[MilanPospisil]

Note that Save button is still active (the previous functionality stays and button does not save invalid data and displays its own error message). Should it be disabled?

[himdel]

Note that Save button is still active (the previous functionality stays and button does not save invalid data and displays its own error message). Should it be disabled?

Ah, ideally yes, when a link fails UI validation, the Save button should be disabled.

Once UI validation is successful, the user can submit and then possibly get hit with the errors from the API from any other model validations.

But I think this is still missing the part where we take the errors from the server response, and show them by the right field as well. Let me know if you need any help with that, I think the error splitting logic is implemented in mapErrorMessages, hopefully it's also compatible with how this form is written :)

[MilanPospisil]

Note that Save button is still active (the previous functionality stays and button does not save invalid data and displays its own error message). Should it be disabled?

Ah, ideally yes, when a link fails UI validation, the Save button should be disabled.

Once UI validation is successful, the user can submit and then possibly get hit with the errors from the API from any other model validations.

But I think this is still missing the part where we take the errors from the server response, and show them by the right field as well. Let me know if you need any help with that, I think the error splitting logic is implemented in mapErrorMessages, hopefully it's also compatible with how this form is written :)

The server does not return errors that could be easily assigned to particular items. For example for 3 invalid items (2 blank names and 3 invalid urls) it says:

0: {status: "400", code: "invalid", title: "Invalid input.", detail: "'asdsadads' is not a valid url.",…}
code: "invalid"
detail: "'asdsadads' is not a valid url."
source: {parameter: "links__url"}
status: "400"
title: "Invalid input."
1: {status: "400", code: "blank", title: "Invalid input.", detail: "This field may not be blank.",…}
code: "blank"
detail: "This field may not be blank."
source: {parameter: "links__name"}
status: "400"
title: "Invalid input."
2: {status: "400", code: "invalid", title: "Invalid input.",…}
code: "invalid"
detail: "'asddasdasb**-ui/pull/1490/files' is not a valid url."
source: {parameter: "links__url"}
status: "400"
title: "Invalid input."
3: {status: "400", code: "blank", title: "Invalid input.", detail: "This field may not be blank.",…}
code: "blank"
detail: "This field may not be blank."
source: {parameter: "links__name"}
status: "400"
title: "Invalid input."
4: {status: "400", code: "invalid", title: "Invalid input.", detail: "'gfhfhf' is not a valid url.",…}
code: "invalid"
detail: "'gfhfhf' is not a valid url."
source: {parameter: "links__url"}
status: "400"
title: "Invalid input."

I suggest that better way is to do good client side validation instead. What do you think?

[MilanPospisil]

Another thing. Company name field has IsRequired in the textinput, but no error message is shown when it is empty, nor the server returns any errors. The IsRequired should not be here i guess?

[himdel]

The server does not return errors that could be easily assigned to particular items...
I suggest that better way is to do good client side validation instead.

Well we still need to show server errors when they do happen, but agreed we can't really show server errors together with the right item, and clients side validation is enough to make it nice :)

[himdel]

Another thing. Company name field has IsRequired in the textinput, but no error message is shown when it is empty, nor the server returns any errors. The IsRequired should not be here i guess?

Yeah agreed, I'm pretty sure the company name is not required now. There used to be an isRequired on the corresponding FormGroup before #50 but I'm not sure this was ever required. (And it seems the TextInput isRequired prop only really sets the <input required> prop, which is ignored in <form novalidate> (html forms vs react forms).)

So, harmless, but feel free to remove it, I agree it doesn't belong there :)

[himdel]

Thanks, the previous issue is fixed, just one more detail :) ...

the width of the right input apparently depends on the presence/length of the message under it, and maybe the validation state of the left input... we should keep a consistent 50:50 layout

(if it helps, , 4 lines are off, 2 to the left, 2 to the right)