Second revision of pivot styles API #1186 #1222
Conversation
|
This PR is needed for the release of ClosedXML.Report |
|
I'm finally looking at this PR. But it seems like you have undone many of the changes I made when we last worked on the pivot table formats. For example, now we're back to using |
|
I had to go back to the old models because the models you entered could not describe all the options for using Pivot styles. And for this reason, I had to make many interfaces open. |
|
Sorry, Francois. I again did ugly interface. If I understood what you don't like, maybe I could have done better. And it seemed to me that we agreed with you that I would make a public interface like yours, and an additional "low-level" API with lambda functions, to which you agreed. |
| var brightYellow = XLColor.FromArgb(255, 255, 153); | ||
| var gray = XLColor.FromArgb(192, 192, 192); | ||
|
|
||
| companyField.AddHeaderStyle().Style.Font.SetBold(); |
There was a problem hiding this comment.
I don't see any fields in the output file that display as bold.
|
Is it possible that you can generate a pivot table that illustrates every kind of pivot table format? If I understand the limitations of the current implementation on the |
|
At first it seemed to me that the options that you described should describe everything possible. But when I began to open and save different documents with pivot tables, I realized that it was impossible to describe everything (or it was very difficult). Therefore, I decided that it is better to return to the API that you do not like. |
ClosedXML/Excel/PivotTables/PivotStyleFormats/XLPivotStyleFormatEnums.cs
Show resolved
Hide resolved
|
|
||
| public enum XLPivotTableAxisValues | ||
| { | ||
| AxisRow = 0, |
There was a problem hiding this comment.
Same here. Unless the particular numbers have some meaning (i.e. they coincide to OpenXML values), it is better to reserve 0 to NotSet than use an arbitrary value as a default for non-assigned variables. It protects from rare unpleasant bugs.
| { } | ||
| public XLPivotValueStyleFormat(IXLPivotFormat pivotFormat) | ||
| { | ||
| PivotFormat = pivotFormat; |
There was a problem hiding this comment.
Better check the input for null here than have NRE in Style getter.
| public IXLPivotValueStyleFormat AndWith(IXLPivotField field, Predicate<Object> predicate) | ||
| { | ||
| FieldReferences.Add(new PivotLabelFieldReference(field, predicate)); | ||
| ((List<IFieldRef>)PivotFormat.FieldReferences).Add(FieldRef.ForField(field.SourceName, predicate)); |
There was a problem hiding this comment.
Better avoid typecast here. For example, in the constructor, you can accept a concrete implementation of IXLPivotFormat that allows modification of FieldReferences collection. Or define an internal interface for that. I can provide a more detailed explanation if needed.
3 participants
What's this PR do?
The refined API now allows to assign styles for:
Contains fix for #1186
Cancels #1189
Replaces #1129