Merge pull request #1330 from ing-bank/fix/validate

fix(form-core): validate on child change + {feedbackType}StateChanged

.changeset/fair-geese-drum.md

@@ -0,0 +1,6 @@
+---
+'@lion/form-core': patch
+---
+
+- validation of form groups when child fires 'model-value-changed'
+- fire {feedbackType}StateChanged event when feedback type (like 'error'/'warning') changed

docs/docs/systems/form/interaction-states.md

@@ -4,7 +4,8 @@
 import { html, render } from '@lion/core';
 import { renderLitAsNode } from '@lion/helpers';
 import '@lion/input/define';
-import { Validator } from '@lion/form-core';
+import '@lion/input-date/define';
+import { Validator, Unparseable, MinDate, Required } from '@lion/form-core';
 import './assets/h-output.js';
 ```
 
@@ -118,7 +119,7 @@ export const feedbackCondition = () => {
     ></lion-input>
   `);
 
-  // 4. When checkboxes change, set the showFeedbackConditionFor method to a new method that checks
+  // 4. When checkboxes change, set the feedbackCondition method to a new method that checks
   // whether every condition that is checked, is true on the field. Otherwise, don't show the feedback.
   const fetchConditionsAndReevaluate = ({ currentTarget: { modelValue } }) => {
     fieldElement._showFeedbackConditionFor = type => {
@@ -142,3 +143,35 @@ export const feedbackCondition = () => {
   `;
 };
 ```
+
+### Changing the feedback show condition (Application Developers)
+
+In some situations, the default condition for showing feedback messages might not apply.
+The conditions as described in 'When is feedback shown to the user' can be overidden via
+the `feedbackCondition` method.
+In this example, we want to implement the following situation:
+
+- for an `input-date`, we have a MinDate condition
+- we want to send feedback as soon as we know the user intentionally typed a full Date
+  (we know if modelValue is not Unparseable)
+
+```js preview-story
+export const feedbackVisibility = () => html`
+  <lion-input-date
+    .validators="${[
+      new Required(),
+      new MinDate(new Date('2000/10/10'), {
+        getMessage: () => `You provided a correctly formatted date, but it's below MinData`,
+      }),
+    ]}"
+    .feedbackCondition="${(type, meta, originalCondition) => {
+      if (meta.modelValue && !(meta.modelValue instanceof Unparseable)) {
+        return true;
+      }
+      return originalCondition(type, meta);
+    }}"
+    help-text="Error appears as soon as a Parseable date before 10/10/2000 is typed"
+    label="Custom feedback visibility"
+  ></lion-input-date>
+`;
+```

docs/docs/systems/form/validate.md

@@ -45,9 +45,11 @@ import {
   Required,
   Validator,
   Pattern,
+  Unparseable,
 } from '@lion/form-core';
 import { localize } from '@lion/localize';
 import { loadDefaultFeedbackMessages } from '@lion/validate-messages';
