Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DataGrid] Focus leaves the datagrid #4201

Closed
2 tasks done
iluvmemes opened this issue Mar 15, 2022 · 7 comments · Fixed by #4060 or #4252
Closed
2 tasks done

[DataGrid] Focus leaves the datagrid #4201

iluvmemes opened this issue Mar 15, 2022 · 7 comments · Fixed by #4060 or #4252
Assignees
Labels
accessibility a11y component: data grid This is the name of the generic UI component, not the React module! feature: Editing Related to the data grid Editing feature new feature New feature or request

Comments

@iluvmemes
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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:

  • Do nothing, focus remains on current field.
  • Wrap to the first field
  • Exit edit mode and save the change (risky)
  • ???

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

@iluvmemes iluvmemes added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 15, 2022
@oliviertassinari oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Mar 15, 2022
@cherniavskii cherniavskii added accessibility a11y new feature New feature or request feature: Editing Related to the data grid Editing feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 16, 2022
@cherniavskii
Copy link
Member

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?

@iluvmemes
Copy link
Author

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:

  • 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, callback fires
  • first cell of next row is focused OR if there isn't a new row, focus is shifted to rowsPerPage menu

@flaviendelangle
Copy link
Member

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

@m4theushw m4theushw changed the title Focus leaves the datagrid [DataGrid] Focus leaves the datagrid Mar 16, 2022
@m4theushw
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

Ahh, thank you for the clarification. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: data grid This is the name of the generic UI component, not the React module! feature: Editing Related to the data grid Editing feature new feature New feature or request
Projects
None yet
6 participants