jenkins-bot has submitted this change and it was merged.

Change subject: Improve embed credit line
......................................................................


Improve embed credit line

Gets the text closer to the spec:
* links have #mediaviewer (lots of code duplication, see #373)
* image has alt text (was title in the spec but that made less sense)
* less convoluted logic in getCreditHtml()

Change-Id: I43db84adb7fe29850706f92ee978016939b59aaa
Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/369
---
M MultimediaViewer.i18n.php
M resources/mmv/mmv.EmbedFileFormatter.js
M tests/qunit/mmv/mmv.EmbedFileFormatter.test.js
3 files changed, 35 insertions(+), 11 deletions(-)

Approvals:
  Gilles: Looks good to me, approved
  Siebrand: Looks good to me, but someone else must approve
  jenkins-bot: Verified



diff --git a/MultimediaViewer.i18n.php b/MultimediaViewer.i18n.php
index 1012db8..5f248b5 100644
--- a/MultimediaViewer.i18n.php
+++ b/MultimediaViewer.i18n.php
@@ -100,6 +100,7 @@
        'multimediaviewer-html-embed-credit-text-tb' => '"$1" by $2.',
        'multimediaviewer-html-embed-credit-text-ts' => '"$1". Via $2.',
        'multimediaviewer-html-embed-credit-text-tl' => '"$1". Licensed under 
$2.',
+       'multimediaviewer-html-embed-credit-text-t' => '"$1".',
        'multimediaviewer-embed-byline' => 'By $1',
        'multimediaviewer-embed-license' => 'Licensed under $1.',
        'multimediaviewer-embed-via' => 'Via $1.',
@@ -293,6 +294,11 @@
 * $1 - name of the work (typically the filename without an extension)
 * $2 - name of the license
 Each of the parameters could be either plain text or a link.',
+       'multimediaviewer-html-embed-credit-text-t' => 'Credit text, used when 
generating HTML to reuse an image.
+Which one of the multimediaviewer-html-embed-credit-text-* messages is used 
will depend on what information about the image is available.
+* $1 - name of the work (typically the filename without an extension)
+Each of the parameters could be either plain text or a link.',
+
        'multimediaviewer-embed-byline' => 'Byline (author credit) text, used 
when generating wikitext/HTML to reuse the image. $1 is author name.',
        'multimediaviewer-embed-license' => 'License information, used when 
generating wikitext/HTML to reuse the image. $1 is the license name.',
        'multimediaviewer-embed-via' => 'Source information (e. g. "via 
Flickr"), used when generating wikitext/HTML to reuse the image.
diff --git a/resources/mmv/mmv.EmbedFileFormatter.js 
b/resources/mmv/mmv.EmbedFileFormatter.js
index 8065593..67f9e41 100644
--- a/resources/mmv/mmv.EmbedFileFormatter.js
+++ b/resources/mmv/mmv.EmbedFileFormatter.js
@@ -95,11 +95,13 @@
         */
        AFP.getCreditHtml = function ( info ) {
                var creditText, creditFormat, creditParams,
-                       title = info.title.getNameText(),
+                       titleText = info.title.getNameText(),
+                       titleUrl = this.getLinkUrl( info ),
+                       $title = $( '<a>' ).text( titleText ).prop( 'href', 
titleUrl ),
                        bylines = this.getBylines( info.author, info.source );
 
                creditFormat = 't';
-               creditParams = [ title ];
+               creditParams = [ this.getOuterHtml( $title ) ];
                if ( bylines.html ) {
                        creditFormat += 'b';
                        creditParams.push( bylines.html );
@@ -112,13 +114,8 @@
                        creditFormat += 's';
                        creditParams.push( info.siteName );
                }
-
-               if ( creditFormat === 't' || creditFormat === 'ts' ) {
-                       creditText = '"' + title + '"';
-               } else {
-                       creditParams.unshift( 
'multimediaviewer-html-embed-credit-text-' + creditFormat );
-                       creditText = mw.message.apply( mw, creditParams 
).plain();
-               }
+               creditParams.unshift( 
'multimediaviewer-html-embed-credit-text-' + creditFormat );
+               creditText = mw.message.apply( mw, creditParams ).plain();
 
                return creditText;
        };
@@ -136,10 +133,11 @@
                return $( '<div>' ).append(
                        $( '<p>' ).append(
                                $( '<a>' )
-                                       .attr( 'href', info.url )
+                                       .attr( 'href', this.getLinkUrl( info ) )
                                        .append(
                                                $( '<img>' )
                                                        .attr( 'src', imgUrl )
+                                                       .attr( 'alt', 
info.title.getMainText() )
                                                        .attr( 'height', height 
)
                                                        .attr( 'width', width )
                                        ),
@@ -149,5 +147,25 @@
                ).html();
        };
 
+       /**
+        * Generare a link which we will be using for sharing stuff.
+        * FIXME this should be handled by mmv.js to be DRY
+        *
+        * @param {mw.mmv.model.EmbedFileInfo} info
+        */
+       AFP.getLinkUrl = function ( info ) {
+               return info.url + '#mediaviewer/' + info.title.getMainText();
+       };
+
+       /**
+        * Returns the HTML code for a jQuery element.
+        * Unlike .html(), this includes code for the element itself.
+        * @param {jQuery} $jq
+        * @return {string}
+        */
+       AFP.getOuterHtml = function ( $jq ) {
+               return $jq.get( 0 ).outerHTML;
+       };
+
        mw.mmv.EmbedFileFormatter = EmbedFileFormatter;
 }( mediaWiki, jQuery ) );
diff --git a/tests/qunit/mmv/mmv.EmbedFileFormatter.test.js 
b/tests/qunit/mmv/mmv.EmbedFileFormatter.test.js
index 687fb0f..359a686 100644
--- a/tests/qunit/mmv/mmv.EmbedFileFormatter.test.js
+++ b/tests/qunit/mmv/mmv.EmbedFileFormatter.test.js
@@ -183,7 +183,7 @@
                assert.ok( generatedHtml.match( title ), 'Title appears in 
generated HTML.');
                assert.ok( generatedHtml.match( filePageUrl ), 'Page url 
appears in generated HTML.' );
                assert.ok( generatedHtml.match( thumbUrl ), 'Thumbnail url 
appears in generated HTML' );
-               assert.ok( ! generatedHtml.match( siteName ), 'Site name should 
not appear in generated HTML' );
+               assert.ok( generatedHtml.match( siteName ), 'Site name should 
appear in generated HTML' );
                assert.ok( ! generatedHtml.match( 'Public License' ), 'License 
should not appear in generated HTML' );
                assert.ok( ! generatedHtml.match( 'Homer' ), 'Author should not 
appear in generated HTML' );
                assert.ok( ! generatedHtml.match( 'Iliad' ), 'Source should not 
appear in generated HTML' );

-- 
To view, visit https://gerrit.wikimedia.org/r/119554
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I43db84adb7fe29850706f92ee978016939b59aaa
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/MultimediaViewer
Gerrit-Branch: master
Gerrit-Owner: GergÅ‘ Tisza <[email protected]>
Gerrit-Reviewer: GergÅ‘ Tisza <[email protected]>
Gerrit-Reviewer: Gilles <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to