Problem/Motivation

Convert state-based selectors to be compatible with with jQuery native-API selector

Proposed resolution

Follow this replacement pattern:

$('selector:hidden') -> $('selector').filter(':hidden')
$('selector:slected') -> $('selector').filter(':selected')
$('selector:visible') -> $('selector').filter(':visible')
$('selector:animated') -> $('selector').filter(':animated')
CommentFileSizeAuthor
#24 interdiff-drupal-convert-state-based-selectors-2159271-19-24.txt5.89 KBahemnell
#24 drupal-convert-state-based-selectors-2159271-24.patch14.37 KBahemnell
#19 drupal-convert-state-based-selectors-2159271-19.patch9.99 KBahemnell
#18 interdiff.txt1.57 KBrteijeiro
#18 drupal-convert-state-based-selectors-2159271-18.patch13.03 KBrteijeiro
#15 drupal-convert-state-based-selectors-2159271-15.patch13.1 KBahemnell
#10 drupal-convert-state-based-selectors-2159271-10.patch14.56 KBInternetDevels
#10 interdiff-2159271-9-10.txt714 bytesInternetDevels
#9 core-js-jquery-state-2159271-9.patch14.46 KBnod_
#2 drupal-convert-state-based-selectors-2159271-2.patch10.79 KBInternetDevels
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tarekdj’s picture

tarekdj CreditAttribution: tarekdj commented
Issue summary: View changes
InternetDevels’s picture

InternetDevels CreditAttribution: InternetDevels commented
Assigned: tarekdj » Unassigned
Status: Needs work » Needs review
FileSize
drupal-convert-state-based-selectors-2159271-2.patch10.79 KB

Seems changed

mathes’s picture

mathes CreditAttribution: mathes commented

yep seems changed.

mathes’s picture

mathes CreditAttribution: mathes commented
Status: Needs review » Reviewed & tested by the community
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

Tested as well, RTBC +1

droplet’s picture

