Skip to content

[DataGrid] Column header design updates#14293

Merged
kenanyusuf merged 18 commits intomui:masterfrom
kenanyusuf:column-header-design-updates
Sep 3, 2024
Merged

[DataGrid] Column header design updates#14293
kenanyusuf merged 18 commits intomui:masterfrom
kenanyusuf:column-header-design-updates

Conversation

@kenanyusuf
Copy link
Copy Markdown
Member

@kenanyusuf kenanyusuf commented Aug 22, 2024

This PR includes a few small design updates to the column headers.

Preview: https://deploy-preview-14293--material-ui-x.netlify.app/x/react-data-grid/#mit-license-free-forever

1. Aggregation label type and layout updates

Before After
localhost_3001_playground_ (3) localhost_3001_playground_ (2)

2. Column separator updates

Before After
localhost_3001_playground_ (3) localhost_3001_playground_
localhost_3001_playground_ (5) localhost_3001_playground_ (1)
localhost_3001_playground_ (2) localhost_3001_playground_ (4)

3. Fixed overlapping of column menu button and resize handle

  • Reduced the resize handle hit area from 24px to 10px and repositioned the column menu buttons to avoid overlapping of the two actions.
  • Also made column menu icon size consistent with the sort icon by switching the size prop from small to inherit.
Before After
Screenshot 2024-08-22 at 15 35 38 Screenshot 2024-08-22 at 15 41 54

@kenanyusuf kenanyusuf added scope: data grid Changes related to the data grid. design This is about UI or UX design, please involve a designer. labels Aug 22, 2024
@mui-bot
Copy link
Copy Markdown

mui-bot commented Aug 22, 2024

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

Generated by 🚫 dangerJS against 8051e7c

Comment thread packages/x-data-grid/src/components/containers/GridRootStyles.ts
@kenanyusuf kenanyusuf marked this pull request as ready for review August 23, 2024 15:56
@kenanyusuf kenanyusuf requested a review from a team August 23, 2024 16:00
'&:hover': {
color: (t.vars || t).palette.text.primary,
// Reset on touch devices, it doesn't add specificity
// Always appear as draggable on touch devices
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍🏻

Comment thread packages/x-data-grid/src/components/containers/GridRootStyles.ts
Comment thread packages/x-data-grid/src/components/containers/GridRootStyles.ts Outdated
Comment thread packages/x-data-grid/src/components/containers/GridRootStyles.ts Outdated
Comment thread packages/x-data-grid/src/components/containers/GridRootStyles.ts
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Side note: Sometimes Argos diff surprises me – it doesn't pick up the separator 😅

Screen.Recording.2024-08-23.at.23.36.55.mov

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, this PR warrants a minor release.

@dosubot dosubot Bot added the size:L label Aug 27, 2024
Comment thread packages/x-data-grid/src/constants/gridClasses.ts
Copy link
Copy Markdown
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

The general direction looks right 👍

Copy link
Copy Markdown
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

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

Nice work!
It also reminded me of a similar issue on the header filters with compact density.
Opened a separate PR for it.

fontWeight: theme.typography.fontWeightMedium,
color: (theme.vars || theme).palette.primary.dark,
textTransform: 'uppercase',
lineHeight: 'normal',
Copy link
Copy Markdown
Member

@MBilalShafi MBilalShafi Aug 28, 2024

Choose a reason for hiding this comment

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

I'm wondering if there is a possibility some users customizing it using theme.*.* variables. What do you think? 🤔

Copy link
Copy Markdown
Member Author

@kenanyusuf kenanyusuf Aug 28, 2024

Choose a reason for hiding this comment

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

I'm purposefully deviating from the theme.typography.caption.lineHeight value here because it is too large. normal line-height should keep the spacing looking ok between the column header title and aggregation label I think.

If users want to override it, they have the class I guess?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed, but I'm only worried if some existing users' styles would break with this.
It's obviously not very likely but theoretically, some users may be using theme.typography.caption.lineHeight to override this style.

It's a nitpick though. Feel free to merge it in current shape.

Comment thread packages/x-data-grid/src/components/columnHeaders/GridColumnHeaderSeparator.tsx Outdated
Copy link
Copy Markdown
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

Looks good to me!
I prefer not to include this PR in this week's release with the Material UI v6 support. Keeping other changes to a minimum would make the upgrade easier for those who need Material UI v6.

@oliviertassinari
Copy link
Copy Markdown
Member

oliviertassinari commented Sep 1, 2024

#5274, #9162, #9776 might be related to this PR.

@kenanyusuf kenanyusuf merged commit bb26d25 into mui:master Sep 3, 2024
@kenanyusuf kenanyusuf deleted the column-header-design-updates branch March 14, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

design This is about UI or UX design, please involve a designer. scope: data grid Changes related to the data grid.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[data grid] Improve column separators to display ability to resize columns [data grid] Improve aggregation label and value UX

6 participants