feat: center focused item in combo-box dropdown

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

@@ -696,11 +696,11 @@ export const ComboBoxBaseMixin = (superClass) =>
     }
 
     /** @protected */
-    _scrollIntoView(index) {
+    _scrollIntoView(index, alignToCenter = false) {
       if (!this._scroller) {
         return;
       }
-      this._scroller.scrollIntoView(index);
+      this._scroller.scrollIntoView(index, alignToCenter);
     }
 
     /** @private */

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

@@ -36,11 +36,14 @@ export const ComboBoxFocusIndexMixin = (superClass) =>
         return;
       }
 
-      // Move the viewport and set the focused row. Rendering rows
-      // around `index` lets placeholder rows fire `index-requested`,
-      // which loads any missing pages via the data-provider chain.
-      this._scrollIntoView(index);
+      // Set the focused row first: this synchronously runs the scroller's
+      // `__focusedIndexChanged` observer, which scrolls the row into view
+      // (bottom-aligned). Then scroll again to center it so the final position
+      // wins. Rendering rows around `index` also lets placeholder rows fire
+      // `index-requested`, loading any missing pages via the data-provider
+      // chain.
       this._focusedIndex = index;
+      this._scrollIntoView(index, true);
 
       // A page-load may have kicked in during the scroll (placeholder
       // rows in the new viewport requested their pages). Re-fire after

packages/combo-box/src/vaadin-combo-box-scroller-mixin.d.ts

@@ -70,10 +70,13 @@ export declare class ComboBoxScrollerMixinClass<TItem, TOwner> {
   requestContentUpdate(): void;
 
   /**
-   * Scrolls an item at given index into view and adjusts `scrollTop`
-   * so that the element gets fully visible on Arrow Down key press.
+   * Scrolls an item at given index into view. By default the item is made
+   * fully visible at the bottom of the viewport (the behavior wanted on
+   * Arrow Down key press). When `alignToCenter` is `true`, the item is placed
+   * in the middle of the viewport instead, as close to center as the list
+   * allows near its start and end.
    */
-  scrollIntoView(index: number): void;
+  scrollIntoView(index: number, alignToCenter?: boolean): void;
 
   protected _isItemSelected(item: TItem, selectedItem: TItem, itemIdPath: string | null | undefined): void;
 

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

@@ -166,29 +166,53 @@ export const ComboBoxScrollerMixin = (superClass) =>
     }
 
     /**
-     * Scrolls an item at given index into view and adjusts `scrollTop`
-     * so that the element gets fully visible on Arrow Down key press.
+     * Scrolls an item at given index into view. By default the item is made
+     * fully visible at the bottom of the viewport (the behavior wanted on
+     * Arrow Down key press). When `alignToCenter` is `true`, the item is placed
+     * in the middle of the viewport instead, as close to center as the list
+     * allows near its start and end.
      * @param {number} index
+     * @param {boolean} [alignToCenter=false]
      */
