ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(form-core): fix infinite loop syncing name to parent

Browse source
This commit is contained in:
Joren Broekema 2021-01-26 10:30:00 +01:00
parent a54d8c3d56
commit a7b2750218
4 changed files with 101 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/rich-pens-repeat.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/form-core': patch
---
Sync name to parent form group conditionally and allow overriding. Also fix sync properly to prevent infinite loop.

17
packages/form-core/src/choice-group/ChoiceInputMixin.js
View file

@ -119,8 +119,7 @@ const ChoiceInputMixinImplementation = superclass =>
// @ts-expect-error // @ts-expect-error
this._parentFormGroup.name !== this.name this._parentFormGroup.name !== this.name
) { ) {
// @ts-expect-error not all choice inputs have a name prop, because this mixin does not have a strict contract with form control mixin this._syncNameToParentFormGroup();
this.name = changedProperties.get('name');
} }
} }
@ -232,6 +231,20 @@ const ChoiceInputMixinImplementation = superclass =>
this.__isHandlingUserInput = false; this.__isHandlingUserInput = false;
} }
/**
* Override this in case of extending ChoiceInputMixin and requiring
* to sync differently with parent form group name
* Right now it checks tag name match where the parent form group tagname
* should include the child field tagname ('checkbox' is included in 'checkbox-group')
*/
_syncNameToParentFormGroup() {
// @ts-expect-error not all choice inputs have a name prop, because this mixin does not have a strict contract with form control mixin
if (this._parentFormGroup.tagName.includes(this.tagName)) {
// @ts-expect-error
this.name = this._parentFormGroup.name;
}
}
/** /**
* @param {boolean} checked * @param {boolean} checked
*/ */

110
packages/form-core/test-suites/choice-group/ChoiceGroupMixin.suite.js
View file

