mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.08k stars 1.26k forks source link

[DataGrid] Focus leaves the datagrid #4201

Closed iluvmemes closed 2 years ago

iluvmemes commented 2 years ago

Duplicates

Latest version

Current behavior 😯

When editMode="row" pressing tab on the final field moves the focus out of the component and onto various browser objects like the refresh button or address bar.

Expected behavior 🤔

The expected behavior would be one of any numerous possibilities including but not limited to:

Steps to reproduce 🕹

Steps:

  1. Create a datagrid with editable fields with the editMode="row" prop
  2. Enable editing
  3. Press tab until you've run out of fields
  4. Focus is now on browser window controls.

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

cherniavskii commented 2 years ago

This one is tricky.

Comparing to cell editing:

Extending this to row editing, I would expect:

What do you think?

iluvmemes commented 2 years ago

This one is tricky.

Comparing to cell editing:

  • start editing last cell in the row
  • press Tab - cell editing ends, changes are saved, same cell is focused
  • press Tab - focus is shifted to rowsPerPage menu

Extending this to row editing, I would expect:

  • start row editing
  • press Tab to navigate between cells until the last cell in the row
  • press Tab in the last cell - row editing ends, changes are saved, last cell is focused
  • press Tab - focus is shifted to rowsPerPage menu

What do you think?

In thinking about it further... In a lot of tables/grids in other applications when you tab after the last cell, the focus would move to the first cell on the next row. My gut instinct tells me that the user would likely expect this behavior.

What about:

flaviendelangle commented 2 years ago

Did you test it on the new editing implementation @cherniavskii ?

m4theushw commented 2 years ago

I pushed 35446f0 in #4060 fixing this behavior in the new API. Now it won't focus the next element in the tab sequence, only exit like in the cell editing.

In thinking about it further... In a lot of tables/grids in other applications when you tab after the last cell, the focus would move to the first cell on the next row. My gut instinct tells me that the user would likely expect this behavior.

The grid is treated as a "composite widget". In this case, only the active cell is included in the page tab sequence. To navigate between cells the arrow keys should be used. Here's one example from W3C: https://www.w3.org/TR/wai-aria-practices-1.2/examples/grid/dataGrids.html

iluvmemes commented 2 years ago

I pushed 35446f0 in #4060 fixing this behavior in the new API. Now it won't focus the next element in the tab sequence, only exit like in the cell editing.

@m4theushw and @flaviendelangle I think that would not be very intuitive behavior. Basically, row editing turns the row into a form for the user. When users typically fill out forms, the tab key is used. Removing the existing behavior of tabbing between elements and replacing it with completely different functionality seems counter-intuitive and regressive. 😞

Some of the datagrids mentioned in other issues on the current roadmap as comparable to the functionality for that particular feature have row editing as well.

See:

@oliviertassinari I tried to find the original issue written for this feature but was unsuccessful. Are we sure this is how this feature should be implemented?

m4theushw commented 2 years ago

We're not replacing the current behavior. In the new API pressing Tab will always go to the next input and in the last input it will exit the edit mode. You can check the demo in the unreleased docs: https://deploy-preview-4060--material-ui-x.netlify.app/components/data-grid/editing/#row-editing

There's still one small bug where it should move focus to the row below. We'll fix that.

iluvmemes commented 2 years ago

Ahh, thank you for the clarification. 👍