diff --git a/.gitignore b/.gitignore
index aa372e9d7f1..10236041c65 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,7 @@ node_modules
#IDEA
.idea/**
.vscode
+*.code-workspace
# testing
coverage
diff --git a/src/js/components/Form/Form.js b/src/js/components/Form/Form.js
index ba100519fd2..21a440bb228 100644
--- a/src/js/components/Form/Form.js
+++ b/src/js/components/Form/Form.js
@@ -81,8 +81,7 @@ const validateName =
(validationRules, required) => (name, formValue, format, messages) => {
const fieldValue = getFieldValue(name, formValue);
let validationResult;
- // ValidateArg is something that gets passed in from a FormField component
- // See 'validate' prop in FormField
+
if (
required &&
// false is for CheckBox
@@ -91,13 +90,12 @@ const validateName =
fieldValue === false ||
(Array.isArray(fieldValue) && !fieldValue.length))
) {
- // There is no value at that name, and one is required
validationResult = format({ id: 'form.required', messages });
} else if (validationRules) {
if (Array.isArray(validationRules)) {
- validationRules.some((validator) => {
+ validationRules.some((rule) => {
validationResult = validate(
- validator,
+ rule,
fieldValue,
formValue,
format,
@@ -118,7 +116,6 @@ const validateName =
return validationResult;
};
-// validations is an array from Object.entries()
// Validates all keys in formValue
const validateForm = (
validationRules,
@@ -177,29 +174,30 @@ const Form = forwardRef(
ref,
) => {
const { format } = useContext(MessageContext);
-
const [valueState, setValueState] = useState(valueProp || defaultValue);
const value = useMemo(
() => valueProp || valueState,
[valueProp, valueState],
);
const [touched, setTouched] = useState(defaultTouched);
- const [validationResults, setValidationResults] = useState(
- defaultValidationResults,
- );
- // when onBlur input validation is triggered, we need to complete any
- // potential click events before running the onBlur validation.
- // otherwise, click events like reset, etc. may not be registered.
- // for a detailed scenario/discussion,
- // see: https://github.com/grommet/grommet/issues/4863
- // the value of pendingValidation is the name of the FormField
- // awaiting validation.
- const [pendingValidation, setPendingValidation] = useState(undefined);
-
+ const [validationResults, setValidationResults] = useState({
+ errors: errorsProp,
+ infos: infosProp,
+ });
+ // maintain a copy of validationResults in a ref for useEffects
+ // which can't depend on validationResults directly without
+ // causing infinite renders.
+ const validationResultsRef = useRef({});
+ // Simulated onMount state. Consider Form to be mounted once it has
+ // accounted for values originating from controlled inputs (available
+ // at second rendering).
+ const [mounted, setMounted] = useState('unmounted');
useEffect(() => {
- setPendingValidation(undefined);
- setValidationResults({ errors: errorsProp, infos: infosProp });
- }, [errorsProp, infosProp]);
+ if (!mounted) setMounted('mounting');
+ else if (mounted === 'mounting') setMounted('mounted');
+ }, [mounted]);
+ // `pendingValidation` is the name of the FormField awaiting validation.
+ const [pendingValidation, setPendingValidation] = useState(undefined);
const validationRulesRef = useRef({});
const requiredFields = useRef([]);
@@ -233,114 +231,108 @@ const Form = forwardRef(
.forEach((n) => delete nextValidations[n]);
};
- // On initial mount, when validateOn is change or blur,
- // set validation results for any set fields and calculate whether
- // the form is valid overall.
- useEffect(() => {
- const validationsForSetFields = Object.entries(
- validationRulesRef.current,
- ).filter(([n]) => getFieldValue(n, value));
- if (validationsForSetFields.length > 0 && validateOn !== 'submit') {
- const [errors, infos] = validateForm(
- validationsForSetFields,
+ const applyValidationRules = useCallback(
+ (validationRules) => {
+ const [validatedErrors, validatedInfos] = validateForm(
+ validationRules,
value,
format,
messages,
);
- filterRemovedFields(errors);
- filterRemovedFields(infos);
- const nextValidationResults = {
- errors,
- infos,
- valid: buildValid(errors),
- };
- if (onValidate) onValidate(nextValidationResults);
- setValidationResults(nextValidationResults);
+ setValidationResults((prevValidationResults) => {
+ // Keep any previous errors and infos for untouched keys,
+ // these may have come from a Submit.
+ const nextErrors = {
+ ...prevValidationResults.errors,
+ ...validatedErrors,
+ };
+ const nextInfos = {
+ ...prevValidationResults.infos,
+ ...validatedInfos,
+ };
+ // Remove previous errors and infos for keys no longer in the
+ // form, these may have been fields removed from a dynamic form.
+ filterRemovedFields(nextErrors);
+ filterRemovedFields(nextInfos);
+ const nextValidationResults = {
+ errors: nextErrors,
+ infos: nextInfos,
+ };
+ if (onValidate)
+ onValidate({
+ ...nextValidationResults,
+ valid: buildValid(nextErrors),
+ });
+ validationResultsRef.current = nextValidationResults;
+ return nextValidationResults;
+ });
+ },
+ [buildValid, format, messages, onValidate, value],
+ );
+
+ // Validate all fields holding values onMount if set to
+ // validate when blur or change.
+ useEffect(() => {
+ const validationRules = Object.entries(validationRulesRef.current);
+ // Use simulated onMount state to account for values provided by
+ // controlled inputs.
+ if (
+ mounted !== 'mounted' &&
+ ['blur', 'change'].includes(validateOn) &&
+ Object.keys(value).length > 0 &&
+ Object.keys(touched).length === 0
+ ) {
+ applyValidationRules(
+ validationRules
+ .filter(([n]) => value[n])
+ // Exlude empty arrays which may be initial values in
+ // an input such as DateInput.
+ .filter(
+ ([n]) => !(Array.isArray(value[n]) && value[n].length === 0),
+ ),
+ );
}
- // We only want to run this for the value we have on initial mount.
- // We don't want subsequent changes to the value to re-run this.
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, []);
+ }, [applyValidationRules, mounted, touched, validateOn, value]);
- // Currently, onBlur validation will trigger after a timeout of 120ms.
+ // Run validation against fields with pendingValidations from onBlur
+ // and/or onChange.
useEffect(() => {
+ const validationRules = Object.entries(validationRulesRef.current);
const timer = setTimeout(() => {
- if (pendingValidation) {
- // run validations on the pending one and any other touched fields
- const [validatedErrors, validatedInfos] = validateForm(
- Object.entries(validationRulesRef.current).filter(
+ if (pendingValidation && ['blur', 'change'].includes(validateOn)) {
+ applyValidationRules(
+ validationRules.filter(
([n]) => touched[n] || pendingValidation.includes(n),
),
- value,
- format,
- messages,
);
setPendingValidation(undefined);
-
- setValidationResults((prevValidationResults) => {
- // keep any previous errors and infos for untouched keys,
- // these may have come from a submit
- const nextErrors = {
- ...prevValidationResults.errors,
- ...validatedErrors,
- };
- const nextInfos = {
- ...prevValidationResults.infos,
- ...validatedInfos,
- };
-
- filterRemovedFields(nextErrors);
- filterRemovedFields(nextInfos);
-
- const nextValidationResults = {
- errors: nextErrors,
- infos: nextInfos,
- valid: buildValid(nextErrors),
- };
- if (onValidate) onValidate(nextValidationResults);
- return nextValidationResults;
- });
}
- // a timeout is needed to ensure that a click event (like one on a reset
- // button) completes prior to running the validation. without a timeout,
- // the blur will always complete and trigger a validation prematurely
- // The following values have been empirically tested, but 120 was
- // selected because it is the largest value
+ // Complete any potential click events before running onBlur validation.
+ // Otherwise, click events like reset, etc. may not be registered. For a
+ // detailed scenario/discussion, see: https://github.com/grommet/grommet/issues/4863
+ // Values empirically tested; 120 was selected because it is the largest
// Chrome: 100, Safari: 120, Firefox: 80
}, 120);
-
return () => clearTimeout(timer);
- }, [
- buildValid,
- format,
- messages,
- pendingValidation,
- onValidate,
- touched,
- value,
- requiredFields,
- ]);
+ }, [applyValidationRules, pendingValidation, touched, validateOn]);
- // clear any errors when value changes
+ // Re-run validation rules for all fields with prior errors.
+ // if validate=blur this helps re-validate if there are errors
+ // as the user fixes them (basically act like validate=change for that)
useEffect(() => {
- if (validateOn !== 'change') setPendingValidation(undefined);
- setValidationResults((prevValidationResults) => {
- const [nextErrors, nextInfos] = validateForm(
- Object.entries(validationRulesRef.current).filter(
- ([n]) =>
- prevValidationResults.errors[n] || prevValidationResults.infos[n],
+ const validationRules = Object.entries(validationRulesRef.current);
+ if (
+ validationResultsRef.current?.errors &&
+ Object.keys(validationResultsRef.current.errors).length > 0
+ ) {
+ applyValidationRules(
+ validationRules.filter(
+ ([n]) => touched[n] && validationResultsRef.current.errors[n],
),
- value,
- format,
- messages,
);
- return {
- errors: { ...prevValidationResults.errors, ...nextErrors },
- infos: { ...prevValidationResults.infos, ...nextInfos },
- };
- });
- }, [format, messages, touched, validateOn, value]);
+ }
+ }, [applyValidationRules, touched]);
// There are three basic patterns of handling form input value state:
//
@@ -425,7 +417,8 @@ const Form = forwardRef(
// eslint-disable-next-line react-hooks/exhaustive-deps
[], // only run onmount and unmount
);
-
+
+ // Create validation rules for fields
useEffect(() => {
if (validateArg) {
if (!validationRulesRef.current[name]) {
@@ -467,11 +460,7 @@ const Form = forwardRef(
// we know to remove its value from the form if it is dynamically
// removed
if (!(name in value)) keyCreated.current = true;
- const nextValue = setFieldValue(
- name,
- nextComponentValue,
- value,
- );
+ const nextValue = setFieldValue(name, nextComponentValue, value);
setValueState(nextValue);
if (onChange) onChange(nextValue, { touched: nextTouched });
}
@@ -493,6 +482,7 @@ const Form = forwardRef(
: errorArg || validationResults.errors[name];
const info = infoArg || validationResults.infos[name];
+ // Create validation rules for field
useEffect(() => {
const index = requiredFields.current.indexOf(name);
if (required) {
@@ -558,7 +548,6 @@ const Form = forwardRef(
}
setTouched(defaultTouched);
setValidationResults(defaultValidationResults);
-
if (onReset) {
event.persist(); // extract from React's synthetic event pool
const adjustedEvent = event;
@@ -588,6 +577,7 @@ const Form = forwardRef(
valid: buildValid(nextErrors),
};
if (onValidate) onValidate(nextValidationResults);
+ validationResultsRef.current = nextValidationResults;
return nextValidationResults;
});
diff --git a/src/js/components/Form/__tests__/Form-test-controlled.js b/src/js/components/Form/__tests__/Form-test-controlled.js
index 0488dc1e4f5..855383e9a68 100644
--- a/src/js/components/Form/__tests__/Form-test-controlled.js
+++ b/src/js/components/Form/__tests__/Form-test-controlled.js
@@ -1,8 +1,8 @@
-import React from 'react';
+import React, { useState } from 'react';
import 'jest-styled-components';
-import { act, render, fireEvent } from '@testing-library/react';
+import { act, render, fireEvent, screen } from '@testing-library/react';
import { Grommet } from '../../Grommet';
import { Form } from '..';
import { FormField } from '../../FormField';
@@ -476,8 +476,7 @@ describe('Form controlled', () => {
moodField.focus();
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 1,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({
errors: { mood: 'required' },
infos: {},
@@ -489,8 +488,7 @@ describe('Form controlled', () => {
fireEvent.change(moodField, { target: { value: 'testy' } });
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 2,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({ errors: {}, infos: {} }),
);
@@ -499,8 +497,7 @@ describe('Form controlled', () => {
fireEvent.change(moodField, { target: { value: '' } });
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 3,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({
errors: { mood: 'required' },
infos: {},
@@ -513,8 +510,7 @@ describe('Form controlled', () => {
nameField.focus();
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 4,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({ errors: {}, infos: {} }),
);
@@ -721,4 +717,214 @@ describe('Form controlled', () => {
);
expect(container.firstChild).toMatchSnapshot();
});
+
+ test('validate on mount - controlled form', () => {
+ const Test = () => {
+ const [formValue, setFormValue] = useState({
+ firstName: 'J',
+ middleName: '1',
+ lastName: '',
+ title: 1,
+ });
+
+ return (
+
+ );
+ };
+
+ render(
+
+
+ ,
+ );
+
+ // firstName triggers string length validation, but not regexp
+ expect(screen.queryAllByText('must be >1 character')).toHaveLength(1);
+ // middleName & title trigger regexp, but not string length validation
+ expect(screen.queryAllByText('invalid')).toHaveLength(2);
+ // lastName should not trigger required validation onMount
+ expect(screen.queryAllByText('required')).toHaveLength(0);
+ });
+
+ test('validate on mount - controlled input', () => {
+ const Test = () => {
+ const [firstName, setFirstName] = useState('J');
+ const [middleName, setMiddleName] = useState('1');
+ const [lastName, setLastName] = useState('');
+ const [title, setTitle] = useState(1);
+
+ return (
+
+ );
+ };
+
+ render(
+
+
+ ,
+ );
+
+ // firstName triggers string length validation, but not regexp
+ expect(screen.queryAllByText('must be >1 character')).toHaveLength(1);
+ // middleName & title trigger regexp, but not string length validation
+ expect(screen.queryAllByText('invalid')).toHaveLength(2);
+ // lastName should not trigger required validation onMount
+ expect(screen.queryAllByText('required')).toHaveLength(0);
+ });
});
diff --git a/src/js/components/Form/__tests__/Form-test-uncontrolled.js b/src/js/components/Form/__tests__/Form-test-uncontrolled.js
index ec25e85e0ad..a4b1c8f5205 100644
--- a/src/js/components/Form/__tests__/Form-test-uncontrolled.js
+++ b/src/js/components/Form/__tests__/Form-test-uncontrolled.js
@@ -600,10 +600,7 @@ describe('Form uncontrolled', () => {
expect(queryAllByText('good')).toHaveLength(1);
});
- test('validate on mount', async () => {
- jest.useFakeTimers('modern');
- window.scrollTo = jest.fn();
-
+ test('validate on mount', () => {
const defaultValue = {
firstName: 'J',
lastName: '',
@@ -1388,8 +1385,7 @@ describe('Form uncontrolled', () => {
moodField.focus();
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 1,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({
errors: { mood: 'required' },
infos: {},
@@ -1401,8 +1397,7 @@ describe('Form uncontrolled', () => {
fireEvent.change(moodField, { target: { value: 'testy' } });
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 2,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({ errors: {}, infos: {} }),
);
@@ -1411,8 +1406,7 @@ describe('Form uncontrolled', () => {
fireEvent.change(moodField, { target: { value: '' } });
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 3,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({
errors: { mood: 'required' },
infos: {},
@@ -1425,8 +1419,7 @@ describe('Form uncontrolled', () => {
nameField.focus();
toggleField.focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 4,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({ errors: {}, infos: {} }),
);
@@ -1491,8 +1484,7 @@ describe('Form uncontrolled', () => {
getByText('Focus out').focus();
act(() => jest.advanceTimersByTime(200)); // allow validations to run
- expect(onValidate).toHaveBeenNthCalledWith(
- 2,
+ expect(onValidate).toHaveBeenLastCalledWith(
expect.objectContaining({
errors: {},
infos: {},
diff --git a/src/js/components/Form/__tests__/__snapshots__/Form-test-uncontrolled.js.snap b/src/js/components/Form/__tests__/__snapshots__/Form-test-uncontrolled.js.snap
index 6c8dca8113a..99a5522bbe5 100644
--- a/src/js/components/Form/__tests__/__snapshots__/Form-test-uncontrolled.js.snap
+++ b/src/js/components/Form/__tests__/__snapshots__/Form-test-uncontrolled.js.snap
@@ -2163,10 +2163,6 @@ exports[`Form uncontrolled errors 1`] = `
}
}
-@media only screen and (max-width:768px) {
-
-}
-
@media only screen and (max-width:768px) {
.c2 {
border-bottom: solid 1px #FF4040;
diff --git a/src/js/components/Form/stories/FieldBorderPosition.js b/src/js/components/Form/stories/FieldBorderPosition.js
index b90c8c11146..973c9bce19e 100644
--- a/src/js/components/Form/stories/FieldBorderPosition.js
+++ b/src/js/components/Form/stories/FieldBorderPosition.js
@@ -132,6 +132,7 @@ export const FieldBorderPosition = () => (
{borderPositions &&
borderPositions.map((example, index) => (