Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: sync the hasInputValue state on input #4335

Merged
merged 2 commits into from
Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions packages/field-base/src/input-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -147,16 +147,36 @@ export const InputMixin = dedupingMixin(
}
}

/**
* An input event listener used to update `_hasInputValue` property.
* Do not override this method.
*
* @param {Event} event
* @private
*/
__onInput(event) {
// In the case a custom web component is passed as `inputElement`,
// the actual native input element, on which the event occurred,
// can be inside shadow trees.
const target = event.composedPath()[0];
this._hasInputValue = target.value.length > 0;
this._onInput(event);
}

/**
* An input event listener used to update the field value.
*
* @param {Event} event
* @protected
*/
_onInput(event) {
// In the case a custom web component is passed as `inputElement`,
// the actual native input element, on which the event occurred,
// can be inside shadow trees.
const target = event.composedPath()[0];
// Ignore fake input events e.g. used by clear button.
this.__userInput = event.isTrusted;
this.value = event.target.value;
this.value = target.value;
this.__userInput = false;
}

Expand All @@ -168,18 +188,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
Expand Down
67 changes: 50 additions & 17 deletions packages/field-base/test/input-mixin.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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',
Expand All @@ -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);
Expand All @@ -146,7 +144,6 @@ const runTests = (baseClass) => {
afterEach(() => {
inputSpy.resetHistory();
changeSpy.resetHistory();
hasInputValueSpy.resetHistory();
});

it('should call an input event listener', () => {
Expand All @@ -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);
Expand All @@ -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', () => {
Expand Down