diff --git a/.changeset/sharp-plants-shave.md b/.changeset/sharp-plants-shave.md index 21f56789cf..e3ac6056ba 100644 --- a/.changeset/sharp-plants-shave.md +++ b/.changeset/sharp-plants-shave.md @@ -2,4 +2,4 @@ '@lion/ui': patch --- -feat: allow Required validator on Fieldset and Form; `static executesOnEmpty` flag on Validators +feat: allow Required validator on Fieldset and Form; diff --git a/packages/ui/components/form-core/src/validate/ValidateMixin.js b/packages/ui/components/form-core/src/validate/ValidateMixin.js index a901123f8f..dc119f373d 100644 --- a/packages/ui/components/form-core/src/validate/ValidateMixin.js +++ b/packages/ui/components/form-core/src/validate/ValidateMixin.js @@ -432,22 +432,30 @@ export const ValidateMixinImplementation = superclass => */ const isEmpty = this.__isEmpty(value); - this.__syncValidationResult = []; + + /** + * So we can have the following scenarios: + * - we're empty + * - we have a single value (we are an 'endpoint') => we want to halt execution, because validation only makes sense when we have a value + * - 1. we have Required => we fill .__syncValidationResult and finish validation, because we have a value to validate + * - 2. we don't have Required => we stop validation + * - we have a group of values (object/array) => we want to continue execution, because the constellation of the object (even though the individual endpoints are empty) might be interesting for validation. + * - 3. we have Required => we fill .__syncValidationResult and continue validation (so we can add more to .__syncValidationResult) + * - 4. we don't have Required => we continue validation + * - we're not empty + * - we have a single value or group of values + * - 5. we may have Required, but if we have it will not be 'active' => we continue execution, because we have a value to validate + */ + if (isEmpty) { + const hasSingleValue = !(/** @type {* & ValidateHost} */ (this)._isFormOrFieldset); const requiredValidator = this._allValidators.find(v => v instanceof Required); if (requiredValidator) { this.__syncValidationResult = [{ validator: requiredValidator, outcome: true }]; } - // TODO: get rid of _isFormOrFieldset in a breaking future update. - // For now, We should keep forms and fieldsets backwards compatible... - const validatorsThatShouldRunOnEmpty = this._allValidators.filter( - v => v.constructor.executesOnEmpty, - ); - const shouldHaltValidationOnEmpty = - !validatorsThatShouldRunOnEmpty.length && !this._isFormOrFieldset; - if (shouldHaltValidationOnEmpty) { + if (hasSingleValue) { this.__finishValidationPass({ syncValidationResult: this.__syncValidationResult, }); @@ -463,7 +471,6 @@ export const ValidateMixinImplementation = superclass => if (v instanceof MetaValidator) { metaValidators.push(v); } else if (v instanceof Required) { - // syncValidators.unshift(v); // Required validator was already handled } else if (/** @type {typeof Validator} */ (v.constructor).async) { asyncValidators.push(v); diff --git a/packages/ui/components/form-core/test-suites/ValidateMixin.suite.js b/packages/ui/components/form-core/test-suites/ValidateMixin.suite.js index 9fbfd78325..54bec85b10 100644 --- a/packages/ui/components/form-core/test-suites/ValidateMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/ValidateMixin.suite.js @@ -855,34 +855,6 @@ export function runValidateMixinSuite(customConfig) { expect(alwaysInvalidSpy.callCount).to.equal(1); // __isRequired returned true (valid) }); - it('does not prevent other Validators from being called when input is empty, but at least one Validator has "executesOnEmpty"', async () => { - class AlwaysInvalidExecutingOnEmpty extends Validator { - static validatorName = 'AlwaysInvalidExecutingOnEmpty'; - - static executesOnEmpty = true; - - execute() { - return true; - } - } - const alwaysInvalidExecutingOnEmpty = new AlwaysInvalidExecutingOnEmpty(); - const aalwaysInvalidExecutingOnEmptySpy = sinon.spy( - alwaysInvalidExecutingOnEmpty, - 'execute', - ); - const el = /** @type {ValidateElement} */ ( - await fixture(html` - <${tag} - .validators=${[new Required(), alwaysInvalidExecutingOnEmpty]} - .modelValue=${''} - >${lightDom} - `) - ); - expect(aalwaysInvalidExecutingOnEmptySpy.callCount).to.equal(1); - el.modelValue = 'foo'; - expect(aalwaysInvalidExecutingOnEmptySpy.callCount).to.equal(2); - }); - it('adds [aria-required="true"] to "._inputNode"', async () => { const el = /** @type {ValidateElement} */ ( await fixture(html` diff --git a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js index ec01adb7f0..02a283d154 100644 --- a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js @@ -589,9 +589,9 @@ export function runFormGroupMixinSuite(cfg = {}) { // initially the group is invalid expect(el.validationStates.error.Required).to.be.true; el.formElements.fieldA.modelValue = 'foo'; - // If at least one child is filled, the group is valid + // if at least one child is filled, the group is valid expect(el.validationStates.error.Required).to.be.undefined; - // // initially the group is invalid + // make Required trigger error state again el.formElements.fieldA.modelValue = ''; expect(el.validationStates.error.Required).to.be.true; });