r77099 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r77098‎ | r77099 | r77100 >
Date:06:27, 22 November 2010
Author:neilk
Status:deferred
Tags:
Comment:
made all thumbnail generation block until thumbnail loaded and cached, simplified success callback
Modified paths:
  • /trunk/extensions/UploadWizard/resources/combined.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/combined.min.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizard.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/resources/combined.js
@@ -8383,7 +8383,7 @@
83848384 mw.GroupProgressBar.prototype = {
83858385
83868386 /**
8387 - * Show the progress bar with a slideout motion
 8387+ * Show the progress bar
83888388 */
83898389 showBar: function() {
83908390 this.$selector.find( '.mwe-upwiz-progress-bar-etr' ).fadeIn( 200 );
@@ -9221,18 +9221,15 @@
92229222 _this.transportProgress = 1;
92239223 _this.ui.setStatus( 'mwe-upwiz-getting-metadata' );
92249224 _this.extractUploadInfo( result );
9225 - var setupDeedsAndShowComplete = function() {
9226 - _this.ui.setPreview( $img );
9227 - _this.deedPreview.setup();
9228 - _this.details.populate();
9229 - _this.state = 'stashed';
9230 - _this.ui.showStashed();
9231 - };
9232 - var $img = $j( '<img/>' ).load( setupDeedsAndShowComplete );
9233 - // blocking preload for thumbnail
 9225+
 9226+ // use blocking preload for thumbnail, no loading spinner.
92349227 _this.getThumbnail(
9235 - function( thumbnailAttrs ) {
9236 - $img.attr( thumbnailAttrs );
 9228+ function( image ) {
 9229+ _this.ui.setPreview( image );
 9230+ _this.deedPreview.setup();
 9231+ _this.details.populate();
 9232+ _this.state = 'stashed';
 9233+ _this.ui.showStashed();
92379234 },
92389235 mw.UploadWizard.config[ 'iconThumbnailWidth' ],
92399236 mw.UploadWizard.config[ 'iconThumbnailMaxHeight' ]
@@ -9353,12 +9350,15 @@
93549351 }
93559352 var thumbnails = data.query.stashimageinfo;
93569353 for ( var i = 0; i < thumbnails.length; i++ ) {
9357 - _this.thumbnails[key] = {
9358 - src: thumbnails[i].thumburl,
9359 - width: thumbnails[i].thumbwidth,
9360 - height: thumbnails[i].thumbheight
9361 - };
9362 - callback( _this.thumbnails[key] );
 9354+ var thumb = thumbnails[i];
 9355+ var image = document.createElement( 'img' );
 9356+ $j( image ).load( function() {
 9357+ callback( image );
 9358+ } );
 9359+ image.width = thumb.thumbwidth;
 9360+ image.height = thumb.thumbheight;
 9361+ image.src = thumb.thumburl;
 9362+ _this.thumbnails[key] = image;
93639363 }
93649364 } );
93659365 }
@@ -9366,14 +9366,15 @@
93679367
93689368 /**
93699369 * Look up thumbnail info and set it in HTML, with loading spinner
9370 - * it might be interesting to make this more of a publish/subscribe thing, since we have to do this 3x
9371 - * the callbacks may pile up, getting unnecessary info
93729370 *
93739371 * @param selector
93749372 * @param width
93759373 * @param height (optional)
93769374 */
93779375 setThumbnail: function( selector, width, height ) {
 9376+ // set the thumbnail on the page to have a loading spinner
 9377+ $j( selector ).loadingSpinner();
 9378+
93789379 var _this = this;
93799380 if ( typeof width === 'undefined' || width === null || width <= 0 ) {
93809381 width = mw.UploadWizard.config[ 'thumbnailWidth' ];
@@ -9383,18 +9384,20 @@
93849385 if ( !mw.isEmpty( height ) ) {
93859386 height = parseInt( height, 10 );
93869387 }
9387 -
9388 - var callback = function( thumbnail ) {
 9388+
 9389+ var callback = function( image ) {
93899390 // side effect: will replace thumbnail's loadingSpinner
93909391 $j( selector ).html(
9391 - $j('<a/>')
 9392+ $j( '<a/>' )
93929393 .attr( { 'href': _this.imageinfo.url,
93939394 'target' : '_new' } )
93949395 .append(
93959396 $j( '<img/>' )
9396 - .attr( 'width', thumbnail.width )
9397 - .attr( 'height', thumbnail.height )
9398 - .attr( 'src', thumbnail.src ) ) );
 9397+ .attr( { 'width': image.width,
 9398+ 'height': image.height,
 9399+ 'src': image.src } )
 9400+ )
 9401+ );
93999402 };
94009403
94019404 $j( selector ).loadingSpinner();
@@ -9507,29 +9510,36 @@
95089511 },
95099512
95109513 /**
9511 - * change the indicator at the far right
 9514+ * change the graphic indicator at the far end of the row for this file
 9515+ * @param String statusClass: corresponds to a class mwe-upwiz-status which changes style of indicator.
95129516 */
95139517 showIndicator: function( statusClass ) {
9514 - var _this = this;
9515 - var $indicator = $j( _this.div ).find( '.mwe-upwiz-file-indicator' );
 9518+ var $indicator = $j( this.div ).find( '.mwe-upwiz-file-indicator' );
 9519+ // remove any other such classes
95169520 $j.each( $indicator.attr( 'class' ).split( /\s+/ ), function( i, className ) {
95179521 if ( className.match( /^mwe-upwiz-status/ ) ) {
95189522 $indicator.removeClass( className );
95199523 }
95209524 } );
9521 - $indicator.addClass( 'mwe-upwiz-status-' + statusClass );
9522 - // is this causing dancing when we switch from spinner to checkmark?
9523 - $j( _this.div ).find( '.mwe-upwiz-visible-file-filename' )
9524 - .css( 'margin-right', ( $indicator.outerWidth() + 24 ).toString() + 'px' );
9525 - $indicator.css( 'visibility', 'visible' );
 9525+ // add the desired class and make it visible, if it wasn't already.
 9526+ $indicator.addClass( 'mwe-upwiz-status-' + statusClass )
 9527+ .css( 'visibility', 'visible' );
95269528 },
95279529
9528 - setPreview: function( $img ) {
 9530+ /**
 9531+ * Set the preview image on the file page for this upload.
 9532+ * @param HTMLImageElement
 9533+ */
 9534+ setPreview: function( image ) {
95299535 // encoding for url here?
9530 - $j( this.div ).find( '.mwe-upwiz-file-preview' ).css( 'background-image', 'url(' + $img.attr( 'src' ) + ')' );
 9536+ $j( this.div ).find( '.mwe-upwiz-file-preview' ).css( 'background-image', 'url(' + image.src + ')' );
95319537 },
95329538
9533 - // too abstract?
 9539+ /**
 9540+ * Set the status line for this upload with an internationalized message string.
 9541+ * @param String msgKey: key for the message
 9542+ * @param Array args: array of values, in case any need to be fed to the image.
 9543+ */
95349544 setStatus: function( msgKey, args ) {
95359545 if ( !mw.isDefined( args ) ) {
95369546 args = [];
@@ -9537,10 +9547,17 @@
95389548 this.setStatusStr( gM( msgKey, args ) );
95399549 },
95409550
 9551+ /**
 9552+ * Set the status line for this upload
 9553+ * @param String str: the string to use
 9554+ */
95419555 setStatusStr: function( str ) {
95429556 $j( this.div ).find( '.mwe-upwiz-file-status' ).html( str ).show();
95439557 },
95449558
 9559+ /**
 9560+ * Clear the status line for this upload (hide it, in case there are paddings and such which offset other things.)
 9561+ */
95459562 clearStatus: function() {
95469563 $j( this.div ).find( '.mwe-upwiz-file-status' ).hide();
95479564 },
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizard.js
@@ -101,18 +101,15 @@
102102 _this.transportProgress = 1;
103103 _this.ui.setStatus( 'mwe-upwiz-getting-metadata' );
104104 _this.extractUploadInfo( result );
105 - var setupDeedsAndShowComplete = function() {
106 - _this.ui.setPreview( $img );
107 - _this.deedPreview.setup();
108 - _this.details.populate();
109 - _this.state = 'stashed';
110 - _this.ui.showStashed();
111 - };
112 - var $img = $j( '<img/>' ).load( setupDeedsAndShowComplete );
113 - // blocking preload for thumbnail
 105+
 106+ // use blocking preload for thumbnail, no loading spinner.
114107 _this.getThumbnail(
115 - function( thumbnailAttrs ) {
116 - $img.attr( thumbnailAttrs );
 108+ function( image ) {
 109+ _this.ui.setPreview( image );
 110+ _this.deedPreview.setup();
 111+ _this.details.populate();
 112+ _this.state = 'stashed';
 113+ _this.ui.showStashed();
117114 },
118115 mw.UploadWizard.config[ 'iconThumbnailWidth' ],
119116 mw.UploadWizard.config[ 'iconThumbnailMaxHeight' ]
@@ -233,12 +230,15 @@
234231 }
235232 var thumbnails = data.query.stashimageinfo;
236233 for ( var i = 0; i < thumbnails.length; i++ ) {
237 - _this.thumbnails[key] = {
238 - src: thumbnails[i].thumburl,
239 - width: thumbnails[i].thumbwidth,
240 - height: thumbnails[i].thumbheight
241 - };
242 - callback( _this.thumbnails[key] );
 234+ var thumb = thumbnails[i];
 235+ var image = document.createElement( 'img' );
 236+ $j( image ).load( function() {
 237+ callback( image );
 238+ } );
 239+ image.width = thumb.thumbwidth;
 240+ image.height = thumb.thumbheight;
 241+ image.src = thumb.thumburl;
 242+ _this.thumbnails[key] = image;
243243 }
244244 } );
245245 }
@@ -246,14 +246,15 @@
247247
248248 /**
249249 * Look up thumbnail info and set it in HTML, with loading spinner
250 - * it might be interesting to make this more of a publish/subscribe thing, since we have to do this 3x
251 - * the callbacks may pile up, getting unnecessary info
252250 *
253251 * @param selector
254252 * @param width
255253 * @param height (optional)
256254 */
257255 setThumbnail: function( selector, width, height ) {
 256+ // set the thumbnail on the page to have a loading spinner
 257+ $j( selector ).loadingSpinner();
 258+
258259 var _this = this;
259260 if ( typeof width === 'undefined' || width === null || width <= 0 ) {
260261 width = mw.UploadWizard.config[ 'thumbnailWidth' ];
@@ -263,18 +264,20 @@
264265 if ( !mw.isEmpty( height ) ) {
265266 height = parseInt( height, 10 );
266267 }
267 -
268 - var callback = function( thumbnail ) {
 268+
 269+ var callback = function( image ) {
269270 // side effect: will replace thumbnail's loadingSpinner
270271 $j( selector ).html(
271 - $j('<a/>')
 272+ $j( '<a/>' )
272273 .attr( { 'href': _this.imageinfo.url,
273274 'target' : '_new' } )
274275 .append(
275276 $j( '<img/>' )
276 - .attr( 'width', thumbnail.width )
277 - .attr( 'height', thumbnail.height )
278 - .attr( 'src', thumbnail.src ) ) );
 277+ .attr( { 'width': image.width,
 278+ 'height': image.height,
 279+ 'src': image.src } )
 280+ )
 281+ );
279282 };
280283
281284 $j( selector ).loadingSpinner();
@@ -387,29 +390,36 @@
388391 },
389392
390393 /**
391 - * change the indicator at the far right
 394+ * change the graphic indicator at the far end of the row for this file
 395+ * @param String statusClass: corresponds to a class mwe-upwiz-status which changes style of indicator.
392396 */
393397 showIndicator: function( statusClass ) {
394 - var _this = this;
395 - var $indicator = $j( _this.div ).find( '.mwe-upwiz-file-indicator' );
 398+ var $indicator = $j( this.div ).find( '.mwe-upwiz-file-indicator' );
 399+ // remove any other such classes
396400 $j.each( $indicator.attr( 'class' ).split( /\s+/ ), function( i, className ) {
397401 if ( className.match( /^mwe-upwiz-status/ ) ) {
398402 $indicator.removeClass( className );
399403 }
400404 } );
401 - $indicator.addClass( 'mwe-upwiz-status-' + statusClass );
402 - // is this causing dancing when we switch from spinner to checkmark?
403 - $j( _this.div ).find( '.mwe-upwiz-visible-file-filename' )
404 - .css( 'margin-right', ( $indicator.outerWidth() + 24 ).toString() + 'px' );
405 - $indicator.css( 'visibility', 'visible' );
 405+ // add the desired class and make it visible, if it wasn't already.
 406+ $indicator.addClass( 'mwe-upwiz-status-' + statusClass )
 407+ .css( 'visibility', 'visible' );
406408 },
407409
408 - setPreview: function( $img ) {
 410+ /**
 411+ * Set the preview image on the file page for this upload.
 412+ * @param HTMLImageElement
 413+ */
 414+ setPreview: function( image ) {
409415 // encoding for url here?
410 - $j( this.div ).find( '.mwe-upwiz-file-preview' ).css( 'background-image', 'url(' + $img.attr( 'src' ) + ')' );
 416+ $j( this.div ).find( '.mwe-upwiz-file-preview' ).css( 'background-image', 'url(' + image.src + ')' );
411417 },
412418
413 - // too abstract?
 419+ /**
 420+ * Set the status line for this upload with an internationalized message string.
 421+ * @param String msgKey: key for the message
 422+ * @param Array args: array of values, in case any need to be fed to the image.
 423+ */
414424 setStatus: function( msgKey, args ) {
415425 if ( !mw.isDefined( args ) ) {
416426 args = [];
@@ -417,10 +427,17 @@
418428 this.setStatusStr( gM( msgKey, args ) );
419429 },
420430
 431+ /**
 432+ * Set the status line for this upload
 433+ * @param String str: the string to use
 434+ */
421435 setStatusStr: function( str ) {
422436 $j( this.div ).find( '.mwe-upwiz-file-status' ).html( str ).show();
423437 },
424438
 439+ /**
 440+ * Clear the status line for this upload (hide it, in case there are paddings and such which offset other things.)
 441+ */
425442 clearStatus: function() {
426443 $j( this.div ).find( '.mwe-upwiz-file-status' ).hide();
427444 },
Index: trunk/extensions/UploadWizard/resources/combined.min.js
@@ -9221,18 +9221,15 @@
92229222 _this.transportProgress=1;
92239223 _this.ui.setStatus('mwe-upwiz-getting-metadata');
92249224 _this.extractUploadInfo(result);
9225 -var setupDeedsAndShowComplete=function(){
9226 -_this.ui.setPreview($img);
 9225+
 9226+
 9227+_this.getThumbnail(
 9228+function(image){
 9229+_this.ui.setPreview(image);
92279230 _this.deedPreview.setup();
92289231 _this.details.populate();
92299232 _this.state='stashed';
92309233 _this.ui.showStashed();
9231 -};
9232 -var $img=$j('<img/>').load(setupDeedsAndShowComplete);
9233 -
9234 -_this.getThumbnail(
9235 -function(thumbnailAttrs){
9236 -$img.attr(thumbnailAttrs);
92379234 },
92389235 mw.UploadWizard.config['iconThumbnailWidth'],
92399236 mw.UploadWizard.config['iconThumbnailMaxHeight']
@@ -9353,12 +9350,15 @@
93549351 }
93559352 var thumbnails=data.query.stashimageinfo;
93569353 for(var i=0;i<thumbnails.length;i++){
9357 -_this.thumbnails[key]={
9358 -src:thumbnails[i].thumburl,
9359 -width:thumbnails[i].thumbwidth,
9360 -height:thumbnails[i].thumbheight
9361 -};
9362 -callback(_this.thumbnails[key]);
 9354+var thumb=thumbnails[i];
 9355+var image=document.createElement('img');
 9356+$j(image).load(function(){
 9357+callback(image);
 9358+});
 9359+image.width=thumb.thumbwidth;
 9360+image.height=thumb.thumbheight;
 9361+image.src=thumb.thumburl;
 9362+_this.thumbnails[key]=image;
93639363 }
93649364 });
93659365 }
@@ -9371,9 +9371,10 @@
93729372
93739373
93749374
 9375+setThumbnail:function(selector,width,height){
93759376
 9377+$j(selector).loadingSpinner();
93769378
9377 -setThumbnail:function(selector,width,height){
93789379 var _this=this;
93799380 if(typeof width==='undefined'||width===null||width<=0){
93809381 width=mw.UploadWizard.config['thumbnailWidth'];
@@ -9384,7 +9385,7 @@
93859386 height=parseInt(height,10);
93869387 }
93879388
9388 -var callback=function(thumbnail){
 9389+var callback=function(image){
93899390
93909391 $j(selector).html(
93919392 $j('<a/>')
@@ -9392,9 +9393,11 @@
93939394 'target':'_new'})
93949395 .append(
93959396 $j('<img/>')
9396 -.attr('width',thumbnail.width)
9397 -.attr('height',thumbnail.height)
9398 -.attr('src',thumbnail.src)));
 9397+.attr({'width':image.width,
 9398+'height':image.height,
 9399+'src':image.src})
 9400+)
 9401+);
93999402 };
94009403
94019404 $j(selector).loadingSpinner();
@@ -9509,27 +9512,34 @@
95109513
95119514
95129515
 9516+
