feat: FormControlMixin refactor for types safety and default values (#745)

* fix types

* getDefaultValue

* make js private to avoid collisions

* Prefix internals

* BREAKING CHANGE: rename to UUIFormControlMixin

* ValueType type

* append the ValueType type on getDefaultValue method

* refactor for typings and default value

* declare _runValidators method

* undefined Default Value Type

* corrections

Author: nielslyngsoe

packages/uui-avatar-group/lib/uui-avatar-group.story.ts

@@ -1,6 +1,6 @@
 import '.';
 
-import { Story } from '@storybook/web-components';
+import { StoryFn } from '@storybook/web-components';
 import { html } from 'lit';
 import readme from '../README.md?raw';
 
@@ -18,7 +18,7 @@ export default {
   },
 };
 
-export const AAAOverview: Story = props => html`
+export const AAAOverview: StoryFn = props => html`
   <uui-avatar-group
     style="font-size: ${props.fontSize}em; --uui-avatar-border-color: ${props[
       '--uui-avatar-border-color'
@@ -39,7 +39,7 @@ AAAOverview.parameters = {};
 
 AAAOverview.storyName = 'Overview';
 
-export const Limit: Story = ({ limit }) => html`
+export const Limit: StoryFn = ({ limit }) => html`
   <uui-avatar-group
     style="font-size: 2rem; --uui-avatar-border-color: white;"
     .limit=${limit}>

packages/uui-base/lib/events/UUIFormControlEvent.ts

@@ -1,9 +1,9 @@
-import { FormControlMixinInterface } from '../mixins';
+import { UUIFormControlMixinInterface } from '../mixins';
 import { UUIEvent } from './UUIEvent';
 
 export class UUIFormControlEvent extends UUIEvent<
   {},
-  FormControlMixinInterface
+  UUIFormControlMixinInterface<unknown, unknown>
 > {
   constructor(evName: string, eventInit: any | null = {}) {
     super(evName, {

packages/uui-base/lib/mixins/FormControlMixin.ts

@@ -8,23 +8,27 @@ type Constructor<T = {}> = new (...args: any[]) => T;
 type NativeFormControlElement = HTMLInputElement; // Eventually use a specific interface or list multiple options like appending these types: ... | HTMLTextAreaElement | HTMLSelectElement
 
 // TODO: make it possible to define FormDataEntryValue type.
-export declare abstract class FormControlMixinInterface extends LitElement {
+// TODO: Prefix with UUI
+export declare abstract class UUIFormControlMixinInterface<
+  ValueType,
+  DefaultValueType,
+> extends LitElement {
   formAssociated: boolean;
-  get value(): FormDataEntryValue | FormData;
-  set value(newValue: FormDataEntryValue | FormData);
+  protected _internals: ElementInternals;
+  protected _runValidators(): void;
+  get value(): ValueType | DefaultValueType;
+  set value(newValue: ValueType | DefaultValueType);
   name: string;
   formResetCallback(): void;
   checkValidity(): boolean;
   get validationMessage(): string;
   get validity(): ValidityState;
   public setCustomValidity(error: string): void;
   public submit(): void;
-  protected _value: FormDataEntryValue | FormData;
-  protected _internals: any;
   protected abstract getFormElement(): HTMLElement | undefined;
   protected addValidator: (
     flagKey: FlagTypes,
-    getMessageMethod: () => String,
+    getMessageMethod: () => string,
     checkMethod: () => boolean,
   ) => void;
   protected addFormControlElement(element: NativeFormControlElement): void;
@@ -55,7 +59,7 @@ type FlagTypes =
 // Acceptable as an internal interface/type, BUT if exposed externally this should be turned into a public class in a separate file.
 interface Validator {
   flagKey: FlagTypes;
-  getMessageMethod: () => String;
+  getMessageMethod: () => string;
   checkMethod: () => boolean;
 }
 
@@ -65,10 +69,15 @@ interface Validator {
  * @param {Object} superClass - superclass to be extended.
  * @mixin
  */
-export const FormControlMixin = <T extends Constructor<LitElement>>(
+export const UUIFormControlMixin = <
+  ValueType = FormDataEntryValue | FormData,
+  T extends Constructor<LitElement> = typeof LitElement,
+  DefaultValueType = unknown,
+>(
   superClass: T,
+  defaultValue: DefaultValueType,
 ) => {
-  abstract class FormControlMixinClass extends superClass {
+  abstract class UUIFormControlMixinClass extends superClass {
     /**
      * This is a static class field indicating that the element is can be used inside a native form and participate in its events.
      * It may require a polyfill, check support here https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/attachInternals.
@@ -88,23 +97,24 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
 
     /**
      * Value of this form control.
+     * If you dont want the setFormValue to be called on the ElementInternals, then prevent calling this method, by not calling super.value = newValue in your implementation of the value setter method.
      * @type {string}
-     * @attr
+     * @attr value
      * @default ''
      */
     @property() // Do not 'reflect' as the attribute is used as fallback.
-    get value() {
-      return this._value;
+    get value(): ValueType | DefaultValueType {
+      return this.#value;
     }
-    set value(newValue) {
-      const oldValue = this._value;
-      this._value = newValue;
+    set value(newValue: ValueType | DefaultValueType) {
+      const oldValue = this.#value;
+      this.#value = newValue;
       if (
         'ElementInternals' in window &&
         //@ts-ignore
         'setFormValue' in window.ElementInternals.prototype
       ) {
-        this._internals.setFormValue(this._value);
+        this._internals.setFormValue((this.#value as any) ?? null);
       }
       this.requestUpdate('value', oldValue);
     }
@@ -150,15 +160,15 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
     @property({ type: String, attribute: 'error-message' })
     errorMessage = 'This field is invalid';
 
-    private _value: FormDataEntryValue | FormData = '';
-    private _internals: any;
-    private _form: HTMLFormElement | null = null;
-    private _validators: Validator[] = [];
-    private _formCtrlElements: NativeFormControlElement[] = [];
+    #value: ValueType | DefaultValueType = defaultValue;
+    _internals: ElementInternals;
+    #form: HTMLFormElement | null = null;
+    #validators: Validator[] = [];
+    #formCtrlElements: NativeFormControlElement[] = [];
 
     constructor(...args: any[]) {
       super(...args);
-      this._internals = (this as any).attachInternals();
+      this._internals = this.attachInternals();
 
       this.addValidator(
         'valueMissing',
@@ -177,12 +187,12 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
     }
 
     /**
-     * Determn wether this FormControl has a value.
+     * Determine wether this FormControl has a value.
      * @method hasValue
      * @returns {boolean}
      */
     public hasValue(): boolean {
-      return this.value !== '';
+      return this.value !== this.getDefaultValue();
     }
 
     /**
@@ -196,11 +206,11 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
 
     disconnectedCallback(): void {
       super.disconnectedCallback();
-      this._removeFormListeners();
+      this.#removeFormListeners();
     }
-    private _removeFormListeners() {
-      if (this._form) {
-        this._form.removeEventListener('submit', this._onFormSubmit);
+    #removeFormListeners() {
+      if (this.#form) {
+        this.#form.removeEventListener('submit', this.#onFormSubmit);
       }
     }
 
@@ -221,22 +231,22 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
      */
     protected addValidator(
       flagKey: FlagTypes,
-      getMessageMethod: () => String,
+      getMessageMethod: () => string,
       checkMethod: () => boolean,
     ): Validator {
       const obj = {
         flagKey: flagKey,
         getMessageMethod: getMessageMethod,
         checkMethod: checkMethod,
       };
-      this._validators.push(obj);
+      this.#validators.push(obj);
       return obj;
     }
 
     protected removeValidator(validator: Validator) {
-      const index = this._validators.indexOf(validator);
+      const index = this.#validators.indexOf(validator);
       if (index !== -1) {
-        this._validators.splice(index, 1);
+        this.#validators.splice(index, 1);
       }
     }
 
@@ -246,7 +256,7 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
      * @param element {NativeFormControlElement} - element to validate and include as part of this form association.
      */
     protected addFormControlElement(element: NativeFormControlElement) {
-      this._formCtrlElements.push(element);
+      this.#formCtrlElements.push(element);
     }
 
     private _customValidityObject?: Validator;
@@ -273,11 +283,18 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
       this._runValidators();
     }
 
-    private _runValidators() {
+    /**
+     * @method _runValidators
+     * @description Run all validators and set the validityState of this form control.
+     * Run this method when you want to re-run all validators.
+     * This can be relevant if you have a validators that is using values that is not triggering the Lit Updated Callback.
+     * Such are mainly properties that are not declared as a Lit state and or Lit property.
+     */
+    protected _runValidators() {
       this._validityState = {};
 
       // Loop through inner native form controls to adapt their validityState.
-      this._formCtrlElements.forEach(formCtrlEl => {
+      this.#formCtrlElements.forEach(formCtrlEl => {
         for (const key in formCtrlEl.validity) {
           if (key !== 'valid' && (formCtrlEl.validity as any)[key]) {
             (this as any)._validityState[key] = true;
@@ -291,7 +308,7 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
       });
 
       // Loop through custom validators, currently its intentional to have them overwritten native validity. but might need to be reconsidered (This current way enables to overwrite with custom messages)
-      this._validators.forEach(validator => {
+      this.#validators.forEach(validator => {
         if (validator.checkMethod()) {
           this._validityState[validator.flagKey] = true;
           this._internals.setValidity(
@@ -322,33 +339,40 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
       this._runValidators();
     }
 
-    private _onFormSubmit = () => {
+    #onFormSubmit = () => {
       this.pristine = false;
     };
 
     public submit() {
-      this._form?.requestSubmit();
+      this.#form?.requestSubmit();
     }
 
     public formAssociatedCallback() {
-      this._removeFormListeners();
-      this._form = this._internals.form;
-      if (this._form) {
+      this.#removeFormListeners();
+      this.#form = this._internals.form;
+      if (this.#form) {
         // This relies on the form begin a 'uui-form':
-        if (this._form.hasAttribute('submit-invalid')) {
+        if (this.#form.hasAttribute('submit-invalid')) {
           this.pristine = false;
         }
-        this._form.addEventListener('submit', this._onFormSubmit);
+        this.#form.addEventListener('submit', this.#onFormSubmit);
       }
     }
     public formResetCallback() {
       this.pristine = true;
-      this.value = this.getAttribute('value') || '';
+      this.value = this.getInitialValue() ?? this.getDefaultValue();
+    }
+
+    protected getDefaultValue(): DefaultValueType {
+      return defaultValue;
+    }
+    protected getInitialValue(): ValueType | DefaultValueType {
+      return this.getAttribute('value') as ValueType | DefaultValueType;
     }
 
     public checkValidity() {
-      for (const key in this._formCtrlElements) {
-        if (this._formCtrlElements[key].checkValidity() === false) {
+      for (const key in this.#formCtrlElements) {
+        if (this.#formCtrlElements[key].checkValidity() === false) {
           return false;
         }
       }
@@ -365,6 +389,8 @@ export const FormControlMixin = <T extends Constructor<LitElement>>(
       return this._internals?.validationMessage;
     }
   }
-  return FormControlMixinClass as unknown as Constructor<FormControlMixinInterface> &
+  return UUIFormControlMixinClass as unknown as Constructor<
+    UUIFormControlMixinInterface<ValueType, DefaultValueType>
+  > &
     T;
 };

packages/uui-boolean-input/lib/uui-boolean-input.element.ts

@@ -1,4 +1,7 @@
-import { FormControlMixin, LabelMixin } from '@umbraco-ui/uui-base/lib/mixins';
+import {
+  UUIFormControlMixin,
+  LabelMixin,
+} from '@umbraco-ui/uui-base/lib/mixins';
 import { css, html, LitElement, TemplateResult } from 'lit';
 import { property, query } from 'lit/decorators.js';
 
@@ -12,15 +15,18 @@ type LabelPosition = 'left' | 'right' | 'top' | 'bottom';
  * @fires UUIBooleanInputEvent#change on change
  * @abstract
  */
-export abstract class UUIBooleanInputElement extends FormControlMixin(
+export abstract class UUIBooleanInputElement extends UUIFormControlMixin(
   LabelMixin('', LitElement),
+  '',
 ) {
+  private _value = '';
+
   /** intentional overwrite of FormControlMixins value getter and setter method. */
   get value() {
-    return this._value as string;
+    return this._value;
   }
   set value(newVal: string) {
-    const oldValue = this._value;
+    const oldValue = super.value;
     this._value = newVal;
     if (
       'ElementInternals' in window &&

packages/uui-boolean-input/lib/uui-boolean-input.test.ts

@@ -75,7 +75,7 @@ describe('UUIBooleanInputElement', () => {
     expect(input?.checked).to.equal(true);
   });
   it('emits an change event when the input changes', async () => {
-    const listener = oneEvent(element, UUIBooleanInputEvent.CHANGE);
+    const listener = oneEvent(element, UUIBooleanInputEvent.CHANGE, false);
     label.click();
 
     const event = await listener;
@@ -128,7 +128,7 @@ describe('BooleanInputBaseElement in a Form', () => {
 
   describe('submit', () => {
     it('should submit when pressing enter', async () => {
-      const listener = oneEvent(formElement, 'submit');
+      const listener = oneEvent(formElement, 'submit', false);
       element.dispatchEvent(new KeyboardEvent('keypress', { key: 'Enter' }));
 
       const event = await listener;

packages/uui-button/lib/uui-button.element.ts

@@ -4,7 +4,7 @@ import {
 } from '@umbraco-ui/uui-base/lib/animations';
 import { demandCustomElement } from '@umbraco-ui/uui-base/lib/utils';
 import {
-  FormControlMixin,
+  UUIFormControlMixin,
   LabelMixin,
   PopoverTargetMixin,
 } from '@umbraco-ui/uui-base/lib/mixins';
@@ -49,8 +49,9 @@ export type UUIButtonType = 'submit' | 'button' | 'reset';
  *  @cssprop --uui-button-content-align - Overwrite justify-content alignment. Possible values: 'left', 'right', 'center'.
  */
 @defineElement('uui-button')
-export class UUIButtonElement extends FormControlMixin(
+export class UUIButtonElement extends UUIFormControlMixin(
   LabelMixin('', PopoverTargetMixin(LitElement)),
+  undefined,
 ) {
   /**
    * Specifies the type of button

packages/uui-button/lib/uui-button.story.ts

@@ -435,7 +435,7 @@ export const MultiLine: Story = props => {
 
 MultiLine.args = { look: 'primary' };
 
-export const ContentAlignment = props => html`
+export const ContentAlignment: Story = props => html`
   <uui-button
     style="max-width: 400px; width: 100%;
       --uui-button-content-align: ${props['--uui-button-content-align']}"

packages/uui-button/lib/uui-button.test.ts

@@ -120,7 +120,7 @@ describe('UuiButton', () => {
   describe('events', () => {
     describe('click', async () => {
       it('emits a click event when native button fires one', async () => {
-        const listener = oneEvent(element, 'click');
+        const listener = oneEvent(element, 'click', false);
 
         button.click();