From cf9ef5367511eae1e49de395966e3dd3813483c8 Mon Sep 17 00:00:00 2001
From: Hadi Amiri <84700249+hadivaadin@users.noreply.github.com>
Date: Wed, 23 Jun 2021 15:55:25 +0300
Subject: [PATCH] fix: issue with grid when grid table body is clicked and
 enter key is hit (#2146)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: it should not throw error when grid table body has focus and enter key is hit

* extract comparison into a separate variable

* Refactor to merge _parseEventPath and _getGridCellFocusLocation into a general _getGridLocation

Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
---
 .../vaadin-grid-keyboard-navigation-mixin.js  | 48 +++++++------------
 .../test/keyboard-navigation.test.js          |  7 +++
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js b/packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js
index 67364c70fe..68ad901db0 100644
--- a/packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js
+++ b/packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js
@@ -342,16 +342,6 @@ export const KeyboardNavigationMixin = (superClass) =>
       dstCell.focus();
     }
 
-    /** @private */
-    _parseEventPath(path) {
-      const tableIndex = path.indexOf(this.$.table);
-      return {
-        rowGroup: path[tableIndex - 1],
-        row: path[tableIndex - 2],
-        cell: path[tableIndex - 3]
-      };
-    }
-
     /** @private */
     _onInteractionKeyDown(e, key) {
       const localTarget = e.composedPath()[0];
@@ -372,9 +362,9 @@ export const KeyboardNavigationMixin = (superClass) =>
           break;
       }
 
-      const { cell } = this._parseEventPath(e.composedPath());
+      const { cell } = this._getGridEventLocation(e);
 
-      if (this.interacting !== wantInteracting) {
+      if (this.interacting !== wantInteracting && cell !== null) {
         if (wantInteracting) {
           const focusTarget = cell._content.querySelector('[focus-target]') || cell._content.firstElementChild;
           if (focusTarget) {
@@ -514,11 +504,10 @@ export const KeyboardNavigationMixin = (superClass) =>
 
     /** @private */
     _onCellFocusIn(e) {
-      const location = this._getCellFocusEventLocation(e);
+      const { section, cell } = this._getGridEventLocation(e);
       this._detectInteracting(e);
 
-      if (location) {
-        const { section, cell } = location;
+      if (section && cell) {
         this._activeRowGroup = section;
         if (this.$.header === section) {
           this._headerFocusable = cell;
@@ -558,8 +547,8 @@ export const KeyboardNavigationMixin = (superClass) =>
 
     /** @private */
     _detectFocusedItemIndex(e) {
-      const { rowGroup, row } = this._parseEventPath(e.composedPath());
-      if (rowGroup === this.$.items) {
+      const { section, row } = this._getGridEventLocation(e);
+      if (section === this.$.items) {
         this._focusedItemIndex = row.index;
       }
     }
@@ -686,31 +675,30 @@ export const KeyboardNavigationMixin = (superClass) =>
     }
 
     /**
-     * @typedef {Object} CellFocusEventLocation
-     * @property {HTMLTableSectionElement} section - The grid section element that contains the focused cell (header, body, or footer)
-     * @property {HTMLElement} cell - The cell element that received focus or is ancestor of the element that received focus
+     * @typedef {Object} GridEventLocation
+     * @property {HTMLTableSectionElement | null} section - The table section element that the event occurred in (header, body, or footer), or null if the event did not occur in a section
+     * @property {HTMLTableRowElement | null} row - The row element that the event occurred in, or null if the event did not occur in a row
+     * @property {HTMLTableCellElement | null} cell - The cell element that the event occurred in, or null if the event did not occur in a cell
      * @private
      */
     /**
-     * Takes a focus event and returns a location object describing in which
-     * section of the grid and in or on which cell the focus event occurred.
-     * The focus event may either target the cell itself or contents of the cell.
-     * If the event does not target a cell then null is returned.
-     * @param {FocusEvent} e
-     * @returns {CellFocusEventLocation | null}
+     * Takes an event and returns a location object describing in which part of the grid the event occurred.
+     * The event may either target table section, a row, a cell or contents of a cell.
+     * @param {Event} e
+     * @returns {GridEventLocation}
      * @private
      */
-    _getCellFocusEventLocation(e) {
+    _getGridEventLocation(e) {
       const path = e.composedPath();
       const tableIndex = path.indexOf(this.$.table);
       // Assuming ascending path to table is: [...,] th|td, tr, thead|tbody, table [,...]
-      const section = tableIndex >= 2 ? path[tableIndex - 1] : null;
+      const section = tableIndex >= 1 ? path[tableIndex - 1] : null;
+      const row = tableIndex >= 2 ? path[tableIndex - 2] : null;
       const cell = tableIndex >= 3 ? path[tableIndex - 3] : null;
 
-      if (!section || !cell) return null;
-
       return {
         section,
+        row,
         cell
       };
     }
diff --git a/packages/vaadin-grid/test/keyboard-navigation.test.js b/packages/vaadin-grid/test/keyboard-navigation.test.js
index c25bc98990..6bdff1b872 100644
--- a/packages/vaadin-grid/test/keyboard-navigation.test.js
+++ b/packages/vaadin-grid/test/keyboard-navigation.test.js
@@ -1882,6 +1882,13 @@ describe('keyboard navigation', () => {
 
       expect(grid.hasAttribute('interacting')).to.be.true;
     });
+
+    it('should not throw error when hit enter after focus on table body', () => {
+      expect(() => {
+        grid.$.items.focus();
+        enter(grid);
+      }).not.to.throw(Error);
+    });
   });
 
   describe('focus events on cell content', () => {