r87562 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r87561‎ | r87562 | r87563 >
Date:18:55, 6 May 2011
Author:tparscal
Status:ok (Comments)
Tags:
Comment:
Added pending message to draw attention to the submit button when ratings are not saved yet.
Modified paths:
  • /trunk/extensions/ArticleFeedback/ArticleFeedback.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/ArticleFeedback.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/images/attention.png (added) (history)
  • /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/ArticleFeedback.hooks.php
@@ -77,6 +77,7 @@
7878 'articlefeedback-form-panel-helpimprove-privacylink',
7979 'articlefeedback-form-panel-submit',
8080 'articlefeedback-form-panel-success',
 81+ 'articlefeedback-form-panel-pending',
8182 'articlefeedback-form-panel-expiry-title',
8283 'articlefeedback-form-panel-expiry-message',
8384 'articlefeedback-report-switch-label',
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js
@@ -83,6 +83,7 @@
8484 </div>\
8585 <button class="articleFeedback-submit articleFeedback-visibleWith-form" type="submit" disabled="disabled"><html:msg key="form-panel-submit" /></button>\
8686 <div class="articleFeedback-success articleFeedback-visibleWith-form"><span><html:msg key="form-panel-success" /></span></div>\
 87+ <div class="articleFeedback-pending articleFeedback-visibleWith-form"><span><html:msg key="form-panel-pending" /></span></div>\
8788 <div style="clear:both;"></div>\
8889 <div class="articleFeedback-notices articleFeedback-visibleWith-form">\
8990 <div class="articleFeedback-expiry">\
@@ -133,14 +134,28 @@
134135 'enableSubmission': function( state ) {
135136 var context = this;
136137 if ( state ) {
137 - // Reset and remove success message
 138+ // Reset success timeout
138139 clearTimeout( context.successTimeout );
139 - context.$ui.find( '.articleFeedback-success span' ).fadeOut( 'fast' );
140 - // Enable
141 - context.$ui.find( '.articleFeedback-submit' ).button( { 'disabled': false } );
 140+ context.$ui
 141+ // Enable
 142+ .find( '.articleFeedback-submit' )
 143+ .button( { 'disabled': false } )
 144+ .end()
 145+ // Hide success
 146+ .find( '.articleFeedback-success span' )
 147+ .hide()
 148+ .end()
 149+ // Show pending
 150+ .find( '.articleFeedback-pending span' )
 151+ .fadeIn( 'fast' );
142152 } else {
143153 // Disable
144 - context.$ui.find( '.articleFeedback-submit' ).button( { 'disabled': true } );
 154+ context.$ui
 155+ .find( '.articleFeedback-submit' )
 156+ .button( { 'disabled': true } )
 157+ .end()
 158+ .find( '.articleFeedback-pending span' )
 159+ .hide();
145160 }
146161 },
147162 'updateRating': function() {
@@ -647,7 +662,7 @@
648663 }
649664 if ( pitches.length ) {
650665 // Select randomly using equal distribution of available pitches
651 - var key = pitches[Math.floor( Math.random() * list.length )];
 666+ var key = pitches[Math.floor( Math.random() * pitches.length )];
652667 context.$ui.find( '.articleFeedback-pitches' )
653668 .css( 'width', context.$ui.width() )
654669 .find( '.articleFeedback-pitch[rel="' + key + '"]' )
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/images/attention.png
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/images/attention.png
___________________________________________________________________
Added: svn:mime-type
655670 + application/octet-stream
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css
@@ -344,25 +344,32 @@
345345 background-color: #FFC0C0;
346346 }
347347
 348+.articleFeedback-pending,
348349 .articleFeedback-success {
349350 float: right;
350351 }
351352
 353+.articleFeedback-pending span,
352354 .articleFeedback-success span {
353355 display: none;
354 - /* @embed */
355 - background-image: url(images/success.png);
 356+ padding: 12px 12px 12px 28px;
 357+ font-size: 0.8em;
 358+ line-height: 3.6em;
356359 background-repeat: no-repeat;
357360 background-position: center left;
358 - padding-left: 28px;
359 - padding-right: 12px;
360 - padding-top: 12px;
361 - padding-bottom: 12px;
362361 color: green;
363 - font-size: 0.8em;
364 - line-height: 3.6em;
365362 }
366363
 364+.articleFeedback-pending span {
 365+ /* @embed */
 366+ background-image: url(images/attention.png);
 367+}
 368+
 369+.articleFeedback-success span {
 370+ /* @embed */
 371+ background-image: url(images/success.png);
 372+}
 373+
367374 .articleFeedback-expiry {
368375 display: none;
369376 border: solid 1px orange;
Index: trunk/extensions/ArticleFeedback/ArticleFeedback.i18n.php
@@ -43,6 +43,7 @@
4444 'articlefeedback-form-panel-helpimprove-privacy' => 'Privacy policy',
4545 'articlefeedback-form-panel-helpimprove-privacylink' => 'Project:Privacy policy',
4646 'articlefeedback-form-panel-submit' => 'Submit ratings',
 47+ 'articlefeedback-form-panel-pending' => 'Your ratings have not been submitted',
4748 'articlefeedback-form-panel-success' => 'Saved successfully',
4849 'articlefeedback-form-panel-expiry-title' => 'Your ratings have expired',
4950 'articlefeedback-form-panel-expiry-message' => 'Please reevaluate this page and submit new ratings.',

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r87467Improved on r87466 - this is much simpler and more fair.tparscal23:04, 4 May 2011

Comments

#Comment by Krinkle (talk | contribs)   16:51, 9 June 2011

Can you brielfy explain this one ?

 							// Select randomly using equal distribution of available pitches
-							var key = pitches[Math.floor( Math.random() * list.length )];
+							var key = pitches[Math.floor( Math.random() * pitches.length )];

Was it broken before ? OK otherwise.

#Comment by Catrope (talk | contribs)   17:09, 9 June 2011

When indexing in pitches[] it's generally a good idea to use pitches.length as the upper bound, as opposed to the length of some other array :)

#Comment by Trevor Parscal (WMF) (talk | contribs)   18:08, 9 June 2011

Yes, especially if that "other" array is an undefined symbol!

Status & tagging log