Merge pull request #32 from jasonmit/persist-scroll-top

Fix persisting the scrollTop on item updates
jasonmit · Feb 5, 2016 · 6cf3da2 · 6cf3da2
2 parents cd5c50d + 6d944b3
commit 6cf3da2
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 19 deletions.
diff --git a/addon/components/virtual-each.js b/addon/components/virtual-each.js
@@ -152,7 +152,8 @@ const VirtualEachComponent = Component.extend(EventListenerMixin, DefaultAttrsMi
     });
   },
 
-  scrollTo(positionIndex) {
+  scrollTo: Ember.observer('_positionIndex', function() {
+    const positionIndex = get(this, '_positionIndex');
     const itemHeight = this.getAttr('itemHeight');
     const totalHeight = get(this, '_totalHeight');
     const _visibleItemCount = get(this, '_visibleItemCount');
@@ -163,27 +164,29 @@ const VirtualEachComponent = Component.extend(EventListenerMixin, DefaultAttrsMi
     const sanitizedIndex = Math.min(startingIndex, maxVisibleItemTop);
     const sanitizedPadding = (startingPadding > maxPadding) ? maxPadding : startingPadding;
 
-    this.calculateVisibleItems(sanitizedIndex);
-    this.$().scrollTop(sanitizedPadding);
-  },
+    this.scheduledRender = emberRun.scheduleOnce('afterRender', () => {
+      this.calculateVisibleItems(sanitizedIndex);
+      this.$().scrollTop(sanitizedPadding);
+    });
+  }),
 
-  didReceiveAttrs(attrs) {
+  didReceiveAttrs() {
     this._super(...arguments);
 
     RSVP.cast(this.getAttr('items')).then((attrItems) => {
       const items = emberArray(attrItems);
 
       this.setProperties({
         _items: items,
+        _positionIndex: this.getAttr('positionIndex'),
         _totalHeight: Math.max(get(items, 'length') * this.getAttr('itemHeight'), 0)
       });
-
-      if (attrs.newAttrs.hasOwnProperty('items') || attrs.newAttrs.hasOwnProperty('positionIndex')) {
-        emberRun.scheduleOnce('afterRender', () => {
-          this.scrollTo(this.getAttr('positionIndex'));
-        });
-      }
     });
+  },
+
+  willDestroyElement() {
+    this._super(...arguments);
+    Ember.run.cancel(this.scheduledRender);
   }
 });
 

diff --git a/tests/unit/virtual-each-test.js b/tests/unit/virtual-each-test.js
@@ -244,23 +244,44 @@ describeComponent('virtual-each', 'VirtualEachComponent', {
 
       describe("positioning to the last item in list", function() {
         beforeEach(function() {
+          let totalHeight = 200*35;
+          let virtualHeight = 500;
+          this.expectedScrollTop = totalHeight - virtualHeight;
           run(() => {
             this.set('positionIndex', 199);
           });
         });
 
-        it("is the last item in the list", function() {
+        it("positions the last item at the bottom of the list", function() {
           var $component = this.$('.virtual-each');
+          expect($component.find('li').first().text()).to.contain('Item 185');
           expect($component.find('li').last().text()).to.contain('Item 199');
         });
 
-        it("sets the scrollTop such that the last item exists is the last visible item", function() {
+        it("sets the scrollTop to the total height less the virtual height", function() {
           var $component = this.$('.virtual-each');
-          let totalHeight = 200*35;
-          let virtualHeight = 500;
+          expect($component.scrollTop()).to.be.closeTo(this.expectedScrollTop, SCROLL_TOP_BUFFER);
+        });
 
-          let expectedScrollTop = totalHeight - virtualHeight;
-          expect($component.scrollTop()).to.be.closeTo(expectedScrollTop, SCROLL_TOP_BUFFER);
+        describe("receiving new items", function() {
+          beforeEach(function() {
+            return run(() => {
+              let items = this.get('items')
+                    .concat(this.get('items'))
+                    .map((value, index)=> { return "Item ".concat(index); });
+              return this.set('items', items);
+            });
+          });
+
+          it("persists the current index", function() {
+            var $component = this.$('.virtual-each');
+            expect($component.find('li').first().text()).to.contain('Item 185');
+          });
+
+          it("persists the scrollTop", function() {
+            var $component = this.$('.virtual-each');
+            expect($component.scrollTop()).to.be.closeTo(this.expectedScrollTop, SCROLL_TOP_BUFFER);
+          });
         });
       });
 
@@ -297,8 +318,10 @@ describeComponent('virtual-each', 'VirtualEachComponent', {
 
           describe("updating items", function() {
             beforeEach(function() {
-              let newItems = this.get('items').map((value, index)=> { return "newItem ".concat(index); });
-              this.set('items', newItems);
+              run(() => {
+                let updatedItems = this.get('items').map((value, index)=> { return "newItem ".concat(index); });
+                this.set('items', updatedItems);
+              });
             });
 
             it("persists the current index", function() {