Merge pull request #1313 from ing-bank/chore/testInitialDirtyState

chore(form-core): fix prefilled FormGroups

.changeset/cold-spies-press.md

@@ -0,0 +1,8 @@
+---
+'@lion/form-core': patch
+'@lion/form-integrations': patch
+---
+
+## Bug fixes
+
+**form-core**: registrationComplete callback executed before initial interaction states are computed

packages/form-core/src/choice-group/ChoiceGroupMixin.js

@@ -11,6 +11,12 @@ import { InteractionStateMixin } from '../InteractionStateMixin.js';
  */
 
 /**
+ * ChoiceGroupMixin applies on both Fields (listbox/select-rich/combobox)  and FormGroups
+ * (radio-group, checkbox-group)
+ * TODO: Ideally, the ChoiceGroupMixin should not depend on InteractionStateMixin, which is only
+ * designed for usage with Fields, in other words: their interaction states are not derived from
+ * children events, like in FormGroups
+ *
  * @type {ChoiceGroupMixin}
  * @param {import('@open-wc/dedupe-mixin').Constructor<import('@lion/core').LitElement>} superclass
  */
@@ -138,34 +144,10 @@ const ChoiceGroupMixinImplementation = superclass =>
       this.__isInitialSerializedValue = true;
       /** @private */
       this.__isInitialFormattedValue = true;
-      /** @type {Promise<any> & {done?:boolean}} */
-      this.registrationComplete = new Promise((resolve, reject) => {
-        /** @private */
-        this.__resolveRegistrationComplete = resolve;
-        /** @private */
-        this.__rejectRegistrationComplete = reject;
-      });
-      this.registrationComplete.done = false;
-      this.registrationComplete.then(
-        () => {
-          this.registrationComplete.done = true;
-        },
-        () => {
-          this.registrationComplete.done = true;
-          throw new Error(
-            'Registration could not finish. Please use await el.registrationComplete;',
-          );
-        },
-      );
     }
 
     connectedCallback() {
       super.connectedCallback();
-      // Double microtask queue to account for Webkit race condition
-      Promise.resolve().then(() =>
-        // @ts-ignore
-        Promise.resolve().then(() => this.__resolveRegistrationComplete()),
-      );
 
       this.registrationComplete.then(() => {
         this.__isInitialModelValue = false;
@@ -174,6 +156,14 @@ const ChoiceGroupMixinImplementation = superclass =>
       });
     }
 
+    /**
+     * @enhance FormRegistrarMixin
+     */
+    _completeRegistration() {
+      // Double microtask queue to account for Webkit race condition
+      Promise.resolve().then(() => super._completeRegistration());
+    }
+
     /** @param {import('@lion/core').PropertyValues} changedProperties */
     updated(changedProperties) {
       super.updated(changedProperties);
@@ -185,18 +175,6 @@ const ChoiceGroupMixinImplementation = superclass =>
       }
     }
 
-    disconnectedCallback() {
-      super.disconnectedCallback();
-
-      if (this.registrationComplete.done === false) {
-        Promise.resolve().then(() => {
-          Promise.resolve().then(() => {
-            this.__rejectRegistrationComplete();
-          });
-        });
-      }
-    }
-
     /**
      * @override from FormRegistrarMixin
      * @param {FormControl} child

packages/form-core/src/form-group/FormGroupMixin.js

@@ -146,32 +146,13 @@ const FormGroupMixinImplementation = superclass =>
       this.addEventListener('validate-performed', this.__onChildValidatePerformed);
 
       this.defaultValidators = [new FormElementsHaveNoError()];
-      /** @type {Promise<any> & {done?:boolean}} */
-      this.registrationComplete = new Promise((resolve, reject) => {
-        this.__resolveRegistrationComplete = resolve;
-        this.__rejectRegistrationComplete = reject;
-      });
-      this.registrationComplete.done = false;
-      this.registrationComplete.then(
-        () => {
-          this.registrationComplete.done = true;
-        },
-        () => {
-          this.registrationComplete.done = true;
-          throw new Error(
-            'Registration could not finish. Please use await el.registrationComplete;',
-          );
-        },
-      );
     }
 
     connectedCallback() {
       super.connectedCallback();
       this.setAttribute('role', 'group');
-      // @ts-ignore
-      Promise.resolve().then(() => this.__resolveRegistrationComplete());
 
-      this.registrationComplete.then(() => {
+      this.initComplete.then(() => {
         this.__isInitialModelValue = false;
         this.__isInitialSerializedValue = false;
         this.__initInteractionStates();
@@ -185,11 +166,6 @@ const FormGroupMixinImplementation = superclass =>
         document.removeEventListener('click', this._checkForOutsideClick);
         this.__hasActiveOutsideClickHandling = false;
       }
-      if (this.registrationComplete.done === false) {
-        Promise.resolve().then(() => {
-          this.__rejectRegistrationComplete();
-        });
-      }
     }
 
     __initInteractionStates() {

packages/form-core/src/registration/FormRegistrarMixin.js

@@ -60,6 +60,61 @@ const FormRegistrarMixinImplementation = superclass =>
         'form-element-name-changed',
         /** @type {EventListenerOrEventListenerObject} */ (this._onRequestToChangeFormElementName),
       );
+
+      /**
+       * initComplete resolves after all pending initialization logic
+       * (for instance `<form-group .serializedValue=${{ child1: 'a', child2: 'b' }}>`)
+       * is executed
+       * @type {Promise<any>}
+       */
+      this.initComplete = new Promise((resolve, reject) => {
+        this.__resolveInitComplete = resolve;
+        this.__rejectInitComplete = reject;
+      });
+
+      /**
+       * registrationComplete waits for all children formElements to have registered
+       * @type {Promise<any> & {done?:boolean}}
+       */
+      this.registrationComplete = new Promise((resolve, reject) => {
+        this.__resolveRegistrationComplete = resolve;
+        this.__rejectRegistrationComplete = reject;
+      });
+      this.registrationComplete.done = false;
+      this.registrationComplete.then(
+        () => {
+          this.registrationComplete.done = true;
+          this.__resolveInitComplete(undefined);
+        },
+        () => {
+          this.registrationComplete.done = true;
+          this.__rejectInitComplete(undefined);
+          throw new Error(
+            'Registration could not finish. Please use await el.registrationComplete;',
+          );
+        },
+      );
+    }
+
+    connectedCallback() {
+      super.connectedCallback();
+      this._completeRegistration();
+    }
+
+    _completeRegistration() {
+      Promise.resolve().then(() => this.__resolveRegistrationComplete(undefined));
+    }
+
+    disconnectedCallback() {
+      super.disconnectedCallback();
+
+      if (this.registrationComplete.done === false) {
+        Promise.resolve().then(() => {
+          Promise.resolve().then(() => {
+            this.__rejectRegistrationComplete();
+          });
+        });
+      }
     }
 
     /**

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

@@ -670,6 +670,24 @@ export function runFormGroupMixinSuite(cfg = {}) {
         expect(el.validationStates.error.Input1IsTen).to.be.true;
         expect(el.hasFeedbackFor).to.deep.equal(['error']);
       });
+
+      it('does not become dirty when elements are prefilled', async () => {
+        const el = /**  @type {FormGroup} */ (await fixture(html`
+        <${tag} .serializedValue="${{ input1: 'x', input2: 'y' }}">
+          <${childTag} name="input1" ></${childTag}>
+          <${childTag} name="input2"></${childTag}>
+        </${tag}>
+      `));
+        expect(el.dirty).to.be.false;
+
+        const el2 = /**  @type {FormGroup} */ (await fixture(html`
+        <${tag} .modelValue="${{ input1: 'x', input2: 'y' }}">
+          <${childTag} name="input1" ></${childTag}>
+          <${childTag} name="input2"></${childTag}>
+        </${tag}>
+      `));
+        expect(el2.dirty).to.be.false;
+      });
     });
 
     // TODO: this should be tested in FormGroupMixin

packages/form-core/types/registration/FormRegistrarMixinTypes.d.ts

