Make drag and drop compatible with SortableJS 1.10

Happened to us once as well. Still not reproducible.

But what can be reproduced is that dragging + save does not save (as it should). Instead you first need to click the button to close the drag & drop mode.

Thanks for the steps, will look into it.

If you want to be extra helpful, you could add a test method to the DragDrop test, we have fairly similar things alread in there just not yet with new paragraphs. Might result in me looking into this earlier :)

We still experience these problems, especially if you combined delete and add operations with drag & drop.

Best would be to expand test coverage and proof the bug.

Came across it today. Further info, in case someone handles it before my company gets to it:

Using Paragraph Experimental Widgets:

Configuration:

1. a field entity_reference_revision of paragraph used as a container (lets call it "box" paragraph) to hold other paragraph types.
2. another paragraph type acts like a category to hold books (lets call it "category").
3. finally the leaf paragraph "books" reside in category. (lets call it "leaf")

Replicate:

1. Add a box and put a category in it (and other paragraphs). Make sure category has some leaf paragraphs in it
2. Save and edit the node again,
3. Add another box under the existing box paragraph
4. enable drag & drop, try to drag & drop the category into the new box,
5. hit complete drag & drop,
6. the whole category will no longer exist on the page (don't save+publish)

Other details:

bug happens in multiple scenarios, one's mentioned above. Inner box can take in leaf level paragraphs, just not anything above it. so the reason category disappears is because they have leaf under them. However, if you move the single leaf they move around fine.

This does not happen if you avoid creating a box within a box. I dont think its the paragraph itself that matters but the entity reference field inside it probably causes recursion or similar issue on field type.

Note: haven't checked the coding yet, the above is only from observations.

I ran into this last week as well, with similar steps as #3.

I am not sure if this case was mentioned but I'll put it here:

Steps to reproduce:

1. Enable paragraphs_demo module
2. Add a new "Paragraphed article" node
3. Add a text paragraph
4. Save the node
5. Edit the node and add a nested paragraph
6. Switch to Drag & Drop mode
7. Move the text paragraph inside the nested one
8. Complete Drag & Drope mode
9. Save the node
10. The text paragraph is gone

Demo:

I was testing with paragraphs 8.x-1.x (bac92b0) and entity_reference_revisions 8.x-1.x (167be1c).

This could be a problem in ERR and if so I will link the related issue when I find it.

Our base library sortable.js seems to have some related glitches. We switched version(s) over the time.
For investigation, please first try to:
* Update sortable to the latest version
* Try a downgrade to our used previous versions

And then please report if switching sortable.js helped to fix your issue.

I can confirm #9 is related to SortableJs library that Drag & Drop requires.

https://github.com/SortableJS/Sortable/commit/5602256 is the actual commit that causes the issue described in #9.

To prevent the confusion (and until we fix SortableJs issues) I filled an issue #3056898: Revert the readme to suggest SortableJs 1.6.0 library + patch in Paragraphs to update the Readme.txt file and suggest the older version of the library.

Interesting..

I see that the commit referred added a new parameter to _dispatchEvent:
NEW

_dispatchEvent(_this, originalTarget, 'filter', target, el, el, startIndex);
function _dispatchEvent(sortable, rootEl, name, targetEl, toEl, fromEl, startIndex, newIndex) {

OLD

_dispatchEvent(_this, originalTarget, 'filter', target, el, startIndex);
function _dispatchEvent(sortable, rootEl, name, targetEl, fromEl, startIndex, newIndex) {

While the event itself has a new attribute there could be two problems:
a) There is still an outdated call without updated arguments
b) If i remember right, we missed access to some context (toEl?) and thus did a workaround. After they now added the missing context and fixed the old argument, our workaround expectations could be wrong.

Needs investigation. :-)

in the new RC3 version there is a lot of new code / rewrite. https://github.com/SortableJS/Sortable/commit/854fed779876074d564c053afb...

Maybe it is providing a solution?

I have just downgraded SortableJS version to 1.6.0. It works as expected!

In the new RC3 there is no fixing for this, just tried it.

Hi agree with jasa, drag and drop works with 1.6.0 but has some wired behavior while dragging in browsers. Newer version is more preceise with this, dragging works but not between nested child/parent paragraphs. We need a fix for this.

On #9, two additional notes that are useful when debugging this:

a) Instead of saving directly, you can press "Complete Drag & Drop", which will "only" result in a php exception about invalid hierachies. Easier to test without actually losing the content.

b) To see what's going on, in \Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget::buildNestedParagraphsFoDragDrop(), change the hidden _path and _weight fields to type textfield, then you can see in the browser how they change when you move things around.

Note sure if there are other issues, but one thing you can see is that the weight isn't properly recalculated on lower paragraphs if a you move a paragraph down into the nested paragraph. Easy to see on the default node that we create.

I tracked down the issue to be in the handleReorder() function. $srcChildren and $children variables were pointing to the same DOM element, which resulted in the parent element not updating its weight and paths with the updateWeightsAndPath() function.

I simplified the function to update the weights of the initial and ending paragraphs nesting level.

I used the most recent version Sortable.js while debugging this (1.10.1). If confirmed, we can delete the required sortable version from the Readme (issue posted above).

Do we already have some related tests that we can update?

Exciting!

Could you please update the readme to drop our compatibility Remarks as well. I read even there is some inconsistency currently..

Removed the 1.6 version requirement from the README.

+++ b/README.txt
+++ b/README.txt
@@ -45,9 +45,6 @@ Use the version 1.8.0+ as it sorts previous bugs with nested drag & drop.

@@ -45,9 +45,6 @@ Use the version 1.8.0+ as it sorts previous bugs with nested drag & drop.
 
 If the file exists, the feature will automatically be available.
 
-Due to known issues with newer versions of SortableJs library, version
-1.6.0 should be used with the patch following patch https://patch-diff.githubusercontent.com/raw/SortableJS/Sortable/pull/1154.diff.
-

guess the inconsistency that they mentioned is that apparently, above this there is text that recommends 1.8 and then below we recommended 1.6 ;)

Since we didn't test < 1.10 now should we update that to recommend using that version?

Here is an improved README change with the suggested change. Does this make sense or should we explicitly state which versions cause troubles with nested drag&drop?

Looks fine. The readme should provide simple information about the current status with recommended versions relevant for the future, not history. If they want to go back in time they can check commit annotations and history.

Soo ready because we commit compatibility with latest sortable without (additional tests)...
Or needs work because we are willing to wait for tests.

Not sure for how much test coverage in drag & drop we should go and how much time it will take. Opinions?

We can't add test coverage yet anyway, or only with a custom drupalci.yml that installs it and I don't want to go there as it would be hard to keep that in sync then.

Created #3090457: Use sortable library from core. Committing and updating issue title to explain what we actually did here.