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 9e26138a9..9b8774137 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 */ @@ -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/test-suites/form-group/FormGroupMixin.suite.js b/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js index c3cac196b..eaff0141e 100644 --- a/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/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} name="input2"> + + `)); + expect(el.dirty).to.be.false; + + const el2 = /** @type {FormGroup} */ (await fixture(html` + <${tag} .modelValue="${{ input1: 'x', input2: 'y' }}"> + <${childTag} name="input1" > + <${childTag} name="input2"> + + `)); + expect(el2.dirty).to.be.false; + }); }); // TODO: this should be tested in FormGroupMixin 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 a9b024778..2780cdb21 100644 --- a/packages/form-integrations/test/form-integrations.test.js +++ b/packages/form-integrations/test/form-integrations.test.js @@ -11,24 +11,24 @@ describe('Form Integrations', () => { const el = /** @type {UmbrellaForm} */ (await fixture(html``)); await el.updateComplete; const formEl = el._lionFormNode; + expect(formEl.serializedValue).to.eql({ - bio: '', - 'checkers[]': [['foo', 'bar']], - comments: '', + full_name: { first_name: '', last_name: '' }, date: '2000-12-12', datepicker: '2020-12-12', - dinosaurs: 'brontosaurus', - email: '', - favoriteColor: 'hotpink', - full_name: { - first_name: '', - last_name: '', - }, - iban: '', - lyrics: '1', + bio: '', 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: '', - 'checkers[]': [['foo', 'bar']], - comments: '', + full_name: { first_name: '', last_name: '' }, date: '12/12/2000', datepicker: '12/12/2020', - dinosaurs: 'brontosaurus', - email: '', - favoriteColor: 'hotpink', - full_name: { - first_name: '', - last_name: '', - }, - iban: '', - lyrics: '1', + bio: '', 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``, + )); + + 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 fb60807ff..5efc3c712 100644 --- a/packages/form-integrations/test/helpers/umbrella-form.js +++ b/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` - +
@@ -75,15 +84,31 @@ export class UmbrellaForm extends LitElement { .validators="${[new Required()]}" > - + + + Apple + Banana + Mango + + + Rocky + Rocky II + Rocky III + Rocky IV + Rocky V + Rocky Balboa + - - Red - Hotpink - Teal - + Red + Hotpink + Teal