search

mailgun / validator-demo

Mailgun email address jquery validation plugin http://mailgun.github.io/validator-demo/
Apache License 2.0
257 stars 80 forks source link

Performance and Compatibility fixes #4

Closed msigley closed 9 years ago

msigley commented 9 years ago

This PR adds lot of script compatibility and overall performance and extensiblity improvements to the plugin. Particular attention has been given to eliminate and prevent useless and unnecessary AJAX API requests.

hozza commented 9 years ago

Seriously think the mailgun JS needs to be updated, I haven’t yet tested msigley's fork but it does look the best... I also like the idea of cache validation results. @jeremyschlatter what do you think?

msigley commented 9 years ago

@hozza I am glad I am not the only one that saw this issue. I run this JS live on my website's and have been since I submitted this PR. I do have a minor fix for the event timestamps I'll attach to this PR as well.

hozza commented 9 years ago

I'm extending the jquery validate plugin's email method with this mailgun validator.

So far, although @msigley code works well as a stand-alone, it does not play well with jquery validate - specifically the element based flood prevention, I feel @eliottwiener approach to 'flood prevention' with the result caching in an object is a lot more robust especially when extending other validators. Should save mailguns servers some unnecessary load.

eddywashere commented 9 years ago

I think I noticed some minor tab/spacing issues that might be caused by missing brackets. If that's the case, mind adding brackets to keep it consistent with the rest of the code? I'll have a better look at this by eod pacific time. Thanks!

msigley commented 9 years ago

@hozza My fork already caches the last successful request, which saves mailgun's servers a bunch of requests. Caching all requests is probably not a good idea in the name of keeping this code as simple as possible. How many use cases for this are there where a user is going to validate bunch of the same emails more than once on the same page? Most visitors are going to validate 1-3 emails on a page, stopping when their email is marked valid, and caching is not going to save requests in that velocity and use. The last successful request caching is there simply to prevent tabbing through the email input from making a new request. Could you illustrate how the flood prevention doesn't play well with jquery validate? All the flood prevention does is abort previous ajax requests.

@eddywashere: The tab issues are a symptom of me using tab characters instead of spaces in my IDE. Personally I think the whole file should be converted to tabs as you can set the width of the tab character in your IDE to fit your personal preference. Should I convert the whole thing to tabs?

eddywashere commented 9 years ago

@msigley I can do it. this looks good to :shipit: