Skip to content

Commit

Permalink
fix: issue with grid when grid table body is clicked and enter key is…
Browse files Browse the repository at this point in the history
… hit (#2146)

* 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>
  • Loading branch information
hdamr and sissbruecker authored Jun 23, 2021
1 parent 95484c1 commit cf9ef53
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 30 deletions.
48 changes: 18 additions & 30 deletions packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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
};
}
Expand Down
7 changes: 7 additions & 0 deletions packages/vaadin-grid/test/keyboard-navigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit cf9ef53

Please # to comment.