BUG: CdkDropList drop position after scroll is wrong

[abdulkareemnalband]

Reopening #13823 as it was closed as duplicate of #13588 but it is not.

#13588 is about implementing automatic scrolling after drag overshoot

I have already implemented a automatic scrolling (it is a very specific solution for our use case).
When item is dropped after automatic scroll , drop receiver is miscalculated.

Bug

CdkDropList: Drop position is wrong after scrolling

What is the expected behavior?

Drop target should match element under mouse pointer after scroll

What is the current behavior?

Scrolling is not taken into consideration while calculating drop element

What are the steps to reproduce?

https://stackblitz.com/edit/cdk-drop-list-scroll

Steps:

1.Start dragging top most element
2. Drag past container towards down
3. Wait till everything is scrolled down
4. now drag element back into container
5. Drop the element

More info

https://github.com/angular/material2/blob/26c73e04350fa9a159a8e0319b9d4837df04ea5b/src/cdk/drag-drop/drop-list.ts#L388

_cachePositions need to be called again whenever drag is active and scroll container is scrolled.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular CLI: 7.0.3
Node: 10.12.0
OS: win32 x64
Angular: 7.0.1
... animations, cdk, common, compiler, compiler-cli, core, forms
... http, language-service, material, platform-browser
... platform-browser-dynamic, router

Package Version

@angular-devkit/architect 0.10.3
@angular-devkit/build-angular 0.10.3
@angular-devkit/build-optimizer 0.10.3
@angular-devkit/build-webpack 0.10.3
@angular-devkit/core 7.0.3
@angular-devkit/schematics 7.0.3
@angular/cli 7.0.3
@ngtools/webpack 7.0.3
@schematics/angular 7.0.3
@schematics/update 0.10.3
rxjs 6.3.3
typescript 3.1.3
webpack 4.19.1

Looks like the new position is lacking the difference of by much the container has been scrolled. Have you tried adding that difference?

[abdulkareemnalband]

@aidvb
Can you elaborate what you mean by adding difference

@aidvb
Can you elaborate what you mean by adding difference

I am not sure about that but that was my first thought: https://postimg.cc/gallery/eldp5f9k/

[abdulkareemnalband]

@aidvb
that's why this bug reort

@abdulkareemnalband How's it going to solve the problem?

[techy2493]

My temporary work-around for #13588 experiences the same. I'm looking into a code work-around right now. This will be a concept that will need to be addressed in #13588. However it is obvious that this could occur in any case where the scroll position of the container is changed whether through dragging or some other means.

[techy2493]

I'm testing a solution now. Which adds an offset value for both x and y scrolling to the drop-list-ref.ts and then uses that value and the scrollLeft and scrollTop of the element to modify the coordinates sent into _getItemIndexFromPointerPosition(). This seems to work on horizontal scrolling. I have yet to test vertical scrolling. The offset is tracked at the start of each drag so that you do not over-compensate for starting with the container already dragged.

This is not perfect, it may be better to keep track of the item positions rather than offsetting the cursor position values used. The control seems to already do this at some level either on drop or drag start (most likely drop) but I didn't want to dig into that with this right now with the dragging likely to change in the future once #13588 is resolved.

[abdulkareemnalband]

@techy2493 can you show your solution, may be on stackblitz
thanks in advance

[techy2493]

@abdulkareemnalband I will try to link something this afternoon. My changes were to the library itself, so I may just create a diff and make it available.

[techy2493]

@abdulkareemnalband
Here is a diff of the changes I had to make the the CDK library to get a working work-around. It is not ideal, and it is given without warranty or guarantee that it will work in any situation other than my own. However it should give you a place to start to find your own solution.

By viewing this diff you agree that this code is not supported by me or the Angular Material team it is given only as an example with no guarantee... Click here to view diff.

diff --git a/src/cdk/drag-drop/drop-list-ref.ts b/src/cdk/drag-drop/drop-list-ref.ts
index a99e89c9..ff81d21f 100644
--- a/src/cdk/drag-drop/drop-list-ref.ts
+++ b/src/cdk/drag-drop/drop-list-ref.ts
@@ -160,6 +160,9 @@ export class DropListRef<T = any> {
 
   /** Amount of connected siblings that currently have a dragged item. */
   private _activeSiblings = 0;
+  /** Scroll Offset */
+  private _scrollXOffset = 0;
+  private _scrollYOffset = 0;
 
   constructor(
     public element: ElementRef<HTMLElement>,
@@ -192,6 +195,8 @@ export class DropListRef<T = any> {
     this._activeDraggables = this._draggables.slice();
     this._cachePositions();
     this._positionCache.siblings.forEach(sibling => sibling.drop._toggleIsReceiving(true));
+    this._scrollXOffset = this.element.nativeElement.scrollLeft;
+    this._scrollYOffset = this.element.nativeElement.scrollTop;
   }
 
   /**
@@ -334,8 +339,11 @@ export class DropListRef<T = any> {
       return;
     }
 
+    const newItemX = pointerX + (this.element.nativeElement.scrollLeft - this._scrollXOffset)
+    const newItemY = pointerY + (this.element.nativeElement.scrollTop - this._scrollYOffset)
+
     const siblings = this._positionCache.items;
-    const newIndex = this._getItemIndexFromPointerPosition(item, pointerX, pointerY, pointerDelta);
+    const newIndex = this._getItemIndexFromPointerPosition(item, newItemX, newItemY, pointerDelta);
 
     if (newIndex === -1 && siblings.length > 0) {
       return;
@@ -463,6 +471,8 @@ export class DropListRef<T = any> {
     this._positionCache.siblings = [];
     this._previousSwap.drag = null;
     this._previousSwap.delta = 0;
+    this._scrollXOffset = 0;
+    this._scrollYOffset = 0;
   }
 
   /**

[abdulkareemnalband]

@techy2493 thanks
I will try this later today

[codeweazel]

any news on this one?

[varminas]

I'm also very eager to get updated on this ticket.