From 9b2bb84b8586cf07f3c5f7529177ecd82312420b Mon Sep 17 00:00:00 2001 From: Alexey Makhrov Date: Thu, 19 Feb 2015 08:27:54 -0800 Subject: [PATCH 1/2] CollectionBinder: Add unit tests for sorting algorithm, both for a dedicated container and for a container with some other static content. --- .../CollectionBinder/sorting.spec.js | 103 +++++++++++++++++- 1 file changed, 101 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/CollectionBinder/sorting.spec.js b/spec/javascripts/CollectionBinder/sorting.spec.js index 9ae6ee2..de75f5d 100644 --- a/spec/javascripts/CollectionBinder/sorting.spec.js +++ b/spec/javascripts/CollectionBinder/sorting.spec.js @@ -50,6 +50,18 @@ describe("CollectionBinder: sorting", function(){ } }); + var ParentViewWithStaticContent = ParentViewAutoSort.extend({ + render: function () { + this.$el.append('
Some static content that precedes the collection. Can be a table header, for instance
'); + + 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}); @@ -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); @@ -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 + 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); + }); + }); + }); }); \ No newline at end of file From ce7cc328fcad8dcf420e543dbf86f556f18ddea8 Mon Sep 17 00:00:00 2001 From: Alexey Makhrov Date: Thu, 19 Feb 2015 08:35:48 -0800 Subject: [PATCH 2/2] CollectionBinder: Update algorithm in sortRootEls to take static content into account and don't move it to bottom each time --- Backbone.CollectionBinder.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Backbone.CollectionBinder.js b/Backbone.CollectionBinder.js index d53d5ef..41dcb9a 100644 --- a/Backbone.CollectionBinder.js +++ b/Backbone.CollectionBinder.js @@ -160,15 +160,28 @@ }, sortRootEls: function(){ + // sorting an empty collection or a collection with a single element won't change anything + if (this._collection.length < 2) return; + + var children = $(this._elManagerFactory._getParentEl()).children(); + var indexes = this._collection.models.map(function (model) { + var modelElManager = this.getManagerForModel(model); + var modelEl = modelElManager.getEl(); + return modelEl.index(); + }, this); + + var minIndex = Math.min.apply(Math, indexes); + this._collection.each(function(model, modelIndex){ var modelElManager = this.getManagerForModel(model); if(modelElManager){ + var realIndex = modelIndex + minIndex; var modelEl = modelElManager.getEl(); var currentRootEls = $(this._elManagerFactory._getParentEl()).children(); - if(currentRootEls[modelIndex] !== modelEl[0]){ + if(currentRootEls[realIndex] !== modelEl[0]){ modelEl.detach(); - modelEl.insertBefore(currentRootEls[modelIndex]); + modelEl.insertBefore(currentRootEls[realIndex]); } } }, this);