- Notifications
You must be signed in to change notification settings - Fork906
Allow specifying gradients for DataBar conditional formatting#2296
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:develop
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jahav left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
API has incorrect shape. It adds an optional parameter, which is not scalable for the rest of parameters ([MS-XLSX]2.6.30 CT_DataBar has many other attributes).
The Gradient prop is onIXLConditionalFormat, where it doesn't belong. It is only for DataBar (possibly other, but only subset), not generic conditional formats.
ClosedXML uses fluent API for similar case (e.g. styles are fluentsomething.Style.Font.SetSize(14).SetBold()).
There are two interfacesIXLCFDataBarMax andIXLCFDataBarMin. Since this thing hasn't gone through API review yet, let's just addIXLCFDataBar and chain it behindIXLCFDataBarMax (currently returns void, to ensure that there are exactly two CFVO though Min and Max interface).
interface IXLCFDataBar { bool Gradient {get;} IXLCFDataBar SetGradient(bool value = true);}Tests should be ClosedXml.Tests.ConditionalFormats directory. In this case, I think jut make a new ConditionalFormatDataBarTests. Test names should beExplain_what_test_does(). One test with TestHelper.CreateSaveLoadAssert should sufficie. Test file should go to ClosedXML.Tests\Resource\Other\ConditionalFormats.
Add good descriptive XML documentation for added/modified methods/props. Add user documentation for DataBar (see /doc directory). Any modification/new feature must include documentation.
Any breaking changes (in this case unification of interfaces) should be added to docs\migrations\migrate-to-0.104.rst.
erenes commentedFeb 13, 2024
Thanks for the feedback, the direction was very helpful! I have now implemented it as a Fluent API and added a unit test. Now that I've gone this far I think it's a logical next step to get rid of the |
Fixes#386,#1686
The dataBar element below the conditionalFormat supports a gradient property as of Excel 2010, which allows a solid fill on the conditional format.
This PR allows specifying the gradient property when creating a dataBar programatically with ClosedXML, as well as support for loading and saving files that contain solid fill dataBars.
I've tried to adhere to the existing code style and ran CodeMaid on the files in the PR. I am happy to implement unit tests as well, but did not find any existing ones for the dataBar elements.
Example code: