-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[data grid] Improve column menu #4929
Comments
I liked the proposal. Some menu items clearly can be grouped, like those for sorting and column pinning, others I don't know if it's worth. We could offer both versions and let users configure how they should be rendered. We could have: <DataGrid componentsProps={{ columnMenu: { condensed: true } }} /> Then, each menu item component, e.g. <DataGrid componentsProps={{ columnMenu: { layout: "submenu" } }} /> // "vertical" (default) | "horizontal" The <GridColumnMenuContainer
hideMenu={hideMenu}
currentColumn={currentColumn}
ownerState={{ color }}
{...other}
>
<SortGridMenuItems onClick={hideMenu} column={currentColumn} layout="horizontal" />
<GridFilterMenuItem onClick={hideMenu} column={currentColumn} layout="vertical" />
<GridColumnPinningMenuItems onClick={hideMenu} column={currentColumn} layout="submenu" />
</GridColumnMenuContainer> #3571 also contains some benchmarks. |
Jumping off from the prototype above, here are some additional explorations: Some notes and open questions:
Fuller exploration in Figma. |
My personal favorite is the most compact version displaying active options with the accent color. (Edit:) but the bolder black avoid some problems in dark mode. It might be a more elegant option.
We have discussed that a couple of times, Perhaps we can solve this for v6.
The full word is definitely clearer, but after the first couple of usages, the concepts are probably fully understood and the space the full word takes might become more of a nuisance.
Same argument as before: Adding the word might be clearer at a first look, but over time it doesn't add much value.
Makes a lot of sense to me, Highlighting the operation is a nice way to provide feedback on user interactions.
Makes sense to me. Regarding the name, it might make sense to discuss at #213.
Writing a "pre-answer" yes, but it needs the team's confirmation. edit: Nice work! |
Thanks for the detailed prototype ! I think we should build this new version with customization in mind.
|
This comment was marked as off-topic.
This comment was marked as off-topic.
@cherniavskii made an important point during our meeting: @m4theushw pointed out that, in the case of filtering, it's not clear which interaction opens the filter panel. It's also not clear what to expect from the "clear" button. Will it clear the field or turn the filtering off? cc/ @gerdadesign |
There's also a couple problems with the "bold" option as well |
Regarding the point raised by @cherniavskii, after discussion @gerdadesign and I concluded that it makes more sense to replace the |
Personally I agree, that selected options should not be bold, but the primary color (mainly for the reasons written by @joserodolfofreitas). |
As per our recent discussion with @gerdadesign, we made a few adjustments to the colors (mostly icon colors) as per the standard theme colors and now we are mostly aligned with the theme. Thoughts on Interface:@gerdadesign suggested making the new column menu the default one and keeping the old one optional. We discussed compact, advanced, sleek, visual, etc as possible names for the new variant and simple for the old one, but they didn't seem great so we ended up calling it I also feel it's a good idea to show the new one by default, as it is more usable and intuitive. For backward compatibility, the users will be able to switch to the previous variant either by using componentsProps of interface GridColumnMenuVariant = 'default' | 'simple'
<DataGrid componentsProps={{ columnMenu: { variant: 'simple' } }} /> // variant: 'default' if not passed
By default, this prop will traverse down the tree automatically but if users wish to have the old variant partially, we could follow the solution @m4theushw suggested, by individually passing the props to each <GridColumnMenuContainer
hideMenu={hideMenu}
currentColumn={currentColumn}
ownerState={{ color }}
{...other}
>
<SortGridMenuItems onClick={hideMenu} column={currentColumn} variant="simple" /> // old variant
<GridFilterMenuItem onClick={hideMenu} column={currentColumn} /> // new variant by default
<XYZMenuItem onClick={hideMenu} column={currentColumn} /> // new variant
</GridColumnMenuContainer> @joserodolfofreitas Do you think making the new variant as default could impact some users in a negative way? (I suspect not though, as anyways it's a v6 so if even there's a catch, we can add this to the migration guide) |
As I understand it, the word So if I understand what |
No, I agree there's no reason to keep the old column menu by default, given we're developing it during v6 alpha. As for the variant name, Maybe instead of using variant names, we can consider exporting a different component to replace the default behavior using a component slot. ´´´ Does that make sense to you? |
In term of tree shaking, 2 components is better indeed (not a huge gain compared to the size of the grid though). At first glance I prefer this approach |
I see, yeah seems like <DataGrid componentsProps={{ columnMenu: { layout: 'default | simple' } }} />
or
<DataGrid componentsProps={{ columnMenu: { simple: 'true | false' } }} />
Seems a good idea and more scalable too but I feel the business logic for these two variants is almost shared and this might end up in replicated code. Also, for |
These menu components should be dump, meaning no complex inside them. If they have some logic we could create an API method abstracting it. I would keep 2 components since it requires less work than doing 1 component and always having to check which "variant" is active. |
Thanks for asking, @MBilalShafi. An example of a use case: As a developer who implemented a custom filter panel above the grid, I want to not list the Perhaps "Hiding" was a bad choice of word, I'm rephrasing it on the issue description. |
Hi, I am working on a MUI column menu options and my requirement is to remove "Manage Column" option from the menu list only for one column. could you please suggest something on this. TIA |
Hi @PayalVerma21, thank you for using MUI X Data Grid. You could achieve something like that by hiding a menu item conditionally (i.e. for a specific column) from Columns Menu. Here's an example where Note that this hides both |
Hi @MBilalShafi , Thank you so much for the quick response. I was stuck since this afternoon, I will definitely gonna try this. Thanks again :) |
Summary 💡
The list of options in the column menu is extensive, and the list is currently quite raw.
We're improving the column menu with a better look and feel.
Updated Design concept:
We need to reorganize the menu by exploring better ways to display all the possible operations with the columns.
Motivation 🔦
Desirable customizations
Highlight of selected option with an accent color or bold text or custom.The text was updated successfully, but these errors were encountered: