Skip to content

[EuiDataGrid] Focus behavior on cells that are isExpandable: false and have 1 interactive child Β #5690

@cee-chen

Description

@cee-chen

To establish context, the scenario this issue refers to is the 3rd column in https://eui.elastic.co/#/tabular-content/data-grid-cells-popovers#focus:

The logic that controls this behavior (focusing immediately into the only interactive child if isExpandable is false) is set in these 3 locations in data_grid_cell:

Expand to show code snippets

if (this.isExpandable() === false && interactables.length === 1) {
// Only one element can be interacted with
interactables[0].focus({ preventScroll });
} else {
cell.focus({ preventScroll });

if (interactables.length === 1 && this.isExpandable() === false) {
interactables[0].focus();
this.setState({ disableCellTabIndex: true });
}

const interactables = this.getInteractables();
if (interactables.length >= 2) {
switch (event.key) {
case keys.ENTER:
// `Enter` only activates the trap

UI concerns

@cchaos originally raised this issue as a UI one. Due to the fact that querying through the cell's children can take JS a non-zero amount of time to do, there is an intermittent flash of focus ring on Chrome before focus is manually shifted to the child:

713a84e3-1a4b-4563-b51b-e0803bc8e5de.mp4

Accessibility concerns

@cchaos also highlighted that this is a potential accessibility issue as well. This behavior unintentionally affects cells that have single interactive children (e.g.) links but also other non-interactive content.

In the above auto heights example/screencap (https://elastic.github.io/eui/#/tabular-content/data-grid-row-heights-options#auto-heights-for-rows), the second column items have a title link that is immediately focused on arrow key navigation, with the rest of the cell contents not being read out.

After some testing, I determined that screen reader ctrl+opt+arrow key navigation still works to get to the rest of the cell contents, so it's not a hugely blocking issue, but still a friction point that could likely be smoothed out.

WAI-ARIA spec

My best guess as to the reason why we implemented the above conditional logic for non-expandable cells with single interactive children is that we were following W3's datagrid spec, particularly the "Description" cells: https://www.w3.org/TR/wai-aria-practices-1.2/examples/grid/dataGrids.html

screencap

Possible solutions

  1. Change our conditional logic to check that there's only 1 child
    I think where our current logic falls apart is determining whether there is one interactive element in the cell vs the interactive element being the only child of the cell. For example, our logic works well for control columns with 1 button, but not for the auto height example with a link and paragraphs of text.
    Caveat: it is not trivial to tell (DOM-wise) whether a cell's content is "just one interactive element" or not; for example if a user wraps their single <button> in a <span> that defeats a quick childNode.length === 1 && isTabbable type check.

  2. Treat cells with 1 interactive child the same way we currently treat cells with 2+ interactive children
    You can examine the way we treat isExpandable cells with 2+ interactive children in Column of https://elastic.github.io/eui/#/tabular-content/data-grid-focus. Currently with 2+ interactive children, keyboard users are required to press the Enter key to enter the cell's focus trap and be able to tab between contents:
    screencap
    Caveat: We should likely still keep an exception for control columns here, since control columns are typically only buttons/checkboxes etc., which should auto-focus if possible

  3. Force cells with any interactive children to be isExpandable
    I played around with a spike for approach 2 and still ran into some screen reader difficulties. I couldn't get screen reader navigation to work quite as expected when focus is trapped within a cell. SR navigation within cell popovers always worked, though, so it's tempting to take the easy way out and force the expansion popover whenever interactive children are present.
    Caveat: This isn't as technically easy as I make it sound, as we'll need to add some sort of mutation observer to update isExpandable state whenever cell contents change (either cells being reused by virtualization or data changing dynamically upstream).

Metadata

Metadata

Assignees

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions