From 0487435252c8dbc74811ed993376ac92d58e99fa Mon Sep 17 00:00:00 2001 From: jorenbroekema Date: Mon, 19 Jul 2021 18:24:05 +0200 Subject: [PATCH] feat(form-core): add overridable filter method for form-group children --- .changeset/seven-rice-smoke.md | 5 ++ .../src/choice-group/ChoiceGroupMixin.js | 31 +++++++++- .../src/form-group/FormGroupMixin.js | 26 ++++++-- .../form-group/FormGroupMixin.suite.js | 60 +++++++++++++++++++ .../choice-group/ChoiceGroupMixinTypes.d.ts | 3 +- .../types/form-group/FormGroupMixinTypes.d.ts | 14 +++-- 6 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 .changeset/seven-rice-smoke.md diff --git a/.changeset/seven-rice-smoke.md b/.changeset/seven-rice-smoke.md new file mode 100644 index 000000000..de264d1d9 --- /dev/null +++ b/.changeset/seven-rice-smoke.md @@ -0,0 +1,5 @@ +--- +'@lion/form-core': patch +--- + +Add a separate protected method for the filter function when filtering out fields for serialized|model|formattedValue in form groups. This makes it easier to specify when you to filter out fields, e.g. disabled fields for serializedValue of a parent form group. diff --git a/packages/form-core/src/choice-group/ChoiceGroupMixin.js b/packages/form-core/src/choice-group/ChoiceGroupMixin.js index 4c0415b17..5fcab6d99 100644 --- a/packages/form-core/src/choice-group/ChoiceGroupMixin.js +++ b/packages/form-core/src/choice-group/ChoiceGroupMixin.js @@ -207,11 +207,34 @@ const ChoiceGroupMixinImplementation = superclass => } /** - * @override + * A filter function which will exclude a form field when returning false + * By default, exclude form fields which are disabled + * + * The type is be passed as well for more fine grained control, e.g. + * distinguish the filter when fetching modelValue versus serializedValue + * + * @param {FormControl} el + * @param {string} type + * @returns {boolean} + */ + // eslint-disable-next-line class-methods-use-this, no-unused-vars + _getFromAllFormElementsFilter(el, type) { + return true; + } + + /** + * Implicit :( @override for FormGroupMixin, as choice fields "fieldsets" + * will always implement both mixins + * + * TODO: Consider making this explicit by extracting this method to its own mixin and + * using it in both FormGroupMixin and ChoiceGroupMixin, then override it here + * This also makes it more DRY as we have same method with similar implementation + * in FormGroupMixin. I (@jorenbroekema) think the abstraction is worth it here.. + * * @param {string} property * @protected */ - _getFromAllFormElements(property, filterCondition = () => true) { + _getFromAllFormElements(property) { // For modelValue, serializedValue and formattedValue, an exception should be made, // The reset can be requested from children if ( @@ -221,7 +244,9 @@ const ChoiceGroupMixinImplementation = superclass => ) { return this[property]; } - return this.formElements.filter(filterCondition).map(el => el.property); + return this.formElements + .filter(el => this._getFromAllFormElementsFilter(el, property)) + .map(el => el.property); } /** diff --git a/packages/form-core/src/form-group/FormGroupMixin.js b/packages/form-core/src/form-group/FormGroupMixin.js index cc0ab2111..44c5fd6c0 100644 --- a/packages/form-core/src/form-group/FormGroupMixin.js +++ b/packages/form-core/src/form-group/FormGroupMixin.js @@ -317,21 +317,39 @@ const FormGroupMixinImplementation = superclass => }); } + /** + * A filter function which will exclude a form field when returning false + * By default, exclude form fields which are disabled + * + * The type is be passed as well for more fine grained control, e.g. + * distinguish the filter when fetching modelValue versus serializedValue + * + * @param {FormControl} el + * @param {string} type + * @returns {boolean} + */ + // eslint-disable-next-line class-methods-use-this, no-unused-vars + _getFromAllFormElementsFilter(el, type) { + return !el.disabled; + } + /** * Gets a keyed be name object for requested property (like modelValue/serializedValue) * @param {string} property * @returns {{[name:string]: any}} */ - _getFromAllFormElements(property, filterFn = (/** @type {FormControl} */ el) => !el.disabled) { + _getFromAllFormElements(property) { const result = {}; // @ts-ignore [allow-protected]: allow Form internals to access this protected method this.formElements._keys().forEach(name => { const elem = this.formElements[name]; if (elem instanceof FormControlsCollection) { - result[name] = elem.filter(el => filterFn(el)).map(el => el[property]); - } else if (filterFn(elem)) { + result[name] = elem + .filter(el => this._getFromAllFormElementsFilter(el, property)) + .map(el => el[property]); + } else if (this._getFromAllFormElementsFilter(elem, property)) { if (typeof elem._getFromAllFormElements === 'function') { - result[name] = elem._getFromAllFormElements(property, filterFn); + result[name] = elem._getFromAllFormElements(property); } else { result[name] = elem[property]; } diff --git a/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js b/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js index 4462828c4..7986bc61d 100644 --- a/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js @@ -323,6 +323,66 @@ export function runFormGroupMixinSuite(cfg = {}) { }); }); + it('allows overriding whether fields are included in when fetching modelValue/serializedValue etc.', async () => { + class FormGroupSubclass extends FormGroupMixin(LitElement) { + constructor() { + super(); + /** @override from FormRegistrarMixin */ + this._isFormOrFieldset = true; + /** @type {'child'|'choice-group'|'fieldset'} */ + this._repropagationRole = 'fieldset'; // configures FormControlMixin + } + + /** + * + * @param {import('../../types/FormControlMixinTypes').FormControlHost & {disabled: boolean}} el + * @param {string} type + */ + _getFromAllFormElementsFilter(el, type) { + if (type === 'serializedValue') { + return !el.disabled; + } + return true; + } + } + + const tagStringSubclass = defineCE(FormGroupSubclass); + const tagSubclass = unsafeStatic(tagStringSubclass); + const el = /** @type {FormGroup} */ ( + await fixture(html` + <${tagSubclass}> + <${childTag} name="a" disabled .modelValue="${'x'}"> + <${childTag} name="b" .modelValue="${'x'}"> + <${tagSubclass} name="newFieldset"> + <${childTag} name="c" .modelValue="${'x'}"> + <${childTag} name="d" disabled .modelValue="${'x'}"> + + <${tagSubclass} name="disabledFieldset" disabled> + <${childTag} name="e" .modelValue="${'x'}"> + + + `) + ); + expect(el.modelValue).to.deep.equal({ + a: 'x', + b: 'x', + newFieldset: { + c: 'x', + d: 'x', + }, + disabledFieldset: { + e: 'x', + }, + }); + + expect(el.serializedValue).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 = /** @type {FormGroup} */ ( await fixture(html` diff --git a/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts b/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts index 7c3ddc2f4..23a5eb489 100644 --- a/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts +++ b/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts @@ -17,7 +17,8 @@ export declare class ChoiceGroupHost { protected _oldModelValue: any; protected _triggerInitialModelValueChangedEvent(): void; - protected _getFromAllFormElements(property: string, filterCondition: Function): void; + protected _getFromAllFormElementsFilter(el: FormControlHost, type: string): boolean; + protected _getFromAllFormElements(property: string): void; protected _throwWhenInvalidChildModelValue(child: FormControlHost): void; protected _isEmpty(): void; protected _checkSingleChoiceElements(ev: Event): void; diff --git a/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts b/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts index 343cd76c1..1975fe1ba 100644 --- a/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts +++ b/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts @@ -82,10 +82,16 @@ export declare class FormGroupHost { /** * Gets a keyed be name object for requested property (like modelValue/serializedValue) */ - protected _getFromAllFormElements( - property: string, - filterFn: (el: FormControlHost) => boolean, - ): { [name: string]: any }; + protected _getFromAllFormElements(property: string): { [name: string]: any }; + + /** + * A filter function which will exclude a form field when returning false + * By default, exclude form fields which are disabled + * + * The type is be passed as well for more fine grained control, e.g. + * distinguish the filter when fetching modelValue versus serializedValue + */ + protected _getFromAllFormElementsFilter(el: FormControlHost, type: string): boolean; /** * Allows to set formElements values via a keyed object structure