r83252 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r83251‎ | r83252 | r83253 >
Date:22:40, 4 March 2011
Author:tparscal
Status:ok
Tags:
Comment:
* Redsigned pitches to emphasize the content of the pitch
* Inline success notification when no pitch is used
* Restructured layout so pitches can be long without breaking the UI
* Flipped tooltip gravity
* Added lock for all controls while submission is taking place
* Added tracking for pitch presentation
Modified paths:
  • /trunk/extensions/ArticleFeedback/ArticleFeedback.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/ArticleFeedback.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.js (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/images/check.png (modified) (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
——@@ -63,6 +63,7 @@
6464 'articlefeedback-form-panel-expertise-hobby',
6565 'articlefeedback-form-panel-expertise-other',
6666 'articlefeedback-form-panel-submit',
 67+ 'articlefeedback-form-panel-success',
6768 'articlefeedback-report-switch-label',
6869 'articlefeedback-report-panel-title',
6970 'articlefeedback-report-panel-description',
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js
——@@ -11,7 +11,7 @@
1212 'tpl': {
1313 'ui': '\
1414 <div class="articleFeedback-panel">\
15 - <div class="articleFeedback-buffer">\
 15+ <div class="articleFeedback-buffer articleFeedback-ui">\
1616 <div class="articleFeedback-switch articleFeedback-switch-report articleFeedback-visibleWith-form" rel="report"><html:msg key="report-switch-label" /></div>\
1717 <div class="articleFeedback-switch articleFeedback-switch-form articleFeedback-visibleWith-report" rel="form"><html:msg key="form-switch-label" /></div>\
1818 <div class="articleFeedback-title articleFeedback-visibleWith-form"><html:msg key="form-panel-title" /></div>\
——@@ -31,10 +31,14 @@
3232 </div>\
3333 </div>\
3434 <button class="articleFeedback-submit articleFeedback-visibleWith-form" type="submit" disabled><html:msg key="form-panel-submit" /></button>\
 35+ <div class="articleFeedback-success articleFeedback-visibleWith-form"><span><html:msg key="form-panel-success" /></span></div>\
3536 <div style="clear:both;"></div>\
3637 </div>\
3738 <div class="articleFeedback-error"><div class="articleFeedback-error-message"><html:msg key="error" /></div></div>\
 39+ <div class="articleFeedback-pitches"></div>\
 40+ <div style="clear:both;"></div>\
3841 </div>\
 42+<div class="articleFeedback-lock"></div>\
3943 ',
4044 'rating': '\
4145 <div class="articleFeedback-rating">\
——@@ -51,14 +55,29 @@
5256 <div class="articleFeedback-pitch">\
5357 <div class="articleFeedback-buffer">\
5458 <div class="articleFeedback-title"></div>\
55 - <div class="articleFeedback-message"></div>\
56 - <button class="articleFeedback-accept"></button>\
57 - <button class="articleFeedback-reject"></button>\
 59+ <div class="articleFeedback-pop">\
 60+ <div class="articleFeedback-message"></div>\
 61+ <button class="articleFeedback-accept"></button>\
 62+ <button class="articleFeedback-reject"></button>\
 63+ </div>\
5864 </div>\
5965 </div>\
6066 '
6167 },
6268 'fn': {
 69+ 'enableSubmission': function( state ) {
 70+ var context = this;
 71+ if ( state ) {
 72+ // Reset and remove success message
 73+ clearTimeout( context.successTimeout );
 74+ context.$ui.find( '.articleFeedback-success span' ).fadeOut( 'fast' );
 75+ // Enable
 76+ context.$ui.find( '.articleFeedback-submit' ).button( { 'disabled': false } );
 77+ } else {
 78+ // Disable
 79+ context.$ui.find( '.articleFeedback-submit' ).button( { 'disabled': true } );
 80+ }
 81+ },
6382 'updateRating': function() {
6483 var $rating = $(this);
6584 $rating.find( 'label' ).removeClass( 'articleFeedback-rating-label-full' );
——@@ -89,8 +108,8 @@
90109 },
91110 'submit': function() {
92111 var context = this;
93 - // Lock the submit button -- TODO: lock the star inputs too
94 - context.$ui.find( '.articleFeedback-submit' ).button( { 'disabled': true } );
 112+ $.articleFeedback.fn.enableSubmission.call( context, false );
 113+ context.$ui.find( '.articleFeedback-lock' ).show();
95114 // Build data from form values
96115 var data = {};
97116 for ( var key in context.options.ratings ) {
——@@ -119,6 +138,7 @@
120139 'success': function( data ) {
121140 var context = this;
122141 $.articleFeedback.fn.load.call( context );
 142+ context.$ui.find( '.articleFeedback-lock' ).hide();
123143 },
124144 'error': function() {
125145 var context = this;
——@@ -128,26 +148,28 @@
129149 } );
130150 },
131151 'executePitch': function( action ) {
132 - $(this)
133 - .closest( '.articleFeedback-pitch' )
134 - .find( '.articleFeedback-accept, .articleFeedback-altAccept' )
135 - .button( { 'disabled': true } )
136 - .end()
137 - .find( '.articleFeedback-reject' )
138 - .attr( 'disabled', true );
139152 var $pitch = $(this).closest( '.articleFeedback-pitch' );
 153+ $pitch
 154+ .find( '.articleFeedback-accept, .articleFeedback-altAccept' )
 155+ .button( { 'disabled': true } )
 156+ .end()
 157+ .find( '.articleFeedback-reject' )
 158+ .attr( 'disabled', true );
140159 var key = $pitch.attr( 'rel' );
141160 // If a pitch's action returns true, hide the pitch and
142161 // re-enable the button
143162 if ( action.call( $(this) ) ) {
144 - $pitch.fadeOut();
145 - $(this)
146 - .closest( '.articleFeedback-pitch' )
147 - .find( '.articleFeedback-accept, .articleFeedback-altAccept' )
148 - .button( { 'disabled': false } )
149 - .end()
150 - .find( '.articleFeedback-reject' )
151 - .attr( 'disabled', false );
 163+ $pitch
 164+ .fadeOut()
 165+ .find( '.articleFeedback-accept, .articleFeedback-altAccept' )
 166+ .button( { 'disabled': false } )
 167+ .end()
 168+ .find( '.articleFeedback-reject' )
 169+ .attr( 'disabled', false )
 170+ .end()
 171+ .closest( '.articleFeedback-panel' )
 172+ .find( '.articleFeedback-ui' )
 173+ .show();
152174 }
153175 return false;
154176 },
——@@ -274,72 +296,76 @@
275297 }
276298 } )
277299 .end()
278 - .each( function() {
279 - for ( var key in context.options.pitches ) {
280 - var $pitch = $( $.articleFeedback.tpl.pitch )
281 - .attr( 'rel', key )
282 - .find( '.articleFeedback-title' )
283 - .text( mw.msg( context.options.pitches[key].title ) )
284 - .end()
285 - .find( '.articleFeedback-message' )
286 - .text( mw.msg( context.options.pitches[key].message ) )
287 - .end()
288 - .find( '.articleFeedback-accept' )
289 - .text( mw.msg( context.options.pitches[key].accept ) )
290 - .click( function() {
291 - var $pitch = $(this).closest( '.articleFeedback-pitch' );
292 - return $.articleFeedback.fn.executePitch.call(
293 - $(this),
294 - context.options.pitches[$pitch.attr( 'rel' )].action
295 - );
296 - } )
297 - .button()
298 - .end()
299 - .find( '.articleFeedback-reject' )
300 - .text( mw.msg( context.options.pitches[key].reject ) )
301 - .click( function() {
302 - var $pitch = $(this).closest( '.articleFeedback-pitch' );
303 - // Remember that the users rejected this, set a cookie to not
304 - // show this for 3 days
305 - $.cookie(
306 - 'jquery.articleFeedback.pitch.' + $pitch.attr( 'rel' ),
307 - 'hide',
308 - { 'expires': 3 }
309 - );
310 - $pitch.fadeOut();
311 - } )
312 - .end()
313 - .appendTo( $(this) );
314 - if (
315 - typeof context.options.pitches[key].altAccept == 'string'
316 - && typeof context.options.pitches[key].altAction == 'function'
317 - ) {
318 - $pitch
 300+ .find( '.articleFeedback-pitches' )
 301+ .each( function() {
 302+ for ( var key in context.options.pitches ) {
 303+ var $pitch = $( $.articleFeedback.tpl.pitch )
 304+ .attr( 'rel', key )
 305+ .find( '.articleFeedback-title' )
 306+ .text( mw.msg( context.options.pitches[key].title ) )
 307+ .end()
 308+ .find( '.articleFeedback-message' )
 309+ .text( mw.msg( context.options.pitches[key].message ) )
 310+ .end()
319311 .find( '.articleFeedback-accept' )
320 - .after( '<button class="articleFeedback-altAccept"></button>' )
321 - .after(
322 - $( '<span class="articleFeedback-pitch-or"></span>' )
323 - .text( mw.msg( 'articlefeedback-pitch-or' ) )
324 - )
325 - .end()
326 - .find( '.articleFeedback-altAccept' )
327 - .text( mw.msg( context.options.pitches[key].altAccept ) )
 312+ .text( mw.msg( context.options.pitches[key].accept ) )
328313 .click( function() {
329314 var $pitch = $(this).closest( '.articleFeedback-pitch' );
330315 return $.articleFeedback.fn.executePitch.call(
331316 $(this),
332 - context.options.pitches[$pitch.attr( 'rel' )].altAction
 317+ context.options.pitches[$pitch.attr( 'rel' )].action
333318 );
334319 } )
335 - .button();
 320+ .button()
 321+ .end()
 322+ .find( '.articleFeedback-reject' )
 323+ .text( mw.msg( context.options.pitches[key].reject ) )
 324+ .click( function() {
 325+ var $pitch = $(this).closest( '.articleFeedback-pitch' );
 326+ // Remember that the users rejected this, set a cookie to not
 327+ // show this for 3 days
 328+ $.cookie(
 329+ 'jquery.articleFeedback-pitch.' + $pitch.attr( 'rel' ),
 330+ 'hide',
 331+ { 'expires': 3 }
 332+ );
 333+ $pitch.fadeOut( 'fast', function() {
 334+ context.$ui.find( '.articleFeedback-ui' ).show();
 335+ } );
 336+ } )
 337+ .end()
 338+ .appendTo( $(this) );
 339+ if (
 340+ typeof context.options.pitches[key].altAccept == 'string'
 341+ && typeof context.options.pitches[key].altAction == 'function'
 342+ ) {
 343+ $pitch
 344+ .find( '.articleFeedback-accept' )
 345+ .after( '<button class="articleFeedback-altAccept"></button>' )
 346+ .after(
 347+ $( '<span class="articleFeedback-pitch-or"></span>' )
 348+ .text( mw.msg( 'articlefeedback-pitch-or' ) )
 349+ )
 350+ .end()
 351+ .find( '.articleFeedback-altAccept' )
 352+ .text( mw.msg( context.options.pitches[key].altAccept ) )
 353+ .click( function() {
 354+ var $pitch = $(this).closest( '.articleFeedback-pitch' );
 355+ return $.articleFeedback.fn.executePitch.call(
 356+ $(this),
 357+ context.options.pitches[$pitch.attr( 'rel' )].altAction
 358+ );
 359+ } )
 360+ .button();
 361+ }
336362 }
337 - }
338 - } )
 363+ } )
 364+ .end()
339365 .localize( { 'prefix': 'articlefeedback-' } )
340366 // Activate tooltips
341367 .find( '[title]' )
342368 .tipsy( {
343 - 'gravity': 'nw',
 369+ 'gravity': 'sw',
344370 'center': false,
345371 'fade': true,
346372 'delayIn': 300,
——@@ -363,9 +389,7 @@
364390 var id = 'articleFeedback-expertise-' + $(this).attr( 'value' );
365391 $(this)
366392 .click( function() {
367 - context.$ui
368 - .find( '.articleFeedback-submit' )
369 - .button( { 'disabled': false } );
 393+ $.articleFeedback.fn.enableSubmission.call( context, true );
370394 } )
371395 .attr( 'id', id )
372396 .next()
——@@ -377,17 +401,35 @@
378402 .button()
379403 .click( function() {
380404 $.articleFeedback.fn.submit.call( context );
 405+ var pitches = [];
381406 for ( var key in context.options.pitches ) {
382407 // Dont' bother checking the condition if there's a cookie that says
383408 // the user has rejected this within 3 days of right now
384 - var display = $.cookie( 'jquery.articleFeedback.pitch.' + key );
 409+ var display = $.cookie( 'jquery.articleFeedback-pitch.' + key );
385410 if ( display !== 'hide' && context.options.pitches[key].condition() ) {
386 - context.$ui
387 - .find( '.articleFeedback-pitch[rel="' + key + '"]' )
388 - .show();
389 - break;
 411+ pitches.push( key );
390412 }
391413 }
 414+ if ( pitches.length ) {
 415+ // Select randomly using equal distribution of available pitches
 416+ var key = pitches[Math.round( Math.random() * ( pitches.length - 1 ) )];
 417+ context.$ui.find( '.articleFeedback-pitches' )
 418+ .css( 'width', context.$ui.width() )
 419+ .find( '.articleFeedback-pitch[rel="' + key + '"]' )
 420+ .fadeIn( 'fast' );
 421+ context.$ui.find( '.articleFeedback-ui' ).hide();
 422+ // Track that a pitch was presented
 423+ if ( typeof $.trackActionWithInfo == 'function' ) {
 424+ $.trackActionWithInfo( 'jquery.articlefeedback-pitch', key );
 425+ }
 426+ } else {
 427+ // Give user some feedback that a save occured
 428+ context.$ui.find( '.articleFeedback-success span' ).fadeIn( 'fast' );
 429+ context.successTimeout = setTimeout( function() {
 430+ context.$ui.find( '.articleFeedback-success span' )
 431+ .fadeOut( 'slow' );
 432+ }, 5000 );
 433+ }
392434 } )
393435 .end()
394436 // Hide report elements initially
——@@ -454,10 +496,8 @@
455497 }
456498 )
457499 .mousedown( function() {
 500+ $.articleFeedback.fn.enableSubmission.call( context, true );
458501 context.$ui
459 - .find( '.articleFeedback-submit' )
460 - .button( { 'disabled': false } )
461 - .end()
462502 .find( '.articleFeedback-expertise' )
463503 .each( function() {
464504 $.articleFeedback.fn.enableExpertise( $(this) );
——@@ -480,9 +520,7 @@
481521 .end()
482522 .find( '.articleFeedback-rating-clear' )
483523 .click( function() {
484 - context.$ui
485 - .find( '.articleFeedback-submit' )
486 - .button( { 'disabled': false } );
 524+ $.articleFeedback.fn.enableSubmission.call( context, true );
487525 $(this).hide();
488526 var $rating = $(this).closest( '.articleFeedback-rating' );
489527 $rating.find( 'input:radio' ).attr( 'checked', false );
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/images/check.png
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css
——@@ -18,19 +18,41 @@
1919 text-align: center;
2020 }
2121
22 -.articleFeedback-pitch,
2322 .articleFeedback-error {
2423 display: none;
2524 position: absolute;
2625 top: 0;
2726 left: 0;
28 - bottom: 0;
2927 right: 0;
3028 background-color: #f9f9f9;
3129 border: 1px solid #cccccc;
3230 padding-bottom: 1px;
3331 }
3432
 33+.articleFeedback-lock {
 34+ display: none;
 35+ position: absolute;
 36+ top: 0;
 37+ left: 0;
 38+ right: 0;
 39+}
 40+
 41+.articleFeedback-pitches {
 42+ float: absolute;
 43+ top: 1;
 44+ left: 1;
 45+ right: 1;
 46+ background-color: #f9f9f9;
 47+}
 48+
 49+.articleFeedback-pitch {
 50+ display: none;
 51+}
 52+
 53+.articleFeedback-lock {
 54+ background-color: transparent;
 55+}
 56+
3557 .articleFeedback-pitch-or {
3658 margin-left: 0.75em;
3759 margin-right: 0.25em;
——@@ -38,7 +60,7 @@
3961
4062 .articleFeedback-reject {
4163 border: none;
42 - background-color: #f9f9f9;
 64+ background-color: transparent;
4365 cursor: pointer;
4466 color: #0645AD;
4567 line-height: 1.4em;
——@@ -65,16 +87,24 @@
6688 }
6789
6890 .articleFeedback-pitch .articleFeedback-title {
69 - padding-left: 60px;
70 - padding-bottom: 0.5em;
71 - line-height: 50px;
 91+ padding-left: 28px;
 92+ line-height: 32px;
7293 background-image: url(images/check.png);
7394 background-repeat: no-repeat;
74 - background-position: left top;
 95+ background-position: left center;
7596 margin-bottom: 0.5em;
76 - border-bottom: solid 1px #dddddd;
7797 }
7898
 99+.articleFeedback-pitch .articleFeedback-pop {
 100+ padding: 1em;
 101+ margin: 0;
 102+ background-color: #CCFFCC;
 103+ text-align: center;
 104+ -webkit-border-radius: 5px;
 105+ -moz-border-radius: 5px;
 106+ border-radius: 5px;
 107+}
 108+
79109 .articleFeedback-message {
80110 margin: 0.5em;
81111 font-size: larger;
——@@ -242,6 +272,7 @@
243273 .articleFeedback-expertise {
244274 float: left;
245275 margin-bottom: 0.5em;
 276+ margin-top: 0.75em;
246277 }
247278
248279 .articleFeedback-expertise-disabled {
——@@ -271,3 +302,20 @@
272303 .articleFeedback-expertise-options input {
273304 margin-left: 2em;
274305 }
 306+
 307+.articleFeedback-success {
 308+ float: right;
 309+}
 310+.articleFeedback-success span {
 311+ display: none;
 312+ background-image: url(images/check.png);
 313+ background-repeat: no-repeat;
 314+ background-position: center left;
 315+ padding-left: 28px;
 316+ padding-right: 12px;
 317+ padding-top: 12px;
 318+ padding-bottom: 12px;
 319+ color: green;
 320+ font-size: 0.8em;
 321+ line-height: 3.6em;
 322+}
Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.js
——@@ -5,25 +5,13 @@
66 ( function( $, mw ) {
77
88 /**
9 - * Tries to win the lottery based on set odds.
10 - *
11 - * Odds are clamped to the range of 0-1.
12 - *
13 - * @param odds Float: Probability of winning in range of 0 (never) to 1 (always)
14 - * @return Boolean: Whether you are a winner
15 - */
16 -function lottery( odds ) {
17 - return Math.random() <= Math.min( 1, Math.max( 0, odds ) );
18 -};
19 -
20 -/**
219 * Checks if a pitch is currently muted
2210 *
2311 * @param pitch String: Name of pitch to check
2412 * @return Boolean: Whether the pitch is muted
2513 */
26 -function isPitchMuted( pitch ) {
27 - return $.cookie( 'ext.articleFeedback.pitches.' + pitch ) == 'hide' ? true : false;
 14+function isPitchVisible( pitch ) {
 15+ return $.cookie( 'ext.articleFeedback-pitches.' + pitch ) == 'hide' ? false : true;
2816 }
2917
3018 /**
——@@ -33,14 +21,14 @@
3422 * @param durration Integer: Number of days to mute the pitch for
3523 */
3624 function mutePitch( pitch, durration ) {
37 - $.cookie( 'ext.articleFeedback.pitches.' + pitch, 'hide', { 'expires': durration } );
 25+ $.cookie( 'ext.articleFeedback-pitches.' + pitch, 'hide', { 'expires': durration } );
3826 }
3927
4028 function trackClick( id ) {
4129 // Track the click so we can figure out how useful this is
4230 if ( typeof $.trackActionWithInfo == 'function' ) {
4331 $.trackActionWithInfo(
44 - 'articlefeedback-' + id, mediaWiki.config.get( 'wgTitle' )
 32+ 'ext.articleFeedback-' + id, mediaWiki.config.get( 'wgTitle' )
4533 )
4634 }
4735 }
——@@ -206,8 +194,7 @@
207195 'pitches': {
208196 'survey': {
209197 'condition': function() {
210 - // If this pitch isn't muted, show this 1/3 of the time
211 - return !isPitchMuted( 'survey' ) ? lottery( 0.333 ) : false;
 198+ return isPitchVisible( 'survey' );
212199 },
213200 'action': function() {
214201 survey.load();
——@@ -223,9 +210,7 @@
224211 },
225212 'join': {
226213 'condition': function() {
227 - // If this pitch isn't muted and the user is anonymous, show this 1/2 of the time
228 - return ( !isPitchMuted( 'join' ) && mediaWiki.user.anonymous() )
229 - ? lottery( 0.5 ) : false;
 214+ return isPitchVisible( 'join' ) && !mediaWiki.user.anonymous();
230215 },
231216 'action': function() {
232217 // Click tracking
——@@ -265,8 +250,8 @@
266251 'condition': function() {
267252 // An empty restrictions array means anyone can edit
268253 var restrictions = mediaWiki.config.get( 'wgRestrictionEdit' );
269 - var groups = mediaWiki.config.get( 'wgUserGroups' );
270254 if ( restrictions.length ) {
 255+ var groups = mediaWiki.config.get( 'wgUserGroups' );
271256 // Verify that each restriction exists in the user's groups
272257 for ( var i = 0; i < restrictions.length; i++ ) {
273258 if ( !$.inArray( restrictions[i], groups ) ) {
——@@ -274,8 +259,7 @@
275260 }
276261 }
277262 }
278 - // If this pitch isn't muted, show this always
279 - return !isPitchMuted( 'edit' );
 263+ return isPitchVisible( 'edit' );
280264 },
281265 'action': function() {
282266 // Click tracking
Index: trunk/extensions/ArticleFeedback/ArticleFeedback.i18n.php
——@@ -36,6 +36,7 @@
3737 'articlefeedback-form-panel-expertise-hobby' => 'It is related to my hobbies or interests',
3838 'articlefeedback-form-panel-expertise-other' => 'The source of my knowledge is not listed here',
3939 'articlefeedback-form-panel-submit' => 'Submit feedback',
 40+ 'articlefeedback-form-panel-success' => 'Saved successfully',
4041 'articlefeedback-report-switch-label' => 'View page ratings',
4142 'articlefeedback-report-panel-title' => 'Page ratings',
4243 'articlefeedback-report-panel-description' => 'Current average ratings.',

Status & tagging log