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

[data grid] Fix keyboard navigation for actions cell with disabled buttons #10882

Merged
merged 7 commits into from
Nov 8, 2023

Conversation

michelengelen
Copy link
Member

@michelengelen michelengelen commented Nov 3, 2023


This fixes the keyboard navigation for the actions cell when the first (or all) button is disabled.
It does also introduce a new check for the managesOwnFocus and renames it to canManageOwnFocus

Screen.Recording.2023-11-03.at.10.17.00.mov

Fixes #10696

@michelengelen michelengelen added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Nov 3, 2023
@michelengelen michelengelen requested a review from a team November 3, 2023 09:18
@michelengelen michelengelen self-assigned this Nov 3, 2023
@mui-bot
Copy link

mui-bot commented Nov 3, 2023

Deploy preview: https://deploy-preview-10882--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against ccbe085

@romgrk
Copy link
Contributor

romgrk commented Nov 3, 2023

Couldn't test this, the CSB build links don't work. Do you have a CSB we can test this in?

@michelengelen
Copy link
Member Author

@michelengelen michelengelen changed the base branch from master to next November 6, 2023 14:17
@romgrk
Copy link
Contributor

romgrk commented Nov 7, 2023

Open question, if the focus is on the cell to the right and the user presses "left", should the focus be on the first action or would it make sense to focus the last?

@michelengelen
Copy link
Member Author

Open question, if the focus is on the cell to the right and the user presses "left", should the focus be on the first action or would it make sense to focus the last?

Good question: I did ask myself the same tbh.
IMO we could do with both, but the more intuitive would be to have it focus the last when coming from the right and the first when coming from the left.

I can create an issue to handle that, since this seems out of scope for this PR.

Agree @romgrk ?

@michelengelen michelengelen enabled auto-merge (squash) November 8, 2023 10:21
@michelengelen michelengelen merged commit ad2bd14 into mui:next Nov 8, 2023
5 checks passed
@michelengelen michelengelen deleted the grid/bug/10696 branch November 8, 2023 10:38
michelengelen added a commit to michelengelen/mui-x that referenced this pull request Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Datagrid] disabled button in actions column breaks keyboard navigation
3 participants