Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve new form validation #28821

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 77 additions & 7 deletions src/components/Form/FormProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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],
);

/**
Expand Down Expand Up @@ -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()) {
Expand All @@ -195,7 +264,7 @@ function FormProvider({validate, shouldValidateOnBlur, shouldValidateOnChange, c
setTimeout(() => {
setTouchedInput(inputID);
if (shouldValidateOnBlur) {
onValidate(inputValues);
onValidate(inputValues, !hasServerError);
}
}, 200);
}
Expand Down Expand Up @@ -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]);

Expand All @@ -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}
Expand Down
5 changes: 3 additions & 2 deletions src/components/Form/FormWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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)),
Expand All @@ -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,
};
Expand Down
11 changes: 11 additions & 0 deletions src/components/Form/errorsPropType.js
Original file line number Diff line number Diff line change
@@ -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]))])),
]),
),
]);