-    scrollIntoView(index) {
+    scrollIntoView(index, alignToCenter = false) {
       if (!this.__virtualizer || !(this.opened && index >= 0)) {
         return;
       }
 
-      const visibleItemsCount = this._visibleItemsCount();
+      // If the target row is already fully visible, leave the scroll untouched.
+      // Re-running scrollToIndex would snap an unaligned scrollTop to a row
+      // boundary — a visible jump when only the focus ring should move (arrow
+      // navigation, clicks). Centering always repositions, so it skips this.
+      const renderedTarget = [...this.children].find((el) => !el.hidden && el.index === index);
+      if (!alignToCenter && renderedTarget) {
+        const targetRect = renderedTarget.getBoundingClientRect();
+        const scrollerRect = this.getBoundingClientRect();
+        if (
+          targetRect.top >= scrollerRect.top &&
+          targetRect.bottom + this._viewportTotalPaddingBottom <= scrollerRect.bottom
+        ) {
+          return;
+        }
+      }
 
+      // When centering, scrolling straight to the target index renders it and
+      // puts it at the top of the scroller; the rect-based correction below
+      // then moves it to the middle. No index recalculation is needed.
       let targetIndex = index;
 
-      if (index > this.__virtualizer.lastVisibleIndex - 1) {
-        // Index is below the bottom, scrolling down. Make the item appear at the bottom.
-        // First scroll to target (will be at the top of the scroller) to make sure it's rendered.
-        this.__virtualizer.scrollToIndex(index);
-        // Then calculate the index for the following scroll (to get the target to bottom of the scroller).
-        targetIndex = index - visibleItemsCount + 1;
-      } else if (index > this.__virtualizer.firstVisibleIndex) {
-        // The item is already visible, scrolling is unnecessary per se. But we need to trigger iron-list to set
-        // the correct scrollTop on the scrollTarget. Scrolling to firstVisibleIndex.
-        targetIndex = this.__virtualizer.firstVisibleIndex;
+      if (!alignToCenter) {
+        const visibleItemsCount = this._visibleItemsCount();
+        if (index > this.__virtualizer.lastVisibleIndex - 1) {
+          // Index is below the bottom, scrolling down. Make the item appear at the bottom.
+          // First scroll to target (will be at the top of the scroller) to make sure it's rendered.
+          this.__virtualizer.scrollToIndex(index);
+          // Then calculate the index for the following scroll (to get the target to bottom of the scroller).
+          targetIndex = index - visibleItemsCount + 1;
+        } else if (index > this.__virtualizer.firstVisibleIndex) {
+          // The item is already visible, scrolling is unnecessary per se. But we need to trigger iron-list to set
+          // the correct scrollTop on the scrollTarget. Scrolling to firstVisibleIndex.
+          targetIndex = this.__virtualizer.firstVisibleIndex;
+        }
       }
       this.__virtualizer.scrollToIndex(Math.max(0, targetIndex));
 
@@ -201,6 +225,15 @@ export const ComboBoxScrollerMixin = (superClass) =>
       }
       const targetRect = target.getBoundingClientRect();
       const scrollerRect = this.getBoundingClientRect();
+      if (alignToCenter) {
+        // Center the target in the viewport. Clamp to 0 at the top; the browser
+        // clamps to the maximum scroll position at the bottom, so near the start
+        // and end the item sits as close to center as the list allows.
+        const targetCenter = targetRect.top + targetRect.height / 2;
+        const scrollerCenter = scrollerRect.top + scrollerRect.height / 2;
+        this.scrollTop = Math.max(0, this.scrollTop + (targetCenter - scrollerCenter));
+        return;
+      }
       const targetBottom = targetRect.bottom + this._viewportTotalPaddingBottom;
       if (targetBottom > scrollerRect.bottom) {
         this.scrollTop += targetBottom - scrollerRect.bottom;

packages/combo-box/test/focus-index.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { arrowUpKeyDown, fixtureSync, nextFrame, nextRender } from '@vaadin/testing-helpers';
+import { arrowDownKeyDown, arrowUpKeyDown, fixtureSync, nextFrame, nextRender } from '@vaadin/testing-helpers';
 import '../src/vaadin-combo-box.js';
 import { ComboBoxPlaceholder } from '../src/vaadin-combo-box-placeholder.js';
 import { flushComboBox, getViewportItems, makeItems } from './helpers.js';
@@ -34,6 +34,39 @@ describe('__focusIndex', () => {
       expect(comboBox._focusedIndex).to.equal(100);
     });
 
+    it('should place the target in the middle of the viewport when opened', async () => {
+      comboBox.opened = true;
+      comboBox.__focusIndex(100);
+      flushComboBox(comboBox);
+      await nextFrame();
+
+      const findItem = (index) => [...comboBox._scroller.children].find((el) => !el.hidden && el.index === index);
+      const scrollerRect = comboBox._scroller.getBoundingClientRect();
+      const targetRect = findItem(100).getBoundingClientRect();
+      // The target is fully visible and centered in the viewport.
+      expect(targetRect.top).to.be.at.least(scrollerRect.top - 1);
+      expect(targetRect.bottom).to.be.at.most(scrollerRect.bottom + 1);
+      const targetCenter = targetRect.top + targetRect.height / 2;
+      const scrollerCenter = scrollerRect.top + scrollerRect.height / 2;
+      expect(targetCenter).to.be.closeTo(scrollerCenter, targetRect.height / 2 + 1);
+    });
+
+    it('should not shift the viewport when arrow-navigating among visible items', async () => {
+      comboBox.opened = true;
+      comboBox.__focusIndex(100);
+      flushComboBox(comboBox);
+      await nextFrame();
+
+      // Centering leaves the scroll position unaligned. Arrow navigation among
+      // already-visible rows must move only the focus ring, not the viewport.
+      const scrollTopBefore = comboBox._scroller.scrollTop;
+      arrowDownKeyDown(comboBox.inputElement);
+      await nextFrame();
+
+      expect(comboBox._focusedIndex).to.equal(101);
+      expect(comboBox._scroller.scrollTop).to.be.closeTo(scrollTopBefore, 1);
+    });
+
     it('should queue the scroll when called before opening', async () => {
       comboBox.__focusIndex(100);
       expect(comboBox.__pendingFocusIndex).to.equal(100);
@@ -121,6 +154,17 @@ describe('__focusIndex', () => {
       return [...comboBox._scroller.children].find((el) => !el.hidden && el.index === index);
     }
 
+    // Asserts the target is fully visible and centered in the viewport.
+    function expectCenteredInViewport(index) {
+      const scrollerRect = getScrollerRect();
+      const targetRect = findRenderedItem(index).getBoundingClientRect();
+      expect(targetRect.top).to.be.at.least(scrollerRect.top - 1);
+      expect(targetRect.bottom).to.be.at.most(scrollerRect.bottom + 1);
+      const targetCenter = targetRect.top + targetRect.height / 2;
+      const scrollerCenter = scrollerRect.top + scrollerRect.height / 2;
+      expect(targetCenter).to.be.closeTo(scrollerCenter, 2.5);
+    }
+
     beforeEach(async () => {
       comboBox = fixtureSync(`
         <vaadin-combo-box
@@ -131,7 +175,7 @@ describe('__focusIndex', () => {
       comboBox.items = items;
     });
 
-    it('should fully reveal target below the viewport with variable heights', async () => {
+    it('should center a target below the viewport with variable heights', async () => {
       comboBox.opened = true;
       flushComboBox(comboBox);
 
@@ -140,35 +184,27 @@ describe('__focusIndex', () => {
       flushComboBox(comboBox);
       await nextFrame();
 
-      const target = findRenderedItem(targetIndex);
-      const targetRect = target.getBoundingClientRect();
-      const scrollerRect = getScrollerRect();
-      expect(Math.round(targetRect.bottom)).to.be.at.most(Math.round(scrollerRect.bottom) + 1);
-      expect(Math.round(targetRect.top)).to.be.at.least(Math.round(scrollerRect.top) - 1);
+      expectCenteredInViewport(targetIndex);
     });
 
-    it('should fully reveal target above the viewport with variable heights', async () => {
+    it('should center a target above the viewport with variable heights', async () => {
       comboBox.opened = true;
       flushComboBox(comboBox);
 
-      // Pre-scroll well past the target, then jump back to a low index.
+      // Pre-scroll well past the target, then jump back to a centerable index.
       comboBox.__focusIndex(150);
       flushComboBox(comboBox);
       await nextFrame();
 
-      const targetIndex = 5;
+      const targetIndex = 30;
       comboBox.__focusIndex(targetIndex);
       flushComboBox(comboBox);
       await nextFrame();
 
-      const target = findRenderedItem(targetIndex);
-      const targetRect = target.getBoundingClientRect();
-      const scrollerRect = getScrollerRect();
-      expect(Math.round(targetRect.top)).to.be.at.least(Math.round(scrollerRect.top) - 1);
-      expect(Math.round(targetRect.bottom)).to.be.at.most(Math.round(scrollerRect.bottom) + 1);
+      expectCenteredInViewport(targetIndex);
     });
 
-    it('should not shift the viewport when target is already visible', async () => {
+    it('should re-center an already-visible target with variable heights', async () => {
       comboBox.opened = true;
       flushComboBox(comboBox);
       await nextFrame();
@@ -177,13 +213,30 @@ describe('__focusIndex', () => {
       const lastVisible = comboBox._scroller.__virtualizer.lastVisibleIndex;
       const targetIndex = firstVisible + Math.floor((lastVisible - firstVisible) / 2);
 
-      const scrollTopBefore = comboBox._scroller.scrollTop;
       comboBox.__focusIndex(targetIndex);
       flushComboBox(comboBox);
       await nextFrame();
 
-      // Scrolling to an already-visible item should not shift the viewport.
-      expect(Math.abs(comboBox._scroller.scrollTop - scrollTopBefore)).to.be.below(2);
+      expectCenteredInViewport(targetIndex);
+    });
+
+    it('should clamp to the top for a near-start target that cannot be centered', async () => {
+      comboBox.opened = true;
+      flushComboBox(comboBox);
+      await nextFrame();
+
+      const targetIndex = 2;
+      comboBox.__focusIndex(targetIndex);
+      flushComboBox(comboBox);
+      await nextFrame();
+
+      // Not enough rows above to center; the list stays scrolled to the top
+      // and the target is fully visible.
+      expect(comboBox._scroller.scrollTop).to.be.closeTo(0, 1);
+      const scrollerRect = getScrollerRect();
+      const targetRect = findRenderedItem(targetIndex).getBoundingClientRect();
+      expect(targetRect.top).to.be.at.least(scrollerRect.top - 1);
+      expect(targetRect.bottom).to.be.at.most(scrollerRect.bottom + 1);
     });
   });