fix: scroll focused item to be first visible item after pagedown (#7050…

…) (CP: 24.2) (#7096) Co-authored-by: Břetislav Wajtr <[email protected]>
vaadin · Jan 22, 2024 · aa55a8f · aa55a8f
1 parent f32dc4b
commit aa55a8f
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 6 deletions.
diff --git a/packages/grid/src/vaadin-grid-keyboard-navigation-mixin.js b/packages/grid/src/vaadin-grid-keyboard-navigation-mixin.js
@@ -105,6 +105,11 @@ export const KeyboardNavigationMixin = (superClass) =>
       });
     }
 
+    /** @private */
+    get _visibleItemsCount() {
+      return this._lastVisibleIndex - this._firstVisibleIndex - 1;
+    }
+
     /** @protected */
     ready() {
       super.ready();
@@ -300,8 +305,9 @@ export const KeyboardNavigationMixin = (superClass) =>
     _onNavigationKeyDown(e, key) {
       e.preventDefault();
 
-      const visibleItemsCount = this._lastVisibleIndex - this._firstVisibleIndex - 1;
       const isRTL = this.__isRTL;
+      const activeRow = e.composedPath().find((el) => this.__isRow(el));
+      const activeCell = e.composedPath().find((el) => this.__isCell(el));
 
       // Handle keyboard interaction as defined in:
       // https://w3c.github.io/aria-practices/#keyboard-interaction-24
@@ -346,18 +352,22 @@ export const KeyboardNavigationMixin = (superClass) =>
           dy = -1;
           break;
         case 'PageDown':
-          dy = visibleItemsCount;
+          // Check if the active group is body
+          if (this.$.items.contains(activeRow)) {
+            const currentRowIndex = this.__getIndexInGroup(activeRow, this._focusedItemIndex);
+            // Scroll the current row to the top...
+            this._scrollToFlatIndex(currentRowIndex);
+          }
+          // ...only then measure the visible items count
+          dy = this._visibleItemsCount;
           break;
         case 'PageUp':
-          dy = -visibleItemsCount;
+          dy = -this._visibleItemsCount;
           break;
         default:
           break;
       }
 
-      const activeRow = e.composedPath().find((el) => this.__isRow(el));
-      const activeCell = e.composedPath().find((el) => this.__isCell(el));
-
       if ((this.__rowFocusMode && !activeRow) || (!this.__rowFocusMode && !activeCell)) {
         // When using a screen reader, it's possible that neither a cell nor a row is focused.
         return;

diff --git a/packages/grid/test/keyboard-navigation.test.js b/packages/grid/test/keyboard-navigation.test.js
@@ -26,6 +26,7 @@ import {
   getCell,
   getCellContent,
   getContainerCell,
+  getFirstVisibleItem,
   getLastVisibleItem,
   getRowCells,
   getRows,
@@ -1236,6 +1237,17 @@ describe('keyboard navigation', () => {
         expect(getFocusedRowIndex()).to.equal(previousLastVisibleIndex - 1);
       });
 
+      it('should previous focused item be first visible item after third page down', () => {
+        focusItem(0);
+        pageDown();
+        pageDown();
+
+        const previousLastIndex = getFocusedRowIndex();
+        pageDown();
+
+        expect(getFirstVisibleItem(grid).index).to.equal(previousLastIndex);
+      });
+
       it('should scroll up one page with page up', async () => {
         focusItem(0);
         pageDown();
@@ -2124,6 +2136,27 @@ describe('keyboard navigation on column groups', () => {
     expect(getFocusedRowIndex()).to.equal(0);
   });
 
+  it('should not scroll body on header pagedown', () => {
+    grid.items = Array.from({ length: 1000 }, (_, i) => String(i));
+
+    // Focus a body cell
+    tabToBody();
+
+    // Scroll down
+    pageDown();
+    pageDown();
+
+    const firstVisibleIndex = getFirstVisibleItem(grid).index;
+    expect(firstVisibleIndex).to.be.above(5);
+
+    // Tab to header
+    tabToHeader();
+
+    pageDown();
+
+    expect(getFirstVisibleItem(grid).index).to.equal(firstVisibleIndex);
+  });
+
   describe('updating tabbable cells', () => {
     describe('header', () => {
       let header;
@@ -2308,6 +2341,11 @@ describe('hierarchical data', () => {
     callback(items, itemsOnEachLevel);
   }
 
+  function getItemForIndex(index) {
+    const { item } = grid._cache.items[index];
+    return item;
+  }
+
   beforeEach(() => {
     grid = fixtureSync(`
       <vaadin-grid>
@@ -2344,6 +2382,17 @@ describe('hierarchical data', () => {
     // Expect the focus to not have changed
     expect(grid.shadowRoot.activeElement.index).to.equal(itemsOnEachLevel - 1);
   });
+
+  it('should previous focused item be first visible item after second page down on expanded tree', () => {
+    grid.expandItem(getItemForIndex(0));
+    focusItem(0);
+
+    pageDown();
+    const previousLastIndex = getFocusedRowIndex();
+    pageDown();
+
+    expect(getFirstVisibleItem(grid).index).to.equal(previousLastIndex);
+  });
 });
 
 describe('lazy data provider', () => {