From e201823ba6cb6188f05a58a1b71c7ab80f7b8dc4 Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Wed, 4 Oct 2023 12:27:53 +0200 Subject: [PATCH 1/7] Improve new form validation --- src/components/Form/FormProvider.js | 70 ++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/src/components/Form/FormProvider.js b/src/components/Form/FormProvider.js index 5261d1258ad0..a8f5d5c4dfca 100644 --- 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], ); /** @@ -195,7 +252,7 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c setTimeout(() => { setTouchedInput(inputID); if (shouldValidateOnBlur) { - onValidate(inputValues); + onValidate(inputValues, !hasServerError); } }, 200); } @@ -237,6 +294,7 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c {/* eslint-disable react/jsx-props-no-spreading */} Date: Thu, 5 Oct 2023 16:36:44 +0200 Subject: [PATCH 2/7] Form validation fixes --- src/components/Form/FormProvider.js | 12 ++++++++++++ src/components/Form/FormWrapper.js | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/components/Form/FormProvider.js b/src/components/Form/FormProvider.js index a8f5d5c4dfca..b8ae09f1cffa 100644 --- a/src/components/Form/FormProvider.js +++ b/src/components/Form/FormProvider.js @@ -243,6 +243,18 @@ function FormProvider({validate, formID, shouldValidateOnBlur, shouldValidateOnC 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()) { diff --git a/src/components/Form/FormWrapper.js b/src/components/Form/FormWrapper.js index 44bfee1a9e4a..d7317a673795 100644 --- a/src/components/Form/FormWrapper.js +++ b/src/components/Form/FormWrapper.js @@ -36,7 +36,7 @@ const propTypes = { isLoading: PropTypes.bool, /** Server side errors keyed by microtime */ - errors: PropTypes.objectOf(PropTypes.oneOf([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])), + errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])), /** Field-specific server side errors keyed by microtime */ errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)), @@ -59,7 +59,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: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])), inputRefs: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object])).isRequired, }; From c77dcb1fefba00c722beb912bdeec3a88bc9c14c Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 6 Oct 2023 11:10:27 +0200 Subject: [PATCH 3/7] Form docs improvements --- src/components/Form/FormProvider.js | 2 +- src/components/Form/FormWrapper.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Form/FormProvider.js b/src/components/Form/FormProvider.js index b8ae09f1cffa..d5345cfbcfe1 100644 --- a/src/components/Form/FormProvider.js +++ b/src/components/Form/FormProvider.js @@ -297,7 +297,7 @@ function FormProvider({validate, formID, shouldValidateOnBlur, shouldValidateOnC }, }; }, - [errors, formState, inputValues, onValidate, setTouchedInput, shouldValidateOnBlur, shouldValidateOnChange], + [errors, formState, hasServerError, inputValues, onValidate, setTouchedInput, shouldValidateOnBlur, shouldValidateOnChange], ); const value = useMemo(() => ({registerInput}), [registerInput]); diff --git a/src/components/Form/FormWrapper.js b/src/components/Form/FormWrapper.js index d7317a673795..7b8cda333507 100644 --- a/src/components/Form/FormWrapper.js +++ b/src/components/Form/FormWrapper.js @@ -59,7 +59,7 @@ const propTypes = { /** Custom content to display in the footer after submit button */ footerContent: PropTypes.oneOfType([PropTypes.func, PropTypes.node]), - errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])), + errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])).isRequired, inputRefs: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object])).isRequired, }; From c74e09d871f3426a3c3477463365179765a9e781 Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 6 Oct 2023 15:00:02 +0200 Subject: [PATCH 4/7] Fix prop types --- src/components/Form/FormWrapper.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/Form/FormWrapper.js b/src/components/Form/FormWrapper.js index 7b8cda333507..4358078f5229 100644 --- a/src/components/Form/FormWrapper.js +++ b/src/components/Form/FormWrapper.js @@ -36,7 +36,7 @@ const propTypes = { isLoading: PropTypes.bool, /** Server side errors keyed by microtime */ - errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])), + errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]))])), /** Field-specific server side errors keyed by microtime */ errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)), @@ -59,7 +59,7 @@ const propTypes = { /** Custom content to display in the footer after submit button */ footerContent: PropTypes.oneOfType([PropTypes.func, PropTypes.node]), - errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])).isRequired, + errors: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]))])).isRequired, inputRefs: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object])).isRequired, }; @@ -84,6 +84,7 @@ function FormWrapper(props) { const latestErrorMessage = ErrorUtils.getLatestErrorMessage(formState); return typeof latestErrorMessage === 'string' ? latestErrorMessage : ''; }, [formState]); + console.log({errors}) const scrollViewContent = useCallback( (safeAreaPaddingBottomStyle) => ( From c619792268ea7c4402602fcf5d5887e16a263305 Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 6 Oct 2023 15:00:28 +0200 Subject: [PATCH 5/7] Remove console log --- src/components/Form/FormWrapper.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Form/FormWrapper.js b/src/components/Form/FormWrapper.js index 4358078f5229..c00c0458d81d 100644 --- a/src/components/Form/FormWrapper.js +++ b/src/components/Form/FormWrapper.js @@ -84,7 +84,6 @@ function FormWrapper(props) { const latestErrorMessage = ErrorUtils.getLatestErrorMessage(formState); return typeof latestErrorMessage === 'string' ? latestErrorMessage : ''; }, [formState]); - console.log({errors}) const scrollViewContent = useCallback( (safeAreaPaddingBottomStyle) => ( From 19062e6110c44036081cdd59ad0efca907c40ee7 Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 13 Oct 2023 11:42:23 +0200 Subject: [PATCH 6/7] Export errors prop type to separate file --- src/components/Form/errorsPropType.js | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 src/components/Form/errorsPropType.js diff --git a/src/components/Form/errorsPropType.js b/src/components/Form/errorsPropType.js new file mode 100644 index 000000000000..3a02bb74e942 --- /dev/null +++ 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]))])), + ]), + ), +]); From d99fa217a50b8772559c912d8592cf79780a075b Mon Sep 17 00:00:00 2001 From: Kamil Owczarz Date: Fri, 13 Oct 2023 11:42:43 +0200 Subject: [PATCH 7/7] Fix errors prop types --- src/components/Form/FormWrapper.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/Form/FormWrapper.js b/src/components/Form/FormWrapper.js index c00c0458d81d..3d9fd37d6f22 100644 --- 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.oneOfType([PropTypes.string, PropTypes.objectOf(PropTypes.oneOfType([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.oneOfType([PropTypes.string, PropTypes.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]))])).isRequired, + errors: errorsPropType.isRequired, inputRefs: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object])).isRequired, };