Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling validation #100

Closed
lonix1 opened this issue Mar 11, 2024 · 6 comments · Fixed by #110
Closed

Disabling validation #100

lonix1 opened this issue Mar 11, 2024 · 6 comments · Fixed by #110

Comments

@lonix1
Copy link
Contributor

lonix1 commented Mar 11, 2024

For diagnostic and/or testing purposes we needed to disable the library once it's already loaded.

I tried this:

v.remove(v.options.root)

And this:

document.querySelectorAll('.my-form input:not([type="hidden"])').forEach(x => v.removeInput(x));

Both complete without error, but when I then interact with the form, and validation is expected to run, I get this console error:

Uncaught TypeError: i is not a function
  a http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  setTimeout handler*i http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  addInput http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  scanInputs http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  scan http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  a http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  bootstrap http://localhost:5000/lib/aspnet-client-validation/dist/aspnet-validation.min.js:1
  <anonymous> http://localhost:5000/signin:355

Are remove and removeInput intended to be usable by callers (in which case have I made a mistake in how I'm using them)? If not, is there some other way to disable the library once it's loaded, or is that impossible (and the only option is not to run it in the first place)?

@dahlbyk
Copy link
Collaborator

dahlbyk commented Mar 19, 2024

Are remove and removeInput intended to be usable by callers?

I'd say yes.

in which case have I made a mistake in how I'm using them

I don't think so. remove() just hasn't been thoroughly tested.


I'm pretty sure i is not a function is because we don't check if validate exists here, which is easy to fix:

let cb = (e: Event, callback?: ValidatedCallback) => {
let validate = this.validators[uid];
clearTimeout(debounceTimeoutID);
debounceTimeoutID = setTimeout(() => {
validate()

Better would be to actually remove that callback (trickier because we need to keep track of which event type it's listening to):

let validateEvent = input.dataset.valEvent;
if (validateEvent) {
input.addEventListener(validateEvent, cb);
}
else {
let eventType = input instanceof HTMLSelectElement ? 'change' : 'input';
input.addEventListener(eventType, cb);
}

We could also remove the form event listeners for forms without any remaining inputs:

form.addEventListener('submit', cb);
form.addEventListener('reset', e => {

Would you also expect existing validation messages/summaries to be cleared? Or just leave them and allow submitting the form, as would be the normal experience with server-only validation?

@lonix1
Copy link
Contributor Author

lonix1 commented Mar 19, 2024

I just wanted to "disconnect" a form from the library. As you put it, to get "the normal experience with server-only validation".

It's a good question though, I'm unsure what other people would expect.

Practically though, if someone goes to the trouble of disabling validation for a form, it's reasonable to expect that it would be "reset" to it's original state, i.e. no existing messages/summaries.

@dahlbyk
Copy link
Collaborator

dahlbyk commented Mar 19, 2024

it's reasonable to expect that it would be "reset" to it's original state, i.e. no existing messages/summaries.

But its original state might have been server-side errors. 🤔

I think most straightforward would be for remove() to not reset validation messages, but also add a public method to explicitly removeMessages() that would clear all summaries and individual messages. If you want a full reset you would:

v.removeMessages();
v.remove();

(IMO a small but useful change to allow scan(), remove(), etc to be called without root, defaulting to this.options.root.)

@lonix1
Copy link
Contributor Author

lonix1 commented Mar 19, 2024

But its original state might have been server-side errors.

Oops, I missed that scenario, you are right! 😊

@dahlbyk
Copy link
Collaborator

dahlbyk commented Apr 2, 2024

I've started working on this. Will open a PR soon.

@dahlbyk
Copy link
Collaborator

dahlbyk commented Apr 26, 2024

"soon" #110

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

Successfully merging a pull request may close this issue.

2 participants