ngx-drag-drop icon indicating copy to clipboard operation
ngx-drag-drop copied to clipboard

Dragging quickly will persist "pointer-events: none"

Open LoaderB0T opened this issue 2 years ago • 18 comments

Describe the bug When dragging an element quickly and repeatedly, the element style of "pointer-events: none" will persist in the element and it cannot be dragged any more. This can be reproduced manually and for some reason it always happens in some of my e2e tests of the first or second drag...

To Reproduce You can actually reproduce this on the demo page: https://reppners.github.io/ngx-drag-drop

Steps to reproduce the behavior:

  1. Go to the demo page
  2. Grab an item
  3. move it a little (not into a drop zone)
  4. let it go
  5. Repeat steps 2-4 quickly

Expected behavior The element should be draggable without problems no matter how often I do it.

Screenshots A video recording of the repro.

Desktop (please complete the following information):

  • OS: Windows 11
  • Browser Chrome & Firefox
  • Version ngx-drag-drop 16.1.0

Additional context In playwright I have this as a stable failing test (not flaky, always failing at the same place) So I assume the bug has some sort of "stable" failing behavior.

LoaderB0T avatar Oct 16 '23 09:10 LoaderB0T

We have the same issue, but it's not triggered only by dragging quickly. It happens because the dragEnd callback is not triggered, causing the pointer-events: none style to remain on the element. The pointer-events style is also not updated after the item is dropped. This breaks our features that rely on hover, so we would greatly appreciate a bug fix.

skaindl avatar Dec 11 '23 14:12 skaindl

@skaindl This is the same bug, waiting for a merge to get it patched

Elytes avatar Dec 12 '23 08:12 Elytes

I tried the fix from that PR but it doesn't solve the entire problem. Even when isDragStarted is true, the pointer-events: none style doesn't seem to be properly removed from the element. There seems to be an event handler binding missing for the dragend callback, but as that also isn't currently called on drop, the style also needs to be removed when an item is successfully dropped.

skaindl avatar Dec 12 '23 08:12 skaindl

There is a line for that https://github.com/reppners/ngx-drag-drop/blob/280b10c71c78aec812e76fe1a241d2035547f5aa/projects/dnd/src/lib/dnd-draggable.directive.ts#L176 on drag end we are supposed to set pointer-events on "unset"

@HostListener('dragend', ['$event']) onDragEnd(event: DragEvent) {
    if (!this.draggable || !this.isDragStarted) {
      return;
    }
    // get drop effect from custom stored state as its not reliable across browsers
    const dropEffect = dndState.dropEffect;

    this.renderer.setStyle(this.dragImage, 'pointer-events', 'unset');

Is your problem related to a persistent "point-events: none" each time you successfully drop something ?

Elytes avatar Dec 12 '23 09:12 Elytes

Yes, but also when the dragging is cancelled. It's a part of our app that hasn't changed aside from updating the library. What I noticed yesterday is that when I manually add an eventListener on elementRef.nativeElement for dragend in ngAfterViewInit like for the drag event, only then is the onDragEnd method actually called when the drag is cancelled. For some reason though, it still isn't fired after a drop event, only an unsuccessful drag.

Screenshot 2023-12-12 at 10 12 00 Here is the drag element we are using, but like I said, it used to work without a problem.

Note: this component also uses OnPush change detection.

skaindl avatar Dec 12 '23 09:12 skaindl

Is the patch #163 causing this problem or just the current latest version of the library ? Do you have something that could prevent the dragEnd event from getting called ?

Elytes avatar Dec 12 '23 09:12 Elytes

We're not exactly sure with which version the bug first appeared (we're currently on version 16.1.0), but since I tried it with the patch #163 as well as version 16.0.0, I suspect it maybe occurred with the upgrade from 15 -> 16. I'm still trying to narrow it down.

I made sure that updates to the steps array are not the problem, by not updating the array after drop so that the list items don't change. The only things that may still be a factor is that the li is nested within an ng-container element with a condition that remains true, which is a child of a mat-drawer-container element. I'll try to find out if there's anything with mat-drawer-container that might conflict.

skaindl avatar Dec 12 '23 09:12 skaindl

Try to check if u aren't preventing dragEnd from getting called, else, try to reproduce the bug with a simple draggable div, if you can't reproduce it with a simple div, check all the DOM of your elements and check if nothing is interacting with dragEnd and check for the component detection (use signals or check that the variable reference of the array change each time) but I doubt changeDetection would prevent an event from getting called

Elytes avatar Dec 12 '23 09:12 Elytes

I think I found the problem. Our dndHandle is only shown on hover, but when you add pointer-events: none to the parent element, the hover state automatically changes and the dndHandle is removed during drag. What do you suggest in that case? Why is pointer-events: none necessary in the first place?

skaindl avatar Dec 12 '23 10:12 skaindl

Oh, check my another fix #162, it's fixing problems with unregistering drag handles if they are not in the DOM anymore ;) pointer-events: none is required to prevent clicking on the current dragging image of the element, as we are using the same DOM object if we want to prevent you from clicking on a button that you are dragging, or trigger another drag from an element that you are dragging, it's required to disable user inputs

Elytes avatar Dec 12 '23 14:12 Elytes

Hi, we're using the library from some time now and it works really nice, but we are experiencing this issue since updating to 16? do you have any idea if this will be merged soon?

andra-flowx avatar Apr 07 '24 09:04 andra-flowx

Hey @andra-flowx, Well there's still no sign of merging but you can still use this branch as a temp fix until someone merge it..

Elytes avatar Apr 07 '24 11:04 Elytes

Hello, any news regarding merge request ?

kenancosic avatar May 27 '24 12:05 kenancosic

@kenancosic Still no news 🤷‍♀️ Hope soon, until then feel free to fork / clone from it :/

Elytes avatar May 27 '24 12:05 Elytes

Any news on this? We've encountered the same issue, and tried to figure out how to avoid it.

yakeer avatar Jun 10 '24 08:06 yakeer

Any news on this? We've encountered the same issue, and tried to figure out how to avoid it.

Still nothing, just clone / fork #162 and #163 so you'll be ok

Elytes avatar Jun 11 '24 11:06 Elytes

Any plans to merge these two PR's into the master?

Any news on this? We've encountered the same issue, and tried to figure out how to avoid it.

Still nothing, just clone / fork #162 and #163 so you'll be ok

yakeer avatar Jul 07 '24 11:07 yakeer

Any plans to merge these two PR's into the master?

Any news on this? We've encountered the same issue, and tried to figure out how to avoid it.

Still nothing, just clone / fork #162 and #163 so you'll be ok

I sadly don't have any rights on this repository so we still have to wait for the author to wake up rn

Elytes avatar Jul 07 '24 18:07 Elytes

Hi, for everyone wondering, this issue seems to be fixed in the latest version v 18.0.2 🥳

LoaderB0T avatar Aug 08 '24 07:08 LoaderB0T