Skip to content

Commit 5f20414

Browse files
committed
fixed default ordering when no order-by is supplied
#15 breaks the default ordering of the tree along with twenty five tests. This fixes all the tests and restores the ordering in the case that the user does not specify an orderBy expression - that is, it does not change the tree order at all when orderBy is not passed in, as one would expect. Previously, the orderBy filter was blindly applied to the data, even when the user didn't specify the orderBy attribute on the tree control. Without this PR, the only way to order the tree is by passing the orderBy attribute in the <tree-control> element. Programatic tree ordering was ignored, since the orderBy filter of the directive itself was always applied last. This is especially problematic since the orderBy only accepts an expression and not a function, so there's no way to customize the ordering further than passing in a property of your nodes. For example, AngularJS's default orderBy is case insensitive, so we don't have the option of ordering in a case sensitive fashion, unless we create a completely separate property on the node purely for ordering. More importantly, if you did not provide an orderBy, the default orderBy was being applied, and it did not understand the objects that we usually use as nodes in our trees. Every user who does not provide an orderBy will get unexpected ordering where angularJS tries to order objects naively.
1 parent d4974fe commit 5f20414

File tree

2 files changed

+16
-1
lines changed

2 files changed

+16
-1
lines changed

angular-tree-control.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,10 @@
149149
};
150150

151151
//tree template
152+
var orderBy = $scope.orderBy ? ' | orderBy:orderBy:reverseOrder' : '';
152153
var template =
153154
'<ul '+classIfDefined($scope.options.injectClasses.ul, true)+'>' +
154-
'<li ng-repeat="node in node.' + $scope.options.nodeChildren + ' | filter:filterExpression:filterComparator | orderBy:orderBy:reverseOrder" ng-class="headClass(node)" '+classIfDefined($scope.options.injectClasses.li, true)+'>' +
155+
'<li ng-repeat="node in node.' + $scope.options.nodeChildren + ' | filter:filterExpression:filterComparator ' + orderBy + '" ng-class="headClass(node)" '+classIfDefined($scope.options.injectClasses.li, true)+'>' +
155156
'<i class="tree-branch-head" ng-class="iBranchClass()" ng-click="selectNodeHead(node)"></i>' +
156157
'<i class="tree-leaf-head '+classIfDefined($scope.options.injectClasses.iLeaf, false)+'"></i>' +
157158
'<div class="tree-label '+classIfDefined($scope.options.injectClasses.label, false)+'" ng-class="selectedClass()" ng-click="selectNodeLabel(node)" tree-transclude></div>' +

test/angular-tree-control-test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,20 @@ describe('treeControl', function() {
263263

264264
describe('options usage', function () {
265265

266+
it('should not reorder nodes if no order-by is provided', function() {
267+
$rootScope.treedata = [
268+
{ label: "a", children: [] },
269+
{ label: "c", children: [] },
270+
{ label: "b", children: [] }
271+
];
272+
273+
element = $compile('<treecontrol tree-model="treedata" reverse-order="{{reverse}}">{{node.label}}</treecontrol>')($rootScope);
274+
$rootScope.$digest();
275+
expect(element.find('li:eq(0)').text()).toBe('a');
276+
expect(element.find('li:eq(1)').text()).toBe('c');
277+
expect(element.find('li:eq(2)').text()).toBe('b');
278+
});
279+
266280
it('should order sibling nodes in normal order', function() {
267281
$rootScope.treedata = [
268282
{ label: "a", children: [] },

0 commit comments

Comments
 (0)