From 91a5e1ace0602f1903ab38a7535241335d05935a Mon Sep 17 00:00:00 2001
From: Sergey Vinogradov <mr.vursen@gmail.com>
Date: Thu, 27 Jul 2023 14:22:35 +0300
Subject: [PATCH] fix: ensure validation between value-changed and change
 events

---
 .../select/src/vaadin-select-base-mixin.js    | 36 ++++++++++++++-----
 packages/select/test/select.common.js         |  3 +-
 packages/select/test/validation.common.js     | 10 ++++--
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/packages/select/src/vaadin-select-base-mixin.js b/packages/select/src/vaadin-select-base-mixin.js
index 96173f2ddb4..51aa47ced3b 100644
--- a/packages/select/src/vaadin-select-base-mixin.js
+++ b/packages/select/src/vaadin-select-base-mixin.js
@@ -259,8 +259,7 @@ export const SelectBaseMixin = (superClass) =>
         menuElement.addEventListener(
           'click',
           () => {
-            this.__userInteraction = true;
-            this.opened = false;
+            this.__dispatchChangePending = true;
           },
           true,
         );
@@ -281,8 +280,10 @@ export const SelectBaseMixin = (superClass) =>
     _valueChanged(value, oldValue) {
       this.toggleAttribute('has-value', Boolean(value));
 
-      // Validate only if `value` changes after initialization.
-      if (oldValue !== undefined) {
+      // Skip validation during initialization and when
+      // a change event is scheduled, as validation will be
+      // triggered by `__dispatchChange()` in that case.
+      if (oldValue !== undefined && !this.__dispatchChangePending) {
         this.validate();
       }
     }
@@ -322,7 +323,7 @@ export const SelectBaseMixin = (superClass) =>
           const currentIdx = selected !== undefined ? selected : -1;
           const newIdx = this._menuElement._searchKey(currentIdx, e.key);
           if (newIdx >= 0) {
-            this.__userInteraction = true;
+            this.__dispatchChangePending = true;
 
             // Announce the value selected with the first letter shortcut
             this._updateAriaLive(true);
@@ -371,7 +372,12 @@ export const SelectBaseMixin = (superClass) =>
         if (this._openedWithFocusRing) {
           this.setAttribute('focus-ring', '');
         }
-        this.validate();
+
+        // Skip validation when a change event is scheduled, as validation
+        // will be triggered by `__dispatchChange()` in that case.
+        if (!this.__dispatchChangePending) {
+          this.validate();
+        }
       }
     }
 
@@ -508,10 +514,9 @@ export const SelectBaseMixin = (superClass) =>
         if (!this._valueChanging) {
           this._selectedChanging = true;
           this.value = selected.value || '';
-          if (this.__userInteraction) {
+          if (this.__dispatchChangePending) {
             this.opened = false;
-            this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
-            this.__userInteraction = false;
+            this.__dispatchChange();
           }
           delete this._selectedChanging;
         }
@@ -600,4 +605,17 @@ export const SelectBaseMixin = (superClass) =>
         listBox.appendChild(this.__createItemElement(item));
       });
     }
+
+    /** @private */
+    async __dispatchChange() {
+      // Wait for the update complete to guarantee that value-changed is fired
+      // before validated and change events when using the Lit version of the component.
+      if (this.updateComplete) {
+        await this.updateComplete;
+      }
+
+      this.validate();
+      this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
+      this.__dispatchChangePending = false;
+    }
   };
diff --git a/packages/select/test/select.common.js b/packages/select/test/select.common.js
index 76f0cd0c07f..4fd5743cd05 100644
--- a/packages/select/test/select.common.js
+++ b/packages/select/test/select.common.js
@@ -634,10 +634,11 @@ describe('vaadin-select', () => {
         expect(changeSpy.called).to.be.false;
       });
 
-      it('should fire `change` event when value changes when alphanumeric keys are pressed', () => {
+      it('should fire `change` event when value changes when alphanumeric keys are pressed', async () => {
         keyDownChar(valueButton, 'o');
         keyDownChar(valueButton, 'p');
         keyDownChar(valueButton, 't');
+        await nextUpdate(select);
         expect(changeSpy.callCount).to.equal(1);
       });
 
diff --git a/packages/select/test/validation.common.js b/packages/select/test/validation.common.js
index 5a8294434b6..e17bca374ff 100644
--- a/packages/select/test/validation.common.js
+++ b/packages/select/test/validation.common.js
@@ -6,7 +6,7 @@ import '@vaadin/item/vaadin-item.js';
 import '@vaadin/list-box/vaadin-list-box.js';
 
 describe('validation', () => {
-  let select, validateSpy, changeSpy;
+  let select, validateSpy, valueChangedSpy, changeSpy;
 
   describe('basic', () => {
     beforeEach(async () => {
@@ -19,6 +19,8 @@ describe('validation', () => {
       validateSpy = sinon.spy(select, 'validate');
       changeSpy = sinon.spy();
       select.addEventListener('change', changeSpy);
+      valueChangedSpy = sinon.spy();
+      select.addEventListener('value-changed', valueChangedSpy);
     });
 
     it('should pass validation by default', () => {
@@ -48,15 +50,17 @@ describe('validation', () => {
       expect(validateSpy.called).to.be.true;
     });
 
-    it('should validate before change event on Enter', async () => {
+    it('should validate between value-changed and change events on Enter', async () => {
       select.focus();
       select.click();
       await nextRender();
 
       await sendKeys({ press: 'Enter' });
       await nextUpdate(select);
+      expect(valueChangedSpy.calledOnce).to.be.true;
+      expect(validateSpy.calledOnce).to.be.true;
       expect(changeSpy.calledOnce).to.be.true;
-      expect(validateSpy.called).to.be.true;
+      expect(validateSpy.calledAfter(valueChangedSpy)).to.be.true;
       expect(validateSpy.calledBefore(changeSpy)).to.be.true;
     });