Possible performance issue with dragover

[danjford]

• I'm submitting a ... [x] bug report [x] feature request [ ] question about the decisions made in the repository
• Do you want to request a feature or report a bug?

A feature and possible also a bug.

• What is the current behavior?

In firefox (and probably other browsers), it seems that the dragover event is fired hundreds of times when an item is dragged over an element. This leads to a drop in FPS and it seems to be made worse depending on how many elements have had draggable / droppable applied.

The performance hit seems to be worse on firefox than e.g. chrome.

• If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar.

Taken using Firefox performance analyser:

Most of these events are dragover.

• What is the expected behavior?

Dragover not to fire so many times.

• What is the motivation / use case for changing the behavior?

Improved performance, keep the FPS from dropping so low.

• Please tell us about your environment:
• Browser: [all | Firefox 49.0.1 ]
• Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, gitter, etc)

Suggestion how to fix:

At the moment when onDragOverCallback it will only fire if this._dragDropService.isDragged is true. Then a class is added this._elem.classList.add(this._config.onDragOverClass);. Maybe a further check should be done so that the code is only fired if this class has not already been added to the target element e.g.:

    _onDragOverCallback (event: MouseEvent) {
        if (this._dragDropService.isDragged && !this._elem.classList.contains(this._config.onDragOverClass);) {
            this._elem.classList.add(this._config.onDragOverClass);
            this.onDragOver.emit({dragData: this._dragDropService.dragData, mouseEvent: event});
        }
    };

[akserg]

Hi @danjford Thank you for your investigation. I will have a look.

[joshvanallen]

Hello! So currently I am running into a performance issue and this looks related. I have dnd-sortable being applied to a complex object because of the nature of this application. With 2 elements, if I start dragging an element between the 2 spots quickly, i noticed a ton of work was being on during the dragover event that would not let the page repaint quickly. I noticed a huge performance hit when I have 88 elements. Because of the nature of the environment I work in, it would be difficult to provide actual code but the element structure I'm flowing relates to the dynamic forms in the Angular cookbook. https://angular.io/docs/ts/latest/cookbook/dynamic-form.html

[joshvanallen]

Any update on this @akserg?

[akserg]

Hi @joshvanal

I have the similar issue inside of my project as well. I have a fix and still test it until rest of the week.

[joshvanallen]

Awesome @akserg. If there is anything I can do to help please let me know!

[alex88]

Same here, dragover event called so many times that makes the application freeze and takes like 3-4 seconds to show the new item position (dragging a div column in a list of 5 elements). Have you set a debounce as a fix? PS: is there a branch we can check with the fix?

[alan345]

Hi same issue here:

