feat(form-core): add overridable filter method for form-group children

.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.

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);
     }
 
     /**

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];
           }

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}>
+            <${childTag} name="b" .modelValue="${'x'}"></${childTag}>
+            <${tagSubclass} name="newFieldset">
+              <${childTag} name="c" .modelValue="${'x'}"></${childTag}>
+              <${childTag} name="d" disabled .modelValue="${'x'}"></${childTag}>
+            </${tagSubclass}>
+            <${tagSubclass} name="disabledFieldset" disabled>
+              <${childTag} name="e" .modelValue="${'x'}"></${childTag}>
+            </${tagSubclass}>
+          </${tagSubclass}>
+        `)
+      );
+      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`

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;

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