+import { renderLitAsNode } from '@lion/helpers';
 ```
 
 ## When validation happens
@@ -804,3 +806,92 @@ export const backendValidation = () => {
   `;
 };
 ```
+
+## Multiple field validation
+
+When validation is dependent on muliple fields, two approaches can be considered:
+
+- Fieldset validation
+- Validators with knowledge about context
+
+### Fieldset validation
+
+Assume we have a field `startDate` and `endDate` field, with condition `startDate` < `endDate`.
+The easiest way to achieve this, is by adding a Validator to a fieldset wrapping those fields.
+
+```js preview-story
+const isInterpretable = mv => mv && !(mv instanceof Unparseable);
+
+class Interval extends Validator {
+  static get validatorName() {
+    return 'Interval';
+  }
+
+  execute({ startDate, endDate }) {
+    if (isInterpretable(startDate) && isInterpretable(endDate)) {
+      return !(startDate < endDate);
+    }
+    return false;
+  }
+
+  static async getMessage() {
+    return `The start date should be before the end date`;
+  }
+}
+export const fieldsetValidation = () => html`
+  <lion-fieldset .validators="${[new Interval()]}">
+    <lion-input-date name="startDate" label="Start date"></lion-input-date>
+    <lion-input-date name="endDate" label="End date"></lion-input-date>
+  </lion-fieldset>
+`;
+```
+
+### Validators with knowledge about context
+
+Assume we want to create password validation with a confirmation field.
+We don't want to show feedback on group level, but right beneath the fields.
+
+```js preview-story
+const isInterpretableValue = mv => mv && !(mv instanceof Unparseable);
+
+class PasswordMatch extends Validator {
+  static get validatorName() {
+    return 'PasswordsMatch';
+  }
+
+  execute(modelValue, { first, second }) {
+    if (isInterpretableValue(first.modelValue) && isInterpretableValue(second.modelValue)) {
+      return first.modelValue !== second.modelValue;
+    }
+    return false;
+  }
+}
+// TODO: use ref directive once on Lit-element 3
+const first = renderLitAsNode(html`
+  <lion-input
+    .feedbackCondition="${(type, meta) => meta.focused}"
+    type="password"
+    name="initialPassword"
+    label="New password"
+  ></lion-input>
+`);
+const second = renderLitAsNode(html`
+  <lion-input
+    .feedbackCondition="${(type, meta) => meta.focused}"
+    type="password"
+    name="confirmPassword"
+    label="Confirm password"
+  ></lion-input>
+`);
+first.validators = [
+  new PasswordMatch(
+    { first, second },
+    { getMessage: () => 'Please match with confirmation field' },
+  ),
+];
+second.validators = [
+  new PasswordMatch({ first, second }, { getMessage: () => 'Please match with initial field' }),
+];
+
+export const contextValidation = () => html` ${first}${second} `;
+```

packages/form-core/src/InteractionStateMixin.js

@@ -201,7 +201,7 @@ const InteractionStateMixinImplementation = superclass =>
 
     get _feedbackConditionMeta() {
       return {
-        // @ts-ignore
+        // @ts-ignore to fix, InteractionStateMixin needs to depend on ValidateMixin
         ...super._feedbackConditionMeta,
         submitted: this.submitted,
         touched: this.touched,

packages/form-core/src/LionField.js

@@ -99,4 +99,11 @@ export class LionField extends FormControlMixin(
       }),
     );
   }
+
+  /**
+   * @configure InteractionStateMixin, ValidateMixin
+   */
+  get _feedbackConditionMeta() {
+    return { ...super._feedbackConditionMeta, focused: this.focused };
+  }
 }

packages/form-core/src/validate/ValidateMixin.js

@@ -85,6 +85,8 @@ export const ValidateMixinImplementation = superclass =>
          * By default, just like the platform, only one message (with highest prio) is visible.
          */
         _visibleMessagesAmount: { attribute: false },
+
+        __childModelValueChanged: { attribute: false },
       };
     }
 
@@ -181,6 +183,12 @@ export const ValidateMixinImplementation = superclass =>
       this.__onValidatorUpdated = this.__onValidatorUpdated.bind(this);
       /** @protected */
       this._updateFeedbackComponent = this._updateFeedbackComponent.bind(this);
+
+      /**
+       * This will be used for FormGroups that listen for `model-value-changed` of children
+       * @private
+       */
+      this.__childModelValueChanged = false;
     }
 
     connectedCallback() {
@@ -200,6 +208,11 @@ export const ValidateMixinImplementation = superclass =>
       super.firstUpdated(changedProperties);
       this.__validateInitialized = true;
       this.validate();
+      if (this._repropagationRole !== 'child') {
+        this.addEventListener('model-value-changed', () => {
+          this.__childModelValueChanged = true;
+        });
+      }
     }
 
     /**
@@ -627,8 +640,8 @@ export const ValidateMixinImplementation = superclass =>
     }
 
     /**
-     * The default showFeedbackConditionFor condition that will be used when the
-     * showFeedbackConditionFor is not overridden.
+     * The default feedbackCondition condition that will be used when the
+     * feedbackCondition is not overridden.
      * Show the validity feedback when returning true, don't show when false
      * @param {string} type could be 'error', 'warning', 'info', 'success' or any other custom
      * Validator type
@@ -641,17 +654,17 @@ export const ValidateMixinImplementation = superclass =>
     }
 
     /**
-     * Allows super classes to add meta info for showFeedbackConditionFor
+     * Allows super classes to add meta info for feedbackCondition
      * @configurable
      */
     get _feedbackConditionMeta() {
-      return {};
+      return { modelValue: this.modelValue, el: this };
     }
 
     /**
      * Allows the end user to specify when a feedback message should be shown
      * @example
-     * showFeedbackConditionFor(type, meta, defaultCondition) {
+     * feedbackCondition(type, meta, defaultCondition) {
      *   if (type === 'info') {
      *     return return;
      *   } else if (type === 'prefilledOnly') {
@@ -668,7 +681,7 @@ export const ValidateMixinImplementation = superclass =>
      * for other types
      * @returns {boolean}
      */
-    showFeedbackConditionFor(
+    feedbackCondition(
       type,
       meta = this._feedbackConditionMeta,
       currentCondition = this._showFeedbackConditionFor.bind(this),
@@ -702,9 +715,30 @@ export const ValidateMixinImplementation = superclass =>
         // Necessary typecast because types aren't smart enough to understand that we filter out undefined
         this.showsFeedbackFor = /** @type {string[]} */ (ctor.validationTypes
           .map(type => (this._hasFeedbackVisibleFor(type) ? type : undefined))
-          .filter(_ => !!_));
+          .filter(Boolean));
         this._updateFeedbackComponent();
       }
+
+      if (changedProperties.has('__childModelValueChanged') && this.__childModelValueChanged) {
+        this.validate({ clearCurrentResult: true });
+        this.__childModelValueChanged = false;
+      }
+
+      if (changedProperties.has('validationStates')) {
+        const prevStates = /** @type {{[key: string]: object;}} */ (changedProperties.get(
+          'validationStates',
+        ));
+        if (prevStates) {
+          Object.entries(this.validationStates).forEach(([type, feedbackObj]) => {
+            if (
+              prevStates[type] &&
+              JSON.stringify(feedbackObj) !== JSON.stringify(prevStates[type])
+            ) {
+              this.dispatchEvent(new CustomEvent(`${type}StateChanged`, { detail: feedbackObj }));
+            }
+          });
+        }
+      }
     }
 
     /**
@@ -717,7 +751,7 @@ export const ValidateMixinImplementation = superclass =>
       // Necessary typecast because types aren't smart enough to understand that we filter out undefined
       const newShouldShowFeedbackFor = /** @type {string[]} */ (ctor.validationTypes
         .map(type =>
-          this.showFeedbackConditionFor(
+          this.feedbackCondition(
             type,
             this._feedbackConditionMeta,
             this._showFeedbackConditionFor.bind(this),
@@ -725,7 +759,7 @@ export const ValidateMixinImplementation = superclass =>
             ? type
             : undefined,
         )
-        .filter(_ => !!_));
+        .filter(Boolean));
 
       if (JSON.stringify(this.shouldShowFeedbackFor) !== JSON.stringify(newShouldShowFeedbackFor)) {
         this.shouldShowFeedbackFor = newShouldShowFeedbackFor;
@@ -748,7 +782,7 @@ export const ValidateMixinImplementation = superclass =>
       // Sort all validators based on the type provided.
       const res = validationResult
         .filter(v =>
-          this.showFeedbackConditionFor(
+          this.feedbackCondition(
             v.type,
             this._feedbackConditionMeta,
             this._showFeedbackConditionFor.bind(this),

packages/form-core/test-suites/ValidateMixin.suite.js

@@ -17,6 +17,11 @@ import {
   AsyncAlwaysInvalid,
   AsyncAlwaysValid,
 } from '../test-helpers/index.js';
+import '../define.js';
+
+/**
+ * @typedef {import('../').LionField} LionField
+ */
 
 /**
  * @param {{tagString?: string | null, lightDom?: string}} [customConfig]
@@ -133,6 +138,20 @@ export function runValidateMixinSuite(customConfig) {
         expect(validateSpy.callCount).to.equal(1);
       });
 
+      it('revalidates when child ".modelValue" changes', async () => {
+        const el = /** @type {ValidateElement} */ (await fixture(html`
+          <${tag}
+            ._repropagationRole="${'fieldset'}"
+            .validators=${[new AlwaysValid()]}
+            .modelValue=${'myValue'}
+          ><lion-field id="child"><input slot="input"></lion-field></${tag}>
+        `));
+        const validateSpy = sinon.spy(el, 'validate');
+        /** @type {LionField} */ (el.querySelector('#child')).modelValue = 'test';
+        await el.updateComplete;
+        expect(validateSpy.callCount).to.equal(1);
+      });
+
       it('revalidates when ".validators" changes', async () => {
         const el = /** @type {ValidateElement} */ (await fixture(html`
           <${tag}
