mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.91k stars 1.19k forks source link

[data grid] Dynamic Aggregation Position #12085

Open Jaryt opened 4 months ago

Jaryt commented 4 months ago

The problem in depth

Hello!

I'm trying to use the aggregation position for a feature I'm working on, where when a treeData group is closed, it should show the aggregation inline, and when it's expanded, it the aggregation should move to the footer. (Furthermore, the cells are locked and blanked out in the group row, but that's irrelevant to this)

Unfortunately, it seems as if doing this isn't super clean. First off, when a group expands, the getAggregationPosition does not seem to refresh for that group. I had to work around this by setting state to get the grid to react to those changes. It's a bit hacky in that demo, but I control this in my project in a more clean way since I need to know those expanded groups.

So as I'd expect due to the order in which the rerender is triggered, there's a noticeable bit of pop in. Also I've noticed in my project that there is a bit of clipping with the footer row while all of the heights are updated. I don't seem to experience that in this demo environment, so that might be chalked up to a performance issue, or the fact that we're on 5.17.

It would be great if this sort of transitional aggregation position could be natively supported!

Code_1UGrnJ8AvG

P.s., We're using DataGrid Premium heavily in our web app, in a lot of different use cases. I would love to arrange a chat with someone from your dev team to show how we're using it, and possibly get tips on what we could be doing better. :)

Thanks!

Your environment

Built from the sample: https://stackblitz.com/edit/react-rpklny?file=Demo.tsx,index.html

Using Material UI Premium 6.19

Search keywords: Dynamic Aggregation Position Order ID: 83110

michelengelen commented 4 months ago

Hey @Jaryt and thanks for using the datagrid!

Could you provide us with a live example of your problem? It seems like the mentioned rendering delays are not prevelant on our demo (and on my local instance) as you are experiencing them. Maybe this is related to the version being 5.17 (we did quite a lot of improvements lately).

Thanks again! πŸ™‡πŸΌ

P.s., We're using DataGrid Premium heavily in our web app, in a lot of different use cases. I would love to arrange a chat with someone from your dev team to show how we're using it, and possibly get tips on what we could be doing better. :)

@cherniavskii would you be up for a chat on this?

Jaryt commented 4 months ago

Hey @Jaryt and thanks for using the datagrid!

Could you provide us with a live example of your problem? It seems like the mentioned rendering delays are not prevelant on our demo (and on my local instance) as you are experiencing them. Maybe this is related to the version being 5.17 (we did quite a lot of improvements lately).

Thanks again! πŸ™‡πŸΌ

P.s., We're using DataGrid Premium heavily in our web app, in a lot of different use cases. I would love to arrange a chat with someone from your dev team to show how we're using it, and possibly get tips on what we could be doing better. :)

@cherniavskii would you be up for a chat on this?

Yeah, I believe you might be right about it being due to the version. We've tried migrating over but hit some issues with the flexing of the grid changing, which required a lot of restructuring of the parent components. I'll be trying to migrate again when possible.

The key point here is probably about the aggegationRowPosition not being reevaluated when the expansion of the row changes. I think that would be helpful so that we don't have to manually handle the re-rendering there.

michelengelen commented 4 months ago

Yeah, I believe you might be right about it being due to the version. We've tried migrating over but hit some issues with the flexing of the grid changing, which required a lot of restructuring of the parent components. I'll be trying to migrate again when possible.

If you need help migrating please feel free to reach out. πŸ’ͺ🏼

The key point here is probably about the aggegationRowPosition not being reevaluated when the expansion of the row changes. I think that would be helpful so that we don't have to manually handle the re-rendering there.

We can definitely take a look at that. @cherniavskii should we put this on board for us, so we don't forget about it?

cherniavskii commented 2 months ago

Hi @Jaryt Sorry for the late reply, I missed the notification on this issue in my notifications.

The workaround for the dynamic getAggregationPosition could be as simple as this:

const apiRef = useGridApiRef();

// workaround to get the grid to re-render when the expanded rows change
const [_, rerender] = React.useState(0);
React.useEffect(() => {
  return apiRef.current.subscribeEvent('rowExpansionChange', () => {
    rerender((prev) => prev + 1);
  });
}, [apiRef]);

Here's a working demo: https://stackblitz.com/edit/react-rpklny-afqhbq?file=Demo.tsx

I think it's a decent workaround for the time being. Changing the current behavior would require a non-trivial refactor. I'm adding a "waiting for upvotes" label to see the demand for this in the community πŸ™‚

Jaryt commented 2 months ago

@cherniavskii No worries, I understand things are busy in the OSS land.

Interesting solution here, I'll give it a shot!

Thank you πŸ‘

github-actions[bot] commented 2 months ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@Jaryt: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Jaryt commented 2 months ago

Actually, I'll keep it open for now, but feel free to close if you feel it's needed