From 77567efcd18d04bf93a3890f10516ba183ca85cf Mon Sep 17 00:00:00 2001 From: jorenbroekema Date: Mon, 26 Jul 2021 10:04:29 +0200 Subject: [PATCH] chore: allow overriding filter fn imperatively --- .../src/choice-group/ChoiceGroupMixin.js | 13 ++-- .../src/form-group/FormGroupMixin.js | 15 +++-- .../form-group/FormGroupMixin.suite.js | 60 +++++++++++++++++++ .../choice-group/ChoiceGroupMixinTypes.d.ts | 8 ++- .../types/form-group/FormGroupMixinTypes.d.ts | 16 ++++- 5 files changed, 97 insertions(+), 15 deletions(-) diff --git a/packages/form-core/src/choice-group/ChoiceGroupMixin.js b/packages/form-core/src/choice-group/ChoiceGroupMixin.js index 5fcab6d99..5985e985d 100644 --- a/packages/form-core/src/choice-group/ChoiceGroupMixin.js +++ b/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 */ @@ -232,9 +232,14 @@ const ChoiceGroupMixinImplementation = superclass => * 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) { + _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 ( @@ -244,9 +249,7 @@ const ChoiceGroupMixinImplementation = superclass => ) { return this[property]; } - return this.formElements - .filter(el => this._getFromAllFormElementsFilter(el, property)) - .map(el => el.property); + return this.formElements.filter(el => _filterFn(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 44c5fd6c0..d455d9b3b 100644 --- a/packages/form-core/src/form-group/FormGroupMixin.js +++ b/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 */ /** @@ -336,18 +336,21 @@ const FormGroupMixinImplementation = superclass => /** * 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) { + _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 => this._getFromAllFormElementsFilter(el, property)) - .map(el => el[property]); - } else if (this._getFromAllFormElementsFilter(elem, property)) { + result[name] = elem.filter(el => _filterFn(el, property)).map(el => el[property]); + } else if (_filterFn(elem, property)) { if (typeof elem._getFromAllFormElements === 'function') { result[name] = elem._getFromAllFormElements(property); } else { 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 7986bc61d..992dc36fc 100644 --- a/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js @@ -383,6 +383,66 @@ export function runFormGroupMixinSuite(cfg = {}) { }); }); + 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} 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'}"> + + + `) + ); + + // @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` diff --git a/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts b/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts index 23a5eb489..ae47f7fac 100644 --- a/packages/form-core/types/choice-group/ChoiceGroupMixinTypes.d.ts +++ b/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,8 +18,11 @@ export declare class ChoiceGroupHost { protected _oldModelValue: any; protected _triggerInitialModelValueChangedEvent(): void; - protected _getFromAllFormElementsFilter(el: FormControlHost, type: string): boolean; - protected _getFromAllFormElements(property: string): 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; diff --git a/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts b/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts index 1975fe1ba..8e4b004b0 100644 --- a/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts +++ b/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 @@ -82,7 +91,10 @@ export declare class FormGroupHost { /** * Gets a keyed be name object for requested property (like modelValue/serializedValue) */ - protected _getFromAllFormElements(property: string): { [name: string]: any }; + protected _getFromAllFormElements( + property: string, + filterFn?: (el: FormControl, property?: string) => boolean, + ): { [name: string]: any }; /** * A filter function which will exclude a form field when returning false @@ -91,7 +103,7 @@ export declare class FormGroupHost { * 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; + protected _getFromAllFormElementsFilter(el: FormControl, type: string): boolean; /** * Allows to set formElements values via a keyed object structure