r105054 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105053‎ | r105054 | r105055 >
Date:03:57, 3 December 2011
Author:rsterbin
Status:deferred
Tags:
Comment:
Cleaned up form validation display:
- ArticleFeedbackv5.i18n.php:
- New message 'articlefeedbackv5-error-nofeedback'
- Moved 'articlefeedbackv5-error' up with the other errors and added a
message doc line
- ArticleFeedbackv5.hooks.php:
- Added 'articlefeedbackv5-error-nofeedback' to the jquery module
requirements
- modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css:
- Added some barebones styles for the form error
- modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js:
- Added a top message block to the forms for buckets 1, 2, and 3
- Added localValidation() to buckets 1, 2, and 3 checking to make sure
the user has actually entered something
- Replaced markFormError() in the buckets all the buckets with a
top-level new method markFormErrors()
- Renamed bucket 5's markFormError() to markFormErrors() for clarity
- Updated submitForm() to call markFormError()
- Added a new convenience method markTopError() and updated
submitForm() and markFormErrors() to use it

Other:
- modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js:
- Replaced missing email in bucket 5's getFormData() -- lost in r105031
- Brought tool tip link into line with usual jquery style
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
——@@ -17,10 +17,12 @@
1818 'articlefeedbackv5-cta1-edit-linktext' => 'Edit this page',
1919
2020 // error messages
 21+ 'articlefeedbackv5-error' => 'An error has occured. Please try again later.',
2122 'articlefeedbackv5-error-email' => 'That email address is not valid',
2223 'articlefeedbackv5-error-validation' => 'Validation error',
2324 'articlefeedbackv5-error-unknown' => 'Unknown error',
2425 'articlefeedbackv5-error-submit' => 'Form submission error.',
 26+ 'articlefeedbackv5-error-nofeedback' => 'Please enter your feedback.',
2527
2628 // Article Feedback special page.
2729 'articlefeedbackv5-form-optionid' => 'Option $1',
——@@ -143,7 +145,6 @@
144146 'articlefeedbackv5-bucket5-wellwritten-tooltip-4' => 'Good clarity',
145147 'articlefeedbackv5-bucket5-wellwritten-tooltip-5' => 'Exceptional clarity',
146148 /* Messages shared by all displays */
147 - 'articlefeedbackv5-error' => 'An error has occured. Please try again later.',
148149 'articlefeedbackv5-shared-on-feedback' => 'Your comment will be shared on this $1.',
149150 'articlefeedbackv5-shared-on-feedback-linktext' => 'feedback page',
150151 'articlefeedbackv5-help-tooltip-title' => 'What\'s this?',
——@@ -304,6 +305,8 @@
305306 This message is used in JavaScript by module 'jquery.articleFeedback'.
306307 $1 is an integer, and the rating count.",
307308 /* Messages shared by all displays */
 309+ 'articlefeedbackv5-error' => 'This error message will be displayed in a grey box replacing the form if there was an unrecoverable error.',
 310+ 'articlefeedbackv5-error-nofeedback' => 'This error message will be displayed above the form (but below the title) if the user has attempted to submit a blank form.',
