From f58ae24f34ca51d45606c813ff7042d447095ed6 Mon Sep 17 00:00:00 2001 From: brendanjbond Date: Thu, 21 Nov 2024 14:17:18 -0600 Subject: [PATCH 1/4] validate current page on wizard change --- src/Wizard.js | 20 +++++++++++--------- src/components/form/Form.js | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Wizard.js b/src/Wizard.js index 35c800f497..4f3ca2563c 100644 --- a/src/Wizard.js +++ b/src/Wizard.js @@ -824,8 +824,9 @@ export default class Wizard extends Webform { } validateCurrentPage(flags = {}) { + const components = this.currentPage?.components.map((component) => component.component); // Accessing the parent ensures the right instance (whether it's the parent Wizard or a nested Wizard) performs its validation - return this.currentPage?.parent.validateComponents(this.currentPage.component.components, this.currentPage.parent.data, flags); + return this.currentPage?.parent.validateComponents(components, this.currentPage.parent.data, flags); } emitPrevPage() { @@ -1000,7 +1001,8 @@ export default class Wizard extends Webform { onChange(flags, changed, modified, changes) { super.onChange(flags, changed, modified, changes); - const errors = this.validate(this.localData, { dirty: false }); + // The onChange loop doesn't need all components for wizards + const errors = this.submitted ? this.validate(this.localData, { dirty: false }) : this.validateCurrentPage(); if (this.alert) { this.showErrors(errors, true, true); } @@ -1074,13 +1076,13 @@ export default class Wizard extends Webform { } showErrors(errors, triggerEvent) { - if (this.hasExtraPages) { - this.subWizards.forEach((subWizard) => { - if(Array.isArray(subWizard.errors)) { - errors = [...errors, ...subWizard.errors] - } - }) - }; + // if (this.hasExtraPages) { + // this.subWizards.forEach((subWizard) => { + // if(Array.isArray(subWizard.errors)) { + // errors = [...errors, ...subWizard.errors] + // } + // }) + // } return super.showErrors(errors, triggerEvent) } diff --git a/src/components/form/Form.js b/src/components/form/Form.js index c9d36281cf..f402414fb2 100644 --- a/src/components/form/Form.js +++ b/src/components/form/Form.js @@ -535,7 +535,7 @@ export default class FormComponent extends Component { options = options || {}; const silentCheck = options.silentCheck || false; - if (this.subForm && !this.isNestedWizard) { + if (this.subForm) { return this.subForm.checkValidity(this.subFormData, dirty, null, silentCheck, errors); } From 144d0c7c0c434d3c0584044143f6f97374c875a8 Mon Sep 17 00:00:00 2001 From: brendanjbond Date: Thu, 21 Nov 2024 18:25:57 -0600 Subject: [PATCH 2/4] add tests --- src/Wizard.js | 11 --- test/unit/Wizard.unit.js | 153 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 146 insertions(+), 18 deletions(-) diff --git a/src/Wizard.js b/src/Wizard.js index 4f3ca2563c..ae50f550c5 100644 --- a/src/Wizard.js +++ b/src/Wizard.js @@ -1075,17 +1075,6 @@ export default class Wizard extends Webform { return super.errors; } - showErrors(errors, triggerEvent) { - // if (this.hasExtraPages) { - // this.subWizards.forEach((subWizard) => { - // if(Array.isArray(subWizard.errors)) { - // errors = [...errors, ...subWizard.errors] - // } - // }) - // } - return super.showErrors(errors, triggerEvent) - } - focusOnComponent(key) { const component = this.getComponent(key); if (component) { diff --git a/test/unit/Wizard.unit.js b/test/unit/Wizard.unit.js index 62eab3c671..2b0f56e6b6 100644 --- a/test/unit/Wizard.unit.js +++ b/test/unit/Wizard.unit.js @@ -42,6 +42,7 @@ import formsWithAllowOverride from '../forms/formsWithAllowOverrideComps'; import WizardWithCheckboxes from '../forms/wizardWithCheckboxes'; import WizardWithRequiredFields from '../forms/wizardWithRequiredFields'; import formWithNestedWizardAndRequiredFields from '../forms/formWithNestedWizardAndRequiredFields'; +import { wait } from '../util'; // eslint-disable-next-line max-statements describe('Wizard tests', () => { @@ -390,7 +391,7 @@ describe('Wizard tests', () => { .catch((err) => done(err)); }).timeout(6000); - it('Should trigger validation of nested wizard before going to the next page', function(done) { + it('Should trigger validation of nested wizard before going to the next page', function (done) { const formElement = document.createElement('div'); const wizard = new Wizard(formElement); const nestedWizard = _.cloneDeep(WizardWithRequiredFields); @@ -415,7 +416,7 @@ describe('Wizard tests', () => { wizard.setForm(parentWizard).then(() => { const nestedFormComp = wizard.getComponent('formNested'); - nestedFormComp.loadSubForm = ()=> { + nestedFormComp.loadSubForm = () => { nestedFormComp.formObj = nestedWizard; nestedFormComp.subFormLoading = false; return new Promise((resolve) => resolve(nestedWizard)); @@ -439,15 +440,153 @@ describe('Wizard tests', () => { assert.equal(errors.length, 2, 'Must err before next page'); errors.forEach((error) => { assert.equal(error.ruleName, 'required'); - assert.equal(error.message, 'Text Field is required' , 'Should set correct lebel in the error message'); + assert.equal(error.message, 'Text Field is required', 'Should set correct lebel in the error message'); }); done(); }, 300) }, 300) }, 300) }) - .catch((err) => done(err)); - }) + .catch((err) => done(err)); + }); + + it('Should only validate the current page components when the form has not been submitted', async function () { + const wizardDefinition = { + display: 'wizard', + components: [ + { + title: 'Page 1', + collapsible: false, + key: 'page1', + type: 'panel', + label: 'Panel', + input: false, + tableView: false, + components: [ + { + type: 'checkbox', + label: 'Trigger Change', + input: true, + key: 'triggerChange', + }, + { + label: 'Text Field', + applyMaskOn: 'change', + tableView: true, + validateWhenHidden: false, + key: 'textField', + type: 'textfield', + input: true, + validate: { + required: true + } + }, + ], + }, + { + title: 'Page 2', + collapsible: false, + key: 'page2', + type: 'panel', + label: 'Panel', + input: false, + tableView: false, + components: [ + { + label: 'Text Field', + applyMaskOn: 'change', + tableView: true, + validateWhenHidden: false, + key: 'textField1', + type: 'textfield', + validate: { + required: true + }, + input: true, + }, + ], + }, + ], + }; + + const form = await Formio.createForm(document.createElement('div'), wizardDefinition); + assert(form, 'Form should be created'); + const checkbox = form.getComponent('triggerChange'); + const clickEvent = new Event('click'); + checkbox.refs.input[0].dispatchEvent(clickEvent); + await wait(200); + assert.equal(form.errors.length, 1, 'Should have one error from the first page'); + }); + + it('Should validate the entire wizard\'s components when the form has been submitted', async function () { + const wizardDefinition = { + display: 'wizard', + components: [ + { + title: 'Page 1', + collapsible: false, + key: 'page1', + type: 'panel', + label: 'Panel', + input: false, + tableView: false, + components: [ + { + type: 'checkbox', + label: 'Trigger Change', + input: true, + key: 'triggerChange', + }, + { + label: 'Text Field', + applyMaskOn: 'change', + tableView: true, + validateWhenHidden: false, + key: 'textField', + type: 'textfield', + input: true, + validate: { + required: true + } + }, + ], + }, + { + title: 'Page 2', + collapsible: false, + key: 'page2', + type: 'panel', + label: 'Panel', + input: false, + tableView: false, + components: [ + { + label: 'Text Field', + applyMaskOn: 'change', + tableView: true, + validateWhenHidden: false, + key: 'textField1', + type: 'textfield', + validate: { + required: true + }, + input: true, + }, + ], + }, + ], + }; + + const form = await Formio.createForm(document.createElement('div'), wizardDefinition); + assert(form, 'Form should be created'); + form.submitted = true; + const checkbox = form.getComponent('triggerChange'); + const clickEvent = new Event('click'); + checkbox.refs.input[0].dispatchEvent(clickEvent); + await wait(200); + assert.equal(form.errors.length, 2, 'Should have two errors total'); + }); + it('Should have validation errors when parent form is valid but nested wizard is not', function(done) { const formElement = document.createElement('div'); @@ -482,11 +621,11 @@ describe('Wizard tests', () => { assert.equal(wizard.submission.data.textField, 'test'); const nestedWizardBreadcrumbBtn = _.get(wizard.refs, `${wizard.wizardKey}-link[4]`); nestedWizardBreadcrumbBtn.dispatchEvent(clickEvent); - + setTimeout(() => { checkPage(4); _.get(wizard.refs, `${wizard.wizardKey}-submit`).dispatchEvent(clickEvent); - + setTimeout(() => { assert.equal(wizard.errors.length, 2); done(); From ec4c1d3d65d1a54b2351e1d4ad7d67c28db10c65 Mon Sep 17 00:00:00 2001 From: brendanjbond Date: Thu, 21 Nov 2024 20:00:03 -0600 Subject: [PATCH 3/4] fix tests --- test/unit/Wizard.unit.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/unit/Wizard.unit.js b/test/unit/Wizard.unit.js index 2b0f56e6b6..036f8085e2 100644 --- a/test/unit/Wizard.unit.js +++ b/test/unit/Wizard.unit.js @@ -517,7 +517,7 @@ describe('Wizard tests', () => { await wait(200); assert.equal(form.errors.length, 1, 'Should have one error from the first page'); }); - + it('Should validate the entire wizard\'s components when the form has been submitted', async function () { const wizardDefinition = { display: 'wizard', @@ -580,13 +580,12 @@ describe('Wizard tests', () => { const form = await Formio.createForm(document.createElement('div'), wizardDefinition); assert(form, 'Form should be created'); form.submitted = true; - const checkbox = form.getComponent('triggerChange'); - const clickEvent = new Event('click'); - checkbox.refs.input[0].dispatchEvent(clickEvent); + form.setSubmission({ data: { triggerChange: true } }); await wait(200); - assert.equal(form.errors.length, 2, 'Should have two errors total'); + assert(form.alert, 'Form should have an error list'); + assert.equal(form.alert.querySelectorAll("span[ref=errorRef]").length, 2, 'Should have two errors total'); }); - + it('Should have validation errors when parent form is valid but nested wizard is not', function(done) { const formElement = document.createElement('div'); From f1002ffd9157e7dd8e676a62f1b895f4413112d1 Mon Sep 17 00:00:00 2001 From: Blake Krammes <49688912+blakekrammes@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:29:15 -0600 Subject: [PATCH 4/4] Specify correct flag for form validation after failed wizard submission - this.validate() is only called in onChange() if the form previously failed to submit. Because of this, the form will already be dirty, so it makes sense to reiterate this by passing dirty: true instead of false. --- src/Wizard.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Wizard.js b/src/Wizard.js index ae50f550c5..f5bafbfea3 100644 --- a/src/Wizard.js +++ b/src/Wizard.js @@ -1002,7 +1002,7 @@ export default class Wizard extends Webform { onChange(flags, changed, modified, changes) { super.onChange(flags, changed, modified, changes); // The onChange loop doesn't need all components for wizards - const errors = this.submitted ? this.validate(this.localData, { dirty: false }) : this.validateCurrentPage(); + const errors = this.submitted ? this.validate(this.localData, { dirty: true }) : this.validateCurrentPage(); if (this.alert) { this.showErrors(errors, true, true); }