PDA

View Full Version : [FIXED-400][3.1] Wrong value passed to TreeSorter.sortType



azuroff
17 Dec 2009, 9:28 AM
In 3.0.3, the parameter passed to the TreeSorter.sortType function was the node itself, which we used to sort the nodes based on which type of node it was (using various node.attributes values).


this.sortFn = function(n1, n2){
if(fs){
if(n1.attributes[leafAttr] && !n2.attributes[leafAttr]){
return 1;
}
if(!n1.attributes[leafAttr] && n2.attributes[leafAttr]){
return -1;
}
}
var v1 = sortType ? sortType(n1) : (cs ? n1.attributes[p] : n1.attributes[p].toUpperCase());
var v2 = sortType ? sortType(n2) : (cs ? n2.attributes[p] : n2.attributes[p].toUpperCase());
if(v1 < v2){
return dsc ? +1 : -1;
}else if(v1 > v2){
return dsc ? -1 : +1;
}else{
return 0;
}
};
In 3.1, just the node's text value (or "property" value if one is defined) is passed to the function, which means that we can't use any of the node's custom attributes to do the sorting. This is a deal-breaker for us for 3.1. :( (yes, I know I can and will patch the source myself, but this is yet another reminder of why the policy regarding bug fixes for non-support subscribers is simply wrong)


this.sortFn = function(n1, n2){
if(fs){
if(n1.attributes[leafAttr] && !n2.attributes[leafAttr]){
return 1;
}
if(!n1.attributes[leafAttr] && n2.attributes[leafAttr]){
return -1;
}
}
var v1 = sortType ? sortType(n1.attributes[p]) : (cs ? n1.attributes[p] : n1.attributes[p].toUpperCase());
var v2 = sortType ? sortType(n2.attributes[p]) : (cs ? n2.attributes[p] : n2.attributes[p].toUpperCase());
if(v1 < v2){
return dsc ? +1 : -1;
}else if(v1 > v2){
return dsc ? -1 : +1;
}else{
return 0;
}
};Fwiw, the documentation for 3.1 still states that the TreeNode object itself is passed to the function, even though the code clearly demonstrates otherwise.

azuroff
17 Dec 2009, 9:42 AM
When I change it back to what it was in 3.0.3, everything works perfectly -


var v1 = sortType ? sortType(n1) : (cs ? n1.attributes[p] : n1.attributes[p].toUpperCase());
var v2 = sortType ? sortType(n2) : (cs ? n2.attributes[p] : n2.attributes[p].toUpperCase());

Condor
18 Dec 2009, 4:41 AM
Interestingly the API docs originally (pre rev. 1893) mentioned that the node value was passed to the sortType function.

Now (post rev. 5705) it's the other way around!

htoyryla
18 Dec 2009, 12:22 PM
Interestingly the API docs originally (pre rev. 1893) mentioned that the node value was passed to the sortType function.

Now (post rev. 5705) it's the other way around!

So what is the logic here? My app suffers from this change too. And now that I have managed to sort out the layout/rendering problems, this remains the major problem we in our company have with 3.1.0. How are we supposed to sort the nodes now? The TreeSorter documentation still describes that TreeSorter gets the node.

albertyips
19 Dec 2009, 9:03 AM
This change also causes problems with our application, and yes patching is relatively easy.

But, the fewer overrides i can make to the ext framework the better. Overrides are evil and cause problems during migration.

Passing the entire node to custom sort function is much more powerful. If someone is writing a custom sort function, it isn't a big effort to get access to a custom attribute.

Another vote to change it back!

Thanks,
Albert

davidbuzatto
19 Dec 2009, 6:28 PM
Hi

I also think that this needs to be changed back. It crashed my sorting capabilities and I thought that was a problem with an AsynchTreeNode... I really don't want to change the ext code...

Why was it changed???

danutungureanu.dgs
21 Dec 2009, 2:24 AM
Same problem here, why was this changed in the first place?

hendricd
22 Dec 2009, 11:36 AM
Reverted to previous version to maintain release compatibility.

azuroff
22 Dec 2009, 1:59 PM
Reverted to previous version to maintain release compatibility.

Thanks!

Now what's the tentative release date for ExtJS 3.2?

hendricd
22 Dec 2009, 3:02 PM
Thanks!

Now what's the tentative release date for ExtJS 3.2?

You'll likely not be waiting that long. 3.1.1 will be released publicly (soon). We're looking into a couple more issues...

jmueller
26 Jan 2010, 9:24 AM
You'll likely not be waiting that long. 3.1.1 will be released publicly (soon). We're looking into a couple more issues...

Yes, but why in the world would you make this particular change in the first place? What was the reasoning? It doesn't inspire confidence...