Components: cdkDropList: event.previousIndex does not update on second move

Created on 9 Nov 2018  路  12Comments  路  Source: angular/components

Bug, feature request, or proposal:

When you drag and drop a row for the second time, the previousIndex is still the same as the initial value.

What is the expected behavior?

previousIndex should reflect the items position when the dragging is started.

What is the current behavior?

It stays the same as the initial value, so neither sorting nor moving works as intended. It only works the first time moving the row, after that it will always jump back/move another item.

What are the steps to reproduce?

Stackblitz here:
https://stackblitz.com/edit/table-drag-n-drop

Is there anything else we should know?

I am using cdkDropList together with Material Table.

P3 cddrag-drop

Most helpful comment

Use [cdkDragData] and compare object by reference.

<table #table #sort="matSort" mat-table [dataSource]="dataSource" 
    class="custom-table" matSort 
    cdkDropList (cdkDropListDropped)="onRowDrop($event)">
...
    <tr mat-row *matRowDef="let row; columns: getDataColumnsIds()" 
         cdkDrag [cdkDragData]="row"></tr>
</table>

```
public onRowDrop(event: CdkDragDrop const previousIndex = this.data.findIndex((row) => row === event.item.data);
moveItemInArray(this.dataSource.data, previousIndex, event.currentIndex);
this.dataSource.data = this.dataSource.data;
}

All 12 comments

The previousIndex depends on the index of an item inside a QueryList that is the result of a ContentChildren query. You might be running into an issue if the item isn't being shifted in the QueryList correctly.

Is there any way I could force a refresh of that query?

Sorry for the delay @lroedal. The indexes in QueryList should update when your data is updated.

I updated the Stackblitz - tried a few approaches, but the only one that works is using JSON.parse(JSON.stringify()). As that is not a good approach, I am open to suggestions.

@lroedal Thx a lot for your StackBlitz, it's a shame to have to clone/recreate data in order to update but it's a little fix not so dirty that can be used waiting for the real one

@MaximeRnR You are welcome. I discovered that the clonedeep from lodash was perfect for this (which is used in the stackblitz now) , so it works well if you only have two tables - but not a good solution if you have multiple drop zones unfortunately. :/

Thanks a lot, this works. I was searching for solution from last 2 days.

Any update or a possible timeline for this issue?

@crisbeto any update on this?

@crisbeto Any update?

Use [cdkDragData] and compare object by reference.

<table #table #sort="matSort" mat-table [dataSource]="dataSource" 
    class="custom-table" matSort 
    cdkDropList (cdkDropListDropped)="onRowDrop($event)">
...
    <tr mat-row *matRowDef="let row; columns: getDataColumnsIds()" 
         cdkDrag [cdkDragData]="row"></tr>
</table>

```
public onRowDrop(event: CdkDragDrop const previousIndex = this.data.findIndex((row) => row === event.item.data);
moveItemInArray(this.dataSource.data, previousIndex, event.currentIndex);
this.dataSource.data = this.dataSource.data;
}

I used an approach similar to what @137x3w has shown.
Instead of re-assigning this.dataSource.data, I called this.table.renderRows():

@ViewChild(MatTable) table: MatTable<any>;

onListDrop(event: CdkDragDrop<any[]>) {
    const previousIndex = this.dataSource.data.findIndex(row => row === event.item.data);
    // Swap the elements around
    moveItemInArray(this.dataSource.data, previousIndex, event.currentIndex);
    // Probably performs better than re-assigning data, since it checks for diffs first
    this.table.renderRows();
}

From the documentation (emphasis mine):

Renders rows based on the table's latest set of data, which was either provided directly as an input or retrieved through an Observable stream (directly or from a DataSource). Checks for differences in the data since the last diff to perform only the necessary changes (add/remove/move rows).
If the table's data source is a DataSource or Observable, this will be invoked automatically each time the provided Observable stream emits a new data array. Otherwise if your data is an array, this function will need to be called to render any changes.

Even though this workaround is simple, this should be fixed in the library, along with new documentation if required.
Plus, this won't scale very well if the underlying data is large: an unnecessary findIndex call will perform worst case O(n).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kara picture kara  路  3Comments

michaelb-01 picture michaelb-01  路  3Comments

shlomiassaf picture shlomiassaf  路  3Comments

crutchcorn picture crutchcorn  路  3Comments

3mp3ri0r picture 3mp3ri0r  路  3Comments