Bugfixes and improvements for the progress bar

- the bar now starts at 5% for a visual indication that
something is going on
- the bar animates (fast) to 100%, instead of
disappearing immediately
- the animation logic has been fixed to avoid seeing
the bar go backwards
- added sanity check in all the callbacks to make sure
that we don't apply any changes to an image we are
not looking at anymore (including progress updates)

Bug: 58055
Change-Id: I765a61c16513e9330a412c5ec96387623ae7dbc7
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/146
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/242

resources/mmv/mmv.js

@@ -245,28 +245,62 @@
 
 		start = $.now();
 
-		imagePromise = this.fetchThumbnail(
-			image.filePageTitle, imageWidths.real
-		).progress( function ( thumbnailInfoResponse, imageResponse ) {
-			if ( viewer.ui && viewer.ui.panel ) {
+		// Reset the progress bar, it could be at any state if we're calling loadImage
+		// while another image is already loading
+		viewer.ui.panel.percent( 0 );
+
+		imagePromise = this.fetchThumbnail( image.filePageTitle, imageWidths.real );
+
+		// Check that the image hasn't already been loaded
+		if ( imagePromise.state() === 'pending' ) {
+			// Animate it to 5 to give a sense to something is happening, even if we're stuck
+			// waiting for server-side processing, such as thumbnail (re)generation
+			viewer.ui.panel.percent( 5 );
+		}
+
+		imagePromise.progress( function ( thumbnailInfoResponse, imageResponse ) {
+			if ( viewer.lightbox.currentIndex !== image.index ) {
+				return;
+			}
+
+			if ( viewer.ui
+				&& viewer.ui.panel
+				&& imageResponse.length === 2
+				&& imageResponse[ 1 ] > 5 ) {
 				viewer.ui.panel.percent( imageResponse[ 1 ] );
 			}
-		} ).done( function ( thumbnail, image ) {
-			viewer.displayRealThumbnail( thumbnail, image, imageWidths, $.now() - start );
+		} ).done( function ( thumbnail, imageElement ) {
+			if ( viewer.lightbox.currentIndex !== image.index ) {
+				return;
+			}
+
+			viewer.displayRealThumbnail( thumbnail, imageElement, imageWidths, $.now() - start );
 		} );
 
 		this.imageInfoProvider.get( image.filePageTitle ).done( function ( imageInfo ) {
+			if ( viewer.lightbox.currentIndex !== image.index ) {
+				return;
+			}
+
 			viewer.displayPlaceholderThumbnail( imageInfo, image, $initialImage, imageWidths );
 		} );
 
 		metadataPromise = this.fetchSizeIndependentLightboxInfo(
 			image.filePageTitle
 		).done( function ( imageInfo, repoInfo, localUsage, globalUsage, userInfo ) {
+			if ( viewer.lightbox.currentIndex !== image.index ) {
+				return;
+			}
+
 			viewer.lightbox.iface.panel.setImageInfo(image, imageInfo, repoInfo,
 				localUsage, globalUsage, userInfo );
 		} );
 
 		$.when( imagePromise, metadataPromise ).then( function() {
+			if ( viewer.lightbox.currentIndex !== image.index ) {
+				return;
+			}
+
 			viewer.stopListeningToScroll();
 			viewer.animateMetadataDivOnce()
 				// We need to wait until the animation is finished before we listen to scroll

resources/mmv/mmv.lightboxinterface.js

@@ -54,11 +54,6 @@
 			addToPost = [],
 			lbinterface = this;
 
-		// Staging area for image resizes
-		this.$staging = $( '<div>' )
-			.addClass( 'mlb-staging-area' );
-		$( document.body ).append( this.$staging );
-
 		// SVG filter, needed to achieve blur in Firefox
 		this.$filter = $( '<svg><filter id="gaussian-blur"><fegaussianblur stdDeviation="3"></filter></svg>' );
 
@@ -267,7 +262,7 @@
 
 		this.$image = $imageElement;
 
-		this.autoResizeImage();
+		this.$imageDiv.html( this.$image );
 
 		// Capture listener so we can remove it later, otherwise
 		// we are going to leak listeners !
@@ -319,14 +314,6 @@
 		this.handleEvent( 'mmv-fade-stopped', function ( e ) { ui.fadeStopped( e ); } );
 	};
 
-	/**
-	 * FIXME refactor and document
-	 */
-	LIP.autoResizeImage = function () {
-		this.$staging.append( this.$image );
-		this.$imageDiv.append( this.$image );
-	};
-
 	/**
 	 * Changes what image is being displayed.
 	 * @param {HTMLImageElement} imageEle

resources/mmv/ui/mmv.ui.metadataPanel.js

@@ -738,12 +738,27 @@
 	 * @param {number} percent
 	 */
 	MPP.percent = function ( percent ) {
-		if ( percent < 100 ) {
-			this.$percent.animate( { width : percent + '%' } );
-			this.$progress.removeClass( 'empty' );
-		} else {
-			this.$percent.css( { width : 0 } );
+		var panel = this;
+
+		if ( percent === 0 ) {
+			// When a 0% update comes in, we jump without animation to 0 and we hide the bar
 			this.$progress.addClass( 'empty' );
+			this.$percent.stop().css( { width : 0 } );
+		} else if ( percent === 100 ) {
+			// When a 100% update comes in, we make sure that the bar is visible, we animate
+			// fast to 100 and we hide the bar when the animation is done
+			this.$progress.removeClass( 'empty' );
+			this.$percent.stop().animate( { width : percent + '%' }, 50, 'swing',
+				function () {
+					// Reset the position for good measure
+					panel.$percent.stop().css( { width : 0 } );
+					panel.$progress.addClass( 'empty' );
+				} );
+		} else {
+			// When any other % update comes in, we make sure the bar is visible
+			// and we animate to the right position
+			this.$progress.removeClass( 'empty' );
+			this.$percent.stop().animate( { width : percent + '%' } );
 		}
 	};
 

tests/qunit/mmv/mmv.test.js

@@ -153,12 +153,13 @@
 		window.location.hash = '';
 	} );
 
-	QUnit.test( 'Progress', 1, function ( assert ) {
+	QUnit.test( 'Progress', 3, function ( assert ) {
 		var imageDeferred = $.Deferred(),
 			viewer = new mw.mmv.MultimediaViewer(),
 			oldImageGet = mw.mmv.provider.Image.prototype.get,
 			oldImageInfoGet = mw.mmv.provider.ImageInfo.prototype.get,
-			oldThumbnailInfoGet = mw.mmv.provider.ThumbnailInfo.prototype.get;
+			oldThumbnailInfoGet = mw.mmv.provider.ThumbnailInfo.prototype.get,
+			i = 0;
 
 		viewer.thumbs = [];
 		viewer.displayPlaceholderThumbnail = $.noop;
@@ -172,8 +173,18 @@
 			getCurrentImageWidths : function () { return { real : 0 }; },
 			panel : { setImageInfo : $.noop,
 				percent : function ( percent ) {
-					assert.strictEqual( percent, 45,
-						'Percentage correctly funneled to panel UI' );
+					if ( i === 0 ) {
+						assert.strictEqual( percent, 0,
+							'Percentage correctly reset by loadImage' );
+					} else if ( i === 1 ) {
+						assert.strictEqual( percent, 5,
+							'Percentage correctly animated to 5 by loadImage' );
+					} else {
+						assert.strictEqual( percent, 45,
+							'Percentage correctly funneled to panel UI' );
+					}
+
+					i++;
 				} }
 		},
 		open : $.noop };
@@ -376,4 +387,76 @@
 		assert.ok( !$image.hasClass( 'blurred' ), 'Placeholder is not blurred' );
 		assert.ok( !viewer.blurredThumbnailShown, 'Placeholder state is correct' );
 	} );
+
+	QUnit.test( 'New image loaded while another one is loading', 1, function ( assert ) {
+		var imageDeferred,
+			ligthboxInfoDeferred,
+			viewer = new mw.mmv.MultimediaViewer(),
+			oldImageGet = mw.mmv.provider.Image.prototype.get,
+			oldImageInfoGet = mw.mmv.provider.ImageInfo.prototype.get,
+			oldThumbnailInfoGet = mw.mmv.provider.ThumbnailInfo.prototype.get,
+			firstImageDeferred = $.Deferred(),
+			secondImageDeferred = $.Deferred(),
+			firstLigthboxInfoDeferred = $.Deferred(),
+			secondLigthboxInfoDeferred = $.Deferred();
+
+		viewer.preloadFullscreenThumbnail = $.noop;
+		viewer.fetchSizeIndependentLightboxInfo = function () { return ligthboxInfoDeferred.promise(); };
+		viewer.lightbox = { iface : {
+			setupForLoad : $.noop,
+			showImage : $.noop,
+			getCurrentImageWidths : function () { return { real : 0 }; },
+			panel : { setImageInfo : function () {
+					assert.ok( false, 'Metadata of the first image should not be shown' );
+				},
+				percent : function ( response, percent ) {
+					if ( percent === 45 ) {
+						assert.ok( false, 'Progress of the first image should not be shown' );
+					}
+				}  },
+			empty: $.noop
+		},
+		open : $.noop };
+		viewer.eachPrealoadableLightboxIndex = $.noop;
+		viewer.animateMetadataDivOnce = function () {
+			assert.ok( false, 'Metadata of the first image should not be animated' );
+			return $.Deferred().reject();
+		};
+
+		mw.mmv.provider.Image.prototype.get = function() { return imageDeferred.promise(); };
+		mw.mmv.provider.ImageInfo.prototype.get = function() { return $.Deferred().reject(); };
+		mw.mmv.provider.ThumbnailInfo.prototype.get = function() { return $.Deferred().resolve( {} ); };
+
+		imageDeferred = firstImageDeferred;
+		ligthboxInfoDeferred = firstLigthboxInfoDeferred;
+		viewer.loadImage( { filePageTitle : new mw.Title( 'File:Foo.jpg' ), index : 0 }, new Image() );
+
+		imageDeferred = secondImageDeferred;
+		ligthboxInfoDeferred = secondLigthboxInfoDeferred;
+		viewer.loadImage( { filePageTitle : new mw.Title( 'File:Bar.jpg' ), index : 1 }, new Image() );
+
+		viewer.displayRealThumbnail = function () {
+			assert.ok( false, 'The first image being done loading should have no effect');
+		};
+
+		firstImageDeferred.notify( undefined, 45 );
+		firstImageDeferred.resolve();
+		firstLigthboxInfoDeferred.resolve();
+
+		viewer.lightbox.iface.panel.setImageInfo = $.noop;
+		viewer.animateMetadataDivOnce = function() { return $.Deferred().reject(); };
+
+		viewer.displayRealThumbnail = function () {
+			assert.ok( true, 'The second image being done loading should result in the image being shown');
+		};
+
+		secondImageDeferred.resolve();
+		secondLigthboxInfoDeferred.resolve();
+
+		viewer.close();
+
+		mw.mmv.provider.Image.prototype.get = oldImageGet;
+		mw.mmv.provider.ImageInfo.prototype.get = oldImageInfoGet;
+		mw.mmv.provider.ThumbnailInfo.prototype.get = oldThumbnailInfoGet;
+	} );
 }( mediaWiki, jQuery ) );

tests/qunit/mmv/ui/mmv.ui.metadataPanel.test.js

@@ -195,28 +195,44 @@
 		assert.strictEqual( result, date1, 'Invalid date is correctly ignored' );
 	} );
 
-	QUnit.test( 'Progress bar', 8, function ( assert ) {
+	QUnit.test( 'Progress bar', 10, function ( assert ) {
 		var $qf = $( '#qunit-fixture' ),
 			panel = new mw.mmv.ui.MetadataPanel( $qf, $( '<div>' ).appendTo( $qf ) ),
 			oldAnimate = $.fn.animate;
 
-		$.fn.animate = function ( target ) {
-			$( this ).css( target );
-		};
-
 		assert.ok( panel.$progress.hasClass( 'empty' ), 'Progress bar is hidden' );
 		assert.strictEqual( panel.$percent.width(), 0, 'Progress bar\'s indicator is at 0' );
 
-		panel.percent( 0 );
+		$.fn.animate = function ( target ) {
+			$( this ).css( target );
 
-		assert.ok( !panel.$progress.hasClass( 'empty' ), 'Progress bar is visible' );
-		assert.strictEqual( panel.$percent.width(), 0, 'Progress bar\'s indicator is at 0' );
+			assert.strictEqual( target.width, '50%', 'Animation should go to 50%' );
+		};
 
 		panel.percent( 50 );
 
 		assert.ok( !panel.$progress.hasClass( 'empty' ), 'Progress bar is visible' );
 		assert.strictEqual( panel.$percent.width(), $qf.width() / 2, 'Progress bar\'s indicator is at half' );
 
+		$.fn.animate = function () {
+			assert.ok( false, 'Going to 0% should not animate' );
+		};
+
+		panel.percent( 0 );
+
+		assert.ok( panel.$progress.hasClass( 'empty' ), 'Progress bar is hidden' );
+		assert.strictEqual( panel.$percent.width(), 0, 'Progress bar\'s indicator is at 0' );
+
+		$.fn.animate = function ( target, duration, transition, callback ) {
+			$( this ).css( target );
+
+			assert.strictEqual( target.width, '100%', 'Animation should go to 100%' );
+
+			if ( callback !== undefined ) {
+				callback();
+			}
+		};
+
 		panel.percent( 100 );
 
 		assert.ok( panel.$progress.hasClass( 'empty' ), 'Progress bar is hidden' );