@ -6,28 +6,42 @@ import { expect, html, fixture, unsafeStatic } from '@open-wc/testing';
import { ChoiceGroupMixin } from '../../src/choice-group/ChoiceGroupMixin.js'; import { ChoiceGroupMixin } from '../../src/choice-group/ChoiceGroupMixin.js';
import { ChoiceInputMixin } from '../../src/choice-group/ChoiceInputMixin.js'; import { ChoiceInputMixin } from '../../src/choice-group/ChoiceInputMixin.js';
class ChoiceInputFoo extends ChoiceInputMixin(LionInput) {}
customElements.define('choice-input-foo', ChoiceInputFoo);
class ChoiceInputBar extends ChoiceInputMixin(LionInput) {
_syncNameToParentFormGroup() {
// Always sync, without conditions
// @ts-expect-error
this.name = this._parentFormGroup.name;
}
}
customElements.define('choice-input-bar', ChoiceInputBar);
class ChoiceInput extends ChoiceInputMixin(LionInput) {} class ChoiceInput extends ChoiceInputMixin(LionInput) {}
customElements.define('choice-group-input', ChoiceInput); customElements.define('choice-input', ChoiceInput);
class ChoiceGroup extends ChoiceGroupMixin(FormGroupMixin(LitElement)) {} class ChoiceInputGroup extends ChoiceGroupMixin(FormGroupMixin(LitElement)) {}
customElements.define('choice-group', ChoiceGroup); customElements.define('choice-input-group', ChoiceInputGroup);
/** /**
* @param {{ parentTagString?:string, childTagString?: string, choiceType?: string}} [config] * @param {{ parentTagString?:string, childTagString?: string, choiceType?: string}} [config]
*/ */
export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choiceType } = {}) { export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choiceType } = {}) {
const cfg = { const cfg = {
parentTagString: parentTagString || 'choice-group', parentTagString: parentTagString || 'choice-input-group',
childTagString: childTagString || 'choice-group-input', childTagString: childTagString || 'choice-input',
childTagStringFoo: 'choice-input-foo',
childTagStringBar: 'choice-input-bar',
choiceType: choiceType || 'single', choiceType: choiceType || 'single',
}; };
const parentTag = unsafeStatic(cfg.parentTagString); const parentTag = unsafeStatic(cfg.parentTagString);
const childTag = unsafeStatic(cfg.childTagString); const childTag = unsafeStatic(cfg.childTagString);
const childTagFoo = unsafeStatic(cfg.childTagStringFoo);
const childTagBar = unsafeStatic(cfg.childTagStringBar);
describe(`ChoiceGroupMixin: ${cfg.parentTagString}`, () => { describe(`ChoiceGroupMixin: ${cfg.parentTagString}`, () => {
if (cfg.choiceType === 'single') { if (cfg.choiceType === 'single') {
it('has a single modelValue representing the currently checked radio value', async () => { it('has a single modelValue representing the currently checked radio value', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
@ -42,7 +56,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('has a single formattedValue representing the currently checked radio value', async () => { it('has a single formattedValue representing the currently checked radio value', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender"> <${parentTag} name="gender">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
@ -58,14 +72,14 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
} }
it('throws if a child element without a modelValue like { value: "foo", checked: false } tries to register', async () => { it('throws if a child element without a modelValue like { value: "foo", checked: false } tries to register', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
<${childTag} .choiceValue=${'other'}></${childTag}> <${childTag} .choiceValue=${'other'}></${childTag}>
</${parentTag}> </${parentTag}>
`)); `));
const invalidChild = /** @type {ChoiceGroup} */ (await fixture(html` const invalidChild = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${childTag} .modelValue=${'Lara'}></${childTag}> <${childTag} .modelValue=${'Lara'}></${childTag}>
`)); `));
@ -77,7 +91,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('automatically sets the name property of child fields 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` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
<${childTag} .choiceValue=${'other'}></${childTag}> <${childTag} .choiceValue=${'other'}></${childTag}>
@ -87,7 +101,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
expect(el.formElements[0].name).to.equal('gender[]'); expect(el.formElements[0].name).to.equal('gender[]');
expect(el.formElements[1].name).to.equal('gender[]'); expect(el.formElements[1].name).to.equal('gender[]');
const validChild = /** @type {ChoiceGroup} */ (await fixture(html` const validChild = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
`)); `));
el.appendChild(validChild); el.appendChild(validChild);
@ -96,7 +110,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('automatically updates the name property of child fields to its own name', async () => { it('automatically updates the name property of child fields to its own name', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag}></${childTag}> <${childTag}></${childTag}>
<${childTag}></${childTag}> <${childTag}></${childTag}>
@ -115,7 +129,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('prevents updating the name property of a child if it is different from its parent', async () => { it('prevents updating the name property of a child if it is different from its parent', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag}></${childTag}> <${childTag}></${childTag}>
<${childTag}></${childTag}> <${childTag}></${childTag}>
@ -131,15 +145,49 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
expect(el.formElements[0].name).to.equal('gender[]'); expect(el.formElements[0].name).to.equal('gender[]');
}); });
it('allows updating the name property of a child if parent tagName does not include childTagname', async () => {
const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]">
<${childTagFoo}></${childTagFoo}>
<${childTagFoo}></${childTagFoo}>
</${parentTag}>
`));
expect(el.formElements[0].name).to.equal('gender[]');
expect(el.formElements[1].name).to.equal('gender[]');
el.formElements[0].name = 'gender2[]';
await el.formElements[0].updateComplete;
expect(el.formElements[0].name).to.equal('gender2[]');
});
it('allows setting the condition for syncing the name property of a child to parent', async () => {
const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]">
<${childTagBar}></${childTagBar}>
<${childTagBar}></${childTagBar}>
</${parentTag}>
`));
expect(el.formElements[0].name).to.equal('gender[]');
expect(el.formElements[1].name).to.equal('gender[]');
el.formElements[0].name = 'gender2[]';
await el.formElements[0].updateComplete;
expect(el.formElements[0].name).to.equal('gender[]');
});
it('adjusts the name of a child element if it has a different name than the group', async () => { 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` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
<${childTag} .choiceValue=${'other'}></${childTag}> <${childTag} .choiceValue=${'other'}></${childTag}>
</${parentTag}> </${parentTag}>
`)); `));
const invalidChild = /** @type {ChoiceGroup} */ (await fixture(html` const invalidChild = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${childTag} name="foo" .choiceValue=${'male'}></${childTag}> <${childTag} name="foo" .choiceValue=${'male'}></${childTag}>
`)); `));
el.addFormElement(invalidChild); el.addFormElement(invalidChild);
@ -148,7 +196,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can set initial modelValue on creation', async () => { it('can set initial modelValue on creation', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]" .modelValue=${'other'}> <${parentTag} name="gender[]" .modelValue=${'other'}>
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -165,7 +213,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can set initial serializedValue on creation', async () => { it('can set initial serializedValue on creation', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]" .serializedValue=${'other'}> <${parentTag} name="gender[]" .serializedValue=${'other'}>
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -182,7 +230,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can set initial formattedValue on creation', async () => { it('can set initial formattedValue on creation', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]" .formattedValue=${'other'}> <${parentTag} name="gender[]" .formattedValue=${'other'}>
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -201,7 +249,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
it('can handle complex data via choiceValue', async () => { it('can handle complex data via choiceValue', async () => {
const date = new Date(2018, 11, 24, 10, 33, 30, 0); const date = new Date(2018, 11, 24, 10, 33, 30, 0);
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="data[]"> <${parentTag} name="data[]">
<${childTag} .choiceValue=${{ some: 'data' }}></${childTag}> <${childTag} .choiceValue=${{ some: 'data' }}></${childTag}>
<${childTag} .choiceValue=${date} checked></${childTag}> <${childTag} .choiceValue=${date} checked></${childTag}>
@ -220,7 +268,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can handle 0 and empty string as valid values', async () => { it('can handle 0 and empty string as valid values', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="data[]"> <${parentTag} name="data[]">
<${childTag} .choiceValue=${0} checked></${childTag}> <${childTag} .choiceValue=${0} checked></${childTag}>
<${childTag} .choiceValue=${''}></${childTag}> <${childTag} .choiceValue=${''}></${childTag}>
@ -239,7 +287,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can check a radio by supplying an available modelValue', async () => { it('can check a radio by supplying an available modelValue', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} <${childTag}
.modelValue="${{ value: 'male', checked: false }}" .modelValue="${{ value: 'male', checked: false }}"
@ -264,7 +312,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
it('expect child nodes to only fire one model-value-changed event per instance', async () => { it('expect child nodes to only fire one model-value-changed event per instance', async () => {
let counter = 0; let counter = 0;
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} <${parentTag}
name="gender[]" name="gender[]"
@model-value-changed=${() => { @model-value-changed=${() => {
@ -311,7 +359,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can be required', async () => { it('can be required', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]" .validators=${[new Required()]}> <${parentTag} name="gender[]" .validators=${[new Required()]}>
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} <${childTag}
@ -335,7 +383,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('returns serialized value', async () => { it('returns serialized value', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -350,7 +398,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('returns serialized value on unchecked state', async () => { it('returns serialized value on unchecked state', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -366,7 +414,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
describe('multipleChoice', () => { describe('multipleChoice', () => {
it('has a single modelValue representing all currently checked values', async () => { it('has a single modelValue representing all currently checked values', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} multiple-choice name="gender[]"> <${parentTag} multiple-choice name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
@ -382,7 +430,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('has a single serializedValue representing all currently checked values', async () => { it('has a single serializedValue representing all currently checked values', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} multiple-choice name="gender[]"> <${parentTag} multiple-choice name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
@ -398,7 +446,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('has a single formattedValue representing all currently checked values', async () => { it('has a single formattedValue representing all currently checked values', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} multiple-choice name="gender[]"> <${parentTag} multiple-choice name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'} checked></${childTag}> <${childTag} .choiceValue=${'female'} checked></${childTag}>
@ -414,7 +462,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('can check multiple checkboxes by setting the modelValue', async () => { it('can check multiple checkboxes by setting the modelValue', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} multiple-choice name="gender[]"> <${parentTag} multiple-choice name="gender[]">
<${childTag} .choiceValue=${'male'}></${childTag}> <${childTag} .choiceValue=${'male'}></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -429,7 +477,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
}); });
it('unchecks non-matching checkboxes when setting the modelValue', async () => { it('unchecks non-matching checkboxes when setting the modelValue', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<${parentTag} multiple-choice name="gender[]"> <${parentTag} multiple-choice name="gender[]">
<${childTag} .choiceValue=${'male'} checked></${childTag}> <${childTag} .choiceValue=${'male'} checked></${childTag}>
<${childTag} .choiceValue=${'female'}></${childTag}> <${childTag} .choiceValue=${'female'}></${childTag}>
@ -450,7 +498,7 @@ export function runChoiceGroupMixinSuite({ parentTagString, childTagString, choi
describe('Integration with a parent form/fieldset', () => { describe('Integration with a parent form/fieldset', () => {
it('will serialize all children with their serializedValue', async () => { it('will serialize all children with their serializedValue', async () => {
const el = /** @type {ChoiceGroup} */ (await fixture(html` const el = /** @type {ChoiceInputGroup} */ (await fixture(html`
<lion-fieldset> <lion-fieldset>
<${parentTag} name="gender[]"> <${parentTag} name="gender[]">
<${childTag} .choiceValue=${'male'} checked disabled></${childTag}> <${childTag} .choiceValue=${'male'} checked disabled></${childTag}>

2
packages/form-core/types/choice-group/ChoiceInputMixinTypes.d.ts vendored
View file

@ -43,6 +43,8 @@ export declare class ChoiceInputHost {
_preventDuplicateLabelClick(ev: Event): void; _preventDuplicateLabelClick(ev: Event): void;
_syncNameToParentFormGroup(): void;
_toggleChecked(ev: Event): void; _toggleChecked(ev: Event): void;
__syncModelCheckedToChecked(checked: boolean): void; __syncModelCheckedToChecked(checked: boolean): void;