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

serious performance issue when using maxDepth with 10+ nodes #194

Closed
lifejuggler opened this issue Nov 12, 2017 · 3 comments
Closed

serious performance issue when using maxDepth with 10+ nodes #194

lifejuggler opened this issue Nov 12, 2017 · 3 comments

Comments

@lifejuggler
Copy link
Collaborator

When I have tree to tree dragging with different maxDepth per tree ( maxDepth= 1 for one and maxDepth=2 for the other), it creates a significant lag in dragging and dropping the nodes to a point where it makes it unusable. I have pin pointed the problem to a dnd-manager where when maxDepth is defined it does a check at every drag that causes the overload... can there be a fix for this flaw?

@fritz-c
Copy link
Member

fritz-c commented Nov 18, 2017

Could you give me some more information on your findings? Specifically:

  • Where are the problem lines in the code?
  • Can you provide some sample code and a description of your environment to reproduce the poor performance? I tried reproducing it in my own environment, but it seemed ok.

@lifejuggler
Copy link
Collaborator Author

lifejuggler commented Nov 20, 2017

GIven this state

this.state = { treeData1: [ { title: 'node1', children: [{ title: 'Child node' }, { title: 'Child node1' }, { title: 'Child node2' }] }, { title: 'node2', children: [{ title: 'Child node' }] }, { title: 'node3', children: [{ title: 'Child node' }] }, { title: 'node4', children: [{ title: 'Child node' }] }, { title: 'node5', children: [{ title: 'Child node' }] }, { title: 'node6', children: [{ title: 'Child node' }] }, { title: 'node7', children: [{ title: 'Child node' }] }, { title: 'node18', children: [{ title: 'Child node' }] }, { title: 'node21', source: 'sourceTree' } ], treeData2: [ { title: 'node22' }, { title: 'node23' }, { title: 'node24' }, { title: 'node25' }, { title: 'node26' }, { title: 'node27' }, { title: 'node28' }, { title: 'node30' }, { title: 'node31' }, { title: 'node32' }, { title: 'node33' }, { title: 'node34' }, { title: 'node35' }, { title: 'node36' }, { title: 'node37' }, { title: 'node38' }, { title: 'node39' }, { title: 'node40' }, { title: 'node41' } ], shouldCopyOnOutsideDrop: false, }; }

In the tree-to-tree.js,
when you define the tree as

`<SortableTree
        treeData={this.state.treeData1}
        onChange={treeData1 => this.setState({ treeData1 })}
        dndType={externalNodeType}
        onNodeClick={this.handleClick}
        shouldCopyOnOutsideDrop={shouldCopyOnOutsideDrop}
        validDrop={false}
        maxDepth={1}
      />
    </div>
    <div
      style={{
        height: 350,
        width: 350,
        float: 'left',
        border: 'solid black 1px',
      }}
    >
      <SortableTree
        treeData={this.state.treeData2}
        onChange={treeData2 => this.setState({ treeData2 })}
        dndType={externalNodeType}
        onNodeClick={this.handleClick}
        shouldCopyOnOutsideDrop={shouldCopyOnOutsideDrop}
        maxDepth={0}
        acceptedSources={["sourceTree"]}
      />
    </div>`

it causes a massive lag in drag and drop animations.
This problem is caused by getTargetDepth method I think where this additional condition of maxDepth is calling getDepth at every pixel drag call. I made a workaround where I make my own candDrop function that allows the animation of dropping into node as a child but doesn't allow the actual drop

@fritz-c
Copy link
Member

fritz-c commented Nov 28, 2017

Thank you for the info. It helped me track down the issue here. I hadn't considered the case of small values of maxDepth, and it was giving back -1 as a targetDepth. I didn't look much into why that was causing performance issues instead of an outright error, but it's fixed now, anyway. Available in v1.5.1

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

2 participants