Skip to content

Commit

Permalink
Improvements to horizontal viewport check
Browse files Browse the repository at this point in the history
* Right was calculated incorrectly (but was never used), which has been fixed.
* The horizontal viewport check took the absolute value of the element's left,
  which didn't accurately check the viewport left/element right bound. This has
  been modified to check the element's left against the viewport's right
  (whether the element is within the right side of the viewport) and the
  element's right against the viewport's left (whether the element is within the
  left side of the viewport).
* Adding a new test suite to test viewports of horizontally scrolling content.
  This verifies that when content is scrolled horizontally (e.g. a list of
  items), the isInViewport calculations are correct after scrolling.
  • Loading branch information
npafundi committed Apr 14, 2015
1 parent 554462d commit c081fef
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 4 deletions.
10 changes: 6 additions & 4 deletions lib/isInViewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
47 changes: 47 additions & 0 deletions tests/horizontallyScrollingViewportTests.js
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
});
21 changes: 21 additions & 0 deletions tests/tests.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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();
Expand Down

0 comments on commit c081fef

Please sign in to comment.