@@ -22,6 +22,8 @@ export declare class FormRegistrarHost {
   _onRequestToAddFormElement(e: CustomEvent): void;
   isRegisteredFormElement(el: FormControlHost): boolean;
   registrationComplete: Promise<boolean>;
+  initComplete: Promise<boolean>;
+  protected _completeRegistration(): void;
 }
 
 export declare function FormRegistrarImplementation<T extends Constructor<LitElement>>(

packages/form-integrations/test/form-integrations.test.js

@@ -11,24 +11,24 @@ describe('Form Integrations', () => {
     const el = /** @type {UmbrellaForm} */ (await fixture(html`<umbrella-form></umbrella-form>`));
     await el.updateComplete;
     const formEl = el._lionFormNode;
+
     expect(formEl.serializedValue).to.eql({
-      bio: '',
+      full_name: { first_name: '', last_name: '' },
-      'checkers[]': [['foo', 'bar']],
-      comments: '',
       date: '2000-12-12',
       datepicker: '2020-12-12',
-      dinosaurs: 'brontosaurus',
+      bio: '',
-      email: '',
-      favoriteColor: 'hotpink',
-      full_name: {
-        first_name: '',
-        last_name: '',
-      },
-      iban: '',
-      lyrics: '1',
       money: '',
+      iban: '',
+      email: '',
+      checkers: ['foo', 'bar'],
+      dinosaurs: '',
+      favoriteFruit: 'Banana',
+      favoriteMovie: 'Rocky',
+      favoriteColor: 'hotpink',
+      lyrics: '1',
       range: 2.3,
-      'terms[]': [[]],
+      terms: [],
+      comments: '',
     });
   });
 
@@ -37,23 +37,52 @@ describe('Form Integrations', () => {
     await el.updateComplete;
     const formEl = el._lionFormNode;
     expect(formEl.formattedValue).to.eql({
-      bio: '',
+      full_name: { first_name: '', last_name: '' },
-      'checkers[]': [['foo', 'bar']],
-      comments: '',
       date: '12/12/2000',
       datepicker: '12/12/2020',
-      dinosaurs: 'brontosaurus',
+      bio: '',
-      email: '',
-      favoriteColor: 'hotpink',
-      full_name: {
-        first_name: '',
-        last_name: '',
-      },
-      iban: '',
-      lyrics: '1',
       money: '',
+      iban: '',
+      email: '',
+      checkers: ['foo', 'bar'],
+      dinosaurs: '',
+      favoriteFruit: 'Banana',
+      favoriteMovie: 'Rocky',
+      favoriteColor: 'hotpink',
+      lyrics: '1',
       range: 2.3,
-      'terms[]': [[]],
+      terms: [],
+      comments: '',
+    });
+  });
+
+  describe('Form Integrations', () => {
+    it('does not become dirty when elements are prefilled', async () => {
+      const el = /** @type {UmbrellaForm} */ (await fixture(
+        html`<umbrella-form
+          .serializedValue="${{
+            full_name: { first_name: '', last_name: '' },
+            date: '2000-12-12',
+            datepicker: '2020-12-12',
+            bio: '',
+            money: '',
+            iban: '',
+            email: '',
+            checkers: ['foo', 'bar'],
+            dinosaurs: 'brontosaurus',
+            favoriteFruit: 'Banana',
+            favoriteMovie: 'Rocky',
+            favoriteColor: 'hotpink',
+            lyrics: '1',
+            range: 2.3,
+            terms: [],
+            comments: '',
+          }}"
+        ></umbrella-form>`,
+      ));
+
+      await el._lionFormNode.initComplete;
+      expect(el._lionFormNode.dirty).to.be.false;
     });
   });
 });

packages/form-integrations/test/helpers/umbrella-form.js

@@ -12,6 +12,8 @@ import '@lion/checkbox-group/define';
 import '@lion/radio-group/define';
 import '@lion/select/define';
 import '@lion/select-rich/define';
+import '@lion/listbox/define';
+import '@lion/combobox/define';
 import '@lion/input-range/define';
 import '@lion/textarea/define';
 import '@lion/button/define';
@@ -23,9 +25,16 @@ export class UmbrellaForm extends LitElement {
     ));
   }
 
