diff --git a/src/next-collection-view.js b/src/next-collection-view.js index 1e6d67860a..cc2a0b5bcb 100644 --- a/src/next-collection-view.js +++ b/src/next-collection-view.js @@ -364,11 +364,11 @@ const CollectionView = Backbone.View.extend({ // Sorts views by viewComparator and sets the children to the new order _sortChildren() { - if (this.viewComparator === false) { return; } + let viewComparator = this.getComparator(); - this.triggerMethod('before:sort', this); + if (!viewComparator) { return; } - let viewComparator = this.getComparator(); + this.triggerMethod('before:sort', this); if (_.isFunction(viewComparator)) { // Must use native bind to preserve length @@ -404,13 +404,18 @@ const CollectionView = Backbone.View.extend({ // Additionally override this function to provide custom // viewComparator logic getComparator() { - return this.viewComparator || this._viewComparator; + if (this.viewComparator) { return this.viewComparator } + + if (!this.sortWithCollection || this.viewComparator === false || !this.collection) { + return false; + } + + return this._viewComparator; }, // Default internal view comparator that order the views by // the order of the collection _viewComparator(view) { - if (!this.collection) { return; } return this.collection.indexOf(view.model); }, @@ -439,7 +444,7 @@ const CollectionView = Backbone.View.extend({ delete this._addedViews; if (!viewFilter) { - if (addedViews && _.every(addedViews, _.bind(this._isAddedAtEnd, this))) { + if (!this.sortWithCollection && addedViews && _.every(addedViews, _.bind(this._isAddedAtEnd, this))) { return addedViews; } diff --git a/test/unit/next-collection-view/collection-view-children.spec.js b/test/unit/next-collection-view/collection-view-children.spec.js index 4b153cfff7..5079c41558 100644 --- a/test/unit/next-collection-view/collection-view-children.spec.js +++ b/test/unit/next-collection-view/collection-view-children.spec.js @@ -494,6 +494,9 @@ describe('next CollectionView Children', function() { it('should only append the added child', function() { this.sinon.stub(myCollectionView, 'attachHtml'); + + // Only true if not maintaining collection sort + myCollectionView.sortWithCollection = false; myCollectionView.addChildView(anotherView); const callArgs = myCollectionView.attachHtml.args[0]; const attachHtmlEls = callArgs[0]; diff --git a/test/unit/next-collection-view/collection-view-data.spec.js b/test/unit/next-collection-view/collection-view-data.spec.js index 7cc60b797b..977dbe38c6 100644 --- a/test/unit/next-collection-view/collection-view-data.spec.js +++ b/test/unit/next-collection-view/collection-view-data.spec.js @@ -209,26 +209,52 @@ describe('next CollectionView Data', function() { let myCollectionView; let collection; - beforeEach(function() { - collection = new Backbone.Collection([{ id: 1 }, { id: 2 }, { id: 3 }]); + describe('when sortWithCollection is true', function() { + beforeEach(function() { + collection = new Backbone.Collection([{ id: 1 }, { id: 2 }, { id: 3 }]); - myCollectionView = new MyCollectionView({ collection }); - myCollectionView.render(); - }); + myCollectionView = new MyCollectionView({ collection }); + myCollectionView.render(); + }); + + it('should append all of the children', function() { + this.sinon.stub(myCollectionView, 'attachHtml'); + collection.add([{ id: 4 }, { id: 5 }]); + + const callArgs = myCollectionView.attachHtml.args[0]; + const attachHtmlEls = callArgs[0]; + expect($(attachHtmlEls).children()).to.have.lengthOf(5); + }); - it('should only append the added children', function() { - this.sinon.stub(myCollectionView, 'attachHtml'); - collection.add([{ id: 4 }, { id: 5 }]); + it('should still have all children attached', function() { + collection.add([{ id: 4 }, { id: 5 }]); - const callArgs = myCollectionView.attachHtml.args[0]; - const attachHtmlEls = callArgs[0]; - expect($(attachHtmlEls).children()).to.have.lengthOf(2); + expect(myCollectionView.$el.children()).to.have.lengthOf(5); + }); }); - it('should still have all children attached', function() { - collection.add([{ id: 4 }, { id: 5 }]); + describe('when sortWithCollection is false', function() { + beforeEach(function() { + collection = new Backbone.Collection([{ id: 1 }, { id: 2 }, { id: 3 }]); + + myCollectionView = new MyCollectionView({ collection, sortWithCollection: false }); + myCollectionView.render(); + }); + + it('should only append the added children', function() { + this.sinon.stub(myCollectionView, 'attachHtml'); + collection.add([{ id: 4 }, { id: 5 }]); + + const callArgs = myCollectionView.attachHtml.args[0]; + const attachHtmlEls = callArgs[0]; + expect($(attachHtmlEls).children()).to.have.lengthOf(2); + }); + + it('should still have all children attached', function() { + collection.add([{ id: 4 }, { id: 5 }]); - expect(myCollectionView.$el.children()).to.have.lengthOf(5); + expect(myCollectionView.$el.children()).to.have.lengthOf(5); + }); }); }); diff --git a/test/unit/next-collection-view/collection-view-sorting.spec.js b/test/unit/next-collection-view/collection-view-sorting.spec.js index de155b2686..43267db5ba 100644 --- a/test/unit/next-collection-view/collection-view-sorting.spec.js +++ b/test/unit/next-collection-view/collection-view-sorting.spec.js @@ -100,40 +100,58 @@ describe('NextCollectionView - Sorting', function() { describe('when viewComparator is falsy but not false', function() { let myCollectionView; - beforeEach(function() { - myCollectionView = new MyCollectionView({ - sortWithCollection: false, - collection + describe('when sortWithCollection is true', function() { + beforeEach(function() { + myCollectionView = new MyCollectionView({ collection }); + + myCollectionView.render(); }); - myCollectionView.render(); - }); + it('should sort the collection by the collection index', function() { + expect(myCollectionView.$el.text()).to.equal(noSortText); + }); - it('should sort the collection by the collection index', function() { - expect(myCollectionView.$el.text()).to.equal(noSortText); - }); + it('should call "before:sort" event', function() { + expect(myCollectionView.onBeforeSort) + .to.have.been.calledOnce + .and.calledWith(myCollectionView); + }); - it('should call "before:sort" event', function() { - expect(myCollectionView.onBeforeSort) - .to.have.been.calledOnce - .and.calledWith(myCollectionView); - }); + it('should call "sort" event', function() { + expect(myCollectionView.onSort) + .to.have.been.calledOnce + .and.calledWith(myCollectionView); + }); - it('should call "sort" event', function() { - expect(myCollectionView.onSort) - .to.have.been.calledOnce - .and.calledWith(myCollectionView); + describe('when resorting the collection', function() { + it('should sort the collectionView by the collection index', function() { + collection.comparator = 'sort'; + collection.sort(); + + myCollectionView.render(); + + expect(myCollectionView.$el.text()).to.equal(sortText); + }); + }); }); - describe('when resorting the collection', function() { - it('should sort the collectionView by the collection index', function() { - collection.comparator = 'sort'; - collection.sort(); + describe('when sortWithCollection is false', function() { + beforeEach(function() { + myCollectionView = new MyCollectionView({ + sortWithCollection: false, + collection + }); myCollectionView.render(); + }); + + it('should not call "before:sort" event', function() { + expect(myCollectionView.onBeforeSort).to.not.be.called; + }); - expect(myCollectionView.$el.text()).to.equal(sortText); + it('should not call "sort" event', function() { + expect(myCollectionView.onSort).to.not.be.called; }); }); });