r87359 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r87358‎ | r87359 | r87360 >
Date:21:30, 3 May 2011
Author:kaldari
Status:ok (Comments)
Tags:
Comment:
new subheader messages
Modified paths:
  • /trunk/extensions/UploadWizard/UploadWizard.config.php (modified) (history)
  • /trunk/extensions/UploadWizard/UploadWizard.i18n.php (modified) (history)
  • /trunk/extensions/UploadWizard/UploadWizardHooks.php (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizard.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/uploadWizard.css (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/UploadWizard.config.php
——@@ -261,5 +261,14 @@
262262
263263 // Wiki page for leaving Upload Wizard feedback, for example 'Commons:Upload wizard feedback'
264264 'feedbackPage' => '',
 265+
 266+ // Bugzilla page for UploadWizard bugs
 267+ 'bugList' => 'http://bugzilla.wikimedia.org/buglist.cgi?cmdtype=runnamed&namedcmd=UploadWizard&list_id=4977',
 268+
 269+ // TranslateWiki page for help with translations
 270+ 'translateHelp' => 'http://translatewiki.net/w/i.php?title=Special:Translate&group=ext-uploadwizard',
 271+
 272+ // URL for alternative uploading form
 273+ 'altUploadForm' => '',
265274
266275 );
Index: trunk/extensions/UploadWizard/UploadWizardHooks.php
——@@ -306,6 +306,10 @@
307307 'mwe-upwiz-thumbnail-failed',
308308 'mwe-upwiz-unparseable-filename',
309309 'mwe-upwiz-image-preview',
 310+ 'mwe-upwiz-subhead-message',
 311+ 'mwe-upwiz-subhead-bugs',
 312+ 'mwe-upwiz-subhead-translate',
 313+ 'mwe-upwiz-subhead-alt-upload',
310314 'mwe-upwiz-feedback-prompt',
311315 'mwe-upwiz-feedback-note',
312316 'mwe-upwiz-feedback-subject',
Index: trunk/extensions/UploadWizard/UploadWizard.i18n.php
——@@ -237,8 +237,13 @@
238238 'mwe-upwiz-unparseable-filename' => 'Could not understand the file name "$1"',
239239 'mwe-upwiz-image-preview' => 'Image preview',
240240
 241+ 'mwe-upwiz-subhead-message' => 'Thanks for using our new upload tool!',
 242+ 'mwe-upwiz-subhead-bugs' => '[$1 Known issues]',
 243+ 'mwe-upwiz-subhead-translate' => '[$1 Help with translations]',
 244+ 'mwe-upwiz-subhead-alt-upload' => '[$1 Back to the old form]',
 245+
241246 /* Feedback interface */
242 - 'mwe-upwiz-feedback-prompt' => 'Please [$1 let us know] what you think of UploadWizard!',
 247+ 'mwe-upwiz-feedback-prompt' => '[$1 Leave feedback]',
243248 'mwe-upwiz-feedback-note' => 'Your feedback will be posted publicly to the page "[$2 $1]", along with your user name, browser version and operating system.',
244249 'mwe-upwiz-feedback-subject' => 'Subject:',
245250 'mwe-upwiz-feedback-message' => 'Message:',
Index: trunk/extensions/UploadWizard/resources/uploadWizard.css
——@@ -2,7 +2,6 @@
33 #contentSub {
44 margin-left: 0;
55 color: #666666;
6 - font-style: italic;
76 }
87
98 form.mwe-upwiz-form {
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizard.js
——@@ -651,19 +651,31 @@
652652
653653 // remove first spinner
654654 $j( '#mwe-first-spinner' ).remove();
655 -
 655+
 656+ // construct the message for the subheader
 657+ $j( '#contentSub' ).append( $j( '<span style="margin-right: 0.5em;"></span>' ).msg( 'mwe-upwiz-subhead-message' ) );
656658 // feedback request
657659 if ( mw.isDefined( mw.UploadWizard.config['feedbackPage'] ) && mw.UploadWizard.config['feedbackPage'] !== '' ) {
658660 var feedback = new mw.Feedback( _this.api,
659 - new mw.Title( mw.UploadWizard.config['feedbackPage'] ) );
660 - $j( '#contentSub' )
661 - .msg( 'mwe-upwiz-feedback-prompt',
662 - function() {
663 - feedback.launch();
664 - return false;
665 - }
666 - );
 661+ new mw.Title( mw.UploadWizard.config['feedbackPage'] ) );
 662+ var feedbackLink = $j( '<span class="contentSubLink"></span>' ).msg( 'mwe-upwiz-feedback-prompt',
 663+ function() {
 664+ feedback.launch();
 665+ return false;
 666+ }
 667+ );
 668+ $j( '#contentSub' ).append( feedbackLink );
667669 }
 670+ if ( mw.UploadWizard.config['bugList'] !== '' ) {
 671+ $j( '#contentSub' ).append( $j( '<span class="contentSubLink"></span>' ).msg( 'mwe-upwiz-subhead-bugs', mw.UploadWizard.config['bugList'] ) );
 672+ }
 673+ if ( mw.UploadWizard.config['translateHelp'] !== '' ) {
 674+ $j( '#contentSub' ).append( $j( '<span class="contentSubLink"></span>' ).msg( 'mwe-upwiz-subhead-translate', mw.UploadWizard.config['translateHelp'] ) );
 675+ }
 676+ if ( mw.UploadWizard.config['altUploadForm'] !== '' ) {
 677+ $j( '#contentSub' ).append( $j( '<span class="contentSubLink"></span>' ).msg( 'mwe-upwiz-subhead-alt-upload', mw.UploadWizard.config['altUploadForm'] ) );
 678+ }
 679+ $j( '#contentSub .contentSubLink:not(:last)' ).after( '&nbsp;&middot;&nbsp;' );
