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.

Sign up for GitHub

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: disable validation of internal text-field #1052

Merged
merged 4 commits into from
Sep 21, 2023
Merged

fix: disable validation of internal text-field #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b2ef0b
fix: disable validation of internal text-field
vursen Sep 21, 2023
fc7685a
fix test to assert checkValidity() instead validate()
vursen Sep 21, 2023
ac20fb9
fix tests
vursen Sep 21, 2023
8959a4e
simplify tests
vursen Sep 21, 2023
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
11 changes: 11 additions & 0 deletions src/vaadin-combo-box-light.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,17 @@
return this.getRootNode().activeElement === this.inputElement;
}

/**
* Returns true if the current input value satisfies all constraints (if any).
* @return {boolean}
*/
checkValidity() {
if (this.inputElement && this.inputElement.validate) {
return this.inputElement.validate();
}
return super.checkValidity();
}

/** @protected */
connectedCallback() {
super.connectedCallback();
Expand Down
6 changes: 4 additions & 2 deletions src/vaadin-combo-box-mixin.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,7 @@
/** @private */
_detectAndDispatchChange() {
if (this.value !== this._lastCommittedValue) {
this.validate();
this.dispatchEvent(new CustomEvent('change', {bubbles: true}));
this._lastCommittedValue = this.value;
}
Expand Down Expand Up @@ -1098,6 +1099,7 @@
}
if (!this.readonly && !this._closeOnBlurIsPrevented) {
this._closeOrCommit();
this.validate();
}
}

Expand Down Expand Up @@ -1164,8 +1166,8 @@
* @return {boolean | undefined}
*/
checkValidity() {
if (this.inputElement.validate) {
return this.inputElement.validate();
if (this.inputElement.checkValidity) {
return this.inputElement.checkValidity();
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/vaadin-combo-box.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,13 @@
ready() {
super.ready();

// Disable the internal text-field's validation to prevent it from overriding
// the invalid state that was propagated to the text-field from the combo-box.
this.inputElement.validate = () => {};
// Restore the internal text-field's invalid state in case
// it got overridden before the validation was disabled above.
this.inputElement.invalid = this.invalid;

this._nativeInput = this.inputElement.focusElement;
this._toggleElement = this.$.toggleButton;
this._clearElement = this.inputElement.shadowRoot.querySelector('[part="clear-button"]');
Expand Down
20 changes: 10 additions & 10 deletions test/vaadin-combo-box-light.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,33 +309,33 @@

it('should clear the selection when clicking on the clear button', () => {
comboBox.open();

fire('click', clearButton);

expect(comboBox.value).to.eql('');
expect(comboBox.$.overlay._selectedItem).to.be.null;
expect(comboBox.selectedItem).to.be.null;
});

it('should not close the dropdown after clearing a selection', () => {
comboBox.open();

fire('click', clearButton);

expect(comboBox.opened).to.eql(true);
});

it('should not open the dropdown after clearing a selection', () => {
fire('click', clearButton);

expect(comboBox.opened).to.eql(false);
});

it('should cancel click event to avoid input blur', () => {
comboBox.open();

const event = fire('click', clearButton);

expect(event.defaultPrevented).to.eql(true);
});

Expand Down
100 changes: 95 additions & 5 deletions test/validation.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

<script src="../../web-component-tester/browser.js"></script>
<script src='../../webcomponentsjs/webcomponents-lite.js'></script>
<link rel="import" href="../../iron-test-helpers/mock-interactions.html">
<link rel="import" href="../src/vaadin-combo-box.html">
<link rel="import" href="helpers.html">
<link rel="import" href="../../test-fixture/test-fixture.html">
Expand All @@ -15,10 +16,32 @@
<body>
<test-fixture id="combo-box">
<template>
<vaadin-combo-box></vaadin-combo-box>
<vaadin-combo-box items='["foo"]'></vaadin-combo-box>
</template>
</test-fixture>

<test-fixture id="combo-box-required">
<template>
<vaadin-combo-box required items='["foo"]'></vaadin-combo-box>
</template>
</test-fixture>

<test-fixture id="combo-box-required-value-invalid">
<template>
<vaadin-combo-box required items='["foo"]' value="foo" invalid></vaadin-combo-box>
</template>
</test-fixture>

<script>
function outsideClick() {
// Move focus to body
document.body.tabIndex = 0;
document.body.focus();
document.body.tabIndex = -1;
// Outside click
document.body.click();
}

describe('initial validation', () => {
let comboBox;
let validateSpy;
Expand Down Expand Up @@ -54,11 +77,46 @@
expect(validateSpy.called).to.be.false;
});
});

describe('basic', () => {
let comboBox;
let comboBox, validateSpy, changeSpy;

beforeEach(() => {
beforeEach(async() => {
comboBox = fixture('combo-box');
await nextRender();
validateSpy = sinon.spy(comboBox, 'validate');
changeSpy = sinon.spy();
comboBox.addEventListener('change', changeSpy);
});

it('should disable validation of internal text-field', () => {
comboBox.required = true;
comboBox.inputElement.validate();
expect(comboBox.inputElement.invalid).to.be.false;
});

it('should validate on focusout', () => {
comboBox.inputElement.dispatchEvent(new CustomEvent('focusout', {bubbles: true, composed: true}));
expect(validateSpy.calledOnce).to.be.true;
});

it('should validate before change event on Enter', () => {
comboBox.inputElement.focus();
comboBox.inputElement.value = 'foo';
comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
MockInteractions.pressAndReleaseKeyOn(comboBox.inputElement, 13, null, 'Enter');
expect(validateSpy.calledOnce).to.be.true;
expect(changeSpy.calledOnce).to.be.true;
expect(changeSpy.calledAfter(validateSpy)).to.be.true;
});

it('should validate before change event on clear button click', () => {
comboBox.value = 'foo';
validateSpy.reset();
comboBox.inputElement.$.clearButton.dispatchEvent(new CustomEvent('click', {composed: true, bubbles: true}));
expect(validateSpy.calledOnce).to.be.true;
expect(changeSpy.calledOnce).to.be.true;
expect(changeSpy.calledAfter(validateSpy)).to.be.true;
});

it('should fire a validated event on validation success', () => {
Expand All @@ -82,15 +140,47 @@
expect(event.detail.valid).to.be.false;
});
});

describe('required', () => {
let comboBox;

beforeEach(async() => {
comboBox = fixture('combo-box-required');
await nextRender();
});

it('should pass validation with value', () => {
comboBox.value = 'foo';
expect(comboBox.checkValidity()).to.be.true;
});

it('should fail validation without value', () => {
expect(comboBox.checkValidity()).to.be.false;
});
});

describe('required + value + invalid', () => {
let comboBox;

beforeEach(async() => {
comboBox = fixture('combo-box-required-value-invalid');
await nextRender();
});

it('should propagate invalid state to internal text-field', () => {
expect(comboBox.inputElement.invalid).to.be.true;
});
});

describe('invalid cannot be set to false', () => {
let comboBox;

beforeEach(async() => {
comboBox = fixture('combo-box');
comboBox._shouldSetInvalid = (invalid) => invalid;
await nextRender();
});

it('should set invalid only when it is true', async() => {
comboBox.required = true;
comboBox.validate();
Expand Down