+  /**
+   * @param {string} v
+   */
+  set serializedValue(v) {
+    this.__serializedValue = v;
+  }
+
   render() {
     return html`
-      <lion-form>
+      <lion-form .serializedValue="${this.__serializedValue}">
         <form>
           <lion-fieldset name="full_name">
             <lion-input
@@ -42,13 +51,13 @@ export class UmbrellaForm extends LitElement {
           <lion-input-date
             name="date"
             label="Date of application"
-            .modelValue="${new Date('2000/12/12')}"
+            .modelValue="${new Date('2000-12-12')}"
             .validators="${[new Required()]}"
           ></lion-input-date>
           <lion-input-datepicker
             name="datepicker"
             label="Date to be picked"
-            .modelValue="${new Date('2020/12/12')}"
+            .modelValue="${new Date('2020-12-12')}"
             .validators="${[new Required()]}"
           ></lion-input-datepicker>
           <lion-textarea
@@ -62,7 +71,7 @@ export class UmbrellaForm extends LitElement {
           <lion-input-email name="email" label="Email"></lion-input-email>
           <lion-checkbox-group
             label="What do you like?"
-            name="checkers[]"
+            name="checkers"
             .validators="${[new Required()]}"
           >
             <lion-checkbox .choiceValue=${'foo'} checked label="I like foo"></lion-checkbox>
@@ -75,15 +84,31 @@ export class UmbrellaForm extends LitElement {
             .validators="${[new Required()]}"
           >
             <lion-radio .choiceValue=${'allosaurus'} label="allosaurus"></lion-radio>
-            <lion-radio .choiceValue=${'brontosaurus'} checked label="brontosaurus"></lion-radio>
+            <lion-radio .choiceValue=${'brontosaurus'} label="brontosaurus"></lion-radio>
             <lion-radio .choiceValue=${'diplodocus'} label="diplodocus"></lion-radio>
           </lion-radio-group>
+          <lion-listbox name="favoriteFruit" label="Favorite fruit">
+            <lion-option .choiceValue=${'Apple'}>Apple</lion-option>
+            <lion-option checked .choiceValue=${'Banana'}>Banana</lion-option>
+            <lion-option .choiceValue=${'Mango'}>Mango</lion-option>
+          </lion-listbox>
+          <lion-combobox
+            .validators="${[new Required()]}"
+            name="favoriteMovie"
+            label="Favorite movie"
+            autocomplete="both"
+          >
+            <lion-option checked .choiceValue=${'Rocky'}>Rocky</lion-option>
+            <lion-option .choiceValue=${'Rocky II'}>Rocky II</lion-option>
+            <lion-option .choiceValue=${'Rocky III'}>Rocky III</lion-option>
+            <lion-option .choiceValue=${'Rocky IV'}>Rocky IV</lion-option>
+            <lion-option .choiceValue=${'Rocky V'}>Rocky V</lion-option>
+            <lion-option .choiceValue=${'Rocky Balboa'}>Rocky Balboa</lion-option>
+          </lion-combobox>
           <lion-select-rich name="favoriteColor" label="Favorite color">
-            <lion-options slot="input">
+            <lion-option .choiceValue=${'red'}>Red</lion-option>
-              <lion-option .choiceValue=${'red'}>Red</lion-option>
+            <lion-option .choiceValue=${'hotpink'} checked>Hotpink</lion-option>
-              <lion-option .choiceValue=${'hotpink'} checked>Hotpink</lion-option>
+            <lion-option .choiceValue=${'teal'}>Teal</lion-option>
-              <lion-option .choiceValue=${'teal'}>Teal</lion-option>
-            </lion-options>
           </lion-select-rich>
           <lion-select label="Lyrics" name="lyrics" .validators="${[new Required()]}">
             <select slot="input">
@@ -100,21 +125,28 @@ export class UmbrellaForm extends LitElement {
             unit="%"
             step="0.1"
             label="Input range"
+          ></lion-input-range>
+          <lion-checkbox-group
+            .mulipleChoice="${false}"
+            name="terms"
+            .validators="${[new Required()]}"
           >
-          </lion-input-range>
-          <lion-checkbox-group name="terms[]" .validators="${[new Required()]}">
             <lion-checkbox label="I blindly accept all terms and conditions"></lion-checkbox>
           </lion-checkbox-group>
+          <lion-switch name="notifications" label="Notifications"></lion-switch>
+          <lion-input-stepper max="5" min="0" name="rsvp">
+            <label slot="label">RSVP</label>
+            <div slot="help-text">Max. 5 guests</div>
+          </lion-input-stepper>
           <lion-textarea name="comments" label="Comments"></lion-textarea>
           <div class="buttons">
             <lion-button raised>Submit</lion-button>
             <lion-button
               type="button"
               raised
-              @click=${() => {
+              @click=${(/** @type {Event} */ ev) =>
-                const lionForm = this._lionFormNode;
+                // @ts-ignore
-                lionForm.resetGroup();
+                ev.currentTarget.parentElement.parentElement.parentElement.resetGroup()}
-              }}
               >Reset</lion-button
             >
           </div>