From 355aabc02caac52022bec4a770709cb265ec9b39 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 6 Apr 2022 16:10:23 +0200 Subject: [PATCH 1/3] fix(switch): unregister as formElement on disconnected --- .changeset/six-camels-cross.md | 5 +++++ packages/switch/src/LionSwitch.js | 1 + 2 files changed, 6 insertions(+) create mode 100644 .changeset/six-camels-cross.md diff --git a/.changeset/six-camels-cross.md b/.changeset/six-camels-cross.md new file mode 100644 index 000000000..2248369de --- /dev/null +++ b/.changeset/six-camels-cross.md @@ -0,0 +1,5 @@ +--- +'@lion/switch': patch +--- + +fix(switch) unregister on disconnectedCallback diff --git a/packages/switch/src/LionSwitch.js b/packages/switch/src/LionSwitch.js index bf750831c..d38ee116a 100644 --- a/packages/switch/src/LionSwitch.js +++ b/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); } From d9d88b7f5515bb1a79e6a756c5736eb4efef5194 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 6 Apr 2022 18:12:47 +0200 Subject: [PATCH 2/3] chore(form-core): make testing form registration easier --- packages/form-core/package.json | 1 + .../form-core/src/registration/FormRegisteringMixin.js | 8 ++++++++ .../types/registration/FormRegisteringMixinTypes.d.ts | 2 ++ 3 files changed, 11 insertions(+) diff --git a/packages/form-core/package.json b/packages/form-core/package.json index 11ff1d772..9e13a810a 100644 --- a/packages/form-core/package.json +++ b/packages/form-core/package.json @@ -20,6 +20,7 @@ "src", "test", "test-helpers", + "test-suites", "translations", "types" ], diff --git a/packages/form-core/src/registration/FormRegisteringMixin.js b/packages/form-core/src/registration/FormRegisteringMixin.js index 73f592116..bdea2910b 100644 --- a/packages/form-core/src/registration/FormRegisteringMixin.js +++ b/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)); } diff --git a/packages/form-core/types/registration/FormRegisteringMixinTypes.d.ts b/packages/form-core/types/registration/FormRegisteringMixinTypes.d.ts index 5f36fd7ef..f369c1a4f 100644 --- a/packages/form-core/types/registration/FormRegisteringMixinTypes.d.ts +++ b/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>( From 24a58cdf576a2a59e0f4aedc0c61f526b8b8b684 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 6 Apr 2022 18:20:19 +0200 Subject: [PATCH 3/3] chore: add input-tel(-dropdown) to umbrella-form + test unregister Co-authored-by: Konstantinos Norgias --- .../systems/overlays/assets/umbrella-form.js | 7 ++++ .../test/dialog-integrations.test.js | 13 ++---- .../test/form-group-methods.test.js | 1 + .../test/form-integrations.test.js | 41 +++++++++++++++---- .../test/helpers/umbrella-form.js | 5 +++ 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/docs/fundamentals/systems/overlays/assets/umbrella-form.js b/docs/fundamentals/systems/overlays/assets/umbrella-form.js index c604b00b1..28d0d257e 100644 --- a/docs/fundamentals/systems/overlays/assets/umbrella-form.js +++ b/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 { + + { 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 - // [4] more of 'the rest' (chronologically ordered registrations) + 'lion-checkbox-group', + ' lion-checkbox', + 'lion-switch', 'lion-input-stepper', 'lion-textarea', ]); diff --git a/packages/form-integrations/test/form-group-methods.test.js b/packages/form-integrations/test/form-group-methods.test.js index 8b42c5643..ba2fb6a92 100644 --- a/packages/form-integrations/test/form-group-methods.test.js +++ b/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: '', }); diff --git a/packages/form-integrations/test/form-integrations.test.js b/packages/form-integrations/test/form-integrations.test.js index de94eb44b..d31585f71 100644 --- a/packages/form-integrations/test/form-integrations.test.js +++ b/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 () => { - const el = /** @type {UmbrellaForm} */ await fixture(html``); - // @ts-ignore - const formEl = /** @type {LionForm} */ (el._lionFormNode); - await formEl.registrationComplete; - const registeredEls = getAllTagNames(formEl); - - expect(registeredEls).to.eql([ + describe('Registering', () => { + const registerTagsResult = [ '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``); + // @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``); + 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; + } + }); }); }); diff --git a/packages/form-integrations/test/helpers/umbrella-form.js b/packages/form-integrations/test/helpers/umbrella-form.js index d09903c3d..76db5eca5 100644 --- a/packages/form-integrations/test/helpers/umbrella-form.js +++ b/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 { +