From 62a01e7a3712e56ca8c021b813b2e372725b0889 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Wed, 17 Apr 2019 11:39:11 +0100 Subject: [PATCH] Track per-field validity with new-style validation This updates the registration form to include the new-style validation state when deciding whether the entire form is valid overall. In addition, this tweaks the validation helper to take functions instead of strings for translated text. This allows the validation helper to be create once per component instead of once every render, which improves performance. --- src/components/views/auth/RegistrationForm.js | 79 ++++++++++++------- src/components/views/elements/Validation.js | 21 +++-- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/components/views/auth/RegistrationForm.js b/src/components/views/auth/RegistrationForm.js index 83ea4dfae6..6e139d7051 100644 --- a/src/components/views/auth/RegistrationForm.js +++ b/src/components/views/auth/RegistrationForm.js @@ -68,7 +68,9 @@ module.exports = React.createClass({ getInitialState: function() { return { // Field error codes by field ID + // TODO: Remove `fieldErrors` once converted to new-style validation fieldErrors: {}, + fieldValid: {}, // The ISO2 country code selected in the phone number entry phoneCountry: this.props.defaultPhoneCountry, username: "", @@ -140,12 +142,19 @@ module.exports = React.createClass({ * @returns {boolean} true if all fields were valid last time they were validated. */ allFieldsValid: function() { - const keys = Object.keys(this.state.fieldErrors); + // TODO: Remove `fieldErrors` here when all fields converted + let keys = Object.keys(this.state.fieldErrors); for (let i = 0; i < keys.length; ++i) { if (this.state.fieldErrors[keys[i]]) { return false; } } + keys = Object.keys(this.state.fieldValid); + for (let i = 0; i < keys.length; ++i) { + if (!this.state.fieldValid[keys[i]]) { + return false; + } + } return true; }, @@ -161,57 +170,57 @@ module.exports = React.createClass({ const email = this.state.email; const emailValid = email === '' || Email.looksValid(email); if (this._authStepIsRequired('m.login.email.identity') && (!emailValid || email === '')) { - this.markFieldValid(fieldID, false, "RegistrationForm.ERR_MISSING_EMAIL"); - } else this.markFieldValid(fieldID, emailValid, "RegistrationForm.ERR_EMAIL_INVALID"); + this.markFieldError(fieldID, false, "RegistrationForm.ERR_MISSING_EMAIL"); + } else this.markFieldError(fieldID, emailValid, "RegistrationForm.ERR_EMAIL_INVALID"); break; } case FIELD_PHONE_NUMBER: { const phoneNumber = this.state.phoneNumber; const phoneNumberValid = phoneNumber === '' || phoneNumberLooksValid(phoneNumber); if (this._authStepIsRequired('m.login.msisdn') && (!phoneNumberValid || phoneNumber === '')) { - this.markFieldValid(fieldID, false, "RegistrationForm.ERR_MISSING_PHONE_NUMBER"); - } else this.markFieldValid(fieldID, phoneNumberValid, "RegistrationForm.ERR_PHONE_NUMBER_INVALID"); + this.markFieldError(fieldID, false, "RegistrationForm.ERR_MISSING_PHONE_NUMBER"); + } else this.markFieldError(fieldID, phoneNumberValid, "RegistrationForm.ERR_PHONE_NUMBER_INVALID"); break; } case FIELD_USERNAME: { const username = this.state.username; if (allowEmpty && username === '') { - this.markFieldValid(fieldID, true); + this.markFieldError(fieldID, true); } else if (username == '') { - this.markFieldValid( + this.markFieldError( fieldID, false, "RegistrationForm.ERR_USERNAME_BLANK", ); } else { - this.markFieldValid(fieldID, true); + this.markFieldError(fieldID, true); } break; } case FIELD_PASSWORD: if (allowEmpty && pwd1 === "") { - this.markFieldValid(fieldID, true); + this.markFieldError(fieldID, true); } else if (pwd1 == '') { - this.markFieldValid( + this.markFieldError( fieldID, false, "RegistrationForm.ERR_PASSWORD_MISSING", ); } else if (pwd1.length < this.props.minPasswordLength) { - this.markFieldValid( + this.markFieldError( fieldID, false, "RegistrationForm.ERR_PASSWORD_LENGTH", ); } else { - this.markFieldValid(fieldID, true); + this.markFieldError(fieldID, true); } break; case FIELD_PASSWORD_CONFIRM: if (allowEmpty && pwd2 === "") { - this.markFieldValid(fieldID, true); + this.markFieldError(fieldID, true); } else { - this.markFieldValid( + this.markFieldError( fieldID, pwd1 == pwd2, "RegistrationForm.ERR_PASSWORD_MISMATCH", ); @@ -220,7 +229,8 @@ module.exports = React.createClass({ } }, - markFieldValid: function(fieldID, valid, errorCode) { + markFieldError: function(fieldID, valid, errorCode) { + // TODO: Remove this function once all fields converted to new-style validation. const { fieldErrors } = this.state; if (valid) { fieldErrors[fieldID] = null; @@ -235,6 +245,14 @@ module.exports = React.createClass({ this.props.onValidationChange(fieldErrors); }, + markFieldValid: function(fieldID, valid) { + const { fieldValid } = this.state; + fieldValid[fieldID] = valid; + this.setState({ + fieldValid, + }); + }, + _classForField: function(fieldID, ...baseClasses) { let cls = baseClasses.join(' '); // TODO: Remove this from fields as they are converted to new-style validation. @@ -298,6 +316,23 @@ module.exports = React.createClass({ }); }, + onUsernameValidate(fieldState) { + const result = this.validateUsernameRules(fieldState); + this.markFieldValid(FIELD_USERNAME, result.valid); + return result; + }, + + validateUsernameRules: withValidation({ + description: () => _t("Use letters, numbers, dashes and underscores only"), + rules: [ + { + key: "safeLocalpart", + regex: SAFE_LOCALPART_REGEX, + invalid: () => _t("Some characters not allowed"), + }, + ], + }), + /** * A step is required if all flows include that step. * @@ -324,18 +359,6 @@ module.exports = React.createClass({ renderUsername() { const Field = sdk.getComponent('elements.Field'); - - const onValidate = withValidation({ - description: _t("Use letters, numbers, dashes and underscores only"), - rules: [ - { - key: "safeLocalpart", - regex: SAFE_LOCALPART_REGEX, - invalid: _t("Some characters not allowed"), - }, - ], - }); - return ; }, diff --git a/src/components/views/elements/Validation.js b/src/components/views/elements/Validation.js index 4770c968e2..8fc584edce 100644 --- a/src/components/views/elements/Validation.js +++ b/src/components/views/elements/Validation.js @@ -19,16 +19,16 @@ import classNames from 'classnames'; /** * Creates a validation function from a set of rules describing what to validate. * - * @param {String} description - * Summary of the kind of value that will meet the validation rules. Shown at - * the top of the validation feedback. + * @param {Function} description + * Function that returns a string summary of the kind of value that will + * meet the validation rules. Shown at the top of the validation feedback. * @param {Object} rules * An array of rules describing how to check to input value. Each rule in an object * and may have the following properties: * - `key`: A unique ID for the rule. Required. * - `regex`: A regex used to determine the rule's current validity. Required. - * - `valid`: Text to show when the rule is valid. Only shown if set. - * - `invalid`: Text to show when the rule is invalid. Only shown if set. + * - `valid`: Function returning text to show when the rule is valid. Only shown if set. + * - `invalid`: Function returning text to show when the rule is invalid. Only shown if set. * @returns {Function} * A validation function that takes in the current input value and returns * the overall validity and a feedback UI that can be rendered for more detail. @@ -58,7 +58,7 @@ export default function withValidation({ description, rules }) { results.push({ key: rule.key, valid: true, - text: rule.valid, + text: rule.valid(), }); } else if (!ruleValid && rule.invalid) { // If the rule's result is invalid and has text to show for @@ -66,7 +66,7 @@ export default function withValidation({ description, rules }) { results.push({ key: rule.key, valid: false, - text: rule.invalid, + text: rule.invalid(), }); } } @@ -96,8 +96,13 @@ export default function withValidation({ description, rules }) { ; } + let summary; + if (description) { + summary =
{description()}
; + } + const feedback =
-
{description}
+ {summary} {details}
;