r86753 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86752‎ | r86753 | r86754 >
Date:11:09, 23 April 2011
Author:krinkle
Status:deferred
Tags:
Comment:
* Remove local mw>mediaWiki aliasing.
* Fix typo from r86739.
* Moving helpimprove-email field into the expertise options.
* Remove duplication between the two, remove placeholder for where bucketing would be done, helpimprove-email is now hidden together with the rest of expertise-options according to the bucketing. (we keep the variable though, since logged-in users and those who have already given the address should still not see the field regardless of the bucket)
* Removing a few redundant config.get fallbacks (r82993)
* Simplify email validation to just applying a background color.
Modified paths:
  • /trunk/extensions/ArticleFeedback/ArticleFeedback.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/SpecialArticleFeedback.php (modified) (history)
  • /trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.startup.js (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/SpecialArticleFeedback.php
——@@ -153,7 +153,7 @@
154154 array(
155155 'title' => 'Main Page',
156156 // List of differences for each rating in the past 7 days
157 - 'differnces' => array( 1, -2, 0, 0 ),
 157+ 'differences' => array( 1, -2, 0, 0 ),
158158 )
159159 );
160160 }
Index: trunk/extensions/ArticleFeedback/ArticleFeedback.hooks.php
——@@ -85,8 +85,6 @@
8686 'articlefeedback-report-empty',
8787 'articlefeedback-report-ratings',
8888 'parentheses',
89 - 'email-address-validity-valid',
90 - 'email-address-validity-invalid',
9189 ),
9290 'dependencies' => array(
9391 'jquery.tipsy',
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.js
——@@ -2,7 +2,7 @@
33 * Script for Article Feedback Plugin
44 */
55
6 -( function( $, mw ) {
 6+( function( $ ) {
77
88 // Only track users who have been assigned to the tracking group
99 var tracked = 'track' === mw.user.bucket(
——@@ -30,19 +30,19 @@
3131 */
3232 var updateMailValidityLabel = function( mail, context ) {
3333 var isValid = mw.util.validateEmail( mail ),
34 - $label = context.$ui.find( '.articleFeedback-helpimprove-email-validity' );
 34+ $that = context.$ui.find( '.articleFeedback-helpimprove-email' );
3535
3636 // We allow empty address
3737 if( isValid === null ) {
38 - $label.text( '' ).removeClass( 'valid invalid' );
 38+ $that.removeClass( 'valid invalid' );
3939
4040 // Valid
4141 } else if ( isValid ) {
42 - $label.text( mw.msg( 'email-address-validity-valid' ) ).addClass( 'valid' ).removeClass( 'invalid' );
 42+ $that.addClass( 'valid' ).removeClass( 'invalid' );
4343
4444 // Not valid
4545 } else {
46 - $label.text( mw.msg( 'email-address-validity-invalid' ) ).addClass( 'invalid' ).removeClass( 'valid' );
 46+ $that.addClass( 'invalid' ).removeClass( 'valid' );
4747 }
4848 };
4949
——@@ -71,17 +71,12 @@
7272 <div><input type="checkbox" value="profession" /><label><html:msg key="form-panel-expertise-profession" /></label></div>\
7373 <div><input type="checkbox" value="hobby" /><label><html:msg key="form-panel-expertise-hobby" /></label></div>\
7474 <div><input type="checkbox" value="other" /><label><html:msg key="form-panel-expertise-other" /></label></div>\
 75+ <div class="articleFeedback-helpimprove"><input type="checkbox" value="helpimprove-email" /><label><html:msg key="form-panel-helpimprove" /><input type="text" placeholder="" class="articleFeedback-helpimprove-email" />\
 76+ <div class="articleFeedback-helpimprove-note"></div>\
 77+ </label></div>\
7578 </div>\
7679 </div>\
7780 <div style="clear:both;"></div>\
78 - <div class="articleFeedback-helpimprove articleFeedback-visibleWith-form" >\
79 - <input type="checkbox" value="on" disabled="disabled" /><label class="articleFeedback-helpimprove-disabled"><html:msg key="form-panel-helpimprove" /></label>\
80 - <div class="articleFeedback-helpimprove-options">\
81 - <div><input type="text" placeholder="" class="articleFeedback-helpimprove-email" /></div>\
82 - <div class="articleFeedback-helpimprove-note"></div>\
83 - </div>\
84 - </div>\
85 - <div style="clear:both;"></div>\
8681 </div>\
8782 <button class="articleFeedback-submit articleFeedback-visibleWith-form" type="submit" disabled="disabled"><html:msg key="form-panel-submit" /></button>\
8883 <div class="articleFeedback-success articleFeedback-visibleWith-form"><span><html:msg key="form-panel-success" /></span></div>\
——@@ -173,14 +168,6 @@
174169 .find( '.articleFeedback-expertise-disabled' )
175170 .removeClass( 'articleFeedback-expertise-disabled' );
176171 },
177 - 'enableHelpimprove': function( $helpimprove ) {
178 - $helpimprove
179 - .find( 'input:checkbox[value=on]' )
180 - .removeAttr( 'disabled' )
181 - .end()
182 - .find( '.articleFeedback-helpimprove-disabled' )
183 - .removeClass( 'articleFeedback-helpimprove-disabled' );
184 - },
185172 'submit': function() {
186173 var context = this;
187174 $.articleFeedback.fn.enableSubmission.call( context, false );
——@@ -228,12 +215,12 @@
229216 } );
230217 // Build data from form values for 'action=emailcapture'
231218 // Ignore if email was invalid
232 - if ( context.$ui.find( '.articleFeedback-helpimprove-email-validity.valid' ).length
 219+ if ( context.$ui.find( '.articleFeedback-helpimprove-email.valid' ).length
233220 // Ignore if email field was empty (it's optional)
234221 && !$.isEmpty( context.$ui.find( '.articleFeedback-helpimprove-email' ).val() )
235222 // Ignore if checkbox was unchecked (ie. user can enter and then decide to uncheck,
236223 // field fades out, then we shouldn't submit)
237 - && $( '#articleFeedback-expertise-on:checked' ).length
 224+ && context.$ui.find('.articleFeedback-helpimprove input:checked' ).length
238225 ) {
239226 $.ajax( {
240227 'url': mw.config.get( 'wgScriptPath' ) + '/api.php',
——@@ -256,7 +243,7 @@
257244 if ( 'error' in data ) {
258245 mw.log( 'EmailCapture: Form submission error' );
259246 mw.log( data.error );
260 - updateMailValidityLabel( 'triggererror' );
 247+ updateMailValidityLabel( 'triggererror', context );
261248
262249 } else {
263250 // Hide helpimprove-email for when user returns to Rate-view
——@@ -278,13 +265,14 @@
279266 // If something was invalid, reset the helpimprove-email part of the form.
280267 // When user returns from submit, it will be clean
281268 } else {
282 - $( '#articleFeedback-expertise-on' ).removeAttr('checked').change();
283269 context.$ui
284 - .find( '.articleFeedback-helpimprove-email' )
285 - .val('')
286 - .end()
287 - .find( '.articleFeedback-helpimprove-email-validity' )
288 - .remove();
 270+ .find( '.articleFeedback-helpimprove' )
 271+ .find( 'input:checkbox' )
 272+ .removeAttr( 'checked' )
 273+ .end()
 274+ .find( '.articleFeedback-helpimprove-email' )
 275+ .val( '' )
 276+ .removeClass( 'valid invalid' );
289277 }
290278 },
291279 'executePitch': function( action ) {
——@@ -370,16 +358,14 @@
371359
372360 var showHelpimprove = true;
373361
374 - // @XXX: Insert bucket thing override here
375 - // bucket thing will set it to false when needed, default is true
376 -
377362 if ( $.cookie( prefix( 'helpimprove-email' ) ) == 'hide'
378363 || !mw.user.anonymous() ) {
379364 showHelpimprove = false;
380365 }
381366
382 - // true: show, false: hide
383 - $helpimprove.toggle( showHelpimprove );
 367+ if ( !showHelpimprove ) {
 368+ $helpimprove.hide();
 369+ }
384370
385371 // Index rating data by rating ID
386372 var ratings = {};
——@@ -558,34 +544,29 @@
559545 }
560546 } )
561547 .end()
562 - .find( '.articleFeedback-helpimprove' )
563 - .find( '.articleFeedback-helpimprove-note' )
564 - // Can't use .text() with mw.message(, /* $1 */ link).toString(),
565 - // because 'link' should not be re-escaped (which would happen if done by mw.message)
566 - .html( function(){
567 - var link = mw.html.element(
568 - 'a', {
569 - href: mw.util.wikiGetlink( mw.msg('articlefeedback-form-panel-helpimprove-privacylink') )
570 - }, mw.msg('articlefeedback-form-panel-helpimprove-privacy')
571 - );
572 - return mw.html.escape( mw.msg( 'articlefeedback-form-panel-helpimprove-note') )
573 - .replace( /\$1/, mw.message( 'parentheses', link ).toString() );
574 - })
575 - .end()
576 - .find( '.articleFeedback-helpimprove-email' )
577 - .attr( 'placeholder', mw.msg( 'articlefeedback-form-panel-helpimprove-email-placeholder' ) )
578 - .placeholder() // back. compat. for older browsers
579 - .one( 'blur', function() {
580 - var $el = $(this);
581 - if ( context.$ui.find( '.articleFeedback-helpimprove-email-validity' ).length === 0 ) {
582 - $el.after( '<div class="articleFeedback-helpimprove-email-validity"></div>' );
583 - }
 548+ .find( '.articleFeedback-helpimprove-note' )
 549+ // Can't use .text() with mw.message(, /* $1 */ link).toString(),
 550+ // because 'link' should not be re-escaped (which would happen if done by mw.message)
 551+ .html( function(){
 552+ var link = mw.html.element(
 553+ 'a', {
 554+ href: mw.util.wikiGetlink( mw.msg('articlefeedback-form-panel-helpimprove-privacylink') )
 555+ }, mw.msg('articlefeedback-form-panel-helpimprove-privacy')
 556+ );
 557+ return mw.html.escape( mw.msg( 'articlefeedback-form-panel-helpimprove-note') )
 558+ .replace( /\$1/, mw.message( 'parentheses', link ).toString() );
 559+ })
 560+ .end()
 561+ .find( '.articleFeedback-helpimprove-email' )
 562+ .attr( 'placeholder', mw.msg( 'articlefeedback-form-panel-helpimprove-email-placeholder' ) )
 563+ .placeholder() // back. compat. for older browsers
 564+ .one( 'blur', function() {
 565+ var $el = $(this);
 566+ updateMailValidityLabel( $el.val(), context );
 567+ $el.keyup( function() {
584568 updateMailValidityLabel( $el.val(), context );
585 - $el.keyup( function() {
586 - updateMailValidityLabel( $el.val(), context );
587 - } );
588 - } )
589 - .end()
 569+ } );
 570+ } )
590571 .end()
591572 .localize( { 'prefix': 'articlefeedback-' } )
592573 // Activate tooltips
——@@ -630,16 +611,6 @@
631612 .next()
632613 .attr( 'for', id );
633614 })
634 - .change( function() {
635 - var $options = context.$ui.find( '.articleFeedback-helpimprove-options' );
636 - if ( $(this).is( ':checked' ) ) {
637 - $options.slideDown( 'fast' );
638 - } else {
639 - $options.slideUp( 'fast', function() {
640 - $options.find( 'input:checkbox' ).attr( 'checked', false );
641 - } );
642 - }
643 - } )
644615 .end()
645616 // Buttonify the button
646617 .find( '.articleFeedback-submit' )
——@@ -740,11 +711,7 @@
741712 .each( function() {
742713 $.articleFeedback.fn.enableExpertise( $(this) );
743714 } )
744 - .end()
745 - .find( '.articleFeedback-helpimprove' )
746 - .each( function() {
747 - $.articleFeedback.fn.enableHelpimprove( $(this) );
748 - } );
 715+ .end();
749716 $(this)
750717 .closest( '.articleFeedback-rating' )
751718 .addClass( 'articleFeedback-rating-new' )
——@@ -822,4 +789,4 @@
823790 return $(this);
824791 };
825792
826 -} )( jQuery, mediaWiki );
 793+} )( jQuery );
