From b22ab96e9ec9d132bbebbc2aaeb036ea77de86f7 Mon Sep 17 00:00:00 2001 From: Colin Skow Date: Tue, 6 Oct 2015 20:49:03 -0700 Subject: [PATCH] fix(repeater) resize scroller correctly, There was some recursion inside VirtualRepeatController.prototype.virtualRepeatUpdate_ which was causing the scroller not to shrink properly when items were removed. Fixes #5027. Closes #5031. In addition, in onDemand mode an undefined length would stop the container from sizing correctly. An undefined length will now be changed to zero.# It could also possibly fix #4950 and may be a better solution than #5009, but I haven't yet tested it against that issue.# fixes #5027# (use "git push" to publish your local commits) --- .../virtualRepeat/virtual-repeater.js | 12 ++-- .../virtualRepeat/virtual-repeater.spec.js | 68 +++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/components/virtualRepeat/virtual-repeater.js b/src/components/virtualRepeat/virtual-repeater.js index 07d8618e931..e1dad891cda 100644 --- a/src/components/virtualRepeat/virtual-repeater.js +++ b/src/components/virtualRepeat/virtual-repeater.js @@ -574,14 +574,9 @@ VirtualRepeatController.prototype.getItemSize = function() { * @private */ VirtualRepeatController.prototype.virtualRepeatUpdate_ = function(items, oldItems) { - var itemsLength = items ? items.length : 0; + var itemsLength = items && items.length || 0; var lengthChanged = false; - if (itemsLength !== this.itemsLength) { - lengthChanged = true; - this.itemsLength = itemsLength; - } - // If the number of items shrank, scroll up to the top. if (this.items && itemsLength < this.items.length && this.container.getScrollOffset() !== 0) { this.items = items; @@ -589,6 +584,11 @@ VirtualRepeatController.prototype.virtualRepeatUpdate_ = function(items, oldItem return; } + if (itemsLength !== this.itemsLength) { + lengthChanged = true; + this.itemsLength = itemsLength; + } + this.items = items; if (items !== oldItems || lengthChanged) { this.updateIndexes_(); diff --git a/src/components/virtualRepeat/virtual-repeater.spec.js b/src/components/virtualRepeat/virtual-repeater.spec.js index 99001aa4388..00aadcb3bdb 100644 --- a/src/components/virtualRepeat/virtual-repeater.spec.js +++ b/src/components/virtualRepeat/virtual-repeater.spec.js @@ -408,6 +408,74 @@ describe('', function() { expect(getRepeated().length).toBe(numItemRenderers); }); + it('should resize the scroller correctly when item length changes (vertical)', function() { + scope.items = createItems(200); + createRepeater(); + scope.$apply(); + $$rAF.flush(); + expect(sizer[0].offsetHeight).toBe(200 * ITEM_SIZE); + + // Scroll down half way + scroller[0].scrollTop = 100 * ITEM_SIZE; + scroller.triggerHandler('scroll'); + scope.$apply(); + $$rAF.flush(); + + // Remove some items + scope.items = createItems(20); + scope.$apply(); + $$rAF.flush(); + expect(scroller[0].scrollTop).toBe(0); + expect(sizer[0].offsetHeight).toBe(20 * ITEM_SIZE); + + // Scroll down half way + scroller[0].scrollTop = 10 * ITEM_SIZE; + scroller.triggerHandler('scroll'); + scope.$apply(); + $$rAF.flush(); + + // Add more items + scope.items = createItems(250); + scope.$apply(); + $$rAF.flush(); + expect(scroller[0].scrollTop).toBe(100); + expect(sizer[0].offsetHeight).toBe(250 * ITEM_SIZE); + }); + + it('should resize the scroller correctly when item length changes (horizontal)', function() { + container.attr({'md-orient-horizontal': ''}); + scope.items = createItems(200); + createRepeater(); + scope.$apply(); + $$rAF.flush(); + expect(sizer[0].offsetWidth).toBe(200 * ITEM_SIZE); + + // Scroll right half way + scroller[0].scrollLeft = 100 * ITEM_SIZE; + scroller.triggerHandler('scroll'); + scope.$apply(); + $$rAF.flush(); + + // Remove some items + scope.items = createItems(20); + scope.$apply(); + $$rAF.flush(); + expect(scroller[0].scrollLeft).toBe(0); + expect(sizer[0].offsetWidth).toBe(20 * ITEM_SIZE); + + // Scroll right half way + scroller[0].scrollLeft = 10 * ITEM_SIZE; + scroller.triggerHandler('scroll'); + scope.$apply(); + $$rAF.flush(); + + // Add more items + scope.items = createItems(250); + scope.$apply(); + $$rAF.flush(); + expect(sizer[0].offsetWidth).toBe(250 * ITEM_SIZE); + }); + it('should update topIndex when scrolling', function() { container.attr({'md-top-index': 'topIndex'}); scope.items = createItems(NUM_ITEMS);