refactor: remove selected attribute from value item clone

vaadin · Dec 5, 2024 · 2d33561 · 2d33561
1 parent ce58212
commit 2d33561
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 20 deletions.
diff --git a/packages/select/src/vaadin-select-base-mixin.js b/packages/select/src/vaadin-select-base-mixin.js
@@ -427,10 +427,10 @@ export const SelectBaseMixin = (superClass) =>
       // Store reference to the original item
       labelItem._sourceItem = selected;
 
-      this.__appendValueItemElement(labelItem, this.focusElement);
+      // Cleanup selected and aria-selected
+      labelItem.selected = false;
 
-      // Ensure the item gets proper styles
-      labelItem.selected = true;
+      this.__appendValueItemElement(labelItem, this.focusElement);
     }
 
     /**
@@ -461,13 +461,8 @@ export const SelectBaseMixin = (superClass) =>
      */
     __appendValueItemElement(itemElement, parent) {
       parent.appendChild(itemElement);
-      // Trigger observer that sets aria-selected attribute
-      // so that we can then synchronously remove it below.
-      if (itemElement.performUpdate) {
-        itemElement.performUpdate();
-      }
       itemElement.removeAttribute('tabindex');
-      itemElement.removeAttribute('aria-selected');
+
       itemElement.removeAttribute('role');
       itemElement.removeAttribute('focused');
       itemElement.removeAttribute('focus-ring');

diff --git a/packages/select/test/keyboard.common.js b/packages/select/test/keyboard.common.js
@@ -151,7 +151,7 @@ describe('keyboard', () => {
       expect(clone.textContent).to.be.equal(item.textContent);
     });
 
-    ['active', 'focused', 'focus-ring', 'role', 'tabindex', 'aria-selected'].forEach((attr) => {
+    ['active', 'focused', 'focus-ring', 'role', 'tabindex', 'selected'].forEach((attr) => {
       it(`should remove ${attr} attribute from the item clone`, async () => {
         await sendKeys({ press: 'Tab' });
 

diff --git a/packages/select/test/select.common.js b/packages/select/test/select.common.js
@@ -146,14 +146,6 @@ describe('vaadin-select', () => {
         expect(select._overlayElement.opened).to.be.false;
       });
 
-      it('should preserve the selected attribute when selecting the disabled item', async () => {
-        menu.selected = 5;
-        await nextUpdate(select);
-        const valueElement = valueButton.firstChild;
-        expect(valueElement.selected).to.be.true;
-        expect(valueElement.disabled).to.be.true;
-      });
-
       it('should not update value if the selected item does not have a value', async () => {
         menu.selected = 2;
         await nextUpdate(select);
@@ -180,13 +172,13 @@ describe('vaadin-select', () => {
         expect(valueButton.textContent.trim()).to.be.equal('o2');
       });
 
-      it('should wrap the selected item `label` string in selected vaadin item', async () => {
+      it('should wrap the selected item `label` string in vaadin-select-item', async () => {
         menu.selected = 1;
         await nextUpdate(select);
         const item = valueButton.firstElementChild;
         expect(item.localName).to.equal('vaadin-select-item');
         expect(item.textContent).to.equal('o2');
-        expect(item.selected).to.be.true;
+        expect(item.selected).to.be.false;
         expect(item.getAttribute('tabindex')).to.be.null;
       });