Skip to content

Commit

Permalink
fix(form-core): validate on child change + {feedbackType}StateChanged
Browse files Browse the repository at this point in the history
  • Loading branch information
tlouisse committed Apr 12, 2021
1 parent a100fb4 commit 53167fd
Show file tree
Hide file tree
Showing 16 changed files with 245 additions and 32 deletions.
6 changes: 6 additions & 0 deletions .changeset/fair-geese-drum.md
Original file line number Diff line number Diff line change
@@ -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
37 changes: 35 additions & 2 deletions docs/docs/systems/form/interaction-states.md
Original file line number Diff line number Diff line change
Expand Up @@ -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';
```

Expand Down Expand Up @@ -118,7 +119,7 @@ export const feedbackCondition = () => {
></lion-input>
`);

// 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 => {
Expand All @@ -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`
<lion-input-date
.validators="${[
new Required(),
new MinDate(new Date('2000/10/10'), {
getMessage: () => `You provided a correctly formatted date, but it's below MinData`,
}),
]}"
.feedbackCondition="${(type, meta, originalCondition) => {
if (meta.modelValue && !(meta.modelValue instanceof Unparseable)) {
return true;
}
return originalCondition(type, meta);
}}"
help-text="Error appears as soon as a Parseable date before 10/10/2000 is typed"
label="Custom feedback visibility"
></lion-input-date>
`;
```
91 changes: 91 additions & 0 deletions docs/docs/systems/form/validate.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`
<lion-fieldset .validators="${[new Interval()]}">
<lion-input-date name="startDate" label="Start date"></lion-input-date>
<lion-input-date name="endDate" label="End date"></lion-input-date>
</lion-fieldset>
`;
```
### 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`
<lion-input
.feedbackCondition="${(type, meta) => meta.focused}"
type="password"
name="initialPassword"
label="New password"
></lion-input>
`);
const second = renderLitAsNode(html`
<lion-input
.feedbackCondition="${(type, meta) => meta.focused}"
type="password"
name="confirmPassword"
label="Confirm password"
></lion-input>
`);
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} `;
```
2 changes: 1 addition & 1 deletion packages/form-core/src/InteractionStateMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions packages/form-core/src/LionField.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,11 @@ export class LionField extends FormControlMixin(
}),
);
}

/**
* @configure InteractionStateMixin, ValidateMixin
*/
get _feedbackConditionMeta() {
return { ...super._feedbackConditionMeta, focused: this.focused };
}
}
54 changes: 44 additions & 10 deletions packages/form-core/src/validate/ValidateMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
};
}

Expand Down Expand Up @@ -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() {
Expand All @@ -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;
});
}
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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') {
Expand All @@ -668,7 +681,7 @@ export const ValidateMixinImplementation = superclass =>
* for other types
* @returns {boolean}
*/
showFeedbackConditionFor(
feedbackCondition(
type,
meta = this._feedbackConditionMeta,
currentCondition = this._showFeedbackConditionFor.bind(this),
Expand Down Expand Up @@ -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 }));
}
});
}
}
}

/**
Expand All @@ -717,15 +751,15 @@ 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),
)
? type
: undefined,
)
.filter(_ => !!_));
.filter(Boolean));

if (JSON.stringify(this.shouldShowFeedbackFor) !== JSON.stringify(newShouldShowFeedbackFor)) {
this.shouldShowFeedbackFor = newShouldShowFeedbackFor;
Expand All @@ -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),
Expand Down
45 changes: 44 additions & 1 deletion packages/form-core/test-suites/ValidateMixin.suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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'}
><lion-field id="child"><input slot="input"></lion-field></${tag}>
`));
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}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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}</${tag}>
`));
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);
});
});
});

Expand Down

0 comments on commit 53167fd

Please sign in to comment.