95139517 showIndicator:function(statusClass){
9514 -var _this=this;
9515 -var $indicator=$j(_this.div).find('.mwe-upwiz-file-indicator');
 9518+var $indicator=$j(this.div).find('.mwe-upwiz-file-indicator');
 9519+
95169520 $j.each($indicator.attr('class').split(/\s+/),function(i,className){
95179521 if(className.match(/^mwe-upwiz-status/)){
95189522 $indicator.removeClass(className);
95199523 }
95209524 });
9521 -$indicator.addClass('mwe-upwiz-status-'+statusClass);
95229525
9523 -$j(_this.div).find('.mwe-upwiz-visible-file-filename')
9524 -.css('margin-right',($indicator.outerWidth()+24).toString()+'px');
9525 -$indicator.css('visibility','visible');
 9526+$indicator.addClass('mwe-upwiz-status-'+statusClass)
 9527+.css('visibility','visible');
95269528 },
95279529
9528 -setPreview:function($img){
95299530
9530 -$j(this.div).find('.mwe-upwiz-file-preview').css('background-image','url('+$img.attr('src')+')');
 9531+
 9532+
 9533+
 9534+setPreview:function(image){
 9535+
 9536+$j(this.div).find('.mwe-upwiz-file-preview').css('background-image','url('+image.src+')');
95319537 },
95329538
95339539
 9540+
 9541+
 9542+
 9543+
95349544 setStatus:function(msgKey,args){
95359545 if(!mw.isDefined(args)){
95369546 args=[];
@@ -9537,10 +9547,17 @@
95389548 this.setStatusStr(gM(msgKey,args));
95399549 },
95409550
 9551+
 9552+
 9553+
 9554+
95419555 setStatusStr:function(str){
95429556 $j(this.div).find('.mwe-upwiz-file-status').html(str).show();
95439557 },
95449558
 9559+
 9560+
 9561+
95459562 clearStatus:function(){
95469563 $j(this.div).find('.mwe-upwiz-file-status').hide();
95479564 },

Status & tagging log