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] Fix separator styles on touch devices #14471

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

KenanYusuf
Copy link
Member

@KenanYusuf KenanYusuf commented Sep 3, 2024

Fixes a couple of issues found on touch devices, introduced here #14293

Snag 1: Double border caused by separator not being hidden when a cell is focused

Before After
Screenshot 2024-09-03 at 21 13 36 Screenshot 2024-09-03 at 21 13 27

Snag 2: Last separator always in "active" state

Before After
Screenshot 2024-09-03 at 21 12 51 Screenshot 2024-09-03 at 21 13 00

@KenanYusuf KenanYusuf added the component: data grid This is the name of the generic UI component, not the React module! label Sep 3, 2024
@mui-bot
Copy link

mui-bot commented Sep 3, 2024

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

Generated by 🚫 dangerJS against 1b18a02

@KenanYusuf KenanYusuf marked this pull request as ready for review September 3, 2024 20:21
opacity: 1,
color: (t.vars || t).palette.primary.main,
Copy link
Member Author

@KenanYusuf KenanYusuf Sep 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was problematic - the conditions of the separator being the primary color are different on touch devices to desktop. Moved here: https://github.com/mui/mui-x/pull/14471/files#diff-e1db53193a627d36d1cd3f651a954ee1e32ecb93caa020446631d722bb1be75cR415-R422

@KenanYusuf KenanYusuf changed the title [DataGrid] Fix separator color on touch devices [DataGrid] Fix separator styles on touch devices Sep 3, 2024
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus),
& .${c.columnHeader}:has(+ .${c.columnHeader}:focus-within)`]: {
[`.${c['columnSeparator--resizable']}`]: {
color: (t.vars || t).palette.primary.main,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation behind having separators in an "active" state when the column header cell is selected?

Copy link
Member Author

@KenanYusuf KenanYusuf Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the colors clash a little when the column group has a blue outline and the separators are grey. This situation never occurs on desktop because the separators are not visible when the column header is focused until the side is hovered (which also turns the separator blue).

Without:
Screenshot 2024-09-04 at 12 28 31

With:
Screenshot 2024-09-04 at 12 28 38

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, this makes sense then 👍🏻

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be in favor of removing these lines: https://github.com/mui/mui-x/pull/14471/files#diff-e1db53193a627d36d1cd3f651a954ee1e32ecb93caa020446631d722bb1be75cR447-R449
So that the separator is in an "active" state when you drag it on the touch screen.
What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout, updated here: 1b18a02

@cherniavskii cherniavskii added enhancement This is not a bug, nor a new feature feature: Column resize labels Sep 4, 2024
@KenanYusuf KenanYusuf merged commit 2e38064 into mui:master Sep 4, 2024
22 checks passed
arthurbalduini pushed a commit to arthurbalduini/mui-x that referenced this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature feature: Column resize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants