r112849 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r112848‎ | r112849 | r112850 >
Date:23:04, 1 March 2012
Author:krinkle
Status:ok
Tags:
Comment:
[CategoryTree] clean up JS, fix undefined bug
* Use dashes versions of data attributes to make it easier to find them (now the data attributes as found in PHP where no where to be found in any JS file, appeared to be bogus)
* Don't create a new <div> element ($parentTag) only to store a single string in it's data object, using local variables for ctTitle, ctMode and ctOptions instead.
Modified paths:
  • /trunk/extensions/CategoryTree/modules/ext.categoryTree.js (modified) (history)

Diff [purge]

Index: trunk/extensions/CategoryTree/modules/ext.categoryTree.js
@@ -27,7 +27,7 @@
2828 */
2929 handleNode: function( e ) {
3030 var $link = $( this );
31 - if ( $link.data( 'ctState' ) === 'collapsed' ) {
 31+ if ( $link.data( 'ct-state' ) === 'collapsed' ) {
3232 categoryTree.expandNode( $link );
3333 } else {
3434 categoryTree.collapseNode( $link );
@@ -48,9 +48,9 @@
4949 $link
5050 .html( mw.msg( 'categorytree-collapse-bullet' ) )
5151 .attr( 'title', mw.msg( 'categorytree-collapse' ) )
52 - .data( 'ctState', 'expanded' );
 52+ .data( 'ct-state', 'expanded' );
5353
54 - if ( !$link.data( 'ctLoaded' ) ) {
 54+ if ( !$link.data( 'ct-loaded' ) ) {
5555 categoryTree.loadChildren( $link, $children );
5656 }
5757 },
@@ -68,7 +68,7 @@
6969 $link
7070 .html( mw.msg( 'categorytree-expand-bullet' ) )
7171 .attr( 'title', mw.msg( 'categorytree-expand' ) )
72 - .data( 'ctState', 'collapsed' );
 72+ .data( 'ct-state', 'collapsed' );
7373 },
7474
7575 /**
@@ -78,26 +78,53 @@
7979 * @param {jQuery} $children
8080 */
8181 loadChildren: function( $link, $children ) {
82 - $link.data( 'ctLoaded', true );
 82+ var $linkParentCTTag, ctTitle, ctMode, ctOptions;
 83+
 84+ /**
 85+ * Error callback
 86+ */
 87+ function error() {
 88+ var $retryLink;
 89+
 90+ $retryLink = $( '<a>' )
 91+ .text( mw.msg( 'categorytree-retry' ) )
 92+ .attr( 'href', '#' )
 93+ .click( function() {
 94+ categoryTree.loadChildren( $link, $children );
 95+ } );
 96+
 97+ $children
 98+ .text( mw.msg( 'categorytree-error' ) )
 99+ .append( $retryLink );
 100+ }
 101+
 102+ $link.data( 'ct-loaded', true );
 103+
83104 $children.html(
84105 $( '<i class="CategoryTreeNotice"></i>' )
85106 .text( mw.msg( 'categorytree-loading' ) )
86107 );
87108
88 - var $parentTag = $link.parents( '.CategoryTreeTag' );
 109+ $linkParentCTTag = $link.parents( '.CategoryTreeTag' );
89110
90 - if ( $parentTag.length === 0 ) {
91 - // Probably a CategoryPage
92 - $parentTag = $( '<div />' )
93 - .hide()
94 - .data( 'ctOptions', mw.config.get( 'wgCategoryTreePageCategoryOptions' ) );
 111+ // Element may not have a .CategoryTreeTag parent, fallback to defauls
 112+ // Probably a CategoryPage (@todo: based on what?)
 113+ ctTitle = $link.data( 'ct-title' );
 114+ ctMode = $linkParentCTTag.data( 'ct-mode' ) || undefined;
 115+ ctOptions = $linkParentCTTag.data( 'ct-options' ) || mw.config.get( 'wgCategoryTreePageCategoryOptions' );
 116+
 117+ // Mode and options have defaults or fallbacks, title does not.
 118+ // Don't make a request if there is no title.
 119+ if ( typeof ctTitle !== 'string' ) {
 120+ error();
 121+ return;
95122 }
96123
97124 $.get(
98125 mw.util.wikiScript(), {
99126 action: 'ajax',
100127 rs: 'efCategoryTreeAjaxWrapper',
101 - rsargs: [$link.data( 'ctTitle' ), $parentTag.data( 'ctOptions' ), 'json'] // becomes &rsargs[]=arg1&rsargs[]=arg2...
 128+ rsargs: [ctTitle, ctOptions, 'json'] // becomes &rsargs[]=arg1&rsargs[]=arg2...
102129 }
103130 )
104131 .success( function ( data ) {
@@ -105,7 +132,7 @@
106133 data = data.replace(/##LOAD##/g, mw.msg( 'categorytree-expand' ) );
107134
108135 if ( data === '' ) {
109 - switch ( $parentTag.data( 'ctMode' ) ) {
 136+ switch ( ctMode ) {
110137 case 0:
111138 data = mw.msg( 'categorytree-no-subcategories' );
112139 break;
@@ -128,15 +155,7 @@
129156 .click( categoryTree.handleNode );
130157 categoryTree.showToggles();
131158 } )
132 - .error( function() {
133 - var $retryLink = $( '<a />' )
134 - .text( mw.msg( 'categorytree-retry' ) )
135 - .attr( 'href', '#' )
136 - .click( function() { categoryTree.loadChildren( $link, $children ); } );
137 - $children
138 - .text( mw.msg( 'categorytree-error' ) )
139 - .append( $retryLink );
140 - } );
 159+ .error( error );
141160 }
142161 };
143162

Follow-up revisions

RevisionCommit summaryAuthorDate
r112851[CategoryTree] clean up JS...krinkle23:43, 1 March 2012
r112862MFT r112849, r112851, r112856, r112859aaron00:40, 2 March 2012
r113037MFT r110703, r110933, r111011, r111218, r112520, r112524, r112660, r112687, r...reedy14:59, 5 March 2012

Status & tagging log