fix: fire has-input-value-changed event on input

vaadin · Aug 10, 2022 · 2e33dc2 · 2e33dc2
1 parent 2a9e261
commit 2e33dc2
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 31 deletions.
diff --git a/packages/field-base/src/input-mixin.js b/packages/field-base/src/input-mixin.js
@@ -68,8 +68,8 @@ export const InputMixin = dedupingMixin(
       constructor() {
         super();
 
-        this._boundOnInput = this._onInput.bind(this);
-        this._boundOnChange = this.__onChange.bind(this);
+        this._boundOnInput = this.__onInput.bind(this);
+        this._boundOnChange = this._onChange.bind(this);
       }
 
       /**
@@ -147,6 +147,18 @@ export const InputMixin = dedupingMixin(
         }
       }
 
+      /**
+       * An input event listener used to update `_hasInputValue` property.
+       * Do not override this method.
+       *
+       * @param {Event} event
+       * @private
+       */
+      __onInput(event) {
+        this._hasInputValue = event.target.value.length > 0;
+        this._onInput(event);
+      }
+
       /**
        * An input event listener used to update the field value.
        *
@@ -168,18 +180,6 @@ export const InputMixin = dedupingMixin(
        */
       _onChange(_event) {}
 
-      /**
-       * A change event listener used to update `_hasInputValue` property.
-       * Do not override this method.
-       *
-       * @param {Event} event
-       * @private
-       */
-      __onChange(event) {
-        this._hasInputValue = event.target.value.length > 0;
-        this._onChange(event);
-      }
-
       /**
        * Toggle the has-value attribute based on the value property.
        * @param {boolean} hasValue

diff --git a/packages/field-base/test/input-mixin.test.js b/packages/field-base/test/input-mixin.test.js
@@ -1,5 +1,5 @@
 import { expect } from '@esm-bundle/chai';
-import { fixtureSync, nextFrame, nextRender } from '@vaadin/testing-helpers';
+import { fire, fixtureSync, nextFrame, nextRender } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import { InputMixin } from '../src/input-mixin.js';
 import { define } from './helpers.js';
@@ -110,12 +110,11 @@ const runTests = (baseClass) => {
   });
 
   describe('events', () => {
-    let eventsTag, inputSpy, changeSpy, hasInputValueSpy;
+    let eventsTag, inputSpy, changeSpy;
 
     before(() => {
       inputSpy = sinon.spy();
       changeSpy = sinon.spy();
-      hasInputValueSpy = sinon.spy();
 
       eventsTag = define[baseClass](
         'input-mixin-events',
@@ -135,7 +134,6 @@ const runTests = (baseClass) => {
 
     beforeEach(async () => {
       element = fixtureSync(`<${eventsTag}></${eventsTag}>`);
-      element.addEventListener('has-input-value-changed', hasInputValueSpy);
       await nextRender();
       input = document.createElement('input');
       element.appendChild(input);
@@ -146,7 +144,6 @@ const runTests = (baseClass) => {
     afterEach(() => {
       inputSpy.resetHistory();
       changeSpy.resetHistory();
-      hasInputValueSpy.resetHistory();
     });
 
     it('should call an input event listener', () => {
@@ -159,18 +156,6 @@ const runTests = (baseClass) => {
       expect(changeSpy.calledOnce).to.be.true;
     });
 
-    it('should fire has-input-value-changed event on user value change', async () => {
-      input.value = 'foo';
-      input.dispatchEvent(new CustomEvent('change'));
-      await nextFrame();
-
-      input.value = '';
-      input.dispatchEvent(new CustomEvent('change'));
-      await nextFrame();
-
-      expect(hasInputValueSpy.calledTwice).to.be.true;
-    });
-
     it('should not call an input event listener when input is unset', async () => {
       element.removeChild(input);
       element._setInputElement(undefined);
@@ -187,6 +172,54 @@ const runTests = (baseClass) => {
       expect(changeSpy.called).to.be.false;
     });
   });
+
+  describe('has-input-value-changed event', () => {
+    let tag, hasInputValueChangedSpy;
+
+    before(() => {
+      tag = define[baseClass](
+        'input-mixin-has-input-value-changed-event',
+        '<slot name="input"></slot>',
+        (Base) => class extends InputMixin(Base) {},
+      );
+    });
+
+    beforeEach(async () => {
+      hasInputValueChangedSpy = sinon.spy();
+      element = fixtureSync(`<${tag}></${tag}>`);
+      element.addEventListener('has-input-value-changed', hasInputValueChangedSpy);
+      await nextRender();
+      input = document.createElement('input');
+      element.appendChild(input);
+      element._setInputElement(input);
+      await nextFrame();
+    });
+
+    it('should fire the event on input value presence change', async () => {
+      input.value = 'foo';
+      fire(input, 'input');
+      await nextFrame();
+      expect(hasInputValueChangedSpy.calledOnce).to.be.true;
+
+      hasInputValueChangedSpy.resetHistory();
+      input.value = '';
+      fire(input, 'input');
+      await nextFrame();
+      expect(hasInputValueChangedSpy.calledOnce).to.be.true;
+    });
+
+    it('should not fire the event on input if input value presence has not changed', async () => {
+      input.value = 'foo';
+      fire(input, 'input');
+      await nextFrame();
+      hasInputValueChangedSpy.resetHistory();
+
+      input.value = 'foobar';
+      fire(input, 'input');
+      await nextFrame();
+      expect(hasInputValueChangedSpy.called).to.be.false;
+    });
+  });
 };
 
 describe('InputMixin + Polymer', () => {