Merge pull request #1658 from ing-bank/fix/switchUnregister

Fix/switch unregister

.changeset/six-camels-cross.md

@@ -0,0 +1,5 @@
+---
+'@lion/switch': patch
+---
+
+fix(switch) unregister on disconnectedCallback

docs/fundamentals/systems/overlays/assets/umbrella-form.js

@@ -15,6 +15,8 @@ import '@lion/select-rich/define';
 import '@lion/input-range/define';
 import '@lion/textarea/define';
 import '@lion/button/define';
+import '@lion/input-tel/define';
+import '@lion/input-tel-dropdown/define';
 
 export class UmbrellaForm extends LitElement {
   get _lionFormNode() {
@@ -60,6 +62,11 @@ export class UmbrellaForm extends LitElement {
           <lion-input-amount name="money" label="Money"></lion-input-amount>
           <lion-input-iban name="iban" label="Iban"></lion-input-iban>
           <lion-input-email name="email" label="Email"></lion-input-email>
+          <lion-input-tel name="tel" label="Telephone Number"></lion-input-tel>
+          <lion-input-tel-dropdown
+            name="tel-dropdown"
+            label="Telephone Number with dropdown list"
+          ></lion-input-tel-dropdown>
           <lion-checkbox-group
             label="What do you like?"
             name="checkers"

packages/form-core/package.json

@@ -20,6 +20,7 @@
     "src",
     "test",
     "test-helpers",
+    "test-suites",
     "translations",
     "types"
   ],

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

@@ -42,6 +42,14 @@ const FormRegisteringMixinImplementation = superclass =>
 
     disconnectedCallback() {
       super.disconnectedCallback();
+      this.__unregisterFormElement();
+    }
+
+    /**
+     * Putting this in a separate method makes testing easier
+     * @private
+     */
+    __unregisterFormElement() {
       if (this._parentFormGroup) {
         this._parentFormGroup.removeFormElement(/** @type {* & FormRegisteringHost} */ (this));
       }

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

@@ -14,6 +14,8 @@ export declare class FormRegisteringHost {
    * ChoiceGroup
    */
   protected _parentFormGroup: FormRegistrarHost | undefined;
+
+  private __unregisterFormElement: void;
 }
 
 export declare function FormRegisteringImplementation<T extends Constructor<LitElement>>(

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

@@ -24,13 +24,6 @@ describe('Form inside dialog Integrations', () => {
     const registeredEls = getAllTagNames(formEl);
 
     expect(registeredEls).to.eql([
-      // [1] In a dialog, these are registered before the rest (which is in chronological dom order)
-      // Ideally, this should be the same. It would be once the platform allows to create dialogs
-      // that don't need to move content to the body
-      'lion-checkbox-group',
-      '  lion-checkbox',
-      'lion-switch',
-      // [2] 'the rest' (chronologically ordered registrations)
       'lion-fieldset',
       '  lion-input',
       '  lion-input',
@@ -41,6 +34,7 @@ describe('Form inside dialog Integrations', () => {
       'lion-input-iban',
       'lion-input-email',
       'lion-input-tel',
+      'lion-input-tel-dropdown',
       'lion-checkbox-group',
       '  lion-checkbox',
       '  lion-checkbox',
@@ -66,8 +60,9 @@ describe('Form inside dialog Integrations', () => {
       '  lion-option',
       'lion-select',
       'lion-input-range',
-      // [3] this is where [1] should have been inserted
+      'lion-checkbox-group',
-      // [4] more of 'the rest' (chronologically ordered registrations)
+      '  lion-checkbox',
+      'lion-switch',
       'lion-input-stepper',
       'lion-textarea',
     ]);

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

@@ -160,6 +160,7 @@ describe(`Submitting/Resetting/Clearing Form`, async () => {
       range: '',
       rsvp: '',
       tel: '',
+      'tel-dropdown': '',
       terms: [],
       comments: '',
     });

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

@@ -1,3 +1,4 @@
+import sinon from 'sinon';
 import { expect, fixture } from '@open-wc/testing';
 import { html } from 'lit/static-html.js';
 import { getAllTagNames } from './helpers/helpers.js';
@@ -33,6 +34,7 @@ describe('Form Integrations', () => {
       notifications: { value: '', checked: false },
       rsvp: '',
       tel: '',
+      'tel-dropdown': '',
       comments: '',
     });
   });
@@ -61,6 +63,7 @@ describe('Form Integrations', () => {
       notifications: '',
       rsvp: '',
       tel: '',
+      'tel-dropdown': '',
       comments: '',
     });
   });
@@ -97,14 +100,8 @@ describe('Form Integrations', () => {
     });
   });
 
-  it('Successfully registers all form components', async () => {
+  describe('Registering', () => {
-    const el = /** @type {UmbrellaForm} */ await fixture(html`<umbrella-form></umbrella-form>`);
+    const registerTagsResult = [
-    // @ts-ignore
-    const formEl = /** @type {LionForm} */ (el._lionFormNode);
-    await formEl.registrationComplete;
-    const registeredEls = getAllTagNames(formEl);
-
-    expect(registeredEls).to.eql([
       'lion-fieldset',
       '  lion-input',
       '  lion-input',
@@ -115,6 +112,7 @@ describe('Form Integrations', () => {
       'lion-input-iban',
       'lion-input-email',
       'lion-input-tel',
+      'lion-input-tel-dropdown',
       'lion-checkbox-group',
       '  lion-checkbox',
       '  lion-checkbox',
@@ -145,6 +143,31 @@ describe('Form Integrations', () => {
       'lion-switch',
       'lion-input-stepper',
       'lion-textarea',
-    ]);
+    ];
+
+    it('successfully registers all form components', async () => {
+      const el = /** @type {UmbrellaForm} */ await fixture(html`<umbrella-form></umbrella-form>`);
+      // @ts-ignore
+      const formEl = /** @type {LionForm} */ (el._lionFormNode);
+      await formEl.registrationComplete;
+      const registeredEls = getAllTagNames(formEl);
+
+      expect(registeredEls).to.eql(registerTagsResult);
+    });
+
+    it('successfully unregisters all form components', async () => {
+      const el = /** @type {UmbrellaForm} */ await fixture(html`<umbrella-form></umbrella-form>`);
+      const offlineContainer = document.createElement('div');
+      // @ts-ignore
+      const formEl = /** @type {LionForm} */ (el._lionFormNode);
+      await formEl.registrationComplete;
+
+      for (const child of formEl.formElements) {
+        const spy = sinon.spy(child, '__unregisterFormElement');
+        offlineContainer.appendChild(child);
+        await child.updateComplete;
+        expect(spy).to.have.been.called;
+      }
+    });
   });
 });

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

@@ -9,6 +9,7 @@ import '@lion/input-amount/define';
 import '@lion/input-iban/define';
 import '@lion/input-email/define';
 import '@lion/input-tel/define';
+import '@lion/input-tel-dropdown/define';
 import '@lion/checkbox-group/define';
 import '@lion/radio-group/define';
 import '@lion/select/define';
@@ -73,6 +74,10 @@ export class UmbrellaForm extends LitElement {
           <lion-input-iban name="iban" label="Iban"></lion-input-iban>
           <lion-input-email name="email" label="Email"></lion-input-email>
           <lion-input-tel name="tel" label="Telephone Number"></lion-input-tel>
+          <lion-input-tel-dropdown
+            name="tel-dropdown"
+            label="Telephone Number with dropdown list"
+          ></lion-input-tel-dropdown>
           <lion-checkbox-group
             label="What do you like?"
             name="checkers"

packages/switch/src/LionSwitch.js

@@ -88,6 +88,7 @@ export class LionSwitch extends ScopedElementsMixin(ChoiceInputMixin(LionField))
   }
 
   disconnectedCallback() {
+    super.disconnectedCallback();
     if (this._inputNode) {
       this.removeEventListener('checked-changed', this.__handleButtonSwitchCheckedChanged);
     }