As mentioned in the parent issue #3238306: [META] Where possible, refactor existing jQuery uses to vanillaJS to reduce jQuery footprint, we are working towards reducing our jQuery footprint. One of the ways to accomplish this is to reduce the number of jQuery features used in Drupal core. We have added eslint rules that identify specific features and fail tests when those features are in use.
There are (or will be) individual issues for each jQuery-use eslint rule. This one is specific to jquery/no-animate, which targets the jQuery animate function.
core/.eslintrc.jquery.json Change "jquery/no-animate": 0, to "jquery/no-animate": 2, to enable eslint checks for uses of jQuery .animate(). With this change, you'll be able to see uses of the undesirable jQuery feature by running yarn lint:core-js-passing from the core directorycore/scripts/dev/commit-code-check.sh so the DrupalCI testing script can catch this jQuery usage on all files, not just those which have changed# @todo Remove the next chunk of lines before committing. This script only lints
# JavaScript files that have changed, so we add this to check all files for
# jQuery-specific lint errors.
cd "$TOP_LEVEL/core"
node ./node_modules/eslint/bin/eslint.js --quiet --config=.eslintrc.passing.json .
CORRECTJQS=$?
if [ "$CORRECTJQS" -ne "0" ]; then
# No need to write any output the node command will do this for us.
printf "${red}FAILURE ${reset}: unsupported jQuery usage. See errors above."
STATUS=1
FINAL_STATUS=1
fi
cd $TOP_LEVEL
# @todo end lines to remove
Add the block about 10 lines before the end of the file, just before if [[ "$FINAL_STATUS" == "1" ]] && [[ "$DRUPALCI" == "1" ]]; then, then remove it once all the jQuery uses have been refactored.
.animate() to use Vanilla (native) JavaScript instead.| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 3239139-nr-bot.txt | 1.45 KB | needs-review-queue-bot |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #4
nod_Possibly need to add a depreciation notice here for dynamic uses of animate (and other jQuery methods)
https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/misc/ajax.es...
Comment #9
shubh_ commentedI am working on this issue...
Comment #11
shubh_ commentedComment #12
nod_Comment #13
shubh_ commentedok, will check that
Comment #14
shubh_ commentedUpdated as per feedback..
Comment #15
nod_Thanks for the update but I don't think we understood each other, the goal is to replace jquery with native capabilities so if we need a 20 line function to replicate jQuery functionality it's not worth it.
Here I think we can replace the calls to .animate with the native .scrollTo() without a new function that does the animation.
Comment #16
shubh_ commentedAddressed the feedback..
Comment #17
smustgrave commentedBelieve @nod_ feedback has been addressed on this one.
Comment #18
nod_small change required
Comment #19
shubh_ commentedAddressed the change..
Comment #20
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #21
shubh_ commentedComment #22
smustgrave commentedLeaning into that @nod_ has reviewed this one already. But feedback appears to be addressed.
Comment #24
alexpottAdded a comment to the MR.
Comment #25
shubh_ commentedUpdated MR with suggestion...
Comment #26
smustgrave commentedAppears comment has been removed.
Comment #27
nod_Regression when a pager is in a modal (like for the media library view), see MR comment. Do not just apply the suggestion, the whole block of code needs to be optimized, see MR comment.
Comment #28
shubh_ commentedAddressed the feedback given
Comment #29
shubh_ commentedUpdated the issue feedbacks
Comment #30
nod_umm why did you add and removed the refactor? commits https://git.drupalcode.org/project/drupal/-/merge_requests/6737/diffs?co... + https://git.drupalcode.org/project/drupal/-/merge_requests/6737/diffs?co... are good, keep them :)
Comment #31
nod_Comment #32
shubh_ commentedThe second commit has double $ thats why was refactored in second commit
Btw this converting of scrollTarget to jquery failing the test cases
Comment #33
shubh_ commented@nod_ all feedbacks resolved here, pls review it
Comment #34
nod_thanks, that works, we can always improve later if needed, not breaking stuff is more important :)
Comment #35
smustgrave commentedGoing to lean on nod_ review for this one
Comment #39
nod_Committed 449f6fb and pushed to 11.x. Thanks!
Comment #41
nod_failing on 10.3.x because of asset size test
Comment #43
shubh_ commentedso what needs to do here can you pls guide ?
Comment #44
alexpottWe need to fix the two test fails here... https://git.drupalcode.org/project/drupal/-/pipelines/150084/test_report...
It'll just be a case of changing numbers...
Comment #46
nod_I'll take care of it, thanks :)
Comment #47
nod_Comment #49
nod_green
Comment #51
nod_