308311 'articlefeedbackv5-shared-on-feedback' => 'This is the top line of the small text that goes beside the submit button and lets the user know that their comment will be posted on the feedback page. $1 will hold the link to the feedback page.',
309312 'articlefeedbackv5-shared-on-feedback-linktext' => 'The text for the feedback page link (see "articlefeedbackv5-shared-on-feedback")',
310313 'articlefeedbackv5-help-tooltip-title' => 'The title for the help tooltip',
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css
——@@ -870,4 +870,10 @@
871871 /*** Bucket 5 ***/
872872 #articleFeedbackv5-bucket5 .articleFeedbackv5-submit {
873873 float: right;
874 -}
\ No newline at end of file
 874+}
 875+
 876+/*** Top error ***/
 877+.articleFeedbackv5-top-error {
 878+ color: #8b0000;
 879+ margin-bottom: 5px;
 880+}
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js
——@@ -176,9 +176,7 @@
177177 <h3><html:msg key="help-tooltip-title" /></h3><span class="articleFeedbackv5-tooltip-close">X</span>\
178178 <div class="clear"></div>\
179179 <p><html:msg key="help-tooltip-info" /></p>\
180 - <p><a target="_blank" href="' +
181 -mw.msg( 'articlefeedbackv5-help-tooltip-linkurl' )
182 -+ '"><html:msg key="help-tooltip-linktext" />&nbsp;&gt;&gt;</a></p>\
 180+ <p><a target="_blank" class="articleFeedbackv5-tooltip-link"><html:msg key="help-tooltip-linktext" />&nbsp;&gt;&gt;</a></p>\
183181 </div>\
184182 <div class="tooltip-bottom"></div>\
185183 </div>\
——@@ -223,6 +221,7 @@
224222 */
225223 block: '\
226224 <form>\
 225+ <div class="articleFeedbackv5-top-error"></div>\
227226 <div class="form-row articleFeedbackv5-bucket1-toggle">\
228227 <p class="instructions-left"><html:msg key="bucket1-question-toggle" /></p>\
229228 <div class="buttons">\
——@@ -402,7 +401,11 @@
403402 getFormData: function () {
404403 var data = {};
405404 var $check = $.articleFeedbackv5.find( '.articleFeedbackv5-bucket1-toggle input[checked]' );
406 - data.found = $check.val() == 'yes' ? 1 : 0;
 405+ if ( $check.val() == 'yes' ) {
 406+ data.found = 1;
 407+ } else if ( $check.val() == 'no' ) {
 408+ data.found = 0;
 409+ }
407410 data.comment = $.articleFeedbackv5.find( '.articleFeedbackv5-comment textarea' ).val();
408411 var def_msg_yes = mw.msg( 'articlefeedbackv5-bucket1-question-comment-yes' );
409412 var def_msg_no = mw.msg( 'articlefeedbackv5-bucket1-question-comment-no' );
——@@ -413,21 +416,24 @@
414417 },
415418
416419 // }}}
417 - // {{{ markFormError
 420+ // {{{ localValidation
