Jhernandez has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/276451

Change subject: Fix mw.viewport.isElementInViewport with scrolled page
......................................................................

Fix mw.viewport.isElementInViewport with scrolled page

With a scrolled page the function would return false since it assumed that
getBoundingClientRect returns the offset when in reality it returns the offset
relative to the window viewport.

This implementation keeps the ability to specify custom viewport rects. If we
wanted to just rely on the window viewport the implementation could be changed
with getBoundingClientRect to be much simpler.

Bug: T129466
Change-Id: I56142fbd1177fe47171a1874f73b7a8359c282ee
---
M resources/src/mediawiki/mediawiki.viewport.js
M tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
2 files changed, 39 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/51/276451/1

diff --git a/resources/src/mediawiki/mediawiki.viewport.js 
b/resources/src/mediawiki/mediawiki.viewport.js
index aa9dd05..a64b594 100644
--- a/resources/src/mediawiki/mediawiki.viewport.js
+++ b/resources/src/mediawiki/mediawiki.viewport.js
@@ -50,14 +50,25 @@
                 * @return {boolean}
                 */
                isElementInViewport: function ( el, rectangle ) {
-                       var elRect = el.getBoundingClientRect(),
+                       var $el = $(el),
+                               offset = $el.offset(),
+                               rect = {
+                                       height: $el.height(),
+                                       width: $el.width(),
+                                       top: offset.top,
+                                       left: offset.left
+                               },
                                viewport = rectangle || 
this.makeViewportFromWindow();
 
                        return (
-                               ( viewport.bottom >= elRect.top ) &&
-                               ( viewport.right >= elRect.left ) &&
-                               ( viewport.top <= elRect.top + elRect.height ) 
&&
-                               ( viewport.left <= elRect.left + elRect.width )
+                               // Top border must be above viewport's bottom
+                               ( viewport.bottom >= rect.top ) &&
+                               // Left border must be before viewport's right 
border
+                               ( viewport.right >= rect.left ) &&
+                               // Bottom border must be below viewport's top
+                               ( viewport.top <= rect.top + rect.height ) &&
+                               // Right border must be after viewport's left 
border
+                               ( viewport.left <= rect.left + rect.width )
                        );
                },
 
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js 
b/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
index 61391d8..f79e5fd 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.viewport.test.js
@@ -60,6 +60,29 @@
                        'It should default to the window object if no viewport 
is given' );
        } );
 
+       QUnit.test( 'isElementInViewport with scrolled page', 1, function ( 
assert ) {
+               var viewport = {
+                               top: 2000,
+                               left: 0,
+                               right: 1000,
+                               bottom: 2500
+                       },
+                       el = $( '<div />' )
+                               .appendTo( '#qunit-fixture' )
+                               .width( 20 )
+                               .height( 20 )
+                               .offset( {
+                                       top: 2300,
+                                       left: 20
+                               } )
+                               .get( 0 );
+               window.scrollTo(viewport.left, viewport.top);
+               assert.ok( mw.viewport.isElementInViewport( el, viewport ),
+                       'It should return true when the element is fully 
enclosed in the ' +
+                       'viewport even when the page is scrolled down' );
+               window.scrollTo(0, 0);
+       } );
+
        QUnit.test( 'isElementCloseToViewport', 3, function ( assert ) {
                var
                        viewport = {

-- 
To view, visit https://gerrit.wikimedia.org/r/276451
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I56142fbd1177fe47171a1874f73b7a8359c282ee
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Jhernandez <jhernan...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to