fix(drag-drop): incorrectly displaying items if user skips multiple positions while sorting

src/cdk-experimental/drag-drop/drag.spec.ts

@@ -675,6 +675,60 @@ describe('CdkDrag', () => {
       flush();
     }));
 
+    it('should lay out the elements correctly, if an element skips multiple positions when ' +
+      'sorting vertically', fakeAsync(() => {
+        const fixture = createComponent(DraggableInDropZone);
+        fixture.detectChanges();
+
+        const items = fixture.componentInstance.dragItems.map(i => i.element.nativeElement);
+        const draggedItem = items[0];
+        const {top, left} = draggedItem.getBoundingClientRect();
+
+        dispatchMouseEvent(draggedItem, 'mousedown', left, top);
+        fixture.detectChanges();
+
+        const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
+        const targetRect = items[items.length - 1].getBoundingClientRect();
+
+        // Add a few pixels to the top offset so we get some overlap.
+        dispatchMouseEvent(document, 'mousemove', targetRect.left, targetRect.top + 5);
+        fixture.detectChanges();
+
+        expect(getElementSibligsByPosition(placeholder, 'top').map(e => e.textContent!.trim()))
+            .toEqual(['One', 'Two', 'Three', 'Zero']);
+
+        dispatchMouseEvent(document, 'mouseup');
+        fixture.detectChanges();
+        flush();
+      }));
+
+    it('should lay out the elements correctly, if an element skips multiple positions when ' +
+      'sorting horizontally', fakeAsync(() => {
+        const fixture = createComponent(DraggableInHorizontalDropZone);
+        fixture.detectChanges();
+
+        const items = fixture.componentInstance.dragItems.map(i => i.element.nativeElement);
+        const draggedItem = items[0];
+        const {top, left} = draggedItem.getBoundingClientRect();
+
+        dispatchMouseEvent(draggedItem, 'mousedown', left, top);
+        fixture.detectChanges();
+
+        const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
+        const targetRect = items[items.length - 1].getBoundingClientRect();
+
+        // Add a few pixels to the left offset so we get some overlap.
+        dispatchMouseEvent(document, 'mousemove', targetRect.right - 5, targetRect.top);
+        fixture.detectChanges();
+
+        expect(getElementSibligsByPosition(placeholder, 'left').map(e => e.textContent!.trim()))
+            .toEqual(['One', 'Two', 'Three', 'Zero']);
+
+        dispatchMouseEvent(document, 'mouseup');
+        fixture.detectChanges();
+        flush();
+      }));
+
     it('should clean up the preview element if the item is destroyed mid-drag', fakeAsync(() => {
       const fixture = createComponent(DraggableInDropZone);
       fixture.detectChanges();
@@ -1343,13 +1397,14 @@ function dragElementViaTouch(fixture: ComponentFixture<any>,
 
 /** Gets the index of an element among its siblings, based on their position on the page. */
 function getElementIndexByPosition(element: HTMLElement, direction: 'top' | 'left') {
-  if (!element.parentElement) {
-    return -1;
-  }
+  return getElementSibligsByPosition(element, direction).indexOf(element);
+}
 
-  return Array.from(element.parentElement.children)
-      .sort((a, b) => a.getBoundingClientRect()[direction] - b.getBoundingClientRect()[direction])
-      .indexOf(element);
+/** Gets the siblings of an element, sorted by their position on the page. */
+function getElementSibligsByPosition(element: HTMLElement, direction: 'top' | 'left') {
+  return element.parentElement ? Array.from(element.parentElement.children).sort((a, b) => {
+    return a.getBoundingClientRect()[direction] - b.getBoundingClientRect()[direction];
+  }) : [];
 }
 
 /**

src/cdk-experimental/drag-drop/drop.ts

@@ -24,6 +24,7 @@ import {CdkDrag} from './drag';
 import {CdkDragExit, CdkDragEnter, CdkDragDrop} from './drag-events';
 import {CDK_DROP_CONTAINER} from './drop-container';
 import {DragDropRegistry} from './drag-drop-registry';
+import {moveItemInArray} from './drag-utils';
 
 /** Counter used to generate unique ids for drop zones. */
 let _uniqueIdCounter = 0;
@@ -214,45 +215,59 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
    */
   _sortItem(item: CdkDrag, xOffset: number, yOffset: number): void {
     const siblings = this._positionCache.items;
-    const isHorizontal = this.orientation === 'horizontal';
     const newIndex = this._getItemIndexFromPointerPosition(item, xOffset, yOffset);
-    const placeholder = item.getPlaceholderElement();
 
     if (newIndex === -1 && siblings.length > 0) {
       return;
     }
 
+    const isHorizontal = this.orientation === 'horizontal';
     const currentIndex = siblings.findIndex(currentItem => currentItem.drag === item);
-    const currentPosition = siblings[currentIndex];
-    const newPosition = siblings[newIndex];
-
-    // Figure out the offset necessary for the items to be swapped.
-    const offset = isHorizontal ?
-        currentPosition.clientRect.left - newPosition.clientRect.left :
-        currentPosition.clientRect.top - newPosition.clientRect.top;
-    const topAdjustment = isHorizontal ? 0 : offset;
-    const leftAdjustment = isHorizontal ? offset : 0;
-
-    // Since we've moved the items with a `transform`, we need to adjust their cached
-    // client rects to reflect their new position, as well as swap their positions in the cache.
-    // Note that we shouldn't use `getBoundingClientRect` here to update the cache, because the
-    // elements may be mid-animation which will give us a wrong result.
-    this._adjustClientRect(currentPosition.clientRect, -topAdjustment, -leftAdjustment);
-    currentPosition.offset -= offset;
-    siblings[currentIndex] = newPosition;
-
-    this._adjustClientRect(newPosition.clientRect, topAdjustment, leftAdjustment);
-    newPosition.offset += offset;
-    siblings[newIndex] = currentPosition;
-
-    // Swap the placeholder's position with the one of the target draggable.
-    placeholder.style.transform = isHorizontal ?
-        `translate3d(${currentPosition.offset}px, 0, 0)` :
-        `translate3d(0, ${currentPosition.offset}px, 0)`;
-
-    newPosition.drag.element.nativeElement.style.transform = isHorizontal ?
-        `translate3d(${newPosition.offset}px, 0, 0)` :
-        `translate3d(0, ${newPosition.offset}px, 0)`;
+    const currentPosition = siblings[currentIndex].clientRect;
+    const newPosition = siblings[newIndex].clientRect;
+    const delta = currentIndex > newIndex ? 1 : -1;
+
+    // How many pixels the item's placeholder should be offset.
+    const itemOffset = isHorizontal ? newPosition.left - currentPosition.left :
+                                      newPosition.top - currentPosition.top;
+
+    // How many pixels all the other items should be offset.
+    const siblingOffset = isHorizontal ? currentPosition.width * delta :
+                                         currentPosition.height * delta;
+
+    // Save the previous order of the items before moving the item to its new index.
+    // We use this to check whether an item has been moved as a result of the sorting.
+    const oldOrder = siblings.slice();
+
+    // Shuffle the array in place.
+    moveItemInArray(siblings, currentIndex, newIndex);
+
+    siblings.forEach((sibling, index) => {
+      // Don't do anything if the position hasn't changed.
+      if (oldOrder[index] === sibling) {
+        return;
+      }
+
+      const isDraggedItem = sibling.drag === item;
+      const offset = isDraggedItem ? itemOffset : siblingOffset;
+      const elementToOffset = isDraggedItem ? item.getPlaceholderElement() :
+                                              sibling.drag.element.nativeElement;
+
+      // Update the offset to reflect the new position.
+      sibling.offset += offset;
+
+      // Since we're moving the items with a `transform`, we need to adjust their cached
+      // client rects to reflect their new position, as well as swap their positions in the cache.
+      // Note that we shouldn't use `getBoundingClientRect` here to update the cache, because the
+      // elements may be mid-animation which will give us a wrong result.
+      if (isHorizontal) {
+        elementToOffset.style.transform = `translate3d(${sibling.offset}px, 0, 0)`;
+        this._adjustClientRect(sibling.clientRect, 0, offset);
+      } else {
+        elementToOffset.style.transform = `translate3d(0, ${sibling.offset}px, 0)`;
+        this._adjustClientRect(sibling.clientRect, offset, 0);
+      }
+    });
   }
 
   /**
@@ -321,8 +336,8 @@ export class CdkDrop<T = any> implements OnInit, OnDestroy {
   /**
    * Updates the top/left positions of a `ClientRect`, as well as their bottom/right counterparts.
    * @param clientRect `ClientRect` that should be updated.
-   * @param top New value for the `top` position.
-   * @param left New value for the `left` position.
+   * @param top Amount to add to the `top` position.
+   * @param left Amount to add to the `left` position.
    */
   private _adjustClientRect(clientRect: ClientRect, top: number, left: number) {
     clientRect.top += top;