fix!: don't focus selected item on combo-box open (#6055) (#6063)

Co-authored-by: Diego Cardoso <[email protected]>

packages/combo-box/src/vaadin-combo-box-mixin.js

@@ -869,14 +869,6 @@ export const ComboBoxMixin = (subclass) =>
 
     /** @private */
     _onOpened() {
-      // Defer scroll position adjustment to improve performance.
-      requestAnimationFrame(() => {
-        this._scrollIntoView(this._focusedIndex);
-
-        // Set attribute after the items are rendered when overlay is opened for the first time.
-        this._updateActiveDescendant(this._focusedIndex);
-      });
-
       // _detectAndDispatchChange() should not consider value changes done before opening
       this._lastCommittedValue = this.value;
     }
@@ -897,6 +889,7 @@ export const ComboBoxMixin = (subclass) =>
         }
         // Make sure input field is updated in case value doesn't change (i.e. FOO -> foo)
         this._inputElementValue = this._getItemLabel(this.selectedItem);
+        this._focusedIndex = -1;
       } else if (this._inputElementValue === '' || this._inputElementValue === undefined) {
         this.selectedItem = null;
 
@@ -1056,10 +1049,6 @@ export const ComboBoxMixin = (subclass) =>
         this._toggleHasValue(true);
         this._inputElementValue = this._getItemLabel(selectedItem);
       }