When I Drag in plunkr (http://embed.plnkr.co/JbG8Si), it works fine:

But on my app:

Memory get saturated.

I am using angular4 and bootstrap4 Thanks

EDITED

I have discovered the most used Activity is driven by 'angular2-jwt'. It represents 93% when we drag an element. Question is now, why JwtHelper is called when we drag an element. @danjford, @joshvanal, @alex88 do you also have angular2-jwt installed?

[alex88]

@alan345 I have, and I have a block in the page that uses the auth service to check if the user is logged in, in my header it shows if the current user is logged in. I had the same issue because I was checking with angular-jwt if the token was expired and it was resulting in some computation, I've fixed it temporarily by checking only page load and then just return the isLogged stored boolean. I think it would be better if I used an Observable using the async pipe.

However, seems the issue is that angular re-renders the whole page

[alexsalmon]

How's it going @akserg?

[CLEMARCx]

If someone needs a quick workaround: Change the change detection to OnPush and trigger it manually.

import { 
  ...
  ChangeDetectionStrategy
} from '@angular/core';

@Component({
   ...
  changeDetection: ChangeDetectionStrategy.OnPush, // set to OnPush
  ...
})
export class BoxComponent {
   ...
}

[KDingeling]

Would be great to hear some news on this, currently I'm running into performance issues while dragging elements around, when there are ~8-10 elements in my sortable container. I see in the elements view of chrome that all draggable + droppable elements get updated permantly (divs are flashing in the DOM) while dragging an element anywhere on the screen. Will try to set the change detection to onPush for now.

Update: In my case, it was not ng2-dnd what caused the performance issues. Instead of binding dropped content to [innerHTML], I had an own directive to sanitize the content after I dropped it. This somehow made it impossible for the ChangeDetector to detect if a change actually happened or not, so it rerendered the complete view all the time while dragging something.

[AnthonySaldana]

So i'm dealing with a similar issue where the dragging is just so laggy when theres a lot going on the page. I have a few components that were repeated in a loop and I can drag it across those components but it's so laggy

The change detector seems to be the cause of the lag. The workaround i did for this was to detach the change detector on drag start and to reattach it on drop. You could also set the change detection strategy to on push but this way you get to keep the change detector strategy and just turn it off while dragging.

constructor( private cd: ChangeDetectorRef) { }

onDragStart(){
    this.cd.detach();
}
onDrop(){
    this.cd.reattach();
}

[attodorov]

I am experiencing exactly the same issue, there's a severe performance hit, dropping is sometimes not even possible (takes more than 20-30 seconds to complete). It also depends how many child elements the draggable has.

Is there an update on this issue?

Thanks

[IdanCo]

Excellent Solutions!!

@CLEMARCx solution is great, just don't forget to add this.cd.markForCheck(); where needed (as explained here)

@AnthonySaldana solution works great as well and much more targeted. Notice though that onDragStart and onDragEnd aren't documented (#195 ) but you can see them here. I also had to use onDragSuccess for cases the drop event was inside a droppable area.

Both solutions add a little code ugliness, but it does the trick until @akserg will be more available (great project BTW)

[basileos]

i faced the same issue, because of having function's call inside draggable element, this function returns text or formatted date due to conditions, so it was called thousands times during dragging, i fixed this by placing already prepared data into the template.

[shashidhargm]

@KDingeling Hi, Even I'm facing the issue in my angular app. Please provide the solution if you have solved your issue.
I am using Angular 4.4.3 and Primeng 4.2.1.

[KDingeling]

@shashidhargm Hey, I will take a look next week when I'm back at work. Can't remember it in detail.

[shashidhargm]

@KDingeling Thanks :)

[jmesa-sistel]

I had similar issue my solution is fork this lib and do the next modification in AbstractComponent

Change

this.elem.ondragover = (event: DragEvent) => {
     this._onDragOver(event);
     if (event.dataTransfer) {
         event.dataTransfer.dropEffect = this.config.dropEffect.name;
     }
     return false;
};

by

        this.ngZone.runOutsideAngular(() => {
            this.elem.ondragover = (event: DragEvent) => {
                this._onDragOver(event);
                if (event.dataTransfer) {
                    event.dataTransfer.dropEffect = this.config.dropEffect.name;
                }
                return false;
            };
        });

This change implies change the constructors to inject ngZone I am not using dnd as a library I have copied all files in my app to do the tests. Also I made my own changes to improve performance a little and clean code. But the main change to improve performance is this. Also as @AnthonySaldana suggested I detach and reattach cdr in onDragStart / onDragEnd in the library, so I do not need to do it in all components that use dnd but I reduce detectChanges timeout to 20ms to get about 50fps

[joedjc]

@jmesa-sistel @akserg I also found running the drag events outside the Angular zone has significant performance benefits. There are still issues with lots of elements but this is much better. Created a PR here: https://github.com/akserg/ng2-dnd/pull/269

[jmesa-sistel]

@joedjc Remenber to execute the events inside angular For example:

_onDragStartCallback(event: MouseEvent) {
        this.dragDropService.isDragged = true;
        this.dragDropService.dragData = this.dragData;
        this.dragDropService.onDragSuccessCallback = this.dragSuccess;
        setTimeout(() => {
            this.elem.classList.add(this.config.onDragStartClass);
        }, 0);
        this.ngZone.run(() => {
            this.dragStart.emit({ dragData: this.dragData, mouseEvent: event });
        });
    }

The setTimeout fix another bug when you try to drag an element and you click near the border I have rewrite the library to fix 4 or 5 bugs and I have clean the code and now the performance is not a problem. I will try to publish soon, when I have time, too busy at work. Too many changes to make a PR.

[apreeti]

@jmesa-sistel @akserg I also found running the drag events outside the Angular zone has significant performance benefits. There are still issues with lots of elements but this is much better. Created a PR here: #269

It really is giving lots of performance benefits!! Thank you :)