Merge pull request #1323 from ing-bank/fix/switch

fix(switch|form-core): enhanced feedback visibility configuration / applied on switch / a11y fix

.changeset/famous-wolves-notice.md

@@ -0,0 +1,5 @@
+---
+'@lion/switch': patch
+---
+
+**switch**: clicking label focuses button

.changeset/sharp-laws-scream.md

@@ -0,0 +1,5 @@
+---
+'@lion/switch': patch
+---
+
+- use .\_showFeedbackConditionFor instead of .submitted

.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

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);
+        }
       },
     );
   }

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() {
+    // eslint-disable-next-line class-methods-use-this, no-unused-vars
-      return (this.touched && this.dirty) || this.prefilled || this.submitted;
+    _showFeedbackConditionFor(type, meta) {
+      return (meta.touched && meta.dirty) || meta.prefilled || meta.submitted;
+    }
+
+    get _feedbackConditionMeta() {
+      return {
+        // @ts-ignore
+        ...super._feedbackConditionMeta,
+        submitted: this.submitted,
+        touched: this.touched,
+        dirty: this.dirty,
+        filled: this.filled,
+        prefilled: this.prefilled,
+      };
     }
   };
 

packages/form-core/src/validate/ValidateMixin.js

@@ -628,15 +628,55 @@ 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, meta, defaultCondition) {
+     *   if (type === 'info') {
+     *     return return;
+     *   } else if (type === 'prefilledOnly') {
+     *     return meta.prefilled;
+     *   }
+     *   return defaultCondition(type, meta);
+     * }
+     * @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 +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)) {
@@ -700,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);
     }

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}</${tag}>
+        `));
+
+        expect(el.showsFeedbackFor).to.eql(['error']);
+      });
+
       describe('Events', () => {
         it('fires "showsFeedbackForChanged" event async after feedbackData got synced to feedbackElement', async () => {
           const spy = sinon.spy();

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,13 @@ export declare class InteractionStateHost {
   _iStateOnValueChange(): void;
   _onTouchedChanged(): void;
   _onDirtyChanged(): void;
+
+  showFeedbackConditionFor(
+    type: string,
+    meta: InteractionStates,
+    currentCondition: Function,
+  ): boolean;
+  _feedbackConditionMeta: InteractionStates;
 }
 
 export declare function InteractionStateImplementation<T extends Constructor<LitElement>>(

packages/form-core/types/validate/ValidateMixinTypes.d.ts

@@ -66,10 +66,12 @@ export declare class ValidateHost {
   __isEmpty(v: unknown): boolean;
   __getFeedbackMessages(validators: Validator[]): Promise<FeedbackMessage[]>;
   _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[];
+  _feedbackConditionMeta: object;
 }
 
 export declare function ValidateImplementation<T extends Constructor<LitElement>>(

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() {
@@ -130,4 +129,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();
+  }
 }

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
  */
@@ -39,6 +50,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`<lion-switch label="Enable Setting"></lion-switch>`);
+    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`<lion-switch disabled label="Enable Setting"></lion-switch>`);
+    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`<lion-switch disabled></lion-switch>`);
     const { inputNode } = getProtectedMembers(el);
@@ -126,8 +149,44 @@ describe('lion-switch', () => {
     expect(handlerSpy.callCount).to.equal(1);
   });
 
-  it('is submitted by default', async () => {
+  it('can be configured to show feedback messages immediately', async () => {
-    const el = await fixture(html`<lion-switch></lion-switch>`);
+    const tagName = 'custom-switch';
-    expect(el.submitted).to.be.true;
+    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`<custom-switch
+        .validators="${[new IsTrue({}, { type: 'info' })]}"
+        .showFeedbackConditionFor="${(
+          /** @type {string} */ type,
+          /** @type {object} */ meta,
+          /** @type {(type: string, meta: object) => any} */ defaultCondition,
+        ) => {
+          if (type === 'info') {
+            return true;
+          }
+          return defaultCondition(type, meta);
+        }}"
+      ></custom-switch>`,
+    );
+    expect(el.showsFeedbackFor).to.eql(['info']);
   });
 });