From ba064c201352bb7916e0d7ecb169c393af12bd4d Mon Sep 17 00:00:00 2001
From: jdlrobson <jdlrobson@gmail.com>
Date: Thu, 26 Jul 2018 18:47:05 +0800
Subject: [PATCH] Avoid abrupt and jarring white flash in media viewer

Previously, the image overlay would always be loaded asynchronously.
Given clicking left/right opens a new mediaviewer this left a jarring
white flash. By loading the image overlay synchronously after the first
has been loaded, we avoid this flash.

Note, the task does propose preloading and animating the images
but this is a much bigger change and deemed out of scope for this
particular task.

Additional change:
* Use a shared mw.Api instance for ImageOverlay and PageGateway

Bug: T197110
Change-Id: I28d06b34cdea4fedcd7fb754572191e904ecc81a
---
 resources/skins.minerva.scripts/init.js | 53 ++++++++++++++++---------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js
index 327b06118..9c388b7b8 100644
--- a/resources/skins.minerva.scripts/init.js
+++ b/resources/skins.minerva.scripts/init.js
@@ -11,6 +11,7 @@
 		OverlayManager = M.require( 'mobile.startup/OverlayManager' ),
 		overlayManager = new OverlayManager( require( 'mediawiki.router' ) ),
 		page = M.getCurrentPage(),
+		api = new mw.Api(),
 		thumbs = page.getThumbnails();
 
 	/**
@@ -92,37 +93,53 @@
 		}
 	}
 
+	/**
+	 * Make an instance of an ImageOverlay. This function assumes that the module
+	 * providing the ImageOverlay has been asynchronously loaded.
+	 * @method
+	 * @ignore
+	 * @param {string} title Url of image
+	 * @return {ImageOverlay}
+	 */
+	function makeImageOverlay( title ) {
+		var ImageOverlay = M.require( 'mobile.mediaViewer/ImageOverlay' ),
+			imageOverlay = new ImageOverlay( {
+				api: api,
+				thumbnails: thumbs,
+				title: decodeURIComponent( title )
+			} );
+		imageOverlay.on( ImageOverlay.EVENT_EXIT, function () {
+			// Actually dismiss the overlay whenever the cross is closed.
+			window.location.hash = '';
+			// Clear the hash.
+			router.navigate( '' );
+		} );
+		imageOverlay.on( ImageOverlay.EVENT_SLIDE, function ( nextThumbnail ) {
+			routeThumbnail( nextThumbnail );
+		} );
+		return imageOverlay;
+	}
+
 	/**
 	 * Load image overlay
 	 * @method
 	 * @ignore
 	 * @uses ImageOverlay
 	 * @param {string} title Url of image
-	 * @return {JQuery.Deferred}
+	 * @return {JQuery.Deferred|ImageOverlay}
 	 */
 	function loadImageOverlay( title ) {
 		if ( mw.loader.getState( 'mmv.bootstrap' ) === 'ready' ) {
 			// This means MultimediaViewer has been installed and is loaded.
 			// Avoid loading it (T169622)
 			return $.Deferred().reject();
+		} else if ( mw.loader.getState( 'mobile.mediaViewer' ) === 'ready' ) {
+			// If module already loaded, do this synchronously to avoid the event loop causing
+			// a visible flash (see T197110)
+			return makeImageOverlay( title );
 		} else {
 			return loader.loadModule( 'mobile.mediaViewer' ).then( function () {
-				var ImageOverlay = M.require( 'mobile.mediaViewer/ImageOverlay' ),
-					imageOverlay = new ImageOverlay( {
-						api: new mw.Api(),
-						thumbnails: thumbs,
-						title: decodeURIComponent( title )
-					} );
-				imageOverlay.on( ImageOverlay.EVENT_EXIT, function () {
-					// Actually dismiss the overlay whenever the cross is closed.
-					window.location.hash = '';
-					// Clear the hash.
-					router.navigate( '' );
-				} );
-				imageOverlay.on( ImageOverlay.EVENT_SLIDE, function ( nextThumbnail ) {
-					routeThumbnail( nextThumbnail );
-				} );
-				return imageOverlay;
+				return makeImageOverlay( title );
 			} );
 		}
 	}
@@ -134,7 +151,7 @@
 
 		return loader.loadModule( 'mobile.languages.structured', true ).then( function ( loadingOverlay ) {
 			var PageGateway = M.require( 'mobile.startup/PageGateway' ),
-				gateway = new PageGateway( new mw.Api() ),
+				gateway = new PageGateway( api ),
 				LanguageOverlay = M.require( 'mobile.languages.structured/LanguageOverlay' );
 
 			return gateway.getPageLanguages( mw.config.get( 'wgPageName' ), lang ).then( function ( data ) {