PDA

View Full Version : [FIXED-395][3.1] Removed TreeNode can't be added again



Supergibbs
19 Dec 2009, 2:32 AM
I have a tree with some draggable nodes. When I move a node, I handle the movenode event to store the move in our database via an ajax call. If for some reason the ajax call fails I want to undo the move. Currently I do it like this:



tree.on('movenode', function (tree, node, oldParent, newParent, index) {
$.ajax({
method: 'GET',
url: '/Tree/Move',
data: {
node: node.id,
oldParentId: oldParent.id,
newParentId: newParent.id
},
error: function (XMLHttpRequest, textStatus, errorThrown) {
newParent.removeChild(node);
oldParent.appendChild(node);
oldParent.expand();
}
});
});
I first call "newParent.removeChild(node);" so the movenode event doesn't fire again but the side effect is that the indentation is messed up. I've fixed that by adding:


node.getUI().renderIndent(); //HACK - private method useThis works but is a hack. So is there a better way to do this or is my original code OK and the indentation problem is a bug? If my original code is OK, I think the problem is in Ext.tree.TreeNode.appendChild(node), the following override fixes it (changes in red):



Ext.override(Ext.tree.TreeNode, {
appendChild: function (node) {
var multi = false;
if (Ext.isArray(node)) {
multi = node;
} else if (arguments.length > 1) {
multi = arguments;
}

if (multi) {
for (var i = 0, len = multi.length; i < len; i++) {
this.appendChild(multi[i]);
}
} else {
if (this.fireEvent("beforeappend", this.ownerTree, this, node) === false) {
return false;
}
var index = this.childNodes.length;
var oldParent = node.parentNode;

if (oldParent) {
if (node.fireEvent("beforemove", node.getOwnerTree(), node, oldParent, this, index) === false) {
return false;
}
oldParent.removeChild(node);
}
index = this.childNodes.length;
if (index === 0) {
this.setFirstChild(node);
}
this.childNodes.push(node);
node.parentNode = this;
var ps = this.childNodes[index - 1];
if (ps) {
node.previousSibling = ps;
ps.nextSibling = node;
} else {
node.previousSibling = null;
}
node.nextSibling = null;
this.setLastChild(node);
node.setOwnerTree(this.getOwnerTree());
this.fireEvent("append", this.ownerTree, this, node, index);
if (oldParent) {
node.fireEvent("move", this.ownerTree, node, oldParent, this, index);
}
else {
node.ui.renderIndent();
}
return node;
}
}
});
Thanks!

Condor
19 Dec 2009, 2:49 AM
Yes, this seems to be a bug. Would you like me to move this thread to the Bugs section?

ps. Why not use a flag to ignore the move event?

tree.on('movenode', function(tree, node, oldParent, newParent, index) {
if(node.ignoreMove){
return;
}
Ext.Ajax.request({
method: 'GET',
url: '/Tree/Move',
params: {
node: node.id,
oldParentId: oldParent.id,
newParentId: newParent.id
},
failure: function (response) {
node.ignoreMove = true;
oldParent.appendChild(node);
delete node.ignoreMove;
oldParent.expand();
}
});
});

Supergibbs
19 Dec 2009, 3:08 AM
Wow that worked great!

Just out of curiosity, should my code before have worked and the override I posted should still be considered or is what I was doing not really proper usage?

Thanks for the quick reply!

Condor
19 Dec 2009, 4:45 AM
You're doing nothing wrong. This is a bug (I'll move this thread).

Example:

var tree = new Ext.tree.TreePanel({
root: {
text: 'Root',
expanded: true,
children: [{
text: 'Node 1',
expanded: true,
children: [{
id: 'node',
text: 'Node 2',
leaf: true
}]
}]
},
loader: {
preloadChildren: true
}
});
new Ext.Viewport({
layout: 'fit',
items: tree
});
var node = tree.getNodeById('node');
node.remove();
tree.getRootNode().appendChild(node);

evant
21 Dec 2009, 3:37 AM
A tentative fix has been added to SVN in rev 5800, needs a bit of extra testing.

Supergibbs
22 Dec 2009, 10:18 AM
That is great to hear but there may be two bugs here. As I said in my OP, the node gets added but the indentation is incorrect. In my project though, if I add two nodes to a parent and then try and move one, it does get deleted. So these bugs may be related. Can you post (or send me privately if needed) the tentative fix for me to test?

Thanks
Jesse

Condor
22 Dec 2009, 10:15 PM
SVN rev. 5800 contains:

Ext.override(Ext.tree.TreeNode, {
appendChild : function(n){
var node, exists;
if(!n.render && !Ext.isArray(n)){
n = this.getLoader().createNode(n);
}else{
exists = !n.parentNode;
}
node = Ext.tree.TreeNode.superclass.appendChild.call(this, n);
if(node){
this.afterAdd(node, exists);
}
this.ui.updateExpandIcon();
return node;
},
insertBefore : function(node, refNode){
var newNode, exists;
if(!node.render){
node = this.getLoader().createNode(node);
}else{
exists = !node.parentNode;
}
newNode = Ext.tree.TreeNode.superclass.insertBefore.call(this, node, refNode);
if(newNode && refNode){
this.afterAdd(newNode, exists);
}
this.ui.updateExpandIcon();
return newNode;
},
afterAdd : function(node, exists){
if(this.childrenRendered){
node.render(exists);
}else if(exists){
node.renderIndent(true, true);
}
}
});

@evant: Can't the code in Ext.tree.TreeNodeUI.onMove be cleaned up now?

Supergibbs
23 Dec 2009, 9:03 AM
Yep that fixed both problems. Thanks!