droplet CreditAttribution: droplet commented 
+++ b/core/modules/views_ui/js/views-admin.js
@@ -690,13 +690,13 @@ $.extend(Drupal.viewsUi.RearrangeFilterHandler.prototype, {
+      var $firstCell = $draggableRow.find('td').filter(':first');
...
-      var $firstCell = $draggableRow.find('td:first');

preferred to use eq()?? Please drop it. https://drupal.org/comment/8364645#comment-8364645

+++ b/core/modules/system/system.modules.js
@@ -20,7 +20,7 @@ Drupal.behaviors.tableFilterByText = {
+      var $visibleRows = $details.find('table:not(.sticky-header)').find('tbody tr').filter(':visible');

:not -> .not ?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

:not is CSS3 nous problem with that

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Reviewed & tested by the community » Needs work

The :not in selector is fine, but the first point of droplet comment still stands. Better to use .eq().

When re-rolling you should know that the indentation standard changed for JavaScript files.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
FileSize
core-js-jquery-state-2159271-9.patch14.46 KB

Rerolled the patch, Comments still needs to be addressed but at least patch applies.

InternetDevels’s picture

InternetDevels CreditAttribution: InternetDevels commented
Status: Needs work » Needs review
FileSize
interdiff-2159271-9-10.txt714 bytes
drupal-convert-state-based-selectors-2159271-10.patch14.56 KB

Something like this?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

yeah, seems like I added some whitespace things during my reroll. :/ i'll have to get rid of them.

droplet’s picture

droplet CreditAttribution: droplet commented 
+++ b/core/modules/views_ui/js/views-admin.js
@@ -690,7 +690,7 @@
+        var $firstCell = $draggableRow.find('td').eq(0);

drop this change we need not reroll second time on other issue. ( https://drupal.org/comment/8364645#comment-8364645 )

anavarre queued 10: drupal-convert-state-based-selectors-2159271-10.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 10: drupal-convert-state-based-selectors-2159271-10.patch, failed testing.

ahemnell’s picture

ahemnell CreditAttribution: ahemnell commented
Status: Needs work » Needs review
FileSize
drupal-convert-state-based-selectors-2159271-15.patch13.1 KB

Patch re-rolled.

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro at Tieto commented
Status: Needs review » Reviewed & tested by the community

I checked responsive tables, ajax autocomplete and table drag and everything seems to work well so this looks like RTBC.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Status: Reviewed & tested by the community » Needs work 
core/misc/ajax.js
  107:14  error  Empty block statement  no-empty

core/modules/block/js/block.admin.js
  23:12  error  $catDetails is defined but never used  no-unused-vars

core/modules/node/content_types.js
  21:50  error  Unexpected identifier

✖ 3 problems (3 errors, 0 warnings)
rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro at Tieto commented
Status: Needs work » Needs review
FileSize
drupal-convert-state-based-selectors-2159271-18.patch13.03 KB
interdiff.txt1.57 KB

Thanks for the review @nod_. I was using jslint for code check :(

ahemnell’s picture

ahemnell CreditAttribution: ahemnell as a volunteer commented
FileSize
drupal-convert-state-based-selectors-2159271-19.patch9.99 KB

You beat me to it Ruben :) Here is a new patch with your fixes but without those speces after named js functions as stated in https://www.drupal.org/node/172169#functions

droplet’s picture

droplet CreditAttribution: droplet commented 
+++ b/core/misc/ajax.js
@@ -712,7 +712,7 @@
-      $(response.selector).find('> tbody > tr:visible, > tr:visible')
+      $(response.selector).find('> tbody > tr, > tr').filter(':visible')

I don't know if we should do it at once. No leading ">" allowed. It should converted to $.children().

Thanks.

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro at Tieto commented

Thanks for your patch @ahemnell. We didn't have the chance to discuss about interdiffs (difference between your patch and the previous patch). It's a good practice to provide them as you can see in #18. Let me know if you need help creating one ;)

ahemnell’s picture

ahemnell CreditAttribution: ahemnell as a volunteer commented

I can't find any references on the web about it being disallowed to start a jQuery selector with ">". I think it's fine.

Just replacing .find with .children won't work because it will not be able to access the tbody > tr and tr at the same time. But there still might be a pretty and more efficient solution using .children.

droplet’s picture

droplet CreditAttribution: droplet commented

@ahemnell,

Leading ">" features provided by selector-native.js but we aim to remove it in these series of issues.

Following code will output errors.
document.querySelectorAll("> div")

ahemnell’s picture

ahemnell CreditAttribution: ahemnell as a volunteer commented
FileSize
drupal-convert-state-based-selectors-2159271-24.patch14.37 KB
interdiff-drupal-convert-state-based-selectors-2159271-19-24.txt5.89 KB

Here's my suggestion. One big thing to consider is that I propose to skip test for table > tr, and always just look for table > tbody > tr, because all modern browser wrap orhan tr tags in tbody (*citation needed ). Edit: seems that xhtml doctype omits the tbody wrapping. I guess we have to take table > tr into account, just in case. What do you guys think?

Proposal example:

$(response.selector).children('tbody').children('tr').filter(':visible')

With support for immediate tr something like this is is needed:

$(response.selector).add(response.selector+' tbody').children('tr').filter(':visible')

which is a quite ugly and heavy. Edit2: above is wrong (nested tbodys are added also).

New try:

var table = $(response.selector);
table.add(table.children('tbody')).children('tr').filter(':visible')

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 for Workday, Inc. commented
Status: Needs review » Postponed

This should be postponed on #2702233: [backport] Add JavaScript tests for Form API #states: required, visible, invisible, expanded, checked, unchecked so that we know there aren't any regressions once this is in.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm as a volunteer commented
Priority: Normal » Major
Issue tags: +Drupal 10

Promoting to major alongside the parent.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Postponed » Active

#2702233: [backport] Add JavaScript tests for Form API #states: required, visible, invisible, expanded, checked, unchecked landed so there seems no reason for this to be postponed.