diff --git a/.changeset/cold-spies-press.md b/.changeset/cold-spies-press.md new file mode 100644 index 000000000..eee51999c --- /dev/null +++ b/.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 diff --git a/packages/form-core/src/choice-group/ChoiceGroupMixin.js b/packages/form-core/src/choice-group/ChoiceGroupMixin.js index be1ca374f..e823fc9f9 100644 --- a/packages/form-core/src/choice-group/ChoiceGroupMixin.js +++ b/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} superclass */ @@ -53,8 +59,8 @@ const ChoiceGroupMixinImplementation = superclass => }; if (this.__isInitialModelValue) { - this.__isInitialModelValue = false; this.registrationComplete.then(() => { + this.__isInitialModelValue = false; this._setCheckedElements(value, checkCondition); this.requestUpdate('modelValue', this.__oldModelValue); }); @@ -89,8 +95,8 @@ const ChoiceGroupMixinImplementation = superclass => const checkCondition = (el, val) => el.serializedValue.value === val; if (this.__isInitialSerializedValue) { - this.__isInitialSerializedValue = false; this.registrationComplete.then(() => { + this.__isInitialSerializedValue = false; this._setCheckedElements(value, checkCondition); this.requestUpdate('serializedValue'); }); @@ -116,8 +122,8 @@ const ChoiceGroupMixinImplementation = superclass => const checkCondition = (el, val) => el.formattedValue === val; if (this.__isInitialFormattedValue) { - this.__isInitialFormattedValue = false; this.registrationComplete.then(() => { + this.__isInitialFormattedValue = false; this._setCheckedElements(value, checkCondition); }); } else { @@ -138,34 +144,10 @@ const ChoiceGroupMixinImplementation = superclass => this.__isInitialSerializedValue = true; /** @private */ this.__isInitialFormattedValue = true; - /** @type {Promise & {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 diff --git a/packages/form-core/src/form-group/FormGroupMixin.js b/packages/form-core/src/form-group/FormGroupMixin.js index 7922eb1ff..69de0baa0 100644 --- a/packages/form-core/src/form-group/FormGroupMixin.js +++ b/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 & {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() { diff --git a/packages/form-core/src/registration/FormRegistrarMixin.js b/packages/form-core/src/registration/FormRegistrarMixin.js index 9ffdf4305..1e839e951 100644 --- a/packages/form-core/src/registration/FormRegistrarMixin.js +++ b/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 ``) + * is executed + * @type {Promise} + */ + this.initComplete = new Promise((resolve, reject) => { + this.__resolveInitComplete = resolve; + this.__rejectInitComplete = reject; + }); + + /** + * registrationComplete waits for all children formElements to have registered + * @type {Promise & {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(); + }); + }); + } } /** diff --git a/packages/form-core/types/registration/FormRegistrarMixinTypes.d.ts b/packages/form-core/types/registration/FormRegistrarMixinTypes.d.ts index ac2df1880..263fec667 100644 --- a/packages/form-core/types/registration/FormRegistrarMixinTypes.d.ts +++ b/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; + initComplete: Promise; + protected _completeRegistration(): void; } export declare function FormRegistrarImplementation>( diff --git a/packages/form-integrations/test/form-integrations.test.js b/packages/form-integrations/test/form-integrations.test.js index 6e29980b9..2780cdb21 100644 --- a/packages/form-integrations/test/form-integrations.test.js +++ b/packages/form-integrations/test/form-integrations.test.js @@ -81,6 +81,7 @@ describe('Form Integrations', () => { >`, )); + await el._lionFormNode.initComplete; expect(el._lionFormNode.dirty).to.be.false; }); }); diff --git a/packages/form-integrations/test/helpers/umbrella-form.js b/packages/form-integrations/test/helpers/umbrella-form.js index ccfdfee4b..5efc3c712 100644 --- a/packages/form-integrations/test/helpers/umbrella-form.js +++ b/packages/form-integrations/test/helpers/umbrella-form.js @@ -25,6 +25,9 @@ export class UmbrellaForm extends LitElement { )); } + /** + * @param {string} v + */ set serializedValue(v) { this.__serializedValue = v; } @@ -141,7 +144,8 @@ export class UmbrellaForm extends LitElement { + @click=${(/** @type {Event} */ ev) => + // @ts-ignore ev.currentTarget.parentElement.parentElement.parentElement.resetGroup()} >Reset