bug(drag-drop): previousIndex is not correct if list contains invisible items

[kurakin-oleksandr]

Reproduction

https://stackblitz.com/edit/angular-rs1rtm

move "Go home" item into "Done" list

Steps to reproduce:

1. Define two drag-drop lists
2. Add both visible and invisible (display: none) items into lists
3. Drag'n'Drop some items from one list into another
4. Sometimes wrong (invisible) item is moved (depending on an item position) (on cdkDropListDropped event.previousIndex of a selected item is not an actual index in the list of items but index of only visible items in this list)

Expected Behavior

Correct (visible) item which was selected should be moved into list (on cdkDropListDropped event.previousIndex must be an index of an item in a list regardless of visibility)

Actual Behavior

On cdkDropListDropped event.previousIndex of a selected item is not an actual index in the list of items but index of only visible items in this list

Environment

• Angular: 9.1.0
• CDK/Material: 9.1.0-9.2.0
• Browser(s): Chrome 80.0.3987.149 (but I think it reproducible in all browsers)
• Operating System: macOS (but I think it reproducible in all OS)

[crisbeto]

This is behaving as expected. The index is calculated based on the items that are present, not whether they're visible or invisible. Checking for visibility won't be very efficient, because we'd have to trigger extra layouts, and it probably won't be very accurate either, because there are many ways of hiding an item using CSS.

[kurakin-oleksandr]

But this is how it worked before 9.1.0.
Currently I see this as a bug because it is not mentioned in CHANGELOG.md and it breaks backward compatibility

[crisbeto]

If that's how it worked before 9.1.0, it was completely coincidental and not how it was intended to work.

[kurakin-oleksandr]

According to documentation I do everything correct: I define lists with cdkDropList and set items with cdkDropListData. So library for sure know about all items. Documentation says nothing about CSS properties which might affect this list (f.e. display: none)
So I still believe it is a bug

[crisbeto]

The CDK doesn't do anything with the cdkDropListData, the property is there for convenience.

[kurakin-oleksandr]

Any updates?

[etiennetalbot]

I just noticed that it messes up the previousIndex when you add display: none to the placeholder of the item.

In my opinion it should not be that way. I have a list from which I have disabled sorting and that serves only as an infinite source of draggable items (items are copied when dropped in other lists). I want to hide the placeholder in that source list, but not in other lists, and I want my placeholder to look a specific way, so I use *cdkDragPlaceholder... But when I hide the placeholder in the source list, the drop event sets the previousIndex to 0, regardless of the dragged item's original position, just because the placeholder is hidden.

Worked on 9.0.1, but broken on 9.1.0

[crisbeto]

It wouldn't be too difficult to solve on our end because we already measure the item when dragging starts and we can skip it if its 0px by 0px. The problem is that if we start excluding items from the list, the indexes won't line up with the data anymore. E.g. if you have a list of [1, 2, 3] where 1 is invisible, excluding the invisible item from the list and swapping 3 and 2 will emit an event with previousIndex: 1; currentIndex: 0 even though the indexes in the data are previousIndex: 2; currentIndex: 1.

[kurakin-oleksandr]

@crisbeto the problem occurs because indexes ARE wrong if invisible items are present in the list
Please check stackblitz

[MA-Maddin]

Faced the same issue.

Workaround:

Add cdkDragData to the cdkDrag element template.
In the OP's StackBlitz that is:

<div *ngFor="let item of todo"
     cdkDrag
     [cdkDragData]="item">{{item.name}}</div>

<!-- ... and on the "done" site as well -->

Now you can look on the receiving cdkDropList's cdkDropListDropped handler for the correct index:

drop(event: CdkDragDrop<any>) {
  if (event.previousContainer !== event.container) {

    // 'name' must be unique in this case of course
    const correctPreviousIndex = event.previousContainer.data.findIndex(
      (pcItem: any) => pcItem.name === event.item.data.name);

    transferArrayItem(
      event.previousContainer.data,
      event.container.data,
      correctPreviousIndex,
      event.currentIndex
    );
  }
}