feat(field): align (pre)filled and empty, fix filled not working

BREAKING: _isPrefilled was removed in favor of _isEmpty. We used to have both, but we decided to align, because they basically do the same thing but opposite. If you were using _isPrefilled, switch to using _isEmpty and just use it in reverse. This change also makes _isEmpty available to all things that implement FormControlMixin. Lastly, filled is available now on all fields that implement InteractionStateMixin

packages/choice-input/src/ChoiceGroupMixin.js

@@ -95,15 +95,14 @@ export const ChoiceGroupMixin = dedupeMixin(
       }
 
       _isEmpty() {
-        const value = this.modelValue;
         if (this.multipleChoice) {
           return this.modelValue.length === 0;
         }
 
-        if (typeof value === 'string' && value === '') {
+        if (typeof this.modelValue === 'string' && this.modelValue === '') {
           return true;
         }
-        if (value === undefined || value === null) {
+        if (this.modelValue === undefined || this.modelValue === null) {
           return true;
         }
         return false;

packages/choice-input/src/ChoiceInputMixin.js

@@ -1,6 +1,6 @@
 /* eslint-disable class-methods-use-this */
 
-import { html, css, nothing } from '@lion/core';
+import { css, html, nothing } from '@lion/core';
 import { FormatMixin } from '@lion/field';
 
 export const ChoiceInputMixin = superclass =>
@@ -151,16 +151,6 @@ export const ChoiceInputMixin = superclass =>
       }
     }
 
-    /**
-     * @override
-     * Override InteractionStateMixin
-     * 'prefilled' should be false when modelValue is { checked: false }, which would return
-     * true in original method (since non-empty objects are considered prefilled by default).
-     */
-    static _isPrefilled(modelValue) {
-      return modelValue.checked;
-    }
-
     /**
      * @override
      * This method is overridden from FormatMixin. It originally fired the normalizing

packages/field/src/FormControlMixin.js

@@ -1,4 +1,5 @@
-import { html, css, nothing, dedupeMixin, SlotMixin } from '@lion/core';
+import { css, dedupeMixin, html, nothing, SlotMixin } from '@lion/core';
+import { Unparseable } from '@lion/validate';
 import { FormRegisteringMixin } from './registration/FormRegisteringMixin.js';
 import { getAriaElementsInRightDomOrder } from './utils/getAriaElementsInRightDomOrder.js';
 
@@ -392,6 +393,25 @@ export const FormControlMixin = dedupeMixin(
         `;
       }
 
+      _isEmpty(modelValue = this.modelValue) {
+        let value = modelValue;
+        if (this.modelValue instanceof Unparseable) {
+          value = this.modelValue.viewValue;
+        }
+
+        // Checks for empty platform types: Objects, Arrays, Dates
+        if (typeof value === 'object' && value !== null && !(value instanceof Date)) {
+          return !Object.keys(value).length;
+        }
+
+        // eslint-disable-next-line no-mixed-operators
+        // Checks for empty platform types: Numbers, Booleans
+        const isNumberValue = typeof value === 'number' && (value === 0 || Number.isNaN(value));
+        const isBooleanValue = typeof value === 'boolean' && value === false;
+
+        return !value && !isNumberValue && !isBooleanValue;
+      }
+
       // eslint-disable-next-line class-methods-use-this
       _feedbackTemplate() {
         return html`

packages/field/src/InteractionStateMixin.js

@@ -1,5 +1,5 @@
 import { dedupeMixin } from '@lion/core';
-import { Unparseable } from '@lion/validate';
+import { FormControlMixin } from './FormControlMixin.js';
 
 /**
  * @desc `InteractionStateMixin` adds meta information about touched and dirty states, that can
@@ -14,7 +14,7 @@ import { Unparseable } from '@lion/validate';
 export const InteractionStateMixin = dedupeMixin(
   superclass =>
     // eslint-disable-next-line no-unused-vars, no-shadow
-    class InteractionStateMixin extends superclass {
+    class InteractionStateMixin extends FormControlMixin(superclass) {
       static get properties() {
         return {
           /**
@@ -31,6 +31,13 @@ export const InteractionStateMixin = dedupeMixin(
             type: Boolean,
             reflect: true,
           },
+          /**
+           * True when the modelValue is non-empty (see _isEmpty in FormControlMixin)
+           */
+          filled: {
+            type: Boolean,
+            reflect: true,
+          },
           /**
            * True when user has left non-empty field or input is prefilled.
            * The name must be seen from the point of view of the input field:
@@ -55,36 +62,25 @@ export const InteractionStateMixin = dedupeMixin(
           this._onTouchedChanged();
         }
 
+        if (name === 'modelValue') {
+          // We do this in _requestUpdate because we don't want to fire another re-render (e.g. when doing this in updated)
+          // Furthermore, we cannot do it on model-value-changed event because it isn't fired initially.
+          this.filled = !this._isEmpty();
+        }
+
         if (name === 'dirty' && this.dirty !== oldVal) {
           this._onDirtyChanged();
         }
       }
 
-      static _isPrefilled(modelValue) {
-        let value = modelValue;
-        if (modelValue instanceof Unparseable) {
-          value = modelValue.viewValue;
-        }
-        // Checks for empty platform types: Objects, Arrays, Dates
-        if (typeof value === 'object' && value !== null && !(value instanceof Date)) {
-          return !!Object.keys(value).length;
-        }
-        // eslint-disable-next-line no-mixed-operators
-        // Checks for empty platform types: Numbers, Booleans
-        const isNumberValue = typeof value === 'number' && (value === 0 || Number.isNaN(value));
-        const isBooleanValue = typeof value === 'boolean' && value === false;
-
-        return !!value || isNumberValue || isBooleanValue;
-      }
-
       constructor() {
         super();
         this.touched = false;
         this.dirty = false;
         this.prefilled = false;
+        this.filled = false;
         this._leaveEvent = 'blur';
         this._valueChangedEvent = 'model-value-changed';
-
         this._iStateOnLeave = this._iStateOnLeave.bind(this);
         this._iStateOnValueChange = this._iStateOnValueChange.bind(this);
       }
@@ -118,23 +114,23 @@ export const InteractionStateMixin = dedupeMixin(
        */
       initInteractionState() {
         this.dirty = false;
-        this.prefilled = this.constructor._isPrefilled(this.modelValue);
+        this.prefilled = !this._isEmpty();
       }
 
       /**
        * Sets touched value to true
        * Reevaluates prefilled state.
        * When false, on next interaction, user will start with a clean state.
-       * @private
+       * @protected
        */
       _iStateOnLeave() {
         this.touched = true;
-        this.prefilled = this.constructor._isPrefilled(this.modelValue);
+        this.prefilled = !this._isEmpty();
       }
 
       /**
        * Sets dirty value and validates when already touched or invalid
-       * @private
+       * @protected
        */
       _iStateOnValueChange() {
         this.dirty = true;
@@ -146,7 +142,7 @@ export const InteractionStateMixin = dedupeMixin(
       resetInteractionState() {
         this.touched = false;
         this.dirty = false;
-        this.prefilled = this.constructor._isPrefilled(this.modelValue);
+        this.prefilled = !this._isEmpty();
       }
 
       _onTouchedChanged() {

packages/field/src/LionField.js

@@ -1,10 +1,10 @@
-import { SlotMixin, LitElement } from '@lion/core';
+import { LitElement, SlotMixin } from '@lion/core';
 import { DisabledMixin } from '@lion/core/src/DisabledMixin.js';
 import { ValidateMixin } from '@lion/validate';
+import { FocusMixin } from './FocusMixin.js';
+import { FormatMixin } from './FormatMixin.js';
 import { FormControlMixin } from './FormControlMixin.js';
 import { InteractionStateMixin } from './InteractionStateMixin.js'; // applies FocusMixin
-import { FormatMixin } from './FormatMixin.js';
-import { FocusMixin } from './FocusMixin.js';
 
 /* eslint-disable wc/guard-super-call */
 
@@ -80,7 +80,6 @@ export class LionField extends FormControlMixin(
     if (this._inputNode) {
       this._setValueAndPreserveCaret(value);
     }
-    this._onValueChanged({ value });
   }
 
   get value() {
@@ -107,7 +106,6 @@ export class LionField extends FormControlMixin(
     // FormatMixin
     this._delegateInitialValueAttr();
     super.connectedCallback();
-
     this._onChange = this._onChange.bind(this);
     this._inputNode.addEventListener('change', this._onChange);
     this.classList.add('form-field'); // eslint-disable-line
@@ -178,16 +176,6 @@ export class LionField extends FormControlMixin(
     );
   }
 
-  _onValueChanged({ value }) {
-    if (super._onValueChanged) {
-      super._onValueChanged();
-    }
-    // For styling purposes, make it known the input field is not empty
-    if (this._inputNode) {
-      this[value ? 'setAttribute' : 'removeAttribute']('filled', '');
-    }
-  }
-
   /**
    * Restores the cursor to its original position after updating the value.
    * @param {string} newValue The value that should be saved.

packages/field/test-suites/InteractionStateMixin.suite.js

@@ -31,9 +31,8 @@ export function runInteractionStateMixinSuite(customConfig) {
 
             set modelValue(v) {
               this._modelValue = v;
-              this.dispatchEvent(
+              this.dispatchEvent(new CustomEvent('model-value-changed', { bubbles: true }));
-                new CustomEvent('model-value-changed', { bubbles: true, composed: true }),
+              this.requestUpdate('modelValue');
-              );
             }
 
             get modelValue() {
@@ -80,6 +79,17 @@ export function runInteractionStateMixinSuite(customConfig) {
       expect(el.hasAttribute('dirty')).to.be.true;
     });
 
+    it('sets an attribute "filled" if the input has a non-empty modelValue', async () => {
+      const el = await fixture(html`<${tag} .modelValue=${'hello'}></${tag}>`);
+      expect(el.hasAttribute('filled')).to.equal(true);
+      el.modelValue = '';
+      await el.updateComplete;
+      expect(el.hasAttribute('filled')).to.equal(false);
+      el.modelValue = 'foo';
+      await el.updateComplete;
+      expect(el.hasAttribute('filled')).to.equal(true);
+    });
+
     it('fires "(touched|dirty)-state-changed" event when state changes', async () => {
       const touchedSpy = sinon.spy();
       const dirtySpy = sinon.spy();

packages/field/test/lion-field.test.js

@@ -1,18 +1,17 @@
+import { unsafeHTML } from '@lion/core';
+import { localize } from '@lion/localize';
+import { localizeTearDown } from '@lion/localize/test-helpers.js';
+import { Required, Validator } from '@lion/validate';
 import {
+  aTimeout,
   expect,
   fixture,
   html,
-  unsafeStatic,
-  triggerFocusFor,
   triggerBlurFor,
-  aTimeout,
+  triggerFocusFor,
+  unsafeStatic,
 } from '@open-wc/testing';
-import { unsafeHTML } from '@lion/core';
 import sinon from 'sinon';
-import { Validator, Required } from '@lion/validate';
-import { localize } from '@lion/localize';
-import { localizeTearDown } from '@lion/localize/test-helpers.js';
-
 import '../lion-field.js';
 
 const tagString = 'lion-field';
@@ -153,17 +152,6 @@ describe('<lion-field>', () => {
     expect(el._inputNode.getAttribute('autocomplete')).to.equal('off');
   });
 
-  it('has an attribute filled if this.value is filled', async () => {
-    const el = await fixture(html`<${tag} value="filled">${inputSlot}</${tag}>`);
-    expect(el.hasAttribute('filled')).to.equal(true);
-    el.value = '';
-    await el.updateComplete;
-    expect(el.hasAttribute('filled')).to.equal(false);
-    el.value = 'bla';
-    await el.updateComplete;
-    expect(el.hasAttribute('filled')).to.equal(true);
-  });
-
   it('preserves the caret position on value change for native text fields (input|textarea)', async () => {
     const el = await fixture(html`<${tag}>${inputSlot}</${tag}>`);
     await triggerFocusFor(el);

packages/fieldset/src/FormGroupMixin.js

@@ -1,6 +1,6 @@
-import { html, dedupeMixin, SlotMixin } from '@lion/core';
+import { dedupeMixin, html, SlotMixin } from '@lion/core';
 import { DisabledMixin } from '@lion/core/src/DisabledMixin.js';
-import { FormControlMixin, FormRegistrarMixin, FormControlsCollection } from '@lion/field';
+import { FormControlMixin, FormControlsCollection, FormRegistrarMixin } from '@lion/field';
 import { getAriaElementsInRightDomOrder } from '@lion/field/src/utils/getAriaElementsInRightDomOrder.js';
 import { ValidateMixin } from '@lion/validate';
 import { FormElementsHaveNoError } from './FormElementsHaveNoError.js';
@@ -361,7 +361,7 @@ export const FormGroupMixin = dedupeMixin(
         }
         // TODO: Unlink in removeFormElement
         this.__linkChildrenMessagesToParent(child);
-        this.validate();
+        this.validate({ clearCurrentResult: true });
       }
 
       /**
@@ -392,7 +392,7 @@ export const FormGroupMixin = dedupeMixin(
        */
       removeFormElement(...args) {
         super.removeFormElement(...args);
-        this.validate();
+        this.validate({ clearCurrentResult: true });
       }
     },
 );

packages/fieldset/test/lion-fieldset.test.js

@@ -1,19 +1,19 @@
+import { LionField } from '@lion/field';
+import '@lion/field/lion-field.js';
+import { localizeTearDown } from '@lion/localize/test-helpers.js';
+import { IsNumber, Validator } from '@lion/validate';
 import {
+  defineCE,
   expect,
   fixtureSync,
   html,
-  unsafeStatic,
-  triggerFocusFor,
   nextFrame,
-  defineCE,
+  triggerFocusFor,
+  unsafeStatic,
 } from '@open-wc/testing';
 import { formFixture as fixture } from '@lion/field/test-helpers.js';
 import sinon from 'sinon';
-import { Validator, IsNumber } from '@lion/validate';
-import { localizeTearDown } from '@lion/localize/test-helpers.js';
 import '../lion-fieldset.js';
-import { LionField } from '@lion/field';
-import '@lion/field/lion-field.js';
 
 const childTagString = defineCE(
   class extends LionField {
@@ -434,6 +434,7 @@ describe('<lion-fieldset>', () => {
       // Edge case: remove all children
       el.removeChild(el.querySelector('[id=c1]'));
       await nextFrame();
+
       expect(el.validationStates.error.HasEvenNumberOfChildren).to.equal(undefined);
     });
   });

packages/form-system/stories/35-System-InteractionStates.stories.mdx

@@ -60,8 +60,8 @@ The example below shows how this is done for checkboxes/radio-inputs.
 /**
  * @override
  */
-static _isPrefilled(modelValue) {
+_isEmpty() {
-  return modelValue.checked;
+  return !this.checked;
 }
 ```
 

packages/validate/src/ValidateMixin.js

@@ -1,6 +1,6 @@
 /* eslint-disable class-methods-use-this, camelcase, no-param-reassign, max-classes-per-file */
 
-import { dedupeMixin, SlotMixin, ScopedElementsMixin } from '@lion/core';
+import { dedupeMixin, ScopedElementsMixin, SlotMixin } from '@lion/core';
 import { localize } from '@lion/localize';
 import { LionValidationFeedback } from './LionValidationFeedback.js';
 import { ResultValidator } from './ResultValidator.js';
@@ -307,10 +307,12 @@ export const ValidateMixin = dedupeMixin(
          * - the validatity is dependent on the formControl type and therefore determined
          * by the formControl.__isEmpty method. Basically, the Required Validator is a means
          * to trigger formControl.__isEmpty.
-         * - when __isEmpty returns false, the input was empty. This means we need to stop
+         * - when __isEmpty returns true, the input was empty. This means we need to stop
          * validation here, because all other Validators' execute functions assume the
          * value is not empty (there would be nothing to validate).
          */
+        // TODO: Try to remove this when we have a single lion form core package, because then we can
+        // depend on FormControlMixin directly, and _isEmpty will always be an existing method on the prototype then
         const isEmpty = this.__isEmpty(value);
         if (isEmpty) {
           if (requiredValidator) {
@@ -423,6 +425,7 @@ export const ValidateMixin = dedupeMixin(
           validationStates[v.type][v.constructor.name] = true;
         });
         this.validationStates = validationStates;
+
         this.hasFeedbackFor = [...new Set(this.__validationResult.map(v => v.type))];
 
         /** private event that should be listened to by LionFieldSet */
@@ -485,7 +488,11 @@ export const ValidateMixin = dedupeMixin(
         // if (typeof this.__isRequired === 'function') {
         //   return !this.__isRequired(v);
         // }
-        return v === null || typeof v === 'undefined' || v === '';
+        return (
+          this.modelValue === null ||
+          typeof this.modelValue === 'undefined' ||
+          this.modelValue === ''
+        );
       }
 
       // ------------------------------------------------------------------------------------------

packages/validate/test-suites/ValidateMixin.suite.js

@@ -665,8 +665,8 @@ export function runValidateMixinSuite(customConfig) {
       it('calls "._isEmpty" when provided (useful for different modelValues)', async () => {
         const customRequiredTagString = defineCE(
           class extends ValidateMixin(LitElement) {
-            _isEmpty(modelValue) {
+            _isEmpty() {
-              return modelValue.model === '';
+              return this.modelValue.model === '';
             }
           },
         );