r113916 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r113915‎ | r113916 | r113917 >
Date:14:22, 15 March 2012
Author:yonishostak
Status:resolved (Comments)
Tags:
Comment:
AFTv5: sync commit, readded i18n msgs etc.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/bin/find_needed_i18n_keys.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
——@@ -419,6 +419,16 @@
420420 testing on the English Encyclopedia and that the feedback page has not been
421421 publicized to the community for this testing period.',
422422
 423+ /* Front-end: feedback link close button */
 424+ 'articlefeedbackv5-link-close-caption' => 'Remove Article Feedback?',
 425+ 'articlefeedbackv5-link-close-text1' => 'To remove this widget, go to',
 426+ 'articlefeedbackv5-link-close-linktext' => '"My Preferences > Appearances"',
 427+ 'articlefeedbackv5-link-close-linkurl' => '#',
 428+ 'articlefeedbackv5-link-close-text2' => 'then check this box:',
 429+ 'articlefeedbackv5-link-cllse-text3' => '"Don\'t show the Article Feedback widget."',
 430+ 'articlefeedbackv5-link-close-submit' => 'Remove',
 431+ 'articlefeedbackv5-link-close-cancel' => 'Cancel',
 432+
423433 );
424434
425435 /** Message documentation (Message documentation)
——@@ -692,6 +702,17 @@
693703 * <code>$2</code> – Page name of item with feedback requiring oversight.
694704 * <code>$3</code> – URL directly to feedback location
695705 * <code>$4</code> – The help link.',
 706+
 707+ /* Front-end: feedback link close button */
 708+ 'articlefeedbackv5-link-close-caption' => 'Remove article feedback tipsy - caption',
 709+ 'articlefeedbackv5-link-close-text1' => 'Remove article feedback tipsy - text line 1',
 710+ 'articlefeedbackv5-link-close-linktext' => 'Remove article feedback tipsy - text for close link',
 711+ 'articlefeedbackv5-link-close-linkurl' => 'Remove article feedback tipsy - URL for close link (also used for confirm button link URL)',
 712+ 'articlefeedbackv5-link-close-text2' => 'Remove article feedback tipsy - text line 2',
 713+ 'articlefeedbackv5-link-cllse-text3' => 'Remove article feedback tipsy - text line 3',
 714+ 'articlefeedbackv5-link-close-submit' => 'Remove article feedback tipsy - confirm button text',
 715+ 'articlefeedbackv5-link-close-cancel' => 'Remove article feedback tipsy - cancel link text',
 716+
