mbraak / jqTree

Tree widget for jQuery
https://mbraak.github.io/jqTree/
Apache License 2.0
1.02k stars 177 forks source link

'tree.open' event is not triggered when dragging nodes #15

Closed rosenfeld closed 12 years ago

rosenfeld commented 12 years ago

If you try to move a node to inside another one which is closed, it will open but 'tree.open' won't be called.

This is a blocker issue if you're populating the nodes as they're opened using AJAX for retrieving the children nodes.

The 'tree.open' event should probably be triggered by FolderElement.open().

Could you please take a look at this issue?

rosenfeld commented 12 years ago

I've tried a minor change for my tests but I found another issue:

_startOpenFolderTimer: (folder) ->
        openFolder = =>
            @_getNodeElementForNode(folder).open(
                =>
                    @_refreshHitAreas()
                    @_updateDropHint()
                    event = $.Event('tree.open')
                    event.node = folder
                    @element.trigger(event)
            )

The problem is that the dragged item will be displaced as the tree dynamically grows up as the folders are opened.

mbraak commented 12 years ago

The 'tree.open' should indeed be fired in this situation. I added the feature to the 'dev' branch.

I can see that dynamically loading nodes while dragging an item is a problem. I will try to setup a demo to see what goes wrong exactly.

rosenfeld commented 12 years ago

This is easy to replicate. Consider '>' as a closed node and 'v' an open open. Just try to move 'b.1' inside 'a' and you'll see:

> a
v  b
  b.1
  b.2

Then populate "a" with some rows when it is opened by the dragging logic.

mbraak commented 12 years ago

I've added the functions 'isDragging' and 'refreshHitAreas' to the 'dev' branch.

To support dynamically loading nodes while dragging, you could use these functions like this:

$tree.bind('tree.open', function(e) {
  // Load new data
  var data = [
    {label: 'c1'},
    {label: 'c2'},
    {label: 'c3'}
  ];
  $tree.tree('loadData', data, e.node);

  if ($tree.tree('isDragging')) {
    $tree.tree('refreshHitAreas');
  }
});

I think this works, but it would be better if the tree did this automatically. I will look into this.

EDIT: jqTree does this now automatically

rosenfeld commented 12 years ago

Thank you a lot! I'll give it a try later today.

rosenfeld commented 12 years ago

I could finally give it a try today and it worked pretty well. Thanks!

There is a remaining issue, though. $('#tree').tree('toggle', node) won't trigger 'tree.open'. I'm willing to add this code to the 'tree.click' event handler, but I don't think I should also handle the logic for triggering 'tree.open' from this event handler after calling tree('toggle', node).

Don't you think that maybe it would be better to trigger the 'tree.open' event from the FolderElement widget?

mbraak commented 12 years ago

Yes, the FolderElement class should fire the 'tree.open' and the 'tree.close'.

mbraak commented 12 years ago

This is now fixed in the 'dev' branch.

mbraak commented 12 years ago

The function loadData now checks if a node is being dragged. If so, jqtree automatically updates the hit areas.

This is updated in the dev branch.

rosenfeld commented 12 years ago

Thank you a lot. All of your changes have worked for me and I was able to remove some code of my own :) I'm therefore closing this.