r86202 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86201‎ | r86202 | r86203 >
Date:11:54, 16 April 2011
Author:diebuche
Status:ok (Comments)
Tags:
Comment:
Free metadata toggle script from wikibits.js dependencies; rename to mediawiki.action.view.metadata.js
Modified paths:
  • /trunk/phase3/includes/ImagePage.php (modified) (history)
  • /trunk/phase3/resources/Resources.php (modified) (history)
  • /trunk/phase3/resources/mediawiki.action/mediawiki.action.view.metadata.js (added) (history)
  • /trunk/phase3/skins/common/metadata.js (deleted) (history)

Diff [purge]

Index: trunk/phase3/skins/common/metadata.js
@@ -1,53 +0,0 @@
2 -// Exif metadata display for MediaWiki file uploads
3 -//
4 -// Add an expand/collapse link and collapse by default if set to
5 -// (with JS disabled, user will see all items)
6 -//
7 -// attachMetadataToggle('mw_metadata', 'More...', 'Fewer...');
8 -
9 -window.attachMetadataToggle = function( tableId, showText, hideText ) {
10 - if ( document.createTextNode ) {
11 - var box = document.getElementById( tableId );
12 - if ( !box ) {
13 - return false;
14 - }
15 -
16 - var tbody = box.getElementsByTagName('tbody')[0];
17 -
18 - var row = document.createElement( 'tr' );
19 -
20 - var col = document.createElement( 'td' );
21 - col.colSpan = 2;
22 -
23 - var link = document.createElement( 'a' );
24 - link.href = '#';
25 -
26 - link.onclick = function() {
27 - if ( box.className == 'mw_metadata collapsed' ) {
28 - changeText( link, hideText );
29 - box.className = 'mw_metadata expanded';
30 - } else {
31 - changeText( link, showText );
32 - box.className = 'mw_metadata collapsed';
33 - }
34 - return false;
35 - };
36 -
37 - var text = document.createTextNode( hideText );
38 -
39 - link.appendChild( text );
40 - col.appendChild( link );
41 - row.appendChild( col );
42 - tbody.appendChild( row );
43 -
44 - // And collapse!
45 - link.onclick();
46 -
47 - return true;
48 - }
49 - return false;
50 -};
51 -
52 -$( document ).ready( function() {
53 - attachMetadataToggle( 'mw_metadata', mw.msg( 'metadata-expand' ), mw.msg( 'metadata-collapse' ) );
54 -} );
Index: trunk/phase3/includes/ImagePage.php
@@ -156,7 +156,7 @@
157157 if ( $showmeta ) {
158158 $wgOut->addHTML( Xml::element( 'h2', array( 'id' => 'metadata' ), wfMsg( 'metadata' ) ) . "\n" );
159159 $wgOut->addWikiText( $this->makeMetadataTable( $formattedMetadata ) );
160 - $wgOut->addModules( array( 'mediawiki.legacy.metadata' ) );
 160+ $wgOut->addModules( array( 'mediawiki.action.view.metadata' ) );
161161 }
162162
163163 $css = $this->repo->getDescriptionStylesheetUrl();
Index: trunk/phase3/resources/mediawiki.action/mediawiki.action.view.metadata.js
@@ -0,0 +1,38 @@
 2+// Exif metadata display for MediaWiki file uploads
 3+//
 4+// Add an expand/collapse link and collapse by default if set to
 5+// (with JS disabled, user will see all items)
 6+//
 7+
 8+$( document ).ready( function() {
 9+ var showText = mw.msg( 'metadata-expand' );
 10+ var hideText = mw.msg( 'metadata-collapse' );
 11+
 12+ var $table = $( '#mw_metadata' );
 13+ var $tbody = $table.find( 'tbody' );
 14+ if ( !$tbody.length ) {
 15+ return;
 16+ }
 17+
 18+ var $row = $( '<tr></tr>' );
 19+ var $col = $( '<td colspan="2"></td>' );
 20+
 21+ var $link = $( '<a></a>', {
 22+ 'text': showText
 23+ }).click(function() {
 24+ if ( $table.is( '.collapsed' ) ) {
 25+ $( this ).text( hideText );
 26+ } else {
 27+ $( this ).text( showText );
 28+ }
 29+ $table.toggleClass( 'expanded, collapsed' );
 30+ return false;
 31+ });
 32+
 33+ $col.append( $link );
 34+ $row.append( $col );
 35+ $tbody.append( $row );
 36+
 37+ // And collapse!
 38+ $table.addClass( 'collapsed' );
 39+} );
