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

[DataGridPremium] Support aggregating data from multiple row fields #6656

Merged
merged 15 commits into from
Nov 14, 2022

Conversation

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user feature: Aggregation Related to the data grid Aggregation feature enhancement This is not a bug, nor a new feature labels Oct 28, 2022
@mui-bot
Copy link

mui-bot commented Oct 28, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-6656--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 607 964.7 623.3 731.18 145.112
Sort 100k rows ms 559.3 1,135.6 717.1 831.78 193.015
Select 100k rows ms 211.6 309.1 235.9 255.22 36.561
Deselect 100k rows ms 135 262 207.3 195.78 47.518

Generated by 🚫 dangerJS against e59cb4c

Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

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

@@ -58,6 +65,13 @@ export interface GridAggregationFunction<V = any, AV = V, FAV = AV> {
* @default `true`
*/
hasCellUnit?: boolean;
/**
* Function that allows to redefine the value of the cells used to generate the aggregated value.
* Useful for multi-column aggregation when aggregated value is generated from multiple columns.
Copy link
Member

Choose a reason for hiding this comment

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

I would avoid to call this "multi-column aggregation", it took me some time to understand that it was one column with aggregated value depending on other columns not just the data of the current column.

For me "multi-column aggregation" is just having two or more items in the aggregation model (like multi filtering or multi sorting)

Copy link
Contributor

Choose a reason for hiding this comment

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

what about something like this?

Screen.Recording.-.Made.with.FlexClip.1.webm

Demo : https://codesandbox.io/s/wonderful-david-px9cjr?file=/demo.tsx

Copy link
Member Author

Choose a reason for hiding this comment

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

@flaviendelangle

For me "multi-column aggregation" is just having two or more items in the aggregation model (like multi filtering or multi sorting)

Yeah, I see the confusion here. Especially because in the demo I've added in this PR there are multiple columns aggregated as well 🙃
Maybe title it as "Aggregating value from multiple fields of the rows"? What do you think?

I think it's worth taking a look at what AG-Grid is doing
https://www.ag-grid.com/javascript-data-grid/aggregation-custom-functions/#custom-full-row-aggregation

Hmm, I don't think I understand the relation to this PR.
What I do in this PR is covered in AG Grid here: https://www.ag-grid.com/javascript-data-grid/aggregation-custom-functions/#multi-column-aggregation

Copy link
Member Author

Choose a reason for hiding this comment

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

what about something like this?

Screen.Recording.-.Made.with.FlexClip.1.webm
Demo : codesandbox.io/s/wonderful-david-px9cjr?file=/demo.tsx

Hey @yaredtsy
Could you explain what problem you are addressing in this demo?
This doesn't seem to be related to aggregation, but maybe I'm missing something?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe title it as "Aggregating value from multiple fields of the rows"? What do you think?

Fine for me !

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @cherniavskii
I was looking at the AGgrid implementation and they support string formulas for valueGetter like 'a+b'. and I thought this would be a nice feature if it was implemented. It can also be extended for users to do the computation to add or update columns from the UI like excel. i might be off topic

Copy link
Member Author

Choose a reason for hiding this comment

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

I was looking at the AGgrid implementation and they support string formulas for valueGetter like 'a+b'

Yes, but it doesn't seem to be related to aggregation though.
We can discuss this in a separate issue if the community requests this.

@cherniavskii cherniavskii changed the title [DataGridPremium] Support multi-column aggregation [DataGridPremium] Support aggregating data from multiple row fields Nov 1, 2022
@cherniavskii cherniavskii marked this pull request as ready for review November 1, 2022 21:29
@gavbrennan
Copy link
Contributor

@m4theushw @DanailH @flaviendelangle Is this likely to make it into the next weekly release?

@gavbrennan
Copy link
Contributor

Any update on getting this into the main build?

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: Aggregation Related to the data grid Aggregation feature plan: Premium Impact at least one Premium user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How can I define custom aggregation in dataGridPro
5 participants