From d7f7ffa2214c0b503c8584b986111b3956d5ad71 Mon Sep 17 00:00:00 2001
From: Thijs Louisse <Thijs.Louisse@ing.com>
Date: Mon, 15 Jul 2019 15:00:02 +0200
Subject: [PATCH] chore(field): small cleanup LionField

---
 packages/field/src/LionField.js        | 92 +++++++++++++-------------
 packages/field/test/lion-field.test.js | 36 +++++-----
 2 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/packages/field/src/LionField.js b/packages/field/src/LionField.js
index 9b06e0c0f..05cd34c81 100644
--- a/packages/field/src/LionField.js
+++ b/packages/field/src/LionField.js
@@ -1,39 +1,43 @@
-import { DelegateMixin, SlotMixin } from '@lion/core';
-import { LionLitElement } from '@lion/core/src/LionLitElement.js';
+import { DelegateMixin, SlotMixin, LitElement } from '@lion/core';
 import { ElementMixin } from '@lion/core/src/ElementMixin.js';
 import { CssClassMixin } from '@lion/core/src/CssClassMixin.js';
 import { ObserverMixin } from '@lion/core/src/ObserverMixin.js';
 import { ValidateMixin } from '@lion/validate';
-
 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 */
+
+// TODO:
+// - Consider exporting as FieldMixin
+// - Add submitted prop to InteractionStateMixin
+// - Find a better way to do value delegation via attr
+
 /**
- * LionField: wraps components input, textarea and select and potentially others
- * (checkbox group, radio group)
+ * `LionField`: wraps <input>, <textarea>, <select> and other interactable elements.
  * Also it would follow a nice hierarchy: lion-form -> lion-fieldset -> lion-field
  *
+ * Note: We don't support placeholders, because we have a helper text and
+ * placeholders confuse the user with accessibility needs.
+ *
+ * Please see the docs for in depth information.
+ *
+ * @example
  * <lion-field name="myName">
  *   <label slot="label">My Input</label>
  *   <input type="text" slot="input">
  * </lion-field>
  *
- * Note: We do not support placeholders, because we have a helper text and
- * placeholders confuse the user with accessibility needs.
- *
  * @customElement
  */
