From 247e64a3cc1522960b3b206026008a495d83fbe3 Mon Sep 17 00:00:00 2001 From: Joren Broekema Date: Mon, 12 Oct 2020 12:26:30 +0200 Subject: [PATCH] fix(form-core): update child choice fields name attr to parent --- .changeset/sour-apes-reply.md | 5 +++ .../src/choice-group/ChoiceGroupMixin.js | 32 ++++++++----------- .../choice-group/ChoiceGroupMixin.suite.js | 32 ++++++++++++++----- .../listbox/test-suites/ListboxMixin.suite.js | 18 ----------- 4 files changed, 42 insertions(+), 45 deletions(-) create mode 100644 .changeset/sour-apes-reply.md diff --git a/.changeset/sour-apes-reply.md b/.changeset/sour-apes-reply.md new file mode 100644 index 000000000..4c624a470 --- /dev/null +++ b/.changeset/sour-apes-reply.md @@ -0,0 +1,5 @@ +--- +'@lion/form-core': patch +--- + +Ensure that the name of a child choice field is always synced with the parent choice field(set) when it changes. No longer error when a child is added with a different name than the parent, simply sync it. diff --git a/packages/form-core/src/choice-group/ChoiceGroupMixin.js b/packages/form-core/src/choice-group/ChoiceGroupMixin.js index d9839632a..0d4fe415a 100644 --- a/packages/form-core/src/choice-group/ChoiceGroupMixin.js +++ b/packages/form-core/src/choice-group/ChoiceGroupMixin.js @@ -156,6 +156,17 @@ const ChoiceGroupMixinImplementation = superclass => }); } + /** @param {import('lit-element').PropertyValues} changedProperties */ + updated(changedProperties) { + super.updated(changedProperties); + if (changedProperties.has('name') && this.name !== changedProperties.get('name')) { + this.formElements.forEach(child => { + // eslint-disable-next-line no-param-reassign + child.name = this.name; + }); + } + } + disconnectedCallback() { super.disconnectedCallback(); @@ -171,7 +182,8 @@ const ChoiceGroupMixinImplementation = superclass => */ addFormElement(child, indexToInsertAt) { this._throwWhenInvalidChildModelValue(child); - this.__delegateNameAttribute(child); + // eslint-disable-next-line no-param-reassign + child.name = this.name; super.addFormElement(child, indexToInsertAt); } @@ -283,24 +295,6 @@ const ChoiceGroupMixinImplementation = superclass => } } - /** - * @param {FormControl} child - */ - __delegateNameAttribute(child) { - if (!child.name || child.name === this.name) { - // eslint-disable-next-line no-param-reassign - child.name = this.name; - } else { - throw new Error( - `The ${this.tagName.toLowerCase()} name="${ - this.name - }" does not allow to register ${child.tagName.toLowerCase()} with custom names (name="${ - child.name - }" given)`, - ); - } - } - /** * @override FormControlMixin * @param {CustomEvent} ev diff --git a/packages/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js b/packages/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js index 60464c0e8..ec118c3b8 100644 --- a/packages/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js +++ b/packages/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js @@ -74,7 +74,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString } = { ); }); - it('automatically sets the name property of child radios to its own name', async () => { + it('automatically sets the name property of child fields to its own name', async () => { const el = /** @type {ChoiceGroup} */ (await fixture(html` <${parentTag} name="gender"> <${childTag} .choiceValue=${'female'} checked> @@ -93,7 +93,26 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString } = { expect(el.formElements[2].name).to.equal('gender'); }); - it('throws if a child element with a different name than the group tries to register', async () => { + it('automatically updates the name property of child fields to its own name', async () => { + const el = /** @type {ChoiceGroup} */ (await fixture(html` + <${parentTag} name="gender"> + <${childTag}> + <${childTag}> + + `)); + + expect(el.formElements[0].name).to.equal('gender'); + expect(el.formElements[1].name).to.equal('gender'); + + el.name = 'gender2'; + + await el.updateComplete; + + expect(el.formElements[0].name).to.equal('gender2'); + expect(el.formElements[1].name).to.equal('gender2'); + }); + + it('adjusts the name of a child element if it has a different name than the group', async () => { const el = /** @type {ChoiceGroup} */ (await fixture(html` <${parentTag} name="gender"> <${childTag} .choiceValue=${'female'} checked> @@ -104,12 +123,9 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString } = { const invalidChild = /** @type {ChoiceGroup} */ (await fixture(html` <${childTag} name="foo" .choiceValue=${'male'}> `)); - - expect(() => { - el.addFormElement(invalidChild); - }).to.throw( - 'The choice-group name="gender" does not allow to register choice-group-input with custom names (name="foo" given)', - ); + el.addFormElement(invalidChild); + await invalidChild.updateComplete; + expect(invalidChild.name).to.equal('gender'); }); it('can set initial modelValue on creation', async () => { diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 8a230ffd7..858ca2a51 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -96,24 +96,6 @@ export function runListboxMixinSuite(customConfig = {}) { ); }); - it('throws if a child element with a different name than the group tries to register', async () => { - const el = await fixture(html` - <${tag} name="gender"> - <${optionTag} .choiceValue=${'female'} checked> - <${optionTag} .choiceValue=${'other'}> - - `); - const invalidChild = await fixture(html` - <${optionTag} name="foo" .choiceValue=${'male'}> - `); - - expect(() => { - el.addFormElement(invalidChild); - }).to.throw( - `The ${cfg.tagString} name="gender" does not allow to register lion-option with custom names (name="foo" given)`, - ); - }); - it('can set initial modelValue on creation', async () => { const el = await fixture(html` <${tag} name="gender" .modelValue=${'other'}>