From 4325025deb3dc6bc85fc260c2f4e3658a32480be Mon Sep 17 00:00:00 2001 From: Andrew Date: Sat, 7 Mar 2015 13:58:53 -0700 Subject: [PATCH] fix(collectionRepeat): fix data change while page disconnected, computed dimensions while no data Closes #3240. Closes #3238. --- js/angular/controller/scrollController.js | 1 - js/angular/directive/collectionRepeat.js | 169 ++++++++++-------- js/utils/utils.js | 4 +- test/html/collection-repeat/contacts.html | 1 - .../directive/collectionRepeat.unit.js | 5 +- 5 files changed, 96 insertions(+), 84 deletions(-) diff --git a/js/angular/controller/scrollController.js b/js/angular/controller/scrollController.js index f06f2483dd2..aa5e69f33c6 100644 --- a/js/angular/controller/scrollController.js +++ b/js/angular/controller/scrollController.js @@ -83,7 +83,6 @@ function($scope, $timeout(function() { scrollView && scrollView.run && scrollView.run(); - $element.triggerHandler('scroll.init'); }); self.getScrollView = function() { diff --git a/js/angular/directive/collectionRepeat.js b/js/angular/directive/collectionRepeat.js index bc76b1c3ffb..12b21356c49 100644 --- a/js/angular/directive/collectionRepeat.js +++ b/js/angular/directive/collectionRepeat.js @@ -95,7 +95,7 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r priority: 1000, transclude: 'element', $$tlb: true, - require: '^$ionicScroll', + require: '^^$ionicScroll', link: postLink }; @@ -122,6 +122,7 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r var heightData = {}; var widthData = {}; var computedStyleDimensions = {}; + var data = []; var repeatManager; // attr.collectionBufferSize is deprecated @@ -135,61 +136,33 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r // attr.collectionItemWidth is deprecated var widthExpr = attr.itemWidth || attr.collectionItemWidth; - //Height and width have four 'modes': - //1) Computed Mode - // - Nothing is supplied, so we getComputedStyle() on one element in the list and use - // that width and height value for the width and height of every item. This is re-computed - // every resize. - //2) Constant Mode, Static Integer - // - The user provides a constant number for width or height, in pixels. We parse it, - // store it on the `value` field, and it never changes - //3) Constant Mode, Percent - // - The user provides a percent string for width or height. The getter for percent is - // stored on the `getValue()` field, and is re-evaluated once every resize. The result - // is stored on the `value` field. - //4) Dynamic Mode - // - The user provides a dynamic expression for the width or height. This is re-evaluated - // for every item, stored on the `.getValue()` field. - if (!heightExpr && !widthExpr) { - heightData.computed = widthData.computed = true; - } else { - if (heightExpr) { - parseDimensionAttr(heightExpr, heightData); - } else { - heightData.computed = true; - } - if (!widthExpr) widthExpr = '"100%"'; - parseDimensionAttr(widthExpr, widthData); - } + var afterItemsContainer = initAfterItemsContainer(); - var afterItemsContainer = angular.element( - scrollView.__content.querySelector('.collection-repeat-after-container') - ); - if (!afterItemsContainer.length) { - var elementIsAfterRepeater = false; - var afterNodes = [].filter.call(scrollView.__content.childNodes, function(node) { if (ionic.DomUtil.contains(node, containerNode)) { - elementIsAfterRepeater = true; - return false; - } - return elementIsAfterRepeater; - }); - afterItemsContainer = angular.element(''); - if (scrollView.options.scrollingX) { - afterItemsContainer.addClass('horizontal'); - } - afterItemsContainer.append(afterNodes); - scrollView.__content.appendChild(afterItemsContainer[0]); - } + initDimensions(); - $$rAF(refreshDimensions); - scrollCtrl.$element.one('scroll.init', refreshDimensions); + var debouncedRefreshDimensions = ionic.animationFrameThrottle(refreshDimensions); + var debouncedOnResize = ionic.animationFrameThrottle(validateResize); - var onWindowResize = ionic.animationFrameThrottle(validateResize); - angular.element($window).on('resize', onWindowResize); + // Dimensions are refreshed on resize or data change. + angular.element($window).on('resize', debouncedOnResize); + $timeout(debouncedRefreshDimensions, 0, false); + + scope.$watchCollection(listGetter, function(newValue) { + data = newValue || (newValue = []); + if (!angular.isArray(newValue)) { + throw new Error("collection-repeat expected an array for '" + listExpr + "', " + + "but got a " + typeof value); + } + + // Wait for this digest to end before refreshing everything. + scope.$$postDigest(function() { + getRepeatManager().refreshData(newValue); + refreshDimensions(); + }); + }); scope.$on('$destroy', function() { - angular.element($window).off('resize', onWindowResize); - scrollCtrl.$element && scrollCtrl.$element.off('scroll.init', refreshDimensions); + angular.element($window).off('resize', debouncedOnResize); computedStyleNode && computedStyleNode.parentNode && computedStyleNode.parentNode.removeChild(computedStyleNode); @@ -215,20 +188,58 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r })); } - scope.$watchCollection(listGetter, function(newValue) { - newValue || (newValue = []); - - if (!angular.isArray(newValue)) { - throw new Error("collection-repeat expected an array for '" + listExpr + "', " + - "but got a " + typeof value); + function initAfterItemsContainer() { + var container = angular.element( + scrollView.__content.querySelector('.collection-repeat-after-container') + ); + // Put everything in the view after the repeater into a container. + if (!container.length) { + var elementIsAfterRepeater = false; + var afterNodes = [].filter.call(scrollView.__content.childNodes, function(node) { + if (ionic.DomUtil.contains(node, containerNode)) { + elementIsAfterRepeater = true; + return false; + } + return elementIsAfterRepeater; + }); + container = angular.element(''); + if (scrollView.options.scrollingX) { + container.addClass('horizontal'); + } + container.append(afterNodes); + scrollView.__content.appendChild(container[0]); } + return container; + } - // Wait for this digest to end before refreshing everything. - $timeout(function() { - getRepeatManager().refreshData(newValue); - if (newValue.length) refreshDimensions(); - }, 0, false); - }); + function initDimensions() { + //Height and width have four 'modes': + //1) Computed Mode + // - Nothing is supplied, so we getComputedStyle() on one element in the list and use + // that width and height value for the width and height of every item. This is re-computed + // every resize. + //2) Constant Mode, Static Integer + // - The user provides a constant number for width or height, in pixels. We parse it, + // store it on the `value` field, and it never changes + //3) Constant Mode, Percent + // - The user provides a percent string for width or height. The getter for percent is + // stored on the `getValue()` field, and is re-evaluated once every resize. The result + // is stored on the `value` field. + //4) Dynamic Mode + // - The user provides a dynamic expression for the width or height. This is re-evaluated + // for every item, stored on the `.getValue()` field. + if (!heightExpr && !widthExpr) { + heightData.computed = widthData.computed = true; + } else { + if (heightExpr) { + parseDimensionAttr(heightExpr, heightData); + } else { + heightData.computed = true; + } + if (!widthExpr) widthExpr = '"100%"'; + parseDimensionAttr(widthExpr, widthData); + } + } // Make sure this resize actually changed the size of the screen function validateResize() { @@ -240,6 +251,8 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r } } function refreshDimensions() { + if (!data.length) return; + if (heightData.computed || widthData.computed) { computeStyleDimensions(); } @@ -419,6 +432,10 @@ function RepeatManagerFactory($rootScope, $window, $$rAF) { var nextItemId = 0; var estimatedItemsAcross; + var scrollViewSetDimensions = isVertical ? + function() { scrollView.setDimensions(null, null, null, view.getContentSize(), true); } : + function() { scrollView.setDimensions(null, null, view.getContentSize(), null, true); }; + // view is a mix of list/grid methods + static/dynamic methods. // See bottom for implementations. Available methods: // @@ -467,8 +484,14 @@ function RepeatManagerFactory($rootScope, $window, $$rAF) { repeaterBeforeSize += current[isVertical ? 'offsetTop' : 'offsetLeft']; } while( ionic.DomUtil.contains(scrollView.__content, current = current.offsetParent) ); + if (!scrollView.__clientHeight || !scrollView.__clientWidth) { + scrollView.__clientWidth = scrollView.__container.clientWidth; + scrollView.__clientHeight = scrollView.__container.clientHeight; + } + (view.onRefreshLayout || angular.noop)(); view.refreshDirection(); + scrollViewSetDimensions(); // Create the pool of items for reuse, setting the size to (estimatedItemsOnScreen) * 2, // plus the size of the renderBuffer. @@ -481,19 +504,20 @@ function RepeatManagerFactory($rootScope, $window, $$rAF) { isLayoutReady = true; if (isLayoutReady && isDataReady) { - forceRerender(); + // If the resize or latest data change caused the scrollValue to + // now be out of bounds, resize the scrollView. + if (scrollView.__scrollLeft > scrollView.__maxScrollLeft || + scrollView.__scrollTop > scrollView.__maxScrollTop) { + scrollView.resize(); + } + forceRerender(true); } }; this.refreshData = function(newData) { data = newData; (view.onRefreshData || angular.noop)(); - isDataReady = true; - if (isLayoutReady && isDataReady) { - scrollView.resize(); - forceRerender(); - } }; this.destroy = function() { @@ -771,13 +795,6 @@ function RepeatManagerFactory($rootScope, $window, $$rAF) { function DynamicViewType() { var self = this; - var scrollViewSetDimensions = isVertical ? - function() { - scrollView.setDimensions(null, null, null, self.getContentSize(), true); - } : - function() { - scrollView.setDimensions(null, null, self.getContentSize(), null, true); - }; var debouncedScrollViewSetDimensions = ionic.debounce(scrollViewSetDimensions, 25, true); var calculateDimensions = isGridView ? calculateDimensionsGrid : calculateDimensionsList; var dimensionsIndex; diff --git a/js/utils/utils.js b/js/utils/utils.js index 7c06232a50e..2058b96971f 100644 --- a/js/utils/utils.js +++ b/js/utils/utils.js @@ -182,7 +182,7 @@ } var parent = scope.$parent; scope.$$disconnected = true; - scope.$broadcast('$ionic.disconnectScope'); + scope.$broadcast('$ionic.disconnectScope', scope); // See Scope.$destroy if (parent.$$childHead === scope) { @@ -211,7 +211,7 @@ } var parent = scope.$parent; scope.$$disconnected = false; - scope.$broadcast('$ionic.reconnectScope'); + scope.$broadcast('$ionic.reconnectScope', scope); // See Scope.$new for this logic... scope.$$prevSibling = parent.$$childTail; if (parent.$$childHead) { diff --git a/test/html/collection-repeat/contacts.html b/test/html/collection-repeat/contacts.html index c249b90cdf0..8964554260b 100644 --- a/test/html/collection-repeat/contacts.html +++ b/test/html/collection-repeat/contacts.html @@ -7,7 +7,6 @@ - diff --git a/test/unit/angular/directive/collectionRepeat.unit.js b/test/unit/angular/directive/collectionRepeat.unit.js index c147f60688f..81e20242a09 100644 --- a/test/unit/angular/directive/collectionRepeat.unit.js +++ b/test/unit/angular/directive/collectionRepeat.unit.js @@ -47,7 +47,7 @@ describe('collectionRepeat', function() { } var element; - inject(function($compile, $rootScope, $timeout) { + inject(function($compile, $rootScope) { repeaterScope = $rootScope.$new(); attrs = attrs || ''; if (!/item-height/.test(attrs)) attrs += ' item-height="25px"'; @@ -60,9 +60,6 @@ describe('collectionRepeat', function() { $rootScope.list = list; $compile(element)(repeaterScope); $rootScope.$apply(); - content.triggerHandler('scroll.init'); - $timeout.flush(); - $rootScope.$apply(); }); return element; }