Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Running (custom) validators should also execute setCustomValidity on the element #16353

Open
1 of 3 tasks
dietergeerts opened this issue Nov 30, 2017 · 5 comments
Open
1 of 3 tasks

Comments

@dietergeerts
Copy link

I'm submitting a ...

  • bug report
  • feature request
  • other

Current behavior:
Right now, when validators run, they only set the AngularJS validity, and not the HTML validtity. Because of this, we can't use the :invalid pseudo selector, which is used by Bootstrap 4 to display the validation styling.

Expected / new behavior:
Running validators should also set the the HTML validity in order to use :invalid pseudo selector.

AngularJS version: 1.5.11 (I checked source code of the latest version on master too)

Browser: [al]

Anything else:
I don't know if PR can still be made on this. If so, I could check it out to create one.

@gkalpak
Copy link
Member

gkalpak commented Nov 30, 2017

The problem with that is that the error shows up to the user, so it is application-specific. Angular can't set it arbitrarily. Maybe we could add a way for the error to be specified by the dev.

@dietergeerts
Copy link
Author

dietergeerts commented Dec 1, 2017

Though the message is not automatically shown to the user, but being stored in the validationMessage property of the element, I agree that we should look for a way to be able to set it, as this can then be used to show the message by some other piece of code. Also, as this message will be the localized version, maybe we need to add some configuration option, that when set to some kind of function, that can be used to get the localized message? And if the option isn't set, then we don't set custom validity?

If you set novalidate on the form, then the messages will not be shown, and in combination with AngularJS, that's something you always want to do.

Now I have done some further reading on the validation api, and found out several things to be aware of:

  • You can't set the default messages for things like required attribute. This means that we need to use setCustomValidity for every constaint, even the default ones.

  • You can only have one error message, so we can do 2 things: Only show the highest priority one (how to decide still to see), or append all the messages, which could become ugly.

So if we add this as a feature, we will need to think about how to do it and how it will impact existing behavior. Though I would like to see it in there and would love to experiment with this.

I would love to hear other opinions, to why we should or should not do this, and if there are better ways etc...

@Narretz
Copy link
Contributor

Narretz commented Dec 1, 2017

In general, what benefit does calling setCustomValidity give you if you decide not to show the message? AngularJS already provides the ng-invalid etc classes that can be used to style the input.

Regarding localization - the message shown is what you pass to setCustomValidity, so you have to localize it yourself. Only the default constraints are localized.

You can't set the default messages for things like required attribute. This means that we need to use setCustomValidity for every constaint, even the default ones.

Not really sure what you mean by this.

You can only have one error message, so we can do 2 things: Only show the highest priority one (how to decide still to see), or append all the messages, which could become ugly.

The browser decides how to display the constraint validation errors. There's nothing AngularJS can do here.

Personally, I see little need for this - except if you want to show the browser's built-in error messages, which people rarely want.

@gkalpak
Copy link
Member

gkalpak commented Dec 1, 2017

Though the message is not automatically shown to the user

@dietergeerts, it is shown to the user if you don't use novalidate. And while it is common to use novalidate with AngularJS, it is neither mandatory nor the only option.

You can't set the default messages for things like required attribute. This means that we need to use setCustomValidity for every constaint, even the default ones.

I am not sure what you mean either 😕. The message (if we choose to implement this) would be tied to the validator.

You can only have one error message, so we can do 2 things: Only show the highest priority one (how to decide still to see), or append all the messages, which could become ugly.

I would stick to what browsers do here. (I haven't looked it up, but whatever they do, we should do.)

In general, what benefit does calling setCustomValidity give you if you decide not to show the message? AngularJS already provides the ng-invalid etc classes that can be used to style the input.

@Narretz, the main reason for using setCustomValidity (even if you don't care about the message) is to communicate to the browser that this is invalid (for example for applying pseudoselectors).
This may be necessary when you rely on 3rd-party frameworks that in turn rely on native features (such as the :invalid pseudoselector), in which case you can really use AngularJS-specific classes (such as ng-invalid).

In general, I think this is a nice-to-have (since it allows better integration with native browser APIs), but wouldn't go to great extends to implement it if it turns out to be tricky.
I would also explore the posibility of implementing it as a 3rd-party module first.

@dietergeerts
Copy link
Author

You can't set the default messages for things like required attribute. This means that we need to use setCustomValidity for every constaint, even the default ones.
I am not sure what you mean either 😕. The message (if we choose to implement this) would be tied to the validator.

I mean that the defaults can't be set. Required for example is being automatically checked by the browser, but it's message can't be set, so setCustomValidity must be used to set it.

It's true that when you don't use novalidate, they are being shown, so an option is needed to use it or not.

Great idea to try to have this as a 3rd-party module. I will give that a go and report here how it goes, though it could take me a while as I don't have much spare time for the moment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants