From 4a80ce79db868c8ad7ecb392549a4466b0a8e2cd Mon Sep 17 00:00:00 2001
From: Ed Sanders <esanders@wikimedia.org>
Date: Fri, 19 Apr 2019 13:08:38 +0100
Subject: [PATCH] Gallery slideshow: Code cleanup

* Use $-prefix jQuery vars
* Use #connect for OOUI events
* Some jQuery cleanups
* Remove unused return valu of setImageSize
* Only resolve one value in loadImage
* Doc fixes

Change-Id: I0ff4252300aa02c228577961a7d1ede3b9628d90
---
 .../src/mediawiki.page.gallery.slideshow.js   | 64 ++++++++++---------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/resources/src/mediawiki.page.gallery.slideshow.js b/resources/src/mediawiki.page.gallery.slideshow.js
index 71b3dfe6e1..b62b45e256 100644
--- a/resources/src/mediawiki.page.gallery.slideshow.js
+++ b/resources/src/mediawiki.page.gallery.slideshow.js
@@ -105,34 +105,34 @@
 	 * Draws the carousel and the interface around it.
 	 */
 	mw.GallerySlideshow.prototype.drawCarousel = function () {
-		var next, prev, toggle, interfaceElements, carouselStack;
+		var nextButton, prevButton, toggleButton, interfaceElements, carouselStack;
 
 		this.$carousel = $( '<li>' ).addClass( 'gallerycarousel' );
 
 		// Buttons for the interface
-		prev = new OO.ui.ButtonWidget( {
+		prevButton = new OO.ui.ButtonWidget( {
 			framed: false,
 			icon: 'previous'
-		} ).on( 'click', this.prevImage.bind( this ) );
+		} ).connect( this, { click: 'prevImage' } );
 
-		next = new OO.ui.ButtonWidget( {
+		nextButton = new OO.ui.ButtonWidget( {
 			framed: false,
 			icon: 'next'
-		} ).on( 'click', this.nextImage.bind( this ) );
+		} ).connect( this, { click: 'nextImage' } );
 
-		toggle = new OO.ui.ButtonWidget( {
+		toggleButton = new OO.ui.ButtonWidget( {
 			framed: false,
 			icon: 'imageGallery',
 			title: mw.msg( 'gallery-slideshow-toggle' )
-		} ).on( 'click', this.toggleThumbnails.bind( this ) );
+		} ).connect( this, { click: 'toggleThumbnails' } );
 
 		interfaceElements = new OO.ui.PanelLayout( {
 			expanded: false,
 			classes: [ 'mw-gallery-slideshow-buttons' ],
 			$content: $( '<div>' ).append(
-				prev.$element,
-				toggle.$element,
-				next.$element
+				prevButton.$element,
+				toggleButton.$element,
+				nextButton.$element
 			)
 		} );
 		this.$interface = interfaceElements.$element;
@@ -203,12 +203,10 @@
 	 * Sets the height and width of {@link #$img} based on the
 	 * proportion of the image and the values generated by
 	 * {@link #setSizeRequirement}.
-	 *
-	 * @return {boolean} Whether or not the image was sized.
 	 */
 	mw.GallerySlideshow.prototype.setImageSize = function () {
 		if ( this.$img === undefined || this.$thumbnail === undefined ) {
-			return false;
+			return;
 		}
 
 		// Reset height and width
@@ -228,50 +226,54 @@
 				info.thumbwidth < this.$img.width() ||
 				info.thumbheight < this.$img.height()
 			) {
-				this.$img.attr( 'width', info.thumbwidth + 'px' );
-				this.$img.attr( 'height', info.thumbheight + 'px' );
+				this.$img.attr( {
+					width: info.thumbwidth + 'px',
+					height: info.thumbheight + 'px'
+				} );
 			}
 		}.bind( this ) );
-
-		return true;
 	};
 
 	/**
 	 * Displays the image set as {@link #$currentImage} in the carousel.
 	 */
 	mw.GallerySlideshow.prototype.showCurrentImage = function () {
-		var imageLi = this.getCurrentImage(),
-			caption = imageLi.find( '.gallerytext' );
+		var $thumbnail,
+			$imageLi = this.getCurrentImage(),
+			$caption = $imageLi.find( '.gallerytext' );
 
 		// The order of the following is important for size calculations
 		// 1. Highlight current thumbnail
 		this.$gallery
 			.find( '.gallerybox.slideshow-current' )
 			.removeClass( 'slideshow-current' );
-		imageLi.addClass( 'slideshow-current' );
+		$imageLi.addClass( 'slideshow-current' );
 
 		// 2. Show thumbnail
-		this.$thumbnail = imageLi.find( 'img' );
-		this.$img.attr( 'src', this.$thumbnail.attr( 'src' ) );
-		this.$img.attr( 'alt', this.$thumbnail.attr( 'alt' ) );
-		this.$imgLink.attr( 'href', imageLi.find( 'a' ).eq( 0 ).attr( 'href' ) );
+		this.$thumbnail = $imageLi.find( 'img' );
+		this.$img.attr( {
+			src: this.$thumbnail.attr( 'src' ),
+			alt: this.$thumbnail.attr( 'alt' )
+		} );
+		this.$imgLink.attr( 'href', $imageLi.find( 'a' ).eq( 0 ).attr( 'href' ) );
 
 		// 3. Copy caption
 		this.$imgCaption
 			.empty()
-			.append( caption.clone() );
+			.append( $caption.clone() );
 
 		// 4. Stretch thumbnail to correct size
 		this.setImageSize();
 
+		$thumbnail = this.$thumbnail;
 		// 5. Load image at the required size
-		this.loadImage( this.$thumbnail ).done( function ( info, $img ) {
+		this.loadImage( this.$thumbnail ).done( function ( info ) {
 			// Show this image to the user only if its still the current one
-			if ( this.$thumbnail.attr( 'src' ) === $img.attr( 'src' ) ) {
+			if ( this.$thumbnail.attr( 'src' ) === $thumbnail.attr( 'src' ) ) {
 				this.$img.attr( 'src', info.thumburl );
 				this.setImageSize();
 
-				// Keep the next image ready
+				// Pre-fetch the next image
 				this.loadImage( this.getNextImage().find( 'img' ) );
 			}
 		}.bind( this ) );
@@ -280,9 +282,9 @@
 	/**
 	 * Loads the full image given the `<img>` element of the thumbnail.
 	 *
-	 * @param {Object} $img
+	 * @param {jQuery} $img
 	 * @return {jQuery.Promise} Resolves with the images URL and original
-	 *   element once the image has loaded.
+	 *  element once the image has loaded.
 	 */
 	mw.GallerySlideshow.prototype.loadImage = function ( $img ) {
 		var img, d = $.Deferred();
@@ -291,7 +293,7 @@
 			img = new Image();
 			img.src = info.thumburl;
 			img.onload = function () {
-				d.resolve( info, $img );
+				d.resolve( info );
 			};
 			img.onerror = function () {
 				d.reject();
-- 
2.20.1