r96186 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96185‎ | r96186 | r96187 >
Date:14:06, 3 September 2011
Author:santhosh
Status:ok (Comments)
Tags:
Comment:
Refactor the code for building the menu to buildMenu.
Modified paths:
  • /trunk/extensions/Narayam/js/ext.narayam.core.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Narayam/js/ext.narayam.core.js
@@ -385,6 +385,36 @@
386386 * from a cookie or wgNarayamEnableByDefault
387387 */
388388 this.setup = function() {
 389+ //build the menu.
 390+ that.buildMenu();
 391+
 392+ // Restore state from cookies
 393+ var savedScheme = $.cookie( 'narayam-scheme' );
 394+ if ( savedScheme && savedScheme in schemes ) {
 395+ that.setScheme( savedScheme );
 396+ $( '#narayam-' + savedScheme ).attr( 'checked', 'checked' );
 397+ } else {
 398+ //if no saved input scheme, select the first.
 399+ $('input.narayam-scheme:first').attr( 'checked', 'checked' );
 400+ }
 401+ var enabledCookie = $.cookie( 'narayam-enabled' );
 402+ if ( enabledCookie == '1' || ( mw.config.get( 'wgNarayamEnabledByDefault' ) && enabledCookie !== '0' ) ) {
 403+ that.enable();
 404+ }
 405+ else {
 406+ $( 'li#pt-narayam').addClass( 'narayam-inactive' );
 407+ }
 408+ // Renew the narayam-enabled cookie. naraym-scheme is renewed by setScheme()
 409+ if ( enabledCookie ) {
 410+ $.cookie( 'narayam-enabled', enabledCookie, { 'path': '/', 'expires': 30 } );
 411+ }
 412+
 413+ };
 414+
 415+ /*
 416+ * Construct the menu for Narayam
 417+ */
 418+ this.buildMenu = function() {
389419 var haveSchemes = false;
390420 // Build schemes option list
391421 var $ul = $( '<ul/>' );
@@ -458,35 +488,12 @@
459489 //if rtl, add to the right of top personal links. Else, to the left
460490 var fn = $('body').hasClass( 'rtl' ) ? "append" : "prepend";
461491 $('#p-personal ul:first')[fn]( $li );
462 -
463492 // Build enable/disable checkbox and label
464493 $checkbox = $( '<input type="checkbox" id="narayam-toggle" />' );
465494 $checkbox
466495 .attr( 'title', mw.msg( 'narayam-checkbox-tooltip' ) )
467496 .click( that.toggle );
468 -
469 - // Restore state from cookies
470 - var savedScheme = $.cookie( 'narayam-scheme' );
471 - if ( savedScheme && savedScheme in schemes ) {
472 - that.setScheme( savedScheme );
473 - $( '#narayam-' + savedScheme ).attr( 'checked', 'checked' );
474 - } else {
475 - //if no saved input scheme, select the first.
476 - $('input.narayam-scheme:first').attr( 'checked', 'checked' );
477 - }
478 - var enabledCookie = $.cookie( 'narayam-enabled' );
479 - if ( enabledCookie == '1' || ( mw.config.get( 'wgNarayamEnabledByDefault' ) && enabledCookie !== '0' ) ) {
480 - that.enable();
481 - }
482 - else {
483 - $( 'li#pt-narayam').addClass( 'narayam-inactive' );
484 - }
485 - // Renew the narayam-enabled cookie. naraym-scheme is renewed by setScheme()
486 - if ( enabledCookie ) {
487 - $.cookie( 'narayam-enabled', enabledCookie, { 'path': '/', 'expires': 30 } );
488 - }
489 -
490 - };
 497+ }
491498
492499 } )();
493500

Follow-up revisions

RevisionCommit summaryAuthorDate
r96412Stoppin further actions if no schemes, fix for r96186junaidpv09:24, 7 September 2011

Comments

#Comment by Junaidpv (talk | contribs)   16:23, 3 September 2011

No need to proceed in setup() if number of schemes is zero. Please see usage of 'haveSchemes' variable.

setup() is proceeding with cookie and accessing elements that might be not present in DOM as build() might have not added before. Need fixing.

#Comment by Junaidpv (talk | contribs)   09:25, 7 September 2011

Done: r96186.

#Comment by Junaidpv (talk | contribs)   09:26, 7 September 2011

Sorry. it is r96412.

Status & tagging log