Make categories behave

The way categories were handled made it impossible to add anything
after them (they always ended up as the last child of the parent
container). This commit fixes that, and also moves the repo link
behind them (as required by #270).

Change-Id: I7c561c43897054e60028bd524d8ad5ea85f39e36

resources/mmv/ui/mmv.ui.categories.js

@@ -26,10 +26,12 @@
 	function Categories( $container ) {
 		mw.mmv.ui.Element.call( this, $container );
 
-		this.$categoryTpl = $( '<span>')
+		this.$categoryTpl = $( '<span>').addClass( 'mw-mmv-tag' )
 			.append( $( '<span>').addClass( 'comma-container' )
 				.text( mw.message( 'comma-separator' ) ) )
 			.append( '<a>' );
+
+		this.$categories = $( '<span>' ).appendTo( this.$container );
 	}
 
 	oo.inheritClass( Categories, mw.mmv.ui.Element );
@@ -42,7 +44,7 @@
 	Categories.prototype.set = function ( articlePath, categories ) {
 		var i, cat, href, $category;
 
-		this.empty();
+		this.$categories.empty();
 
 		if ( !categories || !categories.length ) {
 			return;
@@ -57,11 +59,6 @@
 		for ( i = 0; i < categories.length; i++ ) {
 			cat = categories[i];
 
-			if ( !this.$categories ) {
-				this.$categories = $( '<li>' ).addClass( 'mw-mmv-image-category' )
-					.appendTo( this.$container );
-			}
-
 			href = articlePath.replace( '$1', 'Category:' + cat );
 
 			$category = this.$categoryTpl
@@ -83,10 +80,7 @@
 	 * @inheritdoc
 	 */
 	Categories.prototype.empty = function () {
-		if ( this.$categories ) {
+		this.$categories.empty();
-			this.$categories.remove();
-			this.$categories = undefined;
-		}
 	};
 
 	mw.mmv.ui.Categories = Categories;

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

@@ -251,13 +251,13 @@
 			.addClass( 'mw-mmv-image-links' )
 			.appendTo( this.$imageLinkDiv );
 
-		this.initializeRepoLink();
 		this.initializeUploader();
 		this.initializeDatetime();
 		this.initializeLocation();
+		this.initializeCategories();
+		this.initializeRepoLink();
 
 		this.fileReuse = new mw.mmv.ui.reuse.Dialog( this.$container, this.buttons.buttons.$reuse );
-		this.categories = new mw.mmv.ui.Categories( this.$imageLinks );
 
 		this.fileUsage = new mw.mmv.ui.FileUsage(
 			$( '<div>' ).appendTo( this.$imageMetadataRight )
@@ -265,41 +265,6 @@
 		this.fileUsage.init();
 	};
 
-	/**
-	 * Initializes the link to the file page on the (maybe remote) repository.
-	 */
-	MPP.initializeRepoLink = function () {
-		this.$repoLi = $( '<li>' )
-			.addClass( 'mw-mmv-repo-li empty' )
-			.appendTo( this.$imageLinks );
-
-		this.$repo = $( '<a>' )
-			.addClass( 'mw-mmv-repo' )
-			.prop( 'href', '#' )
-			.click( function ( e ) {
-				var $link = $( this ),
-					redirect;
-
-				if ( e.altKey || e.shiftKey || e.ctrlKey || e.metaKey || e.button === 1 ) {
-					// They are likely opening the link in a new window or tab
-					mw.mmv.logger.log( 'site-link-click' );
-					return;
-				}
-
-				// If it's a plain click, we need to wait for the logging to
-				// be done before navigating to the desired page
-				e.preventDefault();
-
-				redirect = function () {
-					window.location.href = $link.prop( 'href' );
-				};
-
-				// We want to redirect anyway, whether logging worked or not
-				mw.mmv.logger.log( 'site-link-click' ).then( redirect, redirect );
-			} )
-			.appendTo( this.$repoLi );
-	};
-
 	/**
 	 * Initializes the upload date/time element.
 	 */
@@ -340,6 +305,52 @@
 			.appendTo( this.$locationLi );
 	};
 
+	/**
+	 * Initializes the list of categories of the image
+	 */
+	MPP.initializeCategories = function () {
+		this.categories = new mw.mmv.ui.Categories(
+			$( '<li>' )
+				.addClass( 'mw-mmv-image-category' )
+				.appendTo( this.$imageLinks )
+		);
+	};
+
+	/**
+	 * Initializes the link to the file page on the (maybe remote) repository.
+	 */
+	MPP.initializeRepoLink = function () {
+		this.$repoLi = $( '<li>' )
+			.addClass( 'mw-mmv-repo-li empty' )
+			.appendTo( this.$imageLinks );
+
+		this.$repo = $( '<a>' )
+			.addClass( 'mw-mmv-repo' )
+			.prop( 'href', '#' )
+			.click( function ( e ) {
+				var $link = $( this ),
+					redirect;
+
+				if ( e.altKey || e.shiftKey || e.ctrlKey || e.metaKey || e.button === 1 ) {
+					// They are likely opening the link in a new window or tab
+					mw.mmv.logger.log( 'site-link-click' );
+					return;
+				}
+
+				// If it's a plain click, we need to wait for the logging to
+				// be done before navigating to the desired page
+				e.preventDefault();
+
+				redirect = function () {
+					window.location.href = $link.prop( 'href' );
+				};
+
+				// We want to redirect anyway, whether logging worked or not
+				mw.mmv.logger.log( 'site-link-click' ).then( redirect, redirect );
+			} )
+			.appendTo( this.$repoLi );
+	};
+
 	/**
 	 * Initializes two about links at the bottom of the panel.
 	 */

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

@@ -2,7 +2,8 @@
 	QUnit.module( 'mmv.ui.categories', QUnit.newMwEnvironment() );
 
 	QUnit.test( 'Sanity test, object creation and UI construction', 2, function ( assert ) {
-		var categories = new mw.mmv.ui.Categories( $( '<ul>' ).appendTo( $( '#qunit-fixture' ) ) );
+		var $list = $( '<li>' ).appendTo( $( '#qunit-fixture' ) ).wrap( '<ul>' ),
+			categories = new mw.mmv.ui.Categories( $list );
 
 		assert.ok( categories, 'Image categories UI element is created' );
 		assert.strictEqual( categories.$categoryTpl.length, 1, 'Image category template is created' );
@@ -11,13 +12,16 @@
 	QUnit.test( 'Setting data in different combinations works well', 17, function ( assert ) {
 		var i,
 			j = 0,
-			categories = new mw.mmv.ui.Categories( $( '<ul>' ).appendTo( $( '#qunit-fixture' ) ) ),
+			$list = $( '<li>' ).appendTo( $( '#qunit-fixture' ) ).wrap( '<ul>' ),
+			categories = new mw.mmv.ui.Categories( $list ),
 			categoryNames = [ 'Foo', undefined, null, 'Bar', 'Baz', '', 'Quux' ],
-			categoryName;
+			categoryName,
+			$category;
 
 		categories.set( 'http://example.net/wiki/$1', categoryNames );
 
-		assert.strictEqual( $( '.mw-mmv-image-category > span' ).length, 4, 'Only valid categories have an element created for them' );
+		assert.strictEqual( categories.$container.find( '.mw-mmv-tag' ).length, 4,
+			'Only valid categories have an element created for them' );
 
 		for ( i = 0; i < categoryNames.length; i ++ ) {
 			categoryName = categoryNames[ i ];
@@ -25,30 +29,34 @@
 			if ( !categoryName || !categoryName.length ) {
 				continue;
 			}
+			$category = categories.$container.find( '.mw-mmv-tag' ).eq( j );
 
-			assert.strictEqual( $( '.mw-mmv-image-category > span' ).eq( j ).find( '.comma-container' ).length, j > 0 ? 1 : 0, 'Comma is properly set inside of them' );
+			assert.strictEqual( $category.find( '.comma-container' ).length, j > 0 ? 1 : 0,
-			assert.strictEqual( $( '.mw-mmv-image-category > span' ).eq( j ).find( 'a' ).text(), categoryName, 'Category elements have correct text inside of them' );
+				'Comma is properly set inside of them' );
-			assert.strictEqual( $( '.mw-mmv-image-category > span' ).eq( j ).find( 'a' ).prop( 'href' ), 'http://example.net/wiki/Category:' + categoryName, 'Category links are set to the right target' );
+			assert.strictEqual( $category.find( 'a' ).text(), categoryName,
+				'Category elements have correct text inside of them' );
+			assert.strictEqual( $category.find( 'a' ).prop( 'href' ),
+				'http://example.net/wiki/Category:' + categoryName,
+				'Category links are set to the right target' );
 
 			j++;
 		}
 
 		for ( i = 0; i < 3; i ++ ) {
-			assert.ok( !$( '.mw-mmv-image-category > span' ).eq( i ).hasClass( 'extra' ), 'Categories before the fourth are not marked as extra' );
+			assert.ok( !categories.$container.find( '.mw-mmv-tag' ).eq( i ).hasClass( 'extra' ),
+				'Categories before the fourth are not marked as extra' );
 		}
 
-		assert.ok( $( '.mw-mmv-image-category > span' ).eq( 3 ).hasClass( 'extra' ), 'Categories after the third are marked as extra' );
+		assert.ok( categories.$container.find( '.mw-mmv-tag' ).eq( 3 ).hasClass( 'extra' ),
+			'Categories after the third are marked as extra' );
 	} );
 
-	QUnit.test( 'Emptying data works as expected', 4, function ( assert ) {
+	QUnit.test( 'Emptying data works as expected', 1, function ( assert ) {
-		var $list = $( '<ul>' ).appendTo( $( '#qunit-fixture' ) ),
+		var $list = $( '<li>' ).appendTo( $( '#qunit-fixture' ) ).wrap( '<ul>' ),
 			categories = new mw.mmv.ui.Categories( $list );
 
 		categories.set( 'http://example.net/wiki/$1', [ 'Foo', 'Bar', 'Baz', 'Quux' ] );
 		categories.empty();
-		assert.strictEqual( $( '.mw-mmv-image-category > span' ).length, 0, 'All elements are removed from the DOM' );
 		assert.strictEqual( $list.text(), '', 'Text is emptied correctly' );
-		assert.strictEqual( $list.find( 'li' ).length, 0, 'List elements are all removed' );
-		assert.strictEqual( categories.$categories, undefined, 'Category UI element is removed from object' );
 	} );
 }( mediaWiki, jQuery ) );