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

Change subject: Improved file-control positioning
......................................................................


Improved file-control positioning

Currently the UploadWizard sets 2 Intervals per file that are fired
every 500ms accessing the DOM. You can imagine that this can lead to
high CPU load at the client. But these intervals are even not cleared
after they are not required anymore and make it impossible to upload 50
files at once on old, slow machines. (In a test on a Intel Pentium IV,
1.7GHz, Firefox 11, Windows XP SP3, I was unable to upload more than 10
files without running into an unresponsive UI.)

This commit eliminates stupid "polling" and, instead listens to
resize-events. It also avoids caring about uploads that were removed by
the user or about file inputs that are filled and are just kept for
accessing the file contents. All this leads to less CPU load, a more
responsive UI and finally to less pollution of the environment and more
happy users.

A call to $.client.profile() was removed because the variable that
receives the return-value seems to be unused.

Bug: 37997
Change-Id: Icd8642c663869e56c2397b69931f9ade4bf5bf7c
---
M resources/mw.UploadWizard.js
M resources/mw.UploadWizardUploadInterface.js
2 files changed, 39 insertions(+), 15 deletions(-)

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



diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js
index 937d47a..9699a41 100644
--- a/resources/mw.UploadWizard.js
+++ b/resources/mw.UploadWizard.js
@@ -541,7 +541,7 @@
                _this.uploadToAdd = upload;
 
                // we explicitly move the file input to cover the upload button
-               upload.ui.moveFileInputToCover( '#mwe-upwiz-add-file' );
+               upload.ui.moveFileInputToCover( '#mwe-upwiz-add-file', 'resize' 
);
 
                // we bind to the ui div since unbind doesn't work for non-DOM 
objects
                $j( upload.ui.div ).bind( 'filenameAccepted', function(e) { 
_this.updateFileCounts();  e.stopPropagation(); } );
@@ -1020,7 +1020,7 @@
                        $j( '#mwe-upwiz-add-file' ).button( 'option', 
'disabled', false );
                        $j( '#mwe-upwiz-upload-ctrl-flickr' ).button( 'option', 
'disabled', false );
                        $j( _this.uploadToAdd.ui.div ).show();
-                       _this.uploadToAdd.ui.moveFileInputToCover( 
'#mwe-upwiz-add-file' );
+                       _this.uploadToAdd.ui.moveFileInputToCover( 
'#mwe-upwiz-add-file', 'resize' );
                } else {
                        $j( '#mwe-upwiz-add-file' ).button( 'option', 
'disabled', true );
                        $j( '#mwe-upwiz-upload-ctrl-flickr' ).button( 'option', 
'disabled', true );
diff --git a/resources/mw.UploadWizardUploadInterface.js 
b/resources/mw.UploadWizardUploadInterface.js
index 74b9bc6..0780d25 100644
--- a/resources/mw.UploadWizardUploadInterface.js
+++ b/resources/mw.UploadWizardUploadInterface.js
@@ -21,7 +21,6 @@
        _this.previewLoaded = false;
 
        _this.$fileInputCtrl = $j( '<input size="1" 
class="mwe-upwiz-file-input" name="file" type="file"/>' );
-       var profile = $.client.profile();
        if (mw.UploadWizard.config.enableFormData && 
mw.fileApi.isFormDataAvailable() &&
                mw.UploadWizard.config.enableMultiFileSelect && 
mw.UploadWizard.config.enableMultipleFiles ) {
                // Multiple uploads requires the FormData transport
@@ -49,7 +48,10 @@
        _this.$removeCtrl = $j.fn.removeCtrl(
                'mwe-upwiz-remove',
                'mwe-upwiz-remove-upload',
-               function() { _this.upload.remove(); }
+               function() {
+                       _this.upload.remove();
+                       _this.cancelPositionTracking();
+               }
        ).addClass( "mwe-upwiz-file-status-line-item" );
 
        _this.visibleFilenameDiv.find( '.mwe-upwiz-file-status-line' )
@@ -528,9 +530,11 @@
         * It is helpful to sometimes move them to cover certain elements on 
the page, and
         * even to pass events like hover
         * @param selector jquery-compatible selector, for a single element
+        * @param positionTracking string, optional, whether to do 
position-polling ('poll')
+        *     on the selected element or whether to listen to window-resize 
events ('resize')
         */
-       moveFileInputToCover: function( selector ) {
-               var _this = this;
+       moveFileInputToCover: function( selector, positionTracking ) {
+               var iv, to, onResize, $win, _this = this;
                var update = function() {
                        var $covered = $j( selector );
 
@@ -554,20 +558,39 @@
                        } );
                };
 
-               if (this.moveFileInputInterval) {
-                       window.clearInterval(this.moveFileInputInterval);
+
+               this.cancelPositionTracking();
+               if ( positionTracking === 'poll' ) {
+                       iv = window.setInterval( update, 500 );
+                       this.stopTracking = function () {
+                               window.clearInterval( iv );
+                       }
+               } else if ( positionTracking === 'resize' ) {
+                       $win = $( window );
+                       onResize = function () {
+                               // ensure resizing works smoothly
+                               if ( to ) {
+                                       window.clearTimeout( to );
+                               }
+                               to = window.setTimeout( update, 200 );
+                       };
+                       $win.resize( onResize );
+                       this.stopTracking = function () {
+                               $win.off( 'resize', onResize );
+                       }
                }
-               this.moveFileInputInterval = window.setInterval(function() {
-                       update();
-               }, 500);
                update();
        },
 
-       hideFileInput: function() {
-               if (this.moveFileInputInterval) {
-                       window.clearInterval(this.moveFileInputInterval);
+       cancelPositionTracking: function () {
+               if ( $j.isFunction( this.stopTracking ) ) {
+                       this.stopTracking();
+                       this.stopTracking = null;
                }
-               this.moveFileInputInterval = null;
+       },
+
+       hideFileInput: function () {
+               this.cancelPositionTracking();
                // Should we actually hide it?
        },
 
@@ -604,6 +627,7 @@
                        // cover the div with the file input.
                        // we use the visible-file div because it has the same 
offsetParent as the file input
                        // the second argument offsets the fileinput to the 
right so there's room for the close icon to get mouse events
+                       // TODO Why do we care for this element at all and do 
not just hide it, once we have a valid file in it?
                        _this.moveFileInputToCover(
                                $div.find( 
'.mwe-upwiz-visible-file-filename-text' )
                        );

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Icd8642c663869e56c2397b69931f9ade4bf5bf7c
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/UploadWizard
Gerrit-Branch: master
Gerrit-Owner: Rillke <[email protected]>
Gerrit-Reviewer: Alex Monk <[email protected]>
Gerrit-Reviewer: Drecodeam <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: MarkTraceur <[email protected]>
Gerrit-Reviewer: Nischayn22 <[email protected]>
Gerrit-Reviewer: Rillke <[email protected]>
Gerrit-Reviewer: jenkins-bot

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

Reply via email to