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

diff --git a/resources/src/mediawiki.page.gallery.slideshow.js b/resources/src/mediawiki.page.gallery.slideshow.js
index 71b3dfe..b62b45e 100644 (file)
--- 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();