Merge pull request #15 from npafundi/master

Fix erroneous horizontal viewport check

lib/isInViewport.js

@@ -93,7 +93,7 @@
       top    = top - viewportRect.top;
       bottom = bottom - viewportRect.top;
       left   = left - viewportRect.left;
-      right  = left + $viewportWidth;
+      right  = right - viewportRect.left;
 
       //get the scrollbar width from cache or calculate it
       isInViewport.scrollBarWidth = isInViewport.scrollBarWidth || getScrollbarWidth($viewport);
@@ -113,10 +113,12 @@
     //the element is NOT in viewport iff it is completely out of
     //viewport laterally or if it is completely out of the tolerance
     //region. Therefore, if it is partially in view then it is considered
-    //to be in the viewport and hence true is returned
+    //to be in the viewport and hence true is returned. Because we have adjusted
+    //the left/right positions relative to the viewport, we should check the
+    //element's right against the viewport's 0 (left side), and the element's
+    //left against the viewport's width to see if it is outside of the viewport.
 
-    //if the element is laterally outside the viewport
-    if (Math.abs(left) >= $viewportWidth)
+    if (right <= 0 || left >= $viewportWidth)
       return isVisibleFlag;
 
     //if the element is bound to some tolerance

tests/horizontallyScrollingViewportTests.js

@@ -0,0 +1,47 @@
+function runHorizontallyScrollingViewport() {
+  var visible = '';
+  $('li:in-viewport(0, #blocks)').each(function() {
+    visible += $(this).text() + ' ';
+  });
+  return visible.trim();
+}
+
+describe('isInViewport', function() {
+  describe('viewport is a horizonatlly scrollable list (ul#blocks)', function() {
+    var buidList = function() {
+      $('body').append('<ul id="blocks"></ul>');
+
+      // Add 10 list items to the list
+      for (var i=1; i<=10; i++)
+        $('#blocks').append('<li>' + i + '</li>');
+    };
+    var removeList = function() {
+      $('#blocks').remove();
+    };
+    var scrollLeft = function(px) {
+      px = px || $('#blocks')[0].scrollWidth;
+      $('#blocks').scrollLeft(px);
+    };
+
+    before(buidList);
+    after(removeList);
+
+    describe('when the first four items are visible', function() {
+      it('should return the string "1 2 3 4" as a list of currently visible items', function() {
+        runHorizontallyScrollingViewport().should.be.exactly('1 2 3 4');
+      });
+    });
+    describe('when we scroll the list left by 525px', function() {
+      it('should return the string "4 5 6 7 8" as a list of currently visible items', function() {
+        scrollLeft(525);
+        runHorizontallyScrollingViewport().should.be.exactly('4 5 6 7 8');
+      });
+    });
+    describe('when we scroll the list to the end', function() {
+      it('should return the string "7 8 9 10" as a list of currently visible items', function() {
+        scrollLeft();
+        runHorizontallyScrollingViewport().should.be.exactly('7 8 9 10');
+      });
+    });
+  });
+});

tests/tests.html

@@ -53,6 +53,26 @@
       padding: 19px 36px;
       color: #fff;
     }
+    ul#blocks {
+      padding: 0;
+      width: 600px;
+      height: 170px;
+      overflow-x: scroll;
+      white-space: nowrap;
+      font-size: 0;
+    }
+
+    ul#blocks li {
+      box-sizing: border-box;
+      display: inline-block;
+      background-color: #ff0000;
+      height: 150px;
+      width: 150px;
+      text-align: center;
+      font-size: 50px;
+      padding: 50px;
+      border: 1px solid #fff;
+    }
   </style>
 </head>
 
@@ -77,6 +97,7 @@
   <script type="text/javascript" src="./lib/mocha-blanket.js"></script>
   <script src="defaultViewportTests.js"></script>
   <script src="customViewportTests.js"></script>
+  <script src="horizontallyScrollingViewportTests.js"></script>
 
   <script>
     mocha.checkLeaks();