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.56k stars 1.33k forks source link

[data grid] Select All keyboard shortcut doesn't work if a row is selected by clicking to the right of the last column #15352

Open mbiggs-gresham opened 2 weeks ago

mbiggs-gresham commented 2 weeks ago

Steps to reproduce

Steps:

  1. Use the mouse to select a row by clicking in the whitespace to the right of the last column. The row will appear correctly selected.
  2. Press CTRL+A or CMD+A to select all rows.
  3. The selection will select the dom elements and not the rows.

https://github.com/user-attachments/assets/b62ba302-0fcb-421d-9144-3e02c19ef81c

Current behavior

The dom elements are selected instead of the rows.

Expected behavior

The rows should be selected.

Context

No response

Your environment

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

Search keywords: select all Order ID: 45466

mbiggs-gresham commented 2 weeks ago

Actually, none of the keyboard shortcuts work, as i'm guessing it's falling back to the browsers own handling. It's just rather confusing when you have a grid with only a few columns.

k-rajat19 commented 2 weeks ago

Hi, White space contains empty cells and empty cells have no event listener attached to them, which is why it doesn't work. From a UX perspective I think this issue should be fixed, registering a separate key-down event listener for empty cells would help in this case but I'm not sure whether @mui/xgrid team agrees with this or not.

michelengelen commented 2 weeks ago

@cherniavskii is there a W3C requirement for this? I could not find any apart from it stating that "if a grid supports selection CTRL+A typically is used to select all cells" ... it had no mention of whitespace and the selection behavior there! 🤷🏼

michelengelen commented 1 week ago

cc @joserodolfofreitas

cherniavskii commented 1 week ago

is there a W3C requirement for this?

I don't think so. When the white space is clicked, the focus moves from the cell (which actually listens for keyboard shortcuts) to the main element. So the empty cells are never focused.

Should clicking on the empty cell select the row though 🤔?

mbiggs-gresham commented 1 week ago

From a UX point of view you are operating within the context of a grid component. I understand the technical reasons why it currently doesn't work, but it doesn't offer a very good UX. You wouldn't use excel and expect the keyboard to only work on certain cells.

In our app we have two grids stacked vertically. The top grid has lots of columns and the grid underneath only 3. When the users navigate from the top grid to the one below, they naturally just move their cursor down the page, not down and to the left, and expect the grid below to work the same as the one they were just on. The grid may also have blank values in some cells, so trying to explain to them that they have to click a cell from a column that may or may not contain text in order for things to work is just confusing. I appreciate it sounds like a use case specific to our app, but I'm sure there's a lot of other scenarios.

michelengelen commented 1 week ago

@mbiggs-gresham you've got a point on that one. I did some more research and could not find any mention of this behavior from an official resource regarding accessibility, so we can basically decide on our own what would be the best approach to take.

IMHO the least disrupting for our users would be to allow for the selection keyvbard shortcuts (and possibly others) as the current behavior is that rows are hoverable and get selected when clicked into the whitespace. So it's just a feature on top.

The other way around we would remove the hovering and selection capabilities, which in turn would feel weird.

Thoughts @mui/xgrid ?

KenanYusuf commented 1 week ago

Should clicking on the empty cell select the row though 🤔?

@cherniavskii I think so, the empty cell is still visually part of the row so it should behave like a regular cell when interacted with (with the exception to cell selection which does not make sense for empty cells).

I think we should implement this, would be a nice usability improvement even if it's not mentioned on the spec.

cherniavskii commented 1 week ago

I think we should implement this, would be a nice usability improvement even if it's not mentioned on the spec.

@KenanYusuf Do you mean making the Ctrl+A shortcut work when no cell is focused? Or something else?

KenanYusuf commented 6 days ago

@cherniavskii yes, the shortcut.

k-rajat19 commented 6 days ago

Hi @KenanYusuf and @cherniavskii , I would like to work on this

registering a separate key-down event listener for empty cells

does this approach look good?

michelengelen commented 6 days ago

Hi @KenanYusuf and @cherniavskii , I would like to work on this

registering a separate key-down event listener for empty cells

does this approach look good?

Thanks for picking this up @k-rajat19

I guess the best approach would be to look into how the selection events are being handled in cells and use the same approach in the empty filler cell. 👍🏼