You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
@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.
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
Possible solutions
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.
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: 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
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).
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
eui/src/components/datagrid/body/data_grid_cell.tsx
Lines 163 to 167 in 6ed06df
eui/src/components/datagrid/body/data_grid_cell.tsx
Lines 396 to 399 in 6ed06df
eui/src/components/datagrid/body/data_grid_cell.tsx
Lines 575 to 579 in 6ed06df
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
Possible solutions
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 quickchildNode.length === 1 && isTabbabletype check.Treat cells with 1 interactive child the same way we currently treat cells with 2+ interactive children

You can examine the way we treat
isExpandablecells 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 theEnterkey to enter the cell's focus trap and be able to tab between contents: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
Force cells with any interactive children to be
isExpandableI 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
isExpandablestate whenever cell contents change (either cells being reused by virtualization or data changing dynamically upstream).