-
-
Notifications
You must be signed in to change notification settings - Fork 149
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
Replace .click()
, .focus()
and .attr()
jQuery functions with equivalent native JS
#925
Replace .click()
, .focus()
and .attr()
jQuery functions with equivalent native JS
#925
Conversation
…uivalent native JS
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.
Thank you very much for this PR. It's looking good! I haven't tested yet -- I've got some minor changes, mostly to do with quotation mark style, and a few cases where you could convert some other functions as well which don't look difficult to do. I'll test once you've made changes.
Sure! I'll make the changes. |
…ry functions to native equivalent JS
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.
Nearly there -- just a few more small things!
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, thanks (there is just one minor thing, removing a blank line). I'll do some testing later today, in particular on the effect of purging the iframe.
Great. I've removed the empty line. And sure, do test and let me know if there are any issues. I'll be most happy to resolve them :) |
209011a
to
edd185a
Compare
Co-authored-by: Jaifroid <[email protected]>
I tested your suggestion, @Jaifroid and it works flawlessly! Thank you so much. Let me know if anything else is required. Here, take a look! |
@dheerajdlalwani Thank you very much, and also for testing thoroughly. It looks good now! I presume you don't see a button "Squash and merge" below the test results below? If you don't see it, or can't click it, let me know if you are ready to squash/merge, and I'll do it -- it will still appear on GitHub as your contribution/merge. NB it is important to squash down to a single commit, but GitHub can do this automatically. |
Thank you so much, @Jaifroid. No, I don't see the Squash and Merge button. I think you will have to do it. And you don't have to thank me! It was absolutely my pleasure. I'm learning so much from this! Infact, I am enjoying it! |
I have tested the changes in Edge Chromium, Chrome, Firefox and IE 11.
So far everything has been working. Opening PR. Let me know if any changes are required :)
Fixes #919 .