Skip to content

Commit 1a3c35a

Browse files
authored
fix(drag-drop): only return item to initial index if the new container's sorting is disabled (#18706)
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.
1 parent e87de13 commit 1a3c35a

File tree

2 files changed

+45
-3
lines changed

2 files changed

+45
-3
lines changed

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4487,6 +4487,48 @@ describe('CdkDrag', () => {
44874487
expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled();
44884488
}));
44894489

4490+
it('should enter an item into the correct index when returning to the initial container, if ' +
4491+
'sorting is enabled', fakeAsync(() => {
4492+
const fixture = createComponent(ConnectedDropZones);
4493+
fixture.detectChanges();
4494+
4495+
const groups = fixture.componentInstance.groupedDragItems;
4496+
const dropZones = fixture.componentInstance.dropInstances.map(d => d.element.nativeElement);
4497+
const item = groups[0][1];
4498+
const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
4499+
4500+
// Explicitly enable just in case.
4501+
fixture.componentInstance.dropInstances.first.sortingDisabled = false;
4502+
startDraggingViaMouse(fixture, item.element.nativeElement);
4503+
4504+
const placeholder = dropZones[0].querySelector('.cdk-drag-placeholder')!;
4505+
4506+
expect(placeholder).toBeTruthy();
4507+
expect(dropZones[0].contains(placeholder))
4508+
.toBe(true, 'Expected placeholder to be inside the first container.');
4509+
expect(getElementIndexByPosition(placeholder, 'top'))
4510+
.toBe(1, 'Expected placeholder to be at item index.');
4511+
4512+
dispatchMouseEvent(document, 'mousemove', targetRect.left + 1, targetRect.top + 1);
4513+
fixture.detectChanges();
4514+
4515+
expect(dropZones[1].contains(placeholder))
4516+
.toBe(true, 'Expected placeholder to be inside second container.');
4517+
expect(getElementIndexByPosition(placeholder, 'top'))
4518+
.toBe(3, 'Expected placeholder to be at the target index.');
4519+
4520+
const nextTargetRect = groups[0][3].element.nativeElement.getBoundingClientRect();
4521+
4522+
// Return the item to an index that is different from the initial one.
4523+
dispatchMouseEvent(document, 'mousemove', nextTargetRect.left + 1, nextTargetRect.top + 1);
4524+
fixture.detectChanges();
4525+
4526+
expect(dropZones[0].contains(placeholder))
4527+
.toBe(true, 'Expected placeholder to be back inside first container.');
4528+
expect(getElementIndexByPosition(placeholder, 'top'))
4529+
.toBe(2, 'Expected placeholder to be at the index at which it entered.');
4530+
}));
4531+
44904532
it('should toggle a class when dragging an item inside a wrapper component component ' +
44914533
'with OnPush change detection',
44924534
fakeAsync(() => {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -853,10 +853,10 @@ export class DragRef<T = any> {
853853
this._dropContainer!.exit(this);
854854
// Notify the new container that the item has entered.
855855
this._dropContainer = newContainer!;
856-
this._dropContainer.enter(this, x, y,
857-
// If we're re-entering the initial container,
856+
this._dropContainer.enter(this, x, y, newContainer === this._initialContainer &&
857+
// If we're re-entering the initial container and sorting is disabled,
858858
// put item the into its starting index to begin with.
859-
newContainer === this._initialContainer ? this._initialIndex : undefined);
859+
newContainer.sortingDisabled ? this._initialIndex : undefined);
860860
this.entered.next({
861861
item: this,
862862
container: newContainer!,

0 commit comments

Comments
 (0)