From 4f0902d41dc5269dbd1854b24ce215e72db2ce5a Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Wed, 4 Mar 2020 16:56:20 +0100
Subject: [PATCH] fix(drag-drop): only return item to initial index if the new
 container's sorting is disabled

When an item is moved into a different container and then returned within the same drag sequence, we place it at its initial index in order to make it behave correctly when sorting is disabled, but the problem is that the same logic had bled into the case where sorting is enabled. This was causing some weird behavior. These changes add an extra check to ensure that it only applies for disabled sorting.

Fixes #18697.
---
 src/cdk/drag-drop/directives/drag.spec.ts | 42 +++++++++++++++++++++++
 src/cdk/drag-drop/drag-ref.ts             |  6 ++--
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/src/cdk/drag-drop/directives/drag.spec.ts b/src/cdk/drag-drop/directives/drag.spec.ts
index 4bf2e14f2d74..2638c8d4ce44 100644
--- a/src/cdk/drag-drop/directives/drag.spec.ts
+++ b/src/cdk/drag-drop/directives/drag.spec.ts
@@ -4463,6 +4463,48 @@ describe('CdkDrag', () => {
         expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled();
       }));
 
+    it('should enter an item into the correct index when returning to the initial container, if ' +
+      'sorting is enabled', fakeAsync(() => {
+        const fixture = createComponent(ConnectedDropZones);
+        fixture.detectChanges();
+
+        const groups = fixture.componentInstance.groupedDragItems;
+        const dropZones = fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
+        const item = groups[0][1];
+        const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
+
+        // Explicitly enable just in case.
+        fixture.componentInstance.dropInstances.first.sortingDisabled = false;
+        startDraggingViaMouse(fixture, item.element.nativeElement);
+
+        const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!;
+
+        expect(placeholder).toBeTruthy();
+        expect(dropZones[0].contains(placeholder))
+            .toBe(true, 'Expected placeholder to be inside the first container.');
+        expect(getElementIndexByPosition(placeholder, 'top'))
+            .toBe(1, 'Expected placeholder to be at item index.');
+
+        dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1);
+        fixture.detectChanges();
+
+        expect(dropZones[1].contains(placeholder))
+            .toBe(true, 'Expected placeholder to be inside second container.');
+        expect(getElementIndexByPosition(placeholder, 'top'))
+            .toBe(3, 'Expected placeholder to be at the target index.');
+
+        const nextTargetRect = groups[0][3].element.nativeElement.getBoundingClientRect();
+
+        // Return the item to an index that is different from the initial one.
+        dispatchMouseEvent(document, 'mousemove', nextTargetRect.left + 1, nextTargetRect.top + 1);
+        fixture.detectChanges();
+
+        expect(dropZones[0].contains(placeholder))
+            .toBe(true, 'Expected placeholder to be back inside first container.');
+        expect(getElementIndexByPosition(placeholder, 'top'))
+            .toBe(2, 'Expected placeholder to be at the index at which it entered.');
+      }));
+
     it('should toggle a class when dragging an item inside a wrapper component component ' +
            'with OnPush change detection',
        fakeAsync(() => {
diff --git a/src/cdk/drag-drop/drag-ref.ts b/src/cdk/drag-drop/drag-ref.ts
index 556c599a8ce5..a829c486fad5 100644
--- a/src/cdk/drag-drop/drag-ref.ts
+++ b/src/cdk/drag-drop/drag-ref.ts
@@ -853,10 +853,10 @@ export class DragRef<T = any> {
         this._dropContainer!.exit(this);
         // Notify the new container that the item has entered.
         this._dropContainer = newContainer!;
-        this._dropContainer.enter(this, x, y,
-            // If we're re-entering the initial container,
+        this._dropContainer.enter(this, x, y, newContainer === this._initialContainer &&
+            // If we're re-entering the initial container and sorting is disabled,
             // put item the into its starting index to begin with.
-            newContainer === this._initialContainer ? this._initialIndex : undefined);
+            newContainer.sortingDisabled ? this._initialIndex : undefined);
         this.entered.next({
           item: this,
           container: newContainer!,