From 9b905c492a0c0d2226cc1d75c73e2e70dc97815a Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 20 Feb 2020 10:44:17 +0100 Subject: [PATCH 1/7] feat: api normalisation formElements and values --- .../checkbox-group/src/LionCheckboxGroup.js | 5 +- packages/choice-input/src/ChoiceGroupMixin.js | 71 ++- .../test/ChoiceGroupMixin.test.js | 86 ++-- packages/field/index.js | 9 +- packages/field/src/FormControlMixin.js | 24 +- packages/field/src/FormRegistrarMixin.js | 121 ----- .../registration/FormControlsCollection.js | 98 ++++ .../FormRegisteringMixin.js | 0 .../src/registration/FormRegistrarMixin.js | 184 +++++++ .../FormRegistrarPortalMixin.js | 0 .../formRegistrarManager.js | 0 .../FormRegistrationMixins.suite.js | 11 +- packages/fieldset/index.js | 1 + packages/fieldset/src/FormGroupMixin.js | 403 +++++++++++++++ packages/fieldset/src/LionFieldset.js | 482 +----------------- packages/fieldset/test/lion-fieldset.test.js | 227 +++++---- .../test/form-integrations.test.js | 33 ++ .../form-system/test/helpers/umbrella-form.js | 106 ++++ packages/radio-group/src/LionRadioGroup.js | 7 +- .../radio-group/test/lion-radio-group.test.js | 4 +- packages/select-rich/src/LionSelectRich.js | 7 +- .../select-rich/test/lion-select-rich.test.js | 8 +- 22 files changed, 1120 insertions(+), 767 deletions(-) delete mode 100644 packages/field/src/FormRegistrarMixin.js create mode 100644 packages/field/src/registration/FormControlsCollection.js rename packages/field/src/{ => registration}/FormRegisteringMixin.js (100%) create mode 100644 packages/field/src/registration/FormRegistrarMixin.js rename packages/field/src/{ => registration}/FormRegistrarPortalMixin.js (100%) rename packages/field/src/{ => registration}/formRegistrarManager.js (100%) create mode 100644 packages/fieldset/src/FormGroupMixin.js create mode 100644 packages/form-system/test/form-integrations.test.js create mode 100644 packages/form-system/test/helpers/umbrella-form.js diff --git a/packages/checkbox-group/src/LionCheckboxGroup.js b/packages/checkbox-group/src/LionCheckboxGroup.js index 4a3e1046b..6708a0619 100644 --- a/packages/checkbox-group/src/LionCheckboxGroup.js +++ b/packages/checkbox-group/src/LionCheckboxGroup.js @@ -1,12 +1,13 @@ +import { LitElement } from '@lion/core'; import { ChoiceGroupMixin } from '@lion/choice-input'; -import { LionFieldset } from '@lion/fieldset'; +import { FormGroupMixin } from '@lion/fieldset'; /** * A wrapper around multiple checkboxes * * @extends {LionFieldset} */ -export class LionCheckboxGroup extends ChoiceGroupMixin(LionFieldset) { +export class LionCheckboxGroup extends ChoiceGroupMixin(FormGroupMixin(LitElement)) { constructor() { super(); this.multipleChoice = true; diff --git a/packages/choice-input/src/ChoiceGroupMixin.js b/packages/choice-input/src/ChoiceGroupMixin.js index 123d92ac1..6bc3d93be 100644 --- a/packages/choice-input/src/ChoiceGroupMixin.js +++ b/packages/choice-input/src/ChoiceGroupMixin.js @@ -5,12 +5,30 @@ export const ChoiceGroupMixin = dedupeMixin( superclass => // eslint-disable-next-line class ChoiceGroupMixin extends FormRegistrarMixin(InteractionStateMixin(superclass)) { + static get properties() { + return { + /** + * @desc When false (default), modelValue and serializedValue will reflect the + * currently selected choice (usually a string). When true, modelValue will and + * serializedValue will be an array of strings. + * @type {boolean} + */ + multipleChoice: { + type: Boolean, + attribute: 'multiple-choice', + }, + }; + } + get modelValue() { const elems = this._getCheckedElements(); if (this.multipleChoice) { + // TODO: holds for both modelValue and serializedValue of choiceInput: + // consider only allowing strings as values, in which case 'el.value' would suffice + // and choice-input could be simplified return elems.map(el => el.modelValue.value); } - return elems ? elems.modelValue.value : ''; + return elems[0] ? elems[0].modelValue.value : ''; } set modelValue(value) { @@ -18,11 +36,19 @@ export const ChoiceGroupMixin = dedupeMixin( } get serializedValue() { + // We want to filter out disabled values out by default: + // The goal of serializing values could either be submitting state to a backend + // ot storing state in a backend. For this, only values that are entered by the end + // user are relevant, choice values are always defined by the Application Developer + // and known by the backend. + + // Assuming values are always defined as strings, modelValues and serializedValues + // are the same. const elems = this._getCheckedElements(); if (this.multipleChoice) { - return this.modelValue; + return elems.map(el => el.serializedValue.value); } - return elems ? elems.serializedValue : ''; + return elems[0] ? elems[0].serializedValue.value : ''; } set serializedValue(value) { @@ -53,24 +79,22 @@ export const ChoiceGroupMixin = dedupeMixin( */ addFormElement(child, indexToInsertAt) { this._throwWhenInvalidChildModelValue(child); + // TODO: nice to have or does it have a function (since names are meant as keys for + // formElements)? this.__delegateNameAttribute(child); super.addFormElement(child, indexToInsertAt); } /** - * @override from LionFieldset + * @override */ - // eslint-disable-next-line class-methods-use-this - get _childrenCanHaveSameName() { - return true; - } - - /** - * @override from LionFieldset - */ - // eslint-disable-next-line class-methods-use-this - get _childNamesCanBeDuplicate() { - return true; + _getFromAllFormElements(property, filterCondition = () => true) { + // For modelValue and serializedValue, an exception should be made, + // The reset can be requested from children + if (property === 'modelValue' || property === 'serializedValue') { + return this[property]; + } + return this.formElements.filter(filterCondition).map(el => el.property); } _throwWhenInvalidChildModelValue(child) { @@ -108,7 +132,7 @@ export const ChoiceGroupMixin = dedupeMixin( if (target.checked === false) return; const groupName = target.name; - this.formElementsArray + this.formElements .filter(i => i.name === groupName) .forEach(choice => { if (choice !== target) { @@ -119,11 +143,8 @@ export const ChoiceGroupMixin = dedupeMixin( } _getCheckedElements() { - const filtered = this.formElementsArray.filter(el => el.checked === true); - if (this.multipleChoice) { - return filtered; - } - return filtered.length > 0 ? filtered[0] : undefined; + // We want to filter out disabled values out by default + return this.formElements.filter(el => el.checked && !el.disabled); } async _setCheckedElements(value, check) { @@ -131,12 +152,12 @@ export const ChoiceGroupMixin = dedupeMixin( await this.registrationReady; } - for (let i = 0; i < this.formElementsArray.length; i += 1) { + for (let i = 0; i < this.formElements.length; i += 1) { if (this.multipleChoice) { - this.formElementsArray[i].checked = value.includes(this.formElementsArray[i].value); - } else if (check(this.formElementsArray[i], value)) { + this.formElements[i].checked = value.includes(this.formElements[i].value); + } else if (check(this.formElements[i], value)) { // Allows checking against custom values e.g. formattedValue or serializedValue - this.formElementsArray[i].checked = true; + this.formElements[i].checked = true; } } } diff --git a/packages/choice-input/test/ChoiceGroupMixin.test.js b/packages/choice-input/test/ChoiceGroupMixin.test.js index c8566f6c2..5d1f01f32 100644 --- a/packages/choice-input/test/ChoiceGroupMixin.test.js +++ b/packages/choice-input/test/ChoiceGroupMixin.test.js @@ -1,20 +1,21 @@ -import { html } from '@lion/core'; -import { LionFieldset } from '@lion/fieldset'; +import { html, LitElement } from '@lion/core'; +import { FormGroupMixin } from '@lion/fieldset'; import { LionInput } from '@lion/input'; import { Required } from '@lion/validate'; import { expect, fixture, nextFrame } from '@open-wc/testing'; import { ChoiceGroupMixin } from '../src/ChoiceGroupMixin.js'; import { ChoiceInputMixin } from '../src/ChoiceInputMixin.js'; +import '@lion/fieldset/lion-fieldset.js'; describe('ChoiceGroupMixin', () => { before(() => { class ChoiceInput extends ChoiceInputMixin(LionInput) {} customElements.define('choice-group-input', ChoiceInput); - class ChoiceGroup extends ChoiceGroupMixin(LionFieldset) {} + class ChoiceGroup extends ChoiceGroupMixin(FormGroupMixin(LitElement)) {} customElements.define('choice-group', ChoiceGroup); - class ChoiceGroupMultiple extends ChoiceGroupMixin(LionFieldset) { + class ChoiceGroupMultiple extends ChoiceGroupMixin(FormGroupMixin(LitElement)) { constructor() { super(); this.multipleChoice = true; @@ -33,9 +34,9 @@ describe('ChoiceGroupMixin', () => { `); await nextFrame(); expect(el.modelValue).to.equal('female'); - el.formElementsArray[0].checked = true; + el.formElements[0].checked = true; expect(el.modelValue).to.equal('male'); - el.formElementsArray[2].checked = true; + el.formElements[2].checked = true; expect(el.modelValue).to.equal('other'); }); @@ -68,15 +69,15 @@ describe('ChoiceGroupMixin', () => { `); await nextFrame(); - expect(el.formElementsArray[0].name).to.equal('gender'); - expect(el.formElementsArray[1].name).to.equal('gender'); + expect(el.formElements[0].name).to.equal('gender'); + expect(el.formElements[1].name).to.equal('gender'); const validChild = await fixture(html` `); el.appendChild(validChild); - expect(el.formElementsArray[2].name).to.equal('gender'); + 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 () => { @@ -112,7 +113,7 @@ describe('ChoiceGroupMixin', () => { await el.updateComplete; expect(el.modelValue).to.equal('other'); - expect(el.formElementsArray[2].checked).to.be.true; + expect(el.formElements[2].checked).to.be.true; }); it('can handle complex data via choiceValue', async () => { @@ -127,7 +128,7 @@ describe('ChoiceGroupMixin', () => { await nextFrame(); expect(el.modelValue).to.equal(date); - el.formElementsArray[0].checked = true; + el.formElements[0].checked = true; expect(el.modelValue).to.deep.equal({ some: 'data' }); }); @@ -141,7 +142,7 @@ describe('ChoiceGroupMixin', () => { await nextFrame(); expect(el.modelValue).to.equal(0); - el.formElementsArray[1].checked = true; + el.formElements[1].checked = true; expect(el.modelValue).to.equal(''); }); @@ -160,7 +161,7 @@ describe('ChoiceGroupMixin', () => { await nextFrame(); expect(el.modelValue).to.equal('female'); el.modelValue = 'other'; - expect(el.formElementsArray[2].checked).to.be.true; + expect(el.formElements[2].checked).to.be.true; }); it('expect child nodes to only fire one model-value-changed event per instance', async () => { @@ -180,14 +181,14 @@ describe('ChoiceGroupMixin', () => { await nextFrame(); counter = 0; // reset after setup which may result in different results - el.formElementsArray[0].checked = true; + el.formElements[0].checked = true; expect(counter).to.equal(2); // male becomes checked, female becomes unchecked // not changed values trigger no event - el.formElementsArray[0].checked = true; + el.formElements[0].checked = true; expect(counter).to.equal(2); - el.formElementsArray[2].checked = true; + el.formElements[2].checked = true; expect(counter).to.equal(4); // other becomes checked, male becomes unchecked // not found values trigger no event @@ -211,12 +212,12 @@ describe('ChoiceGroupMixin', () => { expect(el.validationStates).to.have.a.property('error'); expect(el.validationStates.error).to.have.a.property('Required'); - el.formElementsArray[0].checked = true; + el.formElements[0].checked = true; expect(el.hasFeedbackFor).not.to.include('error'); expect(el.validationStates).to.have.a.property('error'); expect(el.validationStates.error).not.to.have.a.property('Required'); - el.formElementsArray[1].checked = true; + el.formElements[1].checked = true; expect(el.hasFeedbackFor).not.to.include('error'); expect(el.validationStates).to.have.a.property('error'); expect(el.validationStates.error).not.to.have.a.property('Required'); @@ -229,8 +230,8 @@ describe('ChoiceGroupMixin', () => { `); - el.formElementsArray[0].checked = true; - expect(el.serializedValue).to.deep.equal({ checked: true, value: 'male' }); + el.formElements[0].checked = true; + expect(el.serializedValue).to.deep.equal('male'); }); it('returns serialized value on unchecked state', async () => { @@ -256,9 +257,9 @@ describe('ChoiceGroupMixin', () => { `); await nextFrame(); expect(el.modelValue).to.eql(['female']); - el.formElementsArray[0].checked = true; + el.formElements[0].checked = true; expect(el.modelValue).to.eql(['male', 'female']); - el.formElementsArray[2].checked = true; + el.formElements[2].checked = true; expect(el.modelValue).to.eql(['male', 'female', 'other']); }); @@ -276,8 +277,8 @@ describe('ChoiceGroupMixin', () => { await el.updateComplete; el.modelValue = ['male', 'other']; expect(el.modelValue).to.eql(['male', 'other']); - expect(el.formElementsArray[0].checked).to.be.true; - expect(el.formElementsArray[2].checked).to.be.true; + expect(el.formElements[0].checked).to.be.true; + expect(el.formElements[2].checked).to.be.true; }); it('unchecks non-matching checkboxes when setting the modelValue', async () => { @@ -293,13 +294,40 @@ describe('ChoiceGroupMixin', () => { await el.registrationReady; await el.updateComplete; expect(el.modelValue).to.eql(['male', 'other']); - expect(el.formElementsArray[0].checked).to.be.true; - expect(el.formElementsArray[2].checked).to.be.true; + expect(el.formElements[0].checked).to.be.true; + expect(el.formElements[2].checked).to.be.true; el.modelValue = ['female']; - expect(el.formElementsArray[0].checked).to.be.false; - expect(el.formElementsArray[1].checked).to.be.true; - expect(el.formElementsArray[2].checked).to.be.false; + expect(el.formElements[0].checked).to.be.false; + expect(el.formElements[1].checked).to.be.true; + expect(el.formElements[2].checked).to.be.false; + }); + }); + + describe('Integration with a parent form/fieldset', () => { + it('will serialize all children with their serializedValue', async () => { + const el = await fixture(html` + + + + + + + + `); + + await nextFrame(); + await el.registrationReady; + await el.updateComplete; + expect(el.serializedValue).to.eql({ + gender: 'female', + }); + + const choiceGroupEl = el.querySelector('[name="gender"]'); + choiceGroupEl.multipleChoice = true; + expect(el.serializedValue).to.eql({ + gender: ['female'], + }); }); }); }); diff --git a/packages/field/index.js b/packages/field/index.js index 3dff79439..6c1d2d28d 100644 --- a/packages/field/index.js +++ b/packages/field/index.js @@ -1,9 +1,10 @@ -export { FieldCustomMixin } from './src/FieldCustomMixin.js'; export { FocusMixin } from './src/FocusMixin.js'; export { FormatMixin } from './src/FormatMixin.js'; +export { FieldCustomMixin } from './src/FieldCustomMixin.js'; export { FormControlMixin } from './src/FormControlMixin.js'; export { InteractionStateMixin } from './src/InteractionStateMixin.js'; // applies FocusMixin export { LionField } from './src/LionField.js'; -export { FormRegisteringMixin } from './src/FormRegisteringMixin.js'; -export { FormRegistrarMixin } from './src/FormRegistrarMixin.js'; -export { FormRegistrarPortalMixin } from './src/FormRegistrarPortalMixin.js'; +export { FormRegisteringMixin } from './src/registration/FormRegisteringMixin.js'; +export { FormRegistrarMixin } from './src/registration/FormRegistrarMixin.js'; +export { FormRegistrarPortalMixin } from './src/registration/FormRegistrarPortalMixin.js'; +export { FormControlsCollection } from './src/registration/FormControlsCollection.js'; diff --git a/packages/field/src/FormControlMixin.js b/packages/field/src/FormControlMixin.js index c58717615..6cfd78a90 100644 --- a/packages/field/src/FormControlMixin.js +++ b/packages/field/src/FormControlMixin.js @@ -1,5 +1,5 @@ import { html, css, nothing, dedupeMixin, SlotMixin } from '@lion/core'; -import { FormRegisteringMixin } from './FormRegisteringMixin.js'; +import { FormRegisteringMixin } from './registration/FormRegisteringMixin.js'; import { getAriaElementsInRightDomOrder } from './utils/getAriaElementsInRightDomOrder.js'; /** @@ -28,11 +28,18 @@ export const FormControlMixin = dedupeMixin( class FormControlMixin extends FormRegisteringMixin(SlotMixin(superclass)) { static get properties() { return { + /** + * The name the element will be registered on to the .formElements collection + * of the parent. + */ + name: { + type: String, + reflect: true, + }, /** * When no light dom defined and prop set */ label: String, - /** * When no light dom defined and prop set */ @@ -40,12 +47,10 @@ export const FormControlMixin = dedupeMixin( type: String, attribute: 'help-text', }, - /** * Contains all elements that should end up in aria-labelledby of `._inputNode` */ _ariaLabelledNodes: Array, - /** * Contains all elements that should end up in aria-describedby of `._inputNode` */ @@ -73,6 +78,14 @@ export const FormControlMixin = dedupeMixin( this.requestUpdate('helpText', oldValue); } + set fieldName(value) { + this.__fieldName = value; + } + + get fieldName() { + return this.__fieldName || this.label || this.name; + } + get slots() { return { ...super.slots, @@ -146,9 +159,6 @@ export const FormControlMixin = dedupeMixin( this._enhanceLightDomA11y(); } - /** - * Public methods - */ _enhanceLightDomClasses() { if (this._inputNode) { this._inputNode.classList.add('form-control'); diff --git a/packages/field/src/FormRegistrarMixin.js b/packages/field/src/FormRegistrarMixin.js deleted file mode 100644 index bb341805a..000000000 --- a/packages/field/src/FormRegistrarMixin.js +++ /dev/null @@ -1,121 +0,0 @@ -import { dedupeMixin } from '@lion/core'; -import { FormRegisteringMixin } from './FormRegisteringMixin.js'; -import { formRegistrarManager } from './formRegistrarManager.js'; - -/** - * This allows an element to become the manager of a register - */ -export const FormRegistrarMixin = dedupeMixin( - superclass => - // eslint-disable-next-line no-shadow, no-unused-vars - class FormRegistrarMixin extends FormRegisteringMixin(superclass) { - get formElements() { - return this.__formElements; - } - - set formElements(value) { - this.__formElements = value; - } - - get formElementsArray() { - return this.__formElements; - } - - constructor() { - super(); - this.formElements = []; - this.__readyForRegistration = false; - this.__hasBeenRendered = false; - this.registrationReady = new Promise(resolve => { - this.__resolveRegistrationReady = resolve; - }); - - this._onRequestToAddFormElement = this._onRequestToAddFormElement.bind(this); - this.addEventListener('form-element-register', this._onRequestToAddFormElement); - } - - connectedCallback() { - if (super.connectedCallback) { - super.connectedCallback(); - } - formRegistrarManager.add(this); - if (this.__hasBeenRendered) { - formRegistrarManager.becomesReady(); - } - } - - disconnectedCallback() { - if (super.disconnectedCallback) { - super.disconnectedCallback(); - } - formRegistrarManager.remove(this); - } - - isRegisteredFormElement(el) { - return this.formElementsArray.some(exitingEl => exitingEl === el); - } - - firstUpdated(changedProperties) { - super.firstUpdated(changedProperties); - this.__resolveRegistrationReady(); - this.__readyForRegistration = true; - formRegistrarManager.becomesReady(); - this.__hasBeenRendered = true; - } - - addFormElement(child, index) { - // This is a way to let the child element (a lion-fieldset or lion-field) know, about its parent - // eslint-disable-next-line no-param-reassign - child.__parentFormGroup = this; - - if (index > 0) { - this.formElements.splice(index, 0, child); - } else { - this.formElements.push(child); - } - } - - removeFormElement(child) { - const index = this.formElements.indexOf(child); - if (index > -1) { - this.formElements.splice(index, 1); - } - } - - _onRequestToAddFormElement(ev) { - const child = ev.detail.element; - if (child === this) { - // as we fire and listen - don't add ourselves - return; - } - if (this.isRegisteredFormElement(child)) { - // do not readd already existing elements - return; - } - ev.stopPropagation(); - - // Check for siblings to determine the right order to insert into formElements - // If there is no next sibling, index is -1 - let indexToInsertAt = -1; - if (this.formElements && Array.isArray(this.formElements)) { - indexToInsertAt = this.formElements.indexOf(child.nextElementSibling); - } - this.addFormElement(child, indexToInsertAt); - } - - _onRequestToRemoveFormElement(ev) { - const child = ev.detail.element; - if (child === this) { - // as we fire and listen - don't add ourselves - return; - } - if (!this.isRegisteredFormElement(child)) { - // do not readd already existing elements - return; - } - ev.stopPropagation(); - - this.removeFormElement(child); - } - }, -); diff --git a/packages/field/src/registration/FormControlsCollection.js b/packages/field/src/registration/FormControlsCollection.js new file mode 100644 index 000000000..f425588ad --- /dev/null +++ b/packages/field/src/registration/FormControlsCollection.js @@ -0,0 +1,98 @@ +/** + * @desc This class closely mimics the natively + * supported HTMLFormControlsCollection. It can be accessed + * both like an array and an object (based on control/element names). + * @example + * // This is how a native form works: + *
+ * + *
+ * + * + * + *
+ * + *
+ * + * + *
+ * + *
+ * + * + *
+ *
+ * + * form.elements[0]; // Element input#a + * form.elements[1]; // Element input#b1 + * form.elements[2]; // Element input#b2 + * form.elements[3]; // Element input#c + * form.elements.a; // Element input#a + * form.elements.b; // RadioNodeList [input#b1, input#b2] + * form.elements.c; // input#c + * + * // This is how a Lion form works (for simplicity Lion components have the 'l'-prefix): + * + *
+ * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
+ * + * + * lionForm.formElements[0]; // Element l-input#a + * lionForm.formElements[1]; // Element l-input#b1 + * lionForm.formElements[2]; // Element l-input#b2 + * lionForm.formElements.a; // Element l-input#a + * lionForm.formElements['b[]']; // Array [l-input#b1, l-input#b2] + * lionForm.formElements.c; // Element l-input#c + * + * lionForm.formElements[d-g].formElements; // Array + * + * lionForm.formElements[d-e].value; // String + * lionForm.formElements[f-g].value; // Array + */ +export class FormControlsCollection extends Array { + /** + * @desc Gives back the named keys and filters out array indexes + */ + keys() { + return Object.keys(this).filter(k => Number.isNaN(Number(k))); + } +} diff --git a/packages/field/src/FormRegisteringMixin.js b/packages/field/src/registration/FormRegisteringMixin.js similarity index 100% rename from packages/field/src/FormRegisteringMixin.js rename to packages/field/src/registration/FormRegisteringMixin.js diff --git a/packages/field/src/registration/FormRegistrarMixin.js b/packages/field/src/registration/FormRegistrarMixin.js new file mode 100644 index 000000000..e98bf0a28 --- /dev/null +++ b/packages/field/src/registration/FormRegistrarMixin.js @@ -0,0 +1,184 @@ +// eslint-disable-next-line max-classes-per-file +import { dedupeMixin } from '@lion/core'; +import { FormRegisteringMixin } from './FormRegisteringMixin.js'; +import { formRegistrarManager } from './formRegistrarManager.js'; +import { FormControlsCollection } from './FormControlsCollection.js'; + +// TODO: rename .formElements to .formControls? (or .$controls ?) + +/** + * @desc This allows an element to become the manager of a register. + * It basically keeps track of a FormControlsCollection that it stores in .formElements + * This will always be an array of all elements. + * In case of a form or fieldset(sub form), it will also act as a key based object with FormControl + * (fields, choice groups or fieldsets)as keys. + * For choice groups, the value will only stay an array. + * See FormControlsCollection for more information + */ +export const FormRegistrarMixin = dedupeMixin( + superclass => + // eslint-disable-next-line no-shadow, no-unused-vars + class FormRegistrarMixin extends FormRegisteringMixin(superclass) { + static get properties() { + return { + /** + * @desc Flag that determines how ".formElements" should behave. + * For a regular fieldset (see LionFieldset) we expect ".formElements" + * to be accessible as an object. + * In case of a radio-group, a checkbox-group or a select/listbox, + * it should act like an array (see ChoiceGroupMixin). + * Usually, when false, we deal with a choice-group (radio-group, checkbox-group, + * (multi)select) + * @type {boolean} + */ + _isFormOrFieldset: Boolean, + }; + } + + constructor() { + super(); + this.formElements = new FormControlsCollection(); + + this._isFormOrFieldset = false; + + this.__readyForRegistration = false; + this.__hasBeenRendered = false; + this.registrationReady = new Promise(resolve => { + this.__resolveRegistrationReady = resolve; + }); + + this._onRequestToAddFormElement = this._onRequestToAddFormElement.bind(this); + this.addEventListener('form-element-register', this._onRequestToAddFormElement); + } + + connectedCallback() { + if (super.connectedCallback) { + super.connectedCallback(); + } + formRegistrarManager.add(this); + if (this.__hasBeenRendered) { + formRegistrarManager.becomesReady(); + } + } + + disconnectedCallback() { + if (super.disconnectedCallback) { + super.disconnectedCallback(); + } + formRegistrarManager.remove(this); + } + + isRegisteredFormElement(el) { + return this.formElements.some(exitingEl => exitingEl === el); + } + + firstUpdated(changedProperties) { + super.firstUpdated(changedProperties); + this.__resolveRegistrationReady(); + this.__readyForRegistration = true; + formRegistrarManager.becomesReady(); + this.__hasBeenRendered = true; + } + + addFormElement(child, indexToInsertAt) { + // This is a way to let the child element (a lion-fieldset or lion-field) know, about its parent + // eslint-disable-next-line no-param-reassign + child.__parentFormGroup = this; + + // 1. Add children as array element + if (indexToInsertAt > 0) { + this.formElements.splice(indexToInsertAt, 0, child); + } else { + this.formElements.push(child); + } + + // 2. Add children as object key + if (this._isFormOrFieldset) { + const { name } = child; + if (!name) { + console.info('Error Node:', child); // eslint-disable-line no-console + throw new TypeError('You need to define a name'); + } + if (name === this.name) { + console.info('Error Node:', child); // eslint-disable-line no-console + throw new TypeError(`You can not have the same name "${name}" as your parent`); + } + + if (name.substr(-2) === '[]') { + if (!Array.isArray(this.formElements[name])) { + this.formElements[name] = new FormControlsCollection(); + } + if (indexToInsertAt > 0) { + this.formElements[name].splice(indexToInsertAt, 0, child); + } else { + this.formElements[name].push(child); + } + } else if (!this.formElements[name]) { + this.formElements[name] = child; + } else { + console.info('Error Node:', child); // eslint-disable-line no-console + throw new TypeError( + `Name "${name}" is already registered - if you want an array add [] to the end`, + ); + } + } + } + + removeFormElement(child) { + // 1. Handle array based children + const index = this.formElements.indexOf(child); + if (index > -1) { + this.formElements.splice(index, 1); + } + + // 2. Handle name based object keys + if (this._isFormOrFieldset) { + const { name } = child; + if (name.substr(-2) === '[]' && this.formElements[name]) { + const idx = this.formElements[name].indexOf(child); + if (idx > -1) { + this.formElements[name].splice(idx, 1); + } + } else if (this.formElements[name]) { + delete this.formElements[name]; + } + } + } + + _onRequestToAddFormElement(ev) { + const child = ev.detail.element; + if (child === this) { + // as we fire and listen - don't add ourselves + return; + } + if (this.isRegisteredFormElement(child)) { + // do not readd already existing elements + return; + } + ev.stopPropagation(); + + // Check for siblings to determine the right order to insert into formElements + // If there is no next sibling, index is -1 + let indexToInsertAt = -1; + if (this.formElements && Array.isArray(this.formElements)) { + indexToInsertAt = this.formElements.indexOf(child.nextElementSibling); + } + this.addFormElement(child, indexToInsertAt); + } + + _onRequestToRemoveFormElement(ev) { + const child = ev.detail.element; + if (child === this) { + // as we fire and listen - don't remove ourselves + return; + } + if (!this.isRegisteredFormElement(child)) { + // do not remove non existing elements + return; + } + ev.stopPropagation(); + + this.removeFormElement(child); + } + }, +); diff --git a/packages/field/src/FormRegistrarPortalMixin.js b/packages/field/src/registration/FormRegistrarPortalMixin.js similarity index 100% rename from packages/field/src/FormRegistrarPortalMixin.js rename to packages/field/src/registration/FormRegistrarPortalMixin.js diff --git a/packages/field/src/formRegistrarManager.js b/packages/field/src/registration/formRegistrarManager.js similarity index 100% rename from packages/field/src/formRegistrarManager.js rename to packages/field/src/registration/formRegistrarManager.js diff --git a/packages/field/test-suites/FormRegistrationMixins.suite.js b/packages/field/test-suites/FormRegistrationMixins.suite.js index 871dfd4a3..6d06949d9 100644 --- a/packages/field/test-suites/FormRegistrationMixins.suite.js +++ b/packages/field/test-suites/FormRegistrationMixins.suite.js @@ -1,19 +1,18 @@ import { LitElement } from '@lion/core'; import { defineCE, expect, fixture, html, unsafeStatic } from '@open-wc/testing'; import sinon from 'sinon'; -import { FormRegisteringMixin } from '../src/FormRegisteringMixin.js'; -import { formRegistrarManager } from '../src/formRegistrarManager.js'; -import { FormRegistrarMixin } from '../src/FormRegistrarMixin.js'; -import { FormRegistrarPortalMixin } from '../src/FormRegistrarPortalMixin.js'; +import { FormRegisteringMixin } from '../src/registration/FormRegisteringMixin.js'; +import { formRegistrarManager } from '../src/registration/formRegistrarManager.js'; +import { FormRegistrarMixin } from '../src/registration/FormRegistrarMixin.js'; +import { FormRegistrarPortalMixin } from '../src/registration/FormRegistrarPortalMixin.js'; export const runRegistrationSuite = customConfig => { const cfg = { baseElement: HTMLElement, - suffix: null, ...customConfig, }; - describe(`FormRegistrationMixins${cfg.suffix ? ` (${cfg.suffix})` : ''}`, () => { + describe('FormRegistrationMixins', () => { let parentTag; let childTag; let portalTag; diff --git a/packages/fieldset/index.js b/packages/fieldset/index.js index ea598fcfd..5b8206ecb 100644 --- a/packages/fieldset/index.js +++ b/packages/fieldset/index.js @@ -1 +1,2 @@ export { LionFieldset } from './src/LionFieldset.js'; +export { FormGroupMixin } from './src/FormGroupMixin.js'; diff --git a/packages/fieldset/src/FormGroupMixin.js b/packages/fieldset/src/FormGroupMixin.js new file mode 100644 index 000000000..1b3b771ba --- /dev/null +++ b/packages/fieldset/src/FormGroupMixin.js @@ -0,0 +1,403 @@ +import { html, dedupeMixin, SlotMixin } from '@lion/core'; +import { DisabledMixin } from '@lion/core/src/DisabledMixin.js'; +import { FormControlMixin, FormRegistrarMixin, FormControlsCollection } from '@lion/field'; +import { getAriaElementsInRightDomOrder } from '@lion/field/src/utils/getAriaElementsInRightDomOrder.js'; +import { ValidateMixin } from '@lion/validate'; +import { FormElementsHaveNoError } from './FormElementsHaveNoError.js'; + +/** + * @desc Form group mixin serves as the basis for (sub) forms. Designed to be put on + * elements with role=group (or radiogroup) + * It bridges all the functionality of the child form controls: + * ValidateMixin, InteractionStateMixin, FormatMixin, FormControlMixin etc. + * It is designed to be used on top of FormRegstrarMixin and ChoiceGroupMixin + * Also, the LionFieldset element (which supports name based retrieval of children via formElements + * and the automatic grouping of formElements via '[]') + * + */ +export const FormGroupMixin = dedupeMixin( + superclass => + // eslint-disable-next-line no-shadow + class FormGroupMixin extends FormRegistrarMixin( + FormControlMixin(ValidateMixin(DisabledMixin(SlotMixin(superclass)))), + ) { + static get properties() { + return { + /** + * Interaction state that can be used to compute the visibility of + * feedback messages + */ + // TODO: Move property submitted to InteractionStateMixin. + submitted: { + type: Boolean, + reflect: true, + }, + /** + * Interaction state that will be active when any of the children + * is focused. + */ + focused: { + type: Boolean, + reflect: true, + }, + /** + * Interaction state that will be active when any of the children + * is dirty (see InteractionStateMixin for more details.) + */ + dirty: { + type: Boolean, + reflect: true, + }, + /** + * Interaction state that will be active when the group as a whole is + * blurred + */ + touched: { + type: Boolean, + reflect: true, + }, + /** + * Interaction state that will be active when all of the children + * are prefilled (see InteractionStateMixin for more details.) + */ + prefilled: { + type: Boolean, + reflect: true, + }, + }; + } + + get _inputNode() { + return this; + } + + get modelValue() { + return this._getFromAllFormElements('modelValue'); + } + + set modelValue(values) { + this._setValueMapForAllFormElements('modelValue', values); + } + + get serializedValue() { + return this._getFromAllFormElements('serializedValue'); + } + + set serializedValue(values) { + this._setValueMapForAllFormElements('serializedValue', values); + } + + get formattedValue() { + return this._getFromAllFormElements('formattedValue'); + } + + set formattedValue(values) { + this._setValueMapForAllFormElements('formattedValue', values); + } + + // TODO: should it be any or every? Should we maybe keep track of both, + // so we can configure feedback visibility depending on scenario? + // Should we allow configuring feedback visibility on validator instances + // for maximal flexibility? + // Document this... + get prefilled() { + return this._everyFormElementHas('prefilled'); + } + + constructor() { + super(); + this.disabled = false; + this.submitted = false; + this.dirty = false; + this.touched = false; + this.focused = false; + this.__addedSubValidators = false; + + this._checkForOutsideClick = this._checkForOutsideClick.bind(this); + + this.addEventListener('focusin', this._syncFocused); + this.addEventListener('focusout', this._onFocusOut); + this.addEventListener('dirty-changed', this._syncDirty); + this.addEventListener('validate-performed', this.__onChildValidatePerformed); + + this.defaultValidators = [new FormElementsHaveNoError()]; + } + + connectedCallback() { + // eslint-disable-next-line wc/guard-super-call + super.connectedCallback(); + this.setAttribute('role', 'group'); + this.__initInteractionStates(); + } + + disconnectedCallback() { + super.disconnectedCallback(); // eslint-disable-line wc/guard-super-call + + if (this.__hasActiveOutsideClickHandling) { + document.removeEventListener('click', this._checkForOutsideClick); + this.__hasActiveOutsideClickHandling = false; + } + } + + async __initInteractionStates() { + if (!this.__readyForRegistration) { + await this.registrationReady; + } + this.formElements.forEach(el => { + if (typeof el.initInteractionState === 'function') { + el.initInteractionState(); + } + }); + } + + updated(changedProps) { + super.updated(changedProps); + + if (changedProps.has('disabled')) { + if (this.disabled) { + this.__requestChildrenToBeDisabled(); + } else { + this.__retractRequestChildrenToBeDisabled(); + } + } + + if (changedProps.has('focused')) { + if (this.focused === true) { + this.__setupOutsideClickHandling(); + } + } + } + + __setupOutsideClickHandling() { + if (!this.__hasActiveOutsideClickHandling) { + document.addEventListener('click', this._checkForOutsideClick); + this.__hasActiveOutsideClickHandling = true; + } + } + + _checkForOutsideClick(event) { + const outsideGroupClicked = !this.contains(event.target); + if (outsideGroupClicked) { + this.touched = true; + } + } + + __requestChildrenToBeDisabled() { + this.formElements.forEach(child => { + if (child.makeRequestToBeDisabled) { + child.makeRequestToBeDisabled(); + } + }); + } + + __retractRequestChildrenToBeDisabled() { + this.formElements.forEach(child => { + if (child.retractRequestToBeDisabled) { + child.retractRequestToBeDisabled(); + } + }); + } + + // eslint-disable-next-line class-methods-use-this + inputGroupTemplate() { + return html` +
+ +
+ `; + } + + /** + * @desc Handles interaction state 'submitted'. + * This allows children to enable visibility of validation feedback + */ + submitGroup() { + this.submitted = true; + this.formElements.forEach(child => { + if (typeof child.submitGroup === 'function') { + child.submitGroup(); + } else { + child.submitted = true; // eslint-disable-line no-param-reassign + } + }); + } + + resetGroup() { + this.formElements.forEach(child => { + if (typeof child.resetGroup === 'function') { + child.resetGroup(); + } else if (typeof child.reset === 'function') { + child.reset(); + } + }); + + this.resetInteractionState(); + } + + resetInteractionState() { + this.submitted = false; + this.touched = false; + this.dirty = false; + this.formElements.forEach(formElement => { + if (typeof formElement.resetInteractionState === 'function') { + formElement.resetInteractionState(); + } + }); + } + + _getFromAllFormElements(property, filterCondition = el => !el.disabled) { + const result = {}; + this.formElements.keys().forEach(name => { + const elem = this.formElements[name]; + if (elem instanceof FormControlsCollection) { + result[name] = elem.filter(el => filterCondition(el)).map(el => el[property]); + } else if (filterCondition(elem)) { + if (typeof elem._getFromAllFormElements === 'function') { + result[name] = elem._getFromAllFormElements(property, filterCondition); + } else { + result[name] = elem[property]; + } + } + }); + return result; + } + + _setValueForAllFormElements(property, value) { + this.formElements.forEach(el => { + el[property] = value; // eslint-disable-line no-param-reassign + }); + } + + async _setValueMapForAllFormElements(property, values) { + if (!this.__readyForRegistration) { + await this.registrationReady; + } + + if (values && typeof values === 'object') { + Object.keys(values).forEach(name => { + if (Array.isArray(this.formElements[name])) { + this.formElements[name].forEach((el, index) => { + el[property] = values[name][index]; // eslint-disable-line no-param-reassign + }); + } + this.formElements[name][property] = values[name]; + }); + } + } + + _anyFormElementHas(property) { + return Object.keys(this.formElements).some(name => { + if (Array.isArray(this.formElements[name])) { + return this.formElements[name].some(el => !!el[property]); + } + return !!this.formElements[name][property]; + }); + } + + _anyFormElementHasFeedbackFor(state) { + return Object.keys(this.formElements).some(name => { + if (Array.isArray(this.formElements[name])) { + return this.formElements[name].some(el => !!el.hasFeedbackFor.includes(state)); + } + return !!this.formElements[name].hasFeedbackFor.includes(state); + }); + } + + _everyFormElementHas(property) { + return Object.keys(this.formElements).every(name => { + if (Array.isArray(this.formElements[name])) { + return this.formElements[name].every(el => !!el[property]); + } + return !!this.formElements[name][property]; + }); + } + + /** + * Gets triggered by event 'validate-performed' which enabled us to handle 2 different situations + * - react on modelValue change, which says something about the validity as a whole + * (at least two checkboxes for instance) and nothing about the children's values + * - children validity states have changed, so fieldset needs to update itself based on that + */ + __onChildValidatePerformed(ev) { + if (ev && this.isRegisteredFormElement(ev.target)) { + this.validate(); + } + } + + _syncFocused() { + this.focused = this._anyFormElementHas('focused'); + } + + _onFocusOut(ev) { + const lastEl = this.formElements[this.formElements.length - 1]; + if (ev.target === lastEl) { + this.touched = true; + } + this.focused = false; + } + + _syncDirty() { + this.dirty = this._anyFormElementHas('dirty'); + } + + __linkChildrenMessagesToParent(child) { + // aria-describedby of (nested) children + let parent = this; + while (parent) { + this.constructor._addDescriptionElementIdsToField( + child, + parent._getAriaDescriptionElements(), + ); + // Also check if the newly added child needs to refer grandparents + parent = parent.__parentFormGroup; + } + } + + /** + * @override of FormRegistrarMixin. + * @desc Connects ValidateMixin and DisabledMixin + * On top of this, error messages of children are linked to their parents + */ + addFormElement(child, indexToInsertAt) { + super.addFormElement(child, indexToInsertAt); + if (this.disabled) { + // eslint-disable-next-line no-param-reassign + child.makeRequestToBeDisabled(); + } + // TODO: Unlink in removeFormElement + this.__linkChildrenMessagesToParent(child); + this.validate(); + } + + /** + * Gathers initial model values of all children. Used + * when resetGroup() is called. + */ + get _initialModelValue() { + return this._getFromAllFormElements('_initialModelValue'); + } + + /** + * Add aria-describedby to child element(field), so that it points to feedback/help-text of + * parent(fieldset) + * @param {LionField} field - the child: lion-field/lion-input/lion-textarea + * @param {array} descriptionElements - description elements like feedback and help-text + */ + static _addDescriptionElementIdsToField(field, descriptionElements) { + const orderedEls = getAriaElementsInRightDomOrder(descriptionElements, { reverse: true }); + orderedEls.forEach(el => { + if (field.addToAriaDescribedBy) { + field.addToAriaDescribedBy(el, { reorder: false }); + } + }); + } + + /** + * @override of FormRegistrarMixin. Connects ValidateMixin + */ + removeFormElement(...args) { + super.removeFormElement(...args); + this.validate(); + } + }, +); diff --git a/packages/fieldset/src/LionFieldset.js b/packages/fieldset/src/LionFieldset.js index ece64a2a7..9ea5829d1 100644 --- a/packages/fieldset/src/LionFieldset.js +++ b/packages/fieldset/src/LionFieldset.js @@ -1,473 +1,29 @@ -import { html, LitElement, SlotMixin } from '@lion/core'; -import { DisabledMixin } from '@lion/core/src/DisabledMixin.js'; -import { FormControlMixin, FormRegistrarMixin } from '@lion/field'; -import { getAriaElementsInRightDomOrder } from '@lion/field/src/utils/getAriaElementsInRightDomOrder.js'; -import { ValidateMixin } from '@lion/validate'; -import { FormElementsHaveNoError } from './FormElementsHaveNoError.js'; +import { LitElement } from '@lion/core'; +import { FormGroupMixin } from './FormGroupMixin.js'; /** - * LionFieldset: fieldset wrapper providing extra features and integration with lion-field elements. + * @desc LionFieldset is basically a 'sub form' and can have its own nested sub forms. + * It mimics the native
element in this sense, but has all the functionality of + * a FormControl (advanced styling, validation, interaction states etc.) Also see + * FormGroupMixin it depends on. + * + * LionFieldset enables the '_isFormOrFieldset' flag in FormRegistrarMixin. This makes .formElements + * act not only as an array, but also as an object (see FormRegistarMixin for more information). + * As a bonus, It can also group children having names ending with '[]'. + * + * Above will be helpful for both forms and sub forms, which can contain sub forms as children + * as well and allow for a nested form structure. + * Contrary, other form groups (choice groups like radio-group, checkbox-group and (multi)select) + * don't: they should be considered 'end nodes' or 'leaves' of the form and their children/formElements + * cannot be accessed individually via object keys. * * @customElement lion-fieldset * @extends {LitElement} */ -export class LionFieldset extends FormRegistrarMixin( - FormControlMixin(ValidateMixin(DisabledMixin(SlotMixin(LitElement)))), -) { - static get properties() { - return { - name: { - type: String, - }, - // TODO: Move property submitted to InteractionStateMixin. - submitted: { - type: Boolean, - reflect: true, - }, - focused: { - type: Boolean, - reflect: true, - }, - dirty: { - type: Boolean, - reflect: true, - }, - touched: { - type: Boolean, - reflect: true, - }, - }; - } - - get touched() { - return this.__touched; - } - - set touched(value) { - const oldVal = this.__touched; - this.__touched = value; - this.requestUpdate('touched', oldVal); - } - - get _inputNode() { - return this; - } - - get modelValue() { - return this._getFromAllFormElements('modelValue'); - } - - set modelValue(values) { - this._setValueMapForAllFormElements('modelValue', values); - } - - get serializedValue() { - return this._getFromAllFormElements('serializedValue'); - } - - set serializedValue(values) { - this._setValueMapForAllFormElements('serializedValue', values); - } - - get formattedValue() { - return this._getFromAllFormElements('formattedValue'); - } - - set formattedValue(values) { - this._setValueMapForAllFormElements('formattedValue', values); - } - - get prefilled() { - return this._everyFormElementHas('prefilled'); - } - - get formElementsArray() { - return Object.keys(this.formElements).reduce((result, name) => { - const element = this.formElements[name]; - return result.concat(Array.isArray(element) ? element : [element]); - }, []); - } - - set fieldName(value) { - this.__fieldName = value; - } - - get fieldName() { - const label = - this.label || - (this.querySelector('[slot=label]') && this.querySelector('[slot=label]').textContent); - return this.__fieldName || label || this.name; - } - +export class LionFieldset extends FormGroupMixin(LitElement) { constructor() { super(); - this.disabled = false; - this.submitted = false; - this.dirty = false; - this.touched = false; - this.focused = false; - this.formElements = {}; - this.__addedSubValidators = false; - - this._checkForOutsideClick = this._checkForOutsideClick.bind(this); - - this.addEventListener('focusin', this._syncFocused); - this.addEventListener('focusout', this._onFocusOut); - this.addEventListener('dirty-changed', this._syncDirty); - this.addEventListener('validate-performed', this.__validate); - - this.defaultValidators = [new FormElementsHaveNoError()]; - } - - connectedCallback() { - // eslint-disable-next-line wc/guard-super-call - super.connectedCallback(); - this._setRole(); - } - - disconnectedCallback() { - super.disconnectedCallback(); // eslint-disable-line wc/guard-super-call - - if (this.__hasActiveOutsideClickHandling) { - document.removeEventListener('click', this._checkForOutsideClick); - this.__hasActiveOutsideClickHandling = false; - } - } - - updated(changedProps) { - super.updated(changedProps); - - if (changedProps.has('disabled')) { - if (this.disabled) { - this.__requestChildrenToBeDisabled(); - } else { - this.__retractRequestChildrenToBeDisabled(); - } - } - - if (changedProps.has('focused')) { - if (this.focused === true) { - this.__setupOutsideClickHandling(); - } - } - } - - __setupOutsideClickHandling() { - if (!this.__hasActiveOutsideClickHandling) { - document.addEventListener('click', this._checkForOutsideClick); - this.__hasActiveOutsideClickHandling = true; - } - } - - _checkForOutsideClick(event) { - const outsideGroupClicked = !this.contains(event.target); - if (outsideGroupClicked) { - this.touched = true; - } - } - - __requestChildrenToBeDisabled() { - this.formElementsArray.forEach(child => { - if (child.makeRequestToBeDisabled) { - child.makeRequestToBeDisabled(); - } - }); - } - - __retractRequestChildrenToBeDisabled() { - this.formElementsArray.forEach(child => { - if (child.retractRequestToBeDisabled) { - child.retractRequestToBeDisabled(); - } - }); - } - - // eslint-disable-next-line class-methods-use-this - inputGroupTemplate() { - return html` -
- -
- `; - } - - submitGroup() { - this.submitted = true; - this.formElementsArray.forEach(child => { - if (typeof child.submitGroup === 'function') { - child.submitGroup(); - } else { - child.submitted = true; // eslint-disable-line no-param-reassign - } - }); - } - - serializeGroup() { - const childrenNames = Object.keys(this.formElements); - const serializedValues = childrenNames.length > 0 ? {} : undefined; - childrenNames.forEach(name => { - const element = this.formElements[name]; - if (Array.isArray(element)) { - serializedValues[name] = this.__serializeElements(element); - } else { - const serializedValue = this.__serializeElement(element); - if (serializedValue || serializedValue === 0) { - serializedValues[name] = serializedValue; - } - } - }); - return serializedValues; - } - - resetGroup() { - this.formElementsArray.forEach(child => { - if (typeof child.resetGroup === 'function') { - child.resetGroup(); - } else if (typeof child.reset === 'function') { - child.reset(); - } - }); - - this.resetInteractionState(); - } - - resetInteractionState() { - this.submitted = false; - this.touched = false; - this.dirty = false; - this.formElementsArray.forEach(formElement => { - if (typeof formElement.resetInteractionState === 'function') { - formElement.resetInteractionState(); - } - }); - } - - _getFromAllFormElements(property) { - if (!this.formElements) { - return undefined; - } - const childrenNames = Object.keys(this.formElements); - const values = {}; - childrenNames.forEach(name => { - if (Array.isArray(this.formElements[name])) { - // grouped via myName[] - values[name] = this.formElements[name].map(node => node.modelValue); - } else { - // not grouped - values[name] = this.formElements[name][property]; - } - }); - return values; - } - - _setValueForAllFormElements(property, value) { - this.formElementsArray.forEach(el => { - el[property] = value; // eslint-disable-line no-param-reassign - }); - } - - async _setValueMapForAllFormElements(property, values) { - if (!this.__readyForRegistration) { - await this.registrationReady; - } - - if (values && typeof values === 'object') { - Object.keys(values).forEach(name => { - if (Array.isArray(this.formElements[name])) { - this.formElements[name].forEach((el, index) => { - el[property] = values[name][index]; // eslint-disable-line no-param-reassign - }); - } - this.formElements[name][property] = values[name]; - }); - } - } - - _anyFormElementHas(property) { - return Object.keys(this.formElements).some(name => { - if (Array.isArray(this.formElements[name])) { - return this.formElements[name].some(el => !!el[property]); - } - return !!this.formElements[name][property]; - }); - } - - _anyFormElementHasFeedbackFor(state) { - return Object.keys(this.formElements).some(name => { - if (Array.isArray(this.formElements[name])) { - return this.formElements[name].some(el => !!el.hasFeedbackFor.includes(state)); - } - return !!this.formElements[name].hasFeedbackFor.includes(state); - }); - } - - _everyFormElementHas(property) { - return Object.keys(this.formElements).every(name => { - if (Array.isArray(this.formElements[name])) { - return this.formElements[name].every(el => !!el[property]); - } - return !!this.formElements[name][property]; - }); - } - - /** - * Gets triggered by event 'validate-performed' which enabled us to handle 2 different situations - * - react on modelValue change, which says something about the validity as a whole - * (at least two checkboxes for instance) and nothing about the children's values - * - children validity states have changed, so fieldset needs to update itself based on that - */ - __validate(ev) { - if (ev && this.isRegisteredFormElement(ev.target)) { - this.validate(); - } - } - - _syncFocused() { - this.focused = this._anyFormElementHas('focused'); - } - - _onFocusOut(ev) { - const lastEl = this.formElementsArray[this.formElementsArray.length - 1]; - if (ev.target === lastEl) { - this.touched = true; - } - this.focused = false; - } - - _syncDirty() { - this.dirty = this._anyFormElementHas('dirty'); - } - - _setRole(role) { - this.setAttribute('role', role || 'group'); - } - - // eslint-disable-next-line class-methods-use-this - __serializeElement(element) { - if (!element.disabled) { - if (typeof element.serializeGroup === 'function') { - return element.serializeGroup(); - } - return element.serializedValue; - } - return undefined; - } - - __serializeElements(elements) { - const serializedValues = []; - elements.forEach(element => { - const serializedValue = this.__serializeElement(element); - if (serializedValue || serializedValue === 0) { - serializedValues.push(serializedValue); - } - }); - return serializedValues; - } - - /** - * Adds the element to an object with the child name as a key - * Note: this is different to the default behavior of just beeing an array - * - * @override - */ - addFormElement(child, indexToInsertAt) { - const { name } = child; - if (!name) { - console.info('Error Node:', child); // eslint-disable-line no-console - throw new TypeError('You need to define a name'); - } - if (name === this.name && !this._childrenCanHaveSameName) { - console.info('Error Node:', child); // eslint-disable-line no-console - throw new TypeError(`You can not have the same name "${name}" as your parent`); - } - - if (this.disabled) { - // eslint-disable-next-line no-param-reassign - child.makeRequestToBeDisabled(); - } - - if (name.substr(-2) === '[]' || this._childNamesCanBeDuplicate) { - if (!Array.isArray(this.formElements[name])) { - this.formElements[name] = []; - } - if (indexToInsertAt > 0) { - this.formElements[name].splice(indexToInsertAt, 0, child); - } else { - this.formElements[name].push(child); - } - } else if (!this.formElements[name]) { - this.formElements[name] = child; - } else { - console.info('Error Node:', child); // eslint-disable-line no-console - throw new TypeError( - `Name "${name}" is already registered - if you want an array add [] to the end`, - ); - } - - // This is a way to let the child element (a lion-fieldset or lion-field) know, about its parent - // eslint-disable-next-line no-param-reassign - child.__parentFormGroup = this; - - // aria-describedby of (nested) children - - // TODO: Teardown in removeFormElement - - let parent = this; - while (parent) { - this.constructor._addDescriptionElementIdsToField( - child, - parent._getAriaDescriptionElements(), - ); - // Also check if the newly added child needs to refer grandparents - parent = parent.__parentFormGroup; - } - - this.validate(); - } - - // eslint-disable-next-line class-methods-use-this - get _childrenCanHaveSameName() { - return false; - } - - // eslint-disable-next-line class-methods-use-this - get _childNamesCanBeDuplicate() { - return false; - } - - /** - * Gathers initial model values of all children. Used - * when resetGroup() is called. - */ - get _initialModelValue() { - return this._getFromAllFormElements('_initialModelValue'); - } - - /** - * Add aria-describedby to child element(field), so that it points to feedback/help-text of - * parent(fieldset) - * @param {LionField} field - the child: lion-field/lion-input/lion-textarea - * @param {array} descriptionElements - description elements like feedback and help-text - */ - static _addDescriptionElementIdsToField(field, descriptionElements) { - const orderedEls = getAriaElementsInRightDomOrder(descriptionElements, { reverse: true }); - orderedEls.forEach(el => { - if (field.addToAriaDescribedBy) { - field.addToAriaDescribedBy(el, { reorder: false }); - } - }); - } - - removeFormElement(child) { - const { name } = child; - if (name.substr(-2) === '[]' && this.formElements[name]) { - const index = this.formElements[name].indexOf(child); - if (index > -1) { - this.formElements[name].splice(index, 1); - } - } else if (this.formElements[name]) { - delete this.formElements[name]; - } - this.validate(); + /** @override from FormRegistrarMixin */ + this._isFormOrFieldset = true; } } diff --git a/packages/fieldset/test/lion-fieldset.test.js b/packages/fieldset/test/lion-fieldset.test.js index 568c5d569..497ae31b2 100644 --- a/packages/fieldset/test/lion-fieldset.test.js +++ b/packages/fieldset/test/lion-fieldset.test.js @@ -1,6 +1,7 @@ import { expect, fixture, + fixtureSync, html, unsafeStatic, triggerFocusFor, @@ -39,7 +40,9 @@ beforeEach(() => { localizeTearDown(); }); +// TODO: seperate fieldset and FormGroup tests describe('', () => { + // TODO: Tests below belong to FormControlMixin. Preferably run suite integration test it(`has a fieldName based on the label`, async () => { const el1 = await fixture(html`<${tag} label="foo">${inputSlots}`); expect(el1.fieldName).to.equal(el1._labelNode.textContent); @@ -60,13 +63,15 @@ describe('', () => { expect(el.__fieldName).to.equal(el.fieldName); }); - it(`${tagString} has an up to date list of every form element in #formElements`, async () => { + // TODO: Tests below belong to FormRegistrarMixin. Preferably run suite integration test + + it(`${tagString} has an up to date list of every form element in .formElements`, async () => { const el = await fixture(html`<${tag}>${inputSlots}`); await nextFrame(); - expect(Object.keys(el.formElements).length).to.equal(3); + expect(el.formElements.keys().length).to.equal(3); expect(el.formElements['hobbies[]'].length).to.equal(2); el.removeChild(el.formElements['hobbies[]'][0]); - expect(Object.keys(el.formElements).length).to.equal(3); + expect(el.formElements.keys().length).to.equal(3); expect(el.formElements['hobbies[]'].length).to.equal(1); }); @@ -79,9 +84,9 @@ describe('', () => { `); await nextFrame(); - expect(el.formElementsArray.length).to.equal(1); + expect(el.formElements.length).to.equal(1); el.children[0].removeChild(el.formElements.foo); - expect(el.formElementsArray.length).to.equal(0); + expect(el.formElements.length).to.equal(0); }); it('handles names with ending [] as an array', async () => { @@ -91,7 +96,7 @@ describe('', () => { el.formElements['hobbies[]'][0].modelValue = { checked: false, value: 'chess' }; el.formElements['hobbies[]'][1].modelValue = { checked: false, value: 'rugby' }; - expect(Object.keys(el.formElements).length).to.equal(3); + expect(el.formElements.keys().length).to.equal(3); expect(el.formElements['hobbies[]'].length).to.equal(2); expect(el.formElements['hobbies[]'][0].modelValue.value).to.equal('chess'); expect(el.formElements['gender[]'][0].modelValue.value).to.equal('male'); @@ -166,15 +171,17 @@ describe('', () => { const el = await fixture(html`<${tag}>${inputSlots}`); const newField = await fixture(html`<${childTag} name="lastName">`); - expect(Object.keys(el.formElements).length).to.equal(3); + expect(el.formElements.keys().length).to.equal(3); el.appendChild(newField); - expect(Object.keys(el.formElements).length).to.equal(4); + expect(el.formElements.keys().length).to.equal(4); el._inputNode.removeChild(newField); - expect(Object.keys(el.formElements).length).to.equal(3); + expect(el.formElements.keys().length).to.equal(3); }); + // TODO: Tests below belong to FormGroupMixin. Preferably run suite integration test + it('can read/write all values (of every input) via this.modelValue', async () => { const el = await fixture(html` <${tag}> @@ -231,6 +238,32 @@ describe('', () => { expect(el.formElements.lastName.modelValue).to.equal(2); }); + it('does not list disabled values in this.modelValue', async () => { + const el = await fixture(html` + <${tag}> + <${childTag} name="a" disabled .modelValue="${'x'}"> + <${childTag} name="b" .modelValue="${'x'}"> + <${tag} name="newFieldset"> + <${childTag} name="c" .modelValue="${'x'}"> + <${childTag} name="d" disabled .modelValue="${'x'}"> + + <${tag} name="disabledFieldset" disabled> + <${childTag} name="e" .modelValue="${'x'}"> + + + `); + await el.registrationReady; + const newFieldset = el.querySelector('lion-fieldset'); + await newFieldset.registrationReady; + + expect(el.modelValue).to.deep.equal({ + b: 'x', + newFieldset: { + c: 'x', + }, + }); + }); + it('does not throw if setter data of this.modelValue can not be handled', async () => { const el = await fixture(html` <${tag}> @@ -308,7 +341,7 @@ describe('', () => { expect(el.modelValue).to.eql(initialSerializedValue); }); - describe('validation', () => { + describe('Validation', () => { it('validates on init', async () => { class IsCat extends Validator { constructor() { @@ -409,7 +442,7 @@ describe('', () => { }); }); - describe('interaction states', () => { + describe('Interaction states', () => { it('has false states (dirty, touched, prefilled) on init', async () => { const fieldset = await fixture(html`<${tag}>${inputSlots}`); await nextFrame(); @@ -453,7 +486,7 @@ describe('', () => { it('becomes prefilled if all form elements are prefilled', async () => { const el = await fixture(html` <${tag}> - <${childTag} name="input1" prefilled> + <${childTag} name="input1" .modelValue="${'prefilled'}"> <${childTag} name="input2"> `); @@ -462,8 +495,8 @@ describe('', () => { const el2 = await fixture(html` <${tag}> - <${childTag} name="input1" prefilled> - <${childTag} name="input2" prefilled> + <${childTag} name="input1" .modelValue="${'prefilled'}"> + <${childTag} name="input2" .modelValue="${'prefilled'}"> `); await nextFrame(); @@ -515,7 +548,7 @@ describe('', () => { `); outside.click(); - expect(el.touched, 'unfocused fieldset should stays untouched').to.be.false; + expect(el.touched, 'unfocused fieldset should stay untouched').to.be.false; el.children[1].focus(); el.children[2].focus(); @@ -524,7 +557,6 @@ describe('', () => { outside.click(); // blur the group via a click outside.focus(); // a real mouse click moves focus as well expect(el.touched).to.be.true; - expect(el2.touched).to.be.false; }); @@ -591,9 +623,30 @@ describe('', () => { expect(el.validationStates.error.Input1IsTen).to.be.true; expect(el.hasFeedbackFor).to.deep.equal(['error']); }); + + it.skip('(re)initializes children interaction states on registration ready', async () => { + const fieldset = await fixtureSync(html` + <${tag} .modelValue="${{ a: '1', b: '2' }}"> + <${childTag} name="a"> + <${childTag} name="b"> + `); + const childA = fieldset.querySelector('[name="a"]'); + const childB = fieldset.querySelector('[name="b"]'); + const spyA = sinon.spy(childA, 'initInteractionState'); + const spyB = sinon.spy(childB, 'initInteractionState'); + expect(fieldset.prefilled).to.be.false; + expect(fieldset.dirty).to.be.false; + await fieldset.registrationReady; + await nextFrame(); + expect(spyA).to.have.been.called; + expect(spyB).to.have.been.called; + expect(fieldset.prefilled).to.be.true; + expect(fieldset.dirty).to.be.false; + }); }); - describe('serialize', () => { + // TODO: this should be tested in FormGroupMixin + describe('serializedValue', () => { it('use form elements serializedValue', async () => { const fieldset = await fixture(html`<${tag}>${inputSlots}`); await nextFrame(); @@ -604,7 +657,7 @@ describe('', () => { fieldset.formElements['gender[]'][1].modelValue = { checked: false, value: 'female' }; fieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; expect(fieldset.formElements['hobbies[]'][0].serializedValue).to.equal('Bar-serialized'); - expect(fieldset.serializeGroup()).to.deep.equal({ + expect(fieldset.serializedValue).to.deep.equal({ 'hobbies[]': ['Bar-serialized', { checked: false, value: 'rugby' }], 'gender[]': [ { checked: false, value: 'male' }, @@ -614,6 +667,27 @@ describe('', () => { }); }); + it('treats names with ending [] as arrays', async () => { + const fieldset = await fixture(html`<${tag}>${inputSlots}`); + await nextFrame(); + fieldset.formElements['hobbies[]'][0].modelValue = { checked: false, value: 'chess' }; + fieldset.formElements['hobbies[]'][1].modelValue = { checked: false, value: 'rugby' }; + fieldset.formElements['gender[]'][0].modelValue = { checked: false, value: 'male' }; + fieldset.formElements['gender[]'][1].modelValue = { checked: false, value: 'female' }; + fieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; + expect(fieldset.serializedValue).to.deep.equal({ + 'hobbies[]': [ + { checked: false, value: 'chess' }, + { checked: false, value: 'rugby' }, + ], + 'gender[]': [ + { checked: false, value: 'male' }, + { checked: false, value: 'female' }, + ], + color: { checked: false, value: 'blue' }, + }); + }); + it('0 is a valid value to be serialized', async () => { const fieldset = await fixture(html` <${tag}> @@ -621,48 +695,22 @@ describe('', () => { `); await nextFrame(); fieldset.formElements.price.modelValue = 0; - expect(fieldset.serializeGroup()).to.deep.equal({ price: 0 }); + expect(fieldset.serializedValue).to.deep.equal({ price: 0 }); }); - it('__serializeElements serializes 0 as a valid value', async () => { - const fieldset = await fixture(html`<${tag}>`); + it.skip('serializes undefined values as ""(nb radios/checkboxes are always serialized)', async () => { + const fieldset = await fixture(html` + <${tag}> + <${childTag} name="custom[]"> + <${childTag} name="custom[]"> + + `); await nextFrame(); - const elements = [{ serializedValue: 0 }]; - expect(fieldset.__serializeElements(elements)).to.deep.equal([0]); - }); + fieldset.formElements['custom[]'][0].modelValue = 'custom 1'; + fieldset.formElements['custom[]'][1].modelValue = undefined; - it('form elements which are not disabled', async () => { - const fieldset = await fixture(html`<${tag}>${inputSlots}`); - await nextFrame(); - fieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; - fieldset.formElements['hobbies[]'][0].modelValue = { checked: true, value: 'football' }; - fieldset.formElements['gender[]'][0].modelValue = { checked: true, value: 'male' }; - fieldset.formElements['hobbies[]'][1].modelValue = { checked: false, value: 'rugby' }; - fieldset.formElements['gender[]'][1].modelValue = { checked: false, value: 'female' }; - fieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; - - expect(fieldset.serializeGroup()).to.deep.equal({ - 'hobbies[]': [ - { checked: true, value: 'football' }, - { checked: false, value: 'rugby' }, - ], - 'gender[]': [ - { checked: true, value: 'male' }, - { checked: false, value: 'female' }, - ], - color: { checked: false, value: 'blue' }, - }); - fieldset.formElements.color.disabled = true; - - expect(fieldset.serializeGroup()).to.deep.equal({ - 'hobbies[]': [ - { checked: true, value: 'football' }, - { checked: false, value: 'rugby' }, - ], - 'gender[]': [ - { checked: true, value: 'male' }, - { checked: false, value: 'female' }, - ], + expect(fieldset.serializedValue).to.deep.equal({ + 'custom[]': ['custom 1', ''], }); }); @@ -681,9 +729,9 @@ describe('', () => { newFieldset.formElements['gender[]'][1].modelValue = { checked: false, value: 'female' }; newFieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; fieldset.formElements.comment.modelValue = 'Foo'; - expect(Object.keys(fieldset.formElements).length).to.equal(2); - expect(Object.keys(newFieldset.formElements).length).to.equal(3); - expect(fieldset.serializeGroup()).to.deep.equal({ + expect(fieldset.formElements.keys().length).to.equal(2); + expect(newFieldset.formElements.keys().length).to.equal(3); + expect(fieldset.serializedValue).to.deep.equal({ comment: 'Foo', newfieldset: { 'hobbies[]': [ @@ -699,7 +747,23 @@ describe('', () => { }); }); - it('will exclude form elements within an disabled fieldset', async () => { + it('does not serialize disabled values', async () => { + const fieldset = await fixture(html` + <${tag}> + <${childTag} name="custom[]"> + <${childTag} name="custom[]"> + + `); + await nextFrame(); + fieldset.formElements['custom[]'][0].modelValue = 'custom 1'; + fieldset.formElements['custom[]'][1].disabled = true; + + expect(fieldset.serializedValue).to.deep.equal({ + 'custom[]': ['custom 1'], + }); + }); + + it('will exclude form elements within a disabled fieldset', async () => { const fieldset = await fixture(html` <${tag} name="userData"> <${childTag} name="comment"> @@ -717,7 +781,7 @@ describe('', () => { newFieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; newFieldset.formElements.color.disabled = true; - expect(fieldset.serializeGroup()).to.deep.equal({ + expect(fieldset.serializedValue).to.deep.equal({ comment: 'Foo', newfieldset: { 'hobbies[]': [ @@ -732,7 +796,7 @@ describe('', () => { }); newFieldset.formElements.color.disabled = false; - expect(fieldset.serializeGroup()).to.deep.equal({ + expect(fieldset.serializedValue).to.deep.equal({ comment: 'Foo', newfieldset: { 'hobbies[]': [ @@ -747,43 +811,6 @@ describe('', () => { }, }); }); - - it('treats names with ending [] as arrays', async () => { - const fieldset = await fixture(html`<${tag}>${inputSlots}`); - await nextFrame(); - fieldset.formElements['hobbies[]'][0].modelValue = { checked: false, value: 'chess' }; - fieldset.formElements['hobbies[]'][1].modelValue = { checked: false, value: 'rugby' }; - fieldset.formElements['gender[]'][0].modelValue = { checked: false, value: 'male' }; - fieldset.formElements['gender[]'][1].modelValue = { checked: false, value: 'female' }; - fieldset.formElements.color.modelValue = { checked: false, value: 'blue' }; - expect(fieldset.serializeGroup()).to.deep.equal({ - 'hobbies[]': [ - { checked: false, value: 'chess' }, - { checked: false, value: 'rugby' }, - ], - 'gender[]': [ - { checked: false, value: 'male' }, - { checked: false, value: 'female' }, - ], - color: { checked: false, value: 'blue' }, - }); - }); - - it('does not serialize undefined values (nb radios/checkboxes are always serialized)', async () => { - const fieldset = await fixture(html` - <${tag}> - <${childTag} name="custom[]"> - <${childTag} name="custom[]"> - - `); - await nextFrame(); - fieldset.formElements['custom[]'][0].modelValue = 'custom 1'; - fieldset.formElements['custom[]'][1].modelValue = undefined; - - expect(fieldset.serializeGroup()).to.deep.equal({ - 'custom[]': ['custom 1'], - }); - }); }); describe('Reset', () => { @@ -883,7 +910,7 @@ describe('', () => { fieldset.submitted = true; fieldset.resetGroup(); expect(fieldset.submitted).to.equal(false); - fieldset.formElementsArray.forEach(el => { + fieldset.formElements.forEach(el => { expect(el.submitted).to.equal(false); }); }); diff --git a/packages/form-system/test/form-integrations.test.js b/packages/form-system/test/form-integrations.test.js new file mode 100644 index 000000000..602cf16ba --- /dev/null +++ b/packages/form-system/test/form-integrations.test.js @@ -0,0 +1,33 @@ +import { expect, fixture, html } from '@open-wc/testing'; +import './helpers/umbrella-form.js'; + +// Test umbrella form +describe('Form Integrations', () => { + it.skip('".serializedValue" returns all non disabled fields based on form structure', async () => { + const el = await fixture( + html` + + `, + ); + const formEl = el._lionFormNode; + expect(formEl.serializedValue).to.eql({ + bio: '', + 'checkers[]': [[]], + comments: '', + date: '2000-12-12', + datepicker: '2020-12-12', + dinosaurs: '', + email: '', + favoriteColor: 'hotpink', + full_name: { + first_name: '', + last_name: '', + }, + iban: '', + lyrics: '1', + money: '', + range: 2.3, + terms: [], + }); + }); +}); diff --git a/packages/form-system/test/helpers/umbrella-form.js b/packages/form-system/test/helpers/umbrella-form.js new file mode 100644 index 000000000..35d1eedb4 --- /dev/null +++ b/packages/form-system/test/helpers/umbrella-form.js @@ -0,0 +1,106 @@ +import { LitElement, html } from '@lion/core'; +import { Required, MinLength } from '@lion/validate'; + +export class UmbrellaForm extends LitElement { + get _lionFormNode() { + return this.shadowRoot.querySelector('lion-form'); + } + + render() { + return html` + +
+ + + + + + + + + + + + + + + + + + + + + + + Red + Hotpink + Teal + + + + + + + + + + +
+ Submit + + ev.currentTarget.parentElement.parentElement.parentElement.resetGroup()} + >Reset +
+
+ + `; + } +} +customElements.define('umbrella-form', UmbrellaForm); diff --git a/packages/radio-group/src/LionRadioGroup.js b/packages/radio-group/src/LionRadioGroup.js index 080441e57..2417b2ea0 100644 --- a/packages/radio-group/src/LionRadioGroup.js +++ b/packages/radio-group/src/LionRadioGroup.js @@ -1,15 +1,16 @@ +import { LitElement } from '@lion/core'; import { ChoiceGroupMixin } from '@lion/choice-input'; -import { LionFieldset } from '@lion/fieldset'; +import { FormGroupMixin } from '@lion/fieldset'; /** * A wrapper around multiple radios. * * @extends {LionFieldset} */ -export class LionRadioGroup extends ChoiceGroupMixin(LionFieldset) { +export class LionRadioGroup extends ChoiceGroupMixin(FormGroupMixin(LitElement)) { connectedCallback() { // eslint-disable-next-line wc/guard-super-call super.connectedCallback(); - this._setRole('radiogroup'); + this.setAttribute('role', 'radiogroup'); } } diff --git a/packages/radio-group/test/lion-radio-group.test.js b/packages/radio-group/test/lion-radio-group.test.js index ddc835977..621e6f50c 100644 --- a/packages/radio-group/test/lion-radio-group.test.js +++ b/packages/radio-group/test/lion-radio-group.test.js @@ -37,9 +37,9 @@ describe('', () => { `); await nextFrame(); - const male = el.formElementsArray[0]; + const male = el.formElements[0]; const maleInput = male.querySelector('input'); - const female = el.formElementsArray[1]; + const female = el.formElements[1]; const femaleInput = female.querySelector('input'); expect(male.checked).to.equal(false); diff --git a/packages/select-rich/src/LionSelectRich.js b/packages/select-rich/src/LionSelectRich.js index d2f3bf120..22dbdda67 100644 --- a/packages/select-rich/src/LionSelectRich.js +++ b/packages/select-rich/src/LionSelectRich.js @@ -1,7 +1,7 @@ import { ChoiceGroupMixin } from '@lion/choice-input'; import { css, html, LitElement, SlotMixin } from '@lion/core'; import { FormControlMixin, FormRegistrarMixin, InteractionStateMixin } from '@lion/field'; -import { formRegistrarManager } from '@lion/field/src/formRegistrarManager.js'; +import { formRegistrarManager } from '@lion/field/src/registration/formRegistrarManager.js'; import { OverlayMixin, withDropdownConfig } from '@lion/overlays'; import { ValidateMixin } from '@lion/validate'; import '../lion-select-invoker.js'; @@ -132,6 +132,11 @@ export class LionSelectRich extends ChoiceGroupMixin( this.requestUpdate('modelValue'); } + // TODO: quick and dirty fix. Should be possible to do it nicer on a higher layer + get serializedValue() { + return this.modelValue; + } + get checkedIndex() { let checkedIndex = -1; this.formElements.forEach((option, i) => { diff --git a/packages/select-rich/test/lion-select-rich.test.js b/packages/select-rich/test/lion-select-rich.test.js index 296b2e515..5704db4c1 100644 --- a/packages/select-rich/test/lion-select-rich.test.js +++ b/packages/select-rich/test/lion-select-rich.test.js @@ -41,15 +41,15 @@ describe('lion-select-rich', () => { `); await nextFrame(); - expect(el.formElementsArray[0].name).to.equal('foo'); - expect(el.formElementsArray[1].name).to.equal('foo'); + expect(el.formElements[0].name).to.equal('foo'); + expect(el.formElements[1].name).to.equal('foo'); const validChild = await fixture(html` Item 3 `); el.appendChild(validChild); - expect(el.formElementsArray[2].name).to.equal('foo'); + expect(el.formElements[2].name).to.equal('foo'); }); it('throws if a child element without a modelValue like { value: "foo", checked: false } tries to register', async () => { @@ -108,7 +108,7 @@ describe('lion-select-rich', () => { `); expect(el.modelValue).to.equal('other'); - expect(el.formElementsArray[2].checked).to.be.true; + expect(el.formElements[2].checked).to.be.true; }); it(`has a fieldName based on the label`, async () => { From 583a1820d616d2801f2701ac92d623e777534c1b Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 20 Feb 2020 11:24:03 +0100 Subject: [PATCH 2/7] fix: by default, serialize undefined values as '' --- packages/field/src/FormatMixin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/field/src/FormatMixin.js b/packages/field/src/FormatMixin.js index 79b0b4758..b5c17cfef 100644 --- a/packages/field/src/FormatMixin.js +++ b/packages/field/src/FormatMixin.js @@ -164,7 +164,7 @@ export const FormatMixin = dedupeMixin( * @returns {String} serializedValue */ serializer(v) { - return v; + return v !== undefined ? v : ''; } /** @@ -175,7 +175,7 @@ export const FormatMixin = dedupeMixin( * @returns {Object} modelValue */ deserializer(v) { - return v; + return v === undefined ? '' : v; } /** From ffa4da9649d2500e399b8dc9e4997636ca6c3713 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 20 Feb 2020 14:43:22 +0100 Subject: [PATCH 3/7] fix(input-date): serialize Date as iso string without time information --- packages/input-date/src/LionInputDate.js | 16 ++++++- .../test/lion-input-date-integrations.test.js | 1 - .../input-date/test/lion-input-date.test.js | 43 +++++++++++++++---- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/input-date/src/LionInputDate.js b/packages/input-date/src/LionInputDate.js index 02a38e68b..78f73f639 100644 --- a/packages/input-date/src/LionInputDate.js +++ b/packages/input-date/src/LionInputDate.js @@ -1,5 +1,4 @@ import { LocalizeMixin, formatDate, parseDate } from '@lion/localize'; -import { FieldCustomMixin } from '@lion/field'; import { LionInput } from '@lion/input'; import { IsDate } from '@lion/validate'; @@ -10,7 +9,7 @@ import { IsDate } from '@lion/validate'; * @customElement lion-input-date * @extends {LionInput} */ -export class LionInputDate extends FieldCustomMixin(LocalizeMixin(LionInput)) { +export class LionInputDate extends LocalizeMixin(LionInput) { static get properties() { return { modelValue: Date, @@ -36,4 +35,17 @@ export class LionInputDate extends FieldCustomMixin(LocalizeMixin(LionInput)) { super.connectedCallback(); this.type = 'text'; } + + // eslint-disable-next-line class-methods-use-this + serializer(modelValue) { + if (!(modelValue instanceof Date)) { + return ''; + } + return modelValue.toISOString().slice(0, 10); + } + + // eslint-disable-next-line class-methods-use-this + deserializer(serializedValue) { + return new Date(serializedValue); + } } diff --git a/packages/input-date/test/lion-input-date-integrations.test.js b/packages/input-date/test/lion-input-date-integrations.test.js index 6558ea8da..a7306a959 100644 --- a/packages/input-date/test/lion-input-date-integrations.test.js +++ b/packages/input-date/test/lion-input-date-integrations.test.js @@ -3,7 +3,6 @@ import { runFormatMixinSuite } from '@lion/field/test-suites/FormatMixin.suite.j import '../lion-input-date.js'; const tagString = 'lion-input-date'; - describe(' integrations', () => { runInteractionStateMixinSuite({ tagString, diff --git a/packages/input-date/test/lion-input-date.test.js b/packages/input-date/test/lion-input-date.test.js index 1b511bc9d..c31f897ff 100644 --- a/packages/input-date/test/lion-input-date.test.js +++ b/packages/input-date/test/lion-input-date.test.js @@ -13,17 +13,29 @@ describe('', () => { }); it('returns undefined when value is empty string', async () => { - const el = await fixture(``); + const el = await fixture( + html` + + `, + ); expect(el.parser('')).to.equal(undefined); }); it('has type="text" to activate default keyboard on mobile with all necessary symbols', async () => { - const el = await fixture(``); + const el = await fixture( + html` + + `, + ); expect(el._inputNode.type).to.equal('text'); }); it('has validator "isDate" applied by default', async () => { - const el = await fixture(``); + const el = await fixture( + html` + + `, + ); el.modelValue = '2005/11/10'; expect(el.hasFeedbackFor).to.include('error'); expect(el.validationStates).to.have.a.property('error'); @@ -99,24 +111,39 @@ describe('', () => { it('is accessible', async () => { const el = await fixture( - ``, + html` + + `, ); await expect(el).to.be.accessible(); }); it('is accessible when readonly', async () => { const el = await fixture( - ``, + html` + + `, ); await expect(el).to.be.accessible(); }); it('is accessible when disabled', async () => { const el = await fixture( - ``, + html` + + `, ); await expect(el).to.be.accessible(); }); + + it('serializes to iso format', async () => { + const el = await fixture( + html` + + `, + ); + expect(el.serializedValue).to.equal('2000-12-12'); + }); }); From d81764e9fda4e74db460ebb9b5c29824adc97ce4 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 20 Feb 2020 14:47:45 +0100 Subject: [PATCH 4/7] chore: enhanced docs and enabled integration test forms --- .../stories/15-features-overview.stories.mdx | 40 ++++-- .../17-Validation-Examples.stories.mdx | 6 +- ...System-creating-a-custom-field.stories.mdx | 2 +- .../stories/fieldset-examples.stories.mdx | 2 +- .../stories/form-examples.stories.mdx | 136 +++++++++--------- .../test/form-integrations.test.js | 2 +- 6 files changed, 100 insertions(+), 88 deletions(-) diff --git a/packages/form-system/stories/15-features-overview.stories.mdx b/packages/form-system/stories/15-features-overview.stories.mdx index 10fed8d19..ff84977f0 100644 --- a/packages/form-system/stories/15-features-overview.stories.mdx +++ b/packages/form-system/stories/15-features-overview.stories.mdx @@ -4,6 +4,7 @@ import '@lion/fieldset/lion-fieldset.js'; import '@lion/form/lion-form.js'; import '@lion/input-amount/lion-input-amount.js'; import '@lion/input-date/lion-input-date.js'; +import '@lion/input-datepicker/lion-input-datepicker.js'; import '@lion/input-email/lion-input-email.js'; import '@lion/input-iban/lion-input-iban.js'; import '@lion/input-range/lion-input-range.js'; @@ -28,25 +29,35 @@ For usage and installation please see the appropriate packages. - {html` + {() => { + Required.getMessage = () => 'Please enter a value'; + return html`
- - + + + + + @@ -101,7 +112,7 @@ For usage and installation please see the appropriate packages. step="0.1" label="Input range" > - + @@ -118,7 +129,7 @@ For usage and installation please see the appropriate packages.
- `} + `;}} @@ -129,6 +140,7 @@ import '@lion/fieldset/lion-fieldset.js'; import '@lion/form/lion-form.js'; import '@lion/input-amount/lion-input-amount.js'; import '@lion/input-date/lion-input-date.js'; +import '@lion/input-datepicker/lion-input-datepicker.js'; import '@lion/input-email/lion-input-email.js'; import '@lion/input-iban/lion-input-iban.js'; import '@lion/input-range/lion-input-range.js'; diff --git a/packages/form-system/stories/17-Validation-Examples.stories.mdx b/packages/form-system/stories/17-Validation-Examples.stories.mdx index 3fd22e2a9..93e24387a 100644 --- a/packages/form-system/stories/17-Validation-Examples.stories.mdx +++ b/packages/form-system/stories/17-Validation-Examples.stories.mdx @@ -29,8 +29,10 @@ import { ## Required Validator -The required validator can be put onto every form field element and will make sure that element is not empty. -For an input that may mean that it is not an empty string while for a checkbox group it means at least one checkbox needs to be checked. +The required validator can be put onto every form field element and will make sure that element is +not empty. +For an input that may mean that it is not an empty string, +while for a checkbox group it means at least one checkbox needs to be checked. {html` diff --git a/packages/form-system/stories/40-System-creating-a-custom-field.stories.mdx b/packages/form-system/stories/40-System-creating-a-custom-field.stories.mdx index c18fb904a..f0870739c 100644 --- a/packages/form-system/stories/40-System-creating-a-custom-field.stories.mdx +++ b/packages/form-system/stories/40-System-creating-a-custom-field.stories.mdx @@ -26,7 +26,7 @@ it has a tabindex=“0” applied. Now we want to integrate the slider in our form framework to enrich the user interface, get validation support and get all the other [benefits of LionField](/?path=/docs/forms-system-overview--page). -We start of by creating a component `` that extends from `LionField`. +We start by creating a component `` that extends from `LionField`. Then we follow the steps below: #### 1. Add your interaction element as ‘input slot' diff --git a/packages/form-system/stories/fieldset-examples.stories.mdx b/packages/form-system/stories/fieldset-examples.stories.mdx index 00586fa41..ca7ed46f9 100644 --- a/packages/form-system/stories/fieldset-examples.stories.mdx +++ b/packages/form-system/stories/fieldset-examples.stories.mdx @@ -298,7 +298,7 @@ Alternatively you can also let the fieldset validator be dependent on the error Simply loop over the formElements inside your Validator's `execute` method: ```js -this.formElementsArray.some(el => el.hasFeedbackFor.includes('error')); +this.formElements.some(el => el.hasFeedbackFor.includes('error')); ``` ### Validating multiple fieldsets diff --git a/packages/form-system/stories/form-examples.stories.mdx b/packages/form-system/stories/form-examples.stories.mdx index 1cd2ceac4..7a3a85f0d 100644 --- a/packages/form-system/stories/form-examples.stories.mdx +++ b/packages/form-system/stories/form-examples.stories.mdx @@ -1,4 +1,4 @@ -import { Story, Meta, html } from '@open-wc/demoing-storybook'; +import { Story, Meta, html, Preview } from '@open-wc/demoing-storybook'; import { Required, MaxLength, loadDefaultFeedbackMessages } from '@lion/validate'; import '@lion/fieldset/lion-fieldset.js'; import '@lion/input/lion-input.js'; @@ -10,6 +10,7 @@ import '@lion/form/lion-form.js'; A form can have multiple nested fieldsets. + {html` @@ -25,48 +26,38 @@ A form can have multiple nested fieldsets. - `} - -```html - -
- - - - - - - - - - - - -
- -``` + ## Form Submit / Reset You can control whether a form gets submitted based on validation states. Same thing goes for resetting the inputs to the original state. +Be sure to call `serializedValue` when a you want to submit a form. +This value automatically filters out disabled children and makes sure the values +that are retrieved can be transmitted over a wire. (for instance, an input-date with a modelValue +of type `Date` will be serialized to an iso formatted string). + + +> Note: Offering a reset button is a bad practice in terms of accessibility. +This button is only used here to demonstrate the `serializeGroup()` method. + + {() => { loadDefaultFeedbackMessages(); const submit = () => { const form = document.querySelector('#form2'); if (!form.hasFeedbackFor.includes('error')) { - console.log(form.serializeGroup()); - form.resetGroup(); + document.getElementById('form2_output').innerText = JSON.stringify(form.serializedValue); + document.querySelector('#form2').resetGroup(); } }; return html` @@ -85,58 +76,65 @@ Same thing goes for resetting the inputs to the original state. .validators=${[new Required(), new MaxLength(15)]} > + + - -

- A reset button should never be offered to users. This button is only used here to - demonstrate the functionality. -

+ 
+
`; }} + -```js -import { Required, MaxLength } from '@lion/validate' -const submit = () => { - const form = document.querySelector('#form2'); - if (!form.hasFeedbackFor.includes('error')) { - console.log(form.serializeGroup()); - form.resetGroup(); - } -}; -``` -```html - -
- - - - - - - - -

- A reset button should never be offered to users. This button is only used here to - demonstrate the functionality. -

-
- -``` +## Serialize in a multistep form + +In a multistep form (consisting of multiple forms) it might be handy to wrap the serialized output +with the name of the form. + + + + {() => { + loadDefaultFeedbackMessages(); + const serializeWithName = (formId, outputId) => { + const form = document.getElementById(formId); + if (!form.hasFeedbackFor.includes('error')) { + const output = { [form.name]: form.serializedValue }; + document.getElementById(outputId).innerText = JSON.stringify(output); + } + }; + return html` +
+ + + 

+        

+        

+          
+          
+          

+        

+      `;
+    }}
+  
+
diff --git a/packages/form-system/test/form-integrations.test.js b/packages/form-system/test/form-integrations.test.js
index 602cf16ba..6db704d3d 100644
--- a/packages/form-system/test/form-integrations.test.js
+++ b/packages/form-system/test/form-integrations.test.js
@@ -3,7 +3,7 @@ import './helpers/umbrella-form.js';
 
 // Test umbrella form
 describe('Form Integrations', () => {
-  it.skip('".serializedValue" returns all non disabled fields based on form structure', async () => {
+  it('".serializedValue" returns all non disabled fields based on form structure', async () => {
     const el = await fixture(
       html`
         

From f44d8aa26ae7124d8dcb251e1f66ab9beae71050 Mon Sep 17 00:00:00 2001
From: Thijs Louisse 
Date: Thu, 20 Feb 2020 11:35:41 +0100
Subject: [PATCH 5/7] fix: removed FieldCustomMixin

---
 packages/field/docs/FormattingAndParsing.md  |  6 --
 packages/field/index.js                      |  1 -
 packages/field/src/FieldCustomMixin.js       | 46 ----------
 packages/field/test/FieldCustomMixin.test.js | 31 -------
 packages/input-amount/src/LionInputAmount.js | 93 ++++++++++++--------
 packages/input-email/src/LionInputEmail.js   |  3 +-
 packages/input-iban/src/LionInputIban.js     |  3 +-
 packages/input-range/src/LionInputRange.js   |  3 +-
 8 files changed, 59 insertions(+), 127 deletions(-)
 delete mode 100644 packages/field/src/FieldCustomMixin.js
 delete mode 100644 packages/field/test/FieldCustomMixin.test.js

diff --git a/packages/field/docs/FormattingAndParsing.md b/packages/field/docs/FormattingAndParsing.md
index 87faf49d7..982b8f742 100644
--- a/packages/field/docs/FormattingAndParsing.md
+++ b/packages/field/docs/FormattingAndParsing.md
@@ -101,12 +101,6 @@ function deserializeDate(serializeValue, options) {
 }
 ```
 
-### FieldCustomMixin
-
-When creating your own custom input, please use `FieldCustomMixin` as a basis for this.
-Concrete examples can be found at [``](../../input-date/) and
-[``](../../input-amount/).
-
 ## Flow diagram
 
 The following flow diagram is based on both end user input and interaction programmed by the
diff --git a/packages/field/index.js b/packages/field/index.js
index 6c1d2d28d..090071db2 100644
--- a/packages/field/index.js
+++ b/packages/field/index.js
@@ -1,6 +1,5 @@
 export { FocusMixin } from './src/FocusMixin.js';
 export { FormatMixin } from './src/FormatMixin.js';
-export { FieldCustomMixin } from './src/FieldCustomMixin.js';
 export { FormControlMixin } from './src/FormControlMixin.js';
 export { InteractionStateMixin } from './src/InteractionStateMixin.js'; // applies FocusMixin
 export { LionField } from './src/LionField.js';
diff --git a/packages/field/src/FieldCustomMixin.js b/packages/field/src/FieldCustomMixin.js
deleted file mode 100644
index 46777495c..000000000
--- a/packages/field/src/FieldCustomMixin.js
+++ /dev/null
@@ -1,46 +0,0 @@
-import { dedupeMixin, nothing } from '@lion/core';
-
-/**
- * #FieldCustomMixin
- *
- * @polymerMixin
- * @mixinFunction
- */
-
-export const FieldCustomMixin = dedupeMixin(
-  superclass =>
-    // eslint-disable-next-line no-shadow, max-len
-    class FieldCustomMixin extends superclass {
-      static get properties() {
-        return {
-          /**
-           * When no light dom defined and prop set
-           */
-          disableHelpText: {
-            type: Boolean,
-            attribute: 'disable-help-text',
-          },
-        };
-      }
-
-      get slots() {
-        return {
-          ...super.slots,
-          'help-text': () => {
-            if (!this.disableHelpText) {
-              return super.slots['help-text']();
-            }
-            return null;
-          },
-        };
-      }
-
-      helpTextTemplate(...args) {
-        if (this.disableHelpText || !super.helpTextTemplate) {
-          return nothing;
-        }
-
-        return super.helpTextTemplate.apply(this, args);
-      }
-    },
-);
diff --git a/packages/field/test/FieldCustomMixin.test.js b/packages/field/test/FieldCustomMixin.test.js
deleted file mode 100644
index a4b20e5a8..000000000
--- a/packages/field/test/FieldCustomMixin.test.js
+++ /dev/null
@@ -1,31 +0,0 @@
-import { expect, fixture, defineCE } from '@open-wc/testing';
-import { LionField } from '../src/LionField.js';
-
-import { FieldCustomMixin } from '../src/FieldCustomMixin.js';
-
-describe('FieldCustomMixin', () => {
-  const inputSlot = '';
-  let elem;
-
-  before(async () => {
-    const FieldCustomMixinClass = class extends FieldCustomMixin(LionField) {
-      static get properties() {
-        return {
-          modelValue: {
-            type: String,
-          },
-        };
-      }
-    };
-    elem = defineCE(FieldCustomMixinClass);
-  });
-
-  it('has the capability to disable help text', async () => {
-    const lionField = await fixture(`
-      <${elem} disable-help-text>${inputSlot}
-    `);
-    expect(Array.from(lionField.children).find(child => child.slot === 'help-text')).to.equal(
-      undefined,
-    );
-  });
-});
diff --git a/packages/input-amount/src/LionInputAmount.js b/packages/input-amount/src/LionInputAmount.js
index a6a856478..7f9ebcbdc 100644
--- a/packages/input-amount/src/LionInputAmount.js
+++ b/packages/input-amount/src/LionInputAmount.js
@@ -1,7 +1,6 @@
 import { css } from '@lion/core';
 import { LocalizeMixin, getCurrencyName } from '@lion/localize';
 import { LionInput } from '@lion/input';
-import { FieldCustomMixin } from '@lion/field';
 import { IsNumber } from '@lion/validate';
 import { parseAmount } from './parsers.js';
 import { formatAmount } from './formatters.js';
@@ -12,22 +11,26 @@ import { formatAmount } from './formatters.js';
  * @customElement lion-input-amount
  * @extends {LionInput}
  */
-export class LionInputAmount extends FieldCustomMixin(LocalizeMixin(LionInput)) {
+export class LionInputAmount extends LocalizeMixin(LionInput) {
   static get properties() {
     return {
-      currency: {
-        type: String,
-      },
+      /**
+       * @desc an iso code like 'EUR' or 'USD' that will be displayed next to the input
+       * and from which an accessible label (like 'euros') is computed for screen
+       * reader users
+       * @type {string}
+       */
+      currency: String,
+      /**
+       * @desc the modelValue of the input-amount has the 'Number' type. This allows
+       * Application Developers to easily read from and write to this input or write custom
+       * validators.
+       * @type {number}
+       */
+      modelValue: Number,
     };
   }
 
-  updated(changedProps) {
-    super.updated(changedProps);
-    if (changedProps.has('currency')) {
-      this._onCurrencyChanged({ currency: this.currency });
-    }
-  }
-
   get slots() {
     return {
       ...super.slots,
@@ -45,6 +48,21 @@ export class LionInputAmount extends FieldCustomMixin(LocalizeMixin(LionInput))
     };
   }
 
+  get _currencyDisplayNode() {
+    return Array.from(this.children).find(child => child.slot === 'after');
+  }
+
+  static get styles() {
+    return [
+      ...super.styles,
+      css`
+        .input-group__container > .input-group__input ::slotted(.form-control) {
+          text-align: right;
+        }
+      `,
+    ];
+  }
+
   constructor() {
     super();
     this.parser = parseAmount;
@@ -59,18 +77,6 @@ export class LionInputAmount extends FieldCustomMixin(LocalizeMixin(LionInput))
     this.defaultValidators.push(new IsNumber());
   }
 
-  __callParser(value = this.formattedValue) {
-    // TODO: input and change events both trigger parsing therefore we need to handle the second parse
-    this.__parserCallcountSincePaste += 1;
-    this.__isPasting = this.__parserCallcountSincePaste === 2;
-    this.formatOptions.mode = this.__isPasting === true ? 'pasted' : 'auto';
-    return super.__callParser(value);
-  }
-
-  _reflectBackOn() {
-    return super._reflectBackOn() || this.__isPasting;
-  }
-
   connectedCallback() {
     // eslint-disable-next-line wc/guard-super-call
     super.connectedCallback();
@@ -81,12 +87,29 @@ export class LionInputAmount extends FieldCustomMixin(LocalizeMixin(LionInput))
     }
   }
 
-  __setCurrencyDisplayLabel() {
-    this._currencyDisplayNode.setAttribute('aria-label', getCurrencyName(this.currency));
+  updated(changedProps) {
+    super.updated(changedProps);
+    if (changedProps.has('currency')) {
+      this._onCurrencyChanged({ currency: this.currency });
+    }
   }
 
-  get _currencyDisplayNode() {
-    return Array.from(this.children).find(child => child.slot === 'after');
+  /**
+   * @override of FormatMixin
+   */
+  __callParser(value = this.formattedValue) {
+    // TODO: input and change events both trigger parsing therefore we need to handle the second parse
+    this.__parserCallcountSincePaste += 1;
+    this.__isPasting = this.__parserCallcountSincePaste === 2;
+    this.formatOptions.mode = this.__isPasting === true ? 'pasted' : 'auto';
+    return super.__callParser(value);
+  }
+
+  /**
+   * @override of FormatMixin
+   */
+  _reflectBackOn() {
+    return super._reflectBackOn() || this.__isPasting;
   }
 
   _onCurrencyChanged({ currency }) {
@@ -98,14 +121,10 @@ export class LionInputAmount extends FieldCustomMixin(LocalizeMixin(LionInput))
     this.__setCurrencyDisplayLabel();
   }
 
-  static get styles() {
-    return [
-      ...super.styles,
-      css`
-        .input-group__container > .input-group__input ::slotted(.form-control) {
-          text-align: right;
-        }
-      `,
-    ];
+  __setCurrencyDisplayLabel() {
+    // TODO: for optimal a11y, abbreviations should be part of aria-label
+    // example, for a language switch with text 'en', an aria-label of 'english' is not
+    // sufficient, it should also contain the abbreviation.
+    this._currencyDisplayNode.setAttribute('aria-label', getCurrencyName(this.currency));
   }
 }
diff --git a/packages/input-email/src/LionInputEmail.js b/packages/input-email/src/LionInputEmail.js
index c7198eb0c..4fc769a4f 100644
--- a/packages/input-email/src/LionInputEmail.js
+++ b/packages/input-email/src/LionInputEmail.js
@@ -1,5 +1,4 @@
 import { LocalizeMixin } from '@lion/localize';
-import { FieldCustomMixin } from '@lion/field';
 import { LionInput } from '@lion/input';
 import { IsEmail } from '@lion/validate';
 
@@ -9,7 +8,7 @@ import { IsEmail } from '@lion/validate';
  * @customElement lion-input-email
  * @extends {LionInput}
  */
-export class LionInputEmail extends FieldCustomMixin(LocalizeMixin(LionInput)) {
+export class LionInputEmail extends LocalizeMixin(LionInput) {
   constructor() {
     super();
     // local-part@domain where the local part may be up to 64 characters long
diff --git a/packages/input-iban/src/LionInputIban.js b/packages/input-iban/src/LionInputIban.js
index f9de96c6d..399ef1842 100644
--- a/packages/input-iban/src/LionInputIban.js
+++ b/packages/input-iban/src/LionInputIban.js
@@ -1,6 +1,5 @@
 import { LocalizeMixin } from '@lion/localize';
 import { LionInput } from '@lion/input';
-import { FieldCustomMixin } from '@lion/field';
 import { formatIBAN } from './formatters.js';
 import { parseIBAN } from './parsers.js';
 import { IsIBAN } from './validators.js';
@@ -10,7 +9,7 @@ import { IsIBAN } from './validators.js';
  *
  * @extends {LionInput}
  */
-export class LionInputIban extends FieldCustomMixin(LocalizeMixin(LionInput)) {
+export class LionInputIban extends LocalizeMixin(LionInput) {
   constructor() {
     super();
     this.formatter = formatIBAN;
diff --git a/packages/input-range/src/LionInputRange.js b/packages/input-range/src/LionInputRange.js
index b05ae558a..e3c3bb158 100644
--- a/packages/input-range/src/LionInputRange.js
+++ b/packages/input-range/src/LionInputRange.js
@@ -1,6 +1,5 @@
 /* eslint-disable import/no-extraneous-dependencies */
 import { LocalizeMixin, formatNumber } from '@lion/localize';
-import { FieldCustomMixin } from '@lion/field';
 import { LionInput } from '@lion/input';
 import { html, css, unsafeCSS } from '@lion/core';
 
@@ -10,7 +9,7 @@ import { html, css, unsafeCSS } from '@lion/core';
  * @customElement `lion-input-range`
  * @extends LionInput
  */
-export class LionInputRange extends FieldCustomMixin(LocalizeMixin(LionInput)) {
+export class LionInputRange extends LocalizeMixin(LionInput) {
   static get properties() {
     return {
       min: Number,

From cb7120e3a59f02e7e87f00c4d281e47ca8d33ea8 Mon Sep 17 00:00:00 2001
From: Thijs Louisse 
Date: Thu, 20 Feb 2020 11:48:13 +0100
Subject: [PATCH 6/7] fix: rearrange/cleanup mixin duties

---
 packages/field/src/LionField.js               | 12 -----
 .../field/test-suites/FormatMixin.suite.js    |  2 +-
 packages/fieldset/test/lion-fieldset.test.js  |  4 +-
 packages/input/src/LionInput.js               |  4 +-
 packages/input/test/lion-input.test.js        | 48 +++++++++++--------
 5 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/packages/field/src/LionField.js b/packages/field/src/LionField.js
index 957059232..4615bd28a 100644
--- a/packages/field/src/LionField.js
+++ b/packages/field/src/LionField.js
@@ -37,10 +37,6 @@ export class LionField extends FormControlMixin(
         // make sure validation can be triggered based on observer
         type: Boolean,
       },
-      name: {
-        type: String,
-        reflect: true,
-      },
       autocomplete: {
         type: String,
         reflect: true,
@@ -217,12 +213,4 @@ export class LionField extends FormControlMixin(
       this._inputNode.value = newValue;
     }
   }
-
-  set fieldName(value) {
-    this.__fieldName = value;
-  }
-
-  get fieldName() {
-    return this.__fieldName || this.label || this.name;
-  }
 }
diff --git a/packages/field/test-suites/FormatMixin.suite.js b/packages/field/test-suites/FormatMixin.suite.js
index 88addcc17..6514529d9 100644
--- a/packages/field/test-suites/FormatMixin.suite.js
+++ b/packages/field/test-suites/FormatMixin.suite.js
@@ -52,7 +52,7 @@ export function runFormatMixinSuite(customConfig) {
     }
   }
 
-  describe(`FormatMixin ${cfg.suffix ? `(${cfg.suffix})` : ''}`, async () => {
+  describe('FormatMixin', async () => {
     let elem;
     let nonFormat;
     let fooFormat;
diff --git a/packages/fieldset/test/lion-fieldset.test.js b/packages/fieldset/test/lion-fieldset.test.js
index 497ae31b2..a5b66ca93 100644
--- a/packages/fieldset/test/lion-fieldset.test.js
+++ b/packages/fieldset/test/lion-fieldset.test.js
@@ -624,7 +624,7 @@ describe('', () => {
       expect(el.hasFeedbackFor).to.deep.equal(['error']);
     });
 
-    it.skip('(re)initializes children interaction states on registration ready', async () => {
+    it('(re)initializes children interaction states on registration ready', async () => {
       const fieldset = await fixtureSync(html`
       <${tag} .modelValue="${{ a: '1', b: '2' }}">
         <${childTag} name="a">
@@ -698,7 +698,7 @@ describe('', () => {
       expect(fieldset.serializedValue).to.deep.equal({ price: 0 });
     });
 
-    it.skip('serializes undefined values as ""(nb radios/checkboxes are always serialized)', async () => {
+    it('serializes undefined values as ""(nb radios/checkboxes are always serialized)', async () => {
       const fieldset = await fixture(html`
         <${tag}>
           <${childTag} name="custom[]">
diff --git a/packages/input/src/LionInput.js b/packages/input/src/LionInput.js
index c00335bcc..048d902fe 100644
--- a/packages/input/src/LionInput.js
+++ b/packages/input/src/LionInput.js
@@ -39,9 +39,7 @@ export class LionInput extends LionField {
       input: () => {
         // TODO: Find a better way to do value delegation via attr
         const native = document.createElement('input');
-        if (this.__dataInstanceProps && this.__dataInstanceProps.modelValue) {
-          native.setAttribute('value', this.__dataInstanceProps.modelValue);
-        } else if (this.hasAttribute('value')) {
+        if (this.hasAttribute('value')) {
           native.setAttribute('value', this.getAttribute('value'));
         }
         return native;
diff --git a/packages/input/test/lion-input.test.js b/packages/input/test/lion-input.test.js
index faa0d247f..04ab4bdb3 100644
--- a/packages/input/test/lion-input.test.js
+++ b/packages/input/test/lion-input.test.js
@@ -1,12 +1,13 @@
-import { expect, fixture } from '@open-wc/testing';
+import { expect, fixture, html, unsafeStatic } from '@open-wc/testing';
 
 import '../lion-input.js';
 
+const tagString = 'lion-input';
+const tag = unsafeStatic(tagString);
+
 describe('', () => {
   it('delegates readOnly property and readonly attribute', async () => {
-    const el = await fixture(
-      ``,
-    );
+    const el = await fixture(html`<${tag} readonly>`);
     expect(el._inputNode.readOnly).to.equal(true);
     el.readOnly = false;
     await el.updateComplete;
@@ -14,13 +15,20 @@ describe('', () => {
     expect(el._inputNode.readOnly).to.equal(false);
   });
 
+  it('delegates value attribute', async () => {
+    const el = await fixture(html`<${tag} value="prefilled">`);
+    expect(el._inputNode.value).to.equal('prefilled');
+  });
+
   it('automatically creates an  element if not provided by user', async () => {
-    const el = await fixture(``);
+    const el = await fixture(html`
+      <${tag}>
+    `);
     expect(el.querySelector('input')).to.equal(el._inputNode);
   });
 
   it('has a type which is reflected to an attribute and is synced down to the native input', async () => {
-    const el = await fixture(``);
+    const el = await fixture(html`<${tag}>`);
     expect(el.type).to.equal('text');
     expect(el.getAttribute('type')).to.equal('text');
     expect(el._inputNode.getAttribute('type')).to.equal('text');
@@ -32,7 +40,7 @@ describe('', () => {
   });
 
   it('has an attribute that can be used to set the placeholder text of the input', async () => {
-    const el = await fixture(``);
+    const el = await fixture(html`<${tag} placeholder="text">`);
     expect(el.getAttribute('placeholder')).to.equal('text');
     expect(el._inputNode.getAttribute('placeholder')).to.equal('text');
 
@@ -42,20 +50,20 @@ describe('', () => {
     expect(el._inputNode.getAttribute('placeholder')).to.equal('foo');
   });
 
-  it('is accessible', async () => {
-    const el = await fixture(``);
-    await expect(el).to.be.accessible();
-  });
+  describe('Accessibility', () => {
+    it('is accessible', async () => {
+      const el = await fixture(html`<${tag} label="Label">`);
+      await expect(el).to.be.accessible();
+    });
 
-  it('is accessible when readonly', async () => {
-    const el = await fixture(
-      ``,
-    );
-    await expect(el).to.be.accessible();
-  });
+    it('is accessible when readonly', async () => {
+      const el = await fixture(html`<${tag} readonly label="Label">`);
+      await expect(el).to.be.accessible();
+    });
 
-  it('is accessible when disabled', async () => {
-    const el = await fixture(``);
-    await expect(el).to.be.accessible();
+    it('is accessible when disabled', async () => {
+      const el = await fixture(html`<${tag} disabled label="Label">`);
+      await expect(el).to.be.accessible();
+    });
   });
 });

From d6829ef6199a31e4b2ab1e97dd2ccce354043145 Mon Sep 17 00:00:00 2001
From: Thijs Louisse 
Date: Thu, 20 Feb 2020 11:48:45 +0100
Subject: [PATCH 7/7] fix(field): interaction states initialized after
 registration ready

---
 packages/field/src/InteractionStateMixin.js       |  9 +++------
 .../test-suites/InteractionStateMixin.suite.js    | 15 +++++++++++++--
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/packages/field/src/InteractionStateMixin.js b/packages/field/src/InteractionStateMixin.js
index 945ab2d88..629c23453 100644
--- a/packages/field/src/InteractionStateMixin.js
+++ b/packages/field/src/InteractionStateMixin.js
@@ -24,15 +24,13 @@ export const InteractionStateMixin = dedupeMixin(
             type: Boolean,
             reflect: true,
           },
-
           /**
-           * True when user has typed in something in the input field.
+           * True when user has changed the value of the field.
            */
           dirty: {
             type: Boolean,
             reflect: true,
           },
-
           /**
            * True when user has left non-empty field or input is prefilled.
            * The name must be seen from the point of view of the input field:
@@ -112,9 +110,8 @@ export const InteractionStateMixin = dedupeMixin(
        * Since this method will be called twice in last mentioned scenario, it must stay idempotent.
        */
       initInteractionState() {
-        if (this.constructor._isPrefilled(this.modelValue)) {
-          this.prefilled = true;
-        }
+        this.dirty = false;
+        this.prefilled = this.constructor._isPrefilled(this.modelValue);
       }
 
       /**
diff --git a/packages/field/test-suites/InteractionStateMixin.suite.js b/packages/field/test-suites/InteractionStateMixin.suite.js
index 10c0b3f77..e60f8efa1 100644
--- a/packages/field/test-suites/InteractionStateMixin.suite.js
+++ b/packages/field/test-suites/InteractionStateMixin.suite.js
@@ -15,11 +15,10 @@ export function runInteractionStateMixinSuite(customConfig) {
   const cfg = {
     tagString: null,
     allowedModelValueTypes: [Array, Object, Number, Boolean, String, Date],
-    suffix: '',
     ...customConfig,
   };
 
-  describe(`InteractionStateMixin ${cfg.suffix ? `(${cfg.suffix})` : ''}`, async () => {
+  describe(`InteractionStateMixin`, async () => {
     let tag;
     before(() => {
       if (!cfg.tagString) {
@@ -174,6 +173,18 @@ export function runInteractionStateMixinSuite(customConfig) {
       expect(el.prefilled).to.be.true;
     });
 
+    it('has a method initInteractionState()', async () => {
+      const el = await fixture(html`<${tag}>`);
+      el.modelValue = 'Some value';
+      expect(el.dirty).to.be.true;
+      expect(el.touched).to.be.false;
+      expect(el.prefilled).to.be.false;
+      el.initInteractionState();
+      expect(el.dirty).to.be.false;
+      expect(el.touched).to.be.false;
+      expect(el.prefilled).to.be.true;
+    });
+
     describe('SubClassers', () => {
       it('can override the `_leaveEvent`', async () => {
         const tagLeaveString = defineCE(