fix(collectionRepeat): properly delete items when setting size to 0

Refactored to create a `changeValidator` object that will properly tell you if a resize or a data change requires a refresh. Closes #3299.
ionic-team · Mar 12, 2015 · 3dc6ab6 · 3dc6ab6
1 parent a301483
commit 3dc6ab6
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 23 deletions.
diff --git a/js/angular/directive/collectionRepeat.js b/js/angular/directive/collectionRepeat.js
@@ -138,30 +138,37 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
 
     var afterItemsContainer = initAfterItemsContainer();
 
+    var changeValidator = makeChangeValidator();
     initDimensions();
 
     // Dimensions are refreshed on resize or data change.
-    angular.element($window).on('resize', validateResize);
     scrollCtrl.$element.on('scroll.resize', refreshDimensions);
-    var unlistenToExposeAside = $rootScope.$on('$ionicExposeAside', validateResize);
+
+    angular.element($window).on('resize', onResize);
+    var unlistenToExposeAside = $rootScope.$on('$ionicExposeAside', onResize);
     $timeout(refreshDimensions, 0, false);
 
+    function onResize() {
+      if (changeValidator.resizeRequiresRefresh(scrollView.__clientWidth, scrollView.__clientHeight)) {
+        refreshDimensions();
+      }
+    }
+
     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();
+        getRepeatManager().setData(data);
+        if (changeValidator.dataChangeRequiresRefresh(data)) refreshDimensions();
       });
     });
 
     scope.$on('$destroy', function() {
-      angular.element($window).off('resize', validateResize);
+      angular.element($window).off('resize', onResize);
       unlistenToExposeAside();
       scrollCtrl.$element && scrollCtrl.$element.off('scroll.resize', refreshDimensions);
 
@@ -174,6 +181,32 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
       repeatManager = null;
     });
 
+    function makeChangeValidator() {
+      var self;
+      return (self = {
+        dataLength: 0,
+        width: 0,
+        height: 0,
+        resizeRequiresRefresh: function(newWidth, newHeight) {
+          var requiresRefresh = self.dataLength &&
+            newWidth && newWidth !== self.width &&
+            newHeight && newHeight !== self.height;
+
+          self.width = newWidth;
+          self.height = newHeight;
+
+          return !!requiresRefresh;
+        },
+        dataChangeRequiresRefresh: function(newData) {
+          var requiresRefresh = newData.length > 0 || newData.length < self.dataLength;
+
+          self.dataLength = newData.length;
+
+          return !!requiresRefresh;
+        }
+      });
+    }
+
     function getRepeatManager() {
       return repeatManager || (repeatManager = new $ionicCollectionManager({
         afterItemsNode: afterItemsContainer[0],
@@ -242,23 +275,14 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
       }
     }
 
-    // Make sure this resize actually changed the size of the screen
-    function validateResize() {
-      var h = scrollView.__clientHeight, w = scrollView.__clientWidth;
-      if (w && h && (validateResize.height !== h || validateResize.width !== w)) {
-        validateResize.height = h;
-        validateResize.width = w;
-        refreshDimensions();
-      }
-    }
     function refreshDimensions() {
-      if (!data.length) return;
+      var hasData = data.length > 0;
 
-      if (heightData.computed || widthData.computed) {
+      if (hasData && (heightData.computed || widthData.computed)) {
         computeStyleDimensions();
       }
 
-      if (heightData.computed) {
+      if (hasData && heightData.computed) {
         heightData.value = computedStyleDimensions.height;
         if (!heightData.value) {
           throw new Error('collection-repeat tried to compute the height of repeated elements "' +
@@ -269,7 +293,8 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
         // If it's a constant with a getter (eg percent), we just refresh .value after resize
         heightData.value = heightData.getValue();
       }
-      if (widthData.computed) {
+
+      if (hasData && widthData.computed) {
         widthData.value = computedStyleDimensions.width;
         if (!widthData.value) {
           throw new Error('collection-repeat tried to compute the width of repeated elements "' +
@@ -527,7 +552,7 @@ function RepeatManagerFactory($rootScope, $window, $$rAF) {
       }
     };
 
-    this.refreshData = function(newData) {
+    this.setData = function(newData) {
       data = newData;
       (view.onRefreshData || angular.noop)();
       isDataReady = true;

diff --git a/scss/_util.scss b/scss/_util.scss
@@ -276,6 +276,7 @@
 }
 .collection-repeat-container {
   position: relative;
+  z-index: 1; //make sure it's above the after-container
 }
 .collection-repeat-after-container {
   z-index: 0;

diff --git a/test/html/collection-repeat/basic-list.html b/test/html/collection-repeat/basic-list.html
@@ -15,6 +15,7 @@
     <ion-header-bar class="bar-positive">
       <button class="button" ng-click="$root.showDelete = !$root.showDelete">Toggle Delete</button>
       <h1 class="title">Basic List: Static Dimensions</h1>
+      <button class="button" ng-click="clearList()">Clear List</button>
     </ion-header-bar>
     <ion-content>
       <h4 style="margin: 80px;">I have 80px margin</h4>
@@ -40,10 +41,18 @@ <h2>Stuff after list</h2>
       image: 'http://placekitten.com/'+(100+50%i)+'/'+(100+50%i)
     });
   }
+  init();
 
-  $timeout(function() {
-    for (var i = 0; i < 100; i++) addImage();
-  }, 1000);
+  $scope.clearList = function() {
+    $scope.items.length = 0;
+    init();
+  };
+
+  function init() {
+    $timeout(function() {
+      for (var i = 0; i < 100; i++) addImage();
+    }, 1000);
+  }
 }
   </script>
   <style>