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

Collectionbinder optimize sort #211

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 85 additions & 9 deletions Backbone.CollectionBinder.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,18 +160,94 @@
},

sortRootEls: function(){
this._collection.each(function(model, modelIndex){
// sorting an empty collection or a collection with a single element won't change anything
if (this._collection.length < 2) return;

// Algorithm: one by one take elements and put to the correct position in DOM
// Computational complexity: O(n^2)
// DOM operations:
// - $.children() calls: 1
// - $.before/after calls: n


// -----
// prepare data for sorting

var children = $(this._elManagerFactory._getParentEl()).children();

var order = 0;

var getElData = function (model) {
var modelElManager = this.getManagerForModel(model);
if(modelElManager){
var modelEl = modelElManager.getEl();
var currentRootEls = $(this._elManagerFactory._getParentEl()).children();
if (!modelElManager) return;

if(currentRootEls[modelIndex] !== modelEl[0]){
modelEl.detach();
modelEl.insertBefore(currentRootEls[modelIndex]);
}
var modelEl = modelElManager.getEl();
if (!modelEl) return;

return {
el: modelEl,
elIndex: children.index(modelEl),
order: order++
}
}, this);
};

var elements = _.chain(this._collection.models)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use this._collection.chain(), which I think reads better and also allows for implementation changes on where models are stored in a collection in the future.

.map(getElData, this)
.compact()
.value();

var minElIndex = _.min(elements, function (e) { return e.elIndex; }).elIndex;
var maxElIndex = _.max(elements, function (e) { return e.elIndex; }).elIndex;

// -----
// define some helpers

var findElByOrder = function (order) {
return _.findWhere(elements, {order: order});
};

var findElByElIndex = function (elIndex) {
return _.findWhere(elements, {elIndex: elIndex});
};

var moveEl = function (el, oldIndex, newIndex) {
var moveForward = newIndex > oldIndex;

if (newIndex < maxElIndex) {
var nextEl = findElByElIndex(newIndex + (moveForward ? 1 : 0));
nextEl.el.before(el);
} else {
var lastEl = findElByElIndex(newIndex);
lastEl.el.after(el);
}
};

var updateIndexesAfterMove = function (oldIndex, newIndex) {
var moveForward = newIndex > oldIndex;

// update indexes of all elements that were shifted after the current element movement
_.each(elements, function (e) {
var elIndex = e.elIndex;
if (moveForward && elIndex > oldIndex && elIndex <= newIndex) e.elIndex--;
if (!moveForward && elIndex >= newIndex && elIndex < oldIndex) e.elIndex++;
});
};

// -----
// Actual sorting happens here
_.times(elements.length, function (order) {
var elementData = findElByOrder(order);
var oldIndex = elementData.elIndex;

var newIndex = order + minElIndex;

if (oldIndex === newIndex) return;

moveEl(elementData.el, oldIndex, newIndex);
updateIndexesAfterMove(oldIndex, newIndex);

elementData.elIndex = newIndex;
});
}
});

Expand Down
103 changes: 101 additions & 2 deletions spec/javascripts/CollectionBinder/sorting.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ describe("CollectionBinder: sorting", function(){
}
});

var ParentViewWithStaticContent = ParentViewAutoSort.extend({
render: function () {
this.$el.append('<div class="static-content">Some static content that precedes the collection. Can be a table header, for instance</div>');

return ParentViewAutoSort.prototype.render.apply(this, arguments);
},

getStaticContentIndex: function () {
return this.$('.static-content').index();
}
});


var createViewForTest = function (View, collection) {
var view = new View ({collection: collection});
Expand Down Expand Up @@ -213,8 +225,6 @@ describe("CollectionBinder: sorting", function(){
it('Should rearrange elements on collection.sort()', function () {
var view = createView(this.collection);

var collectionBinder = view.collectionBinder;

this.collection.reset([{id: 1}, {id: 2}]);

expect(view.getNestedElementIndex(1)).toBe(0);
Expand All @@ -233,4 +243,93 @@ describe("CollectionBinder: sorting", function(){
expect(view.getNestedElementIndex(1)).toBe(1);
});
});


var testSequenceSorting = function (sequence, withStaticContent) {
var View = ParentViewAutoSort, minIndex = 0;

if (withStaticContent) {
View = ParentViewWithStaticContent;
minIndex = 1;
}

var collection = new Backbone.Collection();

var view = createViewForTest(View, collection);

var data = _.map(sequence, function (d) {
return {id: d};
});

collection.reset(data);

_.each(sequence, function (id, index) {
expect(view.getNestedElementIndex(id)).toBe(index + minIndex);
});

collection.comparator = 'id';
collection.sort();

if (withStaticContent) {
// index of the static content should remain 0
expect(view.getStaticContentIndex()).toBe(0);
}

// index of element should be index of the corresponding model in the collection plus <minIndex>
collection.each(function (model, index) {
expect(view.getNestedElementIndex(model.id)).toBe(index + minIndex);
});
};


describe('With the sort algorithm', function () {
var sequences = [
[1, 2, 3, 4, 5, 6],
[5, 4, 3, 2, 1],
[1, 2, 4, 5, 3],
[3, 5, 1, 8, 6]
];

_.each(sequences, function (sequence, index) {
it('Should properly sort sequence: '+ sequence, function () {
testSequenceSorting(sequence);
});
});
});


describe('With a container having static content before collection elements', function () {
var createView = function (collection) {
return createViewForTest(ParentViewWithStaticContent, collection);
};

it('Should insert the first element after the static content on collection.add with {at: 0}', function () {
var view = createView(this.unorderedCollection);

this.unorderedCollection.add({id: 1});
this.unorderedCollection.add({id: 0}, {at: 0});

// index of element should be index of the corresponding model in the collection plus 1
// because index=0 is the index of the static content prepended to the container
expect(view.getNestedElementIndex(0)).toBe(0+1);
expect(view.getNestedElementIndex(1)).toBe(1+1);
});

it('Should not touch the leading static content on collection.sort()', function () {
testSequenceSorting([1,2,0,3], true);
});

var sequences = [
[1, 2, 3, 4, 5, 6],
[5, 4, 3, 2, 1],
[1, 2, 4, 5, 3],
[3, 5, 1, 8, 6]
];

_.each(sequences, function (sequence, index) {
it('Should properly sort sequence: '+ sequence, function () {
testSequenceSorting(sequence);
});
});
});
});