From 60199b600f389b7c04bd7c2e98ce1a6a6926ed82 Mon Sep 17 00:00:00 2001
From: Shaun Cutts <shaunc@factfiber.com>
Date: Thu, 20 Aug 2015 14:28:45 +0200
Subject: [PATCH 1/2] fixes buffer to buffer on both sides and tests that
 content in buffer rendered and other content not

---
 addon/components/ember-collection.js |  6 +++-
 tests/helpers/helpers.js             | 44 ++++++++++++++++++++++++++--
 tests/unit/content-test.js           | 10 +++++--
 tests/unit/scroll-top-test.js        |  7 +++--
 4 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/addon/components/ember-collection.js b/addon/components/ember-collection.js
index 85733e48..4f622bfc 100644
--- a/addon/components/ember-collection.js
+++ b/addon/components/ember-collection.js
@@ -154,8 +154,12 @@ export default Ember.Component.extend({
 
     var index = this._cellLayout.indexAt(this._offsetX, this._offsetY, this._width, this._height);
     var count = this._cellLayout.count(this._offsetX, this._offsetY, this._width, this._height);
+    this.currentIndex = index;
+    this.currentCount = count;
     var items = this._items;
-    index = Math.max(index - this._buffer, 0);
+    var bufferBefore = Math.min(index, this._buffer);
+    index -= bufferBefore;
+    count += bufferBefore;
     count = Math.min(count + this._buffer, Ember.get(items, 'length') - index);
     var i, pos, width, height, style, itemIndex, itemKey, cell;
 
diff --git a/tests/helpers/helpers.js b/tests/helpers/helpers.js
index 99c7bbba..c779a849 100644
--- a/tests/helpers/helpers.js
+++ b/tests/helpers/helpers.js
@@ -64,8 +64,9 @@ function extractPosition(element) {
   return position;
 }
 
-function sortItemsByPosition(view) {
-  var items = findItems(view);
+function sortItemsByPosition(view, visibleOnly) {
+  var find = visibleOnly ? findVisibleItems : findItems;
+  var items = find(view);
   return sortElementsByPosition(items);
 }
 
@@ -103,6 +104,44 @@ function itemPositions(view) {
   }).sort(sortByPosition);
 }
 
+function checkContent(view, assert, expectedFirstItem, expectedCount) {
+  var elements = sortItemsByPosition(view, true);
+  var content = view.get('content') || [];
+  assert.ok(
+    expectedFirstItem + expectedCount <= content.length,
+    'No more items than are in content are rendered.');
+  var buffer = view.get('buffer') | 5;
+
+  // TODO: we are recapitulating calculations done by fixed grid, as
+  // we don't have access to the layout. This will not work with
+  // mixed grid layout.
+  //
+  // In the future, if a listener for actual first item and count are
+  // included in interface, we can limit ourselves to just recomputing
+  // the number that should be in the buffer.
+
+  var width = view.get('width') | 0;
+  var itemWidth = view.get('itemWidth') || 1;
+  var istart = Math.max(expectedFirstItem - buffer, 0);
+  // TODO: padding is one extra row -- how to calculate with mixed grid
+  var padding = Math.floor(width / itemWidth);
+  // include buffer before
+  var scount = expectedCount + Math.min(expectedFirstItem, buffer);
+  // include padding (in case of non-integral scroll)
+  var pcount = scount + Math.min(Math.max(content.length - istart - scount, 0), padding);
+  // include buffer after
+  var count = pcount + Math.min(Math.max(content.length - istart - pcount, 0), buffer);
+  assert.equal(
+    elements.length, count, "Rendered expected number of elements.");
+  for (let i = 0; i < count; i++) {
+    let elt = elements[i];
+    let item = content[i + istart];
+    assert.equal(
+      $(elt).text().trim(), item.name, 
+      'Item ' + (i + 1) + ' rendered');
+  }  
+}
+
 export {
   itemPositions,
   generateContent,
@@ -111,4 +150,5 @@ export {
   findContainer,
   findItems,
   findVisibleItems,
+  checkContent,
   sortItemsByPosition };
diff --git a/tests/unit/content-test.js b/tests/unit/content-test.js
index e1e2deec..d5cbb7e6 100644
--- a/tests/unit/content-test.js
+++ b/tests/unit/content-test.js
@@ -1,6 +1,8 @@
 import Ember from 'ember';
 import { test, moduleForComponent } from 'ember-qunit';
-import { generateContent, sortItemsByPosition, findItems, findContainer } from '../helpers/helpers';
+import { 
+  generateContent, sortItemsByPosition, findItems, findContainer, 
+  checkContent } from '../helpers/helpers';
 import template from '../templates/fixed-grid';
 
 var nItems = 100;
@@ -28,6 +30,7 @@ test("replacing the list content", function(assert) {
       .length, 1, "The rendered list was updated");
 
     assert.equal(findItems(this).height(), itemHeight, "The scrollable view has the correct height");
+    checkContent(this, assert, 0, 1);
   });
 });
 
