From 5d47d1da3d5ba60dc1f27da84855a298e8dae9a7 Mon Sep 17 00:00:00 2001 From: Gilles Dubuc Date: Fri, 31 Jan 2014 09:30:26 +0100 Subject: [PATCH] Fix the "view all uses" link when there are both local and global results The code was incorrectly adding the link to the first section no matter what. This wasn't caught by the existing assertions, so these are being improved as well. Change-Id: I4dcbf44b504bd4cb875b4058fe5604f2a3c871b7 Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/44 --- resources/mmv/mmv.ui.fileUsage.js | 18 ++++++---- tests/qunit/mmv.ui.fileUsage.test.js | 52 +++++++++++++++++----------- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/resources/mmv/mmv.ui.fileUsage.js b/resources/mmv/mmv.ui.fileUsage.js index 7bc48cbbc..11721c53c 100644 --- a/resources/mmv/mmv.ui.fileUsage.js +++ b/resources/mmv/mmv.ui.fileUsage.js @@ -105,14 +105,17 @@ * @protected */ FileUsage.prototype.addSection = function( fileUsage, sectionType, limit, viewAllLink ) { + var $section; + if ( fileUsage.totalCount ) { - this.$usageList.append( - $( '
  • ' ).addClass( 'mw-mlb-fileusage-' + sectionType + '-section' ) - .msg( 'multimediaviewer-fileusage-' + sectionType + '-section' ) - ); + $section = $( '
  • ' ).addClass( 'mw-mlb-fileusage-' + sectionType + '-section' ) + .msg( 'multimediaviewer-fileusage-' + sectionType + '-section' ); + + this.$usageList.append( $section ); this.addPageLinks( fileUsage.pages.slice( 0, limit ) ); + if ( fileUsage.pages.length > limit ) { - this.addViewAllLink( viewAllLink ); + this.addViewAllLink( $section, viewAllLink ); } } }; @@ -142,11 +145,12 @@ /** * Adds a 'View all' link (with the given URL) to the end of the usage list. + * @param {jQuery} $section file usage section element * @param {string} url * @protected */ - FileUsage.prototype.addViewAllLink = function( url ) { - this.$usageList.find( 'li:first' ).append( + FileUsage.prototype.addViewAllLink = function( $section, url ) { + $section.append( $( '' ).addClass( 'mw-mlb-fileusage-view-all' ).append( $( '' ).msg( 'multimediaviewer-fileusage-link' ) .attr( 'href', url ) diff --git a/tests/qunit/mmv.ui.fileUsage.test.js b/tests/qunit/mmv.ui.fileUsage.test.js index 2bde2aa07..39f4b6ab1 100644 --- a/tests/qunit/mmv.ui.fileUsage.test.js +++ b/tests/qunit/mmv.ui.fileUsage.test.js @@ -13,7 +13,7 @@ assert.ok( $( '#qunit-fixture' ).hasClass( 'empty' ) ); } ); - QUnit.test( 'File usage panel with local usage', 7, function( assert ) { + QUnit.test( 'File usage panel with local usage', 8, function( assert ) { var $list, fileUsage = new mw.mmv.ui.FileUsage( $( '#qunit-fixture' ) ), file = new mw.Title( 'File:Foo' ), @@ -26,19 +26,20 @@ fileUsage.init(); fileUsage.set( localUsage, globalUsage ); - assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ) ); + assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ), 'The container is not empty' ); $list = $( '#qunit-fixture li:not([class])' ); assert.strictEqual( $list.length, 2 ); assert.strictEqual( $list.eq( 0 ).text(), 'Bar' ); assert.strictEqual( $list.eq( 1 ).text(), 'Baz' ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section' ).length, 1 ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section' ).length, 0 ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-view-all' ).length, 0 ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section' ).length, 1, 'There is a local section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section' ).length, 0, 'There is no global section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section .mw-mlb-fileusage-view-all' ).length, 0, 'The local section has no "view all uses" link' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section .mw-mlb-fileusage-view-all' ).length, 0, 'The global section has no "view all uses" link' ); } ); - QUnit.test( 'File usage panel with local usage and overflow', 3, function( assert ) { + QUnit.test( 'File usage panel with local usage and overflow', 6, function( assert ) { var $list, fileUsage = new mw.mmv.ui.FileUsage( $( '#qunit-fixture' ) ), file = new mw.Title( 'File:Foo' ), @@ -53,14 +54,17 @@ fileUsage.init(); fileUsage.set( localUsage, globalUsage ); - assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ) ); + assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ), 'The container is not empty' ); $list = $( '#qunit-fixture li:not([class])' ); assert.strictEqual( $list.length, fileUsage.MAX_LOCAL ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-view-all' ).length, 1 ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section' ).length, 1, 'There is a local section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section' ).length, 0, 'There is no global section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section .mw-mlb-fileusage-view-all' ).length, 1, 'The local section has its "view all uses" link' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section .mw-mlb-fileusage-view-all' ).length, 0, 'The global section has no "view all uses" link' ); } ); - QUnit.test( 'File usage panel with global usage', 9, function( assert ) { + QUnit.test( 'File usage panel with global usage', 10, function( assert ) { var $list, fileUsage = new mw.mmv.ui.FileUsage( $( '#qunit-fixture' ) ), file = new mw.Title( 'File:Foo' ), @@ -73,7 +77,7 @@ fileUsage.init(); fileUsage.set( localUsage, globalUsage ); - assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ) ); + assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ), 'The container is not empty' ); $list = $( '#qunit-fixture li:not([class])' ); assert.strictEqual( $list.length, 2 ); @@ -82,12 +86,13 @@ assert.strictEqual( $list.eq( 0 ).find( 'aside' ).text(), 'x.com' ); assert.strictEqual( $list.eq( 1 ).find( 'aside' ).text(), 'y.com' ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section' ).length, 0 ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section' ).length, 1 ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-view-all' ).length, 0 ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section' ).length, 0, 'There is no local section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section' ).length, 1, 'There is a global section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section .mw-mlb-fileusage-view-all' ).length, 0, 'The local section has no "view all uses" link' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section .mw-mlb-fileusage-view-all' ).length, 0, 'The global section has no "view all uses" link' ); } ); - QUnit.test( 'File usage panel with lots of uses', 3, function( assert ) { + QUnit.test( 'File usage panel with lots of uses', 7, function( assert ) { var $list, totalCount = 100, fileUsage = new mw.mmv.ui.FileUsage( $( '#qunit-fixture' ) ), @@ -108,10 +113,15 @@ fileUsage.init(); fileUsage.set( localUsage, globalUsage ); + assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ), 'The container is not empty' ); + $list = $( '#qunit-fixture li:not([class])' ); - assert.strictEqual( $list.length, fileUsage.MAX_LOCAL + fileUsage.MAX_GLOBAL ); - assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-view-all' ).length, 2 ); - assert.ok( $( '#qunit-fixture h3' ).text().match( totalCount ) ); + assert.strictEqual( $list.length, fileUsage.MAX_LOCAL + fileUsage.MAX_GLOBAL, 'Total amount of results is correctly capped' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section' ).length, 1, 'There is a local section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section' ).length, 1, 'There is a global section' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-local-section .mw-mlb-fileusage-view-all' ).length, 1, 'The local section has its "view all uses" link' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-global-section .mw-mlb-fileusage-view-all' ).length, 1, 'The global section has its "view all uses" link' ); + assert.ok( $( '#qunit-fixture h3' ).text().match( totalCount ), 'The "Used in" counter has the right total' ); } ); QUnit.test( 'The interface is emptied properly when necessary', 4, function( assert ) { @@ -127,15 +137,15 @@ fileUsage.init(); fileUsage.set( localUsage, globalUsage ); - assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ) ); + assert.ok( ! $( '#qunit-fixture' ).hasClass( 'empty' ), 'The container is not empty' ); fileUsage.empty(); - assert.ok( $( '#qunit-fixture' ).hasClass( 'empty' ) ); + assert.ok( $( '#qunit-fixture' ).hasClass( 'empty' ), 'The container is empty' ); $list = $( '#qunit-fixture li:not([class])' ); - assert.strictEqual( $list.length, 0 ); + assert.strictEqual( $list.length, 0, 'The list is empty' ); - assert.strictEqual( $( '#qunit-fixture h3' ).text(), '' ); + assert.strictEqual( $( '#qunit-fixture .mw-mlb-fileusage-container h3' ).text(), '', 'The "Used in" counter is empty' ); } ); }( mediaWiki, jQuery ) );