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] Filtering on the Header followup actions #9034

Open
6 of 8 tasks
MBilalShafi opened this issue May 18, 2023 · 8 comments
Open
6 of 8 tasks

[DataGridPro] Filtering on the Header followup actions #9034

MBilalShafi opened this issue May 18, 2023 · 8 comments
Assignees
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: Filtering Related to the data grid Filtering feature umbrella For grouping multiple issues to provide a holistic view

Comments

@MBilalShafi
Copy link
Member

MBilalShafi commented May 18, 2023

This one supersedes #7760 with the action items left over.

@MBilalShafi MBilalShafi added component: data grid This is the name of the generic UI component, not the React module! feature: Filtering Related to the data grid Filtering feature enhancement This is not a bug, nor a new feature labels May 18, 2023
@MBilalShafi MBilalShafi self-assigned this May 18, 2023
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid May 18, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented May 22, 2023

  1. Another idea. This Pro feature is in the middle of the MIT page: https://deploy-preview-7760--material-ui-x.netlify.app/x/react-data-grid/filtering/#header-filters.

I think that we could break down the page into multiples, moving the Pro feature to its own page, not alienating the community. Plus the docs page is quite big, it would be more focused.

Otherwise, I think to move to the bottom of the page.

@MBilalShafi
Copy link
Member Author

MBilalShafi commented May 22, 2023

  1. I think that we could break down the page into multiples

@oliviertassinari Yep that's a good idea, I've tried to do a similar thing in this PR: #9074

@oliviertassinari
Copy link
Member

oliviertassinari commented May 31, 2023

  1. Bugs related to the cell blur logic:
Screen.Recording.2023-05-31.at.16.23.48.mov

https://mui.com/x/react-data-grid/filtering/#header-filters

7.1. The data grid cell isn't blurred at the first click. I have to click twice
7.2. The input is still shrunk after the data grid is fully blurred

  1. Enter blur the filter input. I think that we should test this with end-users. I got tricked by this again. I don't know if the filter is applied instantly, my instinct is to press Enter to run the filter but now, when I type again, nothing happens as the input is blurred.
    Last discussion about it in [DataGridPro] Filtering on Column Header #7760 (comment).

@MBilalShafi
Copy link
Member Author

Enter blur the filter input. I think that we should test this with end-users. I got tricked by this again. I don't know if the filter is applied instantly, my instinct is to press Enter to run the filter but now, when I type again, nothing happens as the input is blurred.

This one is interesting, the behavior that filters are applied on debounce and not on enter is something that users already experienced with the regular filter panel, not sure about other users but for me, the loading icon that appears during typing is a nice visual feedback that depicts the filters are being applied in the background, but yes we can test this with users and see what most of the users think about it 👍

If we change the behavior to apply the filter on Enter though, there's a nice way to stop differentiating between the cell and input field. When the user starts typing, the arrow keys are passed to the input, on pressing Enter, the filters are applied, and the focus is still on the input (cursor is blinking), but the arrow keys are now passed to the keyboard navigation (which again can pass to the input when user starts typing something). But I am not sure if this will look natural to the users 🤔 I'll try to do a demo to validate the idea.

@romgrk
Copy link
Contributor

romgrk commented Jun 6, 2023

It feels a bit off to me that the separator between headers & header filters has more visual weight than the one between header filters & cells. I feel like if there is a bolder separator, it should be between the whole header section and the cells.

@MBilalShafi MBilalShafi added the umbrella For grouping multiple issues to provide a holistic view label Jun 6, 2023
@MBilalShafi
Copy link
Member Author

@romgrk Thank you for pointing it out, the border color was not properly adjusting with the dark mode, fixed it in this commit.

@romgrk
Copy link
Contributor

romgrk commented Jun 6, 2023

I must also say that I don't like Material Design inputs too much for this use case, from a UI/UX POV. First, the "Contains" label is bigger than the column title label. I would expect the opposite. Also the underlined input border right next to the bottom row border looks a bit off, imho. Puts a lot of emphasis/visual-weight on that region. I wonder if we wouldn't be better off by placing the filters inside the column title cell. It would also save some vertical space if we can avoid having 2 separate rows for the headers & filters.

image

The filters in their focused state have a nice feel, but their unfocused state has the problems described above.

image

@cherniavskii cherniavskii moved this from 🆕 Needs refinement to 🏗 In progress in MUI X Data Grid Jun 6, 2023
@MBilalShafi
Copy link
Member Author

MBilalShafi commented Jun 8, 2023

I wonder if we wouldn't be better off by placing the filters inside the column title cell. It would also save some vertical space if we can avoid having 2 separate rows for the headers & filters.

That's a nice idea, we discussed it multiple times previously and after discussing the pros and cons, we decided to go with the separate header row. For example: #4934 (comment), #4934 (comment)

I agree, in the current implementation, the state with no filter value doesn't look ideal, as there's an empty space on top and a prominent border near the bottom. Probably we can think about iterating the UI part a bit. Sidenote: For now, I merged the PR to deliver the changes done to the users, we can always follow up to re-iterate!

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: Filtering Related to the data grid Filtering feature umbrella For grouping multiple issues to provide a holistic view
Projects
Status: 🏗 In progress
Development

No branches or pull requests

3 participants