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

Drag and drop doesn't work correctly unless all of the nodes are the same height. #29

Open
SeanRoy opened this issue Apr 19, 2016 · 2 comments

Comments

@SeanRoy
Copy link

SeanRoy commented Apr 19, 2016

The DnD functionality assumes that all nodes are the same height in the diffY calculation. If a node is of a different size and you begin dragging it down, the initial diffY is < 0, indicating to the code that we're actually dragging up. Not a huge deal, but once you've dragged sufficiently to get diffY into positive territory, you have to weight until you've gone twice the height of node you're dragging before it registers as below the next lower node. This is due to the } else if (diffY > dragging.h) { clause at line 167. Should probably be (diffY > index.height), but again, the diffY calculation is off. I think there needs to be separate calculations based on whether the drag is up or down, but I'm not sure.

I've been trying to solve this bug as it impacts my group, but so far haven't had any luck. I'll submit a PR if I do find a solution.

@hangy233
Copy link

Hi, I have the same issue.
This is because in the drag handler, it calculate the offsetTop (distance between the vary top of offsetParent and the very top of the node) of the dragging node with the height of node.

  var diffX = dragging.x - paddingLeft / 2 - (index.left - 2) * paddingLeft
  var diffY = dragging.y - dragging.h / 2 - (index.top - 2) * dragging.h
  // dragging.h is the height of the node
  // (index.top - 1) is the amount of nodes that on top of the dragging node

A workaround of this is get the offsetTop value of the real dom element (instead of calculating it).
Here's my changes

//node.js
...
<div className='inner' ref='inner' id={'node-' + index.id} onMouseDown={this.handleMouseDown}>
...
//react-ui-tree.js
...
var upperNode = document.querySelector('.m-node>#node-' + tree.getNodeByTop(index.top - 1).id)

var diffX = dragging.x - paddingLeft / 2 - (index.left - 2) * paddingLeft
var diffY = dragging.y + dragging.h / 2 - upperNode.offsetTop - upperNode.offsetHeight
...

@benbowes
Copy link

benbowes commented Sep 27, 2018

Slight amendment to the above - ref='inner' is now ref={this.innerRef}

// node.js
<div
    className="inner"
    ref={this.innerRef}
    id={"node-" + index.id}
    onMouseDown={this.handleMouseDown}
>

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

No branches or pull requests

3 participants