Merge pull request #1461 from ing-bank/feat/filter-fn

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

@@ -6,7 +6,7 @@ import { InteractionStateMixin } from '../InteractionStateMixin.js';
  * @typedef {import('../../types/choice-group/ChoiceGroupMixinTypes').ChoiceGroupMixin} ChoiceGroupMixin
  * @typedef {import('../../types/FormControlMixinTypes').FormControlHost} FormControlHost
  * @typedef {import('../../types/registration/FormRegistrarMixinTypes').ElementWithParentFormGroup} ElementWithParentFormGroup
- * @typedef {FormControlHost & HTMLElement & {_parentFormGroup?:HTMLElement, checked?:boolean}} FormControl
+ * @typedef {import('../../types/form-group/FormGroupMixinTypes').FormControl} FormControl
  * @typedef {import('../../types/choice-group/ChoiceInputMixinTypes').ChoiceInputHost} ChoiceInputHost
  */
 
@@ -207,11 +207,39 @@ 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
+     * @param {(el: FormControl, property?: string) => boolean} [filterFn]
+     * @returns {{[name:string]: any}}
      * @protected
      */
-    _getFromAllFormElements(property, filterCondition = () => true) {
+    _getFromAllFormElements(property, filterFn) {
+      // Prioritizes imperatively passed filter function over the protected method
+      const _filterFn = filterFn || this._getFromAllFormElementsFilter;
+
       // For modelValue, serializedValue and formattedValue, an exception should be made,
       // The reset can be requested from children
       if (
@@ -221,7 +249,7 @@ const ChoiceGroupMixinImplementation = superclass =>
       ) {
         return this[property];
       }
-      return this.formElements.filter(filterCondition).map(el => el.property);
+      return this.formElements.filter(el => _filterFn(el, property)).map(el => el.property);
     }
 
     /**

packages/form-core/src/form-group/FormGroupMixin.js

@@ -9,10 +9,10 @@ import { FormElementsHaveNoError } from './FormElementsHaveNoError.js';
 /**
  * @typedef {import('../../types/form-group/FormGroupMixinTypes').FormGroupMixin} FormGroupMixin
  * @typedef {import('../../types/form-group/FormGroupMixinTypes').FormGroupHost} FormGroupHost
+ * @typedef {import('../../types/form-group/FormGroupMixinTypes').FormControl} FormControl
  * @typedef {import('../../types/FormControlMixinTypes').FormControlHost} FormControlHost
  * @typedef {import('../../types/registration/FormRegisteringMixinTypes').FormRegisteringHost} FormRegisteringHost
  * @typedef {import('../../types/registration/FormRegistrarMixinTypes').ElementWithParentFormGroup} ElementWithParentFormGroup
- * @typedef {FormControlHost & HTMLElement & {_parentFormGroup?: HTMLElement, checked?: boolean, disabled: boolean, hasFeedbackFor: string[], makeRequestToBeDisabled: Function }} FormControl
  */
 
 /**
@@ -317,21 +317,42 @@ 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
+     * @param {(el: FormControl, property?: string) => boolean} [filterFn]
      * @returns {{[name:string]: any}}
      */
-    _getFromAllFormElements(property, filterFn = (/** @type {FormControl} */ el) => !el.disabled) {
+    _getFromAllFormElements(property, filterFn) {
       const result = {};
+
+      // Prioritizes imperatively passed filter function over the protected method
+      const _filterFn = filterFn || this._getFromAllFormElementsFilter;
+
       // @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]);
+          result[name] = elem.filter(el => _filterFn(el, property)).map(el => el[property]);
-        } else if (filterFn(elem)) {
+        } else if (_filterFn(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,126 @@ 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('allows imperatively passing a filter function to _getFromAllFormElements', 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}>
+        `)
+      );
+
+      // @ts-expect-error access protected method
+      expect(el._getFromAllFormElements('serializedValue')).to.eql({
+        b: 'x',
+        newFieldset: {
+          c: 'x',
+        },
+      });
+
+      // @ts-expect-error access protected method
+      expect(el._getFromAllFormElements('serializedValue', () => true)).to.eql({
+        a: 'x',
+        b: 'x',
+        newFieldset: {
+          c: 'x',
+        },
+        disabledFieldset: {},
+      });
+    });
+
     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

@@ -1,6 +1,7 @@
 import { Constructor } from '@open-wc/dedupe-mixin';
 import { LitElement } from '@lion/core';
 import { FormControlHost } from '../FormControlMixinTypes';
+import { FormControl } from '../form-group/FormGroupMixinTypes';
 import { FormRegistrarHost } from '../registration/FormRegistrarMixinTypes';
 import { InteractionStateHost } from '../InteractionStateMixinTypes';
 
@@ -17,7 +18,11 @@ export declare class ChoiceGroupHost {
 
   protected _oldModelValue: any;
   protected _triggerInitialModelValueChangedEvent(): void;
-  protected _getFromAllFormElements(property: string, filterCondition: Function): void;
+  protected _getFromAllFormElementsFilter(el: FormControl, type: string): boolean;
+  protected _getFromAllFormElements(
+    property: string,
+    filterFn?: (el: FormControl, property?: string) => boolean,
+  ): void;
   protected _throwWhenInvalidChildModelValue(child: FormControlHost): void;
   protected _isEmpty(): void;
   protected _checkSingleChoiceElements(ev: Event): void;

packages/form-core/types/form-group/FormGroupMixinTypes.d.ts

@@ -6,6 +6,15 @@ import { FormControlHost } from '../FormControlMixinTypes';
 import { FormRegistrarHost } from '../registration/FormRegistrarMixinTypes';
 import { ValidateHost } from '../validate/ValidateMixinTypes';
 
+export declare type FormControl = FormControlHost &
+  HTMLElement & {
+    _parentFormGroup?: HTMLElement;
+    checked?: boolean;
+    disabled: boolean;
+    hasFeedbackFor: string[];
+    makeRequestToBeDisabled: Function;
+  };
+
 export declare class FormGroupHost {
   /**
    * Disables all formElements in group
@@ -84,9 +93,18 @@ export declare class FormGroupHost {
    */
   protected _getFromAllFormElements(
     property: string,
-    filterFn: (el: FormControlHost) => boolean,
+    filterFn?: (el: FormControl, property?: string) => boolean,
   ): { [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: FormControl, type: string): boolean;
+
   /**
    * Allows to set formElements values via a keyed object structure
    */