diff --git a/package.json b/package.json index 5b1ed7b897..c185857f75 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "@vaadin/testing-helpers": "^0.2.1", "@web/dev-server": "^0.1.17", "@web/test-runner": "^0.13.4", + "@web/test-runner-commands": "^0.4.5", "@web/test-runner-playwright": "^0.8.4", "@web/test-runner-saucelabs": "^0.5.0", "@web/test-runner-visual-regression": "^0.6.0", 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 81130ff60a..b0a373f377 100644 --- a/packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js +++ b/packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js @@ -45,7 +45,27 @@ export const KeyboardNavigationMixin = (superClass) => }, /** @private */ - _focusedColumnOrder: Number + _focusedColumnOrder: Number, + + /** + * Indicates whether the grid is currently in interaction mode. + * In interaction mode the user is currently interacting with a control, + * such as an input or a select, within a cell. + * In interaction mode keyboard navigation between cells is disabled. + * Interaction mode also prevents the focus target cell of that section of + * the grid from receiving focus, allowing the user to switch focus to + * controls in adjacent cells, rather than focussing the outer cell + * itself. + * @type {boolean} + * @private + */ + interacting: { + type: Boolean, + value: false, + reflectToAttribute: true, + readOnly: true, + observer: '_interactingChanged' + } }; } @@ -82,10 +102,18 @@ export const KeyboardNavigationMixin = (superClass) => oldFocusable.setAttribute('tabindex', '-1'); } if (focusable) { - focusable.setAttribute('tabindex', '0'); + this._updateGridSectionFocusTarget(focusable); } } + /** @private */ + _interactingChanged() { + // Update focus targets when entering / exiting interaction mode + this._updateGridSectionFocusTarget(this._headerFocusable); + this._updateGridSectionFocusTarget(this._itemsFocusable); + this._updateGridSectionFocusTarget(this._footerFocusable); + } + /** * @param {!KeyboardEvent} e * @protected @@ -119,7 +147,7 @@ export const KeyboardNavigationMixin = (superClass) => } this._detectInteracting(e); - if (this.hasAttribute('interacting') && keyGroup !== 'Interaction') { + if (this.interacting && keyGroup !== 'Interaction') { // When in the interacting mode, only the “Interaction” keys are handled. keyGroup = undefined; } @@ -338,32 +366,32 @@ export const KeyboardNavigationMixin = (superClass) => let wantInteracting; switch (key) { case 'Enter': - wantInteracting = this.hasAttribute('interacting') ? !localTargetIsTextInput : true; + wantInteracting = this.interacting ? !localTargetIsTextInput : true; break; case 'Escape': wantInteracting = false; break; case 'F2': - wantInteracting = !this.hasAttribute('interacting'); + wantInteracting = !this.interacting; break; } const { cell } = this._parseEventPath(e.composedPath()); - if (this.hasAttribute('interacting') !== wantInteracting) { + if (this.interacting !== wantInteracting) { if (wantInteracting) { const focusTarget = cell._content.querySelector('[focus-target]') || cell._content.firstElementChild; if (focusTarget) { e.preventDefault(); focusTarget.focus(); - this._toggleAttribute('interacting', true, this); + this._setInteracting(true); this._toggleAttribute('navigating', false, this); } } else { e.preventDefault(); this._focusedColumnOrder = undefined; cell.focus(); - this._toggleAttribute('interacting', false, this); + this._setInteracting(false); this._toggleAttribute('navigating', true, this); } } @@ -473,7 +501,7 @@ export const KeyboardNavigationMixin = (superClass) => // tabbed (shift-tabbed) into the grid. Move the focus to // the first (the last) focusable. this._predictFocusStepTarget(rootTarget, rootTarget === this.$.table ? 1 : -1).focus(); - this._toggleAttribute('interacting', false, this); + this._setInteracting(false); } else { this._detectInteracting(e); } @@ -490,16 +518,17 @@ export const KeyboardNavigationMixin = (superClass) => /** @private */ _onCellFocusIn(e) { + const location = this._getCellFocusEventLocation(e); this._detectInteracting(e); - if (e.composedPath().indexOf(this.$.table) === 3) { - const cell = e.composedPath()[0]; - this._activeRowGroup = cell.parentNode.parentNode; - if (this._activeRowGroup === this.$.header) { + if (location) { + const { section, cell } = location; + this._activeRowGroup = section; + if (this.$.header === section) { this._headerFocusable = cell; - } else if (this._activeRowGroup === this.$.items) { + } else if (this.$.items === section) { this._itemsFocusable = cell; - } else if (this._activeRowGroup === this.$.footer) { + } else if (this.$.footer === section) { this._footerFocusable = cell; } // Inform cell content of the focus (used in ) @@ -521,13 +550,14 @@ export const KeyboardNavigationMixin = (superClass) => } } - /** @private */ + /** @private + * Enables interaction mode if a cells descendant receives focus or keyboard + * input. Disables it if the event is not related to cell content. + * @param {!KeyboardEvent|!FocusEvent} e + */ _detectInteracting(e) { - this._toggleAttribute( - 'interacting', - e.composedPath().some((el) => el.localName === 'vaadin-grid-cell-content'), - this - ); + const isInteracting = e.composedPath().some((el) => el.localName === 'vaadin-grid-cell-content'); + this._setInteracting(isInteracting); } /** @private */ @@ -538,6 +568,21 @@ export const KeyboardNavigationMixin = (superClass) => } } + /** @private + * Enables or disables the focus target cell of the containing section of the + * grid from receiving focus, based on whether the user is interacting with + * that section of the grid. + * @param {HTMLTableCellElement} focusTargetCell + */ + _updateGridSectionFocusTarget(focusTargetCell) { + if (!focusTargetCell) return; + + const section = this._getGridSectionFromFocusTarget(focusTargetCell); + const isInteractingWithinActiveSection = this.interacting && section === this._activeRowGroup; + + focusTargetCell.tabIndex = isInteractingWithinActiveSection ? -1 : 0; + } + /** * @param {!HTMLTableRowElement} row * @param {number} index @@ -647,4 +692,47 @@ export const KeyboardNavigationMixin = (superClass) => _elementMatches(el, query) { return el.matches ? el.matches(query) : Array.from(el.parentNode.querySelectorAll(query)).indexOf(el) !== -1; } + + /** + * @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 + * @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} + * @private + */ + _getCellFocusEventLocation(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 cell = tableIndex >= 3 ? path[tableIndex - 3] : null; + + if (!section || !cell) return null; + + return { + section, + cell + }; + } + + /** + * Helper method that maps a focus target cell to the containing grid section + * @param {HTMLTableCellElement} focusTargetCell + * @returns {HTMLTableSectionElement | null} + * @private + */ + _getGridSectionFromFocusTarget(focusTargetCell) { + if (focusTargetCell === this._headerFocusable) return this.$.header; + if (focusTargetCell === this._itemsFocusable) return this.$.items; + if (focusTargetCell === this._footerFocusable) return this.$.footer; + return null; + } }; diff --git a/packages/vaadin-grid/test/keyboard-navigation.test.js b/packages/vaadin-grid/test/keyboard-navigation.test.js index 15bbbdc188..eeb837494b 100644 --- a/packages/vaadin-grid/test/keyboard-navigation.test.js +++ b/packages/vaadin-grid/test/keyboard-navigation.test.js @@ -11,6 +11,7 @@ import { nextFrame, up as mouseUp } from '@vaadin/testing-helpers/dist/index-no-side-effects.js'; +import { sendKeys } from '@web/test-runner-commands'; import { flushGrid, getCell, @@ -101,10 +102,19 @@ function getRowFirstCell(rowIndex) { return getRowCell(rowIndex, 0); } -function focusFirstBodyInput(rowIndex) { - const cell = getRowCell(rowIndex || 0, 1); - +function getCellInput(rowIndex, colIndex) { + const cell = getRowCell(rowIndex, colIndex); const input = getCellContent(cell).children[0]; + + if (!input.nodeName || input.nodeName.toLowerCase() !== 'input') { + throw new Error('Cell does not contain an input'); + } + + return input; +} + +function focusFirstBodyInput(rowIndex) { + const input = getCellInput(rowIndex || 0, 1); input.focus(); return input; } @@ -1470,6 +1480,53 @@ describe('keyboard navigation', () => { }); describe('interaction mode', () => { + before(async () => { + grid = fixtureSync(` + + + + + + + + + + + + + + + + + + + `); + + scroller = grid.$.scroller; + header = grid.$.header; + body = grid.$.items; + footer = grid.$.footer; + + grid._observer.flush(); + flushGrid(grid); + + await aTimeout(0); + }); + beforeEach(() => { focusItem(0); clickItem(0); @@ -1484,7 +1541,7 @@ describe('keyboard navigation', () => { }); it('should exit interaction mode when blurred', () => { - grid.setAttribute('interacting', ''); + grid._setInteracting(true); focusable.focus(); @@ -1492,7 +1549,7 @@ describe('keyboard navigation', () => { }); it('should exit interaction mode when tabbed into', () => { - grid.setAttribute('interacting', ''); + grid._setInteracting(true); tabToHeader(); @@ -1500,7 +1557,7 @@ describe('keyboard navigation', () => { }); it('should exit interaction mode when shift-tabbed into', () => { - grid.setAttribute('interacting', ''); + grid._setInteracting(true); shiftTabToFooter(); @@ -1558,14 +1615,48 @@ describe('keyboard navigation', () => { spy.restore(); }); - it('should focus the next input element when tabbing in interaction mode', () => { - right(); // focus the cell with input. + it('should focus the next input element when tabbing in interaction mode', async () => { + // Focus first input + right(); enter(); - tab(getCellContent(getRowCell(0, 1)).children[0]); // tab in the input + const nextInput = getCellInput(0, 2); + + await sendKeys({ press: 'Tab' }); + + expect(document.activeElement).to.equal(nextInput); + }); + + /** + * This test is a workaround for the fact that the sendKeys command does not support shift+tabbing ATM + * Ideally the test would try to shift tab to the previous input and check that the input in a previous cell + * was focused, despite the focus target cell being in the tab order between current and previous input + */ + it('should skip the grid focus target when tabbing in interaction mode', async () => { + // Focus first input + right(); + enter(); + + const nextInput = getCellInput(0, 2); + + // Modify grid state to get the focus target cell in the tab order between current and next input + grid._itemsFocusable = getRowCell(0, 2); + + await sendKeys({ press: 'Tab' }); + + expect(document.activeElement).to.equal(nextInput); + }); + + it('should move cell focus target when focusing the next input element in interaction mode', async () => { + // Focus first input + right(); + enter(); + + const nextCell = getRowCell(0, 2); + + await sendKeys({ press: 'Tab' }); - // expecting focusable item cell to remain in place, instead actual focus moves. - expect(grid._itemsFocusable).to.equal(getRowCell(0, 1)); + expect(grid._itemsFocusable).to.equal(nextCell); }); it('should focus the element with `focus-target` when entering interaction mode', () => { @@ -1730,7 +1821,7 @@ describe('keyboard navigation', () => { }); it('should exit interaction mode with escape', () => { - grid.setAttribute('interacting', ''); + grid._setInteracting(true); escape();