Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Convert type selectors to be compatible with with jQuery native-API selector for better performance
Proposed resolution
Follow this replacement pattern:
:eq -> $("your-pure-css-selector").eq(index)
:even -> .filter(":even")
:odd ->.filter(":odd")
:first -> .filter(":first") or .eq(0)
:gt(index) -> $("your-pure-css-selector").slice(index)
:last -> .filter(":last")
:lt(index) -> $("your-pure-css-selector").slice(0, index)
Comments
Comment #1
tarekdj CreditAttribution: tarekdj commentedComment #2
nod_using :even and :odd do not take us much closer to not using sizzle. The changes should be:
OK.
We can leave those alone, they'll get removed eventually.
so something like:
Also we don't touch files in the assets/vendor folder, it's not ours.
And you should really use JSHint, you have syntax errors that makes the file completely useless :) https://drupal.org/node/1972428
Comment #3
tarekdj CreditAttribution: tarekdj commentedI admit that this patch is really sloppy. Shame on me! :)
Comment #4
tarekdj CreditAttribution: tarekdj commentedI tryed to follow your recomendations. Definitely better!
Comment #5
droplet CreditAttribution: droplet commentedMissing :first
can we add space after comma
Comment #6
droplet CreditAttribution: droplet commented- Fixed mismatch selectors
- :first -> eq(0)
- :last -> eq(-1)
- dropped one :input fix, we fix it in another issue thread.
Comment #8
javisr CreditAttribution: javisr commentedPatch applies cleanly and fixes the issue. I have verified that there are no more pending selectors to search and replace.
Comment #9
javisr CreditAttribution: javisr commentedComment #10
nod_This is a reroll because the indentation standard changed for JavaScript files. No need for commit credit because of this reroll.
Comment #11
jibranThese changes are not present in RTBC patch. This is not a functional change but I think we should revert it because it is not related.
Comment #12
catchThere's no confirmation on here that someone's manually tested the js that gets touched.
Comment #13
catchComment #14
quietone CreditAttribution: quietone commentedHaving a go on this at DrupalSouth 2014
Comment #15
quietone CreditAttribution: quietone commentedTable drag works a treat, thanks.
What should I be looking for when doing manual testing of this patch?
Comment #18
tarekdj CreditAttribution: tarekdj commentedComment #19
Poornima3 CreditAttribution: Poornima3 commentedI have converted some of the position selectors to be compatible with with jQuery native-API selector
Comment #20
zaporylieAs nod_ said "...we don't touch files in the assets/vendor folder, it's not ours."
And, definitely, patch #10 needs re-roll + changes suggested in #11
Comment #21
neelam.chaudhary CreditAttribution: neelam.chaudhary commentedReroll of patch #10 and the changes suggested in #11.
Comment #22
manningpete CreditAttribution: manningpete commentedpatch applies.
Comment #23
nod_Please drop all spacing changes from this patch, my mistake for introducing them in the first place.
We already fixed the spacing issues in another issue (also be sure to run eslint when you make a patch to be sure you conform to our JS coding standards).
Comment #24
neelam.chaudhary CreditAttribution: neelam.chaudhary commentedAs mentioned dropping all the spacing changes and retaining type selectors changes. Please review
Comment #25
jamin_melville CreditAttribution: jamin_melville commentedI've updated the patch, files had been updated since patch created.
Comment #26
nod_Totally my fault but needs reroll. Sorry.
Comment #27
nod_Comment #28
lanchez CreditAttribution: lanchez commentedRerolling.
Comment #29
lanchez CreditAttribution: lanchez commentedRerolled.
Do we want to use :not selector or jQuery .not?
The case is:
var $firstTab = this.details.siblings('.vertical-tabs__pane').not('.vertical-tab--hidden').eq(0);
vs
var $firstTab = this.details.siblings('.vertical-tabs__pane:not(.vertical-tab--hidden)').eq(0);
I guess that :not will be faster with "proper" browsers.
Comment #30
nod_Yep that's CSS3 stuff since it's a simple selector inside the :not.
So yes, please use :not()
( edit ) patch already does it, great :)
Comment #31
nod_Block admin works
Contextual works
Editor works
Tabledrag works (keyboard and mouse) (on display mode (with group))
Responsive table works
Text summary works
Tour works
Vertical tabs works
Views admin works.
All good, thanks!
Comment #33
alexpottThis is a non disruptive change to javascript - therefore permitted during the beta. Committed 22ef398 and pushed to 8.0.x. Thanks!
Comment #36
droplet CreditAttribution: droplet commentedOuch, broken some features: #2489826: tabledrag is broken