Components: 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

1. Drag past container towards down
2. Wait till everything is scrolled down
3. now drag element back into container
4. 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

[ghost]

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

[ghost]

@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

[ghost]

@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.

[amaksimovich2]

I tried to apply code from @techy2493 and faced with needing to possibility to define scrolling container element for offset calculation. I have scrolling container with droplist elements each of which contains another droplist.

[techy2493]

@amaksimovich2 So something like a cdkScrollableParent would be useful to you as an override to using the CDK's own element to get the offset from?

Or would you need to additionally use that parent's offsets?

If the latter it may be that the calculation for drop needs to be more dynamic and less tied to the start position because calculating N number of offsets would be difficult, and might not perform well.

[techy2493]

@amaksimovich2 Have you tried setting the root element with cdkDragRootElement to see if that provides the behavior you are looking for cdkDragRootElement?

[Musuel]

Hello,

I developped a directive allowing a drop list to automatically scroll horizontally/vertically and refresh drop positions. Simply put the dropListScroller directive on the CdkDropList.
_Note that I had to use a private property of DropListRef: _itemPositions._

Here is the stackblitz: drop-list-scroller

Let me know if it fit your needs.

@crisbeto Do you think it would be possible to make _itemPositions public in DropListRef ? What do you think about this kind of directive ?

It may be related to #13588.

[arora-kushal]

@Musuel - This works fine when you have very less items say 10. But it is giving same problem when you have a large list, say you have 500 items.. I just tested it by adding 400 items in your solution and it is giving the same problem.

Eagrly waiting for this problme to be fixed.

[chandujindham1756]

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

[chandujindham1756]

Is there any update on this issue?

[Musuel]

Hello Arora,
Sorry for the late reply, I didn’t have any time to give an eye on the issue you complained about.
I’m glad you found a workaround based on the solution I provided.
Don not hesitate to share a stackblitz with the working workaround for others :)
(And me)

[chandujindham1756]

Hello @arora-kushal ,

Can you please share your developed workaround solution link.

[arora-kushal]

Sorry Guys... The above workaround (posted by me) was not working properly... so I deleted it..

[Anzil-Aufait]

Hello,

I developped a directive allowing a drop list to automatically scroll horizontally/vertically and refresh drop positions. Simply put the dropListScroller directive on the CdkDropList.
_Note that I had to use a private property of DropListRef: _itemPositions._

Here is the stackblitz: drop-list-scroller

Let me know if it fit your needs.

@crisbeto Do you think it would be possible to make _itemPositions public in DropListRef ? What do you think about this kind of directive ?

It may be related to #13588.

Hello , do you know how to add this in a mat tree. like this .
Anyway thanks for your demo.

[Musuel]

Hello,
I’ll try to have a look asap but no guaranty, I’m in a rush for now.

[MiszczProgramowania]

I think a lot of projects are waiting for it. How far this is on roadmap ?

[RadimFornusek]

For regular cases the drop position when scrolling (with scrolled element being parent of the drop list) was sorted in Angular CDK version 9.1 which allows to set a different scrolling elements for DropListRef using withScrollableParents method.

See my example here. Usage of the method is in ngAfterViewInit.
https://stackblitz.com/edit/drag-drop-scroll

Difference from the original reported example is that Angular here handles the scrolling itself, there is no custom code for it. So it doesn't allow the big "overshoot" of drag outside of the element, you need to keep the element close to bottom or to drag (or scroll using a mouse wheel).

[amaksimovich2]

Difference from the original reported example is that Angular here handles the scrolling itself, there is no custom code for it. So it doesn't allow the big "overshoot" of drag outside of the element, you need to keep the element close to bottom or to drag (or scroll using a mouse wheel).

This approach is not working from 9.2.1 CDK version.
It can be fixed by adding cdkScrollable to scrollable container, and scrollable directive will call withScrollableParents method itself.
I've modified your example: stackblitz

[crisbeto]

All the positioning issues after scrolling should be resolved in the latest version. As for marking which containers can be scrollable, @amaksimovich2 is correct that cdkScrollable should be used.

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{_This action has been performed automatically by a bot._}