-
-// TODO: Consider exporting as FieldMixin
-// eslint-disable-next-line max-len, no-unused-vars
 export class LionField extends FormControlMixin(
   InteractionStateMixin(
     FocusMixin(
       FormatMixin(
         ValidateMixin(
-          CssClassMixin(ElementMixin(DelegateMixin(SlotMixin(ObserverMixin(LionLitElement))))),
+          CssClassMixin(ElementMixin(DelegateMixin(SlotMixin(ObserverMixin(LitElement))))),
         ),
       ),
     ),
@@ -65,14 +69,7 @@ export class LionField extends FormControlMixin(
     };
   }
 
-  static get asyncObservers() {
-    return {
-      ...super.asyncObservers,
-      _setDisabledClass: ['disabled'],
-    };
-  }
-
-  // We don't delegate, because we want to 'preprocess' via _setValueAndPreserveCaret
+  // We don't delegate, because we want to preserve caret position via _setValueAndPreserveCaret
   set value(value) {
     // if not yet connected to dom can't change the value
     if (this.inputElement) {
@@ -85,29 +82,26 @@ export class LionField extends FormControlMixin(
     return (this.inputElement && this.inputElement.value) || '';
   }
 
-  _setDisabledClass() {
-    this.classList[this.disabled ? 'add' : 'remove']('state-disabled');
+  static get asyncObservers() {
+    return {
+      ...super.asyncObservers,
+      _setDisabledClass: ['disabled'],
+    };
   }
 
-  resetInteractionState() {
-    if (super.resetInteractionState) super.resetInteractionState();
-    // TODO: add submitted prop to InteractionStateMixin ?
-    this.submitted = false;
-  }
-
-  /* * * * * * * *
-    Lifecycle  */
   connectedCallback() {
     super.connectedCallback();
+
     this._onChange = this._onChange.bind(this);
     this.inputElement.addEventListener('change', this._onChange);
-    this._delegateInitialValueAttr(); // TODO: find a better way to do this
+    this._delegateInitialValueAttr();
     this._setDisabledClass();
-    this.classList.add('form-field');
+    this.classList.add('form-field'); // eslint-disable-line
   }
 
   disconnectedCallback() {
     super.disconnectedCallback();
+
     if (this.__parentFormGroup) {
       const event = new CustomEvent('form-element-unregister', {
         detail: { element: this },
@@ -118,6 +112,10 @@ export class LionField extends FormControlMixin(
     this.inputElement.removeEventListener('change', this._onChange);
   }
 
+  _setDisabledClass() {
+    this.classList[this.disabled ? 'add' : 'remove']('state-disabled');
+  }
+
   /**
    * This is not done via 'get delegations', because this.inputElement.setAttribute('value')
    * does not trigger a value change
@@ -129,33 +127,37 @@ export class LionField extends FormControlMixin(
     }
   }
 
-  /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
-    Public Methods (also notice delegated methods that are available on host)  */
+  resetInteractionState() {
+    if (super.resetInteractionState) {
+      super.resetInteractionState();
+    }
+    this.submitted = false;
+  }
 
   clear() {
-    // Let validationMixin and interactionStateMixin clear their invalid and dirty/touched states
-    // respectively
-    if (super.clear) super.clear();
+    if (super.clear) {
+      // Let validationMixin and interactionStateMixin clear their
+      // invalid and dirty/touched states respectively
+      super.clear();
+    }
     this.value = ''; // can't set null here, because IE11 treats it as a string
   }
 
-  /* * * * * * * * * *
-    Event Handlers */
-
   _onChange() {
-    if (super._onChange) super._onChange();
+    if (super._onChange) {
+      super._onChange();
+    }
     this.dispatchEvent(
       new CustomEvent('user-input-changed', {
         bubbles: true,
       }),
     );
-    this.modelValue = this.parser(this.value);
   }
 
-  /* * * * * * * * * * * *
-    Observer Handlers  */
   _onValueChanged({ value }) {
-    if (super._onValueChanged) super._onValueChanged();
+    if (super._onValueChanged) {
+      super._onValueChanged();
+    }
     // For styling purposes, make it known the input field is not empty
     this.classList[value ? 'add' : 'remove']('state-filled');
   }
diff --git a/packages/field/test/lion-field.test.js b/packages/field/test/lion-field.test.js
index 0ff06278f..5fbdb7d5f 100644
--- a/packages/field/test/lion-field.test.js
+++ b/packages/field/test/lion-field.test.js
@@ -5,6 +5,7 @@ import {
   unsafeStatic,
   triggerFocusFor,
   triggerBlurFor,
+  aTimeout,
 } from '@open-wc/testing';
 import { unsafeHTML } from '@lion/core';
 import sinon from 'sinon';
@@ -152,25 +153,24 @@ describe('<lion-field>', () => {
     expect(lionField.inputElement.selectionEnd).to.equal(2);
   });
 
-  // TODO: add pointerEvents test
-  // TODO: why is this a describe?
-  describe(`<lion-field> with <input disabled>${nameSuffix}`, () => {
-    it('has a class "state-disabled"', async () => {
-      const lionField = await fixture(`<${tagString}>${inputSlotString}</${tagString}>`);
-      expect(lionField.classList.contains('state-disabled')).to.equal(false);
-      expect(lionField.inputElement.hasAttribute('disabled')).to.equal(false);
+  // TODO: add pointerEvents test for disabled
+  it('has a class "state-disabled"', async () => {
+    const lionField = await fixture(`<${tagString}>${inputSlotString}</${tagString}>`);
+    expect(lionField.classList.contains('state-disabled')).to.equal(false);
+    expect(lionField.inputElement.hasAttribute('disabled')).to.equal(false);
 
-      lionField.disabled = true;
-      await lionField.updateComplete;
-      expect(lionField.classList.contains('state-disabled')).to.equal(true);
-      expect(lionField.inputElement.hasAttribute('disabled')).to.equal(true);
+    lionField.disabled = true;
+    await lionField.updateComplete;
+    await aTimeout();
 
-      const disabledlionField = await fixture(
-        `<${tagString} disabled>${inputSlotString}</${tagString}>`,
-      );
-      expect(disabledlionField.classList.contains('state-disabled')).to.equal(true);
-      expect(disabledlionField.inputElement.hasAttribute('disabled')).to.equal(true);
-    });
+    expect(lionField.classList.contains('state-disabled')).to.equal(true);
+    expect(lionField.inputElement.hasAttribute('disabled')).to.equal(true);
+
+    const disabledlionField = await fixture(
+      `<${tagString} disabled>${inputSlotString}</${tagString}>`,
+    );
+    expect(disabledlionField.classList.contains('state-disabled')).to.equal(true);
+    expect(disabledlionField.inputElement.hasAttribute('disabled')).to.equal(true);
   });
 
   describe(`A11y${nameSuffix}`, () => {
@@ -355,7 +355,7 @@ describe('<lion-field>', () => {
 
       mimicUserInput(lionField, 'foo');
       expect(formatterSpy.callCount).to.equal(1);
-      expect(lionField.formattedValue).to.equal('foo');
+      expect(lionField.value).to.equal('foo');
     });
   });