spreadsheet icon indicating copy to clipboard operation
spreadsheet copied to clipboard

Redo Conditional Formatting for #461

Open WoozyG opened this issue 6 years ago • 4 comments

between POI updates and this new attempt at pushing my local conditional formatting work, I have much more properly supported. Note however I've identified at least one POI regression (which I apparently introduced), noted below.

Curious to see how the automated tests run for this, looks good on my end.

Now, cells where a conditional rule applies, and that rule has a number format different than the original cell format, display using the conditional format.

Also addresses the list of issues identified in bug #461 and bug #651. This incorporates work done in #640 but never finished and merged.

Having this in Spreadsheet master means I can next send a PR for bug #529, adding support for table styles, both built-in and custom themed.

The Excel official sample file showing how to use conditional formatting, which is included in the unit tests, now passes for more sheets. I see excellent equivalence except for:

Features Spreadsheet doesn't support yet:

  • icon sets (sheet Quarters, Bike Rating, FY Months, Regional Sales)
  • data bars (Mountains)
  • gradients (Category Sales)

Likely POI bugs in conditional formatting evaluation when rule formulas have relative cell references (I can reproduce the first few I tried in POI unit tests):

  • something goes wrong with styles on Compare to Totals. This works on older POI versions, so I broke it in 4.0 somehow. I'll fix it. We are probably about to a patch release there anyway. Then Spreadsheet 3.0 can use the patched POI as its base version.

  • Style not updating when values change to match conditional rules on Products3

  • all rows show as matching on Customers2, instead of only unique rows. Could be a formula evaluation bug, or Conditional Formatting evaluation bug.


This change is Reviewable

WoozyG avatar Mar 15 '19 19:03 WoozyG

Yes, these 3 sheets are all the same bug, due to a math error I made in POI.

I've fixed POI bug #63264 in trunk. It will be fixed in the next dot release. Don't have a date for that yet, but if Vaadin wants it, I can start the process to get one out in the next few weeks.

WoozyG avatar Mar 16 '19 00:03 WoozyG

There will be a new POI release, 4.1, in the next couple weeks. We should take that. It includes my fixes for conditional formatting, my implementation of SUBTOTAL() variations that skip hidden rows, which trips up a lot of sample workbooks, and maybe most important, some fixes for working with POI in deployment environments with JDK 9+. Those are mostly in the underlying XMLBeans project POI has taken over, which is what is used to parse and manipulate the OOXML in XLSX files.

I think I'll be the release manager for that, so I'll update this PR with a dependency update when it is publicly available.

WoozyG avatar Mar 18 '19 23:03 WoozyG

Released POI 4.1.0, will be available in the next day or two. At that point I'll commit a dependency update and we'll see where we are with these changes.

WoozyG avatar Apr 08 '19 23:04 WoozyG

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Apr 11 '22 10:04 CLAassistant