fix: ensure no multiple hasInputValue changes on clear button click (#…

…1046)
vaadin · May 17, 2023 · 38a5a13 · 38a5a13
1 parent e33fbdc
commit 38a5a13
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 23 deletions.
diff --git a/src/vaadin-combo-box-mixin.html b/src/vaadin-combo-box-mixin.html
@@ -774,7 +774,9 @@
     _inputValueChanged(e) {
       // Handle only input events from our inputElement.
       if (e.composedPath().indexOf(this.inputElement) !== -1) {
-        this._inputElementValue = this.inputElement[this._propertyForValue];
+        if (!e.__fromClearButton) {
+          this._inputElementValue = this.inputElement[this._propertyForValue];
+        }
         this._filterFromInput(e);
       }
     }

diff --git a/src/vaadin-combo-box.html b/src/vaadin-combo-box.html
@@ -38,7 +38,7 @@
         pattern="[[pattern]]"
         prevent-invalid-input="[[preventInvalidInput]]"
 
-        value="{{_inputElementValue}}"
+        value="[[_inputElementValue]]"
         autocomplete="off"
 
         invalid="[[invalid]]"

diff --git a/test/events.html b/test/events.html
@@ -83,13 +83,13 @@
 
             it('should not fire the event when modifying input', async() => {
               input.value = 'foobar';
-              input.dispatchEvent(new CustomEvent('input'));
+              input.dispatchEvent(new CustomEvent('input', {bubbles: true, composed: true}));
               expect(hasInputValueChangedSpy.called).to.be.false;
             });
 
             it('should fire the event once when removing input', async() => {
               input.value = '';
-              input.dispatchEvent(new CustomEvent('input'));
+              input.dispatchEvent(new CustomEvent('input', {bubbles: true, composed: true}));
               expect(hasInputValueChangedSpy.calledOnce).to.be.true;
               expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
             });
@@ -104,7 +104,7 @@
           describe('without user input', () => {
             it('should fire the event once when entering input', async() => {
               input.value = 'foo';
-              input.dispatchEvent(new CustomEvent('input'));
+              input.dispatchEvent(new CustomEvent('input', {bubbles: true, composed: true}));
               expect(hasInputValueChangedSpy.calledOnce).to.be.true;
               expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
             });
@@ -116,7 +116,7 @@
           });
         });
 
-        describe('with value', () => {
+        describe('with custom value', () => {
           beforeEach(async() => {
             comboBox.clearButtonVisible = true;
             clearButton = comboBox.inputElement.$.clearButton;
@@ -143,6 +143,30 @@
             expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
           });
         });
+
+        describe('with dropdown value', () => {
+          beforeEach(async() => {
+            comboBox.items = ['foo', 'bar', 'baz', 'qux'];
+            comboBox.clearButtonVisible = true;
+            clearButton = comboBox.inputElement.$.clearButton;
+            // Open the combo-box
+            arrowDown(input);
+            // select the first option
+            arrowDown(input);
+            enter(input);
+            valueChangedSpy.reset();
+            hasInputValueChangedSpy.reset();
+          });
+
+          it('should be fired on clear button click', () => {
+            expect(input.value).to.equal('foo');
+            clearButton.click();
+            expect(input.value).to.be.empty;
+            expect(valueChangedSpy.calledOnce).to.be.true;
+            expect(hasInputValueChangedSpy.calledOnce).to.be.true;
+            expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
+          });
+        });
       });
     });
   </script>

diff --git a/test/lazy-loading.html b/test/lazy-loading.html
@@ -83,6 +83,7 @@
           comboBox.inputElement.inputElement.value = value;
         } else {
           comboBox.inputElement.value = value;
+          comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
         }
       };
 
@@ -984,22 +985,6 @@
               expect(comboBox.value).to.equal('other value');
             });
 
-            it('should keep previous value if allow-custom-value is set', () => {
-              comboBox.allowCustomValue = true;
-              comboBox.open();
-              setInputValue('other value');
-              comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
-              comboBox.close();
-              expect(comboBox.value).to.eql('other value');
-
-              comboBox.focus();
-              setInputValue('item 12');
-              comboBox.filter = 'item 12';
-
-              expect(comboBox.value).to.eql('other value');
-              expect(comboBox.inputElement.value).to.eql('other value');
-            });
-
           });
 
           describe('after empty data set loaded', () => {

diff --git a/test/using-object-values.html b/test/using-object-values.html
@@ -335,6 +335,7 @@
 
         combobox.readonly = true;
         combobox.inputElement.value = 'not found';
+        combobox.inputElement.dispatchEvent(new CustomEvent('input'));
         combobox.focus();
         fire('focusout', combobox.inputElement);
 

diff --git a/test/vaadin-combo-box-light-events.html b/test/vaadin-combo-box-light-events.html
@@ -118,7 +118,7 @@
           });
         });
 
-        describe('with value', () => {
+        describe('with custom value', () => {
           beforeEach(async() => {
             comboBox.clearButtonVisible = true;
             clearButton = comboBox.inputElement.$.clearButton;
@@ -145,6 +145,30 @@
             expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
           });
         });
+
+        describe('with dropdown value', () => {
+          beforeEach(async() => {
+            comboBox.items = ['foo', 'bar', 'baz', 'qux'];
+            comboBox.clearButtonVisible = true;
+            clearButton = comboBox.inputElement.$.clearButton;
+            // Open the combo-box
+            arrowDown(input);
+            // select the first option
+            arrowDown(input);
+            enter(input);
+            valueChangedSpy.reset();
+            hasInputValueChangedSpy.reset();
+          });
+
+          it('should be fired on clear button click', () => {
+            expect(input.value).to.equal('foo');
+            clearButton.click();
+            expect(input.value).to.be.empty;
+            expect(valueChangedSpy.calledOnce).to.be.true;
+            expect(hasInputValueChangedSpy.calledOnce).to.be.true;
+            expect(hasInputValueChangedSpy.calledBefore(valueChangedSpy)).to.be.true;
+          });
+        });
       });
     });
   </script>

diff --git a/test/vaadin-combo-box.html b/test/vaadin-combo-box.html
@@ -272,6 +272,7 @@
 
             comboBox.open();
             comboBox.inputElement.value = 'a';
+            comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
             comboBox.$.overlay._selector.querySelector('vaadin-combo-box-item').click();
             expect(spy).to.not.have.been.called;
           });
@@ -282,6 +283,7 @@
 
             comboBox.open();
             comboBox.inputElement.value = 'a';
+            comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
             comboBox.close();
             comboBox.inputElement.blur();
             expect(spy).to.not.have.been.called;
@@ -297,6 +299,7 @@
 
             comboBox.open();
             comboBox.inputElement.value = 'foo';
+            comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
             comboBox.close();
 
             expect(spy.called).to.be.false;
@@ -312,6 +315,7 @@
 
             comboBox.open();
             comboBox.inputElement.value = 'bar';
+            comboBox.inputElement.dispatchEvent(new CustomEvent('input'));
             comboBox.close();
 
             expect(spy.calledOnce).to.be.true;