Property changes on: trunk/phase3/resources/mediawiki.action/mediawiki.action.view.metadata.js
___________________________________________________________________
Added: svn:eol-style
140 + native
Index: trunk/phase3/resources/Resources.php
@@ -416,6 +416,10 @@
417417 'mediawiki.action.view.rightClickEdit' => array(
418418 'scripts' => 'resources/mediawiki.action/mediawiki.action.view.rightClickEdit.js',
419419 ),
 420+ 'mediawiki.action.view.metadata' => array(
 421+ 'scripts' => 'resources/mediawiki.action/mediawiki.action.view.metadata.js',
 422+ 'messages' => array( 'metadata-expand', 'metadata-collapse' ),
 423+ ),
420424 'mediawiki.action.watch.ajax' => array(
421425 'scripts' => 'resources/mediawiki.action/mediawiki.action.watch.ajax.js',
422426 'dependencies' => 'mediawiki.util',
@@ -555,13 +559,6 @@
556560 'localBasePath' => "{$GLOBALS['IP']}/skins",
557561 'dependencies' => 'mediawiki.legacy.wikibits',
558562 ),
559 - 'mediawiki.legacy.metadata' => array(
560 - 'scripts' => 'common/metadata.js',
561 - 'remoteBasePath' => $GLOBALS['wgStylePath'],
562 - 'localBasePath' => "{$GLOBALS['IP']}/skins",
563 - 'dependencies' => 'mediawiki.legacy.wikibits',
564 - 'messages' => array( 'metadata-expand', 'metadata-collapse' ),
565 - ),
566563 'mediawiki.legacy.mwsuggest' => array(
567564 'scripts' => 'common/mwsuggest.js',
568565 'remoteBasePath' => $GLOBALS['wgStylePath'],

Follow-up revisions

RevisionCommit summaryAuthorDate
r86259Followup to r86202 per CR: Add href=# to trigger hand cursordiebuche13:48, 17 April 2011
r86596Followup to r86202: Pass jQuery as argument in .ready(). Thanks Krinkle\!diebuche07:03, 21 April 2011
r87052Little performance fix and typo fix. Follow-up r86202krinkle22:19, 27 April 2011

Comments

#Comment by Bawolff (talk | contribs)   17:57, 16 April 2011

Minor issue. Now when you hover over the link, you get the text editing cursor instead of the activate hyperlink "hand" cursor.

Could probably be fixed by having a dummy href="#" attribute (like it did before), or using the css cursor style rule (I think first choice is better personally)

#Comment by Krinkle (talk | contribs)   22:21, 20 April 2011
+$( document ).ready( function() {

Please don't imply the $ as a global, instead alias from jQuery in core javascript. jQuery passes itself as the first argument to the callback of document.ready, so you can do the alias + document.ready like this in a single wrapper:

jQuery( document ).ready( function( $ ) { 
  ...
} );
#Comment by Krinkle (talk | contribs)   22:20, 27 April 2011
+		if ( $table.is( '.collapsed' ) ) {

jQuery's is() function is very flexible but also relatively slow. Use more specific alternatives when possible. Such as hasClass. (jsperf comparison).

+		$table.toggleClass( 'expanded, collapsed' );

This doesn't work (atleast not in the most recent version of jQuery). toggleClass is able to toggle multple classes indepently, but does not take input in comma seperated string. Instead it takes a space seperated string (see also jQuery.fn.toggleClass documentation);

Fixed in r87052.

Status & tagging log