@@ -44,7 +47,6 @@ test("adding to the front of the list content", function(assert) {
   });
 
   var positionSorted = sortItemsByPosition(this);
-
   assert.equal(
     Ember.$(positionSorted[0]).text().trim(),
     "Item -1", "The item has been inserted in the list");
@@ -55,6 +57,7 @@ test("adding to the front of the list content", function(assert) {
     findContainer(this).height(),
     expectedRows * itemHeight,
     "The scrollable view has the correct height");
+    checkContent(this, assert, 0, 50);
 });
 
 test("inserting in the middle of visible content", function(assert) {
@@ -78,6 +81,8 @@ test("inserting in the middle of visible content", function(assert) {
   assert.equal(
     Ember.$(positionSorted[2]).text().trim(),
     "Item 2'", "The item has been inserted in the list");
+
+  checkContent(this, assert, 0, 50);
 });
 
 test("clearing the content", function(assert) {
@@ -120,4 +125,5 @@ test("deleting the first element", function(assert) {
   assert.equal(
     Ember.$(positionSorted[0]).text().trim(),
     "Item 2", "Item 1 has been remove from the list.");
+    checkContent(this, assert, 0, 50);
 });
diff --git a/tests/unit/scroll-top-test.js b/tests/unit/scroll-top-test.js
index 045603e8..050e4f5b 100644
--- a/tests/unit/scroll-top-test.js
+++ b/tests/unit/scroll-top-test.js
@@ -1,6 +1,6 @@
 import Ember from 'ember';
 import { test, moduleForComponent } from 'ember-qunit';
-import { generateContent, sortItemsByPosition } from '../helpers/helpers';
+import { generateContent, sortItemsByPosition, checkContent } from '../helpers/helpers';
 import template from '../templates/fixed-grid';
 
 var content = generateContent(5);
@@ -31,6 +31,7 @@ test("base case", function(assert) {
   });
 
   assert.equal(this.$('.ember-collection').prop('scrollTop'), 0);
+  checkContent(this, assert, 0, 5);
 });
 
 test("scroll but within content length", function(assert){
@@ -58,6 +59,7 @@ test("scroll but within content length", function(assert){
   assert.equal(
     Ember.$(positionSorted[0]).text().trim(),
     "Item 1", "The first item is not visible but in buffer.");
+  checkContent(this, assert, 0, 5);
 });
 
 test("scroll within content length, beyond buffer", function(assert){
@@ -72,7 +74,6 @@ test("scroll within content length, beyond buffer", function(assert){
   });
 
   Ember.run(()=>{ this.set('offsetY', 150);});
-
   assert.equal(
     this.$('.ember-collection').prop('scrollTop'), 150, 'scrolled to item 7');
 
@@ -94,6 +95,7 @@ test("scroll within content length, beyond buffer", function(assert){
   assert.equal(
     Ember.$(positionSorted[0]).text().trim(),
     "Item 1", "The first item is in buffer again.");
+  checkContent(this, assert, 0, 5);
 });
 
 test("scroll but beyond content length", function(assert) {
@@ -112,4 +114,5 @@ test("scroll but beyond content length", function(assert) {
   });
 
   assert.equal(this.$('.ember-collection').prop('scrollTop'), 0);
+  checkContent(this, assert, 0, 5);
 });

From 9fce27f08427f6b584e2b6665ad2ddc000c0a369 Mon Sep 17 00:00:00 2001
From: Shaun Cutts <shaunc@factfiber.com>
Date: Thu, 20 Aug 2015 15:06:49 +0200
Subject: [PATCH 2/2] adds sliceDidChange listener

---
 addon/components/ember-collection.js |  22 ++++--
 tests/helpers/helpers.js             |  29 ++++++++
 tests/unit/content-test.js           |   5 +-
 tests/unit/slice-did-change-test.js  | 104 +++++++++++++++++++++++++++
 4 files changed, 150 insertions(+), 10 deletions(-)
 create mode 100644 tests/unit/slice-did-change-test.js

diff --git a/addon/components/ember-collection.js b/addon/components/ember-collection.js
index 4f622bfc..aaf24023 100644
--- a/addon/components/ember-collection.js
+++ b/addon/components/ember-collection.js
@@ -152,14 +152,14 @@ export default Ember.Component.extend({
     var priorMap = this._cellMap;
     var cellMap = Object.create(null);
 
-    var index = this._cellLayout.indexAt(this._offsetX, this._offsetY, this._width, this._height);
-    var count = this._cellLayout.count(this._offsetX, this._offsetY, this._width, this._height);
-    this.currentIndex = index;
-    this.currentCount = count;
+    var startingIndex = this._cellLayout.indexAt(
+      this._offsetX, this._offsetY, this._width, this._height);
+    var visibleCount = this._cellLayout.count(
+      this._offsetX, this._offsetY, this._width, this._height);
     var items = this._items;
-    var bufferBefore = Math.min(index, this._buffer);
-    index -= bufferBefore;
-    count += bufferBefore;
+    var bufferBefore = Math.min(startingIndex, this._buffer);
+    var index = startingIndex - bufferBefore;
+    var count = visibleCount + bufferBefore;
     count = Math.min(count + this._buffer, Ember.get(items, 'length') - index);
     var i, pos, width, height, style, itemIndex, itemKey, cell;
 
@@ -219,6 +219,14 @@ export default Ember.Component.extend({
       cellMap[itemKey] = cell;
       this._cells.push(cell);
     }
+    if (this._startingIndex !== startingIndex || this._visibleCount !== visibleCount) {
+      this._startingIndex = startingIndex;
+      this._visibleCount = visibleCount;
+      let listener = this.getAttr('sliceDidChange');
+      if (listener != null) {
+        listener(startingIndex, visibleCount);
+      }
+    }
     this._cellMap = cellMap;
     return this._cells;
   },
diff --git a/tests/helpers/helpers.js b/tests/helpers/helpers.js
index c779a849..2d2bb3e0 100644
--- a/tests/helpers/helpers.js
+++ b/tests/helpers/helpers.js
@@ -103,6 +103,35 @@ function itemPositions(view) {
     return extractPosition(e);
   }).sort(sortByPosition);
 }
+function checkContent(view, assert, expectedFirstItem, expectedCount) {
+  var elements = sortElementsByPosition(view.$('.ember-list-item-view'))
+    .filter(function(){ return $(this).css('display') !== 'none'; });
+  var content = view.get('content') || [];
+  assert.ok(
+    expectedFirstItem + expectedCount <= content.length,
+    'No more items than are in content are rendered.');
+  var buffer = view.get('buffer') | 5;
+  var width = view.get('width') | 0;
+  var itemWidth = view.get('itemWidth') || 1;
+  var istart = Math.max(expectedFirstItem - buffer, 0);
+  // TODO: padding is one extra row -- how to calculate with mixed grid
+  var padding = Math.floor(width / itemWidth);
+  // include buffer before
+  var scount = expectedCount + Math.min(expectedFirstItem, buffer);
+  // include padding (in case of non-integral scroll)
+  var pcount = scount + Math.min(Math.max(content.length - istart - scount, 0), padding);
+  // include buffer after
+  var count = pcount + Math.min(Math.max(content.length - istart - pcount, 0), buffer);
+  assert.equal(
+    elements.length, count, "Rendered expected number of elements.");
+  for (let i = 0; i < count; i++) {
+    let elt = elements[i];
+    let item = content[i + istart];
+    assert.equal(
+      $(elt).text().trim(), item.name, 
+      'Item ' + (i + 1) + ' rendered');
+  }
+}
 
 function checkContent(view, assert, expectedFirstItem, expectedCount) {
   var elements = sortItemsByPosition(view, true);
diff --git a/tests/unit/content-test.js b/tests/unit/content-test.js
index d5cbb7e6..881419c7 100644
--- a/tests/unit/content-test.js
+++ b/tests/unit/content-test.js
@@ -57,7 +57,7 @@ test("adding to the front of the list content", function(assert) {
     findContainer(this).height(),
     expectedRows * itemHeight,
     "The scrollable view has the correct height");
-    checkContent(this, assert, 0, 50);
+  checkContent(this, assert, 0, 50);
 });
 
 test("inserting in the middle of visible content", function(assert) {
@@ -81,7 +81,6 @@ test("inserting in the middle of visible content", function(assert) {
   assert.equal(
     Ember.$(positionSorted[2]).text().trim(),
     "Item 2'", "The item has been inserted in the list");
-
   checkContent(this, assert, 0, 50);
 });
 
@@ -125,5 +124,5 @@ test("deleting the first element", function(assert) {
   assert.equal(
     Ember.$(positionSorted[0]).text().trim(),
     "Item 2", "Item 1 has been remove from the list.");
-    checkContent(this, assert, 0, 50);
+  checkContent(this, assert, 0, 50);
 });
diff --git a/tests/unit/slice-did-change-test.js b/tests/unit/slice-did-change-test.js
new file mode 100644
index 00000000..2f2d3013
--- /dev/null
+++ b/tests/unit/slice-did-change-test.js
@@ -0,0 +1,104 @@
+import Ember from 'ember';
+import { test, moduleForComponent } from 'ember-qunit';
+import { generateContent } from '../helpers/helpers';
+import hbs from 'htmlbars-inline-precompile';
+
+var itemWidth = 50;
+var itemHeight = 100;
+var width = 100;
+var height = 400;
+
+var content = generateContent(30);
+
+moduleForComponent('ember-collection', 'slice did change', {integration: true});
+
+test("base case", function(assert) {
+  var offsetY = 0;
+  var startingIndex, visibleCount;
+  this.set('sliceDidChange', function (index, count){
+    startingIndex = index;
+    visibleCount = count;
+  });
+  Ember.run(() => {
+    this.render(hbs`{{#ember-collection
+    items=content
+    cell-layout=(fixed-grid-layout itemWidth itemHeight)
+    width=width
+    height=height
+    offset-x=offsetX
+    offset-y=offsetY
+    sliceDidChange=(action sliceDidChange)
+    class="ember-collection"
+    as |item| ~}}
+  <div class="list-item">{{item.name}}</div>
+{{~/ember-collection~}}`);
+    this.setProperties({ width, height, itemWidth, itemHeight, content, offsetY });
+  });
+  assert.equal(startingIndex, 0, 'without scroll starts at top');
+  assert.equal(visibleCount, 10, '10 elements (including padding) displayed');
+});
+
+test("scroll down", function(assert) {
+  var offsetY = 0;
+  var startingIndex, visibleCount;
+  this.set('sliceDidChange', function (index, count){
+    startingIndex = index;
+    visibleCount = count;
+  });
+  Ember.run(() => {
+    this.render(hbs`{{#ember-collection
+    items=content
+    cell-layout=(fixed-grid-layout itemWidth itemHeight)
+    width=width
+    height=height
+    offset-x=offsetX
+    offset-y=offsetY
+    sliceDidChange=(action sliceDidChange)
+    class="ember-collection"
+    as |item| ~}}
+  <div class="list-item">{{item.name}}</div>
+{{~/ember-collection~}}`);
+    this.setProperties({ width, height, itemWidth, itemHeight, content, offsetY });
+  });
+  assert.equal(startingIndex, 0, 'Without scroll starts at top.');
+  assert.equal(visibleCount, 10, '10 elements (including padding) displayed.');
+  Ember.run(() => {
+    this.set('offsetY', 100);
+  });
+  assert.equal(startingIndex, 2, 'After scroll, one line down.');
+  assert.equal(visibleCount, 10, '10 elements (including padding) displayed.');
+
+});
+
+
+test("change height", function(assert) {
+  var offsetY = 0;
+  var startingIndex, visibleCount;
+  this.set('sliceDidChange', function (index, count){
+    startingIndex = index;
+    visibleCount = count;
+  });
+  Ember.run(() => {
+    this.render(hbs`{{#ember-collection
+    items=content
+    cell-layout=(fixed-grid-layout itemWidth itemHeight)
+    width=width
+    height=height
+    offset-x=offsetX
+    offset-y=offsetY
+    sliceDidChange=(action sliceDidChange)
+    class="ember-collection"
+    as |item| ~}}
+  <div class="list-item">{{item.name}}</div>
+{{~/ember-collection~}}`);
+    this.setProperties({ width, height, itemWidth, itemHeight, content, offsetY });
+  });
+  assert.equal(startingIndex, 0, 'without scroll starts at top');
+  assert.equal(visibleCount, 10, '10 elements (including padding) displayed');
+  Ember.run(() => {
+    this.set('height', 300);
+  });
+  assert.equal(startingIndex, 0, 'After height change still at top.');
+  assert.equal(visibleCount, 8, 'now 6 elements (including padding) displayed.');
+
+});