Merge pull request #57 from aantipov/fix-padding-default-min-values

Fix padding default and min values

Author: mfeingold

src/ui-scroll.js

@@ -197,28 +197,9 @@ angular.module('ui.scroll', [])
         return buffer;
       }
 
-      function Padding(template) {
-        let result;
-        let tagName = template.localName;
-
-        switch (tagName) {
-          case 'dl':
-            throw new Error(`ui-scroll directive does not support <${tagName}> as a repeating tag: ${template.outerHTML}`);
-          case 'tr':
-            let table = angular.element('<table><tr><td><div></div></td></tr></table>');
-            result = table.find('tr');
-            break;
-          case 'li':
-            result = angular.element('<li></li>');
-            break;
-          default:
-            result = angular.element('<div></div>');
-        }
-
-        return result;
-      }
-
       function Viewport(buffer, element, controllers, attrs) {
+        const PADDING_MIN = 0.3;
+        const PADDING_DEFAULT = 0.5;
         let topPadding = null;
         let bottomPadding = null;
         let averageItemHeight = 0;
@@ -232,7 +213,7 @@ angular.module('ui.scroll', [])
         let viewportOffset = viewport.offset() ? () => viewport.offset() : () => ({top: 0});
 
         function bufferPadding() {
-          return viewport.outerHeight() * Math.max(0.1, +attrs.padding || 0.1); // some extra space to initiate preload
+          return viewport.outerHeight() * Math.max(PADDING_MIN, +attrs.padding || PADDING_DEFAULT); // some extra space to initiate preload
         }
 
         angular.extend(viewport, {
@@ -241,6 +222,27 @@ angular.module('ui.scroll', [])
             bottomPadding = new Padding(template);
             element.before(topPadding);
             element.after(bottomPadding);
+
+            function Padding(template) {
+              let result;
+              let tagName = template.localName;
+
+              switch (tagName) {
+                case 'dl':
+                  throw new Error(`ui-scroll directive does not support <${tagName}> as a repeating tag: ${template.outerHTML}`);
+                case 'tr':
+                  let table = angular.element('<table><tr><td><div></div></td></tr></table>');
+                  result = table.find('tr');
+                  break;
+                case 'li':
+                  result = angular.element('<li></li>');
+                  break;
+                default:
+                  result = angular.element('<div></div>');
+              }
+
+              return result;
+            }
           },
 
           bottomDataPos() {
@@ -278,7 +280,7 @@ angular.module('ui.scroll', [])
             // clip the invisible items off the bottom
             let overage = 0;
 
-            for(let i = buffer.length - 1; i >= 0; i--) {
+            for (let i = buffer.length - 1; i >= 0; i--) {
               if (buffer[i].element.offset().top - viewportOffset().top <= viewport.outerHeight() + bufferPadding()) {
                 break;
               }
@@ -302,8 +304,8 @@ angular.module('ui.scroll', [])
             let overage = 0;
             let overageHeight = 0;
 
-            for(let i = 0; i < buffer.length; i++) {
-              if(buffer[i].element.offset().top - viewportOffset().top + buffer[i].element.outerHeight(true) >= (-1) * bufferPadding()) {
+            for (let i = 0; i < buffer.length; i++) {
+              if (buffer[i].element.offset().top - viewportOffset().top + buffer[i].element.outerHeight(true) >= (-1) * bufferPadding()) {
                 break;
               }
               overageHeight += buffer[i].element.outerHeight(true);
@@ -435,7 +437,7 @@ angular.module('ui.scroll', [])
         this.calculateProperties = function () {
           let i, item, itemHeight, itemTop, isNewRow, rowTop;
           let topHeight = 0;
-          for(i = 0; i < buffer.length; i++) {
+          for (i = 0; i < buffer.length; i++) {
             item = buffer[i];
             itemTop = item.element.offset().top;
             isNewRow = rowTop !== itemTop;

test/BasicTestsSpec.js

@@ -110,54 +110,61 @@ describe('uiScroll', function () {
     });
 
     describe('datasource with 20 elements and buffer size 3 - constrained viewport', function () {
-        var scrollSettings = { datasource: 'myMultipageDatasource', itemHeight: 40, bufferSize: 3 };
+        var scrollSettings = { datasource: 'myMultipageDatasource', viewportHeight: 200, itemHeight: 40, bufferSize: 3 };
 
-        it('should create 6 divs with data (+ 2 padding divs)', function () {
+        it('should create 9 divs with data (+ 2 padding divs)', function () {
             runTest(scrollSettings,
-                function (viewport) {
-                    expect(viewport.children().length).toBe(8);
-                    expect(viewport.scrollTop()).toBe(0);
-                    expect(viewport.children().css('height')).toBe('0px');
-                    expect(angular.element(viewport.children()[7]).css('height')).toBe('0px');
-
-                    for (var i = 1; i < 7; i++) {
-                        var row = viewport.children()[i];
-                        expect(row.tagName.toLowerCase()).toBe('div');
-                        expect(row.innerHTML).toBe(i + ': item' + i);
-                    }
+              function (viewport) {
+                var itemsLoaded = 9;
+                var itemsWithPaddings = itemsLoaded + 2;
+                expect(viewport.children().length).toBe(itemsWithPaddings);
+                expect(viewport.scrollTop()).toBe(0);
+                expect(viewport.children().css('height')).toBe('0px');
+                expect(angular.element(viewport.children()[itemsWithPaddings - 1]).css('height')).toBe('0px');
+
+                for (var i = 1; i < itemsLoaded; i++) {
+                  var row = viewport.children()[i];
+                  expect(row.tagName.toLowerCase()).toBe('div');
+                  expect(row.innerHTML).toBe(i + ': item' + i);
                 }
+              }
             );
         });
 
-        it('should call get on the datasource 3 times ', function () {
-            var spy;
-            inject(function (myMultipageDatasource) {
-                spy = spyOn(myMultipageDatasource, 'get').and.callThrough();
-            });
+        it('should call get on the datasource 4 times ', function () {
+          var spy;
+          inject(function (myMultipageDatasource) {
+              spy = spyOn(myMultipageDatasource, 'get').and.callThrough();
+          });
             runTest(scrollSettings,
                 function () {
-                    expect(spy.calls.all().length).toBe(3);
+                    // There are 9 loaded items, so there were 3 data calls
+                    // Additional call was for top items resulted with 0 items.
+                    expect(spy.calls.all().length).toBe(4);
 
                     expect(spy.calls.all()[0].args[0]).toBe(1);
                     expect(spy.calls.all()[1].args[0]).toBe(4);
-                    expect(spy.calls.all()[2].args[0]).toBe(-2);
+                    expect(spy.calls.all()[2].args[0]).toBe(7);
+                    expect(spy.calls.all()[3].args[0]).toBe(-2);
                 }
             );
         });
 
-        it('should create 3 more divs (9 divs total) with data (+ 2 padding divs)', function () {
+        it('should create 3 more divs (12 divs total) with data (+ 2 padding divs)', function () {
+            var itemsLoaded = 12;
+            var itemsWithPaddings = itemsLoaded + 2;
             runTest(scrollSettings,
                 function (viewport) {
                     viewport.scrollTop(100);
                     viewport.trigger('scroll');
                     inject(function ($timeout) {
                         $timeout.flush();
-                        expect(viewport.children().length).toBe(11);
-                        expect(viewport.scrollTop()).toBe(40);
+                        expect(viewport.children().length).toBe(itemsWithPaddings);
+                        expect(viewport.scrollTop()).toBe(100);
                         expect(viewport.children().css('height')).toBe('0px');
-                        expect(angular.element(viewport.children()[10]).css('height')).toBe('0px');
+                        expect(angular.element(viewport.children()[itemsWithPaddings-1]).css('height')).toBe('0px');
 
-                        for (var i = 1; i < 10; i++) {
+                        for (var i = 1; i < itemsLoaded; i++) {
                             var row = viewport.children()[i];
                             expect(row.tagName.toLowerCase()).toBe('div');
                             expect(row.innerHTML).toBe(i + ': item' + i);
@@ -167,7 +174,7 @@ describe('uiScroll', function () {
             );
         });
 
-        it('should call get on the datasource 1 extra time (4 total) ', function () {
+        it('should call get on the datasource 1 extra time (5 total) ', function () {
             var spy;
             inject(function (myMultipageDatasource) {
                 spy = spyOn(myMultipageDatasource, 'get').and.callThrough();
@@ -178,19 +185,23 @@ describe('uiScroll', function () {
                     viewport.trigger('scroll');
                     $timeout.flush();
 
-                    expect(spy.calls.all().length).toBe(4);
+                    expect(spy.calls.all().length).toBe(5);
 
                     expect(spy.calls.all()[0].args[0]).toBe(1);
                     expect(spy.calls.all()[1].args[0]).toBe(4);
-                    expect(spy.calls.all()[2].args[0]).toBe(-2);
-                    expect(spy.calls.all()[3].args[0]).toBe(7);
+                    expect(spy.calls.all()[2].args[0]).toBe(7);
+                    expect(spy.calls.all()[3].args[0]).toBe(-2);
+                    expect(spy.calls.all()[4].args[0]).toBe(10);
                 }
             );
         });
 
-        it('should clip 3 divs from the top and add 3 more divs to the bottom (9 divs total) (+ 2 padding divs)', function () {
+        it('should clip 4 divs from the top and add 3 more divs to the bottom (11 divs total) (+ 2 padding divs)', function () {
             runTest(scrollSettings,
                 function (viewport, scope, $timeout) {
+                    var itemsLoaded = 11;
+                    var itemsWithPaddings = itemsLoaded + 2;
+                    var clippedDivs = 4;
                     viewport.scrollTop(100);
                     viewport.trigger('scroll');
                     $timeout.flush();
@@ -199,27 +210,28 @@ describe('uiScroll', function () {
                     viewport.trigger('scroll');
                     $timeout.flush();
 
-                    expect(viewport.children().length).toBe(11);
-                    expect(viewport.scrollTop()).toBe(160);
-                    expect(viewport.children().css('height')).toBe('120px');
-                    expect(angular.element(viewport.children()[10]).css('height')).toBe('0px');
+                    expect(viewport.children().length).toBe(itemsWithPaddings);
+                    expect(viewport.scrollTop()).toBe(280);
+                    expect(viewport.children().css('height')).toBe('160px');
+                    expect(angular.element(viewport.children()[itemsWithPaddings-1]).css('height')).toBe('0px');
 
-                    for (var i = 1; i < 10; i++) {
+                    for (var i = 1; i <= itemsLoaded; i++) {
                         var row = viewport.children()[i];
                         expect(row.tagName.toLowerCase()).toBe('div');
-                        expect(row.innerHTML).toBe((i + 3) + ': item' + (i + 3));
+                        expect(row.innerHTML).toBe((i + clippedDivs) + ': item' + (i + clippedDivs));
                     }
                 }
             );
         });
 
-        it('should call get on the datasource 1 more time (4 total) ', function () {
+        it('should call get on the datasource 1 more time (6 total) ', function () {
             var spy;
             inject(function (myMultipageDatasource) {
                 spy = spyOn(myMultipageDatasource, 'get').and.callThrough();
             });
             runTest(scrollSettings,
                 function (viewport, scope, $timeout) {
+                    var calls = 6;
 
                     viewport.scrollTop(100);
                     viewport.trigger('scroll');
@@ -229,20 +241,23 @@ describe('uiScroll', function () {
                     viewport.trigger('scroll');
                     $timeout.flush();
 
-                    expect(spy.calls.all().length).toBe(5);
+                    expect(spy.calls.all().length).toBe(calls);
                     expect(spy.calls.all()[0].args[0]).toBe(1);
                     expect(spy.calls.all()[1].args[0]).toBe(4);
-                    expect(spy.calls.all()[2].args[0]).toBe(-2);
-                    expect(spy.calls.all()[3].args[0]).toBe(7);
+                    expect(spy.calls.all()[2].args[0]).toBe(7);
+                    expect(spy.calls.all()[3].args[0]).toBe(-2);
                     expect(spy.calls.all()[4].args[0]).toBe(10);
+                    expect(spy.calls.all()[5].args[0]).toBe(13);
                 }
             );
         });
 
-        it('should re-add 3 divs at the top and clip 3 divs from the bottom (9 divs total) (+ 2 padding divs)', function () {
+        it('should re-add 3 divs at the top and clip 2 divs from the bottom (9 divs total) (+ 2 padding divs)', function () {
             runTest(scrollSettings,
                 function (viewport, scope, $timeout) {
                     var flush = $timeout.flush;
+                    var itemsLoaded = 8;
+                    var itemsWithPaddings = itemsLoaded + 2;
 
                     viewport.scrollTop(100);
                     viewport.trigger('scroll');
@@ -256,12 +271,12 @@ describe('uiScroll', function () {
                     viewport.trigger('scroll');
                     flush();
 
-                    expect(viewport.children().length).toBe(8);
+                    expect(viewport.children().length).toBe(itemsWithPaddings);
                     expect(viewport.scrollTop()).toBe(0);
                     expect(viewport.children().css('height')).toBe('0px');
-                    expect(angular.element(viewport.children()[7]).css('height')).toBe('240px');
+                    expect(angular.element(viewport.children()[itemsWithPaddings-1]).css('height')).toBe('280px');
 
-                    for (var i = 1; i < 7; i++) {
+                    for (var i = 1; i <= itemsLoaded; i++) {
                         var row = viewport.children()[i];
                         expect(row.tagName.toLowerCase()).toBe('div');
                         expect(row.innerHTML).toBe((i) + ': item' + (i));
@@ -270,7 +285,7 @@ describe('uiScroll', function () {
             );
         });
 
-        it('should call get on the datasource 1 more time (4 total) ', function () {
+        it('should call get on the datasource 1 more time (8 total) ', function () {
             var spy;
             inject(function (myMultipageDatasource) {
                 spy = spyOn(myMultipageDatasource, 'get').and.callThrough();
@@ -279,6 +294,7 @@ describe('uiScroll', function () {
             runTest(scrollSettings,
                 function (viewport, scope, $timeout) {
                     var flush = $timeout.flush;
+                    var totalCallsNumber = 8;
 
                     viewport.scrollTop(100);
                     viewport.trigger('scroll');
@@ -292,15 +308,15 @@ describe('uiScroll', function () {
                     viewport.trigger('scroll');
                     flush();
 
-                    expect(spy.calls.all().length).toBe(7);
+                    expect(spy.calls.all().length).toBe(totalCallsNumber);
                     expect(spy.calls.all()[0].args[0]).toBe(1);
                     expect(spy.calls.all()[1].args[0]).toBe(4);
-                    expect(spy.calls.all()[2].args[0]).toBe(-2);
-                    expect(spy.calls.all()[3].args[0]).toBe(7);
+                    expect(spy.calls.all()[2].args[0]).toBe(7);
+                    expect(spy.calls.all()[3].args[0]).toBe(-2);
                     expect(spy.calls.all()[4].args[0]).toBe(10);
-                    expect(spy.calls.all()[5].args[0]).toBe(1);
-                    expect(spy.calls.all()[6].args[0]).toBe(-2);
-
+                    expect(spy.calls.all()[5].args[0]).toBe(13);
+                    expect(spy.calls.all()[6].args[0]).toBe(2);
+                    expect(spy.calls.all()[7].args[0]).toBe(-1);
                 }
             );
         });
@@ -392,7 +408,7 @@ describe('uiScroll', function () {
                     expect(spy.calls.all()[0].args[0]).toBe(1);
                     expect(spy.calls.all()[1].args[0]).toBe(4); //last full
                     expect(spy.calls.all()[2].args[0]).toBe(-2);
-                    expect(spy.calls.all()[3].args[0]).toBe(5); //empty
+                    expect(spy.calls.all()[3].args[0]).toBe(6); //empty
 
                 }
             );
@@ -482,25 +498,25 @@ describe('uiScroll', function () {
                     viewport.trigger('scroll');
 
                     wheelEventElement.dispatchEvent(getNewWheelEvent()); //now we are at the top but preventDefault is occurred because of bof will be reached only after next scroll trigger
-                    expect(documentScrollBubblingCount).toBe(1); //here! the only one prevented wheel-event
+                    expect(documentScrollBubblingCount).toBe(2); //here! the only one prevented wheel-event
 
                     flush();
 
                     wheelEventElement.dispatchEvent(getNewWheelEvent()); //preventDefault will not occurred but document will not scroll because of viewport will be scrolled
-                    expect(documentScrollBubblingCount).toBe(2);
+                    expect(documentScrollBubblingCount).toBe(3);
 
                     viewport.scrollTop(0);
                     viewport.trigger('scroll'); //bof will be reached right after that
 
                     flush();
 
                     wheelEventElement.dispatchEvent(getNewWheelEvent()); //preventDefault will not occurred because of we are at the top and bof is reached
-                    expect(documentScrollBubblingCount).toBe(3);
+                    expect(documentScrollBubblingCount).toBe(4);
 
                     expect(flush).toThrow(); //there is no new data, bof is reached
 
                     wheelEventElement.dispatchEvent(getNewWheelEvent()); //preventDefault will not occurred because of we are at the top and bof is reached
-                    expect(documentScrollBubblingCount).toBe(4);
+                    expect(documentScrollBubblingCount).toBe(5);
 
                 }, {
                     cleanupTest: function () {
@@ -678,7 +694,7 @@ describe('uiScroll', function () {
                     }
 
                     // unstable delta passing
-                    viewport.scrollTop(viewport.scrollTop() - 5);
+                    viewport.scrollTop(viewport.scrollTop());
 
                     // scroll up + expectation
                     for(i = 0; i < limit; i++) {

test/VisibilitySwitchingSpec.js

@@ -16,9 +16,9 @@ describe('uiScroll', function () {
     describe('viewport visibility changing', function () {
         var scrollSettings = { datasource: 'myMultipageDatasource', viewportHeight: 200, itemHeight: 40, bufferSize: 3, adapter: 'adapter' };
         var onePackItemsCount = 3 + 2;
-        var twoPacksItemsCount = 3 * 2 + 2;
+        var twoPacksItemsCount = 3 * 3 + 2;
 
-        it('should create 6 divs with data (+ 2 padding divs)', function () {
+        it('should create 9 divs with data (+ 2 padding divs)', function () {
             runTest(scrollSettings,
                 function (viewport) {
                     expect(viewport.children().length).toBe(twoPacksItemsCount);