Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext-dnd5: Unwanted expanding of folder node when a node is dragged above it #875

Closed
aecreations opened this issue Jun 25, 2018 · 8 comments

Comments

@aecreations
Copy link

In a Fancytree widget with folders and child nodes, if a node is dragged and dropped immediately above a folder to reposition it there, the folder automatically expands. There does not appear to be a way to limit the auto-expand behaviour to only do it when dragging a node to a folder.

This behaviour is reproducible in the complex demo of Fancytree: http://wwwendt.de/tech/fancytree/demo/#sample-multi-ext.html.

Steps to Reproduce:

  1. Go to the complex demo of Fancytree: http://wwwendt.de/tech/fancytree/demo/#sample-multi-ext.html
  2. Expand the folder titled "More..."
  3. Click on a child node of the "More..." folder, e.g. the node titled "Sub item"
  4. Drag the node and drop it immediately above the collapsed "Music" folder

Actual Result:
The "Music" folder is expanded, even though the node wasn't dropped into it.

Expected Result:
The "Music" folder should remain collapsed. It should only expand (after a short delay) if the "Sub item" node was dragged on to the folder, signifying the intention to move the node into the folder.

Other Info:
Firefox 60
Fancytree version 2.23.0 with ext-dnd5
jQuery 3.2.1

@aecreations
Copy link
Author

I've updated Fancytree to v2.29.1 but this issue is still occurring. This issue continues to be reproducible in the Fancytree complex demo.

@mar10
Copy link
Owner

mar10 commented Jun 30, 2018

Thank you for the feedback!
The sample-multi-ext demo still uses ext-dnd (not ext-dnd5). For me it seems to be fixed here:
http://wwwendt.de/tech/fancytree/demo/sample-ext-dnd5.html
(drop Users\jdoe just above etc)
Are you sure you cleared your browser cache after update?

@aecreations
Copy link
Author

I've cleared the browser cache, but the problem still occurs. Reproducible on Firefox and Chrome.
Screen recording: https://www.dropbox.com/s/3bplmo4ndvk5aok/Screen%20Recording%202018-06-30%20at%202.27.25%20PM.mov?dl=0

@mar10
Copy link
Owner

mar10 commented Jul 1, 2018

This is very strange indeed. I see this behavior (Chrome and Safari, macOS)
https://www.dropbox.com/s/te2q8gn3rxko8pc/dnd5-autoexpand.mov?dl=0

I don't know what's going on there. In case you want to look into it or add some logging, this is the relevant spot in jquery.fancytree.dnd5.js:

case "dragover":
  // The dragover event is fired when an element or text
  // selection is being dragged over a valid drop target
  // (every few hundred milliseconds).
  // console.log(event.type, "dropEffect: " + dataTransfer.dropEffect)
  LAST_HIT_MODE = handleDragOver(event, data);
  allowDrop = !!LAST_HIT_MODE;

  // console.log(event.type, LAST_HIT_MODE, DRAG_OVER_STAMP)

  if( LAST_HIT_MODE === "over" &&
    !node.expanded && node.hasChildren() !== false ) {
    if( !DRAG_OVER_STAMP ) {
      DRAG_OVER_STAMP = Date.now();
    } else if( dndOpts.autoExpandMS &&
        (Date.now() - DRAG_OVER_STAMP) > dndOpts.autoExpandMS &&
        (!dndOpts.dragExpand || dndOpts.dragExpand(node, data) !== false)
        ) {
        node.setExpanded();
      }
  } else {
    DRAG_OVER_STAMP = null;
  }

@mar10 mar10 reopened this Jul 1, 2018
@mar10 mar10 added the waiting label Jul 1, 2018
@pmichel-J2S
Copy link

pmichel-J2S commented Aug 7, 2018

Not directly related to this bug, but very rarely in this exact part of code (line 626 in jquery.fancytree.dnd5.js to be precise), the "node" variable happens to be null.

Other cases in the switch do have a safeguard such as :

if( !node ) { tree.debug("Ignore non-node " + event.type + ": " + event.target.tagName + "." + event.target.className); break; }

The "dragover" case could also use it ?

@mar10
Copy link
Owner

mar10 commented Aug 11, 2018

Thanks, I'll add a guard.

mar10 added a commit that referenced this issue Aug 11, 2018
@aecreations
Copy link
Author

@mar10 - I finally took the time to look into this issue further. It is determined not to be an issue with the library itself. My code in the dragDrop handler should have been checking if a node was moved from one folder node to another, by checking if the old parent ID and new parent ID of the moved node are different, and only call node.setExpanded() if that condition is satisfied. Please go ahead and close this issue.

@mar10
Copy link
Owner

mar10 commented Nov 26, 2018

Thanks!

@mar10 mar10 closed this as completed Nov 26, 2018
@mar10 mar10 removed the waiting label Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants