diff --git a/.changeset/fair-geese-drum.md b/.changeset/fair-geese-drum.md new file mode 100644 index 000000000..678fd3144 --- /dev/null +++ b/.changeset/fair-geese-drum.md @@ -0,0 +1,6 @@ +--- +'@lion/form-core': patch +--- + +- validation of form groups when child fires 'model-value-changed' +- fire {feedbackType}StateChanged event when feedback type (like 'error'/'warning') changed diff --git a/docs/docs/systems/form/interaction-states.md b/docs/docs/systems/form/interaction-states.md index 13a0994ec..6aef30eae 100644 --- a/docs/docs/systems/form/interaction-states.md +++ b/docs/docs/systems/form/interaction-states.md @@ -4,7 +4,8 @@ import { html, render } from '@lion/core'; import { renderLitAsNode } from '@lion/helpers'; import '@lion/input/define'; -import { Validator } from '@lion/form-core'; +import '@lion/input-date/define'; +import { Validator, Unparseable, MinDate, Required } from '@lion/form-core'; import './assets/h-output.js'; ``` @@ -118,7 +119,7 @@ export const feedbackCondition = () => { > `); - // 4. When checkboxes change, set the showFeedbackConditionFor method to a new method that checks + // 4. When checkboxes change, set the feedbackCondition method to a new method that checks // whether every condition that is checked, is true on the field. Otherwise, don't show the feedback. const fetchConditionsAndReevaluate = ({ currentTarget: { modelValue } }) => { fieldElement._showFeedbackConditionFor = type => { @@ -142,3 +143,35 @@ export const feedbackCondition = () => { `; }; ``` + +### Changing the feedback show condition (Application Developers) + +In some situations, the default condition for showing feedback messages might not apply. +The conditions as described in 'When is feedback shown to the user' can be overidden via +the `feedbackCondition` method. +In this example, we want to implement the following situation: + +- for an `input-date`, we have a MinDate condition +- we want to send feedback as soon as we know the user intentionally typed a full Date + (we know if modelValue is not Unparseable) + +```js preview-story +export const feedbackVisibility = () => html` + +`; +``` diff --git a/docs/docs/systems/form/validate.md b/docs/docs/systems/form/validate.md index 2e6e78f6f..b0f88284a 100644 --- a/docs/docs/systems/form/validate.md +++ b/docs/docs/systems/form/validate.md @@ -45,9 +45,11 @@ import { Required, Validator, Pattern, + Unparseable, } from '@lion/form-core'; import { localize } from '@lion/localize'; import { loadDefaultFeedbackMessages } from '@lion/validate-messages'; +import { renderLitAsNode } from '@lion/helpers'; ``` ## When validation happens @@ -804,3 +806,92 @@ export const backendValidation = () => { `; }; ``` + +## Multiple field validation + +When validation is dependent on muliple fields, two approaches can be considered: + +- Fieldset validation +- Validators with knowledge about context + +### Fieldset validation + +Assume we have a field `startDate` and `endDate` field, with condition `startDate` < `endDate`. +The easiest way to achieve this, is by adding a Validator to a fieldset wrapping those fields. + +```js preview-story +const isInterpretable = mv => mv && !(mv instanceof Unparseable); + +class Interval extends Validator { + static get validatorName() { + return 'Interval'; + } + + execute({ startDate, endDate }) { + if (isInterpretable(startDate) && isInterpretable(endDate)) { + return !(startDate < endDate); + } + return false; + } + + static async getMessage() { + return `The start date should be before the end date`; + } +} +export const fieldsetValidation = () => html` + + + + +`; +``` + +### Validators with knowledge about context + +Assume we want to create password validation with a confirmation field. +We don't want to show feedback on group level, but right beneath the fields. + +```js preview-story +const isInterpretableValue = mv => mv && !(mv instanceof Unparseable); + +class PasswordMatch extends Validator { + static get validatorName() { + return 'PasswordsMatch'; + } + + execute(modelValue, { first, second }) { + if (isInterpretableValue(first.modelValue) && isInterpretableValue(second.modelValue)) { + return first.modelValue !== second.modelValue; + } + return false; + } +} +// TODO: use ref directive once on Lit-element 3 +const first = renderLitAsNode(html` + +`); +const second = renderLitAsNode(html` + +`); +first.validators = [ + new PasswordMatch( + { first, second }, + { getMessage: () => 'Please match with confirmation field' }, + ), +]; +second.validators = [ + new PasswordMatch({ first, second }, { getMessage: () => 'Please match with initial field' }), +]; + +export const contextValidation = () => html` ${first}${second} `; +``` diff --git a/packages/form-core/src/InteractionStateMixin.js b/packages/form-core/src/InteractionStateMixin.js index d3a2c44c3..353093aea 100644 --- a/packages/form-core/src/InteractionStateMixin.js +++ b/packages/form-core/src/InteractionStateMixin.js @@ -201,7 +201,7 @@ const InteractionStateMixinImplementation = superclass => get _feedbackConditionMeta() { return { - // @ts-ignore + // @ts-ignore to fix, InteractionStateMixin needs to depend on ValidateMixin ...super._feedbackConditionMeta, submitted: this.submitted, touched: this.touched, diff --git a/packages/form-core/src/LionField.js b/packages/form-core/src/LionField.js index 2392de1a5..7bbe212fc 100644 --- a/packages/form-core/src/LionField.js +++ b/packages/form-core/src/LionField.js @@ -99,4 +99,11 @@ export class LionField extends FormControlMixin( }), ); } + + /** + * @configure InteractionStateMixin, ValidateMixin + */ + get _feedbackConditionMeta() { + return { ...super._feedbackConditionMeta, focused: this.focused }; + } } diff --git a/packages/form-core/src/validate/ValidateMixin.js b/packages/form-core/src/validate/ValidateMixin.js index 8e8e8e165..0ce50cfb1 100644 --- a/packages/form-core/src/validate/ValidateMixin.js +++ b/packages/form-core/src/validate/ValidateMixin.js @@ -85,6 +85,8 @@ export const ValidateMixinImplementation = superclass => * By default, just like the platform, only one message (with highest prio) is visible. */ _visibleMessagesAmount: { attribute: false }, + + __childModelValueChanged: { attribute: false }, }; } @@ -181,6 +183,12 @@ export const ValidateMixinImplementation = superclass => this.__onValidatorUpdated = this.__onValidatorUpdated.bind(this); /** @protected */ this._updateFeedbackComponent = this._updateFeedbackComponent.bind(this); + + /** + * This will be used for FormGroups that listen for `model-value-changed` of children + * @private + */ + this.__childModelValueChanged = false; } connectedCallback() { @@ -200,6 +208,11 @@ export const ValidateMixinImplementation = superclass => super.firstUpdated(changedProperties); this.__validateInitialized = true; this.validate(); + if (this._repropagationRole !== 'child') { + this.addEventListener('model-value-changed', () => { + this.__childModelValueChanged = true; + }); + } } /** @@ -627,8 +640,8 @@ export const ValidateMixinImplementation = superclass => } /** - * The default showFeedbackConditionFor condition that will be used when the - * showFeedbackConditionFor is not overridden. + * The default feedbackCondition condition that will be used when the + * feedbackCondition is not overridden. * Show the validity feedback when returning true, don't show when false * @param {string} type could be 'error', 'warning', 'info', 'success' or any other custom * Validator type @@ -641,17 +654,17 @@ export const ValidateMixinImplementation = superclass => } /** - * Allows super classes to add meta info for showFeedbackConditionFor + * Allows super classes to add meta info for feedbackCondition * @configurable */ get _feedbackConditionMeta() { - return {}; + return { modelValue: this.modelValue, el: this }; } /** * Allows the end user to specify when a feedback message should be shown * @example - * showFeedbackConditionFor(type, meta, defaultCondition) { + * feedbackCondition(type, meta, defaultCondition) { * if (type === 'info') { * return return; * } else if (type === 'prefilledOnly') { @@ -668,7 +681,7 @@ export const ValidateMixinImplementation = superclass => * for other types * @returns {boolean} */ - showFeedbackConditionFor( + feedbackCondition( type, meta = this._feedbackConditionMeta, currentCondition = this._showFeedbackConditionFor.bind(this), @@ -702,9 +715,30 @@ export const ValidateMixinImplementation = superclass => // Necessary typecast because types aren't smart enough to understand that we filter out undefined this.showsFeedbackFor = /** @type {string[]} */ (ctor.validationTypes .map(type => (this._hasFeedbackVisibleFor(type) ? type : undefined)) - .filter(_ => !!_)); + .filter(Boolean)); this._updateFeedbackComponent(); } + + if (changedProperties.has('__childModelValueChanged') && this.__childModelValueChanged) { + this.validate({ clearCurrentResult: true }); + this.__childModelValueChanged = false; + } + + if (changedProperties.has('validationStates')) { + const prevStates = /** @type {{[key: string]: object;}} */ (changedProperties.get( + 'validationStates', + )); + if (prevStates) { + Object.entries(this.validationStates).forEach(([type, feedbackObj]) => { + if ( + prevStates[type] && + JSON.stringify(feedbackObj) !== JSON.stringify(prevStates[type]) + ) { + this.dispatchEvent(new CustomEvent(`${type}StateChanged`, { detail: feedbackObj })); + } + }); + } + } } /** @@ -717,7 +751,7 @@ export const ValidateMixinImplementation = superclass => // Necessary typecast because types aren't smart enough to understand that we filter out undefined const newShouldShowFeedbackFor = /** @type {string[]} */ (ctor.validationTypes .map(type => - this.showFeedbackConditionFor( + this.feedbackCondition( type, this._feedbackConditionMeta, this._showFeedbackConditionFor.bind(this), @@ -725,7 +759,7 @@ export const ValidateMixinImplementation = superclass => ? type : undefined, ) - .filter(_ => !!_)); + .filter(Boolean)); if (JSON.stringify(this.shouldShowFeedbackFor) !== JSON.stringify(newShouldShowFeedbackFor)) { this.shouldShowFeedbackFor = newShouldShowFeedbackFor; @@ -748,7 +782,7 @@ export const ValidateMixinImplementation = superclass => // Sort all validators based on the type provided. const res = validationResult .filter(v => - this.showFeedbackConditionFor( + this.feedbackCondition( v.type, this._feedbackConditionMeta, this._showFeedbackConditionFor.bind(this), diff --git a/packages/form-core/test-suites/ValidateMixin.suite.js b/packages/form-core/test-suites/ValidateMixin.suite.js index c311a7e54..d972975af 100644 --- a/packages/form-core/test-suites/ValidateMixin.suite.js +++ b/packages/form-core/test-suites/ValidateMixin.suite.js @@ -17,6 +17,11 @@ import { AsyncAlwaysInvalid, AsyncAlwaysValid, } from '../test-helpers/index.js'; +import '../define.js'; + +/** + * @typedef {import('../').LionField} LionField + */ /** * @param {{tagString?: string | null, lightDom?: string}} [customConfig] @@ -133,6 +138,20 @@ export function runValidateMixinSuite(customConfig) { expect(validateSpy.callCount).to.equal(1); }); + it('revalidates when child ".modelValue" changes', async () => { + const el = /** @type {ValidateElement} */ (await fixture(html` + <${tag} + ._repropagationRole="${'fieldset'}" + .validators=${[new AlwaysValid()]} + .modelValue=${'myValue'} + > + `)); + const validateSpy = sinon.spy(el, 'validate'); + /** @type {LionField} */ (el.querySelector('#child')).modelValue = 'test'; + await el.updateComplete; + expect(validateSpy.callCount).to.equal(1); + }); + it('revalidates when ".validators" changes', async () => { const el = /** @type {ValidateElement} */ (await fixture(html` <${tag} @@ -867,7 +886,7 @@ export function runValidateMixinSuite(customConfig) { const el = /** @type {ValidateElement} */ (await fixture(html` <${tag} .validators="${[new Required({}, { type: 'error' })]}" - .showFeedbackConditionFor="${( + .feedbackCondition="${( /** @type {string} */ type, /** @type {object} */ meta, /** @type {(type: string) => any} */ defaultCondition, @@ -927,6 +946,30 @@ export function runValidateMixinSuite(customConfig) { await el.updateComplete; expect(spy).to.have.callCount(2); }); + + it('fires "{type}StateChanged" event async when type validity changed', async () => { + const spy = sinon.spy(); + const el = /** @type {ValidateElement} */ (await fixture(html` + <${tag} + .submitted=${true} + .validators=${[new MinLength(7)]} + @errorStateChanged=${spy}; + >${lightDom} + `)); + expect(spy).to.have.callCount(0); + + el.modelValue = 'a'; + await el.updateComplete; + expect(spy).to.have.callCount(1); + + el.modelValue = 'abc'; + await el.updateComplete; + expect(spy).to.have.callCount(1); + + el.modelValue = 'abcdefg'; + await el.updateComplete; + expect(spy).to.have.callCount(2); + }); }); }); 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 c02cefec5..acc0d9fef 100644 --- a/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/packages/form-core/test-suites/form-group/FormGroupMixin.suite.js @@ -83,18 +83,18 @@ export function runFormGroupMixinSuite(cfg = {}) { const el = /** @type {FormGroup} */ (await fixture(html` <${tag} label="foo" .fieldName="${'bar'}">${inputSlots} `)); - // @ts-ignore [allow-proteced] in test + // @ts-ignore [allow-protected] in test expect(el.__fieldName).to.equal(el.fieldName); }); // TODO: Tests below belong to FormRegistrarMixin. Preferably run suite integration test it(`${tagString} has an up to date list of every form element in .formElements`, async () => { const el = /** @type {FormGroup} */ (await fixture(html`<${tag}>${inputSlots}`)); - // @ts-ignore [allow-proteced] in test + // @ts-ignore [allow-protected] in test expect(el.formElements._keys().length).to.equal(3); expect(el.formElements['hobbies[]'].length).to.equal(2); el.removeChild(el.formElements['hobbies[]'][0]); - // @ts-ignore [allow-proteced] in test + // @ts-ignore [allow-protected] in test expect(el.formElements._keys().length).to.equal(3); expect(el.formElements['hobbies[]'].length).to.equal(1); }); diff --git a/packages/form-core/test/lion-field.test.js b/packages/form-core/test/lion-field.test.js index 7c4b3cf0a..ee974616a 100644 --- a/packages/form-core/test/lion-field.test.js +++ b/packages/form-core/test/lion-field.test.js @@ -85,7 +85,7 @@ describe('', () => { const el = /** @type {LionField} */ (await fixture( html`<${tag} label="foo" .fieldName="${'bar'}">${inputSlot}`, )); - // @ts-ignore [allow-proteced] in test + // @ts-ignore [allow-protected] in test expect(el.__fieldName).to.equal(el.fieldName); }); diff --git a/packages/form-core/types/NativeTextFieldMixinTypes.d.ts b/packages/form-core/types/NativeTextFieldMixinTypes.d.ts index 8d1acc53a..fce49290e 100644 --- a/packages/form-core/types/NativeTextFieldMixinTypes.d.ts +++ b/packages/form-core/types/NativeTextFieldMixinTypes.d.ts @@ -1,16 +1,9 @@ import { Constructor } from '@open-wc/dedupe-mixin'; -// import { LionField } from '@lion/form-core/src/LionField'; import { LitElement } from '@lion/core'; import { FocusHost } from '@lion/form-core/types/FocusMixinTypes'; import { FormControlHost } from '@lion/form-core/types/FormControlMixinTypes'; -// export declare class NativeTextField extends LionField { -// protected get _inputNode(): HTMLTextAreaElement | HTMLInputElement; -// } - export declare class NativeTextFieldHost { - // protected get _inputNode(): HTMLTextAreaElement | HTMLInputElement; - get selectionStart(): number; set selectionStart(value: number); get selectionEnd(): number; @@ -26,7 +19,5 @@ export declare function NativeTextFieldImplementation & Constructor & Pick; -// & -// Pick; export type NativeTextFieldMixin = typeof NativeTextFieldImplementation; diff --git a/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts b/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts index a1ee735a6..d9ad83c8d 100644 --- a/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts +++ b/packages/form-core/types/form-group/FormGroupMixinTypes.d.ts @@ -24,8 +24,8 @@ export declare class FormGroupHost { protected _initialModelValue: { [x: string]: any }; protected get _inputNode(): HTMLElement; protected static _addDescriptionElementIdsToField(): void; - protected _setValueForAllFormElements(property: string, value: any): void; + private __descriptionElementsInParentChain: Set; } diff --git a/packages/form-core/types/utils/SyncUpdatableMixinTypes.d.ts b/packages/form-core/types/utils/SyncUpdatableMixinTypes.d.ts index bb50ea174..cc9463d92 100644 --- a/packages/form-core/types/utils/SyncUpdatableMixinTypes.d.ts +++ b/packages/form-core/types/utils/SyncUpdatableMixinTypes.d.ts @@ -1,6 +1,5 @@ import { LitElement } from '@lion/core'; import { Constructor } from '@open-wc/dedupe-mixin'; -import { PropertyValues } from '@lion/core'; export declare interface SyncUpdatableNamespace { connected?: boolean; diff --git a/packages/form-core/types/validate/ValidateMixinTypes.d.ts b/packages/form-core/types/validate/ValidateMixinTypes.d.ts index 66b190794..75ade26b3 100644 --- a/packages/form-core/types/validate/ValidateMixinTypes.d.ts +++ b/packages/form-core/types/validate/ValidateMixinTypes.d.ts @@ -36,6 +36,7 @@ export declare class ValidateHost { protected _visibleMessagesAmount: number; protected _allValidators: Validator[]; + protected get _feedbackConditionMeta(): object; protected get _feedbackNode(): LionValidationFeedback; protected _updateFeedbackComponent(): void; @@ -50,6 +51,7 @@ export declare class ValidateHost { private __validationResult: Validator[]; private __prevValidationResult: Validator[]; private __prevShownValidationResult: Validator[]; + private __childModelValueChanged: boolean; private __storePrevResult(): void; private __executeValidators(): void; diff --git a/packages/listbox/src/LionListbox.js b/packages/listbox/src/LionListbox.js index 33804085d..07227b613 100644 --- a/packages/listbox/src/LionListbox.js +++ b/packages/listbox/src/LionListbox.js @@ -10,4 +10,11 @@ import { ListboxMixin } from './ListboxMixin.js'; */ export class LionListbox extends ListboxMixin( FocusMixin(InteractionStateMixin(ValidateMixin(LitElement))), -) {} +) { + /** + * @configure InteractionStateMixin, ValidateMixin + */ + get _feedbackConditionMeta() { + return { ...super._feedbackConditionMeta, focused: this.focused }; + } +} diff --git a/packages/listbox/test-suites/ListboxMixin.suite.js b/packages/listbox/test-suites/ListboxMixin.suite.js index 8f4e9bcd7..36e908d86 100644 --- a/packages/listbox/test-suites/ListboxMixin.suite.js +++ b/packages/listbox/test-suites/ListboxMixin.suite.js @@ -277,7 +277,7 @@ export function runListboxMixinSuite(customConfig = {}) { const el = await fixture(html` <${tag} label="foo" .fieldName="${'bar'}"> `); - // @ts-ignore [allow-proteced] in test + // @ts-ignore [allow-protected] in test expect(el.__fieldName).to.equal(el.fieldName); }); diff --git a/packages/switch/test/lion-switch.test.js b/packages/switch/test/lion-switch.test.js index 104ca39b8..37cd55abb 100644 --- a/packages/switch/test/lion-switch.test.js +++ b/packages/switch/test/lion-switch.test.js @@ -182,7 +182,7 @@ describe('lion-switch', () => { const el = await fixture( html` any} */ defaultCondition,