-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[DataGrid] Improve accessibility of the sort icon #20430
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
Conversation
|
Deploy preview: https://deploy-preview-20430--material-ui-x.netlify.app/ Bundle size report
|
kenanyusuf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although the contrast is undoubtably better, I don't think we should change the color of the icon when the sort is active — it should be consistent with the other icon buttons displayed in the column header, otherwise we get a mix of shades of grey which doesn't look nice:
We could just bump the opacity of the hover icon slightly to 0.75.
@cherniavskii we didn't take this into account when we were discussing the issue. I have reverted the color and increased the opacity. I can leave it like that or round it up to |
kenanyusuf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.78 looks good 👍

Closes #20189
Both, column header sort icon and the pivoting panel sort icons are updated.
I had to change the pivoting panel hover state background color to make it more highlighted. I think that this approach is better in general, because it makes the contrast larger if you hover exactly on the element. Currently, it is the opposite.
For non-text elements contrast ratio should be at least 3:1.
I have measured the sort icon color and background color contrast in various states and in both light and dark modes.