-
Notifications
You must be signed in to change notification settings - Fork 39
Jquery deprecation updates #315
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me!
jQuery( document ).on( 'change', '#protect_files', function() { | ||
if ( this.checked ) { | ||
jQuery( '.hide_protect_files' ).fadeIn( 'slow' ); | ||
jQuery( '#edit_action' ).change(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want to point out that I am intentionally just removing this line and a line shortly underneath it. I think I copied too much here when I added the protect files logic. These lines shouldn't have anything to do with file protection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Just making some notes for a few things we need to make sure get double tested:
- load fields in builder with ajax with form with more than 20 fields
- add two logic rows in a field, remove the first one, and add another. Do they both save correctly? Test the same process with watch lookup fields, form actions, form action logic, submit button logic, create post category & meta, order rows in a view, where rows in a view
- add a page break and collapse page 1 in builder
- save builder page when ajax save fails, and with ajax builder save turned off
Side note:
looks like some of this js should be copied into views and deprecated here (starting at line 4445)
This is a pretty good first go of the jquery deprecation warnings in the free plugin. I think some other shorthand stuff might be missing, but I looked over all of the previous deprecations and I think this covers mostly everything in our code (excluding some third party stuff we're using).
jQuery( element ).submit()
to just a new triggerSubmit function (vanilla solution)..submit
listeners which are now deprecated for.on( 'submit'
that isn't (which is still jQuery), becausejQuery.fn.submit() event shorthand is deprecated
More about the shorthand deprecations can be found here jquery/jquery#3214 besides that I wasn't finding a lot of resources, but the warnings are all over the place.
I also made a few adjustments to move var declarations to the start of their function scopes to help reduce our number of warnings.
In an effort to reduce jQuery I made new functions triggerSubmit and triggerChange. triggerChange didn't look beautiful without jQuery (and several cases were sending jQuery objects as well), plus it's hard to test, so I'm keeping it dependent on jQuery for now, but it's better to have everything referencing one place that depends on jQuery than it is to have everything.
However, triggerSubmit seems to have a nice workable solution of inserting a hidden button and clicking it, so I've replaced our .submit() shorthands for vanilla js :)