From 2d7bb9d5d4c77a6796613912bf908d44718280c2 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Tue, 19 Apr 2016 20:31:24 +0300 Subject: [PATCH 1/4] feat(orderBy): add support for custom comparators With this change the 3rd argument is interpreted as a comparator function, used to compare the values returned by the predicates. Leaving it empty falls back to the default comparator and a value of `false` is a special case that also falls back to the default comparator but reverses the order. Thus the new implementation is backwards compatible. This commit also expands the documentation to cover the algorithm used to sort elements and adds a few more unit and e2e tests (unrelated to the change). Helps with #12572 (maybe this is as close as we want to get). Fixes #13238 Fixes #14455 Closes #5123 Closes #8112 Closes #10368 --- src/ng/filter/orderBy.js | 664 ++++++++++++++++++++++++++-------- test/ng/filter/orderBySpec.js | 315 ++++++++++++++-- 2 files changed, 806 insertions(+), 173 deletions(-) diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js index 8974b1425d74..8f4ff3427c52 100644 --- a/src/ng/filter/orderBy.js +++ b/src/ng/filter/orderBy.js @@ -6,44 +6,118 @@ * @kind function * * @description - * Orders a specified `array` by the `expression` predicate. It is ordered alphabetically - * for strings and numerically for numbers. Note: if you notice numbers are not being sorted - * as expected, make sure they are actually being saved as numbers and not strings. - * Array-like values (e.g. NodeLists, jQuery objects, TypedArrays, Strings, etc) are also supported. + * Returns an array containing the items from the specified `collection`, ordered by a `comparator` + * function based on the values computed using the `expression` predicate. * - * @param {Array} array The array (or array-like object) to sort. - * @param {function(*)|string|Array.<(function(*)|string)>=} expression A predicate to be - * used by the comparator to determine the order of elements. + * The `collection` can be an Array or array-like object (e.g. NodeList, jQuery object, TypedArray, + * String, etc). + * + * The `expression` can be a signle predicate or a list of predicates, each serving as a tie-breaker + * for the preceeding one. The `expression` is evaluated against each item and the output is used + * for comparing with other items. + * + * The comparison is done using the `comparator` function. If none is specified, a default, built-in + * comparator is used (see below for details). Passing `true` as the comparator has a special + * meaning: It will use the built-in comparator, but sort in reverse order. + * + * ### Under the hood + * + * Ordering the specified `collection` happens in two phases: + * + * 1. All items are passed through the predicate (or predicates), and the returned values are saved + * along with their type (`string`, `number` etc). For example, an item `{name: 'foo'}`, passed + * through a predicate that extracts the value of the `name` property, would be transformed to: + * ``` + * { + * value: 'foo', + * type: 'string', + * index: ... + * } + * ``` + * 2. The comparator function is used to sort the items, besed on the derived values, types and + * indices. + * + * If you use a custom comparator, it will be called with pairs of objects of the form + * `{value: ..., type: '...', index: ...}` and is expected to return `0` if the objects are equal + * (as far as the comparator is concerned), `-1` if the 1st one should be ranked higher than the + * second, or `1` otherwise. + * + * In order to ensure that the sorting will be deterministic across platforms, if none of the + * specified predicates can distinguish between two items, `orderBy` will automatically introduce a + * dummy predicate that returns the item's index as `value`. + * (If you are using a custom comparator, make sure it can handle this case as well.) + * + * Finally, in an attempt to simplify things, if a predicate returns an object as the extracted + * value for an item, `orderBy` will try to convert that object to a primitive value, before passing + * it to the comparator. The following rules govern the conversion: + * + * 1. If the object has a `valueOf()` method that returns a primitive, its return value will be + * used instead.
+ * (If the object does have a `valueOf()` method, but it returns another object, then the + * returned object will be used in subsequent steps.) + * 2. If the object has a custom `toString()` method (i.e. not the one inherited from `Object`) that + * returns a primitive, its return value will be used instead.
+ * (If the object does have a `toString()` method, but it returns another object, then the + * returned object will be used in subsequent steps.) + * 3. No conversion; the object itself is used. + * + * ### The default comparator + * + * The default, built-in comparator should be sufficient for most usecases. In short, it compares + * numbers numerically, strings alphabetically (and case-insensitively), for objects falls back to + * using their index in the original colection, and sorts values of different types by type. + * + * More specifically, it follows these steps to determine the relative order of items: + * + * 1. If the compared values are of different types, compare the types themselves alphabetically. + * 2. If both values are of type `string`, compare them alphabetically in a case- and + * locale-insensitive way. + * 3. If both values are objects, compare their indices instead. + * 4. Otherwise, return: + * - `0`, if the values are equal (by strict equality comparison, i.e. using `===`). + * - `-1`, if the 1st value is "less than" the 2nd value (compared using the `<` operator). + * - `1`, otherwise. + * + * **Note:** If you notice numbers not being sorted as expected, make sure they are actually being + * saved as numbers and not strings. + * + * @param {Array|ArrayLike} collection - The collection (array or array-like object) to sort. + * @param {(Function|string|Array.)=} expression - A predicate (or list of + * predicates) to be used by the comparator to determine the order of elements. * * Can be one of: * - * - `function`: Getter function. The result of this function will be sorted using the - * `<`, `===`, `>` operator. - * - `string`: An Angular expression. The result of this expression is used to compare elements - * (for example `name` to sort by a property called `name` or `name.substr(0, 3)` to sort by - * 3 first characters of a property called `name`). The result of a constant expression - * is interpreted as a property name to be used in comparisons (for example `"special name"` - * to sort object by the value of their `special name` property). An expression can be - * optionally prefixed with `+` or `-` to control ascending or descending sort order - * (for example, `+name` or `-name`). If no property is provided, (e.g. `'+'`) then the array - * element itself is used to compare where sorting. - * - `Array`: An array of function or string predicates. The first predicate in the array - * is used for sorting, but when two items are equivalent, the next predicate is used. + * - `Function`: A getter function. Ths function will be called with each item as argument and + * the return value will be used for sorting. + * - `string`: An Angular expression. This expression will be evaluated against each item and the + * result will be used for sorting. For example, use `name` to sort by a property called `name` + * or `name.substring(0, 3)` to sort by the first 3 characters of the `name` property.
+ * (The result of a constant expression is interpreted as a property name to be used for + * comparison. For example, use `"special name"` to sort by a property called `special name`.)
+ * An expression can be optionally prefixed with `+` or `-` to control the sorting direction, + * ascending or descending. For example, `+name` or `-name`. If no property is provided, (e.g. + * `'+'` or `'-'`), the collection element itself is used in comparisons. + * - `Array`: An array of function and/or string predicates. If a predicate cannot determine the + * relative order of two items, the next predicate is used as a tie-breaker. + * + * **Note:** If the predicate is missing or empty then it defaults to `'+'`. * - * If the predicate is missing or empty then it defaults to `'+'`. + * @param {(boolean|Function)=} comparator - The comparator function used to determine the relative + * order of value pairs. If omitted, a built-in comparator will be used. Passing `true` will use + * the built-in comparator, but reverse the order. * - * @param {boolean=} reverse Reverse the order of the array. - * @returns {Array} Sorted copy of the source array. + * @returns {Array} - The sorted array. * * * @example - * The example below demonstrates a simple ngRepeat, where the data is sorted - * by age in descending order (predicate is set to `'-age'`). - * `reverse` is not set, which means it defaults to `false`. - + * The example below demonstrates a simple ngRepeat, where the data is sorted by age in descending + * order (expression is set to `'-age'`). The `comparator` is not set, which means it defaults to + * the built-in comparator. + * +
- +
@@ -58,43 +132,76 @@ - angular.module('orderByExample', []) + angular.module('orderByExample1', []) .controller('ExampleController', ['$scope', function($scope) { - $scope.friends = - [{name:'John', phone:'555-1212', age:10}, - {name:'Mary', phone:'555-9876', age:19}, - {name:'Mike', phone:'555-4321', age:21}, - {name:'Adam', phone:'555-5678', age:35}, - {name:'Julie', phone:'555-8765', age:29}]; + $scope.friends = [ + {name: 'John', phone: '555-1212', age: 10}, + {name: 'Mary', phone: '555-9876', age: 19}, + {name: 'Mike', phone: '555-4321', age: 21}, + {name: 'Adam', phone: '555-5678', age: 35}, + {name: 'Julie', phone: '555-8765', age: 29} + ]; }]); + + .friends { + border-collapse: collapse; + } + + .friends th { + border-bottom: 1px solid; + } + .friends td, .friends th { + border-left: 1px solid; + padding: 5px 10px; + } + .friends td:first-child, .friends th:first-child { + border-left: none; + } + + + // Element locators + var names = element.all(by.repeater('friends').column('friend.name')); + + it('should initially sort friends by age in reverse order', function() { + expect(names.get(0).getText()).toBe('Adam'); + expect(names.get(1).getText()).toBe('Julie'); + expect(names.get(2).getText()).toBe('Mike'); + expect(names.get(3).getText()).toBe('Mary'); + expect(names.get(4).getText()).toBe('John'); + }); + + *
* - * The predicate and reverse parameters can be controlled dynamically through scope properties, - * as shown in the next example. * @example - + * The `expression` and `comparator` parameters can be controlled dynamically through scope + * properties, as shown in the next example. (Remember that `comparator` can be set to `true` for + * reverse ordering.) + * +
- 
Sorting predicate = {{predicate}}; reverse = {{reverse}}
+ 
Sort by = {{propertyName}}; reverse = {{reverse}}
+
+
- -
Name Phone Number
+
- - - + + + - + @@ -103,100 +210,332 @@ - angular.module('orderByExample', []) + angular.module('orderByExample2', []) .controller('ExampleController', ['$scope', function($scope) { - $scope.friends = - [{name:'John', phone:'555-1212', age:10}, - {name:'Mary', phone:'555-9876', age:19}, - {name:'Mike', phone:'555-4321', age:21}, - {name:'Adam', phone:'555-5678', age:35}, - {name:'Julie', phone:'555-8765', age:29}]; - $scope.predicate = 'age'; + var friends = [ + {name: 'John', phone: '555-1212', age: 10}, + {name: 'Mary', phone: '555-9876', age: 19}, + {name: 'Mike', phone: '555-4321', age: 21}, + {name: 'Adam', phone: '555-5678', age: 35}, + {name: 'Julie', phone: '555-8765', age: 29} + ]; + + $scope.propertyName = 'age'; $scope.reverse = true; - $scope.order = function(predicate) { - $scope.reverse = ($scope.predicate === predicate) ? !$scope.reverse : false; - $scope.predicate = predicate; + $scope.friends = friends; + + $scope.sortBy = function(propertyName) { + $scope.reverse = ($scope.propertyName === propertyName) ? !$scope.reverse : false; + $scope.propertyName = propertyName; }; }]); - + + .friends { + border-collapse: collapse; + } + + .friends th { + border-bottom: 1px solid; + } + .friends td, .friends th { + border-left: 1px solid; + padding: 5px 10px; + } + .friends td:first-child, .friends th:first-child { + border-left: none; + } + .sortorder:after { - content: '\25b2'; + content: '\25b2'; // BLACK UP-POINTING TRIANGLE } .sortorder.reverse:after { - content: '\25bc'; + content: '\25bc'; // BLACK DOWN-POINTING TRIANGLE } + + // Element locators + var unsortButton = element(by.partialButtonText('unsorted')); + var nameHeader = element(by.partialButtonText('Name')); + var phoneHeader = element(by.partialButtonText('Phone')); + var ageHeader = element(by.partialButtonText('Age')); + var firstName = element(by.repeater('friends').column('friend.name').row(0)); + var lastName = element(by.repeater('friends').column('friend.name').row(4)); + + it('should sort friends by some property, when clicking on the column header', function() { + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + + phoneHeader.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Mary'); + + nameHeader.click(); + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('Mike'); + + ageHeader.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Adam'); + }); + + it('should sort friends in reverse order, when clicking on the same column', function() { + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + + ageHeader.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Adam'); + + ageHeader.click(); + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + }); + + it('should restore the original order, when clicking "Set to unsorted"', function() { + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + + unsortButton.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Julie'); + }); + + *
* - * It's also possible to call the orderBy filter manually, by injecting `$filter`, retrieving the - * filter routine with `$filter('orderBy')`, and calling the returned filter routine with the - * desired parameters. + * @example + * It's also possible to call the `orderBy` filter manually, by injecting `orderByFilter`, and + * calling it with the desired parameters. (Alternatively, you could inject the `$filter` factory + * and retrieve the `orderBy` filter with `$filter('orderBy')`.) * * Example: * - * @example - - -
- 
Sorting predicate = {{predicate}}; reverse = {{reverse}}
-
- - - - - - - - - + + + + + + + + +
{{friend.name}} {{friend.phone}} {{friend.age}}
- - - - - - - - - - -
- - - - - - - - -
{{friend.name}}{{friend.phone}}{{friend.age}}
-
- - - - angular.module('orderByExample', []) - .controller('ExampleController', ['$scope', '$filter', function($scope, $filter) { - var orderBy = $filter('orderBy'); - $scope.friends = [ - { name: 'John', phone: '555-1212', age: 10 }, - { name: 'Mary', phone: '555-9876', age: 19 }, - { name: 'Mike', phone: '555-4321', age: 21 }, - { name: 'Adam', phone: '555-5678', age: 35 }, - { name: 'Julie', phone: '555-8765', age: 29 } - ]; - $scope.order = function(predicate) { - $scope.predicate = predicate; - $scope.reverse = ($scope.predicate === predicate) ? !$scope.reverse : false; - $scope.friends = orderBy($scope.friends, predicate, $scope.reverse); - }; - $scope.order('age', true); - }]); - - - + + +
+ 
Sort by = {{propertyName}}; reverse = {{reverse}}
+
+ +
+ + + + + + + + + + + +
+ + + + + + + + +
{{friend.name}}{{friend.phone}}{{friend.age}}
+
+ + + angular.module('orderByExample3', []) + .controller('ExampleController', ['$scope', 'orderByFilter', function($scope, orderBy) { + var friends = [ + {name: 'John', phone: '555-1212', age: 10}, + {name: 'Mary', phone: '555-9876', age: 19}, + {name: 'Mike', phone: '555-4321', age: 21}, + {name: 'Adam', phone: '555-5678', age: 35}, + {name: 'Julie', phone: '555-8765', age: 29} + ]; + + $scope.propertyName = 'age'; + $scope.reverse = true; + $scope.friends = orderBy(friends, $scope.propertyName, $scope.reverse); + + $scope.sortBy = function(propertyName) { + $scope.reverse = (propertyName !== null && $scope.propertyName === propertyName) + ? !$scope.reverse : false; + $scope.propertyName = propertyName; + $scope.friends = orderBy(friends, $scope.propertyName, $scope.reverse); + }; + }]); + + + .friends { + border-collapse: collapse; + } + + .friends th { + border-bottom: 1px solid; + } + .friends td, .friends th { + border-left: 1px solid; + padding: 5px 10px; + } + .friends td:first-child, .friends th:first-child { + border-left: none; + } + .sortorder:after { - content: '\25b2'; + content: '\25b2'; // BLACK UP-POINTING TRIANGLE } .sortorder.reverse:after { - content: '\25bc'; + content: '\25bc'; // BLACK DOWN-POINTING TRIANGLE + } + + + // Element locators + var unsortButton = element(by.partialButtonText('unsorted')); + var nameHeader = element(by.partialButtonText('Name')); + var phoneHeader = element(by.partialButtonText('Phone')); + var ageHeader = element(by.partialButtonText('Age')); + var firstName = element(by.repeater('friends').column('friend.name').row(0)); + var lastName = element(by.repeater('friends').column('friend.name').row(4)); + + it('should sort friends by some property, when clicking on the column header', function() { + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + + phoneHeader.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Mary'); + + nameHeader.click(); + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('Mike'); + + ageHeader.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Adam'); + }); + + it('should sort friends in reverse order, when clicking on the same column', function() { + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + + ageHeader.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Adam'); + + ageHeader.click(); + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + }); + + it('should restore the original order, when clicking "Set to unsorted"', function() { + expect(firstName.getText()).toBe('Adam'); + expect(lastName.getText()).toBe('John'); + + unsortButton.click(); + expect(firstName.getText()).toBe('John'); + expect(lastName.getText()).toBe('Julie'); + }); + + + *
+ * + * @example + * If you have very specific requirements about the way items are sorted, you can pass your own + * comparator function. For example, you might need to compare some strings is a locale-sensitive + * way: + * + + +
+
+

Locale-sensitive Comparator

+ + + + + + + + + +
NameFavorite Letter
{{friend.name}}{{friend.favoriteLetter}}
+
+
+

Default Comparator

+ + + + + + + + + +
NameFavorite Letter
{{friend.name}}{{friend.favoriteLetter}}
+
+
+ + + angular.module('orderByExample4', []) + .controller('ExampleController', ['$scope', function($scope) { + $scope.friends = [ + {name: 'John', favoriteLetter: 'Ä'}, + {name: 'Mary', favoriteLetter: 'Ü'}, + {name: 'Mike', favoriteLetter: 'Ö'}, + {name: 'Adam', favoriteLetter: 'H'}, + {name: 'Julie', favoriteLetter: 'Z'} + ]; + + $scope.localeSensitiveComparator = function(v1, v2) { + // If we don't get strings, just compare by index + if (v1.type !== 'string' || v2.type !== 'string') { + return (v1.index < v2.index) ? -1 : 1; + } + + // Compare strings alphabetically, taking locale into account + return v1.value.localeCompare(v2.value); + }; + }]); + + + .friends-container { + display: inline-block; + margin: 0 30px; + } + + .friends { + border-collapse: collapse; + } + + .friends th { + border-bottom: 1px solid; + } + .friends td, .friends th { + border-left: 1px solid; + padding: 5px 10px; + } + .friends td:first-child, .friends th:first-child { + border-left: none; } - - + + + // Element locators + var container = element(by.css('.custom-comparator')); + var names = container.all(by.repeater('friends').column('friend.name')); + + it('should sort friends by favorite letter (in correct alphabetical order)', function() { + expect(names.get(0).getText()).toBe('John'); + expect(names.get(1).getText()).toBe('Adam'); + expect(names.get(2).getText()).toBe('Mike'); + expect(names.get(3).getText()).toBe('Mary'); + expect(names.get(4).getText()).toBe('Julie'); + }); + + + * */ orderByFilter.$inject = ['$parse']; function orderByFilter($parse) { - return function(array, sortPredicate, reverseOrder) { + return function(array, sortPredicate, compareFn) { if (array == null) return array; if (!isArrayLike(array)) { @@ -206,11 +545,13 @@ function orderByFilter($parse) { if (!isArray(sortPredicate)) { sortPredicate = [sortPredicate]; } if (sortPredicate.length === 0) { sortPredicate = ['+']; } - var predicates = processPredicates(sortPredicate, reverseOrder); - // Add a predicate at the end that evaluates to the element index. This makes the - // sort stable as it works as a tie-breaker when all the input predicates cannot - // distinguish between two elements. - predicates.push({ get: function() { return {}; }, descending: reverseOrder ? -1 : 1}); + var predicates = processPredicates(sortPredicate); + + // Define the `compare()` function. Use a default comparator if none is specified. + // A value of `true` means: Use the default comparator, but reverse the order. + var compare = isFunction(compareFn) ? compareFn : + compareFn ? reverseDefaultCompare : + defaultCompare; // The next three lines are a version of a Swartzian Transform idiom from Perl // (sometimes called the Decorate-Sort-Undecorate idiom) @@ -222,8 +563,12 @@ function orderByFilter($parse) { return array; function getComparisonObject(value, index) { + // NOTE: We are adding an extra `tieBreaker` value based on the element's index. + // This will be used to keep the sort stable when all the input predicates cannot + // distinguish between two elements. return { value: value, + tieBreaker: {value: index, type: 'number', index: index}, predicateValues: predicates.map(function(predicate) { return getPredicateValue(predicate.get(value), index); }) @@ -231,17 +576,16 @@ function orderByFilter($parse) { } function doComparison(v1, v2) { - var result = 0; for (var index=0, length = predicates.length; index < length; ++index) { - result = compare(v1.predicateValues[index], v2.predicateValues[index]) * predicates[index].descending; - if (result) break; + var result = compare(v1.predicateValues[index], v2.predicateValues[index]) * predicates[index].descending; + if (result) return result; } - return result; + + return compare(v1.tieBreaker, v2.tieBreaker); } }; - function processPredicates(sortPredicate, reverseOrder) { - reverseOrder = reverseOrder ? -1 : 1; + function processPredicates(sortPredicate) { return sortPredicate.map(function(predicate) { var descending = 1, get = identity; @@ -260,7 +604,7 @@ function orderByFilter($parse) { } } } - return { get: get, descending: descending * reverseOrder }; + return { get: get, descending: descending }; }); } @@ -275,9 +619,9 @@ function orderByFilter($parse) { } } - function objectValue(value, index) { + function objectValue(value) { // If `valueOf` is a valid function use that - if (typeof value.valueOf === 'function') { + if (isFunction(value.valueOf)) { value = value.valueOf(); if (isPrimitive(value)) return value; } @@ -286,8 +630,8 @@ function orderByFilter($parse) { value = value.toString(); if (isPrimitive(value)) return value; } - // We have a basic object so we use the position of the object in the collection - return index; + + return value; } function getPredicateValue(value, index) { @@ -295,23 +639,43 @@ function orderByFilter($parse) { if (value === null) { type = 'string'; value = 'null'; - } else if (type === 'string') { - value = value.toLowerCase(); } else if (type === 'object') { - value = objectValue(value, index); + value = objectValue(value); } - return { value: value, type: type }; + return { value: value, type: type, index: index }; } - function compare(v1, v2) { + function defaultCompare(v1, v2) { var result = 0; - if (v1.type === v2.type) { - if (v1.value !== v2.value) { - result = v1.value < v2.value ? -1 : 1; + var type1 = v1.type; + var type2 = v2.type; + + if (type1 === type2) { + var value1 = v1.value; + var value2 = v2.value; + + if (type1 === 'string') { + // Compare strings case-insensitively + value1 = value1.toLowerCase(); + value2 = value2.toLowerCase(); + } else if (type1 === 'object') { + // For basic objects, use the position of the object + // in the collection instead of the value + if (isObject(value1)) value1 = v1.index; + if (isObject(value2)) value2 = v2.index; + } + + if (value1 !== value2) { + result = value1 < value2 ? -1 : 1; } } else { - result = v1.type < v2.type ? -1 : 1; + result = type1 < type2 ? -1 : 1; } + return result; } + + function reverseDefaultCompare(v1, v2) { + return -1 * defaultCompare(v1, v2); + } } diff --git a/test/ng/filter/orderBySpec.js b/test/ng/filter/orderBySpec.js index 131e98d362e8..f76bc5bddc9a 100644 --- a/test/ng/filter/orderBySpec.js +++ b/test/ng/filter/orderBySpec.js @@ -13,15 +13,18 @@ describe('Filter: orderBy', function() { toThrowMinErr('orderBy', 'notarray', 'Expected array but received: {}'); }); + it('should not throw an exception if a null or undefined value is provided', function() { expect(orderBy(null)).toEqual(null); expect(orderBy(undefined)).toEqual(undefined); }); + it('should not throw an exception if an array-like object is provided', function() { expect(orderBy('cba')).toEqual(['a', 'b', 'c']); }); + it('should return sorted array if predicate is not provided', function() { expect(orderBy([2, 1, 3])).toEqual([1, 2, 3]); @@ -37,13 +40,6 @@ describe('Filter: orderBy', function() { }); - it('should reverse collection if `reverseOrder` param is truthy', function() { - expect(orderBy([{a:15}, {a:2}], 'a', true)).toEqualData([{a:15}, {a:2}]); - expect(orderBy([{a:15}, {a:2}], 'a', "T")).toEqualData([{a:15}, {a:2}]); - expect(orderBy([{a:15}, {a:2}], 'a', "reverse")).toEqualData([{a:15}, {a:2}]); - }); - - it('should sort inherited from array', function() { function BaseCollection() {} BaseCollection.prototype = Array.prototype; @@ -93,6 +89,7 @@ describe('Filter: orderBy', function() { { a:new Date('01/01/2014'), b:3 }]); }); + it('should compare timestamps when sorting dates', function() { expect(orderBy([ new Date('01/01/2015'), @@ -140,22 +137,6 @@ describe('Filter: orderBy', function() { expect(orderBy(array)).toEqualData(array); }); - it('should reverse array of objects with no predicate and reverse is `true`', function() { - var array = [ - { id: 2 }, - { id: 1 }, - { id: 4 }, - { id: 3 } - ]; - var reversedArray = [ - { id: 3 }, - { id: 4 }, - { id: 1 }, - { id: 2 } - ]; - expect(orderBy(array, '', true)).toEqualData(reversedArray); - }); - it('should reverse array of objects with predicate of "-"', function() { var array = [ @@ -237,6 +218,294 @@ describe('Filter: orderBy', function() { {foo: 1, bar: 8}, {foo: 1, bar: 5}, {foo: 1, bar: 2} ]); }); + + + describe('(reversing order)', function() { + it('should reverse collection if `comparator` param is truthy (but not a function)', + function() { + var items = [{a: 2}, {a: 15}]; + var expr = 'a'; + var sorted = [{a: 15}, {a: 2}]; + + expect(orderBy(items, expr, true)).toEqual(sorted); + expect(orderBy(items, expr, 1)).toEqual(sorted); + expect(orderBy(items, expr, 'reverse')).toEqual(sorted); + } + ); + + + it('should reverse collection if `comparator` param is `true`, even without an `expression`', + function() { + var originalItems = [{id: 2}, {id: 1}, {id: 4}, {id: 3}]; + var reversedItems = [{id: 3}, {id: 4}, {id: 1}, {id: 2}]; + expect(orderBy(originalItems, null, true)).toEqual(reversedItems); + } + ); + }); + + describe('(built-in comparator)', function() { + it('should compare numbers numarically', function() { + var items = [100, 3, 20]; + var expr = null; + var sorted = [3, 20, 100]; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + + + it('should compare strings alphabetically', function() { + var items = ['100', '3', '20', '_b', 'a']; + var expr = null; + var sorted = ['100', '20', '3', '_b', 'a']; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + + + it('should compare strings case-insensitively', function() { + var items = ['c', 'B', 'a']; + var expr = null; + var sorted = ['a', 'B', 'c']; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + + + it('should compare objects based on `index`', function() { + var items = [{c: 3}, {b: 2}, {a: 1}]; + var expr = null; + var sorted = [{c: 3}, {b: 2}, {a: 1}]; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + + + it('should compare values of different type alphabetically by type', function() { + var items = [undefined, '1', {}, 999, noop, false]; + var expr = null; + var sorted = [false, noop, 999, {}, '1', undefined]; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + }); + + describe('(custom comparator)', function() { + it('should support a custom comparator', function() { + var items = [4, 42, 2]; + var expr = null; + var sorted = [42, 2, 4]; + + var comparator = function(o1, o2) { + var v1 = o1.value; + var v2 = o2.value; + + if (v1 === v2) return 0; + if (v1 === 42) return -1; + if (v2 === 42) return 1; + + return (v1 < v2) ? -1 : 1; + }; + + expect(orderBy(items, expr, comparator)).toEqual(sorted); + }); + + + it('should pass `{value, type, index}` objects to comparators', function() { + var items = [false, noop, 999, {}, '', undefined]; + var expr = null; + var comparator = jasmine.createSpy('comparator').and.returnValue(-1); + + orderBy(items, expr, comparator); + var allArgsFlat = Array.prototype.concat.apply([], comparator.calls.allArgs()); + + expect(allArgsFlat).toContain({index: 0, type: 'boolean', value: false }); + expect(allArgsFlat).toContain({index: 1, type: 'function', value: noop }); + expect(allArgsFlat).toContain({index: 2, type: 'number', value: 999 }); + expect(allArgsFlat).toContain({index: 3, type: 'object', value: {} }); + expect(allArgsFlat).toContain({index: 4, type: 'string', value: '' }); + expect(allArgsFlat).toContain({index: 5, type: 'undefined', value: undefined}); + }); + + + it('should treat a value of `null` as `"null"`', function() { + var items = [null, null]; + var expr = null; + var comparator = jasmine.createSpy('comparator').and.returnValue(-1); + + orderBy(items, expr, comparator); + var arg = comparator.calls.argsFor(0)[0]; + + expect(arg).toEqual(jasmine.objectContaining({ + type: 'string', + value: 'null' + })); + }); + + + it('should not convert strings to lower-case', function() { + var items = ['c', 'B', 'a']; + var expr = null; + var sorted = ['B', 'a', 'c']; + + var comparator = function(o1, o2) { + return (o1.value < o2.value) ? -1 : 1; + }; + + expect(orderBy(items, expr, comparator)).toEqual(sorted); + }); + + + it('should use `index` as `value` if no other predicate can distinguish between two items', + function() { + var items = ['foo', 'bar']; + var expr = null; + var comparator = jasmine.createSpy('comparator').and.returnValue(0); + + orderBy(items, expr, comparator); + + expect(comparator).toHaveBeenCalledTimes(2); + var lastArgs = comparator.calls.mostRecent().args; + + expect(lastArgs).toContain(jasmine.objectContaining({value: 0, type: 'number'})); + expect(lastArgs).toContain(jasmine.objectContaining({value: 1, type: 'number'})); + } + ); + + + describe('(object as `value`)', function() { + it('should use the return value of `valueOf()` (if primitive)', function() { + var o1 = {k: 1, valueOf: function() { return 2; }}; + var o2 = {k: 2, valueOf: function() { return 1; }}; + + var items = [o1, o2]; + var expr = null; + var sorted = [o2, o1]; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + + + it('should use the return value of `toString()` (if primitive)', function() { + var o1 = {k: 1, toString: function() { return 2; }}; + var o2 = {k: 2, toString: function() { return 1; }}; + + var items = [o1, o2]; + var expr = null; + var sorted = [o2, o1]; + + expect(orderBy(items, expr)).toEqual(sorted); + }); + + + it('should ignore the `toString()` inherited from `Object`', function() { + /* globals toString: true */ + + // The global `toString` variable (in 'src/Angular.js') + // has already captured `Object.prototype.toString` + var originalToString = toString; + toString = jasmine.createSpy('toString').and.callFake(originalToString); + + var o1 = Object.create({toString: toString}); + var o2 = Object.create({toString: toString}); + + var items = [o1, o2]; + var expr = null; + + orderBy(items, expr); + + expect(o1.toString).not.toHaveBeenCalled(); + expect(o2.toString).not.toHaveBeenCalled(); + + toString = originalToString; + }); + + + it('should use the return value of `valueOf()` for subsequent steps (if non-primitive)', + function() { + var o1 = {k: 1, valueOf: function() { return o3; }}; + var o2 = {k: 2, valueOf: function() { return o4; }}; + var o3 = {k: 3, toString: function() { return 4; }}; + var o4 = {k: 4, toString: function() { return 3; }}; + + var items = [o1, o2]; + var expr = null; + var sorted = [o2, o1]; + + expect(orderBy(items, expr)).toEqual(sorted); + } + ); + + + it('should use the return value of `toString()` for subsequent steps (if non-primitive)', + function() { + var o1 = {k: 1, toString: function() { return o3; }}; + var o2 = {k: 2, toString: function() { return o4; }}; + var o3 = {k: 3}; + var o4 = {k: 4}; + + var items = [o1, o2]; + var expr = null; + var comparator = jasmine.createSpy('comparator').and.returnValue(-1); + + orderBy(items, expr, comparator); + var args = comparator.calls.argsFor(0); + + expect(args).toContain(jasmine.objectContaining({value: o3, type: 'object'})); + expect(args).toContain(jasmine.objectContaining({value: o4, type: 'object'})); + } + ); + + + it('should use the object itself as `value` if no conversion took place', function() { + var o1 = {k: 1}; + var o2 = {k: 2}; + + var items = [o1, o2]; + var expr = null; + var comparator = jasmine.createSpy('comparator').and.returnValue(-1); + + orderBy(items, expr, comparator); + var args = comparator.calls.argsFor(0); + + expect(args).toContain(jasmine.objectContaining({value: o1, type: 'object'})); + expect(args).toContain(jasmine.objectContaining({value: o2, type: 'object'})); + }); + }); + + + it('should support multiple predicates and per-predicate sorting direction', function() { + var items = [ + {owner: 'ownerA', type: 'typeA'}, + {owner: 'ownerB', type: 'typeB'}, + {owner: 'ownerC', type: 'typeB'}, + {owner: 'ownerD', type: 'typeB'} + ]; + var expr = ['type', '-owner']; + var sorted = [ + {owner: 'ownerA', type: 'typeA'}, + {owner: 'ownerC', type: 'typeB'}, + {owner: 'ownerB', type: 'typeB'}, + {owner: 'ownerD', type: 'typeB'} + ]; + + var comparator = function(o1, o2) { + var v1 = o1.value; + var v2 = o2.value; + var isNerd1 = v1.toLowerCase().indexOf('nerd') !== -1; + var isNerd2 = v2.toLowerCase().indexOf('nerd') !== -1; + + // Shamelessly promote "nerds" + if (isNerd1 || isNerd2) { + return (isNerd1 && isNerd2) ? 0 : (isNerd1) ? -1 : 1; + } + + // No "nerd"; alpabetical order + return (v1 === v2) ? 0 : (v1 < v2) ? -1 : 1; + }; + + expect(orderBy(items, expr, comparator)).toEqual(sorted); + }); + }); }); From ef6d0be62ee8ea56badd75a1093ec63591486ebe Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Thu, 26 May 2016 22:55:05 +0300 Subject: [PATCH 2/4] fixup 1 --- src/ng/filter/orderBy.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js index 8f4ff3427c52..ea2bad082718 100644 --- a/src/ng/filter/orderBy.js +++ b/src/ng/filter/orderBy.js @@ -12,13 +12,14 @@ * The `collection` can be an Array or array-like object (e.g. NodeList, jQuery object, TypedArray, * String, etc). * - * The `expression` can be a signle predicate or a list of predicates, each serving as a tie-breaker + * The `expression` can be a single predicate, or a list of predicates each serving as a tie-breaker * for the preceeding one. The `expression` is evaluated against each item and the output is used * for comparing with other items. * * The comparison is done using the `comparator` function. If none is specified, a default, built-in - * comparator is used (see below for details). Passing `true` as the comparator has a special - * meaning: It will use the built-in comparator, but sort in reverse order. + * comparator is used (see below for details - in a nutshell, it compares numbers numerically and + * strings alphabetically). Passing `true` as the comparator has a special meaning: It will use the + * built-in comparator, but sort in reverse order. * * ### Under the hood * @@ -45,7 +46,7 @@ * In order to ensure that the sorting will be deterministic across platforms, if none of the * specified predicates can distinguish between two items, `orderBy` will automatically introduce a * dummy predicate that returns the item's index as `value`. - * (If you are using a custom comparator, make sure it can handle this case as well.) + * (If you are using a custom comparator, make sure it can handle this predicate as well.) * * Finally, in an attempt to simplify things, if a predicate returns an object as the extracted * value for an item, `orderBy` will try to convert that object to a primitive value, before passing @@ -53,19 +54,19 @@ * * 1. If the object has a `valueOf()` method that returns a primitive, its return value will be * used instead.
- * (If the object does have a `valueOf()` method, but it returns another object, then the - * returned object will be used in subsequent steps.) + * (If the object has a `valueOf()` method that returns another object, then the returned object + * will be used in subsequent steps.) * 2. If the object has a custom `toString()` method (i.e. not the one inherited from `Object`) that * returns a primitive, its return value will be used instead.
- * (If the object does have a `toString()` method, but it returns another object, then the - * returned object will be used in subsequent steps.) + * (If the object has a `toString()` method that returns another object, then the returned object + * will be used in subsequent steps.) * 3. No conversion; the object itself is used. * * ### The default comparator * * The default, built-in comparator should be sufficient for most usecases. In short, it compares * numbers numerically, strings alphabetically (and case-insensitively), for objects falls back to - * using their index in the original colection, and sorts values of different types by type. + * using their index in the original collection, and sorts values of different types by type. * * More specifically, it follows these steps to determine the relative order of items: * @@ -87,7 +88,7 @@ * * Can be one of: * - * - `Function`: A getter function. Ths function will be called with each item as argument and + * - `Function`: A getter function. This function will be called with each item as argument and * the return value will be used for sorting. * - `string`: An Angular expression. This expression will be evaluated against each item and the * result will be used for sorting. For example, use `name` to sort by a property called `name` @@ -114,7 +115,7 @@ * order (expression is set to `'-age'`). The `comparator` is not set, which means it defaults to * the built-in comparator. * - +
@@ -179,7 +180,7 @@ * properties, as shown in the next example. (Remember that `comparator` can be set to `true` for * reverse ordering.) * - + 
Sort by = {{propertyName}}; reverse = {{reverse}}
@@ -311,7 +312,7 @@ * * Example: * - + 
Sort by = {{propertyName}}; reverse = {{reverse}}
@@ -440,10 +441,10 @@ * * @example * If you have very specific requirements about the way items are sorted, you can pass your own - * comparator function. For example, you might need to compare some strings is a locale-sensitive + * comparator function. For example, you might need to compare some strings in a locale-sensitive * way: * - +
From a35fd7a92cfa912a407fcd9928db41ad3fedb272 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Mon, 30 May 2016 20:56:52 +0300 Subject: [PATCH 3/4] fixup 2 - Decouple `reverse` from `comparator` arguments. --- src/ng/filter/orderBy.js | 69 +++++---- test/ng/filter/orderBySpec.js | 254 ++++++++++++++++++++-------------- 2 files changed, 187 insertions(+), 136 deletions(-) diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js index ea2bad082718..49cb9493f6b4 100644 --- a/src/ng/filter/orderBy.js +++ b/src/ng/filter/orderBy.js @@ -16,10 +16,12 @@ * for the preceeding one. The `expression` is evaluated against each item and the output is used * for comparing with other items. * + * You can change the sorting order by setting `reverse` to `true`. By default, items are sorted in + * ascending order. + * * The comparison is done using the `comparator` function. If none is specified, a default, built-in * comparator is used (see below for details - in a nutshell, it compares numbers numerically and - * strings alphabetically). Passing `true` as the comparator has a special meaning: It will use the - * built-in comparator, but sort in reverse order. + * strings alphabetically). * * ### Under the hood * @@ -35,7 +37,7 @@ * index: ... * } * ``` - * 2. The comparator function is used to sort the items, besed on the derived values, types and + * 2. The comparator function is used to sort the items, based on the derived values, types and * indices. * * If you use a custom comparator, it will be called with pairs of objects of the form @@ -55,11 +57,11 @@ * 1. If the object has a `valueOf()` method that returns a primitive, its return value will be * used instead.
* (If the object has a `valueOf()` method that returns another object, then the returned object - * will be used in subsequent steps.) + * will be used in subsequent steps.) * 2. If the object has a custom `toString()` method (i.e. not the one inherited from `Object`) that * returns a primitive, its return value will be used instead.
* (If the object has a `toString()` method that returns another object, then the returned object - * will be used in subsequent steps.) + * will be used in subsequent steps.) * 3. No conversion; the object itself is used. * * ### The default comparator @@ -103,17 +105,17 @@ * * **Note:** If the predicate is missing or empty then it defaults to `'+'`. * - * @param {(boolean|Function)=} comparator - The comparator function used to determine the relative - * order of value pairs. If omitted, a built-in comparator will be used. Passing `true` will use - * the built-in comparator, but reverse the order. + * @param {boolean=} reverse - If `true`, reverse the sorting order. + * @param {(Function)=} comparator - The comparator function used to determine the relative order of + * value pairs. If omitted, a built-in comparator will be used. * * @returns {Array} - The sorted array. * * * @example - * The example below demonstrates a simple ngRepeat, where the data is sorted by age in descending - * order (expression is set to `'-age'`). The `comparator` is not set, which means it defaults to - * the built-in comparator. + * The example below demonstrates a simple {@link ngRepeat ngRepeat}, where the data is sorted by + * age in descending order (expression is set to `'-age'`). The `comparator` is not set, which means + * it defaults to the built-in comparator. * @@ -164,7 +166,7 @@ // Element locators var names = element.all(by.repeater('friends').column('friend.name')); - it('should initially sort friends by age in reverse order', function() { + it('should sort friends by age in reverse order', function() { expect(names.get(0).getText()).toBe('Adam'); expect(names.get(1).getText()).toBe('Julie'); expect(names.get(2).getText()).toBe('Mike'); @@ -176,9 +178,8 @@ *
* * @example - * The `expression` and `comparator` parameters can be controlled dynamically through scope - * properties, as shown in the next example. (Remember that `comparator` can be set to `true` for - * reverse ordering.) + * The `expression` and `reverse` parameters can be controlled dynamically through scope properties, + * as shown in the next example. * @@ -442,7 +443,8 @@ * @example * If you have very specific requirements about the way items are sorted, you can pass your own * comparator function. For example, you might need to compare some strings in a locale-sensitive - * way: + * way. (When specifying a custom comparator, you also need to pass a value for the `reverse` + * argument - passing `false` retains the default sorting order, i.e. ascending.) * @@ -454,7 +456,7 @@
- + @@ -536,7 +538,7 @@ */ orderByFilter.$inject = ['$parse']; function orderByFilter($parse) { - return function(array, sortPredicate, compareFn) { + return function(array, sortPredicate, reverseOrder, compareFn) { if (array == null) return array; if (!isArrayLike(array)) { @@ -548,11 +550,10 @@ function orderByFilter($parse) { var predicates = processPredicates(sortPredicate); + var descending = reverseOrder ? -1 : 1; + // Define the `compare()` function. Use a default comparator if none is specified. - // A value of `true` means: Use the default comparator, but reverse the order. - var compare = isFunction(compareFn) ? compareFn : - compareFn ? reverseDefaultCompare : - defaultCompare; + var compare = isFunction(compareFn) ? compareFn : defaultCompare; // The next three lines are a version of a Swartzian Transform idiom from Perl // (sometimes called the Decorate-Sort-Undecorate idiom) @@ -565,7 +566,7 @@ function orderByFilter($parse) { function getComparisonObject(value, index) { // NOTE: We are adding an extra `tieBreaker` value based on the element's index. - // This will be used to keep the sort stable when all the input predicates cannot + // This will be used to keep the sort stable when none of the input predicates can // distinguish between two elements. return { value: value, @@ -577,17 +578,19 @@ function orderByFilter($parse) { } function doComparison(v1, v2) { - for (var index=0, length = predicates.length; index < length; ++index) { - var result = compare(v1.predicateValues[index], v2.predicateValues[index]) * predicates[index].descending; - if (result) return result; + for (var i = 0, ii = predicates.length; i < ii; i++) { + var result = compare(v1.predicateValues[i], v2.predicateValues[i]); + if (result) { + return result * predicates[i].descending * descending; + } } - return compare(v1.tieBreaker, v2.tieBreaker); + return compare(v1.tieBreaker, v2.tieBreaker) * descending; } }; - function processPredicates(sortPredicate) { - return sortPredicate.map(function(predicate) { + function processPredicates(sortPredicates) { + return sortPredicates.map(function(predicate) { var descending = 1, get = identity; if (isFunction(predicate)) { @@ -605,7 +608,7 @@ function orderByFilter($parse) { } } } - return { get: get, descending: descending }; + return {get: get, descending: descending}; }); } @@ -643,7 +646,7 @@ function orderByFilter($parse) { } else if (type === 'object') { value = objectValue(value); } - return { value: value, type: type, index: index }; + return {value: value, type: type, index: index}; } function defaultCompare(v1, v2) { @@ -675,8 +678,4 @@ function orderByFilter($parse) { return result; } - - function reverseDefaultCompare(v1, v2) { - return -1 * defaultCompare(v1, v2); - } } diff --git a/test/ng/filter/orderBySpec.js b/test/ng/filter/orderBySpec.js index f76bc5bddc9a..6f1790d1428a 100644 --- a/test/ng/filter/orderBySpec.js +++ b/test/ng/filter/orderBySpec.js @@ -221,7 +221,23 @@ describe('Filter: orderBy', function() { describe('(reversing order)', function() { - it('should reverse collection if `comparator` param is truthy (but not a function)', + it('should not reverse collection if `comparator` param is falsy', + function() { + var items = [{a: 2}, {a: 15}]; + var expr = 'a'; + var sorted = [{a: 2}, {a: 15}]; + + expect(orderBy(items, expr, false)).toEqual(sorted); + expect(orderBy(items, expr, 0)).toEqual(sorted); + expect(orderBy(items, expr, '')).toEqual(sorted); + expect(orderBy(items, expr, NaN)).toEqual(sorted); + expect(orderBy(items, expr, null)).toEqual(sorted); + expect(orderBy(items, expr, undefined)).toEqual(sorted); + } + ); + + + it('should reverse collection if `comparator` param is truthy', function() { var items = [{a: 2}, {a: 15}]; var expr = 'a'; @@ -230,6 +246,9 @@ describe('Filter: orderBy', function() { expect(orderBy(items, expr, true)).toEqual(sorted); expect(orderBy(items, expr, 1)).toEqual(sorted); expect(orderBy(items, expr, 'reverse')).toEqual(sorted); + expect(orderBy(items, expr, {})).toEqual(sorted); + expect(orderBy(items, expr, [])).toEqual(sorted); + expect(orderBy(items, expr, noop)).toEqual(sorted); } ); @@ -243,6 +262,7 @@ describe('Filter: orderBy', function() { ); }); + describe('(built-in comparator)', function() { it('should compare numbers numarically', function() { var items = [100, 3, 20]; @@ -293,29 +313,56 @@ describe('Filter: orderBy', function() { it('should support a custom comparator', function() { var items = [4, 42, 2]; var expr = null; + var reverse = null; var sorted = [42, 2, 4]; var comparator = function(o1, o2) { var v1 = o1.value; var v2 = o2.value; + // 42 always comes first if (v1 === v2) return 0; if (v1 === 42) return -1; if (v2 === 42) return 1; + // Default comparison for other values return (v1 < v2) ? -1 : 1; }; - expect(orderBy(items, expr, comparator)).toEqual(sorted); + expect(orderBy(items, expr, reverse, comparator)).toEqual(sorted); + }); + + + it('should support `reverseOrder` with a custom comparator', function() { + var items = [4, 42, 2]; + var expr = null; + var reverse = true; + var sorted = [4, 2, 42]; + + var comparator = function(o1, o2) { + var v1 = o1.value; + var v2 = o2.value; + + // 42 always comes first + if (v1 === v2) return 0; + if (v1 === 42) return -1; + if (v2 === 42) return 1; + + // Default comparison for other values + return (v1 < v2) ? -1 : 1; + }; + + expect(orderBy(items, expr, reverse, comparator)).toEqual(sorted); }); it('should pass `{value, type, index}` objects to comparators', function() { var items = [false, noop, 999, {}, '', undefined]; var expr = null; + var reverse = null; var comparator = jasmine.createSpy('comparator').and.returnValue(-1); - orderBy(items, expr, comparator); + orderBy(items, expr, reverse, comparator); var allArgsFlat = Array.prototype.concat.apply([], comparator.calls.allArgs()); expect(allArgsFlat).toContain({index: 0, type: 'boolean', value: false }); @@ -330,9 +377,10 @@ describe('Filter: orderBy', function() { it('should treat a value of `null` as `"null"`', function() { var items = [null, null]; var expr = null; + var reverse = null; var comparator = jasmine.createSpy('comparator').and.returnValue(-1); - orderBy(items, expr, comparator); + orderBy(items, expr, reverse, comparator); var arg = comparator.calls.argsFor(0)[0]; expect(arg).toEqual(jasmine.objectContaining({ @@ -345,13 +393,14 @@ describe('Filter: orderBy', function() { it('should not convert strings to lower-case', function() { var items = ['c', 'B', 'a']; var expr = null; + var reverse = null; var sorted = ['B', 'a', 'c']; var comparator = function(o1, o2) { return (o1.value < o2.value) ? -1 : 1; }; - expect(orderBy(items, expr, comparator)).toEqual(sorted); + expect(orderBy(items, expr, reverse, comparator)).toEqual(sorted); }); @@ -359,9 +408,10 @@ describe('Filter: orderBy', function() { function() { var items = ['foo', 'bar']; var expr = null; + var reverse = null; var comparator = jasmine.createSpy('comparator').and.returnValue(0); - orderBy(items, expr, comparator); + orderBy(items, expr, reverse, comparator); expect(comparator).toHaveBeenCalledTimes(2); var lastArgs = comparator.calls.mostRecent().args; @@ -372,138 +422,140 @@ describe('Filter: orderBy', function() { ); - describe('(object as `value`)', function() { - it('should use the return value of `valueOf()` (if primitive)', function() { - var o1 = {k: 1, valueOf: function() { return 2; }}; - var o2 = {k: 2, valueOf: function() { return 1; }}; + it('should support multiple predicates and per-predicate sorting direction', function() { + var items = [ + {owner: 'ownerA', type: 'typeA'}, + {owner: 'ownerB', type: 'typeB'}, + {owner: 'ownerC', type: 'typeB'}, + {owner: 'ownerD', type: 'typeB'} + ]; + var expr = ['type', '-owner']; + var reverse = null; + var sorted = [ + {owner: 'ownerA', type: 'typeA'}, + {owner: 'ownerC', type: 'typeB'}, + {owner: 'ownerB', type: 'typeB'}, + {owner: 'ownerD', type: 'typeB'} + ]; - var items = [o1, o2]; - var expr = null; - var sorted = [o2, o1]; + var comparator = function(o1, o2) { + var v1 = o1.value; + var v2 = o2.value; + var isNerd1 = v1.toLowerCase().indexOf('nerd') !== -1; + var isNerd2 = v2.toLowerCase().indexOf('nerd') !== -1; - expect(orderBy(items, expr)).toEqual(sorted); - }); + // Shamelessly promote "nerds" + if (isNerd1 || isNerd2) { + return (isNerd1 && isNerd2) ? 0 : (isNerd1) ? -1 : 1; + } + // No "nerd"; alpabetical order + return (v1 === v2) ? 0 : (v1 < v2) ? -1 : 1; + }; - it('should use the return value of `toString()` (if primitive)', function() { - var o1 = {k: 1, toString: function() { return 2; }}; - var o2 = {k: 2, toString: function() { return 1; }}; + expect(orderBy(items, expr, reverse, comparator)).toEqual(sorted); + }); + }); - var items = [o1, o2]; - var expr = null; - var sorted = [o2, o1]; + describe('(object as `value`)', function() { + it('should use the return value of `valueOf()` (if primitive)', function() { + var o1 = {k: 1, valueOf: function() { return 2; }}; + var o2 = {k: 2, valueOf: function() { return 1; }}; - expect(orderBy(items, expr)).toEqual(sorted); - }); + var items = [o1, o2]; + var expr = null; + var sorted = [o2, o1]; + expect(orderBy(items, expr)).toEqual(sorted); + }); - it('should ignore the `toString()` inherited from `Object`', function() { - /* globals toString: true */ - // The global `toString` variable (in 'src/Angular.js') - // has already captured `Object.prototype.toString` - var originalToString = toString; - toString = jasmine.createSpy('toString').and.callFake(originalToString); + it('should use the return value of `toString()` (if primitive)', function() { + var o1 = {k: 1, toString: function() { return 2; }}; + var o2 = {k: 2, toString: function() { return 1; }}; - var o1 = Object.create({toString: toString}); - var o2 = Object.create({toString: toString}); + var items = [o1, o2]; + var expr = null; + var sorted = [o2, o1]; - var items = [o1, o2]; - var expr = null; + expect(orderBy(items, expr)).toEqual(sorted); + }); - orderBy(items, expr); - expect(o1.toString).not.toHaveBeenCalled(); - expect(o2.toString).not.toHaveBeenCalled(); + it('should ignore the `toString()` inherited from `Object`', function() { + /* globals toString: true */ - toString = originalToString; - }); + // The global `toString` variable (in 'src/Angular.js') + // has already captured `Object.prototype.toString` + var originalToString = toString; + toString = jasmine.createSpy('toString').and.callFake(originalToString); + var o1 = Object.create({toString: toString}); + var o2 = Object.create({toString: toString}); - it('should use the return value of `valueOf()` for subsequent steps (if non-primitive)', - function() { - var o1 = {k: 1, valueOf: function() { return o3; }}; - var o2 = {k: 2, valueOf: function() { return o4; }}; - var o3 = {k: 3, toString: function() { return 4; }}; - var o4 = {k: 4, toString: function() { return 3; }}; + var items = [o1, o2]; + var expr = null; - var items = [o1, o2]; - var expr = null; - var sorted = [o2, o1]; + orderBy(items, expr); - expect(orderBy(items, expr)).toEqual(sorted); - } - ); + expect(o1.toString).not.toHaveBeenCalled(); + expect(o2.toString).not.toHaveBeenCalled(); + toString = originalToString; + }); - it('should use the return value of `toString()` for subsequent steps (if non-primitive)', - function() { - var o1 = {k: 1, toString: function() { return o3; }}; - var o2 = {k: 2, toString: function() { return o4; }}; - var o3 = {k: 3}; - var o4 = {k: 4}; - var items = [o1, o2]; - var expr = null; - var comparator = jasmine.createSpy('comparator').and.returnValue(-1); + it('should use the return value of `valueOf()` for subsequent steps (if non-primitive)', + function() { + var o1 = {k: 1, valueOf: function() { return o3; }}; + var o2 = {k: 2, valueOf: function() { return o4; }}; + var o3 = {k: 3, toString: function() { return 4; }}; + var o4 = {k: 4, toString: function() { return 3; }}; - orderBy(items, expr, comparator); - var args = comparator.calls.argsFor(0); + var items = [o1, o2]; + var expr = null; + var sorted = [o2, o1]; - expect(args).toContain(jasmine.objectContaining({value: o3, type: 'object'})); - expect(args).toContain(jasmine.objectContaining({value: o4, type: 'object'})); - } - ); + expect(orderBy(items, expr)).toEqual(sorted); + } + ); - it('should use the object itself as `value` if no conversion took place', function() { - var o1 = {k: 1}; - var o2 = {k: 2}; + it('should use the return value of `toString()` for subsequent steps (if non-primitive)', + function() { + var o1 = {k: 1, toString: function() { return o3; }}; + var o2 = {k: 2, toString: function() { return o4; }}; + var o3 = {k: 3}; + var o4 = {k: 4}; var items = [o1, o2]; var expr = null; + var reverse = null; var comparator = jasmine.createSpy('comparator').and.returnValue(-1); - orderBy(items, expr, comparator); + orderBy(items, expr, reverse, comparator); var args = comparator.calls.argsFor(0); - expect(args).toContain(jasmine.objectContaining({value: o1, type: 'object'})); - expect(args).toContain(jasmine.objectContaining({value: o2, type: 'object'})); - }); - }); - + expect(args).toContain(jasmine.objectContaining({value: o3, type: 'object'})); + expect(args).toContain(jasmine.objectContaining({value: o4, type: 'object'})); + } + ); - it('should support multiple predicates and per-predicate sorting direction', function() { - var items = [ - {owner: 'ownerA', type: 'typeA'}, - {owner: 'ownerB', type: 'typeB'}, - {owner: 'ownerC', type: 'typeB'}, - {owner: 'ownerD', type: 'typeB'} - ]; - var expr = ['type', '-owner']; - var sorted = [ - {owner: 'ownerA', type: 'typeA'}, - {owner: 'ownerC', type: 'typeB'}, - {owner: 'ownerB', type: 'typeB'}, - {owner: 'ownerD', type: 'typeB'} - ]; - var comparator = function(o1, o2) { - var v1 = o1.value; - var v2 = o2.value; - var isNerd1 = v1.toLowerCase().indexOf('nerd') !== -1; - var isNerd2 = v2.toLowerCase().indexOf('nerd') !== -1; + it('should use the object itself as `value` if no conversion took place', function() { + var o1 = {k: 1}; + var o2 = {k: 2}; - // Shamelessly promote "nerds" - if (isNerd1 || isNerd2) { - return (isNerd1 && isNerd2) ? 0 : (isNerd1) ? -1 : 1; - } + var items = [o1, o2]; + var expr = null; + var reverse = null; + var comparator = jasmine.createSpy('comparator').and.returnValue(-1); - // No "nerd"; alpabetical order - return (v1 === v2) ? 0 : (v1 < v2) ? -1 : 1; - }; + orderBy(items, expr, reverse, comparator); + var args = comparator.calls.argsFor(0); - expect(orderBy(items, expr, comparator)).toEqual(sorted); + expect(args).toContain(jasmine.objectContaining({value: o1, type: 'object'})); + expect(args).toContain(jasmine.objectContaining({value: o2, type: 'object'})); }); }); }); From f983563f566e6210ac42ab510000eb3b7fa3635a Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Mon, 6 Jun 2016 22:08:25 +0300 Subject: [PATCH 4/4] fixup 3 - Fix typos and wording. --- src/ng/filter/orderBy.js | 37 +++++++++++++++++++++++------------ test/ng/filter/orderBySpec.js | 6 +++--- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js index 49cb9493f6b4..9e5e3e42990e 100644 --- a/src/ng/filter/orderBy.js +++ b/src/ng/filter/orderBy.js @@ -9,6 +9,9 @@ * Returns an array containing the items from the specified `collection`, ordered by a `comparator` * function based on the values computed using the `expression` predicate. * + * For example, `[{id: 'foo'}, {id: 'bar'}] | orderBy:'id'` would result in + * `[{id: 'bar'}, {id: 'foo'}]`. + * * The `collection` can be an Array or array-like object (e.g. NodeList, jQuery object, TypedArray, * String, etc). * @@ -28,8 +31,8 @@ * Ordering the specified `collection` happens in two phases: * * 1. All items are passed through the predicate (or predicates), and the returned values are saved - * along with their type (`string`, `number` etc). For example, an item `{name: 'foo'}`, passed - * through a predicate that extracts the value of the `name` property, would be transformed to: + * along with their type (`string`, `number` etc). For example, an item `{label: 'foo'}`, passed + * through a predicate that extracts the value of the `label` property, would be transformed to: * ``` * { * value: 'foo', @@ -93,13 +96,15 @@ * - `Function`: A getter function. This function will be called with each item as argument and * the return value will be used for sorting. * - `string`: An Angular expression. This expression will be evaluated against each item and the - * result will be used for sorting. For example, use `name` to sort by a property called `name` - * or `name.substring(0, 3)` to sort by the first 3 characters of the `name` property.
+ * result will be used for sorting. For example, use `'label'` to sort by a property called + * `label` or `'label.substring(0, 3)'` to sort by the first 3 characters of the `label` + * property.
* (The result of a constant expression is interpreted as a property name to be used for - * comparison. For example, use `"special name"` to sort by a property called `special name`.)
+ * comparison. For example, use `'"special name"'` (note the extra pair of quotes) to sort by a + * property called `special name`.)
* An expression can be optionally prefixed with `+` or `-` to control the sorting direction, - * ascending or descending. For example, `+name` or `-name`. If no property is provided, (e.g. - * `'+'` or `'-'`), the collection element itself is used in comparisons. + * ascending or descending. For example, `'+label'` or `'-label'`. If no property is provided, + * (e.g. `'+'` or `'-'`), the collection element itself is used in comparisons. * - `Array`: An array of function and/or string predicates. If a predicate cannot determine the * relative order of two items, the next predicate is used as a tie-breaker. * @@ -107,12 +112,14 @@ * * @param {boolean=} reverse - If `true`, reverse the sorting order. * @param {(Function)=} comparator - The comparator function used to determine the relative order of - * value pairs. If omitted, a built-in comparator will be used. + * value pairs. If omitted, the built-in comparator will be used. * * @returns {Array} - The sorted array. * * * @example + * ### Ordering a table with `ngRepeat` + * * The example below demonstrates a simple {@link ngRepeat ngRepeat}, where the data is sorted by * age in descending order (expression is set to `'-age'`). The `comparator` is not set, which means * it defaults to the built-in comparator. @@ -178,8 +185,10 @@ *
* * @example - * The `expression` and `reverse` parameters can be controlled dynamically through scope properties, - * as shown in the next example. + * ### Changing parameters dynamically + * + * All parameters can be changed dynamically. The next example shows how you can make the columns of + * a table sortable, by binding the `expression` and `reverse` parameters to scope properties. * @@ -307,11 +316,11 @@ *
* * @example - * It's also possible to call the `orderBy` filter manually, by injecting `orderByFilter`, and + * ### Using `orderBy` inside a controller + * + * It is also possible to call the `orderBy` filter manually, by injecting `orderByFilter`, and * calling it with the desired parameters. (Alternatively, you could inject the `$filter` factory * and retrieve the `orderBy` filter with `$filter('orderBy')`.) - * - * Example: * @@ -441,6 +450,8 @@ *
* * @example + * ### Using a custom comparator + * * If you have very specific requirements about the way items are sorted, you can pass your own * comparator function. For example, you might need to compare some strings in a locale-sensitive * way. (When specifying a custom comparator, you also need to pass a value for the `reverse` diff --git a/test/ng/filter/orderBySpec.js b/test/ng/filter/orderBySpec.js index 6f1790d1428a..411410e143e7 100644 --- a/test/ng/filter/orderBySpec.js +++ b/test/ng/filter/orderBySpec.js @@ -221,7 +221,7 @@ describe('Filter: orderBy', function() { describe('(reversing order)', function() { - it('should not reverse collection if `comparator` param is falsy', + it('should not reverse collection if `reverse` param is falsy', function() { var items = [{a: 2}, {a: 15}]; var expr = 'a'; @@ -237,7 +237,7 @@ describe('Filter: orderBy', function() { ); - it('should reverse collection if `comparator` param is truthy', + it('should reverse collection if `reverse` param is truthy', function() { var items = [{a: 2}, {a: 15}]; var expr = 'a'; @@ -253,7 +253,7 @@ describe('Filter: orderBy', function() { ); - it('should reverse collection if `comparator` param is `true`, even without an `expression`', + it('should reverse collection if `reverse` param is `true`, even without an `expression`', function() { var originalItems = [{id: 2}, {id: 1}, {id: 4}, {id: 3}]; var reversedItems = [{id: 3}, {id: 4}, {id: 1}, {id: 2}];
Name Favorite Letter
{{friend.name}} {{friend.favoriteLetter}}