Once the sortable library is in 8.8 and we can rely on it to exist on testbot, we should use that.

CommentFileSizeAuthor
#7 paragraphs-3090457-7.patch2.11 KBbartlangelaan
#5 paragraphs-3090457-5.patch2.38 KBbartlangelaan
#4 paragraphs-3090457-4.patch2.37 KBbartlangelaan

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue summary: View changes

Blocked on #3064049: Replace jQuery UI sortable with Sortable js, once that is in we can start with a patch to test it.

martijn de wit’s picture

martijn de wit
Primary language Dutch
Location 🇳🇱 The Netherlands
commented

#3064049: Replace jQuery UI sortable with Sortable js is in Drupal > 8.8.x 🙌

bartlangelaan’s picture

bartlangelaan
Primary language Dutch
Location Netherlands
commented
Status: Active » Needs review
StatusFileSize
new paragraphs-3090457-4.patch2.37 KB

This patch adds core/sortable as a library dependency to core/paragraphs-dragdrop. When core/sortable is not available (8.7 or older), the old behaviour is invoked.

This means that from 8.8 onwards, the core version of sortable is always used.

Because this change makes the drag-and-drop functionality work out-of-the-box with Drupal 8.8+, can we maybe split this issue up and create tests in a separate issue? This patch doen't change and/or improve anything on a functional level.

bartlangelaan’s picture

bartlangelaan
Primary language Dutch
Location Netherlands
commented
StatusFileSize
new paragraphs-3090457-5.patch2.38 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/paragraphs.module
@@ -391,10 +391,23 @@ function paragraphs_preprocess_field_multiple_value_form(&$variables) {
 function paragraphs_libraries_info() {
+  if (_paragraphs_sortable_in_core()) return [];

    short if syntax isn't allowed in the drupal coding standard. 

  2. +++ b/paragraphs.module
@@ -426,6 +439,18 @@ function paragraphs_library_info_alter(&$libraries, $extension) {
 
+  if (_paragraphs_sortable_in_core()) {
+    // When core/sortable is available, the paragraphs/sortable library can be removed.
+    unset($libraries['sortable']);
+    return $libraries;
+  }
+  else {

    Lets do this with a version check, that should be easier, this almost recursively discovers libraries which is a bit scary:

    if (version_compare(\Drupal::VERSION, '8.8', '<')) {

For tests, lets make a deal, you create a follow-up to add test coverage and then I'll commit it without ;)

bartlangelaan’s picture

bartlangelaan
Primary language Dutch
Location Netherlands
commented
Title: Use sortable library from core and add JS test coverage for drag and drop » Use sortable library from core
Issue summary: View changes
StatusFileSize
new paragraphs-3090457-7.patch2.11 KB

See adjusted patch.

I adjusted the issue title & summary. Also, created a follow-up: #3099706: Add JS test coverage for drag and drop.

bartlangelaan’s picture

bartlangelaan
Primary language Dutch
Location Netherlands
commented
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Tested that in our project, works well. And once we require 8.8 we can just remove all that code. Also updated the README.txt a bit.

  • Berdir committed 6d80300 on 8.x-1.x authored by bartlangelaan 
    Issue #3090457 by bartlangelaan, Berdir: Use sortable library from core

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.