Solves "insert Before error" issue on nested lists #1784
Conversation
That solves an issue with nested sortables. example of error https://codepen.io/romualdogrillo/pen/jOPoRJe An error appears when all following conditions are met: 1)You have two nested sortable containers 2)option “pull:clone” i set at least on the parent container 3)You drag an item from the parent container into the nested container The item is moved but not cloned and you get the error: “Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted”
|
I have created an example here: |
| @@ -1692,7 +1692,7 @@ Sortable.prototype = /** @lends Sortable.prototype */ { | |||
| if (Sortable.eventCanceled) return; | |||
|
|
|||
| // show clone at dragEl or original position | |||
| if (rootEl.contains(dragEl) && !this.options.group.revertClone) { | |||
| if (dragEl.parentNode == rootEl && !this.options.group.revertClone) { | |||
There was a problem hiding this comment.
| if (dragEl.parentNode == rootEl && !this.options.group.revertClone) { | |
| if ( (rootEl.contains(dragEl) || dragEl.parentNode == rootEl ) && !this.options.group.revertClone) { |
I have a feeling this might be safer. I'm unsure what remove the old check might do.
There was a problem hiding this comment.
in my opinion the change above is equivalent to the original so doesn't solve the issue.
I try to better explain my proposal:
original) "rootEl.contains(dragEl)" means dragEl is a descendent of Root so it can be child, child of child etc...the condition is too broad and results in an error when dragEl is not just a child.
modified) "dragEl.parentNode == rootEl" means dragEl is child of root. That's exactly the condition you have to check before "rootEl.insertBefore(cloneEl, dragEl)", if the condition is not met the code will simply move on to one of the following methods to find the right insertion point.
|
Thank you for the feedback, I answered on github.
#1784 (comment)
Aldo
…On Mon, 13 Apr 2020 at 15:56, Wayne Van Son ***@***.***> wrote:
***@***.**** requested changes on this pull request.
@RomualdoGrillo <https://github.com/RomualdoGrillo>
It looks like all the tests passed but I'm not confident in the test
suite/my knowledge to approve this.
Thank you for providing an example. I see the error.
------------------------------
In src/Sortable.js
<#1784 (comment)>:
> @@ -1692,7 +1692,7 @@ Sortable.prototype = /** @Lends Sortable.prototype */ {
if (Sortable.eventCanceled) return;
// show clone at dragEl or original position
- if (rootEl.contains(dragEl) && !this.options.group.revertClone) {
+ if (dragEl.parentNode == rootEl && !this.options.group.revertClone) {
⬇️ Suggested change
- if (dragEl.parentNode == rootEl && !this.options.group.revertClone) {
+ if ( (rootEl.contains(dragEl) || dragEl.parentNode == rootEl ) && !this.options.group.revertClone) {
I have a feeling this might be safer. I'm unsure what remove the old check
might do.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1784 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADGF5E23VUJD5XFU4BI4XMLRMMKY3ANCNFSM4L54T45A>
.
|
|
Il giorno mar 14 apr 2020 alle 04:53 Owen Mills <notifications@github.com>
ha scritto:
Merged #1784 <#1784> into
master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1784 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADGF5EZLLDEXTKD5NQ4QLFLRMPF3JANCNFSM4L54T45A>
.
Thank you, I’m happy to see my contribution is useful
Romualdo
|
|
Hy There, I have this problem under the following conditions. Two lists: First list (tools): Second list (board): If the same element is dragged from the first list several times into the second, the nesting will go crazy. The same problem if it's only one item, which is automatically created several times when the page is started. The only way to get around the problem is to work with items that have been in the DOM from the beginning. |
Solves "insert Before error" issue on nested lists
That solves an issue with nested sortables.
example of error https://jsbin.com/xabonox/edit?html,css,js
An error appears when all following conditions are met:
1)You have two nested sortable containers
2)option “pull:clone” i set at least on the parent container
3)You drag an item from the parent container into the nested container
The item is moved but not cloned and you get the error:
“Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted”