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

How to get every group column total #63

Closed
mrjumpy opened this issue May 19, 2015 · 34 comments
Closed

How to get every group column total #63

mrjumpy opened this issue May 19, 2015 · 34 comments

Comments

@mrjumpy
Copy link

mrjumpy commented May 19, 2015

Just like the option groupSummaryPos : 'footer'. But I want its total for group by column.
Example:

| 2011 | 2014 |
| 7 | "total" | 12 | 7 | "total"|

I want that "total". XD

@OlegKi
Copy link
Member

OlegKi commented May 19, 2015

Sorry, but I'm not sure what you mean. Do you use rowTotals: true? Do you want to have multiple "Total" columns? In the case 3 "Total" columns would be more logical: one total for every group of the level 2 (once for "7", once for "12" and "7") and once for the level 1 (2011 and 2014).

I didn't used jqPivot before. I read the code carefully and rewrote some parts short time before only to fix the bugs reported by another user. During analyzing of the code I could see many parts which I personally find not consequent.

For example, if I understand you correctly, you want to have total on every level of y-grouping. So one could define rowTotals: true for example inside of items of yDimension instead of specifying it on the level of global pivot options. Something like

yDimension: [
    { dataName: 'sellyear' }, // no grand total 
    { dataName: 'sellmonth', rowTotals: true, rowTotalsText: "Month Total" }
],
rowTotals: false, // no grand total ??? to hold it for backwards compatibility only
rowTotalsText: "Total"// used for grand total ??? to hold it for backwards compatibility only

Moreover there exist colTotals: true option which generates the footer to the grid, but there are no option to specify the aggregation function for the footer calculation. jqPivot uses currently always the 'sum' aggregation function for generation of the values of the footer. For example one can't calculate sum as the value displayed in the cells of the grid (like the value 187.62, 93.81 and 187.62 in your demo), but to use 'max' in the "Month Total" column or in "Year Total".

Finally I personally find the names colTotals and rowTotals very misunderstandable. The option rowTotals: true add new column on the right side and it uses the same aggregation function like in aggregates. The option colTotals: true add the footer row with 'sum' aggregation.

I think that one should, first of all, to extend the existing options of jqPivot. One should find clear and flexible enough new options and only after that one should make the corresponding changes in the code of jqPivot.

@mrjumpy
Copy link
Author

mrjumpy commented May 19, 2015

Yes, I want rowsTotal.
I've tried what you said.

This is the result:
http://codepen.io/anon/pen/jPrLPJ?editors=101
Still, the rowTotals not show up.
What options I missed?

@OlegKi
Copy link
Member

OlegKi commented May 19, 2015

@mrjumpy Probably there are a misunderstanding.

The options rowTotals: true, rowTotalsText for yDimension items not exist in the current implementation of jqPivot. I tried to explain you that what your want required extension of existing options of jqPivot and of cause the changing of the code of jqPivot. I tried to explain that one should first suggest clear and understandable extension of existing parameters of jqPivot. The properties rowTotals: true, rowTotalsText inside of yDimension was just an example of possible new properties.

I pointed to bad chosen existing options: colTotals and rowTotals which are misunderstandable. I will use the existing names for backwards compatibility, but I don't want to introduce more names which would be better to change.

After clearing of the requirements and choosing the names of new options one have to extend the code of jqPivot.

@mrjumpy
Copy link
Author

mrjumpy commented May 19, 2015

Ok I got it. So, In the future, we will have the options(rowTotals in yDimension) to set.
Because I need that feature recently. Which part I can start to trace the code. Or it will be released soon.

@OlegKi
Copy link
Member

OlegKi commented May 19, 2015

@mrjumpy : I suggest you to discuss what questions are opened in the requirements to jqPivot before I start any implementation. For example please reread your original first post and describe my what you need exactly. The grouping could looks like

------------------------------
|    2011   |      2014      |
------------------------------
| 7 | total | 12 | 7 | total |
------------------------------

or could be like

---------------------------------
| 2011 |       |  2014  |       |
---------------------------------
|  7   | total | 12 | 7 | total |
---------------------------------

or like

---------------------------------
| 2011 |       |  2014  |       |
-------| total |--------| total |
|  7   |       | 12 | 7 |       |
---------------------------------

in case of usage useColSpanStyle: true option of header grouping.