696717 );
697718
698719 /** Afrikaans (Afrikaans)
Index: trunk/extensions/ArticleFeedbackv5/bin/find_needed_i18n_keys.php
——@@ -8,11 +8,8 @@
99
1010 include dirname(__FILE__) . '/../ArticleFeedbackv5.i18n.php';
1111
12 -$en_keys = array_keys($messages['en']);
13 -$qqq_keys = array_keys($messages['qqq']);
14 -
15 -$needed_keys = array_diff($en_keys, $qqq_keys);
16 -
17 -foreach($needed_keys as $name) {
18 - echo "$name\n";
19 -}
\ No newline at end of file
 12+foreach( array_keys($messages['en']) as $needle ) {
 13+ if( !array_key_exists($needle, $messages['qqq']) ) {
 14+ echo "'$needle' => '',\n";
 15+ }
 16+}
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js
——@@ -501,19 +501,22 @@
502502 // }}}
503503 // {{{ maskPost
504504 $.articleFeedbackv5special.maskPost = function( $row, $type ) {
505 - var $screen = $( $.articleFeedbackv5special.maskHtmlTemplate )
506 - .addClass( 'articleFeedbackv5-post-screen' )
 505+ var $screen = $row.find( '.articleFeedbackv5-post-screen' );
 506+ if( 0 == $screen.length ) {
 507+ $screen = $( $.articleFeedbackv5special.maskHtmlTemplate );
 508+ $screen.find( '.articleFeedbackv5-mask-text' )
 509+ .text( mw.msg( 'articlefeedbackv5-mask-text' ) );
 510+ $screen.find( '.articleFeedbackv5-mask-postid' )
 511+ .text( mw.msg( 'articlefeedbackv5-mask-postnumber', $row.attr( 'rel' ) ) );
 512+ $row.prepend( $screen );
 513+ }
 514+ $screen
507515 .height( $row.innerHeight() )
508516 .click( function( e ) {
509517 $( e.target ).closest( '.articleFeedbackv5-post-screen' ).remove();
510518 } );
511519 $screen.find( '.articleFeedbackv5-mask-text-wrapper')
512520 .css( 'top', $screen.innerHeight() / 2 - 12 );
513 - $screen.find( '.articleFeedbackv5-mask-text' )
514 - .text( mw.msg( 'articlefeedbackv5-mask-text-' + $type ) );
515 - $screen.find( '.articleFeedbackv5-mask-postid' )
516 - .text( mw.msg( 'articlefeedbackv5-mask-postnumber', $row.attr( 'rel' ) ) );
517 - $row.prepend( $screen );
518521 }
519522 // }}}
520523 // {{{ markDeleted
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js
——@@ -200,7 +200,7 @@
201201 ',
202202
203203 clear: '<div class="clear"></div>'
204 -
 204+
205205 };
206206
207207 // }}}
——@@ -2177,7 +2177,7 @@
21782178 // Track init at 1%
21792179 if ( Math.random() * 100 < 1 ) {
21802180 $.articleFeedbackv5.trackClick( $.articleFeedbackv5.bucketName() + '-init' );
2181 - }
 2181+ }
21822182 };
21832183
21842184 // }}}
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css
——@@ -1034,3 +1034,61 @@
10351035 border-radius: 0;
10361036 }
10371037
 1038+/* Tipsy */
 1039+tipsy {
 1040+ padding: 5px 5px 11px 5px;
 1041+}
 1042+.tipsy-inner {
 1043+ padding: 0;
 1044+ border-color: #888;
 1045+ max-width: 40em;
 1046+ -moz-box-shadow: 2px 2px 1px #999;
 1047+ -webkit-box-shadow: 2px 2px 1px #999;
 1048+ box-shadow: 2px 2px 1px #999;
 1049+ margin-bottom: 5px;
 1050+ font-size: 0.9em;
 1051+}
 1052+.tipsy-se .tipsy-arrow {
 1053+ /* @embed */
 1054+ background-image: url(images/tipsy-flyover.png);
 1055+ width: 23px;
 1056+ height: 11px;
 1057+ margin-top: -11px;
 1058+}
 1059+.articlefeedbackv5-flyover-header {
 1060+ background-color: #d2e5f7;
 1061+ padding: 5px 10px 8px 10px;
 1062+ -moz-border-radius: 3px 3px 0 0;
 1063+ -webkit-border-radius: 3px 3px 0 0;
 1064+ border-radius: 3px 3px 0 0;
 1065+ -khtml-border-radius: 3px 3px 0 0;
 1066+ width: 200px;
 1067+}
 1068+.articlefeedbackv5-flyover-header h3 {
 1069+ font-size: 1.0em;
 1070+}
 1071+.articlefeedbackv5-flyover-footer {
 1072+ padding: 10px 10px 15px 10px;
 1073+ width: 100%;
 1074+}
 1075+#articlefeedbackv5-noteflyover-close {
 1076+ display: block;
 1077+ position: absolute;
 1078+ top: 0;
 1079+ right: 15px;
 1080+ margin-top: 15px;
 1081+ width: 16px;
 1082+ height: 16px;
 1083+ /* @embed */
 1084+ background-image: url(images/bg-close-off.png);
 1085+ background-repeat: no-repeat;
 1086+ background-position: left top;
 1087+ background-size: 16px 16px;
 1088+}
 1089+#articlefeedbackv5-noteflyover-close:hover {
 1090+ /* @embed */
 1091+ background-image: url(images/bg-close-hov.png);
 1092+}
 1093+div.articlefeedbackv5-form-flyover {
 1094+ padding: 10px 20px 10px 10px;
 1095+}