668680
669681 // construct the arrow steps from the UL in the HTML
670682 $j( '#mwe-upwiz-steps' )

Follow-up revisions

RevisionCommit summaryAuthorDate
r87367adding missing tests, follow-up to r87359kaldari23:16, 3 May 2011
r87368fixing bugzilla link, follow-up to r87359kaldari23:19, 3 May 2011
r87371setting links to open in new windows, per r87359kaldari00:00, 4 May 2011

Comments

#Comment by NeilK (talk | contribs)   23:11, 3 May 2011

1) The tests for ( mw.UploadWizard.config['something'] !== ) are doing the wrong thing.

If those items are missing, the value returned is undefined, which is also !==

You want to use typeof (mw.UploadWizard.config['something']) !== undefined or perhaps mw.isDefined()


2) The URL for bugzilla doesn't work, those saved searches are specific to particular users

This will work better, I think. Be careful of those list_id arguments, they refer to ephemeral cached listings.

https://bugzilla.wikimedia.org/buglist.cgi?query_format=advanced&component=UploadWizard&resolution=---&product=MediaWiki+extensions


3) could you add target=_blank to all the links, so they don't interrupt an upload? Thanks.

#Comment by NeilK (talk | contribs)   23:12, 3 May 2011

uh, if it's not obvious, there are a lot of '' which got turned into italicization in my previous comment

#Comment by Kaldari (talk | contribs)   23:19, 3 May 2011

Oops, I meant to do both tests actually (like I do with feedback). Fixed in r87367. Bugzilla link fixed in r87368.

#Comment by NeilK (talk | contribs)   23:40, 3 May 2011

looks good, just missing target=_blank?

#Comment by NeilK (talk | contribs)   23:44, 3 May 2011

you can do it like this -- pass in a jQuery object instead of a string URL. Magic. :)

.msg( 

   'mwe-upwiz-subhead-bugs',
   $j( '<a></a>' ).attr( { href: mw.UploadWizard.config['bugList'], target: '_blank' } )

);

or, find the <a>'s after you do all .msg() and use jquery to add a target attr to them all

#Comment by Kaldari (talk | contribs)   00:00, 4 May 2011

fixed in r87371.

Status & tagging log