418421
419422 /**
420 - * Marks any errors on the form
 423+ * Performs any local validation
421424 *
422 - * @param object error errors, indexed by field name
 425+ * @param object formdata the form data
 426+ * @return mixed if ok, false; otherwise, an object as { 'field name' : 'message' }
423427 */
424 - markFormError: function ( error ) {
425 - if ( '_api' in error ) {
426 - $.articleFeedbackv5.markShowstopperError( error._api.info );
427 - } else {
428 - alert( mw.msg( 'articlefeedbackv5-error-validation' ) );
429 - mw.log( mw.msg( 'articlefeedbackv5-error-validation' ) );
 428+ localValidation: function ( formdata ) {
 429+ var error = {};
 430+ var ok = true;
 431+ if ( ( !( 'comment' in formdata ) || formdata.comment == '' )
 432+ && !( 'found' in formdata ) ) {
 433+ $.articleFeedbackv5.enableSubmission( false );
 434+ error.nofeedback = mw.msg( 'articlefeedbackv5-error-nofeedback' );
 435+ ok = false;
430436 }
431 - console.log( error );
 437+ return ok ? false : error;
432438 }
433439
434440 // }}}
——@@ -467,6 +473,7 @@
468474 */
469475 block: '\
470476 <form>\
 477+ <div class="articleFeedbackv5-top-error"></div>\
471478 <div>\
472479 <div class="articleFeedbackv5-tags">\
473480 <ul></ul>\
——@@ -711,21 +718,23 @@
712719 },
713720
714721 // }}}
715 - // {{{ markFormError
 722+ // {{{ localValidation
716723
717724 /**
718 - * Marks any errors on the form
 725+ * Performs any local validation
719726 *
720 - * @param object error errors, indexed by field name
 727+ * @param object formdata the form data
 728+ * @return mixed if ok, false; otherwise, an object as { 'field name' : 'message' }
721729 */
722 - markFormError: function ( error ) {
723 - if ( '_api' in error ) {
724 - $.articleFeedbackv5.markShowstopperError( error._api.info );
725 - } else {
726 - alert( mw.msg( 'articlefeedbackv5-error-validation' ) );
727 - mw.log( mw.msg( 'articlefeedbackv5-error-validation' ) );
 730+ localValidation: function ( formdata ) {
 731+ var error = {};
 732+ var ok = true;
 733+ if ( !( 'comment' in formdata ) || formdata.comment == '' ) {
 734+ $.articleFeedbackv5.enableSubmission( false );
 735+ error.nofeedback = mw.msg( 'articlefeedbackv5-error-nofeedback' );
 736+ ok = false;
728737 }
729 - console.log( error );
 738+ return ok ? false : error;
730739 }
731740
732741 // }}}
——@@ -752,6 +761,7 @@
753762 */
754763 block: '\
755764 <form>\
 765+ <div class="articleFeedbackv5-top-error"></div>\
756766 <div>\
757767 <p class="instructions-left"><html:msg key="bucket3-rating-question" /></p>\
758768 <div class="articleFeedbackv5-rating articleFeedbackv5-rating-new">\
——@@ -997,21 +1007,24 @@
9981008 },
9991009
10001010 // }}}
1001 - // {{{ markFormError
 1011+ // {{{ localValidation
10021012
10031013 /**
1004 - * Marks any errors on the form
 1014+ * Performs any local validation
10051015 *
1006 - * @param object error errors, indexed by field name
 1016+ * @param object formdata the form data
 1017+ * @return mixed if ok, false; otherwise, an object as { 'field name' : 'message' }
10071018 */
1008 - markFormError: function ( error ) {
1009 - if ( '_api' in error ) {
1010 - $.articleFeedbackv5.markShowstopperError( error._api.info );
1011 - } else {
1012 - alert( mw.msg( 'articlefeedbackv5-error-validation' ) );
1013 - mw.log( mw.msg( 'articlefeedbackv5-error-validation' ) );
 1019+ localValidation: function ( formdata ) {
 1020+ var error = {};
 1021+ var ok = true;
 1022+ if ( ( !( 'comment' in formdata ) || formdata.comment == '' )
 1023+ && ( !( 'rating' in formdata ) || formdata.rating < 1 ) ) {
 1024+ $.articleFeedbackv5.enableSubmission( false );
 1025+ error.nofeedback = mw.msg( 'articlefeedbackv5-error-nofeedback' );
 1026+ ok = false;
10141027 }
1015 - console.log( error );
 1028+ return ok ? false : error;
10161029 }
10171030
10181031 // }}}
——@@ -1653,6 +1666,9 @@
16541667 $.articleFeedbackv5.find( '.articleFeedbackv5-expertise input:checked' ).each( function () {
16551668 data['expertise-' + $( this ).val()] = 1;
16561669 } );
 1670+ if ( $.articleFeedbackv5.find( '.articleFeedbackv5-helpimprove input:checked' ).length > 0 ) {
 1671+ data.email = $.articleFeedbackv5.find( '.articleFeedbackv5-helpimprove-email' ).val();
 1672+ }
16571673 return data;
16581674 },
16591675
——@@ -1669,8 +1685,7 @@
16701686 var error = {};
16711687 var ok = true;
16721688 if ( $.articleFeedbackv5.find( '.articleFeedbackv5-helpimprove input:checked' ).length > 0 ) {
1673 - var mail = $.articleFeedbackv5.find( '.articleFeedbackv5-helpimprove-email' ).val();
1674 - if ( !mw.util.validateEmail( mail ) ) {
 1689+ if ( 'email' in formdata && !mw.util.validateEmail( formdata.email ) ) {
16751690 error.helpimprove_email = mw.msg( 'articlefeedbackv5-error-email' );
16761691 ok = false;
16771692 }
——@@ -1679,30 +1694,19 @@
16801695 },
16811696
16821697 // }}}
1683 - // {{{ markFormError
 1698+ // {{{ markFormErrors
16841699
16851700 /**
16861701 * Marks any errors on the form
16871702 *
1688 - * @param object error errors, indexed by field name
 1703+ * @param object errors errors, indexed by field name
16891704 */
1690 - markFormError: function ( error ) {
1691 - if ( '_api' in error ) {
1692 - if ($.articleFeedbackv5.debug) {
1693 - $.articleFeedbackv5.find( '.articleFeedbackv5-error-message' )
1694 - .html( error._api.info.replace( "\n", '<br />' ) );
1695 - }
1696 - $.articleFeedbackv5.find( '.articleFeedbackv5-error' ).show();
1697 - } else {
1698 - if ( 'helpimprove_email' in error ) {
1699 - $.articleFeedbackv5.find( '.articleFeedbackv5-helpimprove-email' )
1700 - .addClass( 'invalid' )
1701 - .removeClass( 'valid' );
1702 - }
1703 - alert( mw.msg( 'articlefeedbackv5-error-validation' ) );
1704 - mw.log( mw.msg( 'articlefeedbackv5-error-validation' ) );
 1705+ markFormErrors: function ( errors ) {
 1706+ if ( 'helpimprove_email' in errors ) {
 1707+ $.articleFeedbackv5.find( '.articleFeedbackv5-helpimprove-email' )
 1708+ .addClass( 'invalid' )
 1709+ .removeClass( 'valid' );
17051710 }
1706 - console.log( error );
17071711 },
17081712
17091713 // }}}
——@@ -2049,9 +2053,11 @@
20502054
20512055 // Set up the tooltip for the panel version
20522056 $wrapper.find( '.articleFeedbackv5-title-wrap' ).append( $.articleFeedbackv5.templates.helpToolTip );
 2057+ $wrapper.find( '.articleFeedbackv5-tooltip-link' )
 2058+ .attr( 'href', mw.msg( 'articlefeedbackv5-help-tooltip-linkurl' ) );
20532059 $wrapper.find( '.articleFeedbackv5-tooltip' ).hide();
20542060
2055 - if( $.articleFeedbackv5.bucketId == 5 ) {
 2061+ if ( $.articleFeedbackv5.bucketId == 5 ) {
20562062 $wrapper.find( '.articleFeedbackv5-tooltip-trigger' ).hide();
20572063 } else {
20582064 $wrapper.find( '.articleFeedbackv5-tooltip-trigger' ).click( function () {
——@@ -2096,6 +2102,8 @@
20972103 // Set up the tooltip for the dialoag
20982104 $titlebar.append( $.articleFeedbackv5.templates.helpToolTip );
20992105 $titlebar.find( '.articleFeedbackv5-tooltip' ).hide();
 2106+ $titlebar.find( '.articleFeedbackv5-tooltip-link' )
 2107+ .attr( 'href', mw.msg( 'articlefeedbackv5-help-tooltip-linkurl' ) );
21002108 $titlebar.find( '.articleFeedbackv5-tooltip-trigger' ).click( function ( e ) {
21012109 $( e.target ).next( '.articleFeedbackv5-tooltip' ).toggle();
21022110 } );
——@@ -2139,13 +2147,9 @@
21402148
21412149 // Perform any local validation
21422150 if ( 'localValidation' in bucket ) {
2143 - var error = bucket.localValidation( formdata );
2144 - if ( error ) {
2145 - if ( 'markFormError' in bucket ) {
2146 - bucket.markFormError( error );
2147 - } else {
2148 - alert( error.join( "\n" ) );
2149 - }
 2151+ var errors = bucket.localValidation( formdata );
 2152+ if ( errors ) {
 2153+ $.articleFeedbackv5.markFormErrors( errors );
21502154 return;
21512155 }
21522156 }
——@@ -2190,19 +2194,15 @@
21912195 if ( 'error' in data ) {
21922196 msg = data.error;
21932197 } else {
2194 - msg = mw.msg( 'articlefeedbackv5-error-unknown' );
 2198+ msg = { info: mw.msg( 'articlefeedbackv5-error-unknown' ) };
21952199 }
2196 - if ( 'markFormError' in bucket ) {
2197 - bucket.markFormError( { _api : msg } );
2198 - } else {
2199 - alert( mw.msg( 'articlefeedbackv5-error-submit' ) );
2200 - }
 2200+ $.articleFeedbackv5.markFormErrors( { _api : msg } );
22012201 $.articleFeedbackv5.unlockForm();
22022202 }
22032203 },
22042204 'error': function () {
2205 - mw.log( mw.msg( 'articlefeedbackv5-error-submit' ) );
2206 - alert( mw.msg( 'articlefeedbackv5-error-submit' ) );
 2205+ var err = { _api: { info: mw.msg( 'articlefeedbackv5-error-submit' ) } };
 2206+ $.articleFeedbackv5.markFormErrors( err );
22072207 $.articleFeedbackv5.unlockForm();
22082208 }
22092209 } );
——@@ -2293,6 +2293,48 @@
22942294 };
22952295
22962296 // }}}
 2297+ // {{{ markTopError
 2298+
 2299+ /**
 2300+ * Marks an error at the top of the form
 2301+ *
 2302+ * @param msg string the error message
 2303+ */
 2304+ $.articleFeedbackv5.markTopError = function ( msg ) {
 2305+ $.articleFeedbackv5.find( '.articleFeedbackv5-top-error' ).html( msg );
 2306+ };
 2307+
 2308+ // }}}
 2309+ // {{{ markFormErrors
 2310+
 2311+ /**
 2312+ * Marks any errors on the form
 2313+ *
 2314+ * @param object errors errors, indexed by field name
 2315+ */
 2316+ $.articleFeedbackv5.markFormErrors = function ( errors ) {
 2317+ if ( '_api' in errors ) {
 2318+ if ( $.articleFeedbackv5.debug ) {
 2319+ $.articleFeedbackv5.markTopError( errors._api.info );
 2320+ } else {
 2321+ mw.log( mw.msg( 'articlefeedbackv5-error-submit' ) );
 2322+ }
 2323+ mw.log( mw.msg( errors._api.info ) );
 2324+ } else {
 2325+ mw.log( mw.msg( 'articlefeedbackv5-error-validation' ) );
 2326+ if ( 'nofeedback' in errors ) {
 2327+ $.articleFeedbackv5.markTopError( mw.msg( 'articlefeedbackv5-error-nofeedback' ) );
 2328+ }
 2329+ }
 2330+ if ( $.articleFeedbackv5.debug ) {
 2331+ console.log( errors );
 2332+ }
 2333+ if ( 'markFormErrors' in $.articleFeedbackv5.currentBucket() ) {
 2334+ $.articleFeedbackv5.currentBucket().markFormErrors( errors );
 2335+ }
 2336+ };
 2337+
 2338+ // }}}
22972339 // {{{ lockForm
22982340
22992341 /**
——@@ -2331,7 +2373,6 @@
23322374 */
23332375 $.articleFeedbackv5.addToRemovalQueue = function ( $el ) {
23342376 $.articleFeedbackv5.$toRemove = $.articleFeedbackv5.$toRemove.add( $el );
2335 - console.log( $.articleFeedbackv5.$toRemove );
23362377 };
23372378
23382379 // }}}
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php
——@@ -45,6 +45,7 @@
4646 'messages' => array(
4747 'articlefeedbackv5-error-email',
4848 'articlefeedbackv5-error-validation',
 49+ 'articlefeedbackv5-error-nofeedback',
4950 'articlefeedbackv5-error-unknown',
5051 'articlefeedbackv5-error-submit',
5152 'articlefeedbackv5-cta1-thanks',

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r105031Confirmation/call to actionseanheavey00:36, 3 December 2011

Status & tagging log