Merge pull request #28821 from software-mansion-labs/add-better-form-…

…validation Improve `new form` validation
Expensify · Oct 13, 2023 · 06ab1eb · 06ab1eb
2 parents abfa6a5 + d99fa21
commit 06ab1eb
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 9 deletions.
diff --git a/src/components/Form/FormProvider.js b/src/components/Form/FormProvider.js
@@ -11,6 +11,7 @@ import compose from '../../libs/compose';
 import {withNetwork} from '../OnyxProvider';
 import stylePropTypes from '../../styles/stylePropTypes';
 import networkPropTypes from '../networkPropTypes';
+import CONST from '../../CONST';
 
 const propTypes = {
     /** A unique Onyx key identifying the form */
@@ -98,19 +99,75 @@ function getInitialValueByType(valueType) {
     }
 }
 
-function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, children, formState, network, enabledWhenOffline, onSubmit, ...rest}) {
+function FormProvider({validate, formID, shouldValidateOnBlur, shouldValidateOnChange, children, formState, network, enabledWhenOffline, onSubmit, ...rest}) {
     const inputRefs = useRef(null);
     const touchedInputs = useRef({});
     const [inputValues, setInputValues] = useState({});
     const [errors, setErrors] = useState({});
+    const hasServerError = useMemo(() => Boolean(formState) && !_.isEmpty(formState.errors), [formState]);
 
     const onValidate = useCallback(
-        (values) => {
+        (values, shouldClearServerError = true) => {
+            const trimmedStringValues = {};
+            _.each(values, (inputValue, inputID) => {
+                if (_.isString(inputValue)) {
+                    trimmedStringValues[inputID] = inputValue.trim();
+                } else {
+                    trimmedStringValues[inputID] = inputValue;
+                }
+            });
+
+            if (shouldClearServerError) {
+                FormActions.setErrors(formID, null);
+            }
+            FormActions.setErrorFields(formID, null);
+
             const validateErrors = validate(values) || {};
-            setErrors(validateErrors);
-            return validateErrors;
+
+            // Validate the input for html tags. It should supercede any other error
+            _.each(trimmedStringValues, (inputValue, inputID) => {
+                // If the input value is empty OR is non-string, we don't need to validate it for HTML tags
+                if (!inputValue || !_.isString(inputValue)) {
+                    return;
+                }
+                const foundHtmlTagIndex = inputValue.search(CONST.VALIDATE_FOR_HTML_TAG_REGEX);
+                const leadingSpaceIndex = inputValue.search(CONST.VALIDATE_FOR_LEADINGSPACES_HTML_TAG_REGEX);
+
+                // Return early if there are no HTML characters
+                if (leadingSpaceIndex === -1 && foundHtmlTagIndex === -1) {
+                    return;
+                }
+
+                const matchedHtmlTags = inputValue.match(CONST.VALIDATE_FOR_HTML_TAG_REGEX);
+                let isMatch = _.some(CONST.WHITELISTED_TAGS, (r) => r.test(inputValue));
+                // Check for any matches that the original regex (foundHtmlTagIndex) matched
+                if (matchedHtmlTags) {
+                    // Check if any matched inputs does not match in WHITELISTED_TAGS list and return early if needed.
+                    for (let i = 0; i < matchedHtmlTags.length; i++) {
+                        const htmlTag = matchedHtmlTags[i];
+                        isMatch = _.some(CONST.WHITELISTED_TAGS, (r) => r.test(htmlTag));
+                        if (!isMatch) {
+                            break;
+                        }
+                    }
+                }
+                // Add a validation error here because it is a string value that contains HTML characters
+                validateErrors[inputID] = 'common.error.invalidCharacter';
+            });
+
+            if (!_.isObject(validateErrors)) {
+                throw new Error('Validate callback must return an empty object or an object with shape {inputID: error}');
+            }
+
+            const touchedInputErrors = _.pick(validateErrors, (inputValue, inputID) => Boolean(touchedInputs.current[inputID]));
+
+            if (!_.isEqual(errors, touchedInputErrors)) {
+                setErrors(touchedInputErrors);
+            }
+
+            return touchedInputErrors;
         },
-        [validate],
+        [errors, formID, validate],
     );
 
     /**
@@ -186,6 +243,18 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c
                         propsToParse.onTouched(event);
                     }
                 },
+                onPress: (event) => {
+                    setTouchedInput(inputID);
+                    if (_.isFunction(propsToParse.onPress)) {
+                        propsToParse.onPress(event);
+                    }
+                },
+                onPressIn: (event) => {
+                    setTouchedInput(inputID);
+                    if (_.isFunction(propsToParse.onPressIn)) {
+                        propsToParse.onPressIn(event);
+                    }
+                },
                 onBlur: (event) => {
                     // Only run validation when user proactively blurs the input.
                     if (Visibility.isVisible() && Visibility.hasFocus()) {
@@ -195,7 +264,7 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c
                         setTimeout(() => {
                             setTouchedInput(inputID);
                             if (shouldValidateOnBlur) {
-                                onValidate(inputValues);
+                                onValidate(inputValues, !hasServerError);
                             }
                         }, 200);
                     }
@@ -228,7 +297,7 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c
                 },
             };
         },
-        [errors, formState, inputValues, onValidate, setTouchedInput, shouldValidateOnBlur, shouldValidateOnChange],
+        [errors, formState, hasServerError, inputValues, onValidate, setTouchedInput, shouldValidateOnBlur, shouldValidateOnChange],
     );
     const value = useMemo(() => ({registerInput}), [registerInput]);
 
@@ -237,6 +306,7 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c
             {/* eslint-disable react/jsx-props-no-spreading */}
             <FormWrapper
                 {...rest}
+                formID={formID}
                 onSubmit={submit}
                 inputRefs={inputRefs}
                 errors={errors}

diff --git a/src/components/Form/FormWrapper.js b/src/components/Form/FormWrapper.js
@@ -11,6 +11,7 @@ import SafeAreaConsumer from '../SafeAreaConsumer';
 import ScrollViewWithContext from '../ScrollViewWithContext';
 
 import stylePropTypes from '../../styles/stylePropTypes';
+import errorsPropType from './errorsPropType';
 
 const propTypes = {
     /** A unique Onyx key identifying the form */
@@ -36,7 +37,7 @@ const propTypes = {
         isLoading: PropTypes.bool,
 
         /** Server side errors keyed by microtime */
-        errors: PropTypes.objectOf(PropTypes.oneOf([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])),
+        errors: errorsPropType,
 
         /** Field-specific server side errors keyed by microtime */
         errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)),
@@ -59,7 +60,7 @@ const propTypes = {
     /** Custom content to display in the footer after submit button */
     footerContent: PropTypes.oneOfType([PropTypes.func, PropTypes.node]),
 
-    errors: PropTypes.objectOf(PropTypes.string).isRequired,
+    errors: errorsPropType.isRequired,
 
     inputRefs: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object])).isRequired,
 };

diff --git a/src/components/Form/errorsPropType.js b/src/components/Form/errorsPropType.js
@@ -0,0 +1,11 @@
+import PropTypes from 'prop-types';
+
+export default PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.objectOf(
+        PropTypes.oneOfType([
+            PropTypes.string,
+            PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.bool, PropTypes.number]))])),
+        ]),
+    ),
+]);