[data-grid] Cannot tab to the detail panel in correct order when there are other clickable items in columns

[sshty2018]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Current behavior 😯

When we add a detail panel and there are clickable items in other columns, user cannot tab to the detail panel in the correct order.

Expected behavior 🤔

Detail Panel should get focus in the correct order

Steps to reproduce 🕹

https://codesandbox.io/s/keen-rumple-3sqwdg?file=/src/App.tsx

1. Hit Tab on datagrid rows. The focus is now on "Show All" link in the last column
2. Hit Tab again. The focus is now on the "Show All" link of the next row.
3. Detail Panel gets focus only after tabbing through all the links.

Is there any workaround for this?

Context 🔦

No response

Your environment 🌎

`npx @mui/envinfo`

``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Order ID 💳 (optional)

No response

[alexfauquette]

It sounds like the focus order is broken because of the rendering strategy. components are rendered in the following order:

• left pinned columns
• right pinned columns
• unpinned columns
• detail panel

(see https://github.com/alexfauquette/material-ui-x/blob/0c473f4d24d74860e39e2c9c5e936a5fe25487a2/packages/grid/x-data-grid-pro/src/components/DataGridProVirtualScroller.tsx)

So if we put focusable elements in left/right pinned columns, we get the same behavior, focus move along columns instead of rows https://codesandbox.io/s/winter-rgb-wlxfko?file=/src/App.tsx

To solve this we should refactor the rendering such that each row are added one after the other. About workaround, I only know one way to override focus order: tab-index but using this for all the rows would be a nightmare.

I'm wondering if moving with Tab is the main goal or is it to access the detail panel with keyboard.

For the second option we could imagine that Arrow Down move focus to the detail panel if it is open

[alexfauquette]

I get an idea of a dirty fix if really needed. You can override the tab order programmatically as follow:

• listen to onKeyDown
• look if the detail panel with same row id exists.
• if yes use event.preventDefault() to block focus modification, and set it manually to the detail panel

For this to work, you should also override default focus logic of the first and last focusable element of detail panel

function RenderExtraIcon({ row }) {
    return (
      <Link
        sx={{ whiteSpace: "nowrap" }}
        onKeyDown={(event) => {
          if (event.code === "Tab") {
            const detailPanel = document.getElementById(
              `detail-panel${row.id}`
            );
            if (detailPanel !== null) {
              const firstFocusableElement = detailPanel.querySelector(
                'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
              );
              firstFocusableElement.focus();
              event.preventDefault();
            }
          }
        }}
        component="button"
      >
        Show All
      </Link>
    );
  }

https://codesandbox.io/s/epic-haslett-8f76tx?file=/src/App.tsx

[m4theushw]

@alexfauquette It was implemented this way for performance reasons. I assumed that the detail panels would be complex elements, so rendering them as often as the rows are rendered could lag the scroll. Also, rendering them with GridRow requires a GridRowPro or a custom logic in useGridVirtualScroller to change which row to render. Feel free to experiment changing the way they are rendered.

Another alternative is to use autoFocus in the first tabbable element from the detail panel. As soon it's expanded, the focus will move to this element. The tradeoff is that when the focus is on the last element you need manually move it back to the row when pressing Tab.

So if we put focusable elements in left/right pinned columns, we get the same behavior, focus move along columns instead of rows

This will be fixed by #4198

[alexfauquette]

@m4theushw the fix works only if cells does not contain focussable elements

https://codesandbox.io/s/conditionalvalidationgrid-material-demo-forked-rxnzdz?file=/demo.js

[m4theushw]

@alexfauquette I could argue that this example is not following https://www.w3.org/TR/wai-aria-practices-1.1/examples/grid/dataGrids.html. Only one element of the grid can be focusable. The navigation is primarily through the arrow keys.

[alexfauquette]

That's a good argument (I was struggling to find this W3-ARIA document) 😉

So the objective is not to make the proposed code work with Tab which is inconsistent with accessibility recommendation but to:

• Have a keyboard interaction allowing to enter/leave the detail panel
• Explain in the doc how to handle focus in renderCell (use tabIndex={-1} to remove them from the tab order except if the cell has the focus) which is in the continuity with #1465

[m4theushw]

Have a keyboard interaction allowing to enter/leave the detail panel

Yes, that's something we could implement.

Explain in the doc how to handle focus in renderCell (use tabIndex={-1} to remove them from the tab order except if the cell has the focus) which is in the continuity

I'm not 100% sure that we should be too harsh suggesting using tabIndex=-1 in every button. The ARIA documentation doesn't go into the detail of how master/details should work but I found something from the section about navigating inside cells with interactive widgets:

Tab: moves focus to the next widget in the grid. Optionally, the focus movement may wrap inside a single cell or within the grid itself.

Reading this, I think we could have a similar behavior from the GridFilterForm you implemented. Once the panel is opened, we check if the element passed as the content of the panel sets a value to the focusElementRef. If there's one, we call focusElementRef.focus(). This way the user can implement its own behavior, while Tab will also work. But when on the last element, the developer should handle moving the focus back to the detail panel. Arrow keys inside the detail panel could do nothing. To restore their behavior, press Esc.

Escape: restores grid navigation. If content was being edited, it may also undo edits.

[alexfauquette]

In the filter panel, the problem was to set the focus on an input that was the most interesting one: the last one

Not sure it worth adding an imperative focusing for a cell that should contain 1 or 2 interactive elements

My idea was only to add doc, no code, since it's already possible to respect WARAI as follow:

renderCell: ({ hasFocus }) => (
      <button
            tabIndex={hasFocus ? 0 : -1}
            onKeyDown={(event) => { event.stopPropagation(); }}
      >
          I'm focussable
      </button>
)

The tabIndex={hasFocus ? 0 : -1} prevent navigating with tabulation if the cell does not have the focus The onKeyDown={(event) => { event.stopPropagation(); }} is here to prevent grid navigation when pressing Space

Nice to have, I did not know but it already implement the following:

• When the cell is focused, the first button gets the focus
• When you press Shift+Space the focus move the the rootCell

For the last behavior: "focus movement may wrap inside a single cell or within the grid itself" it could be done with a <TrapFocus/>

Here is a demo with the proposed recomendations https://codesandbox.io/s/conditionalvalidationgrid-material-demo-forked-yfd1c1?file=/demo.js

[m4theushw]

Yeah, setting tabIndex also works. Only one problem is if the element that receives tabIndex has not been rendered yet. I had a problem like that with ButtonBase in React 18: https://github.com/mui/material-ui/pull/31950 Not sure if it won't be a problem here but we can give it a try on this approach. With cells that contain more than one interactive elements I would set tabIndex=0 only on the first, then manage internally which element receives focus.

Only one of the focusable elements contained by the grid is included in the page tab sequence.