fix: do not close select overlay when opened before attached (#8402)

vaadin · Dec 27, 2024 · f12e44c · f12e44c
1 parent c90066f
commit f12e44c
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 15 deletions.
diff --git a/packages/select/src/vaadin-select-base-mixin.js b/packages/select/src/vaadin-select-base-mixin.js
@@ -64,7 +64,6 @@ export const SelectBaseMixin = (superClass) =>
           value: false,
           notify: true,
           reflectToAttribute: true,
-          observer: '_openedChanged',
         },
 
         /**
@@ -160,7 +159,11 @@ export const SelectBaseMixin = (superClass) =>
     }
 
     static get observers() {
-      return ['_updateAriaExpanded(opened, focusElement)', '_updateSelectedItem(value, _items, placeholder)'];
+      return [
+        '_updateAriaExpanded(opened, focusElement)',
+        '_updateSelectedItem(value, _items, placeholder)',
+        '_openedChanged(opened, _overlayElement)',
+      ];
     }
 
     constructor() {
@@ -183,9 +186,8 @@ export const SelectBaseMixin = (superClass) =>
     ready() {
       super.ready();
 
-      this._overlayElement = this.$.overlay;
-
       this._inputContainer = this.shadowRoot.querySelector('[part~="input-field"]');
+      this._overlayElement = this.$.overlay;
 
       this._valueButtonController = new ButtonController(this);
       this.addController(this._valueButtonController);
@@ -361,20 +363,22 @@ export const SelectBaseMixin = (superClass) =>
     }
 
     /** @private */
-    _openedChanged(opened, wasOpened) {
-      if (opened) {
-        // Avoid multiple announcements when a value gets selected from the dropdown
-        this._updateAriaLive(false);
+    _openedChanged(opened, overlayElement) {
+      if (!overlayElement) {
+        return;
+      }
 
-        if (!this._overlayElement || !this._menuElement || !this.focusElement || this.disabled || this.readonly) {
+      if (opened) {
+        if (this.disabled || this.readonly) {
+          // Disallow programmatic opening when disabled or readonly
           this.opened = false;
           return;
         }
 
-        this._overlayElement.style.setProperty(
-          '--vaadin-select-text-field-width',
-          `${this._inputContainer.offsetWidth}px`,
-        );
+        // Avoid multiple announcements when a value gets selected from the dropdown
+        this._updateAriaLive(false);
+
+        overlayElement.style.setProperty('--vaadin-select-text-field-width', `${this._inputContainer.offsetWidth}px`);
 
         // Preserve focus-ring to restore it later
         const hasFocusRing = this.hasAttribute('focus-ring');
@@ -384,7 +388,7 @@ export const SelectBaseMixin = (superClass) =>
         if (hasFocusRing) {
           this.removeAttribute('focus-ring');
         }
-      } else if (wasOpened) {
+      } else if (this.__oldOpened) {
         if (this._openedWithFocusRing) {
           this.setAttribute('focus-ring', '');
         }
@@ -396,6 +400,8 @@ export const SelectBaseMixin = (superClass) =>
           this._requestValidation();
         }
       }
+
+      this.__oldOpened = opened;
     }
 
     /** @private */

diff --git a/packages/select/test/lit-renderer-directives.common.js b/packages/select/test/lit-renderer-directives.common.js
@@ -24,7 +24,7 @@ describe('lit renderer directives', () => {
     describe('basic', () => {
       beforeEach(async () => {
         select = await renderOpenedSelect(container, { content: 'Content' });
-        overlay = select.shadowRoot.querySelector('vaadin-select-overlay');
+        overlay = select._overlayElement;
       });
 
       it('should set `renderer` property when the directive is attached', () => {

diff --git a/packages/select/test/select.common.js b/packages/select/test/select.common.js
@@ -521,6 +521,14 @@ describe('vaadin-select', () => {
         expect(select.opened).to.be.false;
         expect(select._overlayElement.opened).to.be.false;
       });
+
+      it('should disallow programmatic opening when disabled', async () => {
+        select.readonly = true;
+        await nextUpdate(select);
+        select.opened = true;
+        await nextUpdate(select);
+        expect(select._overlayElement.opened).to.be.false;
+      });
     });
 
     describe('readonly', () => {
@@ -533,6 +541,14 @@ describe('vaadin-select', () => {
         click(valueButton);
         expect(select._overlayElement.opened).to.be.false;
       });
+
+      it('should disallow programmatic opening when readonly', async () => {
+        select.readonly = true;
+        await nextUpdate(select);
+        select.opened = true;
+        await nextUpdate(select);
+        expect(select._overlayElement.opened).to.be.false;
+      });
     });
 
     describe('focus', () => {
@@ -780,4 +796,22 @@ describe('vaadin-select', () => {
       expect(parseFloat(window.getComputedStyle(select).width)).to.eql(500);
     });
   });
+
+  describe('pre-opened', () => {
+    beforeEach(() => {
+      select = document.createElement('vaadin-select');
+      select.items = [{ label: 'Option 1', value: 'value-1' }];
+    });
+
+    afterEach(() => {
+      select.remove();
+    });
+
+    it('should not close overlay when opened set before adding to DOM', async () => {
+      select.opened = true;
+      document.body.appendChild(select);
+      await nextUpdate(select);
+      expect(select._overlayElement.opened).to.be.true;
+    });
+  });
 });