pkp/pkp-lib#9996 Initial attempt to ignore hidden fields for validate… (

#357)

* pkp/pkp-lib#9996 Remove error for input that gets hidden

* pkp/pkp-lib#9996 check only visible fields for required

* pkp/pkp-lib#9996 Handle correctly multiple fields removing error synchronously

* pkp/pkp-lib#9996 add jsdocs

src/components/Form/Form.vue

@@ -67,6 +67,7 @@
 <script>
 import FormLocales from './FormLocales.vue';
 import FormPage from './FormPage.vue';
+import {shouldShowField} from './formHelpers';
 
 export default {
 	name: 'PkpForm',
@@ -332,7 +333,10 @@ export default {
 		validateRequired() {
 			let errors = {};
 			this.fields.forEach((field) => {
-				if (!field.isRequired) {
+				if (
+					!field.isRequired ||
+					!shouldShowField(field, this.fields, this.groups)
+				) {
 					return;
 				}
 				let missingValue = false;

src/components/Form/FormGroup.vue

@@ -78,6 +78,8 @@ import FieldUpload from './fields/FieldUpload.vue';
 import FieldSlider from './fields/FieldSlider.vue';
 import FieldUploadImage from './fields/FieldUploadImage.vue';
 
+import {shouldShowFieldWithinGroup} from './formHelpers';
+
 export default {
 	name: 'FormGroup',
 	components: {
@@ -127,7 +129,9 @@ export default {
 		 */
 		fieldsInGroup() {
 			return this.fields.filter(
-				(field) => field.groupId === this.id && this.shouldShowField(field),
+				(field) =>
+					field.groupId === this.id &&
+					shouldShowFieldWithinGroup(field, this.fields),
 			);
 		},
 
@@ -153,30 +157,6 @@ export default {
 			this.$emit('change', name, prop, value, localeKey);
 		},
 
-		/**
-		 * Should a field be shown?
-		 *
-		 * @param {Object} field One of this.fields
-		 * @return {Boolean}
-		 */
-		shouldShowField: function (field) {
-			if (typeof field.showWhen === 'undefined') {
-				return true;
-			}
-			const whenFieldName =
-				typeof field.showWhen === 'string' ? field.showWhen : field.showWhen[0];
-			const whenField = this.fields.find(
-				(field) => field.name === whenFieldName,
-			);
-			if (!whenField) {
-				return false;
-			}
-			if (typeof field.showWhen === 'string') {
-				return !!whenField.value;
-			}
-			return whenField.value === field.showWhen[1];
-		},
-
 		/**
 		 * Respond to a field changing its errors
 		 *
@@ -185,7 +165,19 @@ export default {
 		 * @param {String} localeKey The locale key for a multilingual field
 		 */
 		setFieldErrors: function (name, errors, localeKey = '') {
-			let newErrors = {...this.errors};
+			/**
+			 * Better practice is to make to copy of data, modify it and send it upstream
+			 * But there is challenge with #9996, where each unmounted fields needs to clean up
+			 * its error. And given its happening synchronously, it does not propagate updated errors
+			 * object in between the unmounts. Therefore last operation would override the previous ones.
+			 *
+			 * Changing error object inplace ensures all the updates gets registered.
+			 *
+			 * In future better approach would be to have remove-error event, which defines what to remove
+			 * and would be correctly processed by the state manager
+			 */
+
+			let newErrors = this.errors;
 			if (!errors || !errors.length) {
 				if (localeKey && newErrors[name] && newErrors[name][localeKey]) {
 					delete newErrors[name][localeKey];
@@ -200,7 +192,7 @@ export default {
 					newErrors[name] = errors;
 				}
 			}
-			this.$emit('set-errors', newErrors);
+			this.$emit('set-errors', {...newErrors});
 		},
 	},
 };

src/components/Form/FormPage.vue

@@ -63,7 +63,7 @@
 import ButtonRow from '@/components/ButtonRow/ButtonRow.vue';
 import FormErrors from '@/components/Form/FormErrors.vue';
 import FormGroup from '@/components/Form/FormGroup.vue';
-
+import {shouldShowGroup} from './formHelpers';
 export default {
 	name: 'FormPage',
 	components: {
@@ -107,7 +107,8 @@ export default {
 		 */
 		groupsInPage() {
 			return this.groups.filter(
-				(group) => group.pageId === this.id && this.shouldShowGroup(group),
+				(group) =>
+					group.pageId === this.id && shouldShowGroup(group, this.fields),
 			);
 		},
 
@@ -186,30 +187,6 @@ export default {
 			this.$emit('showLocale', localeKey);
 		},
 
-		/**
-		 * Should a group be shown?
-		 *
-		 * @param {Object} group One of this.groups
-		 * @return {Boolean}
-		 */
-		shouldShowGroup: function (group) {
-			if (typeof group.showWhen === 'undefined') {
-				return true;
-			}
-			const whenFieldName =
-				typeof group.showWhen === 'string' ? group.showWhen : group.showWhen[0];
-			const whenField = this.fields.find(
-				(field) => field.name === whenFieldName,
-			);
-			if (!whenField) {
-				return false;
-			}
-			if (typeof group.showWhen === 'string') {
-				return !!whenField.value;
-			}
-			return whenField.value === group.showWhen[1];
-		},
-
 		/**
 		 * Pass an event up to the form to set the errors object
 		 *

src/components/Form/fields/FieldBase.vue

@@ -61,6 +61,7 @@ export default {
 	emits: [
 		/** Emitted when a field prop changes. Payload: `(fieldName, propName, newValue, [localeKey])`. The `localeKey` will be null for fields that are not multilingual. This event is fired every time the `value` changes, so you should [debounce](https://www.npmjs.com/package/debounce) event callbacks that contain resource-intensive code. */
 		'change',
+		'set-errors',
 	],
 	computed: {
 		/**
@@ -255,6 +256,9 @@ export default {
 			});
 		},
 	},
+	beforeUnmount() {
+		this.$emit('set-errors', this.name, [], this.localeKey);
+	},
 	methods: {
 		/**
 		 * Helper function to compile unique IDs for labels and aria-describedby

src/components/Form/formHelpers.js

@@ -0,0 +1,59 @@
+/**
+ * Should a field be shown?
+ *
+ * @param {Object} field One of this.fields
+ * @return {Boolean}
+ */
+export function shouldShowFieldWithinGroup(field, allFields) {
+	if (typeof field.showWhen === 'undefined') {
+		return true;
+	}
+	const whenFieldName =
+		typeof field.showWhen === 'string' ? field.showWhen : field.showWhen[0];
+	const whenField = allFields.find((field) => field.name === whenFieldName);
+	if (!whenField) {
+		return false;
+	}
+	if (typeof field.showWhen === 'string') {
+		return !!whenField.value;
+	}
+	return whenField.value === field.showWhen[1];
+}
+/**
+ * Should a group be shown?
+ *
+ * @param {Object} group One of this.groups
+ * @return {Boolean}
+ */
+export function shouldShowGroup(group, fields) {
+	if (typeof group.showWhen === 'undefined') {
+		return true;
+	}
+	const whenFieldName =
+		typeof group.showWhen === 'string' ? group.showWhen : group.showWhen[0];
+	const whenField = fields.find((field) => field.name === whenFieldName);
+	if (!whenField) {
+		return false;
+	}
+	if (typeof group.showWhen === 'string') {
+		return !!whenField.value;
+	}
+	return whenField.value === group.showWhen[1];
+}
+
+/**
+ * Should a field be shown?
+ *
+ * @param {Object} field One field to check
+ * @param {Object} fields All form fields
+ * @param {Object} groups All form groups
+ * @return {Boolean}
+ */
+
+export function shouldShowField(field, fields, groups) {
+	const group = groups.find((group) => group.id === field.groupId);
+
+	return (
+		shouldShowGroup(group, fields) && shouldShowFieldWithinGroup(field, fields)
+	);
+}