Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
erenes wants to merge4 commits intoClosedXML:develop
base:develop
Choose a base branch
Loading
fromerenes:dataBarGradient

Conversation

@erenes
Copy link

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:

varworkbook=newXLWorkbook();varws=workbook.AddWorksheet("test");ws.FirstCell().SetValue(0.53).AddConditionalFormat().DataBar(XLColor.Red,showBarOnly:false,gradient:false).Minimum(XLCFContentType.Number,0).Maximum(XLCFContentType.Number,1);workbook.SaveAs("test.xlsx");

hanatharesh2712 reacted with thumbs up emoji
Copy link
Member

@jahavjahav left a 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
Copy link
Author

Thanks for the feedback, the direction was very helpful!

I have now implemented it as a Fluent API and added a unit test.
Addtionally I've also added the breaking changes to the migrate doc, and did my best to add useful documentation (I struggled a lot to come up with anything useful for such a trivial change)

Now that I've gone this far I think it's a logical next step to get rid of theXLCFDataBarMin andXLCFDataBarMax and unify all of those calls in the newXLCFDataBar object (maybe Min can still return the Max interface, so you are nudged towards not just setting one of the two).
Let me know if you would want me to pick that up in this PR as well or to leave it as-is until there is an API review.

@ereneserenes requested a review fromjahavFebruary 13, 2024 21:06
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jahavjahavAwaiting requested review from jahav

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Losing gradient filling format

2 participants

@erenes@jahav

[8]ページ先頭

©2009-2025 Movatter.jp