-
-      if (this.filteredItems) {
-        this._focusedIndex = this.filteredItems.indexOf(selectedItem);
-      }
     }
 
     /**
@@ -1138,18 +1127,6 @@ export const ComboBoxMixin = (subclass) =>
       const focusedItemIndex = this.__getItemIndexByValue(filteredItems, this._getItemValue(focusedItem));
       if (focusedItemIndex > -1) {
         this._focusedIndex = focusedItemIndex;
-      } else {
-        this.__setInitialFocusedIndex();
-      }
-    }
-
-    /** @private */
-    __setInitialFocusedIndex() {
-      const inputValue = this._inputElementValue;
-      if (inputValue === undefined || inputValue === this._getItemLabel(this.selectedItem)) {
-        // When the input element value is the same as the current value or not defined,
-        // set the focused index to the item that matches the value.
-        this._focusedIndex = this.__getItemIndexByLabel(this.filteredItems, this._getItemLabel(this.selectedItem));
       } else {
         // When the user filled in something that is different from the current value = filtering is enabled,
         // set the focused index to the item that matches the filter query.

packages/combo-box/test/basic.test.js

@@ -26,14 +26,6 @@ describe('Properties', () => {
       expect(getViewportItems(comboBox).length).to.eql(1);
     });
 
-    it('should set focused index on items set', () => {
-      comboBox.value = 'bar';
-
-      comboBox.items = ['foo', 'bar'];
-
-      expect(comboBox._focusedIndex).to.eql(1);
-    });
-
     it('should support resetting items', () => {
       comboBox.items = ['foo', 'bar'];
       comboBox.items = undefined;

packages/combo-box/test/external-filtering.test.js

@@ -110,7 +110,7 @@ describe('external filtering', () => {
 
     it('should not throw when passing filteredItems and value as attributes', () => {
       comboBox.open();
-      expect(getFocusedItemIndex(comboBox)).to.equal(1);
+      expect(getFocusedItemIndex(comboBox)).to.equal(-1);
     });
   });
 
@@ -121,9 +121,9 @@ describe('external filtering', () => {
       comboBox.value = 'foo';
     });
 
-    it('should have the value item focused when opened', () => {
+    it('should not have the value item focused when opened', () => {
       comboBox.open();
-      expect(getFocusedItemIndex(comboBox)).to.equal(0);
+      expect(getFocusedItemIndex(comboBox)).to.equal(-1);
     });
 
     it('should have the filtered item focused when opened on changing the filter', () => {
@@ -165,9 +165,9 @@ describe('external filtering', () => {
       expect(comboBox.inputElement.value).to.equal('foo');
     });
 
-    it('should have the value item focused when opened', () => {
+    it('should not have the value item focused when opened', () => {
       comboBox.open();
-      expect(getFocusedItemIndex(comboBox)).to.equal(0);
+      expect(getFocusedItemIndex(comboBox)).to.equal(-1);
     });
 
     // See https://github.com/vaadin/web-components/issues/2615
@@ -193,9 +193,9 @@ describe('external filtering', () => {
       expect(comboBox.inputElement.value).to.equal('bar');
     });
 
-    it('should have the value item focused when opened', () => {
+    it('should not have the value item focused when opened', () => {
       comboBox.open();
-      expect(getFocusedItemIndex(comboBox)).to.equal(1);
+      expect(getFocusedItemIndex(comboBox)).to.equal(-1);
     });
 
     it('should have the filtered item focused when opened after changing the filter', () => {
@@ -251,9 +251,9 @@ describe('external filtering', () => {
       expect(comboBox.inputElement.value).to.equal('Item 0');
     });
 
-    it('should have the correct item focused when opened', () => {
+    it('should not have the correct item focused when opened', () => {
       comboBox.open();
-      expect(getFocusedItemIndex(comboBox)).to.equal(0);
+      expect(getFocusedItemIndex(comboBox)).to.equal(-1);
     });
   });
 });

packages/combo-box/test/keyboard.test.js

@@ -42,12 +42,12 @@ describe('keyboard', () => {
       expect(getFocusedIndex()).to.equal(-1);
     });
 
-    it('should have focus on the selected item after opened', () => {
+    it('should not focus on the selected item after opened', () => {
       comboBox.value = 'foo';
 
       arrowDownKeyDown(input);
 
-      expect(getFocusedIndex()).to.equal(0);
+      expect(getFocusedIndex()).to.equal(-1);
     });
 
     it('should propagate escape key event if dropdown is closed', () => {
@@ -185,7 +185,8 @@ describe('keyboard', () => {
         await aTimeout(1);
         enterKeyDown(input);
         await aTimeout(1);
-        expect(comboBox.value).to.equal('baz');
+        // keyboard navigation starts from the top
+        expect(comboBox.value).to.equal('foo');
       });
 
       it('should clear the selection with enter when input is cleared', () => {
@@ -331,7 +332,7 @@ describe('keyboard', () => {
 
       it('should prefill the input field when navigating down', () => {
         arrowDownKeyDown(input);
-        expect(input.value).to.eql('baz');
+        expect(input.value).to.eql('foo');
       });
 
       it('should select the input field text when navigating down', () => {
@@ -342,7 +343,7 @@ describe('keyboard', () => {
 
       it('should prefill the input field when navigating up', () => {
         arrowUpKeyDown(input);
-        expect(input.value).to.eql('foo');
+        expect(input.value).to.eql('baz');
       });
 
       it('should not prefill the input when there are no items to navigate', () => {
@@ -549,17 +550,6 @@ describe('keyboard', () => {
 
       expect(getViewportItems(comboBox)[0].index).to.eql(0);
     });
-
-    it('should scroll to focused item when opening overlay', async () => {
-      scrollToIndex(comboBox, 0);
-      comboBox.close();
-      comboBox.value = '50';
-
-      comboBox.open();
-
-      await onceScrolled(comboBox);
-      expect(getViewportItems(comboBox)[0].index).to.be.within(50 - getVisibleItemsCount(comboBox), 50);
-    });
   });
 
   describe('auto open disabled', () => {

packages/combo-box/test/lazy-loading.test.js

@@ -520,7 +520,7 @@ describe('lazy loading', () => {
           // Wait for the __loadingChanged observer
           await aTimeout(0);
 
-          expect(comboBox._focusedIndex).to.equal(8);
+          expect(comboBox._focusedIndex).to.equal(-1);
           const items = getViewportItems(comboBox);
           expect(items.some((item) => item.index === 50)).to.be.true;
         });

packages/combo-box/test/scrolling.test.js

@@ -86,26 +86,6 @@ describe('scrolling', () => {
       expect(selectedItemRect.bottom).to.be.at.most(overlayRect.bottom + 1);
     }
 
-    it('should make selected item visible after open', async () => {
-      comboBox.value = comboBox.items[50];
-      comboBox.open();
-      await onceScrolled(comboBox);
-      expectSelectedItemPositionToBeVisible();
-    });
-
-    it('should make selected item visible after reopen', async () => {
-      comboBox.open();
-      await nextFrame();
-
-      comboBox.value = comboBox.items[50];
-      comboBox.close();
-
-      comboBox.open();
-      await nextFrame();
-
-      expectSelectedItemPositionToBeVisible();
-    });
-
     it('should not close the items when touching scroll bar', () => {
       comboBox.open();
       focusout(input, overlay);

packages/time-picker/test/combo-box.test.js

@@ -116,13 +116,6 @@ describe('autoOpenDisabled', () => {
     inputElement = timePicker.inputElement;
   });
 
-  it('should focus the correct item when opened', () => {
-    timePicker.open();
-
-    const items = document.querySelectorAll('vaadin-time-picker-item');
-    expect(items[5].hasAttribute('focused')).to.be.true;
-  });
-
   it('should commit a custom value after setting a predefined value', () => {
     setInputValue(timePicker, '05:10');
     enter(inputElement);

packages/time-picker/test/events.test.js

@@ -63,6 +63,7 @@ describe('events', () => {
       timePicker.open();
       inputElement.value = '';
       arrowDown(inputElement);
+      arrowDown(inputElement);
       enter(inputElement);
       expect(changeSpy.calledOnce).to.be.true;
       // Mimic native change happening on text-field blur