Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pass list items to .sort(fn) and don't set the comparator #2399

Merged
merged 2 commits into from May 19, 2016
Merged

Pass list items to .sort(fn) and don't set the comparator #2399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0b06d8
Revert "Enable comparator set via sort method... 5c4d618" (#2398)
akagomez May 19, 2016
e825494
Pass list items to single use comparator function (#2159)
akagomez May 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 14 additions & 24 deletions list/sort/sort.js
View file
Open in desktop
Expand Up @@ -200,10 +200,10 @@ steal('can/util', 'can/list', function (can) {
/**
* @returns {number} The value that should be passed to the comparator
**/
_getComparatorValue: function (item) {
_getComparatorValue: function (item, singleUseComparator) {

// Get the user supplied comparator
var comparator = this.comparator;
// Use the value passed to .sort() as the comparator value
var comparator = singleUseComparator || this.comparator;

// If the comparator is a string, use it to get the value of that
// property on the item. Example:
Expand All @@ -228,24 +228,15 @@ steal('can/util', 'can/list', function (can) {
return a;
},

sort: function (comparator) {

if (arguments.length) {

// Set the comparator; Sort if the value changed
this.attr('comparator', comparator);
} else {

// Sort without a comparator
this._sort();
}

return this;
},

_sort: function () {
sort: function (singleUseComparator) {
var a, b, c, isSorted;

// Use the value passed to .sort() as the comparator function
// if it is a function
var comparatorFn = can.isFunction(singleUseComparator) ?
singleUseComparator :
this._comparator;

for (var i, iMin, j = 0, n = this.length; j < n-1; j++) {
iMin = j;

Expand All @@ -254,11 +245,11 @@ steal('can/util', 'can/list', function (can) {

for (i = j+1; i < n; i++) {

a = this._getComparatorValue(this.attr(i));
b = this._getComparatorValue(this.attr(iMin));
a = this._getComparatorValue(this.attr(i), singleUseComparator);
b = this._getComparatorValue(this.attr(iMin), singleUseComparator);

// [1, 2, 3, 4(b), 9, 6, 3(a)]
if (this._comparator.call(this, a, b) < 0) {
if (comparatorFn.call(this, a, b) < 0) {
isSorted = false;
iMin = i;
}
Expand All @@ -269,7 +260,7 @@ steal('can/util', 'can/list', function (can) {
// that are improperly sorted.
// Note: This is not part of the original selection
// sort agortithm
if (c && this._comparator.call(this, a, c) < 0) {
if (c && comparatorFn.call(this, a, c) < 0) {
isSorted = false;
}

Expand All @@ -285,7 +276,6 @@ steal('can/util', 'can/list', function (can) {
}
}


// Trigger length change so that {{#block}} helper can re-render
can.batch.trigger(this, 'length', [this.length]);

Expand Down
48 changes: 40 additions & 8 deletions list/sort/sort_test.js
View file
Open in desktop
Expand Up @@ -690,7 +690,7 @@ steal("can/list/sort", "can/test", "can/view/mustache", "can/view/stache", "can/
evaluate();
});

test('Setting comparator with .sort() (#2159)', function () {
test(".sort(comparatorFn) is passed list items regardless of .attr('comparator') value (#2159)", function () {
var list = new can.List([
{ letter: 'x', number: 3 },
{ letter: 'y', number: 2 },
Expand All @@ -703,16 +703,48 @@ steal("can/list/sort", "can/test", "can/view/mustache", "can/view/stache", "can/
equal(list.attr('1.number'), 2, 'Second value is correct');
equal(list.attr('2.number'), 3, 'Third value is correct');

list.sort('letter');
list.sort(function (a, b) {
a = a.attr('letter');
b = b.attr('letter');
return (a === b) ? 0 : (a < b) ? -1 : 1;
});

equal(list.attr('0.letter'), 'x', 'First value is correct after comparator set');
equal(list.attr('1.letter'), 'y', 'Second value is correct after comparator set');
equal(list.attr('2.letter'), 'z', 'Third value is correct after comparator set');
equal(list.attr('0.letter'), 'x',
'First value is correct after sort with single use comparator');
equal(list.attr('1.letter'), 'y',
'Second value is correct after sort with single use comparator');
equal(list.attr('2.letter'), 'z',
'Third value is correct after sort with single use comparator');
});

list.push({ letter: 'w', number: 4 });
test("List is not sorted on change after calling .sort(fn)", function () {
var list = new can.List([
{ letter: 'x', number: 3 },
{ letter: 'y', number: 2 },
{ letter: 'z', number: 1 },
]);

list.sort(function (a, b) {
a = a.attr('letter');
b = b.attr('letter');
return (a === b) ? 0 : (a < b) ? -1 : 1;
});

equal(list.attr('0.letter'), 'x',
'First value is correct after sort with single use comparator');
equal(list.attr('1.letter'), 'y',
'Second value is correct after sort with single use comparator');
equal(list.attr('2.letter'), 'z',
'Third value is correct after sort with single use comparator');

list.sort = function () {
ok(false, 'The list is not sorted as a result of change');
};

equal(list.attr('0.letter'), 'w', 'First value is correct after insert');
equal(list.attr('0.number'), 4, 'First value is correct after insert');
list.attr('2.letter', 'a');

equal(list.attr('0.letter'), 'x','First value is still correct');
equal(list.attr('1.letter'), 'y', 'Second value is still correct');
equal(list.attr('2.letter'), 'a', 'Third value is correctly out of place');
});
});