-
Notifications
You must be signed in to change notification settings - Fork 43
Move from jQuery .bind, :first, .hover, .keyCode, :last, .submit, .trim #6752
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
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6752 +/- ##
========================================
Coverage 79.83% 79.83%
========================================
Files 237 237
Lines 5218 5218
========================================
Hits 4166 4166
Misses 1052 1052 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| if (e.type == 'change') this.form.submit(); // TODO: jQuery deprecation | ||
| if (e.type == 'change') this.form.submit(); |
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.
Should this use .trigger('submit') now?
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 thought so when I added the comment!
I'll check when my system is back up, but I think this.form refers to the <form> instead of the $('form'). If this.form is a jQuery element, it should be .trigger('submit'), yes, but not if it's only the <form>. (jQuery moved to .trigger([event]) to avoid collisions with now-standard things like .click(), .change(), .submit()
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.form is the <form> so .submit() is correct because it isn't a jQuery object.
(and my initial // TODO: jQuery deprecation was wrong)
patphongs
left a comment
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.
-
:first, .hover(), .keyCode, :last, $.trim()looks like they're handled. - I saw one more occurrence of submit that needs to be changed, commented on that
- Lots of occurrences of
.bind()left for filter modules, calendar, dropdown, tables, and widgets. Since there's so much of these changes left, do you want to split bind into a separate PR?
Summary
Resolving the jQuery deprecations has been a burden so let's break #6411 into much smaller pieces
Required reviewers
Impacted areas of the application
General components of the application that this PR will affect:
.bind(),:first,.hover(),.keyCode,:last,.submit(), and.trim()on/in:Screenshots
No changes
Related PRs
Related PRs against other branches:
.click.change.focus,.blur.bind,:first,.hover,.keyCode,:last,.submit,.trim‡you are here
How to test
npm inpm run test-singleshould be clearnpm run build./manage.py runserver.bind()change::firstchanges:<select>elements, but our dropdown selectors).hover()change:.keyCodechanges:/key for inside search.js. Are we still using.js-searchanywhere? (Move from jQuery .focus, .blur #6753 had the same issue):lastchanges:.submit()changes:.trim()change: