PDA

View Full Version : [FIXED-98][3.0.0] TreePanel remove event bubble



excelsis
20 Jul 2009, 12:32 PM
TreePanel includes a proxynodeevent handler that fires the remove event on the treepanel in response to the removal of a node from the tree.
The events args are (treepanel, parent node, node)

In the new 3.0 event bubbling scheme this event bubbles up through all the container hierarchy.

There are 2 things wrong with this IMHO - one a bug and another a performance issue.

The bug is simply that the event signature no longer matches the remove event as defined for Container where the args are container and component. Any remove handler at a higher level defined in accordance with spec will break if it tries to run a component method on the 2nd event argument (e.g. getXType) since the 2nd arg is now a Node object. The event rised on the treepanel should not be "remove" but "removeNode" .
The node remove event fires for every node; now when I refresh (i.e. reload the root) my 100 node tree that sits in an accordion that sits in a window (and so on) I get a bazillion remove events. If the event was removeNode it can be isloated and set to not bubble (although I just stared migrating and am not sure how to do all that yet...). Yeah, yeah I can check for root and suspend events etc but the point is it should not be he "remove" event but the "removeNode" event I suppress.

evant
20 Jul 2009, 10:14 PM
I agree with what you're saying here, the fact that the event gets overwritten can be confusing, especially with event bubbling. However the issue we have is that this could potentially break a lot of applications. I will speak to the other guys and get their input on how best to resolve this.

Thanks for the report.

Animal
30 Jul 2009, 11:45 PM
Let's be a bit clearer. A TreeNode firing an event on its ownerTree, and 3.0's Component->Container event bubbling are two completely unrelated issues.

Are you reporting problems with both things?

Tree is not a Container, and TreeNodes are not Components.

(I've sometimes thought it could be a Container with a special layout type, but that's another issue)

Reloading a node, necessarily removes all descendant nodes. Logically, that will fire events.

Perhaps it can be changed to accept a "silent" parameter:



reload : function(callback, scope, silent){
this.collapse(false, false);
while(this.firstChild){
this.removeChild(this.firstChild, silent).destroy();
}
this.childrenRendered = false;
this.loaded = false;
if(this.isHiddenRoot()){
this.expanded = false;
}
this.expand(false, false, callback, scope);
}


This would mean the removeChild template method in Ext.data.Node and Ext.tree.TreeNode would have to accept a "silent" parameter and not fire events if passed true.

Condor
31 Jul 2009, 12:16 AM
No, the problem arises in the container of the TreePanel.

The container remove listener can fire when the treepanel is removed, but also when a treenode is removed (because it is fired on treepanel and the event bubbles).

IMHO the solution is to disable default bubbling of add and remove events for containers that can't contain items (TreePanel and GridPanel).

31 Jul 2009, 2:47 AM
+1 for condor's suggestion

Condor
31 Jul 2009, 3:17 AM
How about:

Ext.override(Ext.Container, {
bubbleEvents : ['add', 'remove'],
initComponent : function(){
Ext.Container.superclass.initComponent.call(this);
this.addEvents(
'afterlayout',
'beforeadd',
'beforeremove',
'add',
'remove'
);
this.enableBubble(this.bubbleEvents);
var items = this.items;
if(items){
delete this.items;
this.add(items);
}
}
});
Ext.override(Ext.tree.TreePanel, {
bubbleEvents : []
});
Ext.override(Ext.grid.GridPanel, {
bubbleEvents : []
});

Animal
31 Jul 2009, 6:04 AM
OK, I see the problem now, thanks. Yes, It is receiving a remove event even though it's not really a Container, and bubbbling it up.

You're absolutely right, Descendants of Container that cannot actually contain should not bubble these two events.

mystix
31 Jul 2009, 8:31 AM
+1 to @condor's suggestion.