ngx-drag-and-drop-lists icon indicating copy to clipboard operation
ngx-drag-and-drop-lists copied to clipboard

Sorting list by moving an item up the list

Open acooney300 opened this issue 6 years ago • 8 comments

When I try to use this library to just sort a fairly simple list. I drag the fourth item and move it up and release, when I view the new list, it is adding the new item in the index + 1 position of where I dropped it. For instance, if I try to drag an item to the top position, it puts it in the 2nd.

It seems to work correctly if I drag the item down to a lower position.

I'm using the (dndMoved) method.

acooney300 avatar Aug 16 '18 20:08 acooney300

I am basically seeing the same issue. Moving items down works fine, moving items up do not... when I look into what is happening in the event handler, it looks like the item is not duplicated properly to the list before splicing the original:

IN a list of

1 2 3

If I drag 3 up to the second position, I get a list (before splicing):

1 2 3 3

with an index of 3. Index is correct but the list should have been:

1 3 2 3

It looks like a bug to me.

heitorb avatar Aug 17 '18 03:08 heitorb

I found the bug.. it's in the dnd-list.js Line 124 need to change from this: this.dndModel.splice(insertionPoint, 0, data); to: this.dndModel.splice(--insertionPoint, 0, data);

hipwinn avatar Aug 17 '18 20:08 hipwinn

This is working fine for me on version 1.2.5 of the library (current version). I found that if I put --insertionPoint in the splice then I get the opposite problem of what others are seeing here, the element drops in the index -1 position of where it should. Maybe someone could share their code where this is happening?

Here's snippets of the relevant code from the library:

        // Invoke the callback, which can transform the transferredObject and even abort the drop.
        let index: number = this.getPlaceholderIndex();
        // create an offset to account for extra elements (including the placeholder element)
        let offset: number = this.nativeElement.children.length - 1 - this.dndModel.length;

...skip some lines

         // Insert the object into the array, unless dnd-drop took care of that (returned true).
        if (data !== true) {
            // use the offset to create an insertionPoint
            let insertionPoint: number = index - offset;
            if (insertionPoint < 0) {
                insertionPoint = 0;
            }
            this.dndModel.splice(insertionPoint, 0, data);
        }
  • insertionPoint is calculated as "index - offset".
  • "Index" is set to the index value of the placeholder.
  • "Offset" is set to "this.nativeElement.children.length - 1 - this.dndModel.length".
    • "this.nativeElement.children.length" is the number of child elements inside the containing element.
    • "-1" is to account for the addition of the placeholder element.
    • "this.dndModel.length" is the number of child objects inside the containing object.

"Offset" was added as part of this issue: https://github.com/misha130/ngx-drag-and-drop-lists/issues/23

Hopefully whatever solution is proposed to fix this problem will work for both of these issues.

kevinjsmith avatar Aug 17 '18 21:08 kevinjsmith

Yes. I was troubleshooting and pinpointed the issue also. I've found a " temp fix" that works both up and down. One thing that looks wrong to me is that the line:

var offset = this.nativeElement.children.length - 1 - this.dndModel.length;

will always return "-1". I am not sure why it is there, maybe for transfers between lists, which is not what I need.

So, to make it work both ways, I realized that all is needed is for "insertionPoint" to be equal to "index -1" if dragging down, or just equal to "index" if dragging up, in the line below:

this.dndModel.splice(insertionPoint, 0, data);

Now to figure out if the item was dragged up or down, I needed to compare index (which is the new position of the item) to the original position of the item. First I tried:

if (data !== true) { var dataIndex = Array.prototype.indexOf.call(this.dndModel, data);
var insertionPoint = 0; if (dataIndex < index) { insertionPoint = index - offset; } else { insertionPoint = index; } this.dndModel.splice(insertionPoint, 0, data); }

And that should have worked, but for whatever reason:

var dataIndex = Array.prototype.indexOf.call(this.dndModel, data);

does not find the object in the array and returns -1. I am probably misusing this function. Using plain "this.dndModel.IndexOf(data)" also returns -1. Anyhow, since my items have id, the solution that works for me is specific, but It will work for anyone if your items have an "id" property or any other unique property:

if (data !== true) { var dataIndex = this.dndModel.findIndex(i => i.id === data.id); var insertionPoint = 0; if (dataIndex < index) { insertionPoint = index - offset; } else { insertionPoint = index; } this.dndModel.splice(insertionPoint, 0, data); }

If someone know why the statements I used to find the index of the object in the list did not work please let me know.

heitorb avatar Aug 17 '18 23:08 heitorb

I have the same problem with v1.2.5 and angular 6.0.3

Marcell90 avatar Sep 14 '18 13:09 Marcell90

I have the same problem with v1.2.5 and Angular 6.1.0

khackskjs avatar Dec 02 '18 07:12 khackskjs

Same problem with v1.2.5 and Angular 7.1.4

igorosabel avatar Dec 26 '18 19:12 igorosabel

I used @heitorb approach and build on top of that to get this solution with does not require any special logic to retrieve the id. I'm using lodash's findIndex function to do so. Besides I'm now marking the element that is being moved for "removal" and getting rid of it later, though technically not strictly related for this issue.

Hope this helps you: if (data !== true) { var dataIndex = _.findIndex(this.dndModel, data); this.dndModel[dataIndex].toRemove = true; var insertionPoint = 0; if (dataIndex < index) { insertionPoint = index - offset; } else { insertionPoint = index; } this.dndModel.splice(insertionPoint, 0, data); _.remove(this.dndModel, el => el.toRemove); }

kirkrulez83 avatar Feb 06 '19 12:02 kirkrulez83