jenkins-bot has submitted this change and it was merged.

Change subject: Combine scroll handlers into the global one
......................................................................


Combine scroll handlers into the global one

In order to keep the number of handlers attached on scroll to the window, let's
use the events in mw.mobileFrontend so that there is only one handler attached
to the window.

This moves the handlers on back to top and the infinite scrolling component to
use the global event handler (using 'scroll' when debounced or
'scroll:throttled' when needed)

Also fixes the infinite scrolling unit tests to rely on the global event
emitter instead of in syncronous window bound scroll events.

Bug: T124870
Change-Id: Ic21dc04389cab065c6dd52968689f72c045939ab
---
M resources/mobile.backtotop/backtotop.js
M resources/mobile.infiniteScroll/InfiniteScroll.js
M tests/qunit/mobile.infiniteScroll/test_InfiniteScroll.js
3 files changed, 10 insertions(+), 10 deletions(-)

Approvals:
  Jdlrobson: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/resources/mobile.backtotop/backtotop.js 
b/resources/mobile.backtotop/backtotop.js
index 43f4294..3c2efff 100644
--- a/resources/mobile.backtotop/backtotop.js
+++ b/resources/mobile.backtotop/backtotop.js
@@ -5,11 +5,11 @@
        // initialize the back to top element
        backtotop.appendTo( 'body' );
 
-       $( window ).on( 'scroll', $.debounce( 100, function () {
+       M.on( 'scroll', function () {
                if ( $( window ).height() - $( window ).scrollTop() <= 0 ) {
                        backtotop.show();
                } else {
                        backtotop.hide();
                }
-       } ) );
+       } );
 }( mw.mobileFrontend, jQuery ) );
diff --git a/resources/mobile.infiniteScroll/InfiniteScroll.js 
b/resources/mobile.infiniteScroll/InfiniteScroll.js
index 5105cb6..6787c9e 100644
--- a/resources/mobile.infiniteScroll/InfiniteScroll.js
+++ b/resources/mobile.infiniteScroll/InfiniteScroll.js
@@ -69,10 +69,7 @@
                 * @private
                 */
                _bindScroll: function () {
-                       // FIXME: Consider using setInterval instead or some 
sort of
-                       // dethrottling/debouncing to avoid performance 
degradation
-                       // e.g. 
http://benalman.com/projects/jquery-throttle-debounce-plugin/
-                       $( window ).on( 'scroll', $.proxy( this, '_onScroll' ) 
);
+                       M.on( 'scroll:throttled', $.proxy( this, '_onScroll' ) 
);
                },
                /**
                 * Scroll handler. Triggers load event when near the end of the 
container.
diff --git a/tests/qunit/mobile.infiniteScroll/test_InfiniteScroll.js 
b/tests/qunit/mobile.infiniteScroll/test_InfiniteScroll.js
index 2ea393e..d8fb044 100644
--- a/tests/qunit/mobile.infiniteScroll/test_InfiniteScroll.js
+++ b/tests/qunit/mobile.infiniteScroll/test_InfiniteScroll.js
@@ -3,8 +3,8 @@
 
        QUnit.module( 'MobileFrontend InfiniteScroll', {
                teardown: function () {
-                       // Remove all scroll events after each test
-                       $( window ).off( 'scroll' );
+                       // Leave window at the top
+                       window.scrollTo( 0, 0 );
                }
        } );
 
@@ -18,8 +18,9 @@
                assert.strictEqual( is2.threshold, 100,
                        'Without a threshold we get a default' );
 
-               // Scrolling has been bound to the window
-               $( window ).trigger( 'scroll' );
+               // Scrolling has been bound to the global mobileFrontend handler
+               M.emit( 'scroll:throttled' );
+
                assert.ok( scrolledSpy.calledTwice,
                        'Scrolling has been bound and is handler is called on 
scroll' );
        } );
@@ -44,6 +45,7 @@
 
                // Scroll to the bottom of the body
                window.scrollTo( 0, $( 'body' ).offset().top + $( 'body' 
).outerHeight() );
+               M.emit( 'scroll:throttled' );
        } );
 
        QUnit.test( 'doesn\'t emit when disabled', 1, function ( assert ) {
@@ -54,6 +56,7 @@
                // Scroll to top and bottom of the body
                window.scrollTo( 0, 0 );
                window.scrollTo( 0, $( 'body' ).offset().top + $( 'body' 
).outerHeight() );
+               M.emit( 'scroll:throttled' );
                assert.strictEqual( emitSpy.called, false, 'emit should not be 
called' );
        } );
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic21dc04389cab065c6dd52968689f72c045939ab
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jhernandez <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to