From d4dcb7c1fb9564224272c2f4c221bf615d562b1a Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 8 Apr 2021 13:02:56 +0200 Subject: [PATCH 1/3] chore(combobox): clicking label focuses toggle button --- .changeset/famous-wolves-notice.md | 5 +++++ packages/switch/src/LionSwitch.js | 11 +++++++++++ packages/switch/test/lion-switch.test.js | 12 ++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 .changeset/famous-wolves-notice.md diff --git a/.changeset/famous-wolves-notice.md b/.changeset/famous-wolves-notice.md new file mode 100644 index 000000000..316e201a2 --- /dev/null +++ b/.changeset/famous-wolves-notice.md @@ -0,0 +1,5 @@ +--- +'@lion/switch': patch +--- + +**switch**: clicking label focuses button diff --git a/packages/switch/src/LionSwitch.js b/packages/switch/src/LionSwitch.js index fda6552cc..574b96f6d 100644 --- a/packages/switch/src/LionSwitch.js +++ b/packages/switch/src/LionSwitch.js @@ -130,4 +130,15 @@ export class LionSwitch extends ScopedElementsMixin(ChoiceInputMixin(LionField)) _syncButtonSwitch() { this._inputNode.disabled = this.disabled; } + + /** + * @configure FormControlMixin + * @protected + */ + _onLabelClick() { + if (this.disabled) { + return; + } + this._inputNode.focus(); + } } diff --git a/packages/switch/test/lion-switch.test.js b/packages/switch/test/lion-switch.test.js index d0368e2c5..60c828cad 100644 --- a/packages/switch/test/lion-switch.test.js +++ b/packages/switch/test/lion-switch.test.js @@ -39,6 +39,18 @@ describe('lion-switch', () => { expect(el.checked).to.be.false; }); + it('clicking the label should focus the toggle button', async () => { + const el = await fixture(html``); + el._labelNode.click(); + expect(document.activeElement).to.equal(el._inputNode); + }); + + it('clicking the label should not focus the toggle button when disabled', async () => { + const el = await fixture(html``); + el._labelNode.click(); + expect(document.activeElement).to.not.equal(el._inputNode); + }); + it('should sync its "disabled" state to child button', async () => { const el = await fixture(html``); const { inputNode } = getProtectedMembers(el); From 0e910e3f8065a0ca9c7cd76b888a8679f9d5fd21 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 8 Apr 2021 17:00:19 +0200 Subject: [PATCH 2/3] feat(form-core): allow developers to add .showFeedbackConditionFor --- .changeset/sharp-ravens-buy.md | 5 ++ .../form-core/src/InteractionStateMixin.js | 20 +++++++- .../form-core/src/validate/ValidateMixin.js | 46 +++++++++++++++++-- .../test-suites/ValidateMixin.suite.js | 20 ++++++++ .../types/InteractionStateMixinTypes.d.ts | 17 +++++++ .../types/validate/ValidateMixinTypes.d.ts | 3 +- 6 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 .changeset/sharp-ravens-buy.md diff --git a/.changeset/sharp-ravens-buy.md b/.changeset/sharp-ravens-buy.md new file mode 100644 index 000000000..1f032e6a6 --- /dev/null +++ b/.changeset/sharp-ravens-buy.md @@ -0,0 +1,5 @@ +--- +'@lion/form-core': minor +--- + +allow fine grained feedback visibility control via `.showFeedConditionFor(type, meta, currentCondition)` for Application Developers diff --git a/packages/form-core/src/InteractionStateMixin.js b/packages/form-core/src/InteractionStateMixin.js index 9428ba9d2..6689630ad 100644 --- a/packages/form-core/src/InteractionStateMixin.js +++ b/packages/form-core/src/InteractionStateMixin.js @@ -3,6 +3,7 @@ import { FormControlMixin } from './FormControlMixin.js'; /** * @typedef {import('../types/InteractionStateMixinTypes').InteractionStateMixin} InteractionStateMixin + * @typedef {import('../types/InteractionStateMixinTypes').InteractionStates} InteractionStates */ /** @@ -190,9 +191,24 @@ const InteractionStateMixinImplementation = superclass => * When a user enters a field without altering the value(making it `dirty`), * an error message shouldn't be shown either. * @protected + * @param {string} type + * @param {InteractionStates} meta */ - _showFeedbackConditionFor() { - return (this.touched && this.dirty) || this.prefilled || this.submitted; + // eslint-disable-next-line class-methods-use-this, no-unused-vars + _showFeedbackConditionFor(type, meta) { + const { touched, dirty, prefilled, submitted } = meta; + return (touched && dirty) || prefilled || submitted; + } + + get _feedbackConditionMeta() { + return { + ...super._feedbackConditionMeta, + submitted: this.submitted, + touched: this.touched, + dirty: this.dirty, + filled: this.filled, + prefilled: this.prefilled, + }; } }; diff --git a/packages/form-core/src/validate/ValidateMixin.js b/packages/form-core/src/validate/ValidateMixin.js index ecd8bf008..510829df2 100644 --- a/packages/form-core/src/validate/ValidateMixin.js +++ b/packages/form-core/src/validate/ValidateMixin.js @@ -628,15 +628,53 @@ export const ValidateMixinImplementation = superclass => } /** + * The default showFeedbackConditionFor condition that will be used when the + * showFeedbackConditionFor is not overridden. * Show the validity feedback when returning true, don't show when false - * @param {string} type + * @param {string} type could be 'error', 'warning', 'info', 'success' or any other custom + * Validator type + * @param {object} meta meta info (interaction states etc) * @protected */ // eslint-disable-next-line no-unused-vars - _showFeedbackConditionFor(type) { + _showFeedbackConditionFor(type, meta) { return true; } + /** + * Allows super classes to add meta info for showFeedbackConditionFor + * @configurable + */ + get _feedbackConditionMeta() { + return {}; + } + + /** + * Allows the end user to specify when a feedback message should be shown + * @example + * showFeedbackConditionFor(type, defaultCondition) { + * if (type === 'info') { + * return true; + * } + * return defaultCondition(type); + * } + * @overridable + * @param {string} type could be 'error', 'warning', 'info', 'success' or any other custom + * Validator type + * @param {object} meta meta info (interaction states etc) + * @param {((type: string, meta: object) => boolean)} currentCondition this is the _showFeedbackConditionFor + * that can be used if a developer wants to override for a certain type, but wants to fallback + * for other types + * @returns {boolean} + */ + showFeedbackConditionFor( + type, + meta = this._feedbackConditionMeta, + currentCondition = this._showFeedbackConditionFor.bind(this), + ) { + return currentCondition(type, meta); + } + /** * @param {string} type * @protected @@ -677,7 +715,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(type) ? type : undefined)) + .map(type => (this.showFeedbackConditionFor(type) ? type : undefined)) .filter(_ => !!_)); if (JSON.stringify(this.shouldShowFeedbackFor) !== JSON.stringify(newShouldShowFeedbackFor)) { @@ -700,7 +738,7 @@ export const ValidateMixinImplementation = superclass => const types = ctor.validationTypes; // Sort all validators based on the type provided. const res = validationResult - .filter(v => this._showFeedbackConditionFor(v.type)) + .filter(v => this.showFeedbackConditionFor(v.type)) .sort((a, b) => types.indexOf(a.type) - types.indexOf(b.type)); return res.slice(0, this._visibleMessagesAmount); } diff --git a/packages/form-core/test-suites/ValidateMixin.suite.js b/packages/form-core/test-suites/ValidateMixin.suite.js index 73e044f0e..b728e9558 100644 --- a/packages/form-core/test-suites/ValidateMixin.suite.js +++ b/packages/form-core/test-suites/ValidateMixin.suite.js @@ -841,6 +841,26 @@ export function runValidateMixinSuite(customConfig) { expect(el.validationStates.error).to.not.eql({}); }); + it('can be configured to change visibility conditions per type', async () => { + const el = /** @type {ValidateElement} */ (await fixture(html` + <${tag} + .validators="${[new Required({}, { type: 'error' })]}" + .showFeedbackConditionFor="${( + /** @type {string} */ type, + /** @type {object} */ meta, + /** @type {(type: string) => any} */ defaultCondition, + ) => { + if (type === 'error') { + return true; + } + return defaultCondition(type); + }}" + >${lightDom} + `)); + + expect(el.showsFeedbackFor).to.eql(['error']); + }); + describe('Events', () => { it('fires "showsFeedbackForChanged" event async after feedbackData got synced to feedbackElement', async () => { const spy = sinon.spy(); diff --git a/packages/form-core/types/InteractionStateMixinTypes.d.ts b/packages/form-core/types/InteractionStateMixinTypes.d.ts index bd1949f96..2dbf81a0c 100644 --- a/packages/form-core/types/InteractionStateMixinTypes.d.ts +++ b/packages/form-core/types/InteractionStateMixinTypes.d.ts @@ -2,6 +2,17 @@ import { Constructor } from '@open-wc/dedupe-mixin'; import { LitElement } from '@lion/core'; import { FormControlHost } from './FormControlMixinTypes'; +/** + * A set of meta info about a FormControl that helps in the context of determining validation + * feedback visibility + */ +type InteractionStates = { + submitted: boolean; + touched: boolean; + dirty: boolean; + filled: boolean; + prefilled: boolean; +}; export declare class InteractionStateHost { prefilled: boolean; filled: boolean; @@ -20,6 +31,12 @@ export declare class InteractionStateHost { _iStateOnValueChange(): void; _onTouchedChanged(): void; _onDirtyChanged(): void; + + showFeedbackConditionFor( + type: string, + meta: InteractionStates, + currentCondition: Function, + ): boolean; } export declare function InteractionStateImplementation>( diff --git a/packages/form-core/types/validate/ValidateMixinTypes.d.ts b/packages/form-core/types/validate/ValidateMixinTypes.d.ts index 56a567b72..4e433a91c 100644 --- a/packages/form-core/types/validate/ValidateMixinTypes.d.ts +++ b/packages/form-core/types/validate/ValidateMixinTypes.d.ts @@ -66,7 +66,8 @@ export declare class ValidateHost { __isEmpty(v: unknown): boolean; __getFeedbackMessages(validators: Validator[]): Promise; _updateFeedbackComponent(): void; - _showFeedbackConditionFor(type: string): boolean; + _showFeedbackConditionFor(type: string, meta: object): boolean; + showFeedbackConditionFor(type: string, meta: object, currentCondition: Function): boolean; _hasFeedbackVisibleFor(type: string): boolean; _updateShouldShowFeedbackFor(): void; _prioritizeAndFilterFeedback(opts: { validationResult: Validator[] }): Validator[]; From 652f267b99d92896a236dd4b948abaf9c96eada7 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Thu, 8 Apr 2021 18:16:46 +0200 Subject: [PATCH 3/3] fix(switch): use ._showFeedbackConditionFor instead of .submitted --- .changeset/sharp-laws-scream.md | 5 ++ .../components/interaction/switch/features.md | 7 +++ .../form-core/src/InteractionStateMixin.js | 4 +- .../form-core/src/validate/ValidateMixin.js | 26 +++++++-- .../types/InteractionStateMixinTypes.d.ts | 1 + .../types/validate/ValidateMixinTypes.d.ts | 1 + packages/switch/src/LionSwitch.js | 1 - packages/switch/test/lion-switch.test.js | 55 +++++++++++++++++-- 8 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 .changeset/sharp-laws-scream.md diff --git a/.changeset/sharp-laws-scream.md b/.changeset/sharp-laws-scream.md new file mode 100644 index 000000000..59214743f --- /dev/null +++ b/.changeset/sharp-laws-scream.md @@ -0,0 +1,5 @@ +--- +'@lion/switch': patch +--- + +- use .\_showFeedbackConditionFor instead of .submitted diff --git a/docs/components/interaction/switch/features.md b/docs/components/interaction/switch/features.md index 9eaaea0ce..8f0ea02cc 100644 --- a/docs/components/interaction/switch/features.md +++ b/docs/components/interaction/switch/features.md @@ -41,6 +41,13 @@ export const validation = () => { static get validationTypes() { return [...super.validationTypes, 'info']; } + + _showFeedbackConditionFor(type) { + if (type === 'info') { + return true; + } + return super._showFeedbackConditionFor(type); + } }, ); } diff --git a/packages/form-core/src/InteractionStateMixin.js b/packages/form-core/src/InteractionStateMixin.js index 6689630ad..d3a2c44c3 100644 --- a/packages/form-core/src/InteractionStateMixin.js +++ b/packages/form-core/src/InteractionStateMixin.js @@ -196,12 +196,12 @@ const InteractionStateMixinImplementation = superclass => */ // eslint-disable-next-line class-methods-use-this, no-unused-vars _showFeedbackConditionFor(type, meta) { - const { touched, dirty, prefilled, submitted } = meta; - return (touched && dirty) || prefilled || submitted; + return (meta.touched && meta.dirty) || meta.prefilled || meta.submitted; } get _feedbackConditionMeta() { return { + // @ts-ignore ...super._feedbackConditionMeta, submitted: this.submitted, touched: this.touched, diff --git a/packages/form-core/src/validate/ValidateMixin.js b/packages/form-core/src/validate/ValidateMixin.js index 510829df2..497287dcf 100644 --- a/packages/form-core/src/validate/ValidateMixin.js +++ b/packages/form-core/src/validate/ValidateMixin.js @@ -652,11 +652,13 @@ export const ValidateMixinImplementation = superclass => /** * Allows the end user to specify when a feedback message should be shown * @example - * showFeedbackConditionFor(type, defaultCondition) { + * showFeedbackConditionFor(type, meta, defaultCondition) { * if (type === 'info') { - * return true; + * return return; + * } else if (type === 'prefilledOnly') { + * return meta.prefilled; * } - * return defaultCondition(type); + * return defaultCondition(type, meta); * } * @overridable * @param {string} type could be 'error', 'warning', 'info', 'success' or any other custom @@ -715,7 +717,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(type) ? type : undefined)) + .map(type => + this.showFeedbackConditionFor( + type, + this._feedbackConditionMeta, + this._showFeedbackConditionFor.bind(this), + ) + ? type + : undefined, + ) .filter(_ => !!_)); if (JSON.stringify(this.shouldShowFeedbackFor) !== JSON.stringify(newShouldShowFeedbackFor)) { @@ -738,7 +748,13 @@ export const ValidateMixinImplementation = superclass => const types = ctor.validationTypes; // Sort all validators based on the type provided. const res = validationResult - .filter(v => this.showFeedbackConditionFor(v.type)) + .filter(v => + this.showFeedbackConditionFor( + v.type, + this._feedbackConditionMeta, + this._showFeedbackConditionFor.bind(this), + ), + ) .sort((a, b) => types.indexOf(a.type) - types.indexOf(b.type)); return res.slice(0, this._visibleMessagesAmount); } diff --git a/packages/form-core/types/InteractionStateMixinTypes.d.ts b/packages/form-core/types/InteractionStateMixinTypes.d.ts index 2dbf81a0c..9251f235a 100644 --- a/packages/form-core/types/InteractionStateMixinTypes.d.ts +++ b/packages/form-core/types/InteractionStateMixinTypes.d.ts @@ -37,6 +37,7 @@ export declare class InteractionStateHost { meta: InteractionStates, currentCondition: Function, ): boolean; + _feedbackConditionMeta: InteractionStates; } export declare function InteractionStateImplementation>( diff --git a/packages/form-core/types/validate/ValidateMixinTypes.d.ts b/packages/form-core/types/validate/ValidateMixinTypes.d.ts index 4e433a91c..37cb8ab3d 100644 --- a/packages/form-core/types/validate/ValidateMixinTypes.d.ts +++ b/packages/form-core/types/validate/ValidateMixinTypes.d.ts @@ -71,6 +71,7 @@ export declare class ValidateHost { _hasFeedbackVisibleFor(type: string): boolean; _updateShouldShowFeedbackFor(): void; _prioritizeAndFilterFeedback(opts: { validationResult: Validator[] }): Validator[]; + _feedbackConditionMeta: object; } export declare function ValidateImplementation>( diff --git a/packages/switch/src/LionSwitch.js b/packages/switch/src/LionSwitch.js index 574b96f6d..5bcfed026 100644 --- a/packages/switch/src/LionSwitch.js +++ b/packages/switch/src/LionSwitch.js @@ -91,7 +91,6 @@ export class LionSwitch extends ScopedElementsMixin(ChoiceInputMixin(LionField)) this._labelNode.addEventListener('click', this._toggleChecked); } this._syncButtonSwitch(); - this.submitted = true; } disconnectedCallback() { diff --git a/packages/switch/test/lion-switch.test.js b/packages/switch/test/lion-switch.test.js index 60c828cad..f42682c62 100644 --- a/packages/switch/test/lion-switch.test.js +++ b/packages/switch/test/lion-switch.test.js @@ -1,12 +1,23 @@ import { expect, fixture as _fixture, html } from '@open-wc/testing'; import sinon from 'sinon'; +import { Validator } from '@lion/form-core'; +import { LionSwitch } from '@lion/switch'; import '@lion/switch/define'; /** - * @typedef {import('../src/LionSwitch').LionSwitch} LionSwitch * @typedef {import('@lion/core').TemplateResult} TemplateResult */ +const IsTrue = class extends Validator { + static get validatorName() { + return 'IsTrue'; + } + + execute() { + return true; + } +}; + /** * @param {LionSwitch} lionSwitchEl */ @@ -138,8 +149,44 @@ describe('lion-switch', () => { expect(handlerSpy.callCount).to.equal(1); }); - it('is submitted by default', async () => { - const el = await fixture(html``); - expect(el.submitted).to.be.true; + it('can be configured to show feedback messages immediately', async () => { + const tagName = 'custom-switch'; + if (!customElements.get(tagName)) { + customElements.define( + tagName, + class CustomSwitch extends LionSwitch { + static get validationTypes() { + return [...super.validationTypes, 'info']; + } + + /** + * @param {string} type + * @param {object} meta + */ + _showFeedbackConditionFor(type, meta) { + if (type === 'info') { + return true; + } + return super._showFeedbackConditionFor(type, meta); + } + }, + ); + } + const el = await fixture( + html``, + ); + expect(el.showsFeedbackFor).to.eql(['info']); }); });