Problem/Motivation

In #3052002: [meta] Replace JQuery with vanilla Javascript in core and several other places, Tabledrag was mentioned as one of the larger obstacles to removing jQuery from core. An issue to work on that can help move the jQuery removal efforts forward, and it is a large enough task that it will likely surface many additional tasks needed for removing jQuery

Steps to reproduce

Proposed resolution

Remove all jQuery usage from Tabledrag other than once() and event handling, which have their own issues: #2402103: Add once.js to core #3176441: JavaScript event handling without a full jQuery dependency.

Also refactor code that extends on Tabledrag as much as needed in order for it to work with no-jQuery tabledrag.

For the time being, suppress the tests in Claro as getting them to work would require extensive refactoring, yet that refactoring will be largely unnecessary when #3083051: Refactor tabledrag when core issues are resolved lands.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 3177355-10-9.4.x.patch213.56 KBtom konda
#8 interdiff_6-8.txt1.32 KBbnjmnm
#8 3177355-8.patch213.4 KBbnjmnm
#6 3177355-6-REROLL.patch212.08 KBbnjmnm
#2 3177355-2.patch213.98 KBbnjmnm

Issue fork drupal-3177355

Command icon Show commands

Start 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:

About issue forks

Comments

bnjmnm created an issue. See original summary.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
StatusFileSize
new 3177355-2.patch213.98 KB

This removes all jquery use from tabledrag other than event handling and once(). Module code that modifies tabledrag may still have jquery. The refactoring there was limited to what was neccessary for it to work with the modified tabledrag, as the patch size is already quite daunting.

A grep through the patch for @todo jquery will return several items that will likely need to be addressed as part of removing jquery, but is not in the scope of this issue. Many followups will soon be created based on these.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Active » Needs review

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

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Needs work

Need reroll after prettier update.

  1. +++ b/core/misc/tabledrag.es6.js
@@ -57,1691 +57,1793 @@
+  Drupal.TableDrag = class {

    This is a significant change, I would keep Drupal.tableDrag around and make it a helper to initialize a tabledrag. A bit like we did with Drupal.ajax / Drupal.Ajax.

    The structure of the object was changed also so any script extending the tabledrag will have to be revised.

    I think we need a summary of the object structure before/after to see how much it could impact contrib. 

  2. +++ b/core/misc/tabledrag.es6.js
@@ -57,1691 +57,1793 @@
+     * Helper that replicates jQuery prev().
...
+     * @todo jQuery: consider a global replacement for jQuery prev() instead
...
+     * Helper that replicates jQuery next().
...
+     * @todo jQuery: consider a global replacement for jQuery next() instead
...
+     * Helper that replicates jQuery prevAll().
...
+     * @todo jQuery: consider a global replacement for jQuery prevAll() instead
...
+     * Helper that replicates the jQuery :hidden selector.
...
+     * @todo jQuery: consider a global replacement for jQuery :hidden
...
+     * Helper that replicates the jQuery :visible selector.
...
+     * @todo jQuery: consider a global replacement for jQuery :visible

    That's a few methods we still need from jquery, might want to start putting them in a dedicated file and see how much we still need across the different scripts (since a file/library by method might be overkill)

In any case, happy to see this! always a hard script to get into, thanks :)

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review
Issue tags: +Needs followup
StatusFileSize
new 3177355-6-REROLL.patch212.08 KB

Re #5

This is a significant change, I would keep Drupal.tableDrag around and make it a helper to initialize a tabledrag. A bit like we did with Drupal.ajax / Drupal.Ajax.

The structure of the object was changed also so any script extending the tabledrag will have to be revised.

I think we need a summary of the object structure before/after to see how much it could impact contrib.

I figured we would either need a BC shim or make the vanilla tabledrag available as a module + trigger deprecation notices like we are doing in #3076171: Provide a new library to replace jQuery UI autocomplete. When I created this issue, I didn't prefer one option over the other and figured working on this patch would help me figure it out. I'm leaning towards the module option as working on this has me doubting the feasibility of bridging the jQuery/vanilla tabledrags. I'm curious if you have any concerns about the module approach.

That's a few methods we still need from jquery, might want to start putting them in a dedicated file and see how much we still need across the different scripts (since a file/library by method might be overkill)

Agreed, I have @todo items for these indicating that they are probably better to add globally. I wasn't entirely sure what jQuery methods we'd need to hold onto, and I thought working on this issue would help expose what followup(s) would be needed (there are several polyfill issues I need to make as well).

Switching to NR to turn on tests, but clearly this isn't the final patch 😊

Status: Needs review » Needs work

The last submitted patch, 6: 3177355-6-REROLL.patch, failed testing. View results

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review
StatusFileSize
new 3177355-8.patch213.4 KB
new interdiff_6-8.txt1.32 KB

#6 had a missing polyfill, which caused the fails in #6

bnjmnm opened merge request !524

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

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
Status: Needs review » Needs work

I tested patch #8 against the commit 1a5cc0d9ab68d8c7b22143c4642a691365171cd1 and a following "Uncaught TypeError" occurs on block region changing.

Uncaught TypeError: Drupal.TableDrag.row is not a function
    attach http://localhost:8888/core/modules/block/js/block.js?v=9.2.8-dev:118
    jQuery 2
block.js:118:48

The Drupal.TableDrag.row() method isn’t defined as static method but it is called from block.js without instance creation.

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

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented
StatusFileSize
new 3177355-10-9.4.x.patch213.56 KB

Fix patch #8 to support 9.4.x branch.

  • Fix error on block region changing.
  • Use TableDrag.prevAll() in field_ui.es6.js.
  • Remove commented out code.
tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented

I think this issue should be better to change target version to 10.0.x. Because IE 11 doesn’t support :scope: pseudo class and no more fix about this according to #3176438: IE11 JS does not support the :scope pseudo-class.

If it changes target version, need to remove IE 11 compatible code from the existing patch.

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

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now 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.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now 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: 10.1.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, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Tom Konda opened merge request !7672

tom konda’s picture

tom konda
Primary language Japanese
Location Kanagawa, Japan
commented

Reroll a patch for 11.x branch and create the MR 7672 as draft.
Some of functional JavaScript tests are failed, so need to resolve.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.