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 6 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
4 changes: 2 additions & 2 deletions src/components/Form/FormWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]))])),
kowczarz marked this conversation as resolved.
Show resolved Hide resolved

/** Field-specific server side errors keyed by microtime */
errorFields: PropTypes.objectOf(PropTypes.objectOf(PropTypes.string)),
Expand All @@ -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.objectOf(PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]))])).isRequired,

inputRefs: PropTypes.objectOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object])).isRequired,
};
Expand Down
Loading