Index: trunk/extensions/ArticleFeedback/modules/jquery.articleFeedback/jquery.articleFeedback.css
——@@ -293,16 +293,14 @@
294294 margin-top: 0.75em;
295295 }
296296
297 -.articleFeedback-expertise input,
298 -.articleFeedback-helpimprove input {
 297+.articleFeedback-expertise input {
299298 float: left;
300299 margin-bottom: 0.5em;
301300 clear: both;
302301 cursor: pointer;
303302 }
304303
305 -.articleFeedback-expertise label,
306 -.articleFeedback-helpimprove label {
 304+.articleFeedback-expertise label {
307305 margin-left: 0.25em;
308306 margin-bottom: 0.5em;
309307 float: left;
——@@ -310,8 +308,7 @@
311309 cursor: pointer;
312310 }
313311
314 -.articleFeedback-expertise-options,
315 -.articleFeedback-helpimprove-options {
 312+.articleFeedback-expertise-options {
316313 clear: both;
317314 display: none;
318315 }
——@@ -320,35 +317,24 @@
321318 margin-left: 2em;
322319 }
323320
324 -.articleFeedback-helpimprove-options input {
325 - margin-left: 2em;
 321+.articleFeedback-expertise-options .articleFeedback-helpimprove-email {
 322+ clear: none;
 323+ float: none;
 324+ width: 20em;
 325+ margin-left: 0.25em;
326326 }
327327
328328 .articleFeedback-helpimprove-note {
329 - margin-left: 2em;
330329 font-size: 0.8em;
331330 clear: both;
332331 }
333332
334 -.articleFeedback-helpimprove-email {
335 - width: 20em;
336 -}
337 -
338 -.articleFeedback-helpimprove-email-validity {
339 - float: left;
340 - padding: 2px 0.5em;
341 -}
342 -
343 -.articleFeedback-helpimprove-email-validity.valid {
344 - border: 1px solid #80FF80;
 333+.articleFeedback-helpimprove-email.valid {
345334 background-color: #C0FFC0;
346 - color: black;
347335 }
348336
349 -.articleFeedback-helpimprove-email-validity.invalid {
350 - border: 1px solid #FF8080;
 337+.articleFeedback-helpimprove-email.invalid {
351338 background-color: #FFC0C0;
352 - color: black;
353339 }
354340
355341 .articleFeedback-success {
Index: trunk/extensions/ArticleFeedback/modules/ext.articleFeedback/ext.articleFeedback.startup.js
——@@ -2,12 +2,12 @@
33 * Script for Article Feedback Extension
44 */
55
6 -$(document).ready( function() {
 6+jQuery(document).ready( function( $ ) {
77 if (
88 // Main namespace articles
9 - mw.config.get( 'wgNamespaceNumber', false ) === 0
 9+ mw.config.get( 'wgNamespaceNumber' ) === 0
1010 // View pages
11 - && mw.config.get( 'wgAction', false ) === 'view'
 11+ && mw.config.get( 'wgAction' ) === 'view'
1212 // Current revision
1313 && mw.util.getParamValue( 'diff' ) === null
1414 && mw.util.getParamValue( 'oldid' ) === null

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r82993* Created new ext.articleFeedback.startup module which is a light-weight acti...tparscal00:32, 1 March 2011
r86739Added comments and change the output examples to better reflect specifications.tparscal21:09, 22 April 2011

Status & tagging log