jenkins-bot has submitted this change and it was merged. Change subject: Replace alert with red errors for Mustache forms ......................................................................
Replace alert with red errors for Mustache forms No more embarassing 90s validation messages! Uses validate_personal (like GC forms) rather than validate_form. Renders initial errors server side and clears fixed errors on validation. Lots of repetition rendering the error messages - mustache's lack of logic makes the only other alternative setting dummy errors for correct fields (a la getEmptyErrorArray). Red highlight around incorrect fields is not yet working for Mustache but can be fixed in css. Bug: T107363 Change-Id: I09aece9fa6bb992af42bea739c4b7eb642b2eab6 --- M DonationInterface.php M adyen_gateway/forms/js/adyen.js M astropay_gateway/astropay.js M gateway_common/DonationData.php M gateway_forms/Mustache.php A gateway_forms/mustache/error_message.html.mustache M gateway_forms/mustache/index.html.mustache M gateway_forms/mustache/payment_amount.html.mustache M gateway_forms/mustache/personal_info.html.mustache M globalcollect_gateway/forms/css/gc.css M modules/css/gateway.css M modules/validate_input.js M worldpay_gateway/forms/js/esop.js 13 files changed, 185 insertions(+), 104 deletions(-) Approvals: Awight: Looks good to me, approved jenkins-bot: Verified diff --git a/DonationInterface.php b/DonationInterface.php index e8237ea..febb7ad 100644 --- a/DonationInterface.php +++ b/DonationInterface.php @@ -1020,6 +1020,7 @@ 'donate_interface-error-msg-discover', 'donate_interface-error-msg-amount', 'donate_interface-error-msg-email', + 'donate_interface-error-msg-fiscal_number', 'donate_interface-error-msg-fname', 'donate_interface-error-msg-lname', 'donate_interface-error-msg-street', @@ -1052,6 +1053,7 @@ 'donate_interface-donor-postal', 'donate_interface-donor-country', 'donate_interface-donor-email', + 'donate_interface-donor-fiscal_number', 'donate_interface-state-province', 'donate_interface-cvv-explain', ) diff --git a/adyen_gateway/forms/js/adyen.js b/adyen_gateway/forms/js/adyen.js index bdccacb..78513dc 100644 --- a/adyen_gateway/forms/js/adyen.js +++ b/adyen_gateway/forms/js/adyen.js @@ -100,7 +100,7 @@ } $( 'input[name="payment_submethod"]' ).on( 'change', function () { - if ( window.validateAmount() && window.validate_form( document.payment ) ) { + if ( window.validateAmount() && window.validate_personal( document.payment ) ) { window.displayCreditCardForm(); } else { $( '#paymentContinue' ).show(); @@ -108,7 +108,7 @@ } ); $( '#paymentContinueBtn' ).on( 'click', function () { - if ( window.validateAmount() && window.validate_form( document.payment ) ) { + if ( window.validateAmount() && window.validate_personal( document.payment ) ) { window.displayCreditCardForm(); // hide the continue button so that people don't get confused with two of them $( '#paymentContinue' ).hide(); diff --git a/astropay_gateway/astropay.js b/astropay_gateway/astropay.js index 3a6437b..b16e778 100644 --- a/astropay_gateway/astropay.js +++ b/astropay_gateway/astropay.js @@ -9,7 +9,7 @@ // Submit on submethod click if valid, // otherwise show continue button. $( 'input[name="payment_submethod"]' ).on( 'click', function() { - if ( window.validateAmount() && window.validate_form( form ) ) { + if ( window.validateAmount() && window.validate_personal( form ) ) { submitForm(); } else { $( '#paymentContinue' ).show(); @@ -17,7 +17,7 @@ } ); $( '#paymentContinueBtn' ).click( function() { - if ( window.validateAmount() && window.validate_form( form ) ) { + if ( window.validateAmount() && window.validate_personal( form ) ) { submitForm(); } }); diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php index 06bdf0f..b38c167 100644 --- a/gateway_common/DonationData.php +++ b/gateway_common/DonationData.php @@ -24,6 +24,83 @@ protected $logger; /** + * $fieldNames now just contains all the vars we want to + * get poked through to gateways in some form or other, + * from a get or post. We handle the actual + * normalization in normalize() helpers, below. + * @TODO: It would be really neat if the gateways kept + * track of all the things that ***only they will ever + * need***, and could interject those needs here... + * Then we could really clean up. + * @TODO also: Think about putting log alarms on the + * keys we want to see disappear forever, complete with + * ffname and referrer for easy total destruction. + */ + protected static $fieldNames = array( + 'amount', + 'amountGiven', + 'amountOther', + 'appeal', + 'email', + 'emailAdd', + 'fname', + 'lname', + 'street', + 'street_supplemental', + 'city', + 'state', + 'zip', + 'country', + 'card_num', + 'card_type', + 'expiration', + 'cvv', + 'currency', + 'currency_code', + 'payment_method', + 'payment_submethod', + 'issuer_id', + 'order_id', + 'subscr_id', + 'referrer', + 'utm_source', + 'utm_source_id', + 'utm_medium', + 'utm_campaign', + 'utm_key', + 'language', + 'uselang', + 'token', + 'contribution_tracking_id', + 'data_hash', + 'action', + 'gateway', + 'owa_session', + 'owa_ref', + 'descriptor', + 'account_name', + 'account_number', + 'authorization_id', + 'bank_check_digit', + 'bank_name', + 'bank_code', + 'branch_code', + 'country_code_bank', + 'date_collect', + 'direct_debit_text', + 'iban', + 'fiscal_number', + 'transaction_type', + 'form_name', + 'ffname', + 'recurring', + 'recurring_paypal', + 'redirect', + 'user_ip', + 'server_ip', + ); + + /** * DonationData constructor * @param GatewayAdapter $gateway * @param mixed $data An optional array of donation data that will, if @@ -54,85 +131,7 @@ //I don't care if you're a test or not. At all. $this->normalized = $external_data; } else { - - /** - * $varNames now just contains all the vars we want to - * get poked through to gateways in some form or other, - * from a get or post. We handle the actual - * normalization in normalize() helpers, below. - * @TODO: It would be really neat if the gateways kept - * track of all the things that ***only they will ever - * need***, and could interject those needs here... - * Then we could really clean up. - * @TODO also: Think about putting log alarms on the - * keys we want to see disappear forever, complete with - * ffname and referrer for easy total destruction. - */ - $varNames = array ( - 'amount', - 'amountGiven', - 'amountOther', - 'appeal', - 'email', - 'emailAdd', - 'fname', - 'lname', - 'street', - 'street_supplemental', - 'city', - 'state', - 'zip', - 'country', - 'card_num', - 'card_type', - 'expiration', - 'cvv', - 'currency', - 'currency_code', - 'payment_method', - 'payment_submethod', - 'issuer_id', - 'order_id', - 'subscr_id', - 'referrer', - 'utm_source', - 'utm_source_id', - 'utm_medium', - 'utm_campaign', - 'utm_key', - 'language', - 'uselang', - 'token', - 'contribution_tracking_id', - 'data_hash', - 'action', - 'gateway', - 'owa_session', - 'owa_ref', - 'descriptor', - 'account_name', - 'account_number', - 'authorization_id', - 'bank_check_digit', - 'bank_name', - 'bank_code', - 'branch_code', - 'country_code_bank', - 'date_collect', - 'direct_debit_text', - 'iban', - 'fiscal_number', - 'transaction_type', - 'form_name', - 'ffname', - 'recurring', - 'recurring_paypal', - 'redirect', - 'user_ip', - 'server_ip', - ); - - foreach ( $varNames as $var ) { + foreach ( self::$fieldNames as $var ) { $this->normalized[$var] = $this->sourceHarvest( $var ); } @@ -140,7 +139,7 @@ $this->setVal( 'posted', false ); } } - + //if we have saved any donation data to the session, pull them in as well. $this->integrateDataFromSession(); @@ -151,6 +150,10 @@ } } + public static function getFieldNames() { + return self::$fieldNames; + } + /** * Harvest a varname from its source - post, get, maybe even session eventually. * @TODO: Provide a way that gateways can override default behavior here for individual keys. diff --git a/gateway_forms/Mustache.php b/gateway_forms/Mustache.php index d3855e8..de81aca 100644 --- a/gateway_forms/Mustache.php +++ b/gateway_forms/Mustache.php @@ -166,10 +166,19 @@ } } + /** + * Get errors, sorted into two buckets - 'general' errors to display at + * the top of the form, and field errors to display inline. + * Also get some error-related flags. + * @return array with all general errors under the 'general' key, and + * field errors each keyed with their own field. + */ protected function getErrors() { $errors = $this->gateway->getAllErrors(); - $return = array(); - $return['errors'] = array(); + $return = array( 'errors' => array( + 'general' => array(), + ) ); + $fieldNames = DonationData::getFieldNames(); foreach( $errors as $key => $error ) { if ( is_array( $error ) ) { // TODO: set errors consistently @@ -177,10 +186,15 @@ } else { $message = $error; } - $return['errors'][] = array( + $errorContext = array( 'key' => $key, 'message' => $message, ); + if ( in_array( $key, $fieldNames ) ) { + $return['errors'][$key] = $errorContext; + } else { + $return['errors']['general'][] = $errorContext; + } $return["{$key}_error"] = true; if ( $key === 'currency_code' || $key === 'amount' ) { $return['show_amount_input'] = true; diff --git a/gateway_forms/mustache/error_message.html.mustache b/gateway_forms/mustache/error_message.html.mustache new file mode 100644 index 0000000..2ce91d8 --- /dev/null +++ b/gateway_forms/mustache/error_message.html.mustache @@ -0,0 +1 @@ +<span id="{{ key }}Msg" class="errorMsg" >{{{ message }}}</span> diff --git a/gateway_forms/mustache/index.html.mustache b/gateway_forms/mustache/index.html.mustache index 309ca82..3ff715d 100644 --- a/gateway_forms/mustache/index.html.mustache +++ b/gateway_forms/mustache/index.html.mustache @@ -8,7 +8,7 @@ <td id="donate" valign="top"> <div id="greenBackground" class=""> {{> no_script }} - <div id="topError" class="creditcard-error-msg">{{# errors }}<p class="error">{{{ message }}}</p>{{/ errors }}</div> + <div id="topError" class="creditcard-error-msg">{{# errors/general }}<p class="error">{{{ message }}}</p>{{/ errors/general }}</div> <form name="payment" id="payment-form" method="post" action="{{ action }}"> <div id="payment_gateway-personal-info"> diff --git a/gateway_forms/mustache/payment_amount.html.mustache b/gateway_forms/mustache/payment_amount.html.mustache index de36ada..e1db96c 100644 --- a/gateway_forms/mustache/payment_amount.html.mustache +++ b/gateway_forms/mustache/payment_amount.html.mustache @@ -1,7 +1,7 @@ <h3 class="amount_header">{{# recurring }}{{ l10n "donate_interface-monthlybox-amount" }}{{/ recurring }}{{^ recurring }}{{ l10n "donate_interface-amount-legend" }}{{/ recurring }}: <span id="amount_input" {{^ show_amount_input }}class="hidden"{{/ show_amount_input }} > - <input type="number" step="any" name="amount" id="amount" value="{{ amount }}" /> - <select name="currency_code" id="currency_code" {{^ show_currency_selector }}class="hidden"{{/ show_currency_selector }} > + <input type="number" step="any" name="amount" id="amount" title="{{ donate_interface-amount }}" value="{{ amount }}" /> + <select name="currency_code" id="currency_code" title="{{ donate_interface-currency }}" {{^ show_currency_selector }}class="hidden"{{/ show_currency_selector }} > {{# currencies }} <option value="{{ code }}" {{# selected }}selected{{/ selected }} >{{ code }}</option> {{/ currencies }} @@ -14,3 +14,4 @@ {{ amount }} {{ currency_code }} </span> </h3> +{{# errors/amount }}{{> error_message }}{{/ errors/amount }}{{^ errors/amount }}<span id="amountMsg" class="errorMsgHide"></span>{{/ errors/amount }} diff --git a/gateway_forms/mustache/personal_info.html.mustache b/gateway_forms/mustache/personal_info.html.mustache index bde7d5d..3ce9a89 100644 --- a/gateway_forms/mustache/personal_info.html.mustache +++ b/gateway_forms/mustache/personal_info.html.mustache @@ -4,6 +4,16 @@ <input class="halfwidth" name="fname" value="{{ fname }}" type="text" placeholder="{{ l10n "donate_interface-donor-fname" }}" id="fname" required><input class="halfwidth" name="lname" value="{{ lname }}" type="text" placeholder="{{ l10n "donate_interface-donor-lname" }}" id="lname" required> </td> </tr> + <tr> + <td> + {{# errors/fname }}{{> error_message }}{{/ errors/fname }}{{^ errors/fname }}<span id="fnameMsg" class="errorMsgHide"></span>{{/ errors/fname }} + </td> + </tr> + <tr> + <td> + {{# errors/lname }}{{> error_message }}{{/ errors/lname }}{{^ errors/lname }}<span id="lnameMsg" class="errorMsgHide"></span>{{/ errors/lname }} + </td> + </tr> {{/ fname_required }} {{# address_required }} <tr> @@ -28,6 +38,28 @@ {{/ postal_code_required }} </td> </tr> + + {{# city_required }} + <tr> + <td> + {{# errors/city }}{{> error_message }}{{/ errors/city }}{{^ errors/city }}<span id="cityMsg" class="errorMsgHide"></span>{{/ errors/city }} + </td> + </tr> + {{/ city_required }} + {{# state_required }} + <tr> + <td> + {{# errors/state }}{{> error_message }}{{/ errors/state }}{{^ errors/state }}<span id="stateMsg" class="errorMsgHide"></span>{{/ errors/state }} + </td> + </tr> + {{/ state_required }} + {{# postal_code_required }} + <tr> + <td> + {{# errors/postal_code }}{{> error_message }}{{/ errors/postal_code }}{{^ errors/postal_code }}<span id="postal_codeMsg" class="errorMsgHide"></span>{{/ errors/postal_code }} + </td> + </tr> + {{/ postal_code_required }} {{/ address_required }} {{# fiscal_number_required }} <tr> @@ -35,6 +67,11 @@ <input class="fullwidth" name="fiscal_number" value="{{ fiscal_number }}" type="text" title="{{ l10n "donate_interface-donor-fiscal_number" }}" placeholder="{{ l10n "donate_interface-donor-fiscal_number" }}" id="fiscal_number" required > </td> </tr> + <tr> + <td> + {{# errors/fiscal_number }}{{> error_message }}{{/ errors/fiscal_number }}{{^ errors/fiscal_number }}<span id="fiscal_numberMsg" class="errorMsgHide"></span>{{/ errors/fiscal_number }} + </td> + </tr> {{/ fiscal_number_required }} {{# email_required }} <tr> @@ -42,4 +79,9 @@ <input class="fullwidth" name="email" value="{{ email }}" type="email" title="{{ l10n "donate_interface-donor-email" }}" placeholder="{{ l10n "donate_interface-donor-email" }}" id="email" required> </td> </tr> + <tr> + <td> + {{# errors/email }}{{> error_message }}{{/ errors/email }}{{^ errors/email }}<span id="emailMsg" class="errorMsgHide"></span>{{/ errors/email }} + </td> + </tr> {{/ email_required }} diff --git a/globalcollect_gateway/forms/css/gc.css b/globalcollect_gateway/forms/css/gc.css index 6be8c97..1cfbbf4 100644 --- a/globalcollect_gateway/forms/css/gc.css +++ b/globalcollect_gateway/forms/css/gc.css @@ -276,12 +276,3 @@ -webkit-box-shadow: inset 0 1px 4px rgba(0,0,1,.5); box-shadow: inset 0 1px 4px rgba(0,0,1,.5); } -.errorHighlight { - border: 2px solid red !important; -} -.errorMsg { - color: red; - display: inherit; -} -.errorMsgHide { - display: none; diff --git a/modules/css/gateway.css b/modules/css/gateway.css index b05a7a7..9a53336 100644 --- a/modules/css/gateway.css +++ b/modules/css/gateway.css @@ -219,6 +219,19 @@ border: none; } +.errorHighlight { + border: 2px solid red !important; +} + +.errorMsg { + color: red; + display: inherit; +} + +.errorMsgHide { + display: none; +} + /* Respond to small viewport */ @media screen and (max-width: 981px) { /* Overall layout */ diff --git a/modules/validate_input.js b/modules/validate_input.js index a106614..990c922 100644 --- a/modules/validate_input.js +++ b/modules/validate_input.js @@ -45,7 +45,10 @@ amount = $( 'input[name="amount"]' ).val(), // get the amount currency_code = '', rate, - minUsd = mw.config.get( 'wgDonationInterfacePriceFloor' ); + minUsd = mw.config.get( 'wgDonationInterfacePriceFloor' ), + minDisplay, + message = mediaWiki.msg( 'donate_interface-smallamount-error' ), + $amountMsg = $( '#amountMsg' ); // Normalize weird amount formats. // Don't mess with these unless you know what you're doing. @@ -73,8 +76,19 @@ } else { rate = wgCurrencyMinimums[ currency_code ]; } + // if we're on a new form, clear existing amount error + $amountMsg.removeClass( 'errorMsg' ).addClass( 'errorMsgHide' ).text( '' ); if ( ( amount < minUsd * rate ) || error ) { - alert( mediaWiki.msg( 'donate_interface-smallamount-error' ).replace( '$1', minUsd * rate + ' ' + currency_code ) ); + // Round to two decimal places (TODO: no decimals for some currencies) + minDisplay = Math.round( minUsd * rate * 100 ) / 100; + message = message.replace( '$1', minDisplay + ' ' + currency_code ); + if ( $amountMsg.length > 0 ) { + // newness + $amountMsg.removeClass( 'errorMsgHide' ).addClass( 'errorMsg' ).text( message ); + } else { + // ugliness + alert( message ); + } error = true; // See if we're on a webitects accordian form if ( $( '#step1wrapper' ).length ) { @@ -102,7 +116,7 @@ errorsPresent = false, currField = '', i, - fields = [ 'fname', 'lname', 'street', 'city', 'zip', 'email' ], + fields = [ 'fname', 'lname', 'street', 'city', 'zip', 'email', 'fiscal_number' ], errorTemplate = mediaWiki.msg( 'donate_interface-error-msg' ), numFields = fields.length, invalids = [ '..', '/', '\\', ',', '<', '>' ]; diff --git a/worldpay_gateway/forms/js/esop.js b/worldpay_gateway/forms/js/esop.js index f6b7a90..caa77ef 100644 --- a/worldpay_gateway/forms/js/esop.js +++ b/worldpay_gateway/forms/js/esop.js @@ -9,14 +9,14 @@ if ( !$( this ).hasClass( 'enabled' ) ) { return false; } - if ( window.validateAmount() && window.validate_form( form ) ) { + if ( window.validateAmount() && window.validate_personal( form ) ) { submitForm(); } } ); // Submit on submethod selection if valid, otherwise show continute button. $( 'input[name="payment_submethod"]' ).on( 'change', function () { - if ( window.validateAmount() && window.validate_form( form ) ) { + if ( window.validateAmount() && window.validate_personal( form ) ) { submitForm(); } else { $( '#paymentContinue' ).show(); -- To view, visit https://gerrit.wikimedia.org/r/284835 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I09aece9fa6bb992af42bea739c4b7eb642b2eab6 Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/DonationInterface Gerrit-Branch: master Gerrit-Owner: Ejegg <eeggles...@wikimedia.org> Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com> Gerrit-Reviewer: Awight <awi...@wikimedia.org> Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org> Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org> Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org> Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits