diff --git a/packages/field/index.js b/packages/field/index.js index 24f7e1204..3c35e3206 100644 --- a/packages/field/index.js +++ b/packages/field/index.js @@ -4,3 +4,5 @@ export { FormatMixin } from './src/FormatMixin.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'; diff --git a/packages/field/src/FormControlMixin.js b/packages/field/src/FormControlMixin.js index 109edba76..470836c51 100644 --- a/packages/field/src/FormControlMixin.js +++ b/packages/field/src/FormControlMixin.js @@ -1,5 +1,6 @@ import { html, css, nothing, dedupeMixin, SlotMixin } from '@lion/core'; import { ObserverMixin } from '@lion/core/src/ObserverMixin.js'; +import { FormRegisteringMixin } from './FormRegisteringMixin.js'; /** * #FormControlMixin : @@ -14,7 +15,7 @@ import { ObserverMixin } from '@lion/core/src/ObserverMixin.js'; export const FormControlMixin = dedupeMixin( superclass => // eslint-disable-next-line no-shadow, no-unused-vars - class FormControlMixin extends ObserverMixin(SlotMixin(superclass)) { + class FormControlMixin extends FormRegisteringMixin(ObserverMixin(SlotMixin(superclass))) { static get properties() { return { ...super.properties, @@ -105,8 +106,6 @@ export const FormControlMixin = dedupeMixin( super.connectedCallback(); this._enhanceLightDomClasses(); this._enhanceLightDomA11y(); - this._registerFormElement(); - this._requestParentFormGroupUpdateOfResetModelValue(); } /** @@ -150,42 +149,6 @@ export const FormControlMixin = dedupeMixin( this._enhanceLightDomA11yForAdditionalSlots(); } - /** - * Fires a registration event in the next frame. - * - * Why next frame? - * if ShadyDOM is used and you add a listener and fire the event in the same frame - * it will not bubble and there can not be cought by a parent element - * for more details see: https://github.com/Polymer/lit-element/issues/658 - * will requires a `await nextFrame()` in tests - */ - _registerFormElement() { - this.updateComplete.then(() => { - this.dispatchEvent( - new CustomEvent('form-element-register', { - detail: { element: this }, - bubbles: true, - }), - ); - }); - } - - /** - * Makes sure our parentFormGroup has the most up to date resetModelValue - * FormGroups will call the same on their parentFormGroup so the full tree gets the correct - * values. - * - * Why next frame? - * @see {@link this._registerFormElement} - */ - _requestParentFormGroupUpdateOfResetModelValue() { - this.updateComplete.then(() => { - if (this.__parentFormGroup) { - this.__parentFormGroup._updateResetModelValue(); - } - }); - } - /** * Enhances additional slots(prefix, suffix, before, after) defined by developer. * diff --git a/packages/field/src/FormRegisteringMixin.js b/packages/field/src/FormRegisteringMixin.js new file mode 100644 index 000000000..a5b92e975 --- /dev/null +++ b/packages/field/src/FormRegisteringMixin.js @@ -0,0 +1,71 @@ +import { dedupeMixin } from '@lion/core'; +import { formRegistrarManager } from './formRegistrarManager.js'; + +/** + * #FormRegisteringMixin: + * + * This Mixin registers a form element to a Registrar + * + * @polymerMixin + * @mixinFunction + */ +export const FormRegisteringMixin = dedupeMixin( + superclass => + // eslint-disable-next-line no-shadow, no-unused-vars + class FormRegisteringMixin extends superclass { + connectedCallback() { + if (super.connectedCallback) { + super.connectedCallback(); + } + this.__setupRegistrationHook(); + } + + disconnectedCallback() { + if (super.disconnectedCallback) { + super.disconnectedCallback(); + } + this._unregisterFormElement(); + } + + __setupRegistrationHook() { + if (formRegistrarManager.ready) { + this._registerFormElement(); + } else { + formRegistrarManager.addEventListener('all-forms-open-for-registration', () => { + this._registerFormElement(); + }); + } + } + + _registerFormElement() { + this._dispatchRegistration(); + this._requestParentFormGroupUpdateOfResetModelValue(); + } + + _dispatchRegistration() { + this.dispatchEvent( + new CustomEvent('form-element-register', { + detail: { element: this }, + bubbles: true, + }), + ); + } + + _unregisterFormElement() { + if (this.__parentFormGroup) { + this.__parentFormGroup.removeFormElement(this); + } + } + + /** + * Makes sure our parentFormGroup has the most up to date resetModelValue + * FormGroups will call the same on their parentFormGroup so the full tree gets the correct + * values. + */ + _requestParentFormGroupUpdateOfResetModelValue() { + if (this.__parentFormGroup && this.__parentFormGroup._updateResetModelValue) { + this.__parentFormGroup._updateResetModelValue(); + } + } + }, +); diff --git a/packages/field/src/FormRegistrarMixin.js b/packages/field/src/FormRegistrarMixin.js new file mode 100644 index 000000000..b3ac82e8f --- /dev/null +++ b/packages/field/src/FormRegistrarMixin.js @@ -0,0 +1,92 @@ +import { dedupeMixin } from '@lion/core'; +import { formRegistrarManager } from './formRegistrarManager.js'; +import { FormRegisteringMixin } from './FormRegisteringMixin.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.registrationReady = new Promise(resolve => { + this.__resolveRegistrationReady = resolve; + }); + formRegistrarManager.add(this); + + this._onRequestToAddFormElement = this._onRequestToAddFormElement.bind(this); + this.addEventListener('form-element-register', this._onRequestToAddFormElement); + } + + isRegisteredFormElement(el) { + return this.formElementsArray.some(exitingEl => exitingEl === el); + } + + firstUpdated(changedProperties) { + super.firstUpdated(changedProperties); + this.__resolveRegistrationReady(); + this.__readyForRegistration = true; + formRegistrarManager.becomesReady(this); + } + + addFormElement(child) { + // 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; + + 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(); + this.addFormElement(child); + } + + _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/formRegistrarManager.js b/packages/field/src/formRegistrarManager.js new file mode 100644 index 000000000..71254faa4 --- /dev/null +++ b/packages/field/src/formRegistrarManager.js @@ -0,0 +1,36 @@ +/** + * Allows to align the timing for all Registrars (like form, fieldset). + * e.g. it will only be ready once all Registrars have been fully rendered + * + * This is a requirement for ShadyDOM as otherwise forms can not catch registration events + */ +class FormRegistrarManager { + constructor() { + this.__elements = []; + this._fakeExtendsEventTarget(); + this.ready = false; + } + + add(registrar) { + this.__elements.push(registrar); + this.ready = false; + } + + becomesReady() { + if (this.__elements.every(el => el.__readyForRegistration === true)) { + this.dispatchEvent(new Event('all-forms-open-for-registration')); + this.ready = true; + } + } + + // TODO: this method has to be removed when EventTarget polyfill is available on IE11 + // issue: https://gitlab.ing.net/TheGuideComponents/lion-element/issues/12 + _fakeExtendsEventTarget() { + const delegate = document.createDocumentFragment(); + ['addEventListener', 'dispatchEvent', 'removeEventListener'].forEach(funcName => { + this[funcName] = (...args) => delegate[funcName](...args); + }); + } +} + +export const formRegistrarManager = new FormRegistrarManager(); diff --git a/packages/field/test/FormControlMixin.test.js b/packages/field/test/FormControlMixin.test.js index 79c66af2d..d2e58560f 100644 --- a/packages/field/test/FormControlMixin.test.js +++ b/packages/field/test/FormControlMixin.test.js @@ -1,5 +1,4 @@ -import { expect, fixture, html, defineCE, unsafeStatic, nextFrame } from '@open-wc/testing'; -import sinon from 'sinon'; +import { expect, fixture, html, defineCE, unsafeStatic } from '@open-wc/testing'; import { SlotMixin } from '@lion/core'; import { LionLitElement } from '@lion/core/src/LionLitElement.js'; @@ -26,42 +25,6 @@ describe('FormControlMixin', () => { tag = unsafeStatic(elem); }); - it('dispatches event to register in Light DOM', async () => { - const registerSpy = sinon.spy(); - await fixture(html` -
- <${tag}> -
- `); - await nextFrame(); - expect(registerSpy.callCount).to.equal(1); - }); - - it('can by caught by listening in the appropriate dom', async () => { - const registerSpy = sinon.spy(); - const testTag = unsafeStatic( - defineCE( - class extends LionLitElement { - connectedCallback() { - super.connectedCallback(); - this.shadowRoot.addEventListener('form-element-register', registerSpy); - } - - render() { - return html` - <${tag}> - `; - } - }, - ), - ); - await fixture(html` - <${testTag}> - `); - await nextFrame(); - expect(registerSpy.callCount).to.equal(1); - }); - it('has the capability to override the help text', async () => { const lionFieldAttr = await fixture(html` <${tag} help-text="This email address is already taken">${inputSlot} diff --git a/packages/field/test/FormRegistrationMixins.test.js b/packages/field/test/FormRegistrationMixins.test.js new file mode 100644 index 000000000..a84d2a5d7 --- /dev/null +++ b/packages/field/test/FormRegistrationMixins.test.js @@ -0,0 +1,106 @@ +import { expect, fixture, html, defineCE, unsafeStatic } from '@open-wc/testing'; +import sinon from 'sinon'; +import { LitElement, UpdatingElement } from '@lion/core'; + +import { FormRegisteringMixin } from '../src/FormRegisteringMixin.js'; +import { FormRegistrarMixin } from '../src/FormRegistrarMixin.js'; + +describe('FormRegistrationMixins', () => { + before(async () => { + const FormRegistrarEl = class extends FormRegistrarMixin(UpdatingElement) {}; + customElements.define('form-registrar', FormRegistrarEl); + const FormRegisteringEl = class extends FormRegisteringMixin(UpdatingElement) {}; + customElements.define('form-registering', FormRegisteringEl); + }); + + it('can register a formElement', async () => { + const el = await fixture(html` + + + + `); + await el.registrationReady; + expect(el.formElements.length).to.equal(1); + }); + + it('supports nested registrar', async () => { + const el = await fixture(html` + + + + + + `); + await el.registrationReady; + expect(el.formElements.length).to.equal(1); + expect(el.querySelector('form-registrar').formElements.length).to.equal(1); + }); + + it('works for component that have a delayed render', async () => { + const tagWrapperString = defineCE( + class extends FormRegistrarMixin(LitElement) { + async performUpdate() { + await new Promise(resolve => setTimeout(() => resolve(), 10)); + await super.performUpdate(); + } + + render() { + return html` + + `; + } + }, + ); + const tagWrapper = unsafeStatic(tagWrapperString); + const registerSpy = sinon.spy(); + const el = await fixture(html` + <${tagWrapper} @form-element-register=${registerSpy}> + + + `); + await el.registrationReady; + expect(el.formElements.length).to.equal(1); + }); + + it('requests update of the resetModelValue function of its parent formGroup', async () => { + const ParentFormGroupClass = class extends FormRegistrarMixin(LitElement) { + _updateResetModelValue() { + this.resetModelValue = 'foo'; + } + }; + const ChildFormGroupClass = class extends FormRegisteringMixin(LitElement) { + constructor() { + super(); + this.__parentFormGroup = this.parentNode; + } + }; + + const parentClass = defineCE(ParentFormGroupClass); + const formGroup = unsafeStatic(parentClass); + const childClass = defineCE(ChildFormGroupClass); + const childFormGroup = unsafeStatic(childClass); + const parentFormEl = await fixture(html` + <${formGroup}><${childFormGroup} id="child" name="child[]"> + `); + expect(parentFormEl.resetModelValue).to.equal('foo'); + }); + + it('can dynamically add/remove elements', async () => { + const el = await fixture(html` + + + + `); + const newField = await fixture(html` + + `); + + expect(el.formElements.length).to.equal(1); + + el.appendChild(newField); + expect(el.formElements.length).to.equal(2); + + el.removeChild(newField); + expect(el.formElements.length).to.equal(1); + }); +}); diff --git a/packages/fieldset/src/LionFieldset.js b/packages/fieldset/src/LionFieldset.js index c0bd020c0..9adc8a3c1 100644 --- a/packages/fieldset/src/LionFieldset.js +++ b/packages/fieldset/src/LionFieldset.js @@ -3,7 +3,7 @@ import { LionLitElement } from '@lion/core/src/LionLitElement.js'; import { CssClassMixin } from '@lion/core/src/CssClassMixin.js'; import { ObserverMixin } from '@lion/core/src/ObserverMixin.js'; import { ValidateMixin } from '@lion/validate'; -import { FormControlMixin } from '@lion/field'; +import { FormControlMixin, FormRegistrarMixin } from '@lion/field'; // TODO: extract from module like import { pascalCase } from 'lion-element/CaseMapUtils.js' const pascalCase = str => str.charAt(0).toUpperCase() + str.slice(1); @@ -14,8 +14,8 @@ const pascalCase = str => str.charAt(0).toUpperCase() + str.slice(1); * @customElement * @extends LionLitElement */ -export class LionFieldset extends FormControlMixin( - ValidateMixin(CssClassMixin(SlotMixin(ObserverMixin(LionLitElement)))), +export class LionFieldset extends FormRegistrarMixin( + FormControlMixin(ValidateMixin(CssClassMixin(SlotMixin(ObserverMixin(LionLitElement))))), ) { static get properties() { return { @@ -109,8 +109,6 @@ export class LionFieldset extends FormControlMixin( this.addEventListener('focused-changed', this._updateFocusedClass); this.addEventListener('touched-changed', this._updateTouchedClass); this.addEventListener('dirty-changed', this._updateDirtyClass); - this.addEventListener('form-element-register', this.__onFormElementRegister); - this.addEventListener('form-element-unregister', this.__onFormElementUnRegister); this._setRole(); } @@ -121,19 +119,6 @@ export class LionFieldset extends FormControlMixin( this.removeEventListener('focused-changed', this._updateFocusedClass); this.removeEventListener('touched-changed', this._updateTouchedClass); this.removeEventListener('dirty-changed', this._updateDirtyClass); - this.removeEventListener('form-element-register', this.__onFormElementRegister); - this.removeEventListener('form-element-unregister', this.__onFormElementUnRegister); - if (this.__parentFormGroup) { - const event = new CustomEvent('form-element-unregister', { - detail: { element: this }, - bubbles: true, - }); - this.__parentFormGroup.dispatchEvent(event); - } - } - - isRegisteredFormElement(el) { - return Object.keys(this.formElements).some(name => el.name === name); } // eslint-disable-next-line class-methods-use-this @@ -299,10 +284,13 @@ export class LionFieldset extends FormControlMixin( return serializedValues; } - __onFormElementRegister(event) { - const child = event.detail.element; - if (child === this) return; // as we fire and listen - don't add ourselves - + /** + * 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) { const { name } = child; if (!name) { console.info('Error Node:', child); // eslint-disable-line no-console @@ -312,9 +300,9 @@ export class LionFieldset extends FormControlMixin( console.info('Error Node:', child); // eslint-disable-line no-console throw new TypeError(`You can not have the same name "${name}" as your parent`); } - event.stopPropagation(); if (this.disabled) { + // eslint-disable-next-line no-param-reassign child.disabled = true; } if (name.substr(-2) === '[]') { @@ -332,6 +320,7 @@ export class LionFieldset extends FormControlMixin( } // 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 @@ -381,18 +370,15 @@ export class LionFieldset extends FormControlMixin( // might go wrong then when dom order changes per instance. Although we could check if // 'provision' has taken place or not const orderedEls = this._getAriaElementsInRightDomOrder(descriptionElements); - orderedEls.forEach(el => field.addToAriaDescription(el.id)); + orderedEls.forEach(el => { + if (field.addToAriaDescription) { + field.addToAriaDescription(el.id); + } + }); } - __onFormElementUnRegister(event) { - const child = event.detail.element; + removeFormElement(child) { const { name } = child; - if (child === this) { - return; - } // as we fire and listen - don't add ourself - - event.stopPropagation(); - if (name.substr(-2) === '[]' && this.formElements[name]) { const index = this.formElements[name].indexOf(child); if (index > -1) { diff --git a/packages/fieldset/test/lion-fieldset.test.js b/packages/fieldset/test/lion-fieldset.test.js index 13668b0ba..cd33424b4 100644 --- a/packages/fieldset/test/lion-fieldset.test.js +++ b/packages/fieldset/test/lion-fieldset.test.js @@ -77,11 +77,9 @@ describe('', () => { let error = false; const el = await fixture(``); try { - // we need to use the private api here as errors thrown from a web component are in a + // we test the api directly as errors thrown from a web component are in a // different context and we can not catch them here => register fake elements - el.__onFormElementRegister({ - detail: { element: {} }, - }); + el.addFormElement({}); } catch (err) { error = err; } @@ -98,11 +96,9 @@ describe('', () => { let error = false; const el = await fixture(``); try { - // we need to use the private api here as errors thrown from a web component are in a + // we test the api directly as errors thrown from a web component are in a // different context and we can not catch them here => register fake elements - el.__onFormElementRegister({ - detail: { element: { name: 'foo' } }, - }); + el.addFormElement({ name: 'foo' }); } catch (err) { error = err; } @@ -119,16 +115,10 @@ describe('', () => { let error = false; const el = await fixture(``); try { - // we need to use the private api here as errors thrown from a web component are in a + // we test the api directly as errors thrown from a web component are in a // different context and we can not catch them here => register fake elements - el.__onFormElementRegister({ - stopPropagation: () => {}, - detail: { element: { name: 'fooBar', addToAriaDescription: () => {} } }, - }); - el.__onFormElementRegister({ - stopPropagation: () => {}, - detail: { element: { name: 'fooBar' } }, - }); + el.addFormElement({ name: 'fooBar' }); + el.addFormElement({ name: 'fooBar' }); } catch (err) { error = err; } @@ -144,12 +134,10 @@ describe('', () => { it('can dynamically add/remove elements', async () => { const fieldset = await fixture(`<${tagString}>${inputSlotString}`); const newField = await fixture(``); - await nextFrame(); expect(Object.keys(fieldset.formElements).length).to.equal(3); fieldset.appendChild(newField); - await nextFrame(); expect(Object.keys(fieldset.formElements).length).to.equal(4); fieldset.inputElement.removeChild(newField); @@ -163,8 +151,9 @@ describe('', () => { <${tagString} name="newfieldset">${inputSlotString} `); - await nextFrame(); + await fieldset.registrationReady; const newFieldset = fieldset.querySelector('lion-fieldset'); + await newFieldset.registrationReady; fieldset.formElements.lastName.modelValue = 'Bar'; newFieldset.formElements['hobbies[]'][0].modelValue = { checked: true, value: 'chess' }; newFieldset.formElements['hobbies[]'][1].modelValue = { checked: false, value: 'football' }; @@ -649,31 +638,36 @@ describe('', () => { }); it('has correct validation afterwards', async () => { - const isCat = modelValue => ({ isCat: modelValue.value === 'cat' }); - const containsA = modelValues => ({ - containsA: modelValues.color.value ? modelValues.color.value.indexOf('a') > -1 : false, - }); + const isCat = modelValue => ({ isCat: modelValue === 'cat' }); + const containsA = modelValues => { + return { + containsA: modelValues.color ? modelValues.color.indexOf('a') > -1 : false, + }; + }; - const fieldset = await fixture(`<${tagString}>${inputSlotString}`); - await nextFrame(); - fieldset.formElements.color.modelValue = { value: 'onlyb' }; - fieldset.errorValidators = [[containsA]]; - fieldset.formElements.color.errorValidators = [[isCat]]; + const el = await fixture(html` + <${tag} .errorValidators=${[[containsA]]}> + + + + `); + await el.registrationReady; + expect(el.errorState).to.be.true; + expect(el.error.containsA).to.be.true; + expect(el.formElements.color.errorState).to.be.false; - expect(fieldset.errorState).to.equal(true); - expect(fieldset.error.containsA).to.equal(true); - expect(fieldset.formElements.color.error.isCat).to.equal(true); + el.formElements.color.modelValue = 'onlyb'; + expect(el.errorState).to.be.true; + expect(el.error.containsA).to.be.true; + expect(el.formElements.color.error.isCat).to.be.true; - fieldset.formElements.color.modelValue = { value: 'cat' }; - expect(fieldset.errorState).to.equal(false); + el.formElements.color.modelValue = 'cat'; + expect(el.errorState).to.be.false; - fieldset.resetGroup(); - fieldset.formElements.color.modelValue = { value: 'Foo' }; - fieldset.errorValidators = [[containsA]]; - fieldset.formElements.color.errorValidators = [[isCat]]; - expect(fieldset.errorState).to.equal(true); - expect(fieldset.error.containsA).to.equal(true); - expect(fieldset.formElements.color.error.isCat).to.equal(true); + el.resetGroup(); + expect(el.errorState).to.be.true; + expect(el.error.containsA).to.be.true; + expect(el.formElements.color.errorState).to.be.false; }); });