@@ -867,7 +886,7 @@ export function runValidateMixinSuite(customConfig) {
         const el = /** @type {ValidateElement} */ (await fixture(html`
           <${tag}
           .validators="${[new Required({}, { type: 'error' })]}"
-          .showFeedbackConditionFor="${(
+          .feedbackCondition="${(
             /** @type {string} */ type,
             /** @type {object} */ meta,
             /** @type {(type: string) => any} */ defaultCondition,
@@ -927,6 +946,30 @@ export function runValidateMixinSuite(customConfig) {
           await el.updateComplete;
           expect(spy).to.have.callCount(2);
         });
+
+        it('fires "{type}StateChanged" event async when type validity changed', async () => {
+          const spy = sinon.spy();
+          const el = /** @type {ValidateElement} */ (await fixture(html`
+            <${tag}
+              .submitted=${true}
+              .validators=${[new MinLength(7)]}
+              @errorStateChanged=${spy};
+            >${lightDom}</${tag}>
+          `));
+          expect(spy).to.have.callCount(0);
+
+          el.modelValue = 'a';
+          await el.updateComplete;
+          expect(spy).to.have.callCount(1);
+
+          el.modelValue = 'abc';
+          await el.updateComplete;
+          expect(spy).to.have.callCount(1);
+
+          el.modelValue = 'abcdefg';
+          await el.updateComplete;
+          expect(spy).to.have.callCount(2);
+        });
       });
     });
 

packages/form-core/test-suites/form-group/FormGroupMixin.suite.js

@@ -83,18 +83,18 @@ export function runFormGroupMixinSuite(cfg = {}) {
       const el = /**  @type {FormGroup} */ (await fixture(html`
       <${tag} label="foo" .fieldName="${'bar'}">${inputSlots}</${tag}>
     `));
-      // @ts-ignore [allow-proteced] in test
+      // @ts-ignore [allow-protected] in test
       expect(el.__fieldName).to.equal(el.fieldName);
     });
 
     // TODO: Tests below belong to FormRegistrarMixin. Preferably run suite integration test
     it(`${tagString} has an up to date list of every form element in .formElements`, async () => {
       const el = /**  @type {FormGroup} */ (await fixture(html`<${tag}>${inputSlots}</${tag}>`));
-      // @ts-ignore [allow-proteced] in test
+      // @ts-ignore [allow-protected] in test
       expect(el.formElements._keys().length).to.equal(3);
       expect(el.formElements['hobbies[]'].length).to.equal(2);
       el.removeChild(el.formElements['hobbies[]'][0]);
-      // @ts-ignore [allow-proteced] in test
+      // @ts-ignore [allow-protected] in test
       expect(el.formElements._keys().length).to.equal(3);
       expect(el.formElements['hobbies[]'].length).to.equal(1);
     });

packages/form-core/test/lion-field.test.js

@@ -85,7 +85,7 @@ describe('<lion-field>', () => {
     const el = /** @type {LionField} */ (await fixture(
       html`<${tag} label="foo" .fieldName="${'bar'}">${inputSlot}</${tag}>`,
     ));
-    // @ts-ignore [allow-proteced] in test
+    // @ts-ignore [allow-protected] in test
     expect(el.__fieldName).to.equal(el.fieldName);
   });
 

packages/form-core/types/NativeTextFieldMixinTypes.d.ts

@@ -1,16 +1,9 @@
 import { Constructor } from '@open-wc/dedupe-mixin';
-// import { LionField } from '@lion/form-core/src/LionField';
 import { LitElement } from '@lion/core';
 import { FocusHost } from '@lion/form-core/types/FocusMixinTypes';
 import { FormControlHost } from '@lion/form-core/types/FormControlMixinTypes';
 
-// export declare class NativeTextField extends LionField {
-//   protected get _inputNode(): HTMLTextAreaElement | HTMLInputElement;
-// }
-
 export declare class NativeTextFieldHost {
-  // protected get _inputNode(): HTMLTextAreaElement | HTMLInputElement;
-
   get selectionStart(): number;
   set selectionStart(value: number);
   get selectionEnd(): number;
@@ -26,7 +19,5 @@ export declare function NativeTextFieldImplementation<T extends Constructor<LitE
   Pick<typeof FocusHost, keyof typeof FocusHost> &
   Constructor<FormControlHost> &
   Pick<typeof FormControlHost, keyof typeof FormControlHost>;
-// &
-// Pick<typeof NativeTextField, keyof typeof NativeTextField>;
 
 export type NativeTextFieldMixin = typeof NativeTextFieldImplementation;

packages/form-core/types/form-group/FormGroupMixinTypes.d.ts

@@ -24,8 +24,8 @@ export declare class FormGroupHost {
   protected _initialModelValue: { [x: string]: any };
   protected get _inputNode(): HTMLElement;
   protected static _addDescriptionElementIdsToField(): void;
-
   protected _setValueForAllFormElements(property: string, value: any): void;
+
   private __descriptionElementsInParentChain: Set<HTMLElement>;
 }
 

packages/form-core/types/utils/SyncUpdatableMixinTypes.d.ts

@@ -1,6 +1,5 @@
 import { LitElement } from '@lion/core';
 import { Constructor } from '@open-wc/dedupe-mixin';
-import { PropertyValues } from '@lion/core';
 
 export declare interface SyncUpdatableNamespace {
   connected?: boolean;

packages/form-core/types/validate/ValidateMixinTypes.d.ts

@@ -36,6 +36,7 @@ export declare class ValidateHost {
 
   protected _visibleMessagesAmount: number;
   protected _allValidators: Validator[];
+  protected get _feedbackConditionMeta(): object;
   protected get _feedbackNode(): LionValidationFeedback;
 
   protected _updateFeedbackComponent(): void;
@@ -50,6 +51,7 @@ export declare class ValidateHost {
   private __validationResult: Validator[];
   private __prevValidationResult: Validator[];
   private __prevShownValidationResult: Validator[];
+  private __childModelValueChanged: boolean;
 
   private __storePrevResult(): void;
   private __executeValidators(): void;

packages/listbox/src/LionListbox.js

@@ -10,4 +10,11 @@ import { ListboxMixin } from './ListboxMixin.js';
  */
 export class LionListbox extends ListboxMixin(
   FocusMixin(InteractionStateMixin(ValidateMixin(LitElement))),
-) {}
+) {
+  /**
+   * @configure InteractionStateMixin, ValidateMixin
+   */
+  get _feedbackConditionMeta() {
+    return { ...super._feedbackConditionMeta, focused: this.focused };
+  }
+}

packages/listbox/test-suites/ListboxMixin.suite.js

@@ -277,7 +277,7 @@ export function runListboxMixinSuite(customConfig = {}) {
         const el = await fixture(html`
         <${tag} label="foo" .fieldName="${'bar'}"></${tag}>
       `);
-        // @ts-ignore [allow-proteced] in test
+        // @ts-ignore [allow-protected] in test
         expect(el.__fieldName).to.equal(el.fieldName);
       });
 

packages/switch/test/lion-switch.test.js

@@ -182,7 +182,7 @@ describe('lion-switch', () => {
     const el = await fixture(
       html`<custom-switch
         .validators="${[new IsTrue({}, { type: 'info' })]}"
-        .showFeedbackConditionFor="${(
+        .feedbackCondition="${(
           /** @type {string} */ type,
           /** @type {object} */ meta,
           /** @type {(type: string, meta: object) => any} */ defaultCondition,