r85502 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r85501‎ | r85502 | r85503 >
Date:01:20, 6 April 2011
Author:tparscal
Status:ok
Tags:
Comment:
Refactored most of the load success code. Added cache = false to make sure IE doesn't load the old data over and over.
Modified paths:
  • /trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js
@@ -60,8 +60,15 @@
6161 'rating': '\
6262 <div class="articleFeedback-rating">\
6363 <div class="articleFeedback-label"></div>\
64 - <div class="articleFeedback-rating-fields articleFeedback-visibleWith-form"><input type="radio" /><input type="radio" /><input type="radio" /><input type="radio" /><input type="radio" /></div>\
65 - <div class="articleFeedback-rating-labels articleFeedback-visibleWith-form"><label></label><label></label><label></label><label></label><label></label><div class="articleFeedback-rating-clear"></div></div>\
 64+ <input type="hidden" />\
 65+ <div class="articleFeedback-rating-labels articleFeedback-visibleWith-form">\
 66+ <div class="articleFeedback-rating-label" rel="1"></div>\
 67+ <div class="articleFeedback-rating-label" rel="2"></div>\
 68+ <div class="articleFeedback-rating-label" rel="3"></div>\
 69+ <div class="articleFeedback-rating-label" rel="4"></div>\
 70+ <div class="articleFeedback-rating-label" rel="5"></div>\
 71+ <div class="articleFeedback-rating-clear"></div>\
 72+ </div>\
6673 <div class="articleFeedback-rating-average articleFeedback-visibleWith-report"></div>\
6774 <div class="articleFeedback-rating-meter articleFeedback-visibleWith-report"><div></div></div>\
6875 <div class="articleFeedback-rating-count articleFeedback-visibleWith-report"></div>\
@@ -98,18 +105,18 @@
99106 },
100107 'updateRating': function() {
101108 var $rating = $(this);
102 - $rating.find( 'label' ).removeClass( 'articleFeedback-rating-label-full' );
103 - var $label = $rating
104 - .find( 'label[for=' + $rating.find( 'input:radio:checked' )
105 - .attr( 'id' ) + ']' );
 109+ $rating.find( '.articleFeedback-rating-label' )
 110+ .removeClass( 'articleFeedback-rating-label-full' );
 111+ var val = $rating.find( 'input:hidden' ).val();
 112+ var $label = $rating.find( '.articleFeedback-rating-label[rel="' + val + '"]' );
106113 if ( $label.length ) {
107114 $label
108 - .prevAll( 'label' )
 115+ .prevAll( '.articleFeedback-rating-label' )
109116 .add( $label )
110117 .addClass( 'articleFeedback-rating-label-full' )
111118 .end()
112119 .end()
113 - .nextAll( 'label' )
 120+ .nextAll( '.articleFeedback-rating-label' )
114121 .removeClass( 'articleFeedback-rating-label-full' );
115122 $rating.find( '.articleFeedback-rating-clear' ).show();
116123 } else {
@@ -132,8 +139,7 @@
133140 var data = {};
134141 for ( var key in context.options.ratings ) {
135142 var id = context.options.ratings[key].id;
136 - // Default to 0 if the radio set doesn't contain a checked element
137 - data['r' + id] = context.$ui.find( 'input[name=r' + id + ']:checked' ).val() || 0;
 143+ data['r' + id] = context.$ui.find( 'input[name="r' + id + '"]' ).val();
138144 }
139145 var expertise = [];
140146 context.$ui.find( '.articleFeedback-expertise input:checked' ).each( function() {
@@ -204,6 +210,7 @@
205211 'type': 'GET',
206212 'dataType': 'json',
207213 'context': context,
 214+ 'cache': false,
208215 'data': {
209216 'action': 'query',
210217 'format': 'json',
@@ -214,103 +221,101 @@
215222 },
216223 'success': function( data ) {
217224 var context = this;
218 - if ( !$.isArray( data.query.articlefeedback ) ) {
219 - mw.log( 'Report response error' );
 225+ if (
 226+ !$.isArray( data.query.articlefeedback )
 227+ || !data.query.articlefeedback.length
 228+ ) {
 229+ mw.log( 'ArticleFeedback invalid response error.' );
220230 context.$ui.find( '.articleFeedback-error' ).show();
221231 return;
222232 }
223 - if (
224 - data.query.articlefeedback.length
225 - && typeof data.query.articlefeedback[0].expertise === 'string'
226 - ) {
227 - var $expertise = context.$ui.find( '.articleFeedback-expertise' );
228 - var tags = data.query.articlefeedback[0].expertise.split( '|' );
229 - for ( var i = 0; i < tags.length; i++ ) {
230 - $expertise.find( 'input:checkbox[value=' + tags[i] + ']' )
231 - .attr( 'checked', true );
 233+ var feedback = data.query.articlefeedback[0];
 234+
 235+ // Expertise
 236+ var $expertise = context.$ui.find( '.articleFeedback-expertise' );
 237+ if ( typeof feedback.expertise === 'string' ) {
 238+ var tags = feedback.expertise.split( '|' );
 239+ if ( $.inArray( 'general', tags ) !== -1 ) {
 240+ $expertise.find( 'input:checkbox' ).each( function() {
 241+ $(this).attr( 'checked', $.inArray( $(this).val(), tags ) !== -1 );
 242+ } );
 243+ // IE7 seriously has issues, and we have to hide, then show
 244+ $expertise.find( '.articleFeedback-expertise-options' )
 245+ .hide().show();
 246+ $.articleFeedback.fn.enableExpertise( $expertise );
232247 }
233 - if ( $expertise.find( 'input:checkbox[value=general]:checked' ).size() ) {
234 - $expertise
235 - .each( function() {
236 - $.articleFeedback.fn.enableExpertise( $(this) );
237 - } )
238 - .find( '.articleFeedback-expertise-options' )
239 - .show();
 248+ } else {
 249+ $expertise
 250+ .find( 'input:checkbox' )
 251+ .attr( 'checked', false )
 252+ .end()
 253+ .find( '.articleFeedback-expertise-options' )
 254+ .hide();
 255+ }
 256+
 257+ // Index rating data by rating ID
 258+ var ratings = {};
 259+ if ( typeof feedback.ratings === 'object' && feedback.ratings !== null ) {
 260+ for ( var i = 0; i < feedback.ratings.length; i++ ) {
 261+ ratings[feedback.ratings[i].ratingid] = feedback.ratings[i];
240262 }
241263 }
242 - var expired = false;
 264+
 265+ // Ratings
243266 context.$ui.find( '.articleFeedback-rating' ).each( function() {
244 - var ratingData;
245 - // Try to get data provided for this rating
 267+ var name = $(this).attr( 'rel' );
 268+ var rating = name in context.options.ratings
 269+ && context.options.ratings[name].id in ratings ?
 270+ ratings[context.options.ratings[name].id] : null;
 271+ // Report
246272 if (
247 - data.query.articlefeedback.length &&
248 - typeof data.query.articlefeedback[0].ratings !== 'undefined'
 273+ rating !== null
 274+ && 'total' in rating
 275+ && 'count' in rating
 276+ && rating.total > 0
249277 ) {
250 - if ( 'status' in data.query.articlefeedback[0] ) {
251 - if ( data.query.articlefeedback[0].status == 'expired' ) {
252 - expired = true;
253 - }
254 - }
255 - if ( 'ratings' in data.query.articlefeedback[0] ) {
256 - var ratingsData = data.query.articlefeedback[0].ratings;
257 - var id = context.options.ratings[$(this).attr( 'rel' )].id;
258 - for ( var i = 0; i < ratingsData.length; i++ ) {
259 - if ( ratingsData[i].ratingid == id ) {
260 - ratingData = ratingsData[i];
261 - }
262 - }
263 - }
264 - }
265 - // Report
266 - if ( typeof ratingData === 'undefined' || ratingData.total == 0 ) {
267 - // Setup in "no ratings" mode
 278+ var average = Math.round( ( rating.total / rating.count ) * 10 ) / 10;
268279 $(this)
269280 .find( '.articleFeedback-rating-average' )
270 - .html( '&nbsp;' )
 281+ .text( average + ( average % 1 === 0 ? '.0' : '' ) )
271282 .end()
272283 .find( '.articleFeedback-rating-meter div' )
273 - .css( 'width', 0 )
 284+ .css( 'width', Math.round( average * 21 ) + 'px' )
274285 .end()
275286 .find( '.articleFeedback-rating-count' )
276 - .text( mw.msg( 'articlefeedback-report-empty' ) )
 287+ .text(
 288+ mw.msg( 'articlefeedback-report-ratings', rating.count )
 289+ )
277290 .end();
278291 } else {
279 - // Setup using ratingData
280 - var average =
281 - Math.round( ( ratingData.total / ratingData.count ) * 10 ) / 10;
 292+ // Special case for no ratings
282293 $(this)
283294 .find( '.articleFeedback-rating-average' )
284 - .text( average + ( average % 1 === 0 ? '.0' : '' ) )
 295+ .html( '&nbsp;' )
285296 .end()
286297 .find( '.articleFeedback-rating-meter div' )
287 - .css( 'width', Math.round( average * 21 ) + 'px' )
 298+ .css( 'width', 0 )
288299 .end()
289300 .find( '.articleFeedback-rating-count' )
290 - .text( mw.msg(
291 - 'articlefeedback-report-ratings', ratingData.count
292 - ) )
 301+ .text( mw.msg( 'articlefeedback-report-empty' ) )
293302 .end();
294303 }
295304 // Form
296 - if ( typeof ratingData.userrating !== 'undefined' ) {
297 - if ( ratingData.userrating == 0 ) {
298 - $(this)
299 - .find( 'input' )
300 - .attr( 'checked', false );
301 - } else {
302 - $(this)
303 - .find( 'input[value="' + ratingData.userrating + '"]' )
304 - .attr( 'checked', true );
 305+ if ( rating !== null && typeof rating.userrating !== 'undefined' ) {
 306+ $(this).find( 'input:hidden' ).val( rating.userrating );
 307+ if ( rating.userrating > 0 ) {
 308+ // If any ratings exist, make sure expertise is enabled so users can
 309+ // suppliment their ratings with expertise information
 310+ $.articleFeedback.fn.enableExpertise( $expertise );
305311 }
306 - // If any ratings exist, make sure expertise is enabled so users can
307 - // suppliment their ratings.
308 - $.articleFeedback.fn.enableExpertise(
309 - context.$ui.find( '.articleFeedback-expertise' )
310 - );
 312+ } else {
 313+ $(this).find( 'input:hidden' ).val( 0 );
311314 }
 315+ // Update rating controls based on the form data
312316 $.articleFeedback.fn.updateRating.call( $(this) );
313317 } );
314 - if ( expired ) {
 318+ // Expiration
 319+ if ( typeof feedback.status === 'string' && feedback.status === 'expired' ) {
315320 context.$ui
316321 .addClass( 'articleFeedback-expired' )
317322 .find( '.articleFeedback-expiry' )
@@ -321,7 +326,7 @@
322327 .find( '.articleFeedback-expiry' )
323328 .slideUp( 'fast' );
324329 }
325 - // If being called just after a submit, we need to un-new the rating controls
 330+ // Status change - un-new the rating controls
326331 context.$ui.find( '.articleFeedback-rating-new' )
327332 .removeClass( 'articleFeedback-rating-new' );
328333 },
@@ -503,26 +508,10 @@
504509 .find( '.articleFeedback-visibleWith-report' )
505510 .hide()
506511 .end()
507 - // Connect labels and fields
 512+ // Name the hidden fields
508513 .find( '.articleFeedback-rating' )
509514 .each( function( rating ) {
510 - var rel = $(this).attr( 'rel' );
511 - var id = 'articleFeedback-rating-field-' + rel + '-';
512 - $(this)
513 - .find( '.articleFeedback-rating-fields input' )
514 - .attr( 'name', rel )
515 - .each( function( field ) {
516 - $(this).attr( {
517 - 'value': field + 1,
518 - 'name': 'r' + ( rating + 1 ),
519 - 'id': id + ( field + 1 )
520 - } );
521 - } )
522 - .end()
523 - .find( '.articleFeedback-rating-labels label' )
524 - .each( function( i ) {
525 - $(this).attr( 'for', id + ( i + 1 ) );
526 - } );
 515+ $(this).find( 'input:hidden' ) .attr( 'name', 'r' + ( rating + 1 ) );
527516 } )
528517 .end()
529518 // Setup switch behavior
@@ -544,18 +533,18 @@
545534 } )
546535 .end()
547536 // Setup rating behavior
548 - .find( '.articleFeedback-rating-labels label' )
 537+ .find( '.articleFeedback-rating-label' )
549538 .hover(
550539 function() {
551540 $(this)
552541 .addClass( 'articleFeedback-rating-label-hover-head' )
553 - .prevAll( 'label' )
 542+ .prevAll( '.articleFeedback-rating-label' )
554543 .addClass( 'articleFeedback-rating-label-hover-tail' );
555544 },
556545 function() {
557546 $(this)
558547 .removeClass( 'articleFeedback-rating-label-hover-head' )
559 - .prevAll( 'label' )
 548+ .prevAll( '.articleFeedback-rating-label' )
560549 .removeClass( 'articleFeedback-rating-label-hover-tail' );
561550 $.articleFeedback.fn.updateRating.call(
562551 $(this).closest( '.articleFeedback-rating' )
@@ -564,6 +553,7 @@
565554 )
566555 .mousedown( function() {
567556 $.articleFeedback.fn.enableSubmission.call( context, true );
 557+
568558 if ( context.$ui.hasClass( 'articleFeedback-expired' ) ) {
569559 // Changing one means the rest will get submitted too
570560 context.$ui
@@ -579,6 +569,9 @@
580570 $(this)
581571 .closest( '.articleFeedback-rating' )
582572 .addClass( 'articleFeedback-rating-new' )
 573+ .find( 'input:hidden' )
 574+ .val( $(this).attr( 'rel' ) )
 575+ .end()
583576 .end()
584577 .addClass( 'articleFeedback-rating-label-down' )
585578 .nextAll()
@@ -597,7 +590,7 @@
598591 $.articleFeedback.fn.enableSubmission.call( context, true );
599592 $(this).hide();
600593 var $rating = $(this).closest( '.articleFeedback-rating' );
601 - $rating.find( 'input:radio' ).attr( 'checked', false );
 594+ $rating.find( 'input:hidden' ).val( 0 );
602595 $.articleFeedback.fn.updateRating.call( $rating );
603596 } );
604597 // Show initial form and report values
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css
@@ -164,17 +164,11 @@
165165 margin-bottom: 0.75em;
166166 }
167167
168 -.articleFeedback-rating-fields {
169 - width: 0px;
170 - height: 0px;
171 - overflow: hidden;
172 -}
173 -
174168 .articleFeedback-rating-labels {
175169 margin-left: 10px;
176170 }
177171
178 -.articleFeedback-rating-labels label, .articleFeedback-rating-clear {
 172+.articleFeedback-rating-label, .articleFeedback-rating-clear {
179173 float: left;
180174 height: 21px;
181175 width: 21px;
@@ -182,7 +176,7 @@
183177 background-position: center center;
184178 cursor: pointer;
185179 }
186 -.articleFeedback-rating-labels label {
 180+.articleFeedback-rating-label {
187181 /* @embed */
188182 background-image: url(images/star-empty.png);
189183 }
@@ -198,28 +192,28 @@
199193 background-image: url(images/trash-hover.png);
200194 }
201195
202 -.articleFeedback-rating-labels label.articleFeedback-rating-label-full {
 196+.articleFeedback-rating-label.articleFeedback-rating-label-full {
203197 /* @embed */
204198 background-image: url(images/star-full.png);
205199 }
206200
207 -.articleFeedback-expired .articleFeedback-rating-labels label.articleFeedback-rating-label-full {
 201+.articleFeedback-expired .articleFeedback-rating-label.articleFeedback-rating-label-full {
208202 /* @embed */
209203 background-image: url(images/star-full-expired.png);
210204 }
211205
212 -.articleFeedback-rating-new .articleFeedback-rating-labels label.articleFeedback-rating-label-full,
213 -.articleFeedback-rating .articleFeedback-rating-labels label.articleFeedback-rating-label-hover-tail {
 206+.articleFeedback-rating-new .articleFeedback-rating-label.articleFeedback-rating-label-full,
 207+.articleFeedback-rating .articleFeedback-rating-label.articleFeedback-rating-label-hover-tail {
214208 /* @embed */
215209 background-image: url(images/star-new.png);
216210 }
217211
218 -.articleFeedback-rating .articleFeedback-rating-labels label.articleFeedback-rating-label-hover-head {
 212+.articleFeedback-rating .articleFeedback-rating-label.articleFeedback-rating-label-hover-head {
219213 /* @embed */
220214 background-image: url(images/star-new-hover.png);
221215 }
222216
223 -.articleFeedback-rating-new .articleFeedback-rating-labels label.articleFeedback-rating-label-down {
 217+.articleFeedback-rating-new .articleFeedback-rating-label.articleFeedback-rating-label-down {
224218 /* @embed */
225219 background-image: url(images/star-new-down.png);
226220 }

Status & tagging log