ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(form-core): only show success after recovery from shown feedback

Browse source
This commit is contained in:
Joren Broekema 2021-03-10 17:17:06 +01:00
parent 82722168f1
commit af90b20ed0
8 changed files with 161 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/orange-walls-sniff.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/form-core': patch
---
Only show success feedback if the user is recovering from a shown error/warning.

4
packages/form-core/src/validate/ResultValidator.js
View file

@ -10,12 +10,12 @@ export class ResultValidator extends Validator {
/**
* @param {Object} context
* @param {Validator[]} context.regularValidationResult
* @param {Validator[]} context.prevValidationResult
* @param {string} context.prevShownValidationFeedback
* @param {Validator[]} [context.validators]
* @returns {boolean}
*/
// eslint-disable-next-line no-unused-vars, class-methods-use-this
executeOnResults({ regularValidationResult, prevValidationResult, validators }) {
executeOnResults({ regularValidationResult, prevShownValidationFeedback, validators }) {
return true;
}
}

24
packages/form-core/src/validate/ValidateMixin.js
View file

@ -161,8 +161,8 @@ export const ValidateMixinImplementation = superclass =>
* @type {Validator[]}
*/
this.__validationResult = [];
/** @type {Validator[]} */
this.__prevValidationResult = [];
/** @type {string} */
this.__prevShownValidationFeedback = '';
this.__onValidatorUpdated = this.__onValidatorUpdated.bind(this);
this._updateFeedbackComponent = this._updateFeedbackComponent.bind(this);
@ -279,7 +279,6 @@ export const ValidateMixinImplementation = superclass =>
return;
}
this.__storePrevResult();
if (clearCurrentResult) {
// Clear ('invalidate') all pending and existing validation results.
// This is needed because we have async (pending) validators whose results
@ -289,10 +288,6 @@ export const ValidateMixinImplementation = superclass =>
await this.__executeValidators();
}
__storePrevResult() {
this.__prevValidationResult = this.__validationResult;
}
/**
* @desc step A1-3 + B (as explained in 'validate')
*/
@ -401,7 +396,7 @@ export const ValidateMixinImplementation = superclass =>
return resultValidators.filter(v =>
v.executeOnResults({
regularValidationResult,
prevValidationResult: this.__prevValidationResult,
prevShownValidationFeedback: this.__prevShownValidationFeedback,
}),
);
}
@ -582,8 +577,12 @@ export const ValidateMixinImplementation = superclass =>
this.__prioritizedResult = this._prioritizeAndFilterFeedback({
validationResult: this.__validationResult,
});
const messageMap = await this.__getFeedbackMessages(this.__prioritizedResult);
if (this.__prioritizedResult.length > 0) {
this.__prevShownValidationFeedback = this.__prioritizedResult[0].type;
}
const messageMap = await this.__getFeedbackMessages(this.__prioritizedResult);
_feedbackNode.feedbackData = messageMap.length ? messageMap : [];
});
} else {
@ -636,10 +635,15 @@ export const ValidateMixinImplementation = superclass =>
_updateShouldShowFeedbackFor() {
const ctor = /** @type {typeof import('../../types/validate/ValidateMixinTypes').ValidateHost} */ (this
.constructor);
// Necessary typecast because types aren't smart enough to understand that we filter out undefined
this.shouldShowFeedbackFor = /** @type {string[]} */ (ctor.validationTypes
const newShouldShowFeedbackFor = /** @type {string[]} */ (ctor.validationTypes
.map(type => (this._showFeedbackConditionFor(type) ? type : undefined))
.filter(_ => !!_));
if (JSON.stringify(this.shouldShowFeedbackFor) !== JSON.stringify(newShouldShowFeedbackFor)) {
this.shouldShowFeedbackFor = newShouldShowFeedbackFor;
}
}
/**

11
packages/form-core/src/validate/resultValidators/DefaultSuccess.js
View file

@ -17,15 +17,18 @@ export class DefaultSuccess extends ResultValidator {
*
* @param {Object} context
* @param {Validator[]} context.regularValidationResult
* @param {Validator[]} context.prevValidationResult
* @param {string} context.prevShownValidationFeedback
* @returns {boolean}
*/
// eslint-disable-next-line class-methods-use-this
executeOnResults({ regularValidationResult, prevValidationResult }) {
executeOnResults({ regularValidationResult, prevShownValidationFeedback }) {
const errorOrWarning = /** @param {Validator} v */ v =>
v.type === 'error' || v.type === 'warning';
const hasErrorOrWarning = !!regularValidationResult.filter(errorOrWarning).length;
const prevHadErrorOrWarning = !!prevValidationResult.filter(errorOrWarning).length;
return !hasErrorOrWarning && prevHadErrorOrWarning;
return (
!hasErrorOrWarning &&
(prevShownValidationFeedback === 'error' || prevShownValidationFeedback === 'warning')
);
}
}

20
packages/form-core/test-suites/ValidateMixin.suite.js
View file

@ -548,15 +548,21 @@ export function runValidateMixinSuite(customConfig) {
/**
*
* @param {{ regularValidationResult: Validator[], prevValidationResult: Validator[]}} param0
* @param {Object} context
* @param {Validator[]} context.regularValidationResult
* @param {string} context.prevShownValidationFeedback
* @returns {boolean}
*/
// eslint-disable-next-line class-methods-use-this
executeOnResults({ regularValidationResult, prevValidationResult }) {
executeOnResults({ regularValidationResult, prevShownValidationFeedback }) {
const errorOrWarning = /** @param {Validator} v */ v =>
v.type === 'error' || v.type === 'warning';
const hasErrorOrWarning = !!regularValidationResult.filter(errorOrWarning).length;
const prevHadErrorOrWarning = !!prevValidationResult.filter(errorOrWarning).length;
return !hasErrorOrWarning && prevHadErrorOrWarning;
return (
!hasErrorOrWarning &&
(prevShownValidationFeedback === 'error' || prevShownValidationFeedback === 'warning')
);
}
};
@ -607,15 +613,15 @@ export function runValidateMixinSuite(customConfig) {
.modelValue=${'myValue'}
>${lightDom}</${withSuccessTag}>
`));
const prevValidationResult = el.__prevValidationResult;
const prevShownValidationFeedback = el.__prevShownValidationFeedback;
const regularValidationResult = [
...el.__syncValidationResult,
...el.__asyncValidationResult,
];
expect(resultValidateSpy.args[0][0]).to.eql({
prevValidationResult,
regularValidationResult,
prevShownValidationFeedback,
});
});
@ -1140,7 +1146,7 @@ export function runValidateMixinSuite(customConfig) {
>${lightDom}</${elTag}>
`));
const spy = sinon.spy(el, '_updateFeedbackComponent');
const spy = sinon.spy(el, '_updateShouldShowFeedbackFor');
let counter = 0;
// for ... of is already allowed we should update eslint...
// eslint-disable-next-line no-restricted-syntax

9
packages/form-core/test/validate/ResultValidator.test.js
View file

@ -15,18 +15,19 @@ describe('ResultValidator', () => {
*
* @param {Object} context
* @param {Validator[]} context.regularValidationResult
* @param {Validator[]} context.prevValidationResult
* @param {string} context.prevShownValidationFeedback
* @returns {boolean}
*/
executeOnResults({ regularValidationResult, prevValidationResult }) {
const hasSuccess = regularValidationResult.length && !prevValidationResult.length;
executeOnResults({ regularValidationResult, prevShownValidationFeedback }) {
const hasSuccess =
regularValidationResult.length && prevShownValidationFeedback === 'error';
return !!hasSuccess;
}
}
expect(
new MyResultValidator().executeOnResults({
regularValidationResult: [new Required(), new MinLength(3)],
prevValidationResult: [],
prevShownValidationFeedback: 'error',
}),
).to.be.true;
});

2
packages/form-core/types/validate/ValidateMixinTypes.d.ts vendored
View file

@ -37,7 +37,7 @@ export declare class ValidateHost {
__syncValidationResult: Validator[];
__asyncValidationResult: Validator[];
__validationResult: Validator[];
__prevValidationResult: Validator[];
__prevShownValidationFeedback: string;
connectedCallback(): void;
disconnectedCallback(): void;

114
packages/form-integrations/test/form-validation-integrations.test.js Normal file
View file

@ -0,0 +1,114 @@
import { expect, fixture, html, defineCE, unsafeStatic } from '@open-wc/testing';
import { Required, DefaultSuccess, Validator } from '@lion/form-core';
import { loadDefaultFeedbackMessages } from '@lion/validate-messages';
import { LionInput } from '@lion/input';
import sinon from 'sinon';
describe('Form Validation Integrations', () => {
const lightDom = '';
before(() => {
loadDefaultFeedbackMessages();
});
describe('DefaultSuccess', () => {
it('does not show success feedback if the user is not recovering from a shown error/warning feedback', async () => {
class WarnValidator extends Validator {
/**
*
* @param {?} [param]
* @param {Object.<string,?>} [config]
*/
constructor(param, config) {
super(param, config);
this.type = 'warning';
}
/** @param {string} value */
execute(value) {
let hasError = false;
if (value === 'warn') {
hasError = true;
}
return hasError;
}
}
class ValidateElementCustomTypes extends LionInput {
static get validationTypes() {
return ['error', 'warning', 'success'];
}
}
const elTagString = defineCE(ValidateElementCustomTypes);
const elTag = unsafeStatic(elTagString);
const el = /** @type {ValidateElementCustomTypes} */ (await fixture(html`
<${elTag}
.validators=${[
new Required(null, { getMessage: () => 'error' }),
new WarnValidator(null, { getMessage: () => 'warning' }),
new DefaultSuccess(),
]}
>${lightDom}</${elTag}>
`));
expect(el._feedbackNode.feedbackData?.length).to.equal(0);
el.modelValue = 'w';
el.touched = true;
await el.updateComplete;
await el.feedbackComplete;
expect(el.showsFeedbackFor).to.eql([]);
el.modelValue = 'warn';
await el.updateComplete;
await el.feedbackComplete;
expect(el._feedbackNode.feedbackData?.[0].message).to.equal('warning');
el.modelValue = 'war';
await el.updateComplete;
await el.feedbackComplete;
expect([
'Okay',
'Correct',
'Succeeded',
'Ok!',
'This is right.',
'Changed!',
'Ok, correct.',
]).to.include(
/** @type {{ message: string ;type: string; validator?: Validator | undefined;}[]} */ (el
._feedbackNode.feedbackData)[0].message,
);
el.modelValue = '';
await el.updateComplete;
await el.feedbackComplete;
expect(el._feedbackNode.feedbackData?.[0].message).to.equal('error');
el.modelValue = 'war';
await el.updateComplete;
await el.feedbackComplete;
expect([
'Okay',
'Correct',
'Succeeded',
'Ok!',
'This is right.',
'Changed!',
'Ok, correct.',
]).to.include(
/** @type {{ message: string ;type: string; validator?: Validator | undefined;}[]} */ (el
._feedbackNode.feedbackData)[0].message,
);
// Check that change in focused or other interaction states does not refresh the success message
// without a change in validation results
const spy = sinon.spy(el, '_updateFeedbackComponent');
el._updateShouldShowFeedbackFor();
await el.updateComplete;
await el.feedbackComplete;
expect(spy.called).to.be.false;
});
});
});