Skip to content
This repository has been archived by the owner on Mar 10, 2024. It is now read-only.

Performance issue in the dragover event of the dndLists directive. #413

Open
johntdowney opened this issue May 9, 2017 · 9 comments
Open

Comments

@johntdowney
Copy link

Hey, just leaving this here for the author and/or those who care because I spent a little while solving this issue while working with a very large drag and drop list.

Line 339/340 is as follows:

listNode.insertBefore(placeholderNode,
	isFirstHalf ? listItemNode : listItemNode.nextSibling);

Replacing these two lines with

if(isFirstHalf) {
	if(listItemNode.previousSibling != placeholderNode) listNode.insertBefore(placeholderNode, listItemNode);
} else {
	if(listItemNode.nextSibling != placeholderNode) listNode.insertBefore(placeholderNode, listItemNode.nextSibling);
}

dramatically cuts down on the number of DOM layouts and improves the execution time of the dragover event handler. Without this, it's pretty hobbled at around ~60 dnd list items.

Thanks,
John Downey

@tagisen
Copy link

tagisen commented May 16, 2017

amazing job @johntdowney
I had a short list to work with, but with complex moving item. Replacing those lines did the magic. Thanks

@GeorgeChackungal
Copy link

Awesome Job! Can Someone add it into the original source?

@johntdowney
Copy link
Author

Glad I could help! I see this "Close and comment" button, but I'm not sure of the etiquette here regarding whether or not I should close this issue since I opened it, so I'll just hit the comment button instead and assume someone with more authority can close it once the change has been applied.

@orlandovallejos
Copy link

@marceljuenemann please review the PR, and run the minifier.

mhartz82 pushed a commit to ErKatta/angular-drag-and-drop-lists that referenced this issue Sep 29, 2017
@sinzii
Copy link

sinzii commented Mar 25, 2018

Awesome! Thank you so much, that improve a lot than the original.

seyisulu added a commit to seyisulu/angular-drag-and-drop-lists that referenced this issue Jun 7, 2018
See issue marceljuenemann#413 from origin about layout trashing.

Signed-off-by: Emmanuel Olowosulu <eolowo@sulusoft.com>
@abtx
Copy link

abtx commented Aug 8, 2018

Fantastic, thanks @johntdowney ! Has a PR been submitted for this fix?

abtx added a commit to abtx/angular-drag-and-drop-lists that referenced this issue Aug 8, 2018
@sagg629
Copy link

sagg629 commented May 17, 2019

Thank you so much!
Now the drag procedure is much, much more smoothly, especially on a Mac PC with Chrome.

I would like this library to be updated more constantly, the last update is from 2017!

@Miki-AG
Copy link

Miki-AG commented Mar 18, 2021

You can improve performance a bit more by throttling:

      var throttle = true; 
      element.on('dragover', function(event) {
        if (throttle) {
          throttle = false;
          event = event.originalEvent || event;
          ...
          ent.addClass("dndDragover");
          event.stopPropagation();
          setTimeout(() => {
            throttle = true;
          }, 50);
        }
        return false;
      });

@royalone
Copy link

I also added some throttling, using lodash.
replacing line 310

element.on('dragover', function(event) {

with

element.on('dragover', _.throttle(dragOver, 50, { leading: true, trailing: false }))
function dragOver(event){

you'll also need to get rid of the extra bracket etc. at the end of the function

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants