r87032 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r87031‎ | r87032 | r87033 >
Date:19:17, 27 April 2011
Author:krinkle
Status:ok (Comments)
Tags:
Comment:
AFT JsPerf commit:
* Use internal cached rootJQuery/$(document)
* Using $.fn.insertBefore instead of $.fn.before (~ 40% faster (since one calls the other internally); also makes code more readable imho)
* Appending events to #t-articlefeedback *before* appending it to the dom (also saves a find() query)
* Not using 'mediaWiki' inside a ((mw){ })(mediaWiki) wrapper (!)
* Removing the alias from the wrapper, mw is a global alias.
* (more soon, commiting this for now)
Modified paths:
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.js (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.startup.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.js
——@@ -1,9 +1,8 @@
22 /*
33 * Script for Article Feedback Extension
44 */
 5+( function( $ ) {
56
6 -( function( $, mw ) {
7 -
87 // Only track users who have been assigned to the tracking group
98 var tracked = 'track' === mw.user.bucket(
109 'ext.articleFeedback-tracking', mw.config.get( 'wgArticleFeedbackTracking' )
——@@ -42,13 +41,13 @@
4342
4443 function trackClick( id ) {
4544 // Track the click so we can figure out how useful this is
46 - if ( tracked && typeof $.trackActionWithInfo == 'function' ) {
47 - $.trackActionWithInfo( prefix( id ), mediaWiki.config.get( 'wgTitle' ) )
 45+ if ( tracked && $.isFunction( $.trackActionWithInfo ) ) {
 46+ $.trackActionWithInfo( prefix( id ), mw.config.get( 'wgTitle' ) )
4847 }
4948 }
5049
5150 function trackClickURL( url, id ) {
52 - if ( tracked && typeof $.trackActionURL == 'function' ) {
 51+ if ( tracked && $.isFunction( $.trackActionURL ) ) {
5352 return $.trackActionURL( url, prefix( id ) );
5453 } else {
5554 return url;
——@@ -61,7 +60,7 @@
6261 * This object makes use of Special:SimpleSurvey, and uses some nasty hacks - this needs to be
6362 * replaced with something much better in the future.
6463 */
65 -var survey = new ( function( mw ) {
 64+var survey = new ( function() {
6665
6766 /* Private Members */
6867
——@@ -193,7 +192,7 @@
194193 .appendTo( $dialog );
195194 $dialog.dialog( 'option', 'height', $message.height() + 100 )
196195 };
197 -} )( mediaWiki );
 196+} )();
198197
199198 var config = {
200199 'ratings': {
——@@ -238,7 +237,7 @@
239238 },
240239 'join': {
241240 'condition': function() {
242 - return isPitchVisible( 'join' ) && mediaWiki.user.anonymous();
 241+ return isPitchVisible( 'join' ) && mw.user.anonymous();
243242 },
244243 'action': function() {
245244 // Mute for 1 day
——@@ -246,10 +245,10 @@
247246 // Go to account creation page
248247 // Track the click through an API redirect
249248 window.location = trackClickURL(
250 - mediaWiki.config.get( 'wgScript' ) + '?' + $.param( {
 249+ mw.config.get( 'wgScript' ) + '?' + $.param( {
251250 'title': 'Special:UserLogin',
252251 'type': 'signup',
253 - 'returnto': mediaWiki.config.get( 'wgPageName' )
 252+ 'returnto': mw.config.get( 'wgPageName' )
254253 } ), 'pitch-signup-accept' );
255254 return false;
256255 },
——@@ -266,9 +265,9 @@
267266 // Go to login page
268267 // Track the click through an API redirect
269268 window.location = trackClickURL(
270 - mediaWiki.config.get( 'wgScript' ) + '?' + $.param( {
 269+ mw.config.get( 'wgScript' ) + '?' + $.param( {
271270 'title': 'Special:UserLogin',
272 - 'returnto': mediaWiki.config.get( 'wgPageName' )
 271+ 'returnto': mw.config.get( 'wgPageName' )
273272 } ), 'pitch-join-accept' );
274273 return false;
275274 }
——@@ -276,9 +275,9 @@
277276 'edit': {
278277 'condition': function() {
279278 // An empty restrictions array means anyone can edit
280 - var restrictions = mediaWiki.config.get( 'wgRestrictionEdit' );
 279+ var restrictions = mw.config.get( 'wgRestrictionEdit' );
281280 if ( restrictions.length ) {
282 - var groups = mediaWiki.config.get( 'wgUserGroups' );
 281+ var groups = mw.config.get( 'wgUserGroups' );
283282 // Verify that each restriction exists in the user's groups
284283 for ( var i = 0; i < restrictions.length; i++ ) {
285284 if ( !$.inArray( restrictions[i], groups ) ) {
——@@ -294,8 +293,8 @@
295294 // Go to edit page
296295 // Track the click through an API redirect
297296 window.location = trackClickURL(
298 - mediaWiki.config.get( 'wgScript' ) + '?' + $.param( {
299 - 'title': mediaWiki.config.get( 'wgPageName' ),
 297+ mw.config.get( 'wgScript' ) + '?' + $.param( {
 298+ 'title': mw.config.get( 'wgPageName' ),
300299 'action': 'edit',
301300 'clicktrackingsession': $.cookie( 'clicktracking-session' ),
302301 'clicktrackingevent': prefix( 'pitch-edit-save' )
——@@ -311,13 +310,12 @@
312311 }
313312 };
314313
315 -/* Load at the bottom of the article */
316 -$( '#catlinks' ).before( $( '<div id="mw-articlefeedback"></div>' ).articleFeedback( config ) );
 314+/* Load at the bottom of the article
 315+$( '<div id="mw-articlefeedback"></div>' );.articleFeedback( config ).insertBefore( '#catlinks' );*/
317316
318 -/* Add link so users can navigate to the feedback tool from the toolbox */
319 -$( '#p-tb ul' )
320 - .append( '<li id="t-articlefeedback"><a href="#mw-articlefeedback"></a></li>' )
321 - .find( '#t-articlefeedback a' )
 317+/* Add link so users can navigate to the feedback tool from the toolbox
 318+var $tbAft = $( '<li id="t-articlefeedback"><a href="#mw-articlefeedback"></a></li>')
 319+ .find( 'a' )
322320 .text( mw.msg( 'articlefeedback-form-switch-label' ) )
323321 .click( function() {
324322 // Click tracking
——@@ -336,7 +334,8 @@
337335 clearInterval( interval );
338336 }
339337 }, 200 );
340 - return true;
341 - } );
342 -
343 -} )( jQuery, mediaWiki );
 338+ } )
 339+ .end();
 340+$( '#p-tb' ).find( 'ul' ).append( $tbAft );
 341+ */
 342+} )( jQuery );
Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.startup.js
——@@ -2,7 +2,7 @@
33 * Script for Article Feedback Extension
44 */
55
6 -jQuery(document).ready( function( $ ) {
 6+jQuery( function( $ ) {
77 if (
88 // Main namespace articles
99 mw.config.get( 'wgNamespaceNumber' ) === 0

Follow-up revisions

RevisionCommit summaryAuthorDate
r87033Remove debug from r87032.krinkle19:18, 27 April 2011

Comments

#Comment by DieBuche (talk | contribs)   13:25, 2 July 2011

L~336: Why are you using end() when it's the last command anyway?

Status & tagging log