The next question: which place and which form is better to specify the titles? The usage of boolean rowTotals and string rowTotalsText in elements of yDimension was my fist suggestion. The options produces for example multiple columns with the same header. The above examples uses the same "total" text for 2011 and for 2014. The same problem exist in data grouping (see here). One can uses {0} and {1} inside of groupText values or to specify the value as callback function. If we would follow the same syntax then rowTotalsText: "total {0}" would generate "total 2011" and "total 2014" as header for the above case. I repeat once more that I discuss only possible options. No such options exist currently.

The next problem: The aggregation function used for Total columns. It will be calculated internally the value using the same aggregation function. So one can use it, but it can be that one need to display another totals. For example one display the number of visitors in month in pivot table, but one need to see the max of the all visitors or even the column name (the month) when it was the maximal number of visitors.

I ask you for the input because I never used jqPivot or any other pivot tables in any projects for my customers. It's interesting which input one needs.

For example, do you use aggregates with one elements or with multiple aggregates?

It was an interesting feature request which I hold in my head. One suggested to display the aggregation functions as dropdown (see here). The user could choose the aggregation function and grouping (or pivot in our case) would calculate and to display the required results. It would allows better to analyse the input data with respect of pivot.

One more opened question I asked you before. There are exist colTotals option which allows to add the footer row in the grid. The current implementation use always the 'sum' aggregation and there are no option to specify the aggregation function. Even if one would choose for example aggregator : 'max' then the footer row will display the sum of max values which have not much sense.

Do you use colTotals option? Which new options would be good to introduce in the future to have more flexibility in generation of the footer row?

Which callbacks would be be helpful in jqPivot? One can for example add the callback which would be called after the data, colModel and grouping headers of column are generated by pivotSetup, but before the grid or the grouping headers are created. It would allow to modify the column properties (for example to insert a column or to modify some headers) before the grid will be created. Having the callback you would be able to implement your current requirements by writing some code for the callback.

I repeat once more the first sentence of my post: I suggest you to discuss what questions are opened in the requirements to jqPivot before I start any implementation.

Best regards
Oleg

@OlegKi
Copy link
Member

OlegKi commented May 19, 2015

Hi!

After posting the last comment I can't stop thinking about the problems. After some time I start changing the code and now I already implemented all described above features. I described the features shortly in the comment to the commit. The features works in all the tests which I made. Please try the features and report bugs if you would find someone.

Best regards
Oleg

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

I'm surprising your effective performance.
It's really fantastic.

  1. Bug fix - It's OK.
  2. useColSpanStyle - This is great. But, it's too bad that only support 2 level. I need multiple level.
  3. colTotalAggregator - No matter how I tried, I can't trigger this feature. Is there a example?
  4. items of yDimension can have now two additional properties - I think there are bugs.
    Example and exception is here:
    http://codepen.io/anon/pen/jPrLPJ?editors=101

The fourth options is really that I want it.

And, reply your question.
"For example, do you use aggregates with one elements or with multiple aggregates"?
Yes, I'm using multiple aggregates and multiple levels.
maybe I'll show you my example later.

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

@mrjumpy : Thank you for the feedback. I will analyse the problem later in details. First of all small fix. Please retry with new sources.

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

It's work.
Thank you!
And,Did you see my pull request?

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

@mrjumpy : Try the demo which is small modification of youth. Three other demos demonstrates the usage of new features and provides an example with more as one aggregations:
demo1
demo2
demo3

I have seen your pull request. Thank you very much! I have some other important things to do now. I will look at the pull request later.

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

Looks great!!
Can I use these demo to our documents?

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

Yes of cause. You can use the demos or modify there in any way. I recommend you to include Font-Awesome CSS and to add iconSet: "fontAwesome" to jqGrid options. Moreover I personally use

cmTemplate: { autoResizable: true },
shrinkToFit: false,
autoresizeOnLoad: true,
autoResizing: { compact: true },

in all the demos to reduce the width of all columns based on the width of content.

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

I'm found a bug.
When I change aggregate to multiple aggregates, it will look like:
http://codepen.io/mrjumpy/pen/YXWmrE
The month dimension are disappeared.

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

Sorry, but I'm not sure what are expected results. You use fixed label in aggregates (label: 'avg' and label: 'Price2'). If you remove the properties then the month will be shown. I could modify the code and allow to use templates of callbacks in label of aggregates in the same way like one can use there in rowTotalsText. Is it what you need? Please describe more exactly the expected results.

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

