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] Improve column header separators #9183

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MBilalShafi
Copy link
Member

@MBilalShafi MBilalShafi commented Jun 1, 2023

@MBilalShafi MBilalShafi added component: data grid This is the name of the generic UI component, not the React module! design: ux labels Jun 1, 2023
@mui-bot
Copy link

mui-bot commented Jun 1, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-9183--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 645.1 1,206.6 645.1 901.22 207.933
Sort 100k rows ms 543 1,015.1 853.4 835.8 156.75
Select 100k rows ms 277.7 351.5 295.8 300.44 26.592
Deselect 100k rows ms 179.8 312.6 223.2 230.4 48.325

Generated by 🚫 dangerJS against 05411c8

@MBilalShafi MBilalShafi marked this pull request as ready for review June 5, 2023 19:02
@@ -445,7 +445,6 @@
"columnHeaders",
"columnHeadersInner",
"columnHeadersInner--scrollable",
"columnSeparator--resizable",
Copy link
Member Author

Choose a reason for hiding this comment

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

Or possibly keep this class until v7 for being non-breaking?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 12, 2023
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@cherniavskii
Copy link
Member

I have merged a slightly related PR recently - #9076 (it's also the one that caused conflicts for this PR, sorry about that 😅)
Should we show the column header borders on mobile by default (without hover)?

@MBilalShafi
Copy link
Member Author

MBilalShafi commented Jun 15, 2023

Should we show the column header borders on mobile by default (without hover)?

I think it makes sense since we enabled other hover actions on mobile by default. Not sure about the resize handle in the hovered state though if it'll look too heavier 🤔

@cherniavskii
Copy link
Member

Not sure about the resize handle in the hovered state though if it'll look too heavier 🤔

We don't have to enable all hover styles by default on mobile, but only those applied on the column header cell level, so that we avoid issues like #8762 and #9073

@oliviertassinari oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ux labels Aug 18, 2023
@michelengelen michelengelen changed the base branch from master to next November 6, 2023 14:27
@MBilalShafi MBilalShafi changed the base branch from next to master March 21, 2024 02:38
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! design This is about UI or UX design, please involve a designer PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Improve column separators to display ability to resize columns
4 participants