Index: trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js
——@@ -6,6 +6,23 @@
77 /* Load at the bottom of the article */
88 var $aftDiv = $( '<div id="mw-articlefeedbackv5"></div>' ).articleFeedbackv5();
99
 10+var closeAftTipsyHtml = '\
 11+ <div class="articlefeedbackv5-flyover-header">\
 12+ <h3 id="articlefeedbackv5-noteflyover-caption">' + mw.msg( 'articlefeedbackv5-link-close-caption' ) + '</h3>\
 13+ <a id="articlefeedbackv5-noteflyover-close" href="#"></a>\
 14+ </div>\
 15+ <div class="articlefeedbackv5-form-flyover">\
 16+ <div>' + mw.msg( 'articlefeedbackv5-link-close-text1' ) + '</div>\
 17+ <a href="' + mw.msg( 'articlefeedbackv5-link-close-linkurl' ) + '">' + mw.msg( 'articlefeedbackv5-link-close-linktext' ) + '</a>\
 18+ <div>' + mw.msg( 'articlefeedbackv5-link-close-text2' ) + '</div>\
 19+ <div>' + mw.msg( 'articlefeedbackv5-link-close-text3' ) + '</div>\
 20+ <div class="articlefeedbackv5-flyover-footer">\
 21+ <a id="articlefeedbackv5-noteflyover-submit" class="articlefeedbackv5-flyover-button" href="#">CLOSE</a>\
 22+ <a id="articlefeedbackv5-noteflyover-cancel" href="#">CANCEL</a>\
 23+ <a class="articlefeedbackv5-flyover-help" id="articlefeedbackv5-noteflyover-help" href="#">[?]</a>\
 24+ </div>\
 25+ </div>';
 26+
1027 // Put on bottom of article before #catlinks (if it exists)
1128 // Except in legacy skins, which have #catlinks above the article but inside content-div.
1229 var legacyskins = [ 'standard', 'cologneblue', 'nostalgia' ];
——@@ -148,11 +165,56 @@
149166 clickFeedbackLink( $( e.target ) );
150167 } );
151168 $bottomRightTab.insertBefore( $aftDiv );
 169+
152170 $aftDiv.articleFeedbackv5( 'addToRemovalQueue', $bottomRightTab );
153171 }
154172
155 -// E: Button fixed to bottom center
156 -// NOT IMPLEMENTED
 173+// E: Same as D, with other colors
 174+if ( 'E' == linkBucket ) {
 175+ var $bottomRightTab = $( '\
 176+ <div id="articleFeedbackv5-bottomrighttab" class="articleFeedbackv5-bottomrighttab">\
 177+ <div id="articleFeedbackv5-bottomrighttabbox" class="articleFeedbackv5-bottomrighttabbox">\
 178+ <div class="articleFeedbackv5-bottomrighttablink">\
 179+ <a href="#mw-articleFeedbackv5" id="articleFeedbackv5-bottomrighttablink"></a>\
 180+ <a href="#" id="articleFeedbackv5-bottmrighttabclose" class="articleFeedbackv5-bottomrighttabclose">X</a>\
 181+ </div>\
 182+ </div>\
 183+ </div>' );
 184+ $bottomRightTab.find( '#articleFeedbackv5-bottomrighttablink' )
 185+ .data( 'linkId', linkBucket )
 186+ .text( mw.msg( 'articlefeedbackv5-bottomrighttab-linktext' ) )
 187+ .click( function( e ) {
 188+ e.preventDefault();
 189+ clickFeedbackLink( $( e.target ) );
 190+ } );
 191+ $bottomRightTab.find( '#articleFeedbackv5-bottmrighttabclose' )
 192+ .tipsy( {
 193+ delayIn: 0, // delay before showing tooltip (ms)
 194+ delayOut: 0, // delay before hiding tooltip (ms)
 195+ fade: false, // fade tooltips in/out?
 196+ fallback: '', // fallback text to use when no tooltip text
 197+ gravity: 'se', // gravity
 198+ html: true, // is tooltip content HTML?
 199+ live: false, // use live event support?
 200+ offset: 10, // pixel offset of tooltip from element
 201+ opacity: 1.0, // opacity of tooltip
 202+ trigger: 'manual', // how tooltip is triggered - hover | focus | manual
 203+ title: function() {
 204+ return closeAftTipsyHtml;
 205+ }
 206+ } );
 207+ $bottomRightTab.find( '#articleFeedbackv5-bottmrighttabclose' )
 208+ .click( function( e ) {
 209+ e.preventDefault();
 210+ //dropFeedbackLink( $( e.target ).parents( '#articleFeedbackv5-bottomrighttab' ), linkBucket );
 211+ $( e.target ).tipsy( 'show' );
 212+ } );
 213+ $bottomRightTab.insertBefore( $aftDiv );
 214+
 215+ // Setup close tipsy
 216+
 217+ $aftDiv.articleFeedbackv5( 'addToRemovalQueue', $bottomRightTab );
 218+}
157219
158220 // F: Button fixed to left side
159221 // NOT IMPLEMENTED
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php
——@@ -81,9 +81,9 @@
8282 // placed in a bucket.
8383 'buckets' => array(
8484 'zero' => 0,
85 - 'one' => 34,
86 - 'two' => 33,
87 - 'three' => 33,
 85+ 'one' => 100,
 86+ 'two' => 0,
 87+ 'three' => 0,
8888 'four' => 0,
8989 'five' => 0,
9090 ),
——@@ -140,7 +140,7 @@
141141 // B: Below the titlebar on the right
142142 // C: Button fixed to right side
143143 // D: Button fixed to bottom right
144 - // E: Button fixed to bottom center
 144+ // E: Button fixed to bottom right, design D2
145145 // F: Button fixed to left side
146146 // G: Button below logo
147147 // H: Link on each section bar
——@@ -149,8 +149,8 @@
150150 'A' => 0,
151151 'B' => 0,
152152 'C' => 0,
153 - 'D' => 100,
154 - 'E' => 0,
 153+ 'D' => 0,
 154+ 'E' => 100,
155155 'F' => 0,
156156 'G' => 0,
157157 'H' => 0,
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php
——@@ -146,6 +146,14 @@
147147 'articlefeedbackv5-transparency-terms',
148148 'articlefeedbackv5-transparency-terms-linktext',
149149 'parentheses',
 150+ 'articlefeedbackv5-link-close-caption',
 151+ 'articlefeedbackv5-link-close-text1',
 152+ 'articlefeedbackv5-link-close-linktext',
 153+ 'articlefeedbackv5-link-close-linkurl',
 154+ 'articlefeedbackv5-link-close-text2',
 155+ 'articlefeedbackv5-link-cllse-text3',
 156+ 'articlefeedbackv5-link-close-submit',
 157+ 'articlefeedbackv5-link-close-cancel',
150158 ),
151159 'dependencies' => array(
152160 'jquery.appear',

Follow-up revisions

RevisionCommit summaryAuthorDate
r113925r113916: Update keys. 1 to ignoreraymond15:48, 15 March 2012
r113979Fix typo and add a bunch of FIXMEs because of a butt ugly patchwork message.siebrand00:06, 16 March 2012

Comments

#Comment by Raymond (talk | contribs)   15:52, 15 March 2012

I do not know how these messages are rendered. Does {{int:...}} works? If yes I suggest to replace 

'articlefeedbackv5-link-close-linktext' => '"My Preferences > Appearances"',

with 

'articlefeedbackv5-link-close-linktext' => '"{{int:mypreferences}} > {{int:prefs-rendering}}"',

to ensure consistent translations.

#Comment by Catrope (talk | contribs)   18:37, 20 March 2012 
+ 'articlefeedbackv5-link-close-text1' => 'To remove this widget, go to',
+ 'articlefeedbackv5-link-close-linktext' => '"My Preferences > Appearances"',
+ 'articlefeedbackv5-link-close-linkurl' => '#',
+ 'articlefeedbackv5-link-close-text2' => 'then check this box:',
+ 'articlefeedbackv5-link-cllse-text3' => '"Don\'t show the Article Feedback widget."',

Per Siebrand in r113979, this is a lego message which should be cleaned up. Create one message with 'To remove this widget, go to $1, then check this box.' , and then build the link from the other message and insert it as $1. Marking fixme for this oh it's been removed already, marking resolved. 

+tipsy {

This was removed later, but for the record, it doesn't work, you probably missed a dot here. 

+.tipsy-inner {

This was removed later, but for the record: this and other rules concerning tipsy should be namespaced so they only affect tipsy tooltips created by ArticleFeedbackv5, not any other tooltips that might exist on the page. 

+ <a href="' + mw.msg( 'articlefeedbackv5-link-close-linkurl' ) + '">' + mw.msg( 'articlefeedbackv5-link-close-linktext' ) + '</a>\

This code was removed later, but for the record, this is not a secure way of building HTML.

Status & tagging log