fix: scroll to correct focused row on keyboard navigation (#8003) (#8010)

Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>

Author: vaadin-bot

packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js

@@ -508,7 +508,7 @@ export const InlineEditingMixin = (superClass) =>
       if (!this.singleCellEdit && nextCell !== cell) {
         this._startEdit(nextCell, nextColumn);
       } else {
-        this._ensureScrolledToIndex(nextIndex);
+        this.__ensureFlatIndexInViewport(nextIndex);
         nextCell.focus();
       }
     }

packages/grid/src/vaadin-grid-keyboard-navigation-mixin.js

@@ -275,10 +275,10 @@ export const KeyboardNavigationMixin = (superClass) =>
     }
 
     /** @private */
-    _ensureScrolledToIndex(index) {
+    __ensureFlatIndexInViewport(index) {
       const targetRowInDom = [...this.$.items.children].find((child) => child.index === index);
       if (!targetRowInDom) {
-        this.scrollToIndex(index);
+        this._scrollToFlatIndex(index);
       } else {
         this.__scrollIntoViewport(index);
       }
@@ -499,7 +499,7 @@ export const KeyboardNavigationMixin = (superClass) =>
       }
 
       // Ensure correct vertical scroll position, destination row is visible
-      this._ensureScrolledToIndex(dstRowIndex);
+      this.__ensureFlatIndexInViewport(dstRowIndex);
 
       // When scrolling with repeated keydown, sometimes FocusEvent listeners
       // are too late to update _focusedItemIndex. Ensure next keydown
@@ -729,23 +729,14 @@ export const KeyboardNavigationMixin = (superClass) =>
         // The focus is about to exit the grid to the bottom.
         this.$.focusexit.focus();
       } else if (focusTarget === this._itemsFocusable) {
-        let itemsFocusTarget = focusTarget;
-        const targetRow = isRow(focusTarget) ? focusTarget : focusTarget.parentNode;
-        this._ensureScrolledToIndex(this._focusedItemIndex);
-        if (targetRow.index !== this._focusedItemIndex && isCell(focusTarget)) {
-          // The target row, which is about to be focused next, has been
-          // assigned with a new index since last focus, probably because of
-          // scrolling. Focus the row for the stored focused item index instead.
-          const columnIndex = Array.from(targetRow.children).indexOf(this._itemsFocusable);
-          const focusedItemRow = Array.from(this.$.items.children).find(
-            (row) => !row.hidden && row.index === this._focusedItemIndex,
-          );
-          if (focusedItemRow) {
-            itemsFocusTarget = focusedItemRow.children[columnIndex];
-          }
-        }
+        this.__ensureFlatIndexInViewport(this._focusedItemIndex);
+
+        // Ensure the correct element is set as focusable after scrolling.
+        // The virtualizer may use a different element to render the item.
+        this.__updateItemsFocusable();
+
         e.preventDefault();
-        itemsFocusTarget.focus();
+        this._itemsFocusable.focus();
       } else {
         e.preventDefault();
         focusTarget.focus();

packages/grid/test/keyboard-navigation-row-focus.common.js

@@ -99,19 +99,17 @@ function getTabbableRows(root) {
   return root.querySelectorAll('tr[tabindex]:not([hidden]):not([tabindex="-1"])');
 }
 
-function hierarchicalDataProvider({ parentItem }, callback) {
-  // Let's use a count lower than pageSize so we can ignore page + pageSize for now
-  const itemsOnEachLevel = 5;
-
-  const items = [...Array(itemsOnEachLevel)].map((_, i) => {
+function hierarchicalDataProvider({ parentItem, page, pageSize }, callback) {
+  const items = Array.from({ length: 100 }, (_, i) => {
     return {
       name: `${parentItem ? `${parentItem.name}-` : ''}${i}`,
       // Let's only have child items on every second item
       children: i % 2 === 0,
     };
   });
 
-  callback(items, itemsOnEachLevel);
+  const offset = page * pageSize;
+  callback(items.slice(offset, offset + pageSize), items.length);
 }
 
 describe('keyboard navigation - row focus', () => {
@@ -145,6 +143,39 @@ describe('keyboard navigation - row focus', () => {
     await nextRender(grid);
   });
 
+  describe('scrolling and navigating', () => {
+    it('should scroll focused nested row into view on arrow key', () => {
+      focusItem(0);
+      // Expand first row
+      right();
+      // Focus first nested row
+      down();
+      // Simulate real scrolling to get the virtualizer to render
+      // the focused item in a different element.
+      grid.$.table.scrollTop = grid.$.table.scrollHeight / 2;
+      flushGrid(grid);
+      down();
+      expect(getFocusedRowIndex(grid)).to.equal(2);
+    });
+
+    it('should scroll focused nested row into view on Tab', () => {
+      focusItem(0);
+      // Expand first row
+      right();
+      // Focus first nested row
+      down();
+      // Move focus to header
+      shiftTab();
+      // Simulate real scrolling to get the virtualizer to render
+      // the focused item in a different element.
+      grid.$.table.scrollTop = grid.$.table.scrollHeight / 2;
+      flushGrid(grid);
+      // Move focus back to items
+      tab();
+      expect(getFocusedRowIndex(grid)).to.equal(1);
+    });
+  });
+
   describe('navigating with tab', () => {
     it('should have single tabbable row in every section', () => {
       const tabbableElements = getTabbableElements(grid.shadowRoot);
@@ -339,7 +370,7 @@ describe('keyboard navigation - row focus', () => {
 
         end();
 
-        expect(getFocusedRowIndex(grid)).to.equal(4);
+        expect(getFocusedRowIndex(grid)).to.equal(99);
       });
     });
   });

packages/grid/test/keyboard-navigation.common.js

@@ -1305,43 +1305,73 @@ describe('keyboard navigation', () => {
         up();
         expect(grid.$.table.scrollTop).to.equal(scrollTop);
       });
+    });
 
-      describe('rotating focus indicator prevention', () => {
-        it('should hide navigation mode when a focused row goes off screen', () => {
-          focusItem(0);
-          right();
+    describe('scrolling and navigating', () => {
+      beforeEach(() => {
+        grid.items = undefined;
+        grid.size = 200;
+        grid.dataProvider = infiniteDataProvider;
+        flushGrid(grid);
+      });
 
-          expect(grid.hasAttribute('navigating')).to.be.true;
+      it('should hide navigation mode when a focused row goes off screen', () => {
+        focusItem(0);
+        right();
 
-          grid.scrollToIndex(100);
-          flushGrid(grid);
+        expect(grid.hasAttribute('navigating')).to.be.true;
 
-          expect(grid.hasAttribute('navigating')).to.be.false;
-        });
+        grid.scrollToIndex(100);
+        flushGrid(grid);
 
-        it('should reveal navigation mode when a focused row is back on screen', () => {
-          focusItem(0);
-          right();
-          grid.scrollToIndex(100);
-          flushGrid(grid);
+        expect(grid.hasAttribute('navigating')).to.be.false;
+      });
 
-          grid.scrollToIndex(0);
-          flushGrid(grid);
+      it('should reveal navigation mode when a focused row is back on screen', () => {
+        focusItem(0);
+        right();
+        grid.scrollToIndex(100);
+        flushGrid(grid);
 
-          expect(grid.hasAttribute('navigating')).to.be.true;
-        });
+        grid.scrollToIndex(0);
+        flushGrid(grid);
 
-        it('should not hide navigation mode if a header cell is focused', () => {
-          tabToHeader();
-          right();
+        expect(grid.hasAttribute('navigating')).to.be.true;
+      });
 
-          expect(grid.hasAttribute('navigating')).to.be.true;
+      it('should not hide navigation mode if a header cell is focused', () => {
+        tabToHeader();
+        right();
 
-          grid.scrollToIndex(100);
-          flushGrid(grid);
+        expect(grid.hasAttribute('navigating')).to.be.true;
 
-          expect(grid.hasAttribute('navigating')).to.be.true;
-        });
+        grid.scrollToIndex(100);
+        flushGrid(grid);
+
+        expect(grid.hasAttribute('navigating')).to.be.true;
+      });
+
+      it('should scroll focused row into view on arrow key', () => {
+        focusItem(0);
+        // Simulate real scrolling to get the virtualizer to render
+        // the focused item in a different element.
+        grid.$.table.scrollTop = grid.$.table.scrollHeight / 2;
+        flushGrid(grid);
+        down();
+        expect(getFocusedRowIndex(grid)).to.equal(1);
+        expect(getFocusedCellIndex(grid)).to.equal(0);
+      });
+
+      it('should scroll focused row into view on Tab', () => {
+        focusItem(0);
+        tabToHeader();
+        // Simulate real scrolling to get the virtualizer to render
+        // the focused item in a different element.
+        grid.$.table.scrollTop = grid.$.table.scrollHeight / 2;
+        flushGrid(grid);
+        tab();
+        expect(getFocusedRowIndex(grid)).to.equal(0);
+        expect(getFocusedCellIndex(grid)).to.equal(0);
       });
     });
   });