Bartosz Dziewoński has uploaded a new change for review. https://gerrit.wikimedia.org/r/270295
Change subject: [WIP] Move "Add files" functionality from UWUploadInterface to UploadWizard ...................................................................... [WIP] Move "Add files" functionality from UWUploadInterface to UploadWizard This simplifies things greatly. * All rejoice, for UploadWizardUploadInterface#moveFileInputToCover is now gone. No more slow positioning on every new upload or "polling" for resizes. An <input type=file /> is still overlaid, but using a pure CSS solution, possible now that we only have a single one. * The control flow is also simpler. There is less jumping all over the place using events now. * Retained the code paths that try to handle browsers without File API support, but I haven't tested them since I don't know any browsers that we still support that lack it. (We dropped IE 8 recently.) Perhaps some thinning of the herds is in order. TODO A few doc comments need updating, I'll review the diff later. Bug: T126712 Change-Id: I9638d958ed5ebbbc4346ed03a1e9bbbf46147749 --- M resources/mw.FlickrChecker.js M resources/mw.UploadWizard.js M resources/mw.UploadWizardUpload.js M resources/mw.UploadWizardUploadInterface.js M resources/ui/steps/uw.ui.Upload.js M resources/uploadWizard.css 6 files changed, 100 insertions(+), 237 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/UploadWizard refs/changes/95/270295/1 diff --git a/resources/mw.FlickrChecker.js b/resources/mw.FlickrChecker.js index 5abcbb7..16319b4 100644 --- a/resources/mw.FlickrChecker.js +++ b/resources/mw.FlickrChecker.js @@ -670,7 +670,7 @@ method: 'flickr.photos.getSizes', photo_id: photoId } ).done( function ( data ) { - var nameParts, newUpload; + var nameParts; if ( typeof data.sizes !== 'undefined' && @@ -691,10 +691,8 @@ upload.name = nameParts.join( '.' ) + '.' + upload.originalFormat; } upload.url = largestSize.source; - // Need to call the newUpload here, otherwise some code would have to be written to detect the completion of the API call. - newUpload = checker.wizard.newUpload(); - - newUpload.fill( upload ); + // Need to call the addUpload here, otherwise some code would have to be written to detect the completion of the API call. + checker.wizard.addUpload( upload ); } else { mw.errorDialog( mw.message( 'mwe-upwiz-error-no-image-retrieved', 'Flickr' ).escaped() ); checker.wizard.flickrInterfaceReset(); diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js index 64376a2..7f79413 100644 --- a/resources/mw.UploadWizard.js +++ b/resources/mw.UploadWizard.js @@ -70,6 +70,56 @@ this.showDeed = false; this.removeMatchingUploads( function () { return true; } ); this.hasLoadedBefore = true; + this.$fileInputCtrl = this.setupFileInputCtrl(); + }, + + setupFileInputCtrl: function () { + var + $fileInputCtrl, + wizard = this; + + $fileInputCtrl = $( '<input size="1" class="mwe-upwiz-file-input" name="file" type="file"/>' ); + if ( mw.UploadWizard.config.enableFormData && mw.fileApi.isFormDataAvailable() && + mw.UploadWizard.config.enableMultiFileSelect && mw.UploadWizard.config.enableMultipleFiles ) { + // Multiple uploads requires the FormData transport + $fileInputCtrl.attr( 'multiple', '1' ); + } + + // #mwe-upwiz-add-file is a ButtonWidget constructed somewhere else, so this is hacky. + // But it's less bad than how this was done before. + $( '#mwe-upwiz-add-file' ).append( $fileInputCtrl ); + + $fileInputCtrl.on( 'change', function () { + var + totalSize, + files = mw.fileApi.isAvailable() && $fileInputCtrl[ 0 ].files, + totalFiles = ( files ? files.length : 1 ) + wizard.uploads.length, + tooManyFiles = totalFiles > wizard.config.maxUploads; + + if ( tooManyFiles ) { + wizard.steps.file.showTooManyFilesWarning( totalFiles ); + return; + } + + if ( mw.fileApi.isAvailable() ) { + totalSize = 0; + $.each( files, function ( i, file ) { + totalSize += file.size; + } ); + + // Now that first file has been prepared, process remaining files + // in case of a multi-file upload. + $.each( files, function ( i, file ) { + wizard.addUpload( file, totalSize > 10000000 ); + } ); + } else { + wizard.addUpload( $fileInputCtrl.off( 'change' ).detach(), false ); + // The new upload owns this $fileInputCtrl now. Create a new one. + wizard.$fileInputCtrl = wizard.setupFileInputCtrl(); + } + + uw.eventFlowLogger.logUploadEvent( 'uploads-added', { quantity: files.length } ); + } ); }, /** @@ -246,8 +296,6 @@ */ resetFileStepUploads: function () { if ( this.uploads.length === 0 ) { - // add one upload field to start (this is the big one that asks you to upload something) - this.newUpload(); // hide flickr uploading button if user doesn't have permissions if ( !mw.UploadWizard.config.UploadFromUrl || mw.UploadWizard.config.flickrApiKey === '' ) { $( '#mwe-upwiz-upload-ctrl-flickr-container, #mwe-upwiz-flickr-select-list-container' ).hide(); @@ -263,7 +311,7 @@ * * @return {UploadWizardUpload|false} the new upload */ - newUpload: function () { + addUpload: function ( fileLike, disablePreview ) { var upload, wizard = this; @@ -272,44 +320,8 @@ } upload = new mw.UploadWizardUpload( this, '#mwe-upwiz-filelist' ) - .on( 'file-changed', function ( upload, files ) { - var totalFiles = files.length + wizard.uploads.length, - tooManyFiles = totalFiles > wizard.config.maxUploads; - - if ( tooManyFiles ) { - wizard.steps.file.showTooManyFilesWarning( totalFiles ); - return; - } - - upload.checkFile( - upload.ui.getFilename(), - files, - function () { upload.fileChangedOk(); } - ); - - uw.eventFlowLogger.logUploadEvent( 'uploads-added', { quantity: files.length } ); - } ) - .on( 'filled', function () { wizard.setUploadFilled( upload ); - } ) - - .on( 'extra-files', function ( files, toobig ) { - $.each( files, function ( i, file ) { - // NOTE: By running newUpload we will end up calling checkfile() again. - var newUpload = wizard.newUpload(); - - if ( toobig ) { - newUpload.disablePreview(); - } - - newUpload.fill( file ); - } ); - - // Add a new upload to cover the button - wizard.newUpload(); - - wizard.steps.file.updateFileCounts( wizard.uploads ); } ) .on( 'filename-accepted', function () { @@ -320,13 +332,24 @@ uw.eventFlowLogger.logError( 'file', { code: code, message: message } ); } ); - // we explicitly move the file input to cover the upload button - upload.ui.moveFileInputToCover( '#mwe-upwiz-add-file', 'poll' ); - upload.connect( this, { 'remove-upload': [ 'removeUpload', upload ] } ); + // Local previews are slow due to the data URI insertion into the DOM; for batches we + // don't generate them if the size of the batch exceeds 10 MB + if ( disablePreview ) { + upload.disablePreview(); + } + + upload.fill( fileLike ); + + upload.checkFile( + upload.ui.getFilename(), + mw.fileApi.isAvailable() ? fileLike : null, + function () { upload.fileChangedOk(); } + ); + return upload; }, diff --git a/resources/mw.UploadWizardUpload.js b/resources/mw.UploadWizardUpload.js index 9317d94..7687f94 100644 --- a/resources/mw.UploadWizardUpload.js +++ b/resources/mw.UploadWizardUpload.js @@ -82,16 +82,16 @@ * @param {File} providedFile */ mw.UploadWizardUpload.prototype.fill = function ( providedFile ) { - // check to see if the File is being uplaoded from a 3rd party URL. - if ( providedFile ) { + if ( providedFile && !( providedFile instanceof jQuery ) ) { this.providedFile = providedFile; + // check to see if the File is being uploaded from a 3rd party URL. if ( providedFile.fromURL ) { this.fromURL = true; } - - this.ui.fill( providedFile ); } + + this.ui.fill( providedFile ); }; mw.UploadWizardUpload.prototype.acceptDeed = function () { @@ -377,11 +377,11 @@ * and delete it from the third parameter of the error callback. The end. * * @param {string} filename The filename - * @param {Array} files Array of files, usually one; can be more for multi-file select. + * @param {Object} file File, if available * @param {Function} fileNameOk Callback to use when ok, and upload object is ready */ - mw.UploadWizardUpload.prototype.checkFile = function ( filename, files, fileNameOk ) { - var totalSize, duplicate, extension, toobig, + mw.UploadWizardUpload.prototype.checkFile = function ( filename, file, fileNameOk ) { + var duplicate, extension, actualMaxSize, binReader, upload = this, @@ -399,21 +399,6 @@ // Eternal optimism this.hasError = false; - - if ( files.length > 1 ) { - totalSize = 0; - $.each( files, function ( i, file ) { - totalSize += file.size; - } ); - - toobig = totalSize > 10000000; - - // Local previews are slow due to the data URI insertion into the DOM; for batches we - // don't generate them if the size of the batch exceeds 10 MB - if ( toobig ) { - this.disablePreview(); - } - } // check to see if the file has already been selected for upload. duplicate = false; @@ -455,12 +440,7 @@ // we want to still trudge forward. // if the JavaScript FileReader is available, extract more info via fileAPI if ( mw.fileApi.isAvailable() ) { - - // An UploadWizardUpload object already exists (us) when we add a file. - // So, when multiple files are provided (via select multiple), add the first file to this UploadWizardUpload - // and create new UploadWizardUpload objects and corresponding interfaces for the rest. - - this.file = files[ 0 ]; + this.file = file; // If chunked uploading is enabled, we can transfer any file that MediaWiki // will accept. Otherwise we're bound by PHP's limits. @@ -539,9 +519,6 @@ } } - // Now that first file has been prepared, process remaining files - // in case of a multi-file upload. - this.emit( 'extra-files', files.slice( 1 ), toobig ); } else { this.filename = filename; if ( this.hasError === false ) { diff --git a/resources/mw.UploadWizardUploadInterface.js b/resources/mw.UploadWizardUploadInterface.js index 43cf415..2e18dba 100644 --- a/resources/mw.UploadWizardUploadInterface.js +++ b/resources/mw.UploadWizardUploadInterface.js @@ -23,15 +23,6 @@ this.isFilled = false; - this.$fileInputCtrl = $( '<input size="1" class="mwe-upwiz-file-input" name="file" type="file"/>' ); - if ( mw.UploadWizard.config.enableFormData && mw.fileApi.isFormDataAvailable() && - mw.UploadWizard.config.enableMultiFileSelect && mw.UploadWizard.config.enableMultipleFiles ) { - // Multiple uploads requires the FormData transport - this.$fileInputCtrl.attr( 'multiple', '1' ); - } - - this.initFileInputCtrl(); - this.$indicator = $( '<div class="mwe-upwiz-file-indicator"></div>' ); this.visibleFilenameDiv = $( '<div class="mwe-upwiz-visible-file"></div>' ) @@ -56,7 +47,6 @@ framed: false } ).on( 'click', function () { ui.upload.remove(); - ui.cancelPositionTracking(); } ); if ( mw.UploadWizard.config.defaults && mw.UploadWizard.config.defaults.objref !== '' ) { @@ -73,21 +63,9 @@ this.filenameCtrl = $( '<input type="hidden" name="filename" value=""/>' ).get( 0 ); - // this file Ctrl container is placed over other interface elements, intercepts clicks and gives them to the file input control. - // however, we want to pass hover events to interface elements that we are over, hence the bindings. - // n.b. not using toggleClass because it often gets this event wrong -- relies on previous state to know what to do - this.fileCtrlContainer = $( '<div class="mwe-upwiz-file-ctrl-container">' ); - - // the css trickery (along with css) - // here creates a giant size file input control which is contained within a div and then - // clipped for overflow. The effect is that we have a div (ctrl-container) we can position anywhere - // which works as a file input. It will be set to opacity:0 and then we can do whatever we want with - // interface "below". - // XXX caution -- if the add file input changes size we won't match, unless we add some sort of event to catch this. this.form = $( '<form method="POST" encType="multipart/form-data" class="mwe-upwiz-form"></form>' ) .attr( { action: this.upload.api.defaults.ajax.url } ) .append( this.visibleFilenameDiv ) - .append( this.fileCtrlContainer ) .append( this.filenameCtrl ) .get( 0 ); @@ -101,9 +79,6 @@ return; } - if ( !this.upload.fromURL ) { - $( this.fileCtrlContainer ).append( this.$fileInputCtrl ); - } $( this.div ).append( this.form ); // XXX evil hardcoded @@ -123,12 +98,13 @@ * @param {File} providedFile */ mw.UploadWizardUploadInterface.prototype.fill = function ( providedFile ) { - if ( providedFile ) { + if ( providedFile instanceof jQuery ) { + this.$fileInputCtrl = providedFile; + this.form.append( this.$fileInputCtrl ); + } else if ( providedFile ) { this.providedFile = providedFile; - - // if a file is already present, trigger the change event immediately. - this.$fileInputCtrl.trigger( 'change' ); } + this.clearErrors(); }; /** @@ -218,8 +194,6 @@ * Show that upload is transported */ mw.UploadWizardUploadInterface.prototype.showStashed = function () { - this.$fileInputCtrl.detach(); - this.showIndicator( 'stashed' ); this.setStatus( 'mwe-upwiz-stashed-upload' ); this.setAdditionalStatus( null ); @@ -256,36 +230,6 @@ this.setAdditionalStatus( $additionalStatus ); }; - mw.UploadWizardUploadInterface.prototype.initFileInputCtrl = function () { - var ui = this; - - this.$fileInputCtrl.change( function () { - ui.emit( 'file-changed', ui.getFiles() ); - - ui.clearErrors(); - } ); - }; - - /** - * Get a list of the files from this file input, defaulting to the value from the input form - * - * @return {Array} of File objects - */ - mw.UploadWizardUploadInterface.prototype.getFiles = function () { - var files = []; - if ( mw.fileApi.isAvailable() ) { - if ( this.providedFile ) { - files[ 0 ] = this.providedFile; - } else { - $.each( this.$fileInputCtrl.get( 0 ).files, function ( i, file ) { - files.push( file ); - } ); - } - } - - return files; - }; - /** * Get just the filename. * @@ -293,7 +237,7 @@ */ mw.UploadWizardUploadInterface.prototype.getFilename = function () { var input; - if ( this.providedFile && !this.$fileInputCtrl.get( 0 ).value ) { // default to the fileinput if it's defined. + if ( this.providedFile ) { if ( this.providedFile.fileName ) { return this.providedFile.fileName; } else { @@ -382,17 +326,12 @@ }; mw.UploadWizardUploadInterface.prototype.fileChangedError = function ( code, info ) { - var filename = this.getFilename(), + var filename = this.getFilename(); - // ok we now have a fileInputCtrl with a "bad" file in it - // you cannot blank a file input ctrl in all browsers, so we - // replace existing file input with empty clone - $newFileInput = this.$fileInputCtrl.clone(); - - this.$fileInputCtrl.replaceWith( $newFileInput ); - this.$fileInputCtrl = $newFileInput; - this.initFileInputCtrl(); - + if ( this.$fileInputCtrl ) { + this.$fileInputCtrl.remove(); + delete this.$fileInputCtrl; + } if ( this.providedFile ) { this.providedFile = null; } @@ -468,75 +407,6 @@ }; /** - * Move the file input to cover a certain element on the page. - * We use invisible file inputs because this is the only way to style a file input - * or otherwise get it to do what you want. - * It is helpful to sometimes move them to cover certain elements on the page, and - * even to pass events like hover - * - * @param {string} selector A jQuery-compatible selector, for a single element - * @param {string} [positionTracking] Whether to do position-polling ('poll') - * on the selected element or whether to listen to window-resize events ('resize') - */ - mw.UploadWizardUploadInterface.prototype.moveFileInputToCover = function ( selector, positionTracking ) { - var iv, to, onResize, $win, - ui = this; - - function update() { - var $covered = $( selector ); - - ui.fileCtrlContainer - .css( $covered.position() ) - .css( 'marginTop', $covered.css( 'marginTop' ) ) - .css( 'marginRight', $covered.css( 'marginRight' ) ) - .css( 'marginBottom', $covered.css( 'marginBottom' ) ) - .css( 'marginLeft', $covered.css( 'marginLeft' ) ) - .width( $covered.outerWidth() ) - .height( $covered.outerHeight() ); - - ui.fileCtrlContainer.css( { 'z-index': 1 } ); - - // shift the file input over with negative margins, - // internal to the overflow-containing div, so the div shows all button - // and none of the textfield-like input - ui.$fileInputCtrl.css( { - 'margin-left': '-' + ( ui.$fileInputCtrl.width() - $covered.outerWidth() - 10 ) + 'px', - 'margin-top': '-' + ( ui.$fileInputCtrl.height() - $covered.outerHeight() - 10 ) + 'px' - } ); - } - - 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.$fileInputCtrl.show(); - update(); - }; - - mw.UploadWizardUploadInterface.prototype.cancelPositionTracking = function () { - if ( $.isFunction( this.stopTracking ) ) { - this.stopTracking(); - this.stopTracking = null; - } - }; - - /** * this does two things: * 1 ) since the file input has been hidden with some clever CSS ( to avoid x-browser styling issues ), * update the visible filename @@ -565,14 +435,6 @@ $div = $( this.div ); this.isFilled = true; $div.addClass( 'filled' ); - - // 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' ) - ); this.emit( 'upload-filled' ); } else { this.emit( 'filename-accepted' ); diff --git a/resources/ui/steps/uw.ui.Upload.js b/resources/ui/steps/uw.ui.Upload.js index ed90472..8fa2e05 100644 --- a/resources/ui/steps/uw.ui.Upload.js +++ b/resources/ui/steps/uw.ui.Upload.js @@ -251,7 +251,7 @@ this.addFlickrFile.setDisabled( !fewerThanMax ); } - this.$fileList.find( '.mwe-upwiz-file:not(.filled) .mwe-upwiz-file-input' ).prop( 'disabled', !fewerThanMax ); + this.addFile.$element.find( '.mwe-upwiz-file-input' ).prop( 'disabled', !fewerThanMax ); }; /** diff --git a/resources/uploadWizard.css b/resources/uploadWizard.css index f49c199..0174ffd 100644 --- a/resources/uploadWizard.css +++ b/resources/uploadWizard.css @@ -75,12 +75,6 @@ margin-top: 10px; } -/* CSS styling hack for file inputs - http://www.quirksmode.org/dom/inputfile.html */ -.mwe-upwiz-file-ctrl-container { - position: absolute; - overflow: hidden; -} - .mwe-upwiz-file-input, .mwe-upwiz-visible-file { cursor: pointer; } @@ -89,9 +83,18 @@ cursor: auto !important; } -/* file inputs are freakishly large to overflow the containing div -- we get a div - that can act as a more styleable single-click file input */ -.mwe-upwiz-file-input { +#mwe-upwiz-add-file { + position: relative; + z-index: 0; + overflow: hidden; +} + +#mwe-upwiz-add-file .mwe-upwiz-file-input { + position: absolute; + top: 0; + bottom: 0; + left: 0; + right: 0; font-size: 100px; width: 100%; -moz-opacity: 0.3; -- To view, visit https://gerrit.wikimedia.org/r/270295 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9638d958ed5ebbbc4346ed03a1e9bbbf46147749 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/UploadWizard Gerrit-Branch: master Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits