Focus trap issue in Datagrid when Full Screen with Flyout

[logeekal]

Hello Team,

We recently encountered a weird issue in kibana when using EuiDataGrid Component. Please find details below:

Issue:

When EuiDataGrid is expanded to full screen and one of the columns of data grid triggers an EuiFlyout, Focus is trapped inside datagrid and it is no-longer possible to focus on the fields inside flyout. We have a use-case where we want to autofocus on the field inside the flyout.

Note: Issue only occurs if DataGrid is in fullscreen mode.

Example

Please checkout this sandbox: https://codesandbox.io/s/euidatagrid-focus-trap-example-odfcp6

Please follow below steps to replicate the issue:

1. Switch EuiDataGrid to full screen mode.
2. Click on the first column ( icon - >) of each row to trigger flyout for that row.
3. You will notice rownumber being changed. But you can no longer focus on input box in the flyout.
4. If you press Tab, you will notice that focus is only trapped to the field in datagrid.

Please let me know if you need more details.

Observations

Please note the code section below :

https://github.com/elastic/eui/blob/a8805f47d4abaa706fef92321a1422da12ef1059/src/components/datagrid/data_grid.tsx#L368-L372

1. I believe line number 70 is the culprit which enables FocusTrap component.
2. If FocusTrap is enabled, it should have props 'clickOutsideDisables` true so that user can move out of FocusTrap if needed.
3. I am happy to raise a PR if you agree with the solution of adding clickOutsideDisables=isFullScreen prop.

[chandlerprall]

Looks like there is possibly something else going on with the focus trapping - if you only open one flyout the focus isn't trapped in the grid and the flyout's input is focusable. Your proposal to manage clickOutsideDisables makes sense but I'd like to first understand why the focus trap doesn't affect a single flyout.

[logeekal]

Thanks @chandlerprall for your revert.

I 100% agree. I have not been able to zero in on the cause of that issue yet. Will let you know if I have any update.

[cee-chen]

Thank you so much for the detailed CodeSandbox repro @logeekal, this is super helpful!

As Chandler mentions, the issue appears to be updating a single EuiFlyout in place by changing rowIndex. If you add a key={rowIndex} to the flyout (thus completely remounting the flyout whenever the rowIndex changes), this issue stops occurring:

https://codesandbox.io/s/euidatagrid-focus-trap-example-forked-xxrw1e?file=/demo.js:8129-8154

I also added some extra logic to toggle the flyout when clicking the same rowIndex:

https://codesandbox.io/s/euidatagrid-focus-trap-example-forked-xxrw1e?file=/demo.js:6771-7119

Another alternate approach/fix would be to set ownFocus={true} on the flyout, re-enabling the overlay mask. Closing the flyout between rowIndex changes also resolves the issue.

[logeekal]

Thank you @constancecchen , this works perfectly in our use case. You are awesome 🦸‍♀️ .

Although, should this approach be considered a workaround?

I say so because of 2 reasons:

1. I think Datagrid full screen focustrap should be predictable but not restrictive and the implementing app should be able to customize where the focus should be (datagrid or not).

2. Datagrid's behaviour should be similar in full screen and non full-screen mode from the user experience perspective. If there no need to remount the flyout when datagrid is not in full-screen mode, there should not be a need to remount it when it is full-screen mode.

Additionally, I am curious, why Datagrid lets flyout pull focus when it is remounted but NOT when it is updated.

Please let me know your thoughts.

Another alternate approach/fix would be to set ownFocus={true} on the flyout, re-enabling the overlay mask. Closing the flyout between rowIndex changes also resolves the issue.

You are right about that but currently, we want to have ownFocus as false so that user can navigate from one row to other without having to close the flyout. but this point is good to be here for future reference.

[cee-chen]

This is definitely a workaround, but to be honest a focus trap within a focus trap (i.e., the flyout within full screen mode) is an edge case scenario that generally shouldn't be happening. I would say the "most correct" solution(s) would be to set ownFocus to true on the EuiFlyout, or alternatively to possibly disable fullscreen mode on the data grid when the flyout opens (which you can do by accessing the imperative ref API of datagrid). That being said I appreciate that that is not the UX you want, hence the workaround 😅

Additionally, I am curious, why Datagrid lets flyout pull focus when it is remounted but NOT when it is updated

I don't think this is quite what's happening - I think the issue is the nested focus traps / 2 focus traps being present on the page, as I mentioned. Normally ownFocus={true} outside clicks will dismiss the flyout, and ownFocus={false} outside clicks are allowed but focus is still technically trapped within the flyout (see docs). But in this case, the outside clicks are taking the consumer into the fullscreen data grid focus trap and deactivating the flyout focus. You can see this happening below:

So what the workaround is actually doing is remounting/resetting the flyout focus trap's disabled focus 😅

To be honest, the most technically correct "fix" here on EUI's end might be to remove the focus trap completely when ownFocus={false} is set - it can cause shenanigans like this. However, that's a significant accessibility/keyboard user UX loss. So in this case it might have to be a conscious tradeoff for consumers who opt to turn off ownFocus (and one we can try to guide consumers to avoid).

[github-actions[bot]]

👋 This issue hasn't seen activity in 3 days, so we're automatically closing this issue as answered. Please leave a comment if that's not the case, or if you have any remaining questions or issues.