r86150 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86149‎ | r86150 | r86151 >
Date:23:04, 15 April 2011
Author:neilk
Status:resolved (Comments)
Tags:
Comment:
refactored Feedback tool out of uploadwizard. Still dependent on some UW libraries but this is going to be generally useful
so we might as well start separating it.

made Feedback generally easier to read, now an object managing its state

internationalized some remaining english strings

eliminated various string hacking and such with mw.Title and the linking features in mediawiki.language.parser

added a 'newSection' method to mw.Api.edit.js, so Feedback could call it
Modified paths:
  • /trunk/extensions/UploadWizard/UploadWizard.i18n.php (modified) (history)
  • /trunk/extensions/UploadWizard/UploadWizardHooks.php (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.Api.edit.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.Feedback.js (added) (history)
  • /trunk/extensions/UploadWizard/resources/mw.Title.js (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/UploadWizardHooks.php
——@@ -49,6 +49,7 @@
5050 'resources/mw.Api.js',
5151 'resources/mw.Api.edit.js',
5252 'resources/mw.Title.js',
 53+ 'resources/mw.Feedback.js',
5354
5455 // language menus
5556 'resources/mw.LanguageUpWiz.js',
——@@ -303,6 +304,7 @@
304305 'mwe-upwiz-thumbnail-failed',
305306 'mwe-upwiz-unparseable-filename',
306307 'mwe-upwiz-image-preview',
 308+ 'mwe-upwiz-feedback-prompt',
307309 'mwe-upwiz-feedback-note',
308310 'mwe-upwiz-feedback-subject',
309311 'mwe-upwiz-feedback-message',
Index: trunk/extensions/UploadWizard/UploadWizard.i18n.php
——@@ -255,9 +255,10 @@
256256 'mwe-upwiz-image-preview' => 'Image preview',
257257
258258 /* Feedback interface */
259 - 'mwe-upwiz-feedback-note' => 'Your feedback will be posted to $1.',
 259+ 'mwe-upwiz-feedback-prompt' => 'Please [$1 let us know] what you think of UploadWizard!',
 260+ '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.',
260261 'mwe-upwiz-feedback-subject' => 'Subject:',
261 - 'mwe-upwiz-feedback-message' => 'Message (without a signature):',
 262+ 'mwe-upwiz-feedback-message' => 'Message:',
262263 'mwe-upwiz-feedback-title' => 'Leave feedback about Upload Wizard',
263264 'mwe-upwiz-feedback-cancel' => 'Cancel',
264265 'mwe-upwiz-feedback-submit' => 'Submit Feedback',
Index: trunk/extensions/UploadWizard/resources/mw.Title.js
——@@ -189,6 +189,15 @@
190190 return ext;
191191 };
192192
 193+
 194+ /**
 195+ * Return the URL to this title
 196+ * returns null if there is no wgArticlePath
 197+ * @return {String|null}
 198+ */
 199+ this.getUrl = function() {
 200+ return wgArticlePath ? wgArticlePath.replace( '$1', this.toString() ) : null;
 201+ };
193202
194203 // initialization
195204 var matches = title.match( /^(?:([^:]+):)?(.*?)(?:\.(\w{1,5}))?$/ );
Index: trunk/extensions/UploadWizard/resources/mw.Feedback.js
——@@ -0,0 +1,136 @@
 2+( function( mw, $ ) {
 3+
 4+ /**
 5+ * Thingy for collecting user feedback on a wiki page
 6+ * @param {mw.Api} api properly configured to talk to this wiki
 7+ * @param {mw.Title} the title of the page where you collect feedback
 8+ */
 9+ mw.Feedback = function( api, feedbackTitle ) {
 10+ var _this = this;
 11+ this.api = api;
 12+ this.feedbackTitle = feedbackTitle;
 13+ this.setup();
 14+ };
 15+
 16+ mw.Feedback.prototype = {
 17+ setup: function() {
 18+ var _this = this;
 19+
 20+ // Set up buttons for dialog box. We have to do it the hard way since the json keys are localized
 21+ _this.buttons = {};
 22+ _this.buttons[ gM( 'mwe-upwiz-feedback-cancel' ) ] = function() { _this.cancel(); };
 23+ _this.buttons[ gM( 'mwe-upwiz-feedback-submit' ) ] = function() { _this.submit(); };
 24+
 25+ var $feedbackPageLink = $j( '<a></a>' ).attr( { 'href': _this.feedbackTitle.getUrl(), 'target': '_blank' } );
 26+ this.$dialog =
 27+ $( '<div style="position:relative;"></div>' ).append(
 28+ $j( '<div class="mwe-upwiz-feedback-mode mwe-upwiz-feedback-form"></div>' ).append(
 29+ $( '<div style="margin-top:0.4em;"></div>' ).append(
 30+ $( '<small></small>' ).msg( 'mwe-upwiz-feedback-note',
 31+ _this.feedbackTitle.getNameText(),
 32+ $feedbackPageLink )
 33+ ),
 34+ $( '<div style="margin-top:1em;"></div>' ).append(
 35+ gM( 'mwe-upwiz-feedback-subject' ),
 36+ $( '<br/>' ),
 37+ $( '<input type="text" class="mwe-upwiz-feedback-subject" name="subject" maxlength="60" style="width:99%;"/>' )
 38+ ),
 39+ $( '<div style="margin-top:0.4em;"></div>' ).append(
 40+ gM( 'mwe-upwiz-feedback-message' ),
 41+ $( '<br/>' ),
 42+ $( '<textarea name="message" class="mwe-upwiz-feedback-message" style="width:99%;" rows="5" cols="60"></textarea>' )
 43+ )
 44+ ),
 45+ $( '<div class="mwe-upwiz-feedback-mode mwe-upwiz-feedback-submitting" style="text-align:center;margin:3em 0;"></div>' ).append(
 46+ gM( 'mwe-upwiz-feedback-adding' ),
 47+ $( '<br/>' ),
 48+ $( '<img src="http://upload.wikimedia.org/wikipedia/commons/4/42/Loading.gif" />' )
 49+ ),
 50+ $( '<div class="mwe-upwiz-feedback-mode mwe-upwiz-feedback-error" style="position:relative;"></div>' ).append(
 51+ $( '<div class="mwe-upwiz-feedback-error-msg style="color:#990000;margin-top:0.4em;"></div>' )
 52+
 53+ )
 54+ ).dialog({
 55+ width: 500,
 56+ autoOpen: false,
 57+ title: gM( 'mwe-upwiz-feedback-title' ),
 58+ modal: true,
 59+ buttons: _this.buttons
 60+ });
 61+
 62+ this.subjectInput = this.$dialog.find( 'input.mwe-upwiz-feedback-subject' ).get(0);
 63+ this.messageInput = this.$dialog.find( 'textarea.mwe-upwiz-feedback-message' ).get(0);
 64+ this.displayForm();
 65+ },
 66+
 67+ display: function( s ) {
 68+ this.$dialog.dialog( { buttons:{} } ); // hide the buttons
 69+ this.$dialog.find( '.mwe-upwiz-feedback-mode' ).hide(); // hide everything
 70+ this.$dialog.find( '.mwe-upwiz-feedback-' + s ).show(); // show the desired div
 71+ },
 72+
 73+ displaySubmitting: function() {
 74+ this.display( 'submitting' );
 75+ },
 76+
 77+ displayForm: function() {
 78+ this.subjectInput.value = '';
 79+ this.messageInput.value = '';
 80+ this.display( 'form' );
 81+ this.$dialog.dialog( { buttons: this.buttons } ); // put the buttons back
 82+ },
 83+
 84+ displayError: function( message ) {
 85+ this.display( 'error' );
 86+ this.$dialog.find( '.mwe-upwiz-feedback-error-msg' ).msg( message );
 87+ },
 88+
 89+ cancel: function() {
 90+ this.$dialog.dialog( 'close' );
 91+ },
 92+
 93+ submit: function() {
 94+ var _this = this;
 95+
 96+ // get the values to submit
 97+ var subject = this.subjectInput.value;
 98+
 99+ message = "<small>User agent: " + navigator.userAgent + "</small>\n\n"
 100+ + this.messageInput.value;
 101+ if ( message.indexOf( '~~~' ) == -1 ) {
 102+ message += " ~~~~";
 103+ }
 104+
 105+ this.displaySubmitting();
 106+
 107+ var ok = function( result ) {
 108+ if ( mw.isDefined( result.edit ) ) {
 109+ if ( result.edit.result === 'Success' ) {
 110+ _this.$dialog.dialog( 'close' ); // edit complete, close dialog box
 111+ } else {
 112+ _this.displayError( 'mwe-upwiz-feedback-error1' ); // unknown API result
 113+ }
 114+ } else {
 115+ displayError( 'mwe-upwiz-feedback-error2' ); // edit failed
 116+ }
 117+ };
 118+
 119+ var err = function( code, info ) {
 120+ displayError( 'mwe-upwiz-feedback-error3' ); // ajax request failed
 121+ };
 122+
 123+ this.api.newSection( this.feedbackTitle, subject, message, ok, err );
 124+
 125+ }, // close submit button function
 126+
 127+
 128+ launch: function() {
 129+ this.displayForm();
 130+ this.$dialog.dialog( 'open' );
 131+ this.subjectInput.focus();
 132+ }
 133+
 134+ };
 135+
 136+
 137+} )( window.mediaWiki, jQuery );
Property changes on: trunk/extensions/UploadWizard/resources/mw.Feedback.js
___________________________________________________________________
Added: svn:eol-style
1138 + native
Index: trunk/extensions/UploadWizard/resources/mw.Api.edit.js
——@@ -23,7 +23,7 @@
2424 // an infinite loop. If this fresh token is bad, something else is very wrong.
2525 var useTokenToPost = function( token ) {
2626 params.token = token;
27 - this.post( params, ok, err );
 27+ api.post( params, ok, err );
2828 };
2929 api.getEditToken( useTokenToPost, err );
3030 } else {
——@@ -54,7 +54,8 @@
5555 * @param {Function} error callback
5656 */
5757 getEditToken: function( tokenCallback, err ) {
58 -
 58+ var api = this;
 59+
5960 var parameters = {
6061 'prop': 'info',
6162 'intoken': 'edit',
——@@ -81,11 +82,29 @@
8283
8384 var ajaxOptions = { 'ok': ok, 'err': err };
8485
85 - this.get( parameters, ajaxOptions );
 86+ api.get( parameters, ajaxOptions );
 87+ },
 88+
 89+ /**
 90+ * Create a new section of the page.
 91+ * @param {mw.Title|String} target page
 92+ * @param {String} header
 93+ * @param {String} wikitext message
 94+ * @param {Function} success handler
 95+ * @param {Function} error handler
 96+ */
 97+ newSection: function( title, header, message, ok, err ) {
 98+ var params = {
 99+ action: 'edit',
 100+ section: 'new',
 101+ format: 'json',
 102+ title: title.toString(),
 103+ summary: header,
 104+ text: message
 105+ };
 106+ this.postWithEditToken( params, ok, err );
86107 }
87108
88 -
89 -
90 - } );
 109+ } ); // end extend
91110
92 -}) ( window.mediaWiki, jQuery );
 111+} )( window.mediaWiki, jQuery );
Index: trunk/extensions/UploadWizard/resources/uploadWizard.css
——@@ -2,6 +2,7 @@
33 #contentSub {
44 margin-left: 0;
55 color: #666666;
 6+ font-style: italic;
67 }
78
89 form.mwe-upwiz-form {
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizard.js
——@@ -577,13 +577,16 @@
578578 $j( '#mwe-first-spinner' ).remove();
579579
580580 // feedback request
581 - if ( UploadWizardConfig['feedbackPage'] !== '' ) {
582 - $j( '#contentSub' ).html('<i>Please <a id="mwe-upwiz-feedback" href="#">let us know</a> what you think of Upload Wizard!</i>');
583 - $j( '#mwe-upwiz-feedback')
584 - .click( function() {
585 - _this.launchFeedback();
586 - return false;
587 - } );
 581+ if ( mw.isDefined( mw.UploadWizard.config['feedbackPage'] ) ) {
 582+ var feedback = new mw.Feedback( _this.api,
 583+ new mw.Title( mw.UploadWizard.config['feedbackPage'] ) );
 584+ $j( '#contentSub' )
 585+ .msg( 'mwe-upwiz-feedback-prompt',
 586+ function() {
 587+ feedback.launch();
 588+ return false;
 589+ }
 590+ );
588591 }
589592
590593 // construct the arrow steps from the UL in the HTML
——@@ -1250,83 +1253,6 @@
12511254 },
12521255
12531256 /**
1254 - * Build interface for collecting user feedback on Upload Wizard
1255 - */
1256 - launchFeedback: function() {
1257 - _this = this;
1258 -
1259 - var displayError = function( message ) {
1260 - $j( '#mwe-upwiz-feedback-form div' ).hide(); // remove everything else from the dialog box
1261 - $j( '#mwe-upwiz-feedback-form' ).append ( $j( '<div style="color:#990000;margin-top:0.4em;"></div>' ).msg( message ) );
1262 - };
1263 -
1264 - // Set up buttons for dialog box. We have to do it the hard way since the json keys are localized
1265 - var cancelButton = gM( 'mwe-upwiz-feedback-cancel' );
1266 - var submitButton = gM( 'mwe-upwiz-feedback-submit' );
1267 - var buttonSettings = {};
1268 - buttonSettings[cancelButton] = function() { $j( this ).dialog( 'close' ); };
1269 - buttonSettings[submitButton] = function() {
1270 - $feedbackForm.dialog({buttons:{}});
1271 - $j( '#mwe-upwiz-feedback-form div' ).hide(); // remove everything else from the dialog box
1272 - $j( '#mwe-upwiz-feedback-form' ).append ( $j( '<div style="text-align:center;margin:3em 0;"></div>' ).append( gM( 'mwe-upwiz-feedback-adding' ), $j( '<br/>' ), $j( '<img src="http://upload.wikimedia.org/wikipedia/commons/4/42/Loading.gif" />' ) ) );
1273 - var subject = $j( '#mwe-upwiz-feedback-subject' ).val();
1274 - var message = "<!--User agent: "+navigator.userAgent+"-->\n";
1275 - message += $j( '#mwe-upwiz-feedback-message' ).val();
1276 - if ( message.indexOf( '~~~' ) == -1 ) {
1277 - message += ' ~~~~';
1278 - }
1279 - var useTokenToPostFeedback = function( token ) {
1280 - $j.ajax({
1281 - url: wgScriptPath + '/api.php',
1282 - data: $.param({
1283 - action: 'edit',
1284 - title: mw.UploadWizard.config['feedbackPage'],
1285 - section: 'new',
1286 - summary: subject,
1287 - text: message,
1288 - format: 'json',
1289 - token: token
1290 - }),
1291 - dataType: 'json',
1292 - type: 'POST',
1293 - success: function( data ) {
1294 - if ( typeof data.edit != 'undefined' ) {
1295 - if ( data.edit.result == 'Success' ) {
1296 - $feedbackForm.dialog( 'close' ); // edit complete, close dialog box
1297 - } else {
1298 - displayError( 'mwe-upwiz-feedback-error1' ); // unknown API result
1299 - }
1300 - } else {
1301 - displayError( 'mwe-upwiz-feedback-error2' ); // edit failed
1302 - }
1303 - },
1304 - error: function( xhr ) {
1305 - displayError( 'mwe-upwiz-feedback-error3' ); // ajax request failed
1306 - }
1307 - }); // close Ajax request
1308 - }; // close useTokenToPost function
1309 - _this.api.getEditToken( useTokenToPostFeedback );
1310 - }; // close submit button function
1311 -
1312 - // Construct the feedback form
1313 - var feedbackLink = '<a href="'+wgArticlePath.replace( '$1', mw.UploadWizard.config['feedbackPage'].replace( /\s/g, '_' ) )+'" target="_blank">'+mw.UploadWizard.config['feedbackPage']+'</a>';
1314 - $feedbackForm = $j( '<div id="mwe-upwiz-feedback-form" style="position:relative;"></div>' )
1315 - .append( $j( '<div style="margin-top:0.4em;"></div>' ).append( $j( '<small></small>' ).msg( 'mwe-upwiz-feedback-note', feedbackLink ) ) )
1316 - .append( $j( '<div style="margin-top:1em;"></div>' ).append( gM( 'mwe-upwiz-feedback-subject' ), $j( '<br/>' ), $j( '<input type="text" id="mwe-upwiz-feedback-subject" name="subject" maxlength="60" style="width:99%;"/>' ) ) )
1317 - .append( $j( '<div style="margin-top:0.4em;"></div>' ).append( gM( 'mwe-upwiz-feedback-message' ), $j( '<br/>' ), $j( '<textarea name="message" id="mwe-upwiz-feedback-message" style="width:99%;" rows="5" cols="60"></textarea>' ) ) )
1318 - .dialog({
1319 - width: 500,
1320 - autoOpen: false,
1321 - title: gM( 'mwe-upwiz-feedback-title' ),
1322 - modal: true,
1323 - buttons: buttonSettings
1324 - }); // close dialog, end $feedbackForm definition
1325 -
1326 - $feedbackForm.dialog( 'open' );
1327 -
1328 - }, // close launchFeedback function
1329 -
1330 - /**
13311257 * Set a cookie which lets the user skip the tutorial step in the future
13321258 */
13331259 setSkipTutorialCookie: function() {

Follow-up revisions

RevisionCommit summaryAuthorDate
r86868fixed inadvertent globalneilk16:19, 25 April 2011

Comments

#Comment by Catrope (talk | contribs)   11:10, 25 April 2011 
+			_this.buttons[ gM( 'mwe-upwiz-feedback-cancel' ) ] = function() { _this.cancel(); };
+			_this.buttons[ gM( 'mwe-upwiz-feedback-submit' ) ] = function() { _this.submit(); };

Can't you just use _this.buttons[ gM( 'foo' ) ] = _this.cancel; ? 

+			message = "<small>User agent: " + navigator.userAgent + "</small>\n\n"
+				 + this.messageInput.value;

This leaks message into the global scope.

#Comment by NeilK (talk | contribs)   16:24, 25 April 2011

> Can't you just use _this.buttons[ gM( 'foo' ) ] = _this.cancel; ?

Nope, that's not how Javascript works, sadly. Consider... 

  function Foo(x) {
     this.x = x;
  }

  Foo.prototype = {
     getX: function() {
        return this.x;
     }
  }

  var f = new Foo("blah");
  console.log( f.getX() );  // "blah";

  var fgetX = f.getX;
  console.log( fgetX() ); // undefined


> This leaks message into the global scope.

fixed in r86868.

#Comment by Catrope (talk | contribs)   17:37, 25 April 2011

Oh, does it not pass the context (this object) correctly when passed that way?

#Comment by NeilK (talk | contribs)   16:19, 25 April 2011

> This leaks message into the global scope.

fixed in r86868

#Comment by Trevor Parscal (WMF) (talk | contribs)   16:56, 25 April 2011

Not critical, but getEditToken aliases this as api, but doesn't really need to.

Status & tagging log