jenkins-bot has submitted this change and it was merged.
Change subject: Get rid of pubsub once and for all
......................................................................
Get rid of pubsub once and for all
* Move code for displaying thumbnails out of mw.UploadWizardUpload.
As noted in some todo comments, it doesn't belong there.
* Emit 'success' event in mw.UploadWizardUpload when the file is
uploaded.
* Make more functions return promises rather than rely on global
state. getThumbnail() method now calls anything that's needed to
acquire a thumbnail, rather than wait for a thumbnail to magically
appear.
* Factor out and reuse code for displaying the thumbnail (or not) in
mw.UploadWizardUploadInterface, rather than rely on the thumbnail
never being generated when we don't want to show it.
Bug: T51988
Change-Id: Ibc12fe7a452607e920d1ab22e2abe5ff4e672367
---
M UploadWizardHooks.php
D resources/jquery/jquery.pubsub.js
M resources/mw.UploadWizard.js
M resources/mw.UploadWizardDetails.js
M resources/mw.UploadWizardUpload.js
M resources/mw.UploadWizardUploadInterface.js
M resources/ui/uw.ui.DeedPreview.js
M resources/ui/uw.ui.Thanks.js
M tests/qunit/controller/uw.controller.Deed.test.js
9 files changed, 120 insertions(+), 217 deletions(-)
Approvals:
MarkTraceur: Looks good to me, approved
jenkins-bot: Verified
diff --git a/UploadWizardHooks.php b/UploadWizardHooks.php
index 929dff6..377c255 100644
--- a/UploadWizardHooks.php
+++ b/UploadWizardHooks.php
@@ -66,7 +66,6 @@
'resources/jquery/jquery.morphCrossfade.js',
'resources/jquery/jquery.removeCtrl.js',
'resources/jquery/jquery.showThumbCtrl.js',
- 'resources/jquery/jquery.pubsub.js',
'resources/jquery/jquery.lazyload.js',
// common utilities
diff --git a/resources/jquery/jquery.pubsub.js
b/resources/jquery/jquery.pubsub.js
deleted file mode 100644
index d598f3a..0000000
--- a/resources/jquery/jquery.pubsub.js
+++ /dev/null
@@ -1,119 +0,0 @@
-/**
- * Minimal pubsub framework
- *
- * Loosely based on
https://github.com/phiggins42/bloody-jquery-plugins/pubsub.js, which is itself
BSD-licensed.
- * Concept of 'ready' events is new, though.
- *
- * @author Neil Kandalgaonkar <[email protected]>
- */
-
-( function ( $ ) {
- var
- /**
- * Store of events -> array of listener callbacks
- */
- subs = {},
-
- /**
- * Store of ready events, as object of event name -> argument
array
- */
- ready = {};
-
- /**
- * Publish an event
- * Additional variadic arguments after the event name are passed as
arguments to the subscriber functions
- * @param {string} name of event
- * @return {number} number of subscribers
- */
- $.publish = function ( name /* , args... */ ) {
- var args = [].slice.call( arguments, 1 );
- if ( typeof subs[name] !== 'undefined' && subs[name] instanceof
Array ) {
- $.each( subs[name], function ( i, sub ) {
- sub.apply( null, args );
- } );
- return subs[name].length;
- }
- return 0;
- };
-
- /**
- * Publish a ready event. Ready events occur once only, so
- * subscribers will be called even if they subscribe later.
- * Additional variadic arguments after the event name are passed as
arguments to the subscriber functions
- * @param {string} name of event
- * @return {number} number of subscribers
- */
- $.publishReady = function ( name /*, args... */ ) {
- if ( typeof ready[name] === 'undefined' ) {
- var args = [].slice.call( arguments, 1 );
- ready[name] = args;
- $.publish.apply( null, arguments );
- }
- };
-
- /**
- * Subscribe to an event.
- * @param {string} name of event to listen for
- * @param {Function} callback to run when event occurs
- * @return {Array} returns handle which can be used as argument to
unsubscribe()
- */
- $.subscribe = function ( name, fn ) {
- if ( typeof subs[name] === 'undefined' ) {
- subs[name] = [];
- }
- subs[name].push( fn );
- return [ name, fn ];
- };
-
- /**
- * Subscribe to a ready event. See publishReady().
- * Subscribers will be called even if they subscribe long after the
event fired.
- * @param {string} name of event to listen for
- * @param {Function} callback to run now (if event already occurred) or
when event occurs
- * @return {Array} returns handle which can be used as argument to
unsubscribe()
- */
- $.subscribeReady = function ( name, fn ) {
- if ( ready[name] ) {
- fn.apply( null, ready[name] );
- } else {
- $.subscribe( name, fn );
- }
- };
-
- /**
- * Given the handle of a particular subscription, remove it
- * @param {Array} object returned by subscribe ( array of event name
and callback )
- * @return {Boolean} success
- */
- $.unsubscribe = function ( nameFn ) {
- var name = nameFn[0],
- fn = nameFn[1],
- success = false;
-
- if ( subs[name].length ) {
- $.each( subs[name], function ( i, fni ) {
- if ( fni === fn ) {
- subs[name].splice( i, 1 );
- success = true;
- return false; /* early exit loop */
- }
- } );
- }
- return success;
- };
-
- /**
- * Prevent ready objects from hanging around forever
- */
- $.purgeReadyEvents = function () {
- ready = {};
- };
-
- /**
- * Remove all subscriptions from everything
- */
- $.purgeSubscriptions = function () {
- subs = {};
- };
-
-} )( jQuery );
diff --git a/resources/mw.UploadWizard.js b/resources/mw.UploadWizard.js
index 2d3992d..7690bfe 100644
--- a/resources/mw.UploadWizard.js
+++ b/resources/mw.UploadWizard.js
@@ -115,8 +115,6 @@
}
this.showDeed = false;
- $.purgeReadyEvents();
- $.purgeSubscriptions();
this.removeMatchingUploads( function () { return true;
} );
this.hasLoadedBefore = true;
},
@@ -481,6 +479,27 @@
};
/**
+ * Helper method to put a thumbnail somewhere.
+ *
+ * @param {string|jQuery} selector String representing a jQuery
selector, or a jQuery object
+ * @param {HTMLCanvasElement|HTMLImageElement|null} image
+ */
+ mw.UploadWizard.placeThumbnail = function ( selector, image ) {
+ if ( image === null ) {
+ $( selector ).addClass( 'mwe-upwiz-file-preview-broken'
);
+ return;
+ }
+
+ $( selector )
+ .css( { background: 'none' } )
+ .html(
+ $( '<a>' )
+ .addClass( 'mwe-upwiz-thumbnail-link' )
+ .append( image )
+ );
+ };
+
+ /**
* Check if a value is null, undefined, or the empty string.
*
* @param {mixed} v Variable to be checked
diff --git a/resources/mw.UploadWizardDetails.js
b/resources/mw.UploadWizardDetails.js
index 9a9afd9..0e4c9e4 100644
--- a/resources/mw.UploadWizardDetails.js
+++ b/resources/mw.UploadWizardDetails.js
@@ -1161,11 +1161,13 @@
* @param {Object} result Upload API result object
*/
populate: function () {
- this.upload.setThumbnail(
- this.thumbnailDiv,
+ var thumbnailDiv = this.thumbnailDiv;
+ this.upload.getThumbnail(
mw.UploadWizard.config.thumbnailWidth,
mw.UploadWizard.config.thumbnailMaxHeight
- );
+ ).done( function ( thumb ) {
+ mw.UploadWizard.placeThumbnail( thumbnailDiv,
thumb );
+ } );
this.prefillDate();
this.prefillAuthor();
this.prefillTitle();
diff --git a/resources/mw.UploadWizardUpload.js
b/resources/mw.UploadWizardUpload.js
index ef8b3ae..4c7acc6 100644
--- a/resources/mw.UploadWizardUpload.js
+++ b/resources/mw.UploadWizardUpload.js
@@ -45,7 +45,7 @@
this.file = undefined;
this.ignoreWarning = {};
this.fromURL = false;
- this.previewLoaded = false;
+ this.previewPromise = null;
this.generatePreview = true;
this.fileKey = undefined;
@@ -58,8 +58,7 @@
this.ui = new mw.UploadWizardUploadInterface( this, filesDiv )
.connect( this, {
'file-changed': [ 'emit', 'file-changed',
upload ],
- 'filename-accepted': [ 'emit',
'filename-accepted' ],
- 'show-preview': 'makePreview'
+ 'filename-accepted': [ 'emit',
'filename-accepted' ]
} )
.on( 'upload-filled', function () {
@@ -336,8 +335,10 @@
this.extractUploadInfo( result.upload );
this.state = 'stashed';
this.ui.showStashed();
- $.publishReady( 'thumbnails.' + this.index, 'api' );
+
+ this.emit( 'success' );
// check all uploads, if they're complete, show the
next button
+ // TODO Make wizard connect to 'success' event
this.wizard.steps.file.showNext();
} else {
this.setError( 'noimageinfo' );
@@ -832,7 +833,7 @@
* @return {jQuery.Promise} Promise resolved with a HTMLImageElement,
or null if thumbnail
* couldn't be generated
*/
- mw.UploadWizardUpload.prototype.getAndPublishApiThumbnail = function (
width, height ) {
+ mw.UploadWizardUpload.prototype.getApiThumbnail = function ( width,
height ) {
var deferred,
key = width + '|' + height;
@@ -1073,69 +1074,53 @@
height: ( height === undefined ? null : parseInt(
height, 10 ) )
};
- return mw.canvas.isAvailable() ?
this.getTransformedCanvasElement( image, constraints )
- :
this.getBrowserScaledImageElement( image, constraints );
+ return mw.canvas.isAvailable()
+ ? this.getTransformedCanvasElement( image, constraints )
+ : this.getBrowserScaledImageElement( image, constraints
);
};
/**
- * Given a jQuery selector, subscribe to the "ready" event that fills
the thumbnail
- * This will trigger if the thumbnail is added in the future or if it
already has been
+ * Acquire a thumbnail of given dimensions for this upload.
*
- * @param {string} selector String representing a jQuery selector
* @param {number} width Width constraint
* @param {number} [height] Height constraint
+ * @return {jQuery.Promise} Promise resolved with the HTMLImageElement
or HTMLCanvasElement
+ * containing a thumbnail, or resolved with `null` when one can't be
produced
*/
- mw.UploadWizardUpload.prototype.setThumbnail = function ( selector,
width, height ) {
+ mw.UploadWizardUpload.prototype.getThumbnail = function ( width, height
) {
var upload = this,
- placed = false;
+ deferred = $.Deferred();
/**
- * This callback will add an image to the selector, using
in-browser scaling if necessary
- *
* @param {HTMLImageElement|null} image
*/
- function placeImageCallback( image ) {
- var elm;
-
+ function imageCallback( image ) {
if ( image === null ) {
- $( selector ).addClass(
'mwe-upwiz-file-preview-broken' );
upload.ui.setStatus(
'mwe-upwiz-thumbnail-failed' );
+ deferred.resolve( image );
return;
}
- elm = upload.getScaledImageElement( image, width,
height );
- // add the image to the DOM, finally
- $( selector )
- .css( { background: 'none' } )
- .html(
- $( '<a/></a>' )
- .addClass(
'mwe-upwiz-thumbnail-link' )
- .append( elm )
- );
- placed = true;
+ image = upload.getScaledImageElement( image, width,
height );
+ deferred.resolve( image );
}
- // Listen for even which says some kind of thumbnail is
available.
- // The argument is an either an ImageHtmlElement ( if we could
get the thumbnail locally ) or the string 'api' indicating you
- // now need to get the scaled thumbnail via the API
- $.subscribeReady(
- 'thumbnails.' + this.index,
- function ( x ) {
- if ( !placed ) {
- if ( x === 'api' ) {
- // get the thumbnail via API.
Queries are cached, so if this thumbnail was already
- // fetched for some reason,
we'll get it immediately
-
upload.getAndPublishApiThumbnail( width, height ).done( placeImageCallback );
- } else if ( x instanceof
HTMLImageElement ) {
- placeImageCallback( x );
- } else {
- // something else went wrong,
place broken image
- mw.log.warn( 'Unexpected
argument to thumbnails event: ' + x );
- placeImageCallback( null );
- }
+ this.makePreview()
+ .done( imageCallback )
+ .fail( function () {
+ // Can't generate the thumbnail locally, get
the thumbnail via API after
+ // the file is uploaded. Queries are cached, so
if this thumbnail was
+ // already fetched for some reason, we'll get
it immediately.
+ if ( upload.state !== 'new' && upload.state !==
'transporting' ) {
+ upload.getApiThumbnail( width, height
).done( imageCallback );
+ } else {
+ upload.once( 'success', function () {
+ upload.getApiThumbnail( width,
height ).done( imageCallback );
+ } );
}
- }
- );
+ } );
+
+ return deferred.promise();
};
mw.UploadWizardUpload.prototype.createDetails = function () {
@@ -1149,12 +1134,6 @@
*/
mw.UploadWizardUpload.prototype.fileChangedOk = function () {
this.ui.fileChangedOk( this.imageinfo, this.file, this.fromURL
);
-
- if ( this.generatePreview ) {
- this.makePreview();
- } else {
- this.ui.makeShowThumbCtrl();
- }
};
/**
@@ -1162,12 +1141,14 @@
*/
mw.UploadWizardUpload.prototype.makePreview = function () {
var first, video, url, dataUrlReader,
+ deferred = $.Deferred(),
upload = this;
// don't run this repeatedly.
- if ( this.previewLoaded ) {
- return;
+ if ( this.previewPromise ) {
+ return this.previewPromise;
}
+ this.previewPromise = deferred.promise();
// do preview if we can
if ( this.isPreviewable() ) {
@@ -1197,7 +1178,7 @@
canvas.height =
Math.round( canvas.width * video.videoHeight / video.videoWidth );
context =
canvas.getContext( '2d' );
context.drawImage(
video, 0, 0, canvas.width, canvas.height );
- upload.loadImage(
canvas.toDataURL() );
+ upload.loadImage(
canvas.toDataURL(), deferred );
upload.URL().revokeObjectURL( video.url );
}, 500 );
}
@@ -1209,22 +1190,27 @@
dataUrlReader.onload = function () {
// this step (inserting
image-as-dataurl into image object) is slow for large images, which
// is why this is optional and has a
control attached to it to load the preview.
- upload.loadImage( dataUrlReader.result
);
+ upload.loadImage( dataUrlReader.result,
deferred );
};
dataUrlReader.readAsDataURL( this.file );
}
+ } else {
+ deferred.reject();
}
+
+ return this.previewPromise;
};
/**
* Loads an image preview.
*/
- mw.UploadWizardUpload.prototype.loadImage = function ( url ) {
- var image = document.createElement( 'img' ),
- upload = this;
+ mw.UploadWizardUpload.prototype.loadImage = function ( url, deferred ) {
+ var image = document.createElement( 'img' );
image.onload = function () {
- $.publishReady( 'thumbnails.' + upload.index, image );
- upload.previewLoaded = true;
+ deferred.resolve( image );
+ };
+ image.onerror = function () {
+ deferred.reject();
};
image.src = url;
};
diff --git a/resources/mw.UploadWizardUploadInterface.js
b/resources/mw.UploadWizardUploadInterface.js
index 2f48e39..2bf9088 100644
--- a/resources/mw.UploadWizardUploadInterface.js
+++ b/resources/mw.UploadWizardUploadInterface.js
@@ -10,7 +10,7 @@
* @param {string} filesDiv Selector for the `div` into which to insert
file interface
*/
mw.UploadWizardUploadInterface = function
MWUploadWizardUploadInterface( upload, filesDiv ) {
- var $preview,
+ var
ui = this;
OO.EventEmitter.call( this );
@@ -116,14 +116,6 @@
// this.progressBar = ( no progress bar for individual uploads
yet )
// we bind to the ui div since unbind doesn't work for non-DOM
objects
$( this.div ).bind( 'transportProgressEvent', function () {
ui.showTransportProgress(); } );
-
- // XXX feature envy
- $preview = $( this.div ).find( '.mwe-upwiz-file-preview' );
- this.upload.setThumbnail(
- $preview,
- mw.UploadWizard.config.thumbnailWidth,
- mw.UploadWizard.config.thumbnailMaxHeight
- );
};
OO.mixinClass( mw.UploadWizardUploadInterface, OO.EventEmitter );
@@ -358,6 +350,12 @@
this.clearStatus();
this.setStatusString( statusItems.join( ' \u00b7 ' ) );
+
+ if ( this.upload.generatePreview ) {
+ this.showThumbnail();
+ } else {
+ this.makeShowThumbCtrl();
+ }
};
/**
@@ -368,15 +366,30 @@
// add a control for showing the preview if the user needs it
this.$showThumbCtrl = $.fn.showThumbCtrl(
- 'mwe-upwiz-show-thumb',
- 'mwe-upwiz-show-thumb-tip',
- function () { ui.emit( 'show-preview' ); }
- );
+ 'mwe-upwiz-show-thumb',
+ 'mwe-upwiz-show-thumb-tip',
+ function () {
+ ui.showThumbnail();
+ }
+ );
this.visibleFilenameDiv.find( '.mwe-upwiz-file-status-line' )
.append( '<br>' ).append( this.$showThumbCtrl );
};
+ /**
+ * Display thumbnail preview.
+ */
+ mw.UploadWizardUploadInterface.prototype.showThumbnail = function () {
+ var $preview = $( this.div ).find( '.mwe-upwiz-file-preview' );
+ this.upload.getThumbnail(
+ mw.UploadWizard.config.thumbnailWidth,
+ mw.UploadWizard.config.thumbnailMaxHeight
+ ).done( function ( thumb ) {
+ mw.UploadWizard.placeThumbnail( $preview, thumb );
+ } );
+ };
+
mw.UploadWizardUploadInterface.prototype.fileChangedError = function (
code, info ) {
var filename = this.getFilename(),
diff --git a/resources/ui/uw.ui.DeedPreview.js
b/resources/ui/uw.ui.DeedPreview.js
index 11a4b13..a88f83b 100644
--- a/resources/ui/uw.ui.DeedPreview.js
+++ b/resources/ui/uw.ui.DeedPreview.js
@@ -15,7 +15,7 @@
* along with UploadWizard. If not, see <http://www.gnu.org/licenses/>.
*/
-( function ( $, uw ) {
+( function ( $, mw, uw ) {
/**
* Represents the UI for a thumbnail in the Deed step.
*
@@ -25,12 +25,14 @@
* @param {Object} config The UW config object.
*/
uw.ui.DeedPreview = function UWUIDeedPreview( upload, config ) {
- this.$thumbnailDiv = $( '<div>' ).addClass(
'mwe-upwiz-thumbnail' );
- upload.setThumbnail(
- this.$thumbnailDiv,
+ var $thumbnailDiv = $( '<div>' ).addClass(
'mwe-upwiz-thumbnail' );
+ this.$thumbnailDiv = $thumbnailDiv;
+ upload.getThumbnail(
config.thumbnailWidth,
config.thumbnailMaxHeight
- );
+ ).done( function ( thumb ) {
+ mw.UploadWizard.placeThumbnail( $thumbnailDiv, thumb );
+ } );
$( '#mwe-upwiz-deeds-thumbnails' ).append( this.$thumbnailDiv );
};
@@ -40,4 +42,4 @@
}
};
-}( jQuery, mediaWiki.uploadWizard ) );
+}( jQuery, mediaWiki, mediaWiki.uploadWizard ) );
diff --git a/resources/ui/uw.ui.Thanks.js b/resources/ui/uw.ui.Thanks.js
index d164a72..bad8be3 100644
--- a/resources/ui/uw.ui.Thanks.js
+++ b/resources/ui/uw.ui.Thanks.js
@@ -139,11 +139,12 @@
)
);
- upload.setThumbnail(
- $thumbnailDiv,
+ upload.getThumbnail(
mw.UploadWizard.config.thumbnailWidth,
mw.UploadWizard.config.thumbnailMaxHeight
- );
+ ).done( function ( thumb ) {
+ mw.UploadWizard.placeThumbnail( $thumbnailDiv, thumb );
+ } );
// Set the thumbnail links so that they point to the image
description page
$thumbnailLink.add( $thumbnailDiv.find(
'.mwe-upwiz-thumbnail-link' ) ).attr( {
diff --git a/tests/qunit/controller/uw.controller.Deed.test.js
b/tests/qunit/controller/uw.controller.Deed.test.js
index 31be594..1cf1bdf 100644
--- a/tests/qunit/controller/uw.controller.Deed.test.js
+++ b/tests/qunit/controller/uw.controller.Deed.test.js
@@ -15,7 +15,7 @@
* along with DeedWizard. If not, see <http://www.gnu.org/licenses/>.
*/
-( function ( mw, uw ) {
+( function ( $, mw, uw ) {
QUnit.module( 'mw.uw.controller.Deed', QUnit.newMwEnvironment() );
QUnit.test( 'Constructor sanity test', 3, function ( assert ) {
@@ -30,12 +30,12 @@
new mw.Api(),
{ licensing: { thirdParty: { type: 'test',
licenses: [] } } }
),
- ststub = this.sandbox.stub(),
+ ststub = this.sandbox.stub().returns(
$.Deferred().promise() ),
uploads = [
- { fromURL: true, setThumbnail: ststub },
- { setThumbnail: ststub },
- { fromURL: true, setThumbnail: ststub },
- { setThumbnail: ststub }
+ { fromURL: true, getThumbnail: ststub },
+ { getThumbnail: ststub },
+ { fromURL: true, getThumbnail: ststub },
+ { getThumbnail: ststub }
];
this.sandbox.stub( step.ui, 'moveTo' );
@@ -43,4 +43,4 @@
assert.strictEqual( ststub.callCount, 2 );
} );
-}( mediaWiki, mediaWiki.uploadWizard ) );
+}( jQuery, mediaWiki, mediaWiki.uploadWizard ) );
--
To view, visit https://gerrit.wikimedia.org/r/248908
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc12fe7a452607e920d1ab22e2abe5ff4e672367
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/UploadWizard
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: MarkTraceur <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
