Sortable: When using multiple lists, dropping back the item in the list where it was dragged cause item creation in the last other list it has been dragged over

[jamendub]

So I was reading Issue #633 on this github, and agree that the issue describe can be a feature but i think this one is a bug because it works when sort is enabled...

So here is the full description of the steps to reproduce the problem :
1) I pick an item from list 1 where settings are:
- group: { name: 'blocks', pull: 'clone', put: false }
- sort : false
2) I drag it over the other list which settings are:
- group: { name: 'blocks', pull: true, put: true }
- sort : true
3) The ghost item appears well in the 2nd list
4) Drag it out of the 2nd list, ghost item is still there
5) Drag it BACK over the 1st list, ghost item disappear (which is what is wanted)
6) Drop it in the list where it was originally dragged : item is created in the 2nd list !!! (BUG)

NB : if I activate the sort on the first list, the bug is gone, but my item possibly change place in that list and I don't want that behavior... I'll try to dig in the code and fix the bug

[jamendub]

Ok so after some research in your the Sortable.js file,
I've found out that moving line 687 to line 694 was solving the issue.
When sort is disabled, the parentEl is not updated when you drag back to the rootEl, but it should be because the sorting will anyway be canceled in the _onDrop function.

That issue is global to any version of your library, not only to the AngularJS port, so can you remove the angular label ?

[jamendub]

_onDragOver: function (/*_Event_/evt) {
var el = this.el,
target,
dragRect,
revert,
options = this.options,
group = options.group,
groupPut = group.put,
isOwner = (activeGroup === group),
canSort = options.sort;

        if (evt.preventDefault !== void 0) {
            evt.preventDefault();
            !options.dragoverBubble && evt.stopPropagation();
        }

        moved = true;

        if (activeGroup && !options.disabled &&
            (isOwner
                ? canSort || (revert = !rootEl.contains(dragEl)) // Reverting item into the original list
                : activeGroup.pull && groupPut && (
                    (activeGroup.name === group.name) || // by Name
                    (groupPut.indexOf && ~groupPut.indexOf(activeGroup.name)) // by Array
                )
            ) &&
            (evt.rootEl === void 0 || evt.rootEl === this.el) // touch fallback
        ) {
            // Smart auto-scrolling
            _autoScroll(evt, options, this.el);

            if (_silent) {
                return;
            }

            target = _closest(evt.target, options.draggable, el);
            dragRect = dragEl.getBoundingClientRect();

            if (revert) {
                _cloneHide(true);

                if (cloneEl || nextEl) {
                    rootEl.insertBefore(dragEl, cloneEl || nextEl);
                }
                else if (!canSort) {
                    rootEl.appendChild(dragEl);
                }

                return;
            }


            if ((el.children.length === 0) || (el.children[0] === ghostEl) ||
                (el === evt.target) && (target = _ghostIsLast(el, evt))
            ) {

                if (target) {
                    if (target.animated) {
                        return;
                    }

                    targetRect = target.getBoundingClientRect();
                }

                _cloneHide(isOwner);

                if (_onMove(rootEl, el, dragEl, dragRect, target, targetRect) !== false) {
                    if (!dragEl.contains(el)) {
                        el.appendChild(dragEl);
                        parentEl = el; // actualization
                    }

                    this._animate(dragRect, dragEl);
                    target && this._animate(targetRect, target);
                }
            }
            else if (target && !target.animated && target !== dragEl && (target.parentNode[expando] !== void 0)) {
                if (lastEl !== target) {
                    lastEl = target;
                    lastCSS = _css(target);
                    lastParentCSS = _css(target.parentNode);
                }


                var targetRect = target.getBoundingClientRect(),
                    width = targetRect.right - targetRect.left,
                    height = targetRect.bottom - targetRect.top,
                    floating = /left|right|inline/.test(lastCSS.cssFloat + lastCSS.display)
                        || (lastParentCSS.display == 'flex' && lastParentCSS['flex-direction'].indexOf('row') === 0),
                    isWide = (target.offsetWidth > dragEl.offsetWidth),
                    isLong = (target.offsetHeight > dragEl.offsetHeight),
                    halfway = (floating ? (evt.clientX - targetRect.left) / width : (evt.clientY - targetRect.top) / height) > 0.5,
                    nextSibling = target.nextElementSibling,
                    moveVector = _onMove(rootEl, el, dragEl, dragRect, target, targetRect),
                    after
                ;

                if (moveVector !== false) {
                    _silent = true;
                    setTimeout(_unsilent, 30);

                    _cloneHide(isOwner);

                    if (moveVector === 1 || moveVector === -1) {
                        after = (moveVector === 1);
                    }
                    else if (floating) {
                        var elTop = dragEl.offsetTop,
                            tgTop = target.offsetTop;

                        if (elTop === tgTop) {
                            after = (target.previousElementSibling === dragEl) && !isWide || halfway && isWide;
                        } else {
                            after = tgTop > elTop;
                        }
                    } else {
                        after = (nextSibling !== dragEl) && !isLong || halfway && isLong;
                    }

                    if (!dragEl.contains(el)) {
                        if (after && !nextSibling) {
                            el.appendChild(dragEl);
                        } else {
                            target.parentNode.insertBefore(dragEl, after ? nextSibling : target);
                        }
                    }

                    this._animate(dragRect, dragEl);
                    this._animate(targetRect, target);
                }
            }
        }

  if (dragEl) {
    parentEl = dragEl.parentNode; // actualization
  }
    },

[jamendub]

Here is the corrected "_onDragOver" function.
Can you merge this into the next tag please ?
(i dunno how to pull-request, sorry)

[excentris]

I have come across the same issue as well.

[latviancoder]

thanks, that solved my issue.

[jamendub]

Can the owner of this repo please patch his library ?

[RubaXa]

@jamendub Please send PR, I check it.

[jamendub]

I cannot pull-request, but i sent you the corrected version of your _onDragOver function just above, you can check it ;)

[RubaXa]

Stop. But I have already answered in #633 (https://github.com/RubaXa/Sortable/issues/633#issuecomment-153302481) that the current behavior is correctly.

[RubaXa]

If do as you suggest, it results in other errors, that have been corrected.

[jamendub]

Ok well I'm pretty sure it won't, I tested your list of patched bugs on my correction it pretty much works

Basically what I do is what your answered in #633 : "To return the item, you need to to translate it back to the initial list."
But it does not work.

So i think this one is an actual bug and apparently i'm not the only one to have run into it and other people said it corrected the problem... You know I really like you library and I just to help you make it better !

I'm trying now to make a PR for you.

PS : and this is not an angular issue, it concerns the library itself.

[latviancoder]

Konstantin, with these options "translating it back to the initial list" doesn't help, because the item is still added to the target list.

Just moving that line further and adding an 'if', like suggested by @jamendub, fixes the problem.

Unfortunately I have no idea about other problems this fix may introduce.

[jamendub]

Aight, PR is done.
I ran a quick test of all previously resolved problems and it does not seem to break something.
If it does, could you please list me the issues concerned, i can try to make a better fix.
Otherwise is it possible to merge it with current or next version please ?
Thank you for your time.

[jamendub]

Here you have a JSBIN with the issue : http://jsbin.com/kimesobapi/1/edit?html,js,output
To reproduce, take a block from 1st list, drag it over 2nd list, then translate it back to first list and drop.
It will be added to the second list anyway.

[jamendub]

Ok i re-tested the dev-branch and it is now working on that branch.
When do you plan to make the next release ?
Anyway, thanks for taking the time to look into it.
Much appreciated.

[RubaXa]

While cannot find the the time to test everything.

[jamendub]

Ok, well did you think about implementing unit testing ?
I know this testing takes loads of time...

Oh, and well I cannot work on the dev branch, because I can't drop a block in the second list...
New error is :

Uncaught TypeError: Cannot read property 'split' of undefined_matches @ Sortable.js:1254_index @ Sortable.js:1244_sync @ ng-sortable.js:113Sortable.create.Object.keys.reduce.onAdd @ ng-sortable.js:168_dispatchEvent @ Sortable.js:1156Sortable._onDrop @ Sortable.js:835Sortable.handleEvent @ Sortable.js:914

[RubaXa]

Ok, well did you think about implementing unit testing ?

Such a issue is already there: #140
But within two years of existence this issue, no one helped me.

[dleffler]

FWIW, in the current release (v1.4.2), this 'hack' above is still needed (by me) to prevent the situation where once you drag an item from the source to over the target it WILL ALWAYS be added to the target regardless of where you actually drop it. With the 'hack' above it will be removed from the target once you drag it back over the source (_which is what I require_).

As a note my scenario (a form designer) has three (3) lists:

• List A is the form with a list of controls on that form
• List B is the list of available controls which may be added to a form
• List C is a drop box acting as a recycle bin to remove controls from the form
• List A can be sorted, or can be dragged-out/'pull' to list C to delete, or can receive/'put' items from list B to add new controls which then presents an ajax populated dialog
• List B is NOT sorted, can't receive/'put', but can give/'pull' to list A
• List C is NOT sorted, can't give/'pull', but can receive/'put' from list A and when it does the item is immediately removed from the list

The code for this is:

    $(document).ready(function(){
        Sortable.create(form, {
            group: {
                name: 'form',
                put: ['controls']
            },
            onAdd: function (evt) {
                var itemEl = $(evt.item);  // dragged HTMLElement
                var dialog = BootstrapDialog.show({
...//there is code here for building the dialog
                });
                evt.preventDefault();
            },
        });

        Sortable.create(controls, {
            group: {
                name: 'controls',
                pull: 'clone',
                put: false
            },
           sort: false
        });

        Sortable.create(trash, {
            group: {
                name: 'trash',
                pull: false,
                put: ['form']
            },
           sort: false,
            onAdd: function (evt) {
                var itemEl = $(evt.item);  // dragged HTMLElement
                itemEl.remove();  //we don't won't to show removed controls
            },
        });
    });

[jamendub]

I was pissed to see this issue not corrected so i finally switched to another library.
It's only for angular though : "angular-drag-and-drop-lists"

[snurra]

I have also thought about switching because of this, but haven't found any good alternatives that covers all my use-cases so far.

[jamendub]

What tech do you use ? What are your use-cases ? I have been around all that mess, maybe i can save you some time ?

[KuroNoDev]

@jamendub Thanks for the fix. On a side note, I'm working with nested angular lists. There's also a bug in angular nested lists that isn't fixed (most likely won't be) in dev/master so I'm just using a fork. I will be adding your fix onto that.

Edit: This also fixes #328

[RubaXa]

Dear all! ng-sortable.js is moved to the separate repository. If this issue is still actual, please create it there once again and this rojects needs a maintainer:

• 2+: https://github.com/SortableJS/angular-sortablejs 🆘
• 1.*: https://github.com/SortableJS/angular-legacy-sortablejs

[snurra]

@RubaXa There seems to be some kind of misunderstanding that this is an Angular issue, but as been mentioned previously it is not. The bug exists in the main library (1.4.2).

The solution suggested in https://github.com/RubaXa/Sortable/issues/722#issuecomment-171638234 seems to be working very well from what I can see.

It would be great if this fix could finally be integrated with the main version of this library as I'm not too fond of monkey patching.

[a5hik]

Hi Can you release a new npm version with this Fix, Looks like the 1.4.2 is published 11 months back.

[rynkk]

Apparently this is still not fixed, why?

[damienix]

I think it's not.

Recently, I was facing the same issue and I managed to work this around.
My solution was to just place another draggable inside the original draggable. This fake draggable should expand over the whole space that is left below the original container. This way, when you move back to original container, the fake draggable catches your item.

I know it's far from perfect as requires extra logic and styles, but in my case was still much simpler than migrating to another library.

Excerpt from my code (using vuedraggable):

<draggable
           :value="value"
           @input="changed"
           :options="{ draggable: '.card', ...}"
           ...>
    <card v-for="issue in value" class='card' ...></card>

    <draggable
            :value="[]"
            class="fix-move-back-spacer"
            @input="dropToSpacer"
            ...
    ></draggable>
</draggable>

[rynkk]

I was able to fix it without any hacks by changing/fixing the sortable.js code the way mentioned above.

Works like a charm now and I have not encountered any illbehaviour since the change