fix: disable grid section focus target in interaction mode #1960
Conversation
|
|
||
| /** @private */ | ||
| _onCellFocusIn(e) { | ||
| const location = this._getCellFocusEventLocation(e); |
There was a problem hiding this comment.
This is just a refactoring to give some semantic meaning to the magic jumbling of constants below.
| const cell = e.composedPath()[0]; | ||
| this._activeRowGroup = cell.parentNode.parentNode; | ||
| if (this._activeRowGroup === this.$.header) { | ||
| if (location) { |
There was a problem hiding this comment.
I changed this behaviour to update the focus target, even if the focus event targets an element in the cells content. Previously the focus target would only update when explicitely the cell itself was focussed.
I found that behaviour weird when you enter interaction mode, tab through multiple inputs, then exit interaction mode, and then the focus jumps back to the cell where you entered interaction mode. Instead now the focus is moved to the cell where you exit interaction mode.
| "@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", |
There was a problem hiding this comment.
Added this plugin to be able to send actual keyboard inputs in tests. Our current approach of creating keyboard events does not trigger the default browser action, such as switching focus.
A much bigger task here would be to upgrade existing tests to use this instead of the event-based approach, however that's a huge change as all tests using keyboard inputs need to be adapted to use the async API of the plugin.
|
@web-padawan Just requesting you here to double-check you are fine with just adding the send keys plugin here and employing it in the few tests that I needed. As mentioned above, updating existing tests to use this would be a larger effort as the API for our helpers would need to change to async. |
packages/vaadin-grid/src/vaadin-grid-keyboard-navigation-mixin.js
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
* fix: disable grid section focus target when in interaction mode * Add tests * Reenable test * Make interacting property readonly * Refactor to use destructuring
…1966) * fix: disable grid section focus target when in interaction mode * Add tests * Reenable test * Make interacting property readonly * Refactor to use destructuring Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
|
Seems to have caused a test breakage here. Something that the Web Component tests didn't catch. |
tulioag
deleted the
fix/disable-grid-section-focus-target-in-interaction-mode
branch
Description
When the grid is in interaction mode, then pressing tab / shift-tab is supposed to cycle focus between the focusable elements (inputs, selects,...) in the cell contents. The cycle could be broken if the focus target cell in a grid section was in the tab order between two inputs. For example:
This change modifies the interaction mode to remove the focus target cell in a grid section from the tab order while the user is in interaction mode. That allows to cycle between all inputs in that section of the grid, until the user exits interaction mode (e.g. pressing escape) or the focus leaves that section of the grid.
Fixes #1552
Type of change