I want to sketch a table just like you did , but I can't find out where can I edit.
Wait a moment,please.

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

Try to use rowTotalsText: "{0} {1}" in yDimension, remove label from aggregates and use rowTotalsText: "Total {0}" on the grid level (near frozenStaticCols: true). You can improve the text if you would use rowTotalsText of yDimension defined as function. Just try to use rowTotalsText: "{0} {1} {3}". The last parameter in the index in aggregates array. One can get the member and display it for example.

I don't understand what you mean with fancyAlert.

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

Just try to use rowTotalsText: "{0} {1} {3}" in yDimension. The last parameter in the index in aggregates array. One can get the member and display it for example. You can improve the text if you would use rowTotalsText of yDimension defined as function.

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

I tried what you said.
But, I think this is the correct pivot:
https://docs.google.com/spreadsheets/d/16fp6K8k6mAFzELwANls_aiuJXCXyE3YfieFWkBwgs2M/edit?
usp=sharing

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

Do you mean that one should create one more grouping level and to place the results of different aggregations on the level below?

@OlegKi
Copy link
Member

OlegKi commented May 20, 2015

Before I made more changes and add generation of one more grouping level I posted the changes. See the demo. Be careful that I changed a little the values of parameters in the callback function and the parameters of the template.

You can use try to use label: '{2} {1} {0}' in aggregates and rowTotalsText: "Grand Total {1} {0}" for grand total and see the possibilities of customization of the header text.

@mrjumpy
Copy link
Author

mrjumpy commented May 20, 2015

Yes, you're right.
Pivot table usually grouping level like the links which is providing previously.

@mrjumpy
Copy link
Author

mrjumpy commented May 25, 2015

I'm testing some options.
And I'm sure this is a bug.
I'm using two options to compare each other.
The only difference is in aggregates.
The problem is that row month disappear.
http://codepen.io/mrjumpy/pen/RPRXjM?editors=101
Any suggestion?
Or I can fix this by myself?

@OlegKi
Copy link
Member

OlegKi commented May 25, 2015

Could you describe more exactly the problem? I opened the demo, I started to analyse the input data and the results, but I found later that the demo displays two pivots: the second grid will be displayed below. So: could you describe more exactly the problem?

@mrjumpy
Copy link
Author

mrjumpy commented May 25, 2015

Sorry for my abstract description.
check this again, please.
http://codepen.io/mrjumpy/pen/RPRXjM?editors=101

Pivot3 is my expected view. It's correct.
Pivot1 is not.

And, look at pivot1 and pivot3 yDimension.
pivot1's yDimension option is expected setting. since pivot3 is not.

That is the problem.
When I adding more aggregates. I'll expect that the pivot seem pivot3 instead of pivot1.

@OlegKi
Copy link
Member

OlegKi commented May 25, 2015

Thank you! I will analyse the problem and will write you later. I want only to post you small common remark about the demos which you use. You made the same errors in all previous demos, which are not important to the described problem, but there are still incorrect:

  1. You should use //rawgit.com/free-jqgrid/jqGrid/master/css/ui.jqgrid.css and not the old ui.jqgrid.css from free jqGrid 4.8 if you use the latest version of jquery.jqgrid.src.js from //rawgit.com/free-jqgrid/jqGrid/master/js/jquery.jqgrid.src.js.
  2. You should include one jQuery UI theme. You demos includes two theme from different jQuery UI versions: redmond theme from jQuery UI 1.11.2 and black-tie theme from jQuery UI 1.11.4. It's wrong. You should remove the line with //ajax.googleapis.com/ajax/libs/jqueryui/1.11.2/themes/redmond/jquery-ui.min.cs or just replace it to //ajax.googleapis.com/ajax/libs/jqueryui/1.11.4/themes/black-tie/jquery-ui.min.css and to remove the next line with https://code.jquery.com/ui/1.11.4/themes/black-tie/jquery-ui.css

I would recommend you additionally to replace the option like pager: "#basic-pgrid-pager3" to pager: true and to remove unneeded empty dives like <div id='basic-pgrid-pager3'></div>.

I will write you later about the main problem which you reported.

OlegKi added a commit that referenced this issue May 26, 2015
@OlegKi
Copy link
Member

OlegKi commented May 26, 2015

I made more changes in the code of jqPivot. I hope that the new code works like you expect.

@mrjumpy
Copy link
Author

mrjumpy commented May 26, 2015

Thank you for quick and clean bug fix.
I think the only thing that I can contribute is testing the features.
Have a look at example:
http://codepen.io/mrjumpy/pen/RPRXjM?editors=001

Pivot1 is OK, Pivot 3 have some bugs and enhancement.
Bug:
Look at pivot3, find the border that I have remark between "Hello" and "Oleg".
Try to dragging and resize it. You'll find the bugs

Enhancement:
If we have > 1 aggregates.
rowTotals: true,
rowTotalsText: "Month Total"

Maybe somebody want to rowTotalsText can be express by two different meaning.
Just like the images that I show you.
One is Hello and one is Oleg.
But I can't do that thing now.

image

@OlegKi
Copy link
Member

OlegKi commented May 26, 2015

Thank you for the bug report! I posted the corresponding fixes.

Common remark for the demos which you post. Please remove duplicate width definition and use the grid option cmTemplate: { autoResizable: true, width: 100 }, autoResizing: { compact: true } instead (near the width: 700 setting). It allows to set the column width on one place and simplify changing during tests. The option autoResizable: true allows to use double click on the column header to resize the column to the optimal width. Moreover I personally find the setting width: 700 unneeded because it makes the width of all columns too small in the most scenarios.

About the Enhancement: It's already exist in jqPivot. I wrote you before that you can use templates in label of aggregates items, in rowTotalsText of yDimension items (if rowTotals: true is set). I recommend you just to remove rowTotalsText: "Month Total" which you use in yDimension items or to replace it to rowTotalsText: "Month Total {0} {1}". Additionally you can first remove label properties from aggregates (remove label: 'Price' and label: 'Quntity') and to see the results. Then you can try to set label: '{1}' and you should to see the results which you need. You can try to use or label: '{0} {1}' or any other strings with {0}, {1} or {2} inside. You can use label or rowTotalsText as functions too.

@OlegKi
Copy link
Member

OlegKi commented May 27, 2015

Look at the last commit. The new feature rowTotalsTitle could be interesting for you.

@mrjumpy
Copy link
Author

mrjumpy commented May 27, 2015

rowTotalsTitle is fantastic! It's work very well.

After so many pivot features improved.
I really want to help for documents. Let more people use it.
But I seem that you did not receive pull request.
Is there problem that I can help you?

@OlegKi
Copy link
Member

OlegKi commented May 27, 2015

First of all you can close the issue if you don't see some clear bugs in group column total.

I plan to publish jqGrid 4.9 beta1 in the next days. Directly after that I will apply your pull request together with my changes and write some more pages of documentation and wiki articles. I plan to write more documentation and wiki articles in the next two weeks and then to release jqGrid 4.9 after fixing possible errors which will be reported.

@mrjumpy
Copy link
Author

mrjumpy commented May 27, 2015

OK!
I'll close this later.
The Last of problem about group column total:
When I'm apply useColSpanStyle: true to more aggregates.
The result:
http://codepen.io/mrjumpy/pen/RPRXjM?editors=101

@OlegKi
Copy link
Member

OlegKi commented May 27, 2015

It's not a simple problem. I wrote the method setGroupHeaders some years ago at the time for the version jqGrid 4.2.0 (see my name and the email at the beginning of the code grid.custom.js v4.4.0). The code was not oriented on calling the method multiple times like jqPivot do. It's a luck that the results are relatively correct. Even in case of usage useColSpanStyle: false the internal structure of the headers is a little strange.

One have to go though setGroupHeaders and change it to support multiple calls or to change the structure of input parameters to describe multilevel headers. After that one have to adjust probably the code of jqPivot.

Short to say the problem which you describe is not a bug. It's the restriction which exists in the current implementation. There are many other methods of jqGrid which have relatively long list of limitations. It's good to reduce the list, but all the changes required time. I want to concentrate me on publishing jqGrid 4.9 and to hold the enhancements for the next release. I suggest you to post the suggestion as separate issue to be more sure that we don't forget the new feature.

@mrjumpy mrjumpy closed this as completed May 27, 2015
@mrjumpy
Copy link
Author

mrjumpy commented May 27, 2015

OK! I got it.
Thank you for series of bugs fix.
you really help me a lot.

Thank you!!

@OlegKi OlegKi added the pivot label May 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants