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

[DataGridPro] Improve compact density UI for header filters #14373

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MBilalShafi
Copy link
Member

@MBilalShafi MBilalShafi commented Aug 28, 2024

Fixes #13048

The current UI of the inputs being used in filter input components doesn't leave much room for the label to be shown on the top, this PR hides the top label when density="compact"

Before Preview: https://stackblitz.com/edit/react-jre9au?file=Demo.tsx
After Preview: https://codesandbox.io/p/sandbox/vibrant-neumann-wnvjpx

@MBilalShafi MBilalShafi added 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 feature: Filtering on header Related to the header filtering (Pro) feature labels Aug 28, 2024
@mui-bot
Copy link

mui-bot commented Aug 28, 2024

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

Generated by 🚫 dangerJS against b22d23d

@MBilalShafi MBilalShafi marked this pull request as ready for review August 28, 2024 13:30
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 28, 2024
@MBilalShafi MBilalShafi requested review from KenanYusuf and a team August 28, 2024 13:30
@KenanYusuf
Copy link
Member

@MBilalShafi definitely an improvement over the current design, however, I worry that we're impacting usability by removing the label as there is no other way to tell which operator is selected without removing the value or opening the operator menu.

What do you think of modifying the control styles slightly so that they take up the whole column header cell? We could drop the font-size to 14px, and adjust the floating label position so that it's closer to the input value. Here's a quick mockup:

Header filter mockup (3)

Let me know what you think.

@MBilalShafi
Copy link
Member Author

@KenanYusuf Thank you for creating the mockup. So much crammed-up space previously didn't seem nice to me which is why I was inclined to remove the label altogether instead of reducing the font sizes. But in the mockup, it looks kinda decent. 👍

@MBilalShafi MBilalShafi force-pushed the compact-header-filters branch from b3b429e to 7048c47 Compare September 3, 2024 19:15
@arminmeh
Copy link
Contributor

The position of the filter operator icon feels off.

With the default density it is aligned with the content and the input reset icon
default

For compact density it is centered
compact

@MBilalShafi
Copy link
Member Author

Thanks @arminmeh for pointing it out. We are having an active discussion around updating the header filters UI (around the text underline) and UX (around keyboard navigation), due to which the current PR is kind-of stale, I'll mark it as a Draft until then.

Feel free to check or jump into the conversation.

@MBilalShafi MBilalShafi marked this pull request as draft September 30, 2024 11:42
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 feature: Filtering on header Related to the header filtering (Pro) feature size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Filter